diff --git a/CHANGELOG.md b/CHANGELOG.md index b892cc72..0a6a11c1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ This document outlines major changes between releases. New features: Behaviour changes: + * adjust behaviour of ProcessPreBlock callback (#129) Improvements: * minimum required Go version is 1.22 (#122, #126) diff --git a/check.go b/check.go index 6e43404c..4109f721 100644 --- a/check.go +++ b/check.go @@ -56,20 +56,27 @@ func (d *DBFT[H]) checkPreCommit() { } if count < d.M() { - d.Logger.Debug("not enough PreCommits to create PreBlock", zap.Int("count", count)) + d.Logger.Debug("not enough PreCommits to process PreBlock", zap.Int("count", count)) return } d.preBlock = d.CreatePreBlock() - d.Logger.Info("processing PreBlock", - zap.Uint32("height", d.BlockIndex), - zap.Uint("view", uint(d.ViewNumber)), - zap.Int("tx_count", len(d.preBlock.Transactions()))) - if !d.preBlockProcessed { + d.Logger.Info("processing PreBlock", + zap.Uint32("height", d.BlockIndex), + zap.Uint("view", uint(d.ViewNumber)), + zap.Int("tx_count", len(d.preBlock.Transactions())), + zap.Int("preCommit_count", count)) + + err := d.ProcessPreBlock(d.preBlock) + if err != nil { + d.Logger.Info("can't process PreBlock, waiting for more PreCommits to be collected", + zap.Error(err), + zap.Int("count", count)) + return + } d.preBlockProcessed = true - d.ProcessPreBlock(d.preBlock) } // Require PreCommit sent by self for reliability. This condition may be removed diff --git a/config.go b/config.go index 2d32aa02..3db107a2 100644 --- a/config.go +++ b/config.go @@ -49,7 +49,7 @@ type Config[H Hash] struct { // Broadcast should broadcast payload m to the consensus nodes. Broadcast func(m ConsensusPayload[H]) // ProcessBlock is called every time new preBlock is accepted. - ProcessPreBlock func(b PreBlock[H]) + ProcessPreBlock func(b PreBlock[H]) error // ProcessBlock is called every time new block is accepted. ProcessBlock func(b Block[H]) // GetBlock should return block with hash. @@ -283,7 +283,7 @@ func WithProcessBlock[H Hash](f func(b Block[H])) func(config *Config[H]) { } // WithProcessPreBlock sets ProcessPreBlock. -func WithProcessPreBlock[H Hash](f func(b PreBlock[H])) func(config *Config[H]) { +func WithProcessPreBlock[H Hash](f func(b PreBlock[H]) error) func(config *Config[H]) { return func(cfg *Config[H]) { cfg.ProcessPreBlock = f } diff --git a/context.go b/context.go index 71d4cf02..c580349c 100644 --- a/context.go +++ b/context.go @@ -370,16 +370,11 @@ func (c *Context[H]) MakeHeader() Block[H] { if !c.RequestSentOrReceived() { return nil } - // For anti-MEV dBFT extension it's important to have at least M PreCommits received - // because PrepareRequest is not enough to construct proper block. + // For anti-MEV dBFT extension it's important to have PreBlock processed and + // all envelopes decrypted, because a single PrepareRequest is not enough to + // construct proper Block. if c.isAntiMEVExtensionEnabled() { - var count int - for _, preCommit := range c.PreCommitPayloads { - if preCommit != nil && preCommit.ViewNumber() == c.ViewNumber { - count++ - } - } - if count < c.M() { + if !c.preBlockProcessed { return nil } } diff --git a/dbft.go b/dbft.go index 995d5da7..2ce20dc8 100644 --- a/dbft.go +++ b/dbft.go @@ -61,6 +61,8 @@ func (d *DBFT[H]) addTransaction(tx Transaction[H]) { return } + d.verifyPreCommitPayloadsAgainstPreBlock() + d.extendTimer(2) d.sendPrepareResponse() d.checkPrepare() @@ -391,24 +393,34 @@ func (d *DBFT[H]) updateExistingPayloads(msg ConsensusPayload[H]) { } if d.isAntiMEVExtensionEnabled() { - for i, m := range d.PreCommitPayloads { - if m != nil && m.ViewNumber() == d.ViewNumber { - if preHeader := d.MakePreHeader(); preHeader != nil { - pub := d.Validators[m.ValidatorIndex()] - if err := preHeader.Verify(pub, m.GetPreCommit().Data()); err != nil { - d.PreCommitPayloads[i] = nil - d.Logger.Warn("can't validate preCommit data", - zap.Error(err)) - } - } - } - } + d.verifyPreCommitPayloadsAgainstPreBlock() // Commits can't be verified, we have no idea what's the header. } else { d.verifyCommitPayloadsAgainstHeader() } } +// verifyPreCommitPayloadsAgainstPreBlock performs verification of PreCommit payloads +// against generated PreBlock. +func (d *DBFT[H]) verifyPreCommitPayloadsAgainstPreBlock() { + if !d.hasAllTransactions() { + return + } + for i, m := range d.PreCommitPayloads { + if m != nil && m.ViewNumber() == d.ViewNumber { + if preBlock := d.CreatePreBlock(); preBlock != nil { + pub := d.Validators[m.ValidatorIndex()] + if err := preBlock.Verify(pub, m.GetPreCommit().Data()); err != nil { + d.PreCommitPayloads[i] = nil + d.Logger.Warn("PreCommit verification failed", + zap.Uint16("from", m.ValidatorIndex()), + zap.Error(err)) + } + } + } + } +} + // verifyCommitPayloadsAgainstHeader performs verification of commit payloads // against generated header. func (d *DBFT[H]) verifyCommitPayloadsAgainstHeader() { @@ -532,10 +544,13 @@ func (d *DBFT[H]) onPreCommit(msg ConsensusPayload[H]) { d.Logger.Info("received PreCommit", zap.Uint("validator", uint(msg.ValidatorIndex()))) d.extendTimer(4) - preHeader := d.MakePreHeader() - if preHeader != nil { + if !d.hasAllTransactions() { + return + } + preBlock := d.CreatePreBlock() + if preBlock != nil { pub := d.Validators[msg.ValidatorIndex()] - if err := preHeader.Verify(pub, msg.GetPreCommit().Data()); err == nil { + if err := preBlock.Verify(pub, msg.GetPreCommit().Data()); err == nil { d.checkPreCommit() } else { d.PreCommitPayloads[msg.ValidatorIndex()] = nil diff --git a/dbft_test.go b/dbft_test.go index 75f003ea..da113e80 100644 --- a/dbft_test.go +++ b/dbft_test.go @@ -1177,7 +1177,10 @@ func (s *testState) getAMEVOptions() []func(*dbft.Config[crypto.Uint256]) { dbft.WithNewCommit[crypto.Uint256](consensus.NewAMEVCommit), dbft.WithNewPreBlockFromContext[crypto.Uint256](newPreBlockFromContext), dbft.WithNewBlockFromContext[crypto.Uint256](newAMEVBlockFromContext), - dbft.WithProcessPreBlock(func(b dbft.PreBlock[crypto.Uint256]) { s.preBlocks = append(s.preBlocks, b) }), + dbft.WithProcessPreBlock(func(b dbft.PreBlock[crypto.Uint256]) error { + s.preBlocks = append(s.preBlocks, b) + return nil + }), ) return opts diff --git a/pre_block.go b/pre_block.go index 4b8a476c..a1097005 100644 --- a/pre_block.go +++ b/pre_block.go @@ -12,7 +12,8 @@ type PreBlock[H Hash] interface { SetData(key PrivateKey) error // Verify checks if data related to PreCommit phase is correct. This method is // refined on PreBlock rather than on PreCommit message since PreBlock itself is - // required for PreCommit's data verification. + // required for PreCommit's data verification. It's guaranteed that all + // proposed transactions are collected by the moment of call to Verify. Verify(key PublicKey, data []byte) error // Transactions returns PreBlock's transaction list. This list may be different