feat(x/vm): apply stack-based StateDB snapshot mechamism for precompile call #244

cloudgray · 2025-06-24T11:41:41Z

Description

Summary

Applying the stack-based snapshot mechanism achieves the same effect as a deep copy of the original store.
Benchmark results show that the stack-based snapshot mechanism uses about 1.4× more memory than CacheMultiStore, but far less than a deep copy—and the gap grows larger as the load increases.

Stack-based Snapshot Mechanism

When executing a precompile, a new store layer is added to the top of the stack, and state writes are performed on this new layer.
If a revert occurs during execution, the invalid store layers are popped off the stack to ensure that a valid store layer remains at the top.
When committing changes, the writes are propagated sequentially from the top of the stack down to the bottom and applied to the initialStore.
The Snapshot() method of the new store uses approximately 1.4 times more memory than the previously used CacheMultiStore(), but it provides a more robust structure that avoids edge cases.

Benchmark

The benchmark results show that the new Snapshot() method uses significantly less memory than a deep copy, and the gap widens as the number of iterations increases.

Like CacheMultiStore(), the new Snapshot() method exhibits linear memory growth with increasing iterations, using approximately 1.4× more memory than CacheMultiStore()

Closes: #225

Author Checklist

All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.

I have...

tackled an existing issue or discussed with a team member
left instructions on how to review the changes
targeted the main branch

Reviewers Checklist

All items are required.
Please add a note if the item is not applicable
and please add your handle next to the items reviewed
if you only reviewed selected items.

I have...

added a relevant changelog entry to the Unreleased section in CHANGELOG.md
confirmed all author checklist items have been addressed
confirmed that this PR does not change production code
reviewed content
tested instructions (if applicable)
confirmed all CI checks have passed

x/vm/statedb/statedb.go

x/vm/store/snapshotmulti/store.go

dep(evmd): go mod tidy refactor: change package name of stack based store refactor(x/vm/store): modify type casting refactor(x/vm): rename store packages

zsystm

@cloudgray
Since the actual type of s.cacheCtx.MultiStore() at this point is *snapshotmulti.Store, I think it's clearer to cast it explicitly rather than going through the storetypes.CacheMultiStore interface.

While *snapshotmulti.Store does implement CacheMultiStore, making that implicit assumption here could lead to confusion for future maintainers who aren’t aware of the underlying implementation. Being explicit improves readability and reduces ambiguity in understanding what store we’re operating on.

I checked that there are no errors after the suggested change.

x/vm/statedb/statedb.go

x/vm/store/snapshotmulti/store_bench_test.go

x/vm/store/snapshotmulti/store_test.go

zsystm · 2025-06-26T06:51:28Z

@cloudgray
Left a couple of nits, but overall LGTM — nice work!

vladjdk · 2025-06-30T21:10:48Z

tests/integration/precompiles/distribution/test_distribution.go

 				s.Require().NoError(err, "expected no error when running the precompile")
 				s.Require().NotNil(resp.Ret, "expected returned bytes not to be nil")
-				testutil.ValidateWrites(s.T(), cms, 2)
+				// NOTES: After stack-based snapshot mechanism is added for precopile call,


typo in precopile

fixed in 264a8fe

vladjdk · 2025-06-30T21:10:54Z

tests/integration/precompiles/distribution/test_distribution.go

 				s.Require().ErrorContains(err, tc.errContains)
-				// Writes once because of gas usage
-				testutil.ValidateWrites(s.T(), cms, 1)
+				// NOTES: After stack-based snapshot mechanism is added for precopile call,


same as above

fixed in 264a8fe

vladjdk · 2025-06-30T21:14:35Z

tests/integration/precompiles/staking/test_integration.go

+						}
+
+						delegateCheck := passCheck.WithExpEvents(staking.EventTypeDelegate, staking.EventTypeDelegate)
+						// Tx should be successful, but no state changes happened


What do you mean by no state changes? Isn't there a delegation being tested here?

After running through the tests, I'm going to assume you mean no unintended state changes from the reversions

That comment was left unchanged during the process of copying and modifying an existing test case. 😅
Fixed in a881f29

vladjdk

Approving this. Great work! I have a much better understanding of how the entire snapshotting system works because of your testing and the cleanliness of your code. We should work with Cordt to turn this into some solid docs!

vladjdk · 2025-06-30T22:34:01Z

x/vm/store/snapshotmulti/store.go

+	return s.CacheMultiStore().(storetypes.CacheWrap)
+}
+
+// CacheWrapWithTrace implements the CacheWrapper interface.


Let's add in the docs that this doesn't do anything

applied! ae1c9fc

x/vm/store/snapshotmulti/store_bench_test.go

vladjdk · 2025-07-01T02:10:13Z

x/vm/store/snapshotmulti/store_test.go

+	snapshotStore.GetKVStore(key).Set([]byte("c"), []byte("3"))
+
+	snapshotStore.RevertToSnapshot(idx1)
+	require.Nil(t, snapshotStore.GetKVStore(key).Get([]byte("c")))


Could we test this using the same key for the KVs (i.e. use "a" each time with a different value)? It would simulate someone overwriting the same address store for something like a delegation over multiple different snapshots.

Maybe as an added test case - I do like this one as well.

applied! 33f939a

vladjdk · 2025-07-01T02:13:22Z

x/vm/store/snapshotmulti/store_test.go

+func TestSnapshotMultiCacheWrapWithTrace(t *testing.T) {
+	snapshotStore, _ := setupStore()
+
+	wrap := snapshotStore.CacheWrapWithTrace(nil, nil)


Document that this is just a regular cache wrap

applied! 33f939a

commit 3b20e2b727685443693838ffcbd5851b2e4c26b8 Author: Kyuhyeon Choi <[email protected]> Date: Fri Jul 11 14:56:44 2025 +0900 feat(x/vm): apply stack-based StateDB snapshot mechanism for precompile call This commit is backport of cosmos/evm [PR#244](#244) commit 3dbee41fca3beda2778ed7e2431800086fe3ea04 Author: Kyuhyeon Choi <[email protected]> Date: Fri Jul 11 09:56:24 2025 +0900 chore: reset consensusVersion of modules to 1 commit 25eb3afa5ff4b96dcb1f8a69d9eb7a326695dc89 Author: Kyuhyeon Choi <[email protected]> Date: Fri Jul 11 09:41:48 2025 +0900 chore(precompiles/common): fix comment commit a7cdcc537e0a33f36e8de36f0247bfe3a2a1d9dc Author: Kyuhyeon Choi <[email protected]> Date: Thu Jul 10 19:45:25 2025 +0900 fix(precompiles/common): hex address parsing method commit 5a937ecd545703515993e6af47def0574a398897 Author: Kyuhyeon Choi <[email protected]> Date: Tue Jul 8 09:49:35 2025 +0900 chore: fix lint commit c32a6e5128c592d5fc791fc13733fc52e18553bc Author: Kyuhyeon Choi <[email protected]> Date: Mon Jul 7 23:09:24 2025 +0900 feat(precompiles): add BalanceHandler to handle native balance change (#201) * feat(precompiles): add BalanceHandler to handle native balance change * refactor: remove parts of calling SetBalanceChangeEntries * chore: fix lint * chore(precompiles/distribution): remove unused helper function * chore(precompiles): modify comments * chore: restore modification to be applied later * chore: fix typo * chore: resolve conflict * chore: fix lint * test(precompiles/common) add unit test cases * chore: fix lint * fix(test): precompile test case that intermittently fails * refactor: move mock evm keeper to x/vm/types/mocks * chore: add KVStoreKeys() method to mock evmKeeper * refactoring balance handling * test(precompile/common): improve unit test for balance handler * refactor(precompiles): separate common logic * Revert "refactor(precompiles): separate common logic" This reverts commit 25b89f3. * Revert "Merge pull request #1 from zsystm/poc/precompiles-balance-handler" This reverts commit 46cd527, reversing changes made to b532fd5. --------- Co-Authored-By: zsystm <[email protected]> Co-Authored-By: Vlad J <[email protected]> commit 37e0f9a8cdd362c78d411bc4291e61b64bbc3dbc Author: Haber <[email protected]> Date: Mon Jul 7 10:28:01 2025 +0900 feat(precompiles): add BalanceHandler to handle native balance change (#201) * feat(precompiles): add BalanceHandler to handle native balance change * refactor: remove parts of calling SetBalanceChangeEntries * chore: fix lint * chore(precompiles/distribution): remove unused helper function * chore(precompiles): modify comments * chore: restore modification to be applied later * chore: fix typo * chore: resolve conflict * chore: fix lint * test(precompiles/common) add unit test cases * chore: fix lint * fix(test): precompile test case that intermittently fails * refactor: move mock evm keeper to x/vm/types/mocks * chore: add KVStoreKeys() method to mock evmKeeper * refactoring balance handling * test(precompile/common): improve unit test for balance handler * refactor(precompiles): separate common logic * Revert "refactor(precompiles): separate common logic" This reverts commit 25b89f3. * Revert "Merge pull request #1 from zsystm/poc/precompiles-balance-handler" This reverts commit 46cd527, reversing changes made to b532fd5. --------- Co-authored-by: zsystm <[email protected]> Co-authored-by: Vlad J <[email protected]> commit a5bf7d1f255ab43a28ac19d3348d56f20d3dbe29 Author: Haber <[email protected]> Date: Thu Jun 12 11:17:44 2025 +0900 refactor(precompiles): apply journal-based revert approach (#205) * refactor(precompiles): apply journal-based revert approach * refactor: remove unused Snapshot type * chore: fix lint

…le call (cosmos#244) * feat(x/vm): add cacheStack for CacheMultiStore snapshot * fix(x/vm): StateDB.snapshotter * feat(x/vm): add keys field to x/vm keeper and use keys for snapshot * deps: remove cosmossdk.io/store fork * refactor(x/vm): refactor snapshot multi store and add comments * test(x/vm): add benchmark test for snapshotmulti.Store * test(precompiles): add edge case test for precompile * refactor(x/vm): modify snapshot stores dep(evmd): go mod tidy refactor: change package name of stack based store refactor(x/vm/store): modify type casting refactor(x/vm): rename store packages * fix(x/vm): order store keys for snapshot store * chore(x/vm/store): modify comments * chore(x/vm/store): add comments * test(x/vm/store): add unit tests * chore(x/vm): fix lint * chore: fix lint * chore: fix lint * chore(x/vm/store): remove unnecessary function call * test: modify TestCMS * fix(tests): precompile test case that intermittently fails * chore(x/vm/store): rename variable cms into snapshotStore in test code * chore: resolve merge conflict * chore(x/vm) fix lint * chore: fix typo * chore: modify comment * chore(x/vm/store): add comments * test(x/vm/store): add test case for overwrite to same key

cloudgray added 7 commits June 23, 2025 21:37

feat(x/vm): add cacheStack for CacheMultiStore snapshot

ddd79b5

fix(x/vm): StateDB.snapshotter

7516b28

feat(x/vm): add keys field to x/vm keeper and use keys for snapshot

293bc02

deps: remove cosmossdk.io/store fork

f14eea0

refactor(x/vm): refactor snapshot multi store and add comments

30040a7

test(x/vm): add benchmark test for snapshotmulti.Store

a6628ef

Merge branch 'main' into poc/apply-cache-stack

490a0b6

cloudgray changed the title ~~Poc/apply cache stack~~ feat(x/vm): apply stack-based StateDB snapshot mechamism for precompile call Jun 24, 2025

cloudgray self-assigned this Jun 24, 2025

github-advanced-security bot found potential problems Jun 24, 2025

View reviewed changes

cloudgray added 9 commits June 25, 2025 14:05

test(precompiles): add edge case test for precompile

3fcb7db

refactor(x/vm): modify snapshot stores

fdc0c16

dep(evmd): go mod tidy refactor: change package name of stack based store refactor(x/vm/store): modify type casting refactor(x/vm): rename store packages

fix(x/vm): order store keys for snapshot store

229367d

chore(x/vm/store): modify comments

6ececba

chore(x/vm/store): add comments

ae18153

test(x/vm/store): add unit tests

f7b26bb

chore(x/vm): fix lint

d2f40c7

chore: fix lint

2972d22

chore: fix lint

6af41b0

cloudgray marked this pull request as ready for review June 25, 2025 14:09

cloudgray added 2 commits June 25, 2025 23:20

chore(x/vm/store): remove unnecessary function call

ee190f6

test: modify TestCMS

c6cb162

cloudgray requested review from Eric-Warehime, vladjdk and zsystm June 25, 2025 14:28

zsystm reviewed Jun 26, 2025

View reviewed changes

x/vm/statedb/statedb.go Show resolved Hide resolved

x/vm/statedb/statedb.go Show resolved Hide resolved

zsystm reviewed Jun 26, 2025

View reviewed changes

x/vm/store/snapshotmulti/store_bench_test.go Show resolved Hide resolved

x/vm/store/snapshotmulti/store_test.go Outdated Show resolved Hide resolved

zsystm mentioned this pull request Jun 26, 2025

Research Possibility of Keeping CMS instead of Copy() #225

Closed

Merge branch 'main' into poc/apply-cache-stack

70cddf2

cloudgray added 5 commits June 27, 2025 12:30

fix(tests): precompile test case that intermittently fails

0f00851

chore(x/vm/store): rename variable cms into snapshotStore in test code

df311f3

Merge branch 'main' into poc/apply-cache-stack

c4f0266

chore: resolve merge conflict

3ad3763

chore(x/vm) fix lint

4a9b5aa

vladjdk reviewed Jun 30, 2025

View reviewed changes

cloudgray added 2 commits July 1, 2025 10:27

chore: fix typo

264a8fe

chore: modify comment

a881f29

vladjdk approved these changes Jul 1, 2025

View reviewed changes

cloudgray added 2 commits July 1, 2025 13:11

chore(x/vm/store): add comments

ae1c9fc

test(x/vm/store): add test case for overwrite to same key

33f939a

cloudgray merged commit e6fe094 into cosmos:main Jul 1, 2025
15 checks passed

Pitasi mentioned this pull request Jul 30, 2025

feat: remove MaxPrecompileCalls #381

Closed

9 tasks

feat(x/vm): apply stack-based StateDB snapshot mechamism for precompile call #244

feat(x/vm): apply stack-based StateDB snapshot mechamism for precompile call #244

Uh oh!

Conversation

cloudgray commented Jun 24, 2025

Description

Summary

Stack-based Snapshot Mechanism

Benchmark

Author Checklist

Reviewers Checklist

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

zsystm left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

zsystm commented Jun 26, 2025

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!

Choose a reason for hiding this comment

Uh oh!

vladjdk left a comment

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!

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

zsystm left a comment •

edited

Loading