Add Verifier::verify_misbehaviour_header for verifying headers coming from a misbehaviour evidence
#1300
Conversation
…ng from a misbehaviour evidence
Codecov Report
@@ Coverage Diff @@
## main #1300 +/- ##
=======================================
- Coverage 64.3% 64.2% -0.1%
=======================================
Files 271 271
Lines 24450 24484 +34
=======================================
+ Hits 15723 15725 +2
- Misses 8727 8759 +32
... and 5 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
| ) -> Verdict { | ||
| ensure_verdict_success!(self.verify_validator_sets(&untrusted)); | ||
| ensure_verdict_success!( | ||
| self.validate_misbehaviour_against_trusted(&untrusted, &trusted, options, now) |
There was a problem hiding this comment.
I think it would be clearer to use self.validate_against_trusted() directly to see the correspondence with the verify() implementation
| @@ -51,32 +51,44 @@ impl From<Result<(), VerificationError>> for Verdict { | |||
| /// - [TMBC-VAL-CONTAINS-CORR.1] | |||
| /// - [TMBC-VAL-COMMIT.1] | |||
| pub trait Verifier: Send + Sync { | |||
| /// Perform the verification. | |||
| /// Verify a header received in a `MsgUpdateClient`. | |||
| fn verify( | |||
There was a problem hiding this comment.
Perhaps verify_update_header() would be a more descriptive name?
| ensure_verdict_success!(self.verify_validator_sets(&untrusted)); | ||
| ensure_verdict_success!(self.validate_against_trusted(&untrusted, &trusted, options, now)); | ||
| ensure_verdict_success!(self.check_header_is_from_past(&untrusted, options, now)); | ||
| ensure_verdict_success!(self.verify_commit_against_trusted(&untrusted, &trusted, options)); | ||
| ensure_verdict_success!(self.verify_commit(&untrusted)); |
There was a problem hiding this comment.
Since this implementation is a subset of verify_misbehaviour_header(), maybe we could call verify_misbehaviour_header(), and then check_header_is_from_past()? This would make it clear that there's only one additional check between an update and a misbehaviour header.
Although then it might be confusing to call a "misbehaviour" in an "update" verification? Or maybe we do some renaming? Not sure...
There was a problem hiding this comment.
I would rather keep it as is, since it's only a few lines of code and the one check that is omitted is clearly documented in the doc comment.
Closes: #1294
.changelog/