Txpool context by danielradu10 · Pull Request #7380 · multiversx/mx-chain-go

danielradu10 · 2025-10-30T08:26:02Z

Reasoning behind the pull request

At the moment, when we derive the virtual selection session, there is always a mismatch between the latest root hash, saved on the OnExecuted flow, and the root hash from the accounts proposal. This PR solves this issue.
NOTE: the mismatch on the OnProposedBlock is not solved in this PR.

Proposed changes

propagate the error for roothash mismatch from the selection tracker
added a log in the updateRootHashNoLock from the selection tracker for the cases when the latest root hash is not updated
called the RecreateTrieIfNeeded on the createMiniBlocks flow from both shardProcessor and metaProcessor.
called the OnExecutedBlock on the CommitBlock flow from both shardProcessor and metaProcessor.
(in the integration tests) removed the mocked root hashes from the creation of genesis blocks
(in the integration tests) called the OnExecutedBlock with the right root hash at the right moment (i.e. after the accounts adapter is modified)
(in the integration tests) other fixes (setting the right root hash for each genesis block, setting the current blockchain header with the correct header etc)
fixed unit tests, integration tests

Testing procedure

integration tests, unit tests

Pre-requisites

Based on the Contributing Guidelines the PR author and the reviewers must check the following requirements are met:

was the PR targeted to the correct branch?
if this is a larger feature that probably needs more than one PR, is there a feat branch created?
if this is a feat branch merging, do all satellite projects have a proper tag inside go.mod?

…to txpool-context

codecov · 2025-11-04T17:43:49Z

Codecov Report

❌ Patch coverage is 76.08696% with 11 lines in your changes missing coverage. Please review.
✅ Project coverage is 77.39%. Comparing base (00c3e00) to head (007e1fe).
⚠️ Report is 37 commits behind head on feat/supernova-async-exec.

Files with missing lines	Patch %	Lines
node/chainSimulator/chainSimulator.go	0.00%	4 Missing ⚠️
process/block/metablock.go	33.33%	2 Missing and 2 partials ⚠️
process/block/shardblock.go	60.00%	1 Missing and 1 partial ⚠️
txcache/selectionTracker.go	90.00%	1 Missing ⚠️

Additional details and impacted files

@@                      Coverage Diff                      @@
##           feat/supernova-async-exec    #7380      +/-   ##
=============================================================
- Coverage                      77.40%   77.39%   -0.01%     
=============================================================
  Files                            868      868              
  Lines                         117512   117549      +37     
=============================================================
+ Hits                           90956    90980      +24     
- Misses                         20577    20586       +9     
- Partials                        5979     5983       +4

☔ View full report in Codecov by Sentry.
📢 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.

Copilot

Pull Request Overview

This PR refactors transaction selection and block processing to properly track root hash changes through the OnExecutedBlock flow. The changes ensure that when blocks are executed, the transaction pool is notified with the correct root hash information, enabling better coordination between state changes and transaction selection.

Key changes:

Refactored block processors to call OnExecutedBlock after block commitment
Added RecreateTrieIfNeeded method to ensure correct state trie before transaction selection
Updated selection session to use accountsProposal instead of accounts
Enhanced error handling for ErrAccountNotFoundAtBlock in ephemeral provider

Reviewed Changes

Copilot reviewed 25 out of 25 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
txcache/selectionTracker.go	Added logging for root hash tracking and returned errRootHashMismatch instead of just logging
state/accountsEphemeralProvider.go	Added handling for ErrAccountNotFoundAtBlock error type
process/block/shardblock.go	Refactored to call OnExecutedBlock and RecreateTrieIfNeeded
process/block/metablock.go	Added OnExecutedBlock call and RecreateTrieIfNeeded
process/block/baseProcess.go	Added OnExecutedBlock and RecreateTrieIfNeeded methods
process/block/preprocess/transactions.go	Changed to use accountsProposal for selection session
process/coordinator/process_test.go	Updated mocks to include RecreateTrieIfNeededCalled and RootHashCalled
Multiple test files	Updated test mocks to return nil instead of hardcoded root hash values
integrationTests/*.go	Added SetRootHashOfGenesisBlocks and OnExecutedBlock initialization

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-11-05T12:15:16Z

+		require.NoError(t, err)
+
+		err = bp.RecreateTrieIfNeeded()
+		require.Equal(t, expectedError, err)


Variable name mismatch: the test is checking for expectedError but the mock returns expectedErr. This assertion will always fail. Change expectedError to expectedErr to match the variable defined on line 4095.

Suggested change

require.Equal(t, expectedError, err)

require.Equal(t, expectedErr, err)

mariusmihaic · 2025-11-05T15:40:28Z

 			"latestRootHash", st.latestRootHash,
 			"session rootHash", rootHash,
 		)
+		return nil, errRootHashMismatch


[question]
Is this backwards compatible? Or is this just new code for supernova?

should also apply on legacy, as the onExecuted is done there as well.
If it is not done everywhere, should be added, and if added in all needed places, then it should be backwards compatible

this has to be checked

removed TODO

AdoAdoAdo · 2025-11-06T12:25:29Z

you can also remove the todo, as now I think the correct information is passed, and the error is returned

AdoAdoAdo · 2025-11-06T12:26:28Z

 			"latestRootHash", st.latestRootHash,
 			"session rootHash", rootHash,
 		)
+		return nil, errRootHashMismatch


should also apply on legacy, as the onExecuted is done there as well.
If it is not done everywhere, should be added, and if added in all needed places, then it should be backwards compatible

mariusmihaic · 2025-11-06T12:53:48Z

 		accountsAdapter := &stateMock.AccountsStub{
 			RootHashCalled: func() ([]byte, error) {
-				return []byte("rootHash"), nil
+				return nil, nil


So both mocks will return nil and the checks are valid, right?
[question[
If so, woulnd't it make sens to make the selection tracker mock return the same hash?

mariusmihaic · 2025-11-06T12:58:48Z


 	session, err := NewSelectionSession(ArgsSelectionSession{
-		AccountsAdapter:       txs.accounts,
+		AccountsAdapter:       txs.accountsProposal,


Got it, thanks for clarification

danielradu10 added 2 commits October 29, 2025 18:41

dirty fixes

096d60e

propagate the error

6a1972d

Base automatically changed from prepare-for-legacy-sync to feat/supernova-async-exec October 30, 2025 09:23

danielradu10 and others added 9 commits October 30, 2025 13:02

another round of dirty fixes

9cf8676

Merge branch 'feat/supernova-async-exec' into txpool-context

b577d82

fixes

15a320e

Merge branch 'txpool-context' of github.com:multiversx/mx-chain-go in…

4d2668c

…to txpool-context

fixes

3d68013

fix

b5f901c

fixed check of wrapped error

9872803

fixes

78f5677

uncomment code

0dc220b

AdoAdoAdo mentioned this pull request Oct 31, 2025

dirty fixes #7377

Closed

danielradu10 and others added 11 commits October 31, 2025 18:26

remove mocked roothash

a357dde

Merge branch 'feat/supernova-async-exec' into txpool-context

809e766

Merge branch 'feat/supernova-async-exec' into txpool-context

4d3e6bf

initialize with the updated root hash

34142f7

fixed more tests

4efb0d6

Merge branch 'feat/supernova-async-exec' into txpool-context

77102c0

fixed unit tests

7f3fb6a

fixed integration test

a27cb0b

refactoring, small fixes

1225020

fix

9536d18

small linter fix

747b210

refactoring and unit tests

88c7f7c

danielradu10 marked this pull request as ready for review November 5, 2025 11:48

danielradu10 self-assigned this Nov 5, 2025

danielradu10 added type:improvement interested:protocol labels Nov 5, 2025

AdoAdoAdo requested a review from Copilot November 5, 2025 12:09

Copilot AI reviewed Nov 5, 2025

View reviewed changes

mariusmihaic self-requested a review November 5, 2025 13:13

mariusmihaic reviewed Nov 5, 2025

View reviewed changes

AdoAdoAdo and others added 3 commits November 6, 2025 13:40

Merge branch 'feat/supernova-async-exec' into txpool-context

8a382f2

fixes after reviews

ff3c3a7

Merge branch 'feat/supernova-async-exec' into txpool-context

aeeb873

AdoAdoAdo reviewed Nov 6, 2025

View reviewed changes

linter fix

2ed42f0

mariusmihaic reviewed Nov 6, 2025

View reviewed changes

danielradu10 and others added 6 commits November 6, 2025 16:09

fixes after reviews

4430c0e

small fixes

e4d4321

small fix

b499f65

Merge branch 'feat/supernova-async-exec' into txpool-context

74a3719

Merge branch 'feat/supernova-async-exec' into txpool-context

5b527d8

Merge branch 'feat/supernova-async-exec' into txpool-context

9314cb7

AdoAdoAdo reviewed Nov 7, 2025

View reviewed changes

Comment thread txcache/selectionTracker.go

AdoAdoAdo approved these changes Nov 7, 2025

View reviewed changes

mariusmihaic approved these changes Nov 7, 2025

View reviewed changes

AdoAdoAdo added 2 commits November 7, 2025 14:21

Merge branch 'feat/supernova-async-exec' into txpool-context

045f677

Merge branch 'feat/supernova-async-exec' into txpool-context

007e1fe

AdoAdoAdo merged commit 19c13e3 into feat/supernova-async-exec Nov 7, 2025
10 of 11 checks passed

AdoAdoAdo deleted the txpool-context branch November 7, 2025 15:03

	require.Equal(t, expectedError, err)
	require.Equal(t, expectedErr, err)

Uh oh!

Conversation

danielradu10 commented Oct 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reasoning behind the pull request

Proposed changes

Testing procedure

Pre-requisites

Uh oh!

codecov Bot commented Nov 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Nov 5, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

danielradu10 commented Oct 30, 2025 •

edited

Loading

codecov Bot commented Nov 4, 2025 •

edited

Loading