MockHeader and test_is_within_trust_period #104
Conversation
I would definitely recommend using traits for these sorts of interfaces and making your mocks a struct which impls the trait you'd like to mock. Libra pervasively uses this pattern, and the other added benefit is it makes the code more easily (re)composable. |
|
@tarcieri we're already using traits for everything. The issue here is that mocking the trait would be too involved, because we need to mock a I think I solved it here: #105. The solution was to make one trait (ValidatorSet) an associated type of the other (Commit). That eliminates the need for the |
* remove Validator trait; make ValidatorSet associated to Commit * remove stale comment
* Add bisection to light client module: - implement `can_trust` according to the latest spec CanTrustBisection (wip) - add a `Requester` and a `Store` trait - sue refs in parameters (as they are used in several places and we don't want to Copy) * Group params h1 & ha_next_vals into a type `TrustedState` - reduce params to clippy threshold (7); still a lot ... - rename vars for better readability * Add VerifyHeader logic from spec: - rename expired to is_within_trust_period to be closer to the spec - use TrusteState trait in check_support params - use TrusteState in lite tests * Review: doc improvements and minor improvements on naming * Review: doc improvements * More doc improvements * Minor doc improvement * MockHeader and test_is_within_trust_period (#104) * MockHeader and test_is_within_trust_period * remove Validator trait; make ValidatorSet associated to Commit (#105) * remove Validator trait; make ValidatorSet associated to Commit * remove stale comment * Fix clippy errors (#108) * Review comments: (#107) * Review comments: - update some comments / documentation - verify vals (not next vals) - store header and vals in trusted state * One offs & renaming related to trusted state * Rename & remove redundant store of trusted state: c - an_trust -> can_trust_bisection - remove a redundant check for trust period - remove redundant adding state to the store (added TODO) * Bucky/some light follow up (#109) * use ? instead of return Err(err) * remove unused errors * move a validation and add TODOs about them * check_support returns early for sequential case * drop let _ = * add comment to voting_power_in about signers Co-authored-by: Ethan Buchman <ethan@coinculture.info>
Initial mock and test for the Header and
is_within_trust_period.I hesitated on mocking the Commit because it ultimately requires Validator, which has a
verify_signaturemethod, which I think we'd like to avoid needing to test this core logic (ie. #96 (comment)).We should consider some more changes to the traits to eliminate the need for this. It will make mocking everything much easier!