*: attach full transactions to PrepareRequest - #160
Open
Turalchik wants to merge 1 commit into
Open
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #160 +/- ##
==========================================
- Coverage 57.65% 54.83% -2.83%
==========================================
Files 33 33
Lines 1913 1882 -31
==========================================
- Hits 1103 1032 -71
- Misses 717 754 +37
- Partials 93 96 +3 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
AnnaShaleva
reviewed
Aug 11, 2026
| } | ||
|
|
||
| func (d *DBFT[H]) processMissingTx() { | ||
| func (d *DBFT[H]) processMissingTx(attached []Transaction[H]) { |
Member
There was a problem hiding this comment.
Revert changes of this method. Panic if this method is called. There's no missing transactions in the new scheme.
Comment on lines
+24
to
+27
| txx := make([]Transaction[H], len(c.TransactionHashes)) | ||
| for i, h := range c.TransactionHashes { | ||
| txx[i] = c.Transactions[h] | ||
| } |
Member
There was a problem hiding this comment.
Use only c.Transactions to construct txx, don't use c.TransactionHashes. In the new scheme c.TransactionHashes is always empty (and will be removed in future).
| @@ -187,7 +192,7 @@ func (d *DBFT[H]) sendRecoveryRequest() { | |||
| // If we're here, something is wrong, we either missing some messages or | |||
| // transactions or both, so re-request missing transactions here too. | |||
| if d.RequestSentOrReceived() && !d.hasAllTransactions() { | |||
Member
There was a problem hiding this comment.
Adjust hasAllTransactions -- it should always return true respectively from c.TransactinoHashes.
|
|
||
| d.Logger.Info("received PrepareRequest", zap.Uint16("validator", msg.ValidatorIndex()), zap.Int("tx", len(d.TransactionHashes))) | ||
| d.processMissingTx() | ||
| d.processMissingTx(p.Transactions()) |
Member
There was a problem hiding this comment.
Don't call d.processMissingTx at all. There's no missing tx.
Turalchik
force-pushed
the
extend-prepare-request
branch
from
August 12, 2026 17:37
087c01c to
945ab02
Compare
AnnaShaleva
reviewed
Aug 13, 2026
| @@ -62,6 +63,8 @@ type Context[H Hash] struct { | |||
| // Transactions is a map containing actual transactions for the current block. | |||
| Transactions map[H]Transaction[H] | |||
| // we don't skip a call to Block.SetTransactions since it may be used as a | ||
| // signal to the user's code to finalize the block. | ||
| c.block.SetTransactions(txx) | ||
| c.block.SetTransactions(slices.Clone(c.TransactionsOrdered)) |
Member
There was a problem hiding this comment.
Why do you need Clone here? Affects performance, no real profit.
| } | ||
|
|
||
| c.preBlock.SetTransactions(txx) | ||
| c.preBlock.SetTransactions(slices.Clone(c.TransactionsOrdered)) |
| if d.RequestSentOrReceived() && !d.hasAllTransactions() { | ||
| d.processMissingTx() | ||
| } | ||
| // If we're here, something is wrong, we're missing some messages. |
Signed-off-by: Tural Devrishev <tural@nspcc.ru>
Turalchik
force-pushed
the
extend-prepare-request
branch
from
August 13, 2026 10:25
945ab02 to
514cc36
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Extend
PrepareRequestwithTransactions().NewPrepareRequestnow also receives the full transaction bodies, andprocessMissingTxtakes them as an argument, so a backup can pull a missing transaction straight from the request instead of a network request that may go unanswered. See nspcc-dev/neo-go#4325.