From 45faea49e01e31948db86255d3f62a13278f2e87 Mon Sep 17 00:00:00 2001 From: Anna Shaleva Date: Mon, 2 Sep 2024 13:50:03 +0300 Subject: [PATCH 1/5] context: allow ProcessPreBlock to return an error TPKE decryption shares verification is performed in ProcessPreBlock callback when PreBlock is already constructed and all transactions are available. If some of the received shares can't decrypt Envelopes' content then dBFT should wait for more PreCommit payloads from other "honest" CNs. Signed-off-by: Anna Shaleva --- CHANGELOG.md | 1 + check.go | 19 +++++++++++++------ config.go | 4 ++-- context.go | 13 ++++--------- dbft_test.go | 5 ++++- 5 files changed, 24 insertions(+), 18 deletions(-) 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..736fce0b 100644 --- a/check.go +++ b/check.go @@ -62,14 +62,21 @@ func (d *DBFT[H]) checkPreCommit() { 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_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 From 625fb67bc7aeebf14f4d0549d0fb56e1936bbd8e Mon Sep 17 00:00:00 2001 From: Anna Shaleva Date: Mon, 2 Sep 2024 16:04:42 +0300 Subject: [PATCH 2/5] context: ensure all txs are collected before PreBlock construction It's a must-have condition for PreBlock construction. Signed-off-by: Anna Shaleva --- context.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/context.go b/context.go index c580349c..c98b688d 100644 --- a/context.go +++ b/context.go @@ -344,6 +344,9 @@ func (c *Context[H]) CreatePreBlock() PreBlock[H] { if c.preBlock = c.MakePreHeader(); c.preBlock == nil { return nil } + if !c.hasAllTransactions() { + return nil + } txx := make([]Transaction[H], len(c.TransactionHashes)) From f2e6bd26f42d51ae1d1bb5eca6c81d4c5a50bd9e Mon Sep 17 00:00:00 2001 From: Anna Shaleva Date: Mon, 2 Sep 2024 16:05:34 +0300 Subject: [PATCH 3/5] dbft: require PreBlock for PreCommit verification PreCommits can't be verified against PreHeader because PreCommit carries transactions-sensitive information. Signed-off-by: Anna Shaleva --- dbft.go | 42 +++++++++++++++++++++++++++--------------- pre_block.go | 3 ++- 2 files changed, 29 insertions(+), 16 deletions(-) diff --git a/dbft.go b/dbft.go index 995d5da7..4cb380d8 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,10 @@ 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 { + 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/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 From a70ec441e90c560366929a56c03b73abb582938a Mon Sep 17 00:00:00 2001 From: Anna Shaleva Date: Mon, 2 Sep 2024 16:11:26 +0300 Subject: [PATCH 4/5] check: adjust PreBlock processing message PreBlock may be created earlier at the PreCommit verification level, and it's OK. However, it's required at least M PreCommits to _process_ PreBlock. Signed-off-by: Anna Shaleva --- check.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/check.go b/check.go index 736fce0b..4109f721 100644 --- a/check.go +++ b/check.go @@ -56,7 +56,7 @@ 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 } From a6c502619bad292d4d06b125b2d3286aa85143b7 Mon Sep 17 00:00:00 2001 From: Anna Shaleva Date: Tue, 3 Sep 2024 13:35:58 +0300 Subject: [PATCH 5/5] dbft: simplify PreBlock constructor Move hasAllTransactions check from PreBlock constructor to onPreCommit callback. The rest of CreatePreBlock usages already have this check. Signed-off-by: Anna Shaleva --- context.go | 3 --- dbft.go | 3 +++ 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/context.go b/context.go index c98b688d..c580349c 100644 --- a/context.go +++ b/context.go @@ -344,9 +344,6 @@ func (c *Context[H]) CreatePreBlock() PreBlock[H] { if c.preBlock = c.MakePreHeader(); c.preBlock == nil { return nil } - if !c.hasAllTransactions() { - return nil - } txx := make([]Transaction[H], len(c.TransactionHashes)) diff --git a/dbft.go b/dbft.go index 4cb380d8..2ce20dc8 100644 --- a/dbft.go +++ b/dbft.go @@ -544,6 +544,9 @@ func (d *DBFT[H]) onPreCommit(msg ConsensusPayload[H]) { d.Logger.Info("received PreCommit", zap.Uint("validator", uint(msg.ValidatorIndex()))) d.extendTimer(4) + if !d.hasAllTransactions() { + return + } preBlock := d.CreatePreBlock() if preBlock != nil { pub := d.Validators[msg.ValidatorIndex()]