Skip to content
This repository has been archived by the owner on Jun 11, 2022. It is now read-only.

Improve trait bounds on blockchain entities #443

Merged
merged 7 commits into from
Feb 13, 2019
Merged

Improve trait bounds on blockchain entities #443

merged 7 commits into from
Feb 13, 2019

Conversation

mzabaluev
Copy link
Contributor

Add chain-core property trait FromStr to impose extra requirements on parsing values such as block dates from strings.

Require the error types for deserialization and string parsing to be safe for passing between threads, so that they can be boxed and contained in e.g. client::Error of network-core.

Trait work in gRPC client code to reduce repetitive bound clauses.

Mikhail Zabaluev added 3 commits February 13, 2019 21:48
It's good for Deserialize::Error to satisfy Send + Sync + 'static, as it
can then be boxed into higher level errors.

For objects passed from strings, we need to impose the similar bounds
on the error type of standard trait FromStr. To this end, define a
property trait FromStr, blanket-implemented with the new bounds.
Use utility traits defined in module chain_bounds to centralize
bounds on the chain types necessary for the client implementation.
fn from_str(s: &str) -> Result<Self, Self::Error> {
std::str::FromStr::from_str(s)
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

argh... this is very hacky... :/

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rust-lang/rust#32722 does not allow just adding a subtrait with extra bounds on FromStr::Err.

@codecov-io
Copy link

codecov-io commented Feb 13, 2019

Codecov Report

Merging #443 into master will increase coverage by 0.17%.
The diff coverage is 79.62%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #443      +/-   ##
==========================================
+ Coverage   47.62%   47.79%   +0.17%     
==========================================
  Files         123      124       +1     
  Lines       12108    12151      +43     
==========================================
+ Hits         5766     5808      +42     
- Misses       6342     6343       +1
Impacted Files Coverage Δ
chain-impl-mockchain/src/lib.rs 100% <ø> (ø) ⬆️
chain-impl-mockchain/src/block.rs 85.38% <ø> (+3.51%) ⬆️
chain-core/src/property.rs 94.66% <ø> (ø) ⬆️
chain-impl-mockchain/src/date.rs 79.62% <79.62%> (ø)
...tocol-tokio/src/network_transport/response_code.rs 54.54% <0%> (-12.13%) ⬇️
chain-impl-mockchain/src/transaction.rs 93.1% <0%> (+3.44%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9c13780...9c0dd5b. Read the comment docs.

@NicolasDP NicolasDP merged commit 084ecef into master Feb 13, 2019
@mzabaluev mzabaluev deleted the chain-bounds branch February 14, 2019 08:09
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants