Merge pull request #129 from nspcc-dev/add-precommit-cb

*: extend PreBlock processing callback

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)

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

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
 	}

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
 			}
 		}

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

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

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