Skip to content

Commit 196c419

Browse files
authored
fix: improve SignedHeader.Verify function (#1742)
Major change in this PR is the way how non-adjacent `SignedHeader`s are `Verify`ed. This was long-overdue issue. Incompatibility was introduced in go-header v0.1.1. `Verify` method was refactored and simplified. Resolves #1496 Resolves #1741 <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Introduced additional verification methods for header and commit hashes to enhance validation processes. - Added functionality to check the adjacency of header heights. - **Bug Fixes** - Improved error handling within the verification process. - **Tests** - Added new test cases for non-adjacent headers and proposer verification failures. - Updated existing test cases for better coverage and accuracy. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
1 parent a2a97f9 commit 196c419

File tree

2 files changed

+56
-36
lines changed

2 files changed

+56
-36
lines changed

types/signed_header.go

Lines changed: 35 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -29,9 +29,6 @@ func (sh *SignedHeader) IsZero() bool {
2929
}
3030

3131
var (
32-
// ErrNonAdjacentHeaders is returned when the headers are not adjacent.
33-
ErrNonAdjacentHeaders = errors.New("non-adjacent headers")
34-
3532
// ErrLastHeaderHashMismatch is returned when the last header hash doesn't match.
3633
ErrLastHeaderHashMismatch = errors.New("last header hash mismatch")
3734

@@ -48,38 +45,49 @@ func (sh *SignedHeader) Verify(untrstH *SignedHeader) error {
4845
}
4946
}
5047

51-
if sh.Height()+1 < untrstH.Height() {
52-
return &header.VerifyError{
53-
Reason: fmt.Errorf("%w: untrusted %d, trusted %d",
54-
ErrNonAdjacentHeaders,
55-
untrstH.Height(),
56-
sh.Height(),
57-
),
58-
SoftFailure: true,
48+
if sh.isAdjacent(untrstH) {
49+
if err := sh.verifyHeaderHash(untrstH); err != nil {
50+
return err
51+
}
52+
if err := sh.verifyCommitHash(untrstH); err != nil {
53+
return err
5954
}
6055
}
6156

62-
sHHash := sh.Header.Hash()
63-
if !bytes.Equal(untrstH.LastHeaderHash[:], sHHash) {
64-
return &header.VerifyError{
65-
Reason: fmt.Errorf("%w: expected %v, but got %v",
66-
ErrLastHeaderHashMismatch,
67-
untrstH.LastHeaderHash[:], sHHash,
68-
),
69-
}
57+
return nil
58+
}
59+
60+
// verifyHeaderHash verifies the header hash.
61+
func (sh *SignedHeader) verifyHeaderHash(untrstH *SignedHeader) error {
62+
hash := sh.Hash()
63+
if !bytes.Equal(hash, untrstH.LastHeader()) {
64+
return sh.newVerifyError(ErrLastHeaderHashMismatch, hash, untrstH.LastHeader())
7065
}
71-
sHLastCommitHash := sh.Commit.GetCommitHash(&untrstH.Header, sh.ProposerAddress)
72-
if !bytes.Equal(untrstH.LastCommitHash[:], sHLastCommitHash) {
73-
return &header.VerifyError{
74-
Reason: fmt.Errorf("%w: expected %v, but got %v",
75-
ErrLastCommitHashMismatch,
76-
untrstH.LastCommitHash[:], sHHash,
77-
),
78-
}
66+
return nil
67+
}
68+
69+
// verifyCommitHash verifies the commit hash.
70+
func (sh *SignedHeader) verifyCommitHash(untrstH *SignedHeader) error {
71+
expectedCommitHash := sh.Commit.GetCommitHash(&untrstH.Header, sh.ProposerAddress)
72+
if !bytes.Equal(expectedCommitHash, untrstH.LastCommitHash) {
73+
return sh.newVerifyError(ErrLastCommitHashMismatch, expectedCommitHash, untrstH.LastCommitHash)
7974
}
75+
8076
return nil
8177
}
8278

79+
// isAdjacent checks if the height of headers is adjacent.
80+
func (sh *SignedHeader) isAdjacent(untrstH *SignedHeader) bool {
81+
return sh.Height()+1 == untrstH.Height()
82+
}
83+
84+
// newVerifyError creates and returns a new error verification.
85+
func (sh *SignedHeader) newVerifyError(err error, expected, got []byte) *header.VerifyError {
86+
return &header.VerifyError{
87+
Reason: fmt.Errorf("verification error at height %d: %w: expected %X, but got %X", sh.Height(), err, expected, got),
88+
}
89+
}
90+
8391
var (
8492
// ErrAggregatorSetHashMismatch is returned when the aggregator set hash
8593
// in the signed header doesn't match the hash of the validator set.

types/signed_header_test.go

Lines changed: 21 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -32,14 +32,14 @@ func testVerify(t *testing.T, trusted *SignedHeader, untrustedAdj *SignedHeader,
3232
prepare func() (*SignedHeader, bool) // Function to prepare the test case
3333
err error // Expected error
3434
}{
35-
// 1. Test valid
35+
// 0. Test valid
3636
// Verify valid adjacent headers
3737
// Expect success
3838
{
3939
prepare: func() (*SignedHeader, bool) { return untrustedAdj, false },
4040
err: nil,
4141
},
42-
// 2. Test invalid LastHeaderHash link
42+
// 1. Test invalid LastHeaderHash link
4343
// break the LastHeaderHash link between the trusted and untrusted header
4444
// Expect failure
4545
{
@@ -52,7 +52,7 @@ func testVerify(t *testing.T, trusted *SignedHeader, untrustedAdj *SignedHeader,
5252
Reason: ErrLastHeaderHashMismatch,
5353
},
5454
},
55-
// 3. Test LastCommitHash link between trusted and untrusted header
55+
// 2. Test LastCommitHash link between trusted and untrusted header
5656
// break the LastCommitHash link between the trusted and untrusted header
5757
// Expect failure
5858
{
@@ -65,27 +65,39 @@ func testVerify(t *testing.T, trusted *SignedHeader, untrustedAdj *SignedHeader,
6565
Reason: ErrLastCommitHashMismatch,
6666
},
6767
},
68-
// 4. Test non-adjacent
69-
// increments the BaseHeader.Height so it's unexpected
70-
// Expect failure
68+
// 3. Test non-adjacent
69+
// increments the BaseHeader.Height so it's headers are non-adjacent
70+
// Expect success
7171
{
7272
prepare: func() (*SignedHeader, bool) {
7373
// Checks for non-adjacency
7474
untrusted := *untrustedAdj
7575
untrusted.Header.BaseHeader.Height++
7676
return &untrusted, true
7777
},
78+
err: nil,
79+
},
80+
// 4. Test proposer verification
81+
// changes the proposed address to a random address
82+
// Expect failure
83+
{
84+
prepare: func() (*SignedHeader, bool) {
85+
untrusted := *untrustedAdj
86+
untrusted.Header.ProposerAddress = GetRandomBytes(32)
87+
return &untrusted, true
88+
},
7889
err: &header.VerifyError{
79-
Reason: ErrNonAdjacentHeaders,
90+
Reason: ErrProposerVerificationFailed,
8091
},
8192
},
82-
// 5. Test proposer verification
83-
// changes the proposed address to a random address
93+
// 5. Test proposer verification for non-adjacent headers
94+
// changes the proposed address to a random address and updates height
8495
// Expect failure
8596
{
8697
prepare: func() (*SignedHeader, bool) {
8798
untrusted := *untrustedAdj
8899
untrusted.Header.ProposerAddress = GetRandomBytes(32)
100+
untrusted.BaseHeader.Height++
89101
return &untrusted, true
90102
},
91103
err: &header.VerifyError{

0 commit comments

Comments
 (0)