Skip to content

Commit

Permalink
dbft: add commit verification callback
Browse files Browse the repository at this point in the history
Signed-off-by: Anna Shaleva <[email protected]>
  • Loading branch information
AnnaShaleva committed Nov 27, 2024
1 parent 815155a commit f77b768
Show file tree
Hide file tree
Showing 4 changed files with 29 additions and 0 deletions.
13 changes: 13 additions & 0 deletions config.go
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,10 @@ type Config[H Hash] struct {
// Note that PreBlock-dependent PreCommit verification should be performed inside PreBlock.Verify
// callback.
VerifyPreCommit func(p ConsensusPayload[H]) error
// VerifyCommit performs external Commit verification and returns nil if it's successful.
// Note that Block-dependent Commit verification should be performed inside Block.Verify
// callback.
VerifyCommit func(p ConsensusPayload[H]) error
}

const defaultSecondsPerBlock = time.Second * 15
Expand Down Expand Up @@ -151,6 +155,8 @@ func checkConfig[H Hash](cfg *Config[H]) error {
return errors.New("NewRecoveryRequest is nil")
} else if cfg.NewRecoveryMessage == nil {
return errors.New("NewRecoveryMessage is nil")
} else if cfg.VerifyCommit == nil {
return errors.New("VerifyCommit is nil")
} else if cfg.AntiMEVExtensionEnablingHeight >= 0 {
if cfg.NewPreBlockFromContext == nil {
return errors.New("NewPreBlockFromContext is nil")
Expand Down Expand Up @@ -400,3 +406,10 @@ func WithVerifyPreCommit[H Hash](f func(preCommit ConsensusPayload[H]) error) fu
cfg.VerifyPreCommit = f
}
}

// WithVerifyCommit sets VerifyCommit.
func WithVerifyCommit[H Hash](f func(commit ConsensusPayload[H]) error) func(config *Config[H]) {
return func(cfg *Config[H]) {
cfg.VerifyCommit = f
}
}
6 changes: 6 additions & 0 deletions dbft.go
Original file line number Diff line number Diff line change
Expand Up @@ -586,6 +586,12 @@ func (d *DBFT[H]) onCommit(msg ConsensusPayload[H]) {
}
d.CommitPayloads[msg.ValidatorIndex()] = msg
if d.ViewNumber == msg.ViewNumber() {
if err := d.VerifyCommit(msg); err != nil {
d.CommitPayloads[msg.ValidatorIndex()] = nil
d.Logger.Warn("invalid Commit", zap.Uint16("from", msg.ValidatorIndex()), zap.String("error", err.Error()))
return
}

Check warning on line 593 in dbft.go

View check run for this annotation

Codecov / codecov/patch

dbft.go#L590-L593

Added lines #L590 - L593 were not covered by tests

d.Logger.Info("received Commit", zap.Uint("validator", uint(msg.ValidatorIndex())))
d.extendTimer(4)
header := d.MakeHeader()
Expand Down
9 changes: 9 additions & 0 deletions dbft_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -578,6 +578,14 @@ func TestDBFT_Invalid(t *testing.T) {
opts = append(opts, dbft.WithNewRecoveryMessage[crypto.Uint256](func() dbft.RecoveryMessage[crypto.Uint256] {
return nil
}))
t.Run("without VerifyCommit", func(t *testing.T) {
_, err := dbft.New(opts...)
require.Error(t, err)
})

opts = append(opts, dbft.WithVerifyCommit[crypto.Uint256](func(dbft.ConsensusPayload[crypto.Uint256]) error {
return nil
}))
t.Run("with all defaults", func(t *testing.T) {
d, err := dbft.New(opts...)
require.NoError(t, err)
Expand Down Expand Up @@ -1150,6 +1158,7 @@ func (s *testState) getOptions() []func(*dbft.Config[crypto.Uint256]) {
dbft.WithNewRecoveryMessage[crypto.Uint256](func() dbft.RecoveryMessage[crypto.Uint256] {
return consensus.NewRecoveryMessage(nil)
}),
dbft.WithVerifyCommit[crypto.Uint256](func(p dbft.ConsensusPayload[crypto.Uint256]) error { return nil }),
}

verify := s.verify
Expand Down
1 change: 1 addition & 0 deletions internal/consensus/consensus.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ func New(logger *zap.Logger, key dbft.PrivateKey, pub dbft.PublicKey,
dbft.WithGetValidators[crypto.Uint256](getValidators),
dbft.WithVerifyPrepareRequest[crypto.Uint256](verifyPayload),
dbft.WithVerifyPrepareResponse[crypto.Uint256](verifyPayload),
dbft.WithVerifyCommit[crypto.Uint256](verifyPayload),

dbft.WithNewBlockFromContext[crypto.Uint256](newBlockFromContext),
dbft.WithNewConsensusPayload[crypto.Uint256](defaultNewConsensusPayload),
Expand Down

0 comments on commit f77b768

Please sign in to comment.