added commitInMemory method for synching optimization by raduchis · Pull Request #7674 · multiversx/mx-chain-go

raduchis · 2026-02-05T21:35:02Z

Reasoning behind the pull request

synching blocks on supernova is fast -> 10 blocks per second but it is still kinda slow

Proposed changes

looking in the logs for a meta node, commit account state took ~50-70 ms. Created an commitInMemory every 9 out of 10 blocks and the 10th block do a normal commit.

Testing procedure

Remove DB with and without the current branch and see that the current branch synchronizez ~5x faster

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?

Copilot

Pull request overview

Adds an “in-memory commit” path to speed up node syncing by avoiding disk persistence on most blocks, while still computing root hashes and periodically performing full commits.

Changes:

Extend state.AccountsAdapter with CommitInMemory() and implement it across adapters/stubs.
Add AccountsDB.CommitInMemory() and integrate a sync-time commit interval optimization in baseProcessor.
Add unit tests for CommitInMemory() and for the sync commit interval decision logic.

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 9 comments.

Show a summary per file

File	Description
testscommon/state/accountsAdapterStub.go	Extends accounts adapter stub with `CommitInMemory()` for tests.
state/interface.go	Adds `CommitInMemory()` to `AccountsAdapter` interface.
state/accountsDB.go	Implements `CommitInMemory()` and internal `commitInMemory()` flow.
state/accountsDB_test.go	Adds tests for `AccountsDB.CommitInMemory()`.
state/accountsDBApi.go	Implements `CommitInMemory()` as not permitted for API adapter.
state/accountsDBApiWithHistory.go	Implements `CommitInMemory()` as not permitted for API-with-history adapter.
process/transactionEvaluator/simulationAccountsDB.go	Adds no-op `CommitInMemory()` for simulation DB.
epochStart/bootstrap/disabled/disabledAccountsAdapter.go	Adds no-op `CommitInMemory()` for disabled adapter.
process/block/baseProcess.go	Adds sync commit interval optimization + in-memory commit path during sync.
process/block/export_test.go	Exposes constants/helpers for testing the new optimization logic.
process/block/baseProcess_test.go	Adds tests for sync commit optimization and in-memory commit invocation.

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

Copilot · 2026-02-09T08:36:46Z

state/accountsDB.go

+			return nil, err
+		}
+	}
+	adb.dataTries.Reset()


commitInMemory() calls adb.dataTries.Reset(), which clears the holder map (see state/dataTriesHolder.go:70-82). This drops references to dirty data tries without committing them, so a subsequent full Commit() may no longer flush those data tries to disk, risking missing persistence / corrupted state. Consider not resetting dataTries in CommitInMemory (or tracking/retaining dirty tries until the next full commit).

Suggested change

adb.dataTries.Reset()

Copilot · 2026-02-09T08:36:47Z

process/block/baseProcess.go

 	if headerHandler.IsStartOfEpochBlock() {
+		bp.resetSyncCommitCounter()
 		return bp.commitInLastEpoch(headerHandler.GetEpoch())
 	}


In the start-of-epoch path, the deferred log will still print in memory=true because inMemory is initialized to true and never set to false before returning commitInLastEpoch(). This makes the log line misleading (epoch commits are persisted). Set inMemory=false before the early return, or initialize it to false and only set true on the in-memory path.

Copilot · 2026-02-09T08:36:47Z

state/accountsDB_test.go

+	accnt, _ := adb.LoadAccount(make([]byte, 32))
+	_ = adb.SaveAccount(accnt)


This test ignores the errors from LoadAccount and SaveAccount, which can mask a broken setup (e.g., TrieStub.UpdateCalled is unset so SaveAccount will error). Prefer asserting require.NoError and stubbing the trie to allow SaveAccount to succeed, so the test validates behavior under realistic conditions.

Copilot · 2026-02-09T08:36:48Z

process/block/baseProcess_test.go

+	// Call 5 times - first 4 should return true (in-memory), 5th should return false (full commit)
+	for i := 0; i < 4; i++ {
+		result := sp.ShouldUseSyncCommitOptimization(header)
+		assert.True(t, result, "should return true for block %d", i+1)


assert.True(t, result, "should return true for block %d", i+1) will not format the message (testify doesn't treat this as printf-style). Use assert.Truef/require.Truef, or pre-format the string, to keep failure output correct.

Suggested change

assert.True(t, result, "should return true for block %d", i+1)

assert.Truef(t, result, "should return true for block %d", i+1)

Copilot · 2026-02-09T08:36:48Z

state/accountsDBApi.go

+// CommitInMemory is a not permitted operation in this implementation and thus, will return an error
+func (accountsDB *accountsDBApi) CommitInMemory() ([]byte, error) {


Comment grammar: "CommitInMemory is a not permitted operation" should be "CommitInMemory is not a permitted operation" (or "CommitInMemory is not permitted").

Copilot · 2026-02-09T08:36:48Z

process/block/baseProcess.go

+	// Disabled if syncCommitInterval is 0
+	if bp.syncCommitInterval == 0 {
+		return false
+	}


shouldUseSyncCommitOptimization() reads bp.syncCommitInterval without holding mutSyncCommit, but SetSyncCommitInterval() writes it under the mutex. This is an actual data race if the interval is ever updated at runtime (and also leaves blocksSinceLastCommit stale when interval is set to 0). Read syncCommitInterval under the same lock (or use atomics), and consider resetting the counter when disabling/changing the interval.

Or if the SetSyncCommitInterval is meant to be used only in tests, move it to the export_test file.

moved to export_test

Copilot · 2026-02-09T08:36:48Z

state/accountsDB_test.go

+	accnt, _ := adb.LoadAccount(make([]byte, 32))
+	_ = adb.SaveAccount(accnt)


This test ignores the errors from LoadAccount and SaveAccount, which can mask a broken setup (e.g., TrieStub.UpdateCalled is unset so SaveAccount will error). Prefer asserting require.NoError and stubbing the trie to allow SaveAccount to succeed, so the test validates behavior under realistic conditions.

Copilot · 2026-02-09T08:36:49Z

state/accountsDB_test.go

+	accnt, _ := adb.LoadAccount(make([]byte, 32))
+	_ = adb.SaveAccount(accnt)


This test ignores the errors from LoadAccount and SaveAccount, which can mask a broken setup (e.g., TrieStub.UpdateCalled is unset so SaveAccount will error). Prefer asserting require.NoError and stubbing the trie to allow SaveAccount to succeed, so the test validates behavior under realistic conditions.

Copilot · 2026-02-09T08:36:49Z

state/accountsDBApiWithHistory.go

+// CommitInMemory is a not permitted operation in this implementation and thus, will return an error
+func (accountsDB *accountsDBApiWithHistory) CommitInMemory() ([]byte, error) {


Comment grammar: "CommitInMemory is a not permitted operation" should be "CommitInMemory is not a permitted operation" (or "CommitInMemory is not permitted").

BeniaminDrasovean · 2026-02-09T09:25:37Z

process/block/baseProcess.go

+	// Disabled if syncCommitInterval is 0
+	if bp.syncCommitInterval == 0 {
+		return false
+	}


Or if the SetSyncCommitInterval is meant to be used only in tests, move it to the export_test file.

BeniaminDrasovean · 2026-02-09T11:33:27Z

state/accountsDB.go

+	return adb.commitInMemory()
+}
+
+func (adb *AccountsDB) commitInMemory() ([]byte, error) {


There a couple issues with this approach:

The dataTries holder reset drops the reference to all of the changed data tries. When an account is loaded, it will have the new root hash, so it will try to load the data trie with that root hash from the db, but since the dataTrie was not commited, the recreate from db will fail. To fix this, you can remove the reset, but

If a revert occurs, a recreateFromDb will happen - and will fail

Another solution would be to commit the data tries, but into a cache.

Removed the reset from the commitInMemory. It should only happen on the normal Commit. Reverts should not happen because we are on sync and we do not do the commitInMemory if we are closer than 50 nonces from the consensus.

Ok. This can be tested with an observer node started on mainnet with StartInEpoch, right?

# Conflicts: # process/block/baseProcess.go # process/block/baseProcess_test.go

codecov · 2026-02-11T14:23:27Z

Codecov Report

❌ Patch coverage is 79.71014% with 14 lines in your changes missing coverage. Please review.
✅ Project coverage is 77.51%. Comparing base (862985f) to head (f090fe5).

Files with missing lines	Patch %	Lines
process/block/baseProcess.go	85.36%	4 Missing and 2 partials ⚠️
...ocess/transactionEvaluator/simulationAccountsDB.go	0.00%	2 Missing ⚠️
state/accountsDB.go	90.90%	1 Missing and 1 partial ⚠️
state/accountsDBApi.go	0.00%	2 Missing ⚠️
state/accountsDBApiWithHistory.go	0.00%	2 Missing ⚠️

Additional details and impacted files

@@                      Coverage Diff                      @@
##           feat/supernova-async-exec    #7674      +/-   ##
=============================================================
- Coverage                      77.51%   77.51%   -0.01%     
=============================================================
  Files                            882      882              
  Lines                         122760   122829      +69     
=============================================================
+ Hits                           95157    95207      +50     
- Misses                         21259    21274      +15     
- Partials                        6344     6348       +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.

…mory

BeniaminDrasovean

Another issue came to mind: The in-epoch pruning will not work with this approach. Also, state snapshot might be incompatible, if a snapshot is triggered while the tries are not yet committed to db.

BeniaminDrasovean · 2026-02-12T11:41:48Z

state/accountsDB.go

+	return adb.commitInMemory()
+}
+
+func (adb *AccountsDB) commitInMemory() ([]byte, error) {


Ok. This can be tested with an observer node started on mainnet with StartInEpoch, right?

raduchis added 3 commits February 5, 2026 23:27

added commitInMemory method for synching optimization

b6ca39f

added unit tests for sync commit optimization

ac5b918

Merge branch 'feat/supernova-async-exec' into commit-in-memory

3d4fc43

raduchis marked this pull request as ready for review February 9, 2026 07:45

BeniaminDrasovean requested review from BeniaminDrasovean and Copilot February 9, 2026 08:30

Copilot started reviewing on behalf of BeniaminDrasovean February 9, 2026 08:31 View session

Copilot AI reviewed Feb 9, 2026

View reviewed changes

BeniaminDrasovean requested changes Feb 9, 2026

View reviewed changes

Merge branch 'feat/supernova-async-exec' into commit-in-memory

72604f7

# Conflicts: # process/block/baseProcess.go # process/block/baseProcess_test.go

raduchis added 2 commits February 12, 2026 12:33

Merge branch 'feat/supernova-async-exec' into commit-in-memory

add1e7e

fixes after review: removed unnecessary dataTries reset in CommitInMe…

f090fe5

…mory

BeniaminDrasovean reviewed Feb 12, 2026

View reviewed changes

		accnt, _ := adb.LoadAccount(make([]byte, 32))
		_ = adb.SaveAccount(accnt)

	assert.True(t, result, "should return true for block %d", i+1)
	assert.Truef(t, result, "should return true for block %d", i+1)

		// CommitInMemory is a not permitted operation in this implementation and thus, will return an error
		func (accountsDB *accountsDBApi) CommitInMemory() ([]byte, error) {

		// CommitInMemory is a not permitted operation in this implementation and thus, will return an error
		func (accountsDB *accountsDBApiWithHistory) CommitInMemory() ([]byte, error) {

Conversation

raduchis commented Feb 5, 2026

Reasoning behind the pull request

Proposed changes

Testing procedure

Pre-requisites

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 9, 2026

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!

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 9, 2026

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!

codecov bot commented Feb 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

BeniaminDrasovean left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

codecov bot commented Feb 11, 2026 •

edited

Loading