Skip to content

*: attach full transactions to PrepareRequest - #160

Open
Turalchik wants to merge 2 commits into
masterfrom
extend-prepare-request
Open

*: attach full transactions to PrepareRequest#160
Turalchik wants to merge 2 commits into
masterfrom
extend-prepare-request

Conversation

@Turalchik

@Turalchik Turalchik commented Aug 11, 2026

Copy link
Copy Markdown

Extend PrepareRequest with Transactions(). NewPrepareRequest now also receives the full transaction bodies, and processMissingTx takes 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.

@codecov

codecov Bot commented Aug 11, 2026 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 62.68657% with 25 lines in your changes missing coverage. Please review.
✅ Project coverage is 62.76%. Comparing base (01db3a4) to head (cd348ce).
⚠️ Report is 7 commits behind head on master.

Files with missing lines Patch % Lines
internal/consensus/consensus.go 0.00% 11 Missing ⚠️
internal/consensus/constructors.go 0.00% 5 Missing ⚠️
context.go 84.21% 3 Missing ⚠️
dbft.go 72.72% 3 Missing ⚠️
internal/consensus/recovery_message.go 0.00% 2 Missing ⚠️
internal/consensus/prepare_request.go 66.66% 1 Missing ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##           master     #160      +/-   ##
==========================================
+ Coverage   57.65%   62.76%   +5.10%     
==========================================
  Files          33       32       -1     
  Lines        1913     1614     -299     
==========================================
- Hits         1103     1013      -90     
+ Misses        717      600     -117     
+ Partials       93        1      -92

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@Turalchik Turalchik mentioned this pull request Aug 11, 2026
Open
AnnaShaleva
AnnaShaleva reviewed Aug 11, 2026
View reviewed changes
Comment thread dbft.go Outdated
Comment thread send.go Outdated
Comment thread send.go
Comment thread dbft.go Outdated
@Turalchik
Turalchik force-pushed the extend-prepare-request branch from 087c01c to 945ab02 Compare August 12, 2026 17:37
AnnaShaleva
AnnaShaleva reviewed Aug 13, 2026
View reviewed changes
Comment thread context.go
Comment thread context.go Outdated
Comment thread context.go Outdated
Comment thread dbft.go
Comment thread send.go Outdated
@Turalchik
Turalchik force-pushed the extend-prepare-request branch 2 times, most recently from 514cc36 to 8ecec1c Compare August 19, 2026 16:36
AnnaShaleva
AnnaShaleva reviewed Sep 4, 2026
View reviewed changes

@AnnaShaleva AnnaShaleva left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's align dbft code wrt nspcc-dev/neo-go@ff4eb10. No decoding optimisation is required. Both old (master) and new (feature) behaviour should be preserved, the behaviour switch (something like a configurable callback or an additional parameter to (DBFT).Start or (DBFT).Reset) should be exposed to the external user to enable the extension starting from some point without dBFT shutdown.

Comment thread internal/consensus/consensus.go Outdated
Comment thread internal/consensus/consensus.go
Comment thread context.go Outdated
Comment thread dbft.go Outdated
@Turalchik
Turalchik force-pushed the extend-prepare-request branch from 8ecec1c to dcfb34e Compare September 8, 2026 08:59
AnnaShaleva
AnnaShaleva reviewed Sep 8, 2026
View reviewed changes

@AnnaShaleva AnnaShaleva left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CHANGELOG CI is failing.

Comment thread config.go Outdated
Comment thread config.go Outdated
Comment thread config.go Outdated
Comment thread config.go Outdated
Comment thread context.go Outdated
MissingTransactions []H
// Transactions is a map containing actual transactions for the current block.
// Transactions stores transactions collected so far for the current block,
// indexed by hash. It's used for pre-Huyao behaviour (FullTransactionsEnabled

@AnnaShaleva AnnaShaleva Sep 8, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove all Huyao mentions from the dBFT. For the dBFT library it's just an extension that enables new way of PrepareRequest construction. This extension will be used in NeoX network with a different hardfork name. So refer to it as to an extension.

Comment thread dbft.go
Comment thread dbft.go Outdated
Comment thread dbft.go
Comment thread dbft.go Outdated
Comment thread config.go Outdated
@Turalchik
Turalchik force-pushed the extend-prepare-request branch 4 times, most recently from a8774e3 to eb4f98d Compare September 9, 2026 17:11
@Turalchik
Turalchik force-pushed the extend-prepare-request branch 4 times, most recently from d59d366 to 22729f1 Compare September 10, 2026 09:38
AnnaShaleva
AnnaShaleva reviewed Sep 10, 2026
View reviewed changes

@AnnaShaleva AnnaShaleva left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should work now. Before the merge you need to finalize the NeoGo part and test the Huyao transition on the running privnet. From my side an integration with NeoX is required, will push the PR soon.

Comment thread internal/consensus/constructors.go Outdated
Comment thread config.go Outdated
Comment thread config.go
Comment thread context.go Outdated
Comment thread context.go Outdated
Comment thread context.go Outdated
Comment thread context.go Outdated
Comment thread context.go Outdated
Comment thread dbft.go
Comment thread prepare_request.go Outdated
Tural Devrishev
config: fix transactionHashes parameter naming
0a24595 
Signed-off-by: Tural Devrishev <tural@nspcc.ru>
@Turalchik
Turalchik force-pushed the extend-prepare-request branch from 22729f1 to aa29c57 Compare September 11, 2026 08:36
@AnnaShaleva AnnaShaleva mentioned this pull request Sep 11, 2026
Draft
AnnaShaleva
AnnaShaleva reviewed Sep 14, 2026
View reviewed changes

@AnnaShaleva AnnaShaleva left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Almost there.

Comment thread internal/consensus/message_test.go
Comment thread internal/consensus/message_test.go
Comment thread internal/consensus/prepare_request.go Outdated
Comment thread internal/consensus/prepare_request.go Outdated
Comment thread CHANGELOG.md Outdated
Comment thread context.go Outdated
Comment thread context.go Outdated
Comment thread context.go Outdated
Tural Devrishev
*: attach full transactions to PrepareRequest
cd348ce 
Signed-off-by: Tural Devrishev <tural@nspcc.ru>
@Turalchik
Turalchik force-pushed the extend-prepare-request branch from aa29c57 to cd348ce Compare September 15, 2026 14:40
AnnaShaleva
AnnaShaleva reviewed Sep 15, 2026
View reviewed changes

@AnnaShaleva AnnaShaleva left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, waiting for the NeoX part to settle before the merge.

Comment thread internal/consensus/transaction.go
var _ dbft.Transaction[crypto.Uint256] = (*Tx64)(nil)

func init() {
gob.Register(new(Tx64))

@AnnaShaleva AnnaShaleva Sep 15, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why? Not sure if it's needed.

@Turalchik Turalchik Sep 15, 2026 •
edited
Loading

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Without gob.Register EncodeBinary fails with gob: type not registered for interface: consensus.Tx64.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@AnnaShaleva AnnaShaleva AnnaShaleva left review comments
@roman-khimov roman-khimov Awaiting requested review from roman-khimov roman-khimov is a code owner

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@Turalchik @AnnaShaleva