EVM support

[kakysha]

Summary by CodeRabbit

• New Features

  • Added support for mounting additional object stores.
  • Introduced virtual account crediting and enhanced coin transfer methods between user accounts and module accounts.
  • Enabled parallel transaction execution with enriched context tracking (e.g., transaction indices and block gas metrics).

• Improvements

  • Optimized storage and mempool layers with refined gas management and type-safe generics for more efficient state handling.

[github-actions]

Change potentially affects state.

Call sequence:

(*github.com/cosmos/cosmos-sdk/baseapp.BaseApp).internalFinalizeBlock (baseapp/abci.go:703)
(*github.com/cosmos/cosmos-sdk/baseapp.BaseApp).FinalizeBlock (baseapp/abci.go:876)

[github-actions]

Change potentially affects state.

Call sequence:

(*github.com/cosmos/cosmos-sdk/baseapp.BaseApp).internalFinalizeBlock (baseapp/abci.go:703)
(*github.com/cosmos/cosmos-sdk/baseapp.BaseApp).FinalizeBlock (baseapp/abci.go:876)

[maxim-inj]

This PR must not include things that are not directly related to EVM.
We shall cherry-pick only relevent EVM changes and skip stuff related to parallel execution and multistore hacks.

[maxim-inj]

90f3048

We need to cut this out and all deps of it. I suggest instead cherry picking what's really relevant to EVM.

[coderabbitai]

Walkthrough

The changes update the CI configuration and linter settings, introduce new APIs and refactor methods across the Cosmos SDK. Major modifications include upgrading Go versions in workflows, removing deprecated linters, and enhancing transaction processing via a new parallel executor and additional context fields. Extensive refactoring has been applied to the store layer—including generics integration and removal of legacy cache wrapping—and the banking module now supports virtual account transfers and object store integration. Supporting test utilities and proto definitions have been updated accordingly.

Changes

File(s)	Change Summary
.github/workflows/test.yml & .golangci.yml	Updated Go version (1.20 → 1.21) in workflow jobs; removed linters gci and nolintlint and their settings in the linter configuration.
CHANGELOG.md	Documented new features and improvements: TxExecutor option, new Context fields (e.g. TxIndex, BlockGasWanted), ObjectStore API, RunAtomic API, and various optimizations & bug fixes.
baseapp/ (abci.go, abci_utils.go, baseapp.go, genesis.go, options.go, test_helpers.go, testutil/mock/mocks.go, txexecutor.go)	Refactored transaction execution by introducing an executeTxs method and TxExecutor type; updated method signatures to include additional parameters (e.g. txIndex) and gas information; modularized logic and updated mocks accordingly.
crypto/keyring/keyring.go	Added a new DB() method to the Keyring interface and its implementation in the keystore.
runtime/ (module.go, store.go)	Added ProvideObjectStoreKey in module registration; removed CacheWrapWithTrace and updated iterator return types in store implementations.
server/mock/store.go & simapp/ (app.go, app_config.go)	Adjusted multi-store type assertions and removed deprecated cache wrap methods; added okeys to SimApp for object store management and updated module end blocker ordering.
store/ (all subdirectories)	Major refactoring across cachekv, cachemulti, dbadapter, gaskv, iavl, internal/btree, listenkv, mem, prefix, rootmulti, tracekv, transient: migrated to generic GStore structures, removed legacy CacheWrapWithTrace APIs, added new APIs such as RunAtomic, Discard, NewFromParent, and updated iterator types.
testutil/ (context.go, integration tests, router.go)	Added DefaultContextWithObjectStore and updated multi-store creation functions to handle additional object store keys.
types/ (context.go, mempool/*, and related tests)	Introduced new fields and accessor/setter methods in Context; added new mempool type Tx with gas information, new GasTx interface, and updated several mempool-related methods and tests to reflect the encapsulated transaction type.
x/bank/ (abci.go, keeper/*, module.go, types/keys.go, restrictions_test.go)	Enhanced banking functionality with virtual account transfers (new methods for sending coins virtually and crediting virtual accounts), integrated object store key (ObjStoreKey) into keeper initialization, and refactored send restrictions to use a single coin instead of sdk.Coins.
x/gov/ and x/group/ (expected_keepers_mocks.go, orm/testsupport.go)	Updated mocks for bank keeper virtual methods and modified store retrieval to use the generic CommitStore interface.
x/tx/internal/testpb/signers.proto	Modified the BadSigner message by changing the signer field type from bytes to int32.

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant BankKeeper
  participant BaseSendKeeper
  participant VirtualStore
  participant Account
  
  Client->>BankKeeper: SendCoinsFromAccountToModuleVirtual(ctx, sender, module, amt)
  BankKeeper->>BaseSendKeeper: Process virtual coin transfer
  BaseSendKeeper->>Account: ensureAccountCreated()
  BaseSendKeeper->>VirtualStore: subVirtualCoin(ctx, sender, amt)
  BaseSendKeeper->>VirtualStore: addVirtualCoin(ctx, recipient, amt)
  alt End of Block
      BaseSendKeeper->>BankKeeper: CreditVirtualAccounts(ctx)
      BankKeeper->>Account: Update balances accordingly
  end

Loading

sequenceDiagram
  participant Client
  participant BaseApp
  participant TxExecutor
  participant ExecutionCore
  
  Client->>BaseApp: Submit Transaction
  BaseApp->>TxExecutor: executeTxs(ctx, blockSize, multiStore, deliverTxWithMultiStore)
  TxExecutor->>ExecutionCore: Process individual transactions
  ExecutionCore-->>TxExecutor: Return tx execution results
  TxExecutor-->>BaseApp: Aggregate results and metrics
  BaseApp-->>Client: Return Transaction Response

Loading

Poem

I’m a rabbit with ears so keen,
Hopping through code on a vibrant scene.
New executors and stores, all shiny and neat,
Virtual coins dancing in a rhythmic beat.
With every change, my heart does skip – a coder’s joyful trip!
🐇💻

✨ Finishing Touches

• 📝 Generate Docstrings

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

• Review comments: Directly reply to a review comment made by CodeRabbit. Example:
  • I pushed a fix in commit <commit_id>, please review it.
  • Generate unit testing code for this file.
  • Open a follow-up GitHub issue for this discussion.
• Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
  • @coderabbitai generate unit testing code for this file.
  • @coderabbitai modularize this function.
• PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
  • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
  • @coderabbitai read src/utils.ts and generate unit testing code.
  • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
  • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

• @coderabbitai pause to pause the reviews on a PR.
• @coderabbitai resume to resume the paused reviews.
• @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
• @coderabbitai full review to do a full review from scratch and review all the files again.
• @coderabbitai summary to regenerate the summary of the PR.
• @coderabbitai generate docstrings to generate docstrings for this PR.
• @coderabbitai resolve resolve all the CodeRabbit review comments.
• @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
• @coderabbitai help to get help.

Other keywords and placeholders

• Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
• Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
• Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

• You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
• Please see the configuration documentation for more information.
• If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

• Visit our Documentation for detailed information on how to use CodeRabbit.
• Join our Discord Community to get help, request features, and share feedback.
• Follow us on X/Twitter for updates and announcements.

[github-actions]

Change potentially affects state.

Call sequence:

(*github.com/cosmos/cosmos-sdk/baseapp.BaseApp).internalFinalizeBlock (baseapp/abci.go:703)
(*github.com/cosmos/cosmos-sdk/baseapp.BaseApp).FinalizeBlock (baseapp/abci.go:876)

[github-actions]

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[maxim-inj]

wtf unclose

[github-actions]

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[coderabbitai]

Actionable comments posted: 11

♻️ Duplicate comments (1)

store/internal/btreeadaptor.go (1)

37-43: ⚠️ Potential issue

Consider handling errors instead of panicking.

Panics in storage operations could potentially cause chain halts if they occur during critical consensus paths like BeginBlock or EndBlock. Consider returning errors instead of panicking to allow graceful handling.

-func (ts *BTreeStore[V]) Iterator(start, end []byte) types.GIterator[V] {
+func (ts *BTreeStore[V]) Iterator(start, end []byte) (types.GIterator[V], error) {
 	it, err := ts.BTree.Iterator(start, end)
 	if err != nil {
-		panic(err)
+		return nil, err
 	}
-	return it
+	return it, nil
}

If modifying the return type is not possible due to interface requirements, consider adding robust error logs before panicking.

🧰 Tools

🪛 GitHub Check: CodeQL

[warning] 40-40: Panic in BeginBock or EndBlock consensus methods
Possible panics in BeginBock- or EndBlock-related consensus methods could cause a chain halt

🧹 Nitpick comments (66)

store/internal/btreeadaptor.go (3)

1-7: Add package documentation.

This is a new file introducing generic adapters for BTree storage. Consider adding package documentation that explains the purpose and usage patterns of these adapters.

 package internal
 
+// Package internal provides storage adapters and utilities that are meant for
+// internal use within the store module. This package contains the BTreeStore
+// implementation which adapts a BTree to the KVStore and ObjKVStore interfaces.
 
 import (
 	"cosmossdk.io/store/cachekv"

---

26-29: Add documentation for the Get method.

The Get method lacks documentation explaining its behavior, particularly what happens when a key isn't found.

+// Get returns the value for the given key. If the key does not exist, it returns
+// the zero value for type V.
 func (ts *BTreeStore[V]) Get(key []byte) (value V) {
 	value, _ = ts.BTree.Get(key)
 	return
 }

---

21-24: Improve constructor documentation.

The constructor documentation could be more descriptive about its parameters.

-// NewBTreeStore constructs new BTree adapter
+// NewBTreeStore constructs a new BTree adapter that implements the KVStore interface.
+// It requires:
+// - a BTree implementation
+// - an isZero function that determines if a value is effectively "empty"
+// - a valueLen function that returns the logical size of a value
 func NewBTreeStore[V any](btree btree.BTree[V], isZero func(V) bool, valueLen func(V) int) *BTreeStore[V] {
 	return &BTreeStore[V]{btree, isZero, valueLen}
 }

store/listenkv/store.go (1)

131-135: Outdated comment needs updating

The method comment still states that it "panics as a Store cannot be cache wrapped" but the implementation now successfully returns a cache-wrapped store using cachekv.

-// CacheWrap implements the KVStore interface. It panics as a Store
-// cannot be cache wrapped.
+// CacheWrap implements the KVStore interface. It returns a new cache-wrapped
+// store using cachekv.

store/listenkv/store_test.go (1)

275-275: Updated test to match new non-panicking behavior

This test has been updated to no longer expect a panic when calling CacheWrap(), which is consistent with the implementation changes.

However, the test should be expanded to verify that the returned cache-wrapped store works correctly.

 func TestListenKVStoreCacheWrap(t *testing.T) {
 	store := newEmptyListenKVStore(nil)
-	store.CacheWrap()
+	cacheStore := store.CacheWrap()
+	require.NotNil(t, cacheStore, "CacheWrap should return a valid cache store")
+	
+	// Test basic operations on the cache store
+	key := []byte("test_key")
+	value := []byte("test_value")
+	cacheStore.Set(key, value)
+	require.Equal(t, value, cacheStore.Get(key))
+	
+	// Verify the cache doesn't affect the underlying store
+	require.Nil(t, store.Get(key))
+	
+	// Test write operations
+	cacheStore.(types.CacheWrap).Write()
+	require.Equal(t, value, store.Get(key))
}

store/tracekv/store_test.go (1)

286-286: Test updated to match new CacheWrap behavior

The test has been updated to no longer expect a panic when CacheWrap is called, which is consistent with the implementation change. However, the test now simply calls CacheWrap without verifying anything about the returned value.

Consider enhancing this test to verify that the returned cache wrapper works as expected. For example:

-store.CacheWrap()
+cacheStore := store.CacheWrap()
+require.NotNil(t, cacheStore, "Cache wrapper should not be nil")
+
+// Test that the cache store behaves correctly
+testKey := []byte("cache_test_key")
+testValue := []byte("cache_test_value")
+
+// Set a value in the cache
+cacheStore.Set(testKey, testValue)
+
+// Value should be in cache but not in the original store
+require.Equal(t, testValue, cacheStore.Get(testKey))
+require.Nil(t, store.Get(testKey))

store/CHANGELOG.md (1)

26-38: Use the definite article for clarity in changelog entries.

Consider inserting the article "the" before “RunAtomic API” and “CoW cache store” to align with standard writing conventions, as indicated by your static analysis tool.

Below is an example of how you might refine line 35:

- * [#243](https://github.com/crypto-org-chain/cosmos-sdk/pull/243) Support `RunAtomic` API to use new CoW cache store.
+ * [#243](https://github.com/crypto-org-chain/cosmos-sdk/pull/243) Support the `RunAtomic` API to use the new CoW cache store.

🧰 Tools

🪛 LanguageTool

[uncategorized] ~35-~35: You might be missing the article “the” here.
Context: ...ull/243) Support RunAtomic API to use new CoW cache store. * [cosmos#244](https://githu...

(AI_EN_LECTOR_MISSING_DETERMINER_THE)

---

[uncategorized] ~36-~36: You might be missing the article “the” here.
Context: ...ypto-org-chain/cosmos-sdk/pull/244) Add Discard method to CacheWrap to discard ...

(AI_EN_LECTOR_MISSING_DETERMINER_THE)

store/types/validity.go (2)

30-36: Consider returning an error rather than panicking.

Using a panic for invalid data can result in a chain halt. You might opt to return an error from AssertValidValueGeneric and handle it gracefully. This approach can improve robustness in a production environment.

---

38-43: Avoid panic with an error object.

Since you aren’t returning the error, using panic with errors.New(...) offers no practical advantage over a simple string. For consistency, consider:

-	panic(errors.New("value is too large"))
+	panic("value is too large")

🧰 Tools

🪛 GitHub Check: CodeQL

[warning] 41-41: Panic in BeginBock or EndBlock consensus methods
Possible panics in BeginBock- or EndBlock-related consensus methods could cause a chain halt

types/mempool/mempool.go (1)

10-13: Consider renaming the new Tx struct.

While this addition is functional, naming it simply Tx could cause confusion when used alongside sdk.Tx. You might consider a more descriptive name like MempoolTx.

-type Tx struct {
+type MempoolTx struct {
    Tx        sdk.Tx
    GasWanted uint64
 }

server/mock/store.go (1)

109-112: New GetObjKVStore method is unimplemented.

func (ms multiStore) GetObjKVStore(storetypes.StoreKey) storetypes.ObjKVStore {
	panic("not implemented")
}

Please return a proper mock or partial implementation if tests require this functionality.

types/context.go (1)

69-78: New fields for transaction and block tracking.
Adding txIndex, msgIndex, txCount, blockGasUsed, and blockGasWanted expands contextual state. Make sure they're well-covered in tests to maintain correctness.

store/internal/btree/memiterator.go (4)

1-12: Consider isolating generic refactor from EVM-related changes.

The addition of generics (e.g., memIterator[V any]) and package rename appear unrelated to direct EVM support. If these changes are not strictly necessary for EVM, consider splitting them into a separate PR to maintain focus and reduce risk.

---

76-76: Domain method remains consistent.

The updated signature for Domain() still returns (start, end []byte). There's no apparent EVM tie here, so confirm it’s necessary.

---

80-80: Avoid silent returns on iterator close.

This method quietly releases the iterator but doesn’t log or handle potential incomplete usage. Usually it’s fine, but consider logging or tracking resource closure if needed.

---

128-128: Panic approach.

assertValid() panics on invalid state. That is acceptable for internal iterators but be mindful that panics can propagate if not handled.

baseapp/abci_utils.go (2)

292-292: Leverage signer extraction safely.

signerData, err := h.signerExtAdapter.GetSigners(memTx.Tx) obtains signers from the mempool transaction. Ensure the EVM inbound transactions also integrate with GetSigners.

Would you like help customizing signer extraction for EVM Tx?

---

327-334: Selective removal vs. acceptance logic.

When PrepareProposalVerifyTx fails, the code removes the transaction from the mempool. Otherwise, it calls SelectTxForProposal. This flow is correct but keep an eye on performance implications with large mempools.

store/internal/btree/btree.go (3)

34-43: Ensure concurrency safety for the lazy initialization logic.
The init() method lazily initializes the B-tree, but there is no synchronization. Concurrent calls may result in multiple B-tree allocations if not externally synchronized.

Consider adding synchronization if this store can be accessed by multiple goroutines. Alternatively, document that callers must provide concurrency protection where needed.

---

66-79: Validate iteration bounds more thoroughly.
Currently, the iterator constructors only check for empty slices (i.e., len(...) == 0). Consider also validating that start <= end to prevent unintended iteration usage. This helps avoid subtle off-by-one behavior or infinite iteration when the range bounds are reversed.

 func (bt BTree[V]) Iterator(start, end []byte) (types.GIterator[V], error) {
   if (start != nil && len(start) == 0) || (end != nil && len(end) == 0) {
     return nil, errKeyEmpty
   }
+  if end != nil && start != nil && bytes.Compare(start, end) > 0 {
+    return nil, errors.New("start key must not exceed end key")
+  }
   return newMemIterator(start, end, bt, true), nil
 }

---

118-126: Check key validity in item creation helpers.
The helper functions newItem and newItemWithKey do not validate that keys are non-nil. Since several other methods assume non-nil keys, consider adding a nil check or returning an error for unexpected key states.

 func newItem[V any](key []byte, value V) item[V] {
+  if key == nil {
+    panic("nil key not allowed")
+  }
   return item[V]{key: key, value: value}
 }

store/rootmulti/store.go (2)

66-77: Maintain clarity around newly added fields.
The new fields (stores, keysByName, removalMap, etc.) greatly expand store functionality. Consider grouping these fields under comments or separate them with blank lines to improve readability, given their importance in the store’s lifecycle.

---

181-189: Gracefully handle store type mismatch.
Currently, when the store type assertion fails, the code panics. While this may be customary for Cosmos SDK code, consider introducing an error return in future refactors to avoid chain halts in production if a malformed store key is ever used.

store/transient/store.go (3)

28-30: Maintain consistent naming for specialized types.
Store is specialized to []byte, but is generically named “Store”. For clarity, consider a more descriptive name like TransientByteStore or similarly indicative type.

---

49-54: Add usage note for ObjStore.
ObjStore uses a fixed valueLen of 1 in NewObjStore, presumably to validate that any non-nil object counts as a single entry. This is fine, but consider clarifying in doc comments to avoid confusion for future maintainers.

---

62-64: Customize commit logic for GStore.
Calling ts.Clear() on commit is correct for a transient store. Possibly log a debug message upon commit to help differentiate ephemeral store commits from persistent store commits.

store/cachemulti/store.go (9)

27-29: Reassess naming and usage for parentStore and branched.
While introducing parentStore and branched can add flexibility, please confirm if these changes are strictly required for EVM support. If not, consider placing them in a separate PR to maintain PR focus.

---

77-91: Support tracing for other store types.
initStore only applies tracing if the wrapped store is a KVStore. If you plan to trace object stores or specialized stores (e.g., for EVM data), consider extending tracing coverage.

---

93-101: Revisit panic for LatestVersion.
Returning an error or a descriptive message might be less disruptive than panicking, particularly if EVM code calls this function implicitly.

---

131-135: Panic on branched store write.
While this ensures correctness, consider returning an error to avoid chain halt scenarios if an accidental write is attempted on a branched store.

---

162-171: Panic instead of returning errors.
getCacheWrapper panics if a store is missing or unregistered. Evaluate whether an error return is more suitable to avoid unexpected chain halts.

🧰 Tools

🪛 GitHub Check: CodeQL

[warning] 169-169: Panic in BeginBock or EndBlock consensus methods
Possible panics in BeginBock- or EndBlock-related consensus methods could cause a chain halt

---

183-189: Consistent approach to type checking.
Similarly, GetStore panics if the store isn’t the expected type. Confirm that these typed getters won’t harm consensus if misused.

---

193-199: Same panic pattern for GetKVStore.
Be sure all usage sites guarantee the correct type. Otherwise, consider gracefully failing with an error or log message.

🧰 Tools

🪛 GitHub Check: CodeQL

[warning] 196-196: Panic in BeginBock or EndBlock consensus methods
Possible panics in BeginBock- or EndBlock-related consensus methods could cause a chain halt

---

201-208: Extend or unify tracing for GetObjKVStore.
Trace support might be needed here if EVM uses object-based stores. Also ensure consistent error-handling strategy with KVStore.

---

249-257: Atomic operation scope.
RunAtomic is powerful, but confirm its usage belongs in the EVM PR. If not strictly needed, consider moving it to a separate changeset.

store/cachekv/store.go (7)

17-23: Check necessity of specialized constructor.
NewStore now returns a *Store with specific zero checks. If EVM doesn’t require these changes, consider isolating them from the PR.

---

38-46: Initialization logic for NewGStore.
The btree-based writeSet is straightforward. Ensure that parent store also meets EVM’s concurrency or performance expectations.

---

73-79: Fallback to parent store on cache misses.
If you rely on EVM’s typical immediate finality, confirm it’s acceptable to fetch data from the parent store only when absent in writeSet.

---

103-107: Delete with zeroValue.
Storing zeroValue to mark a deletion can lead to confusion if any part of EVM logic interprets zero or nil as valid data.

---

110-120: Write iteration over writeSet.
Check whether iteration ordering influences consensus or EVM semantics. If order matters, consider sorting by keys.

---

149-150: Separate variable or inline usage for clarity.
parent, cache might be clearer if defined inline, unless you specifically need them for the subsequent logic.

---

163-164: Merge iterator approach.
NewCacheMergeIterator merges parent vs. cache results. Confirm that EVM or end consumers handle potential duplicate or out-of-order keys gracefully.

store/types/store.go (4)

249-250: Consistent value checks.
Set(key []byte, value V) now depends on V’s validity rules. Confirm you can still handle large or dynamic contract data.

---

277-305: New GIterator[V] interface.
This is a well-defined approach to typed iteration. EVM-based logic might still require []byte despite generics. Confirm bridging is straightforward.

---

363-367: BranchStore interface introduction.
Clone and Restore will help advanced forking or parallel transactions, but confirm that the EVM module needs them.

---

490-512: New ObjectStoreKey.
This store key is presumably for in-memory objects or ephemeral usage. Double-check if EVM integration truly requires it.

store/gaskv/store.go (3)

24-24: Consider renaming the unused parameter.

The parameter v any in the anonymous function is unused (lint warning). You can replace it with _ or remove it completely to avoid confusion.

Apply this diff to rename the unused parameter:

-func(v any) int { return ObjectValueLength }
+func(_ any) int { return ObjectValueLength }

🧰 Tools

🪛 golangci-lint (1.62.2)

[warning] 24-24: unused-parameter: parameter 'v' seems to be unused, consider removing or renaming it as _

(revive)

---

69-69: Address the "TODO overflow-safe math?" comment.

The inline TODO comment indicates that there may be a risk of integer overflow when calculating gas. Consider using checked arithmetic or conversions to ensure the result cannot overflow silently in extreme cases.

Also applies to: 70-70

---

21-25: Discuss customizing ObjectValueLength.

Currently, ObjectValueLength is a fixed constant (16) for objects. If some objects are significantly larger, the gas calculation may become inaccurate. Provide an option to override or calculate the real size for accurate gas consumption.

🧰 Tools

🪛 golangci-lint (1.62.2)

[warning] 24-24: unused-parameter: parameter 'v' seems to be unused, consider removing or renaming it as _

(revive)

store/prefix/store.go (2)

21-27: Enhance user control over object size calculations.

Like the store/gaskv PR, this uses a preset or trivial value length for object-oriented stores. If object sizes can vary, consider adding a customizable or dynamic size function.

---

180-181: Consider invalidating the iterator more explicitly.

The comment hints at possibly setting the iterator to nil. A more explicit approach (e.g., returning an error or a separate invalid state object) might improve clarity for users who call Next() after exhaustion.

store/cachekv/internal/mergeiterator.go (2)

29-39: Check nil safety for isZero function.
NewCacheMergeIterator relies on isZero for delete-check logic. There are no explicit nil-checks. Consider gracefully handling a nil isZero to prevent potential runtime panics in edge cases.

---

161-165: assertValid() approach.
Panic on invalid iterator usage is consistent with the existing design, though consider returning errors if the caller might need graceful handling.

types/mempool/sender_nonce.go (1)

158-165: New Insert method with gas extraction

The new Insert method provides backward compatibility by extracting the gas limit from transactions that implement the GasTx interface, then delegating to InsertWithGasWanted.

Consider adding fallback behavior documentation for transactions that don't implement GasTx - currently they silently use zero gas limit.

x/bank/keeper/send.go (1)

235-237: Refactor account creation and event emission.

Good refactoring that separates concerns by extracting account creation and event emission into dedicated methods, improving code maintainability.

baseapp/baseapp.go (7)

208-210: Clarify usage of txExecutor.

The traceFlightRecorder field appears unused in this diff. If it is indeed needed for tracing, please ensure it’s integrated where necessary or remove it to avoid confusion. The new txExecutor TxExecutor field seems important for parallel or custom transaction execution. Confirm that it’s invoked in relevant code paths and that proper initialization procedures are documented.

---

691-691: Revisit the new txIndex parameter.

The txIndex parameter in getContextForTx helps track transaction ordering. Confirm that all consumer logic (e.g., logs, metrics, or indexing) is aware of this field to avoid inconsistencies, especially if any other indexing mechanism exists elsewhere.

---

788-791: Adopt consistent naming or parameter structure.

The new deliverTx(tx []byte, txIndex int) method clarifies that each transaction is identified by its index. Consider reflecting its usage in the method name (e.g., DeliverTxAtIndex) or adding method-level comments explaining the parameter’s purpose if it remains an optional approach to transaction ordering.

---

869-872: Avoid possible confusion with -1 txIndex.

Returning runTxWithMultiStore(mode, txBytes, -1, nil) for runTx methods sets txIndex to -1. While it indicates no specific index, ensure that subsequent code does not attempt out-of-range logic or store lookups expecting a non-negative index.

---

1178-1178: Document the returned gas usage.

ProcessProposalVerifyTx now returns a uint64 for gasWanted. Include a brief doc comment or clarifying note regarding how the calculated gas is intended for proposal verification or if it’s used later in finalize steps.

---

1183-1183: Refine partial result usage.

Capturing gInfo from runTx but disregarding other return values is valid here, but confirm that ignoring partial results (e.g., result or anteEvents) is intentional when verifying proposals.

---

1185-1185: Provide context for error-based returns.

When returning nil, 0, err on transaction verification failure, consider logging additional info or incrementing a counter. This can help diagnose repeated invalid proposals during consensus.

x/bank/keeper/virtual.go (3)

1-13: Confirm all import dependencies.

All lines are new. Ensure each imported package is necessary and used (e.g., bytes, hex). Unused imports can introduce clutter and confusion.

---

15-25: Consolidate error handling & logs.

SendCoinsFromAccountToModuleVirtual panics if the module account does not exist. A panic can be valid for a misconfigured chain, but you might consider returning an error to allow graceful recovery in certain scenarios. If panic is desired, ensure a system-level rationale is documented.

---

43-63: Ensure correct event emission and rollback logic.

SendCoinsToVirtual transfers amounts into a transient store. If a subsequent step fails (e.g., a post handler or a chain-level invariance check), confirm that the partial coins added to the virtual store can be rolled back or reconciled. Emitting standard “send” events is logical, but ensure it does not confuse block explorers about finality.

x/gov/testutil/expected_keepers_mocks.go (1)

813-826: Replicate error handling expectations.

SendCoinsFromAccountToModuleVirtual mock must match the real keeper logic—panic on missing module accounts vs. returning errors for blocked addresses. Ensure the mock logic is aligned and that tests expecting errors vs. panics are well-delineated.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f0db156 and 7be9b7c.

📒 Files selected for processing (81)

• .github/workflows/test.yml (7 hunks)
• .golangci.yml (0 hunks)
• CHANGELOG.md (1 hunks)
• baseapp/abci.go (4 hunks)
• baseapp/abci_utils.go (7 hunks)
• baseapp/baseapp.go (11 hunks)
• baseapp/genesis.go (1 hunks)
• baseapp/options.go (2 hunks)
• baseapp/test_helpers.go (1 hunks)
• baseapp/testutil/mock/mocks.go (2 hunks)
• baseapp/txexecutor.go (1 hunks)
• crypto/keyring/keyring.go (2 hunks)
• runtime/module.go (2 hunks)
• runtime/store.go (1 hunks)
• server/mock/store.go (2 hunks)
• simapp/app.go (6 hunks)
• simapp/app_config.go (1 hunks)
• store/CHANGELOG.md (1 hunks)
• store/cachekv/internal/btree.go (0 hunks)
• store/cachekv/internal/mergeiterator.go (11 hunks)
• store/cachekv/search_benchmark_test.go (0 hunks)
• store/cachekv/search_test.go (0 hunks)
• store/cachekv/store.go (2 hunks)
• store/cachekv/store_test.go (1 hunks)
• store/cachemulti/store.go (5 hunks)
• store/cachemulti/store_test.go (2 hunks)
• store/dbadapter/store.go (0 hunks)
• store/dbadapter/store_test.go (0 hunks)
• store/gaskv/store.go (2 hunks)
• store/gaskv/store_test.go (0 hunks)
• store/iavl/store.go (0 hunks)
• store/iavl/store_test.go (1 hunks)
• store/internal/btree/btree.go (1 hunks)
• store/internal/btree/btree_test.go (3 hunks)
• store/internal/btree/memiterator.go (6 hunks)
• store/internal/btreeadaptor.go (1 hunks)
• store/listenkv/store.go (2 hunks)
• store/listenkv/store_test.go (1 hunks)
• store/mem/mem_test.go (0 hunks)
• store/mem/store.go (0 hunks)
• store/prefix/store.go (9 hunks)
• store/prefix/store_test.go (0 hunks)
• store/rootmulti/proof_test.go (1 hunks)
• store/rootmulti/store.go (17 hunks)
• store/rootmulti/store_test.go (6 hunks)
• store/tracekv/store.go (2 hunks)
• store/tracekv/store_test.go (1 hunks)
• store/transient/store.go (1 hunks)
• store/types/iterator_test.go (1 hunks)
• store/types/store.go (9 hunks)
• store/types/validity.go (2 hunks)
• tests/integration/bank/keeper/deterministic_test.go (2 hunks)
• tests/integration/distribution/keeper/msg_server_test.go (2 hunks)
• tests/integration/evidence/keeper/infraction_test.go (2 hunks)
• tests/integration/gov/keeper/keeper_test.go (2 hunks)
• tests/integration/slashing/keeper/keeper_test.go (2 hunks)
• tests/integration/staking/keeper/common_test.go (2 hunks)
• tests/integration/staking/keeper/determinstic_test.go (2 hunks)
• testutil/context.go (1 hunks)
• testutil/integration/example_test.go (2 hunks)
• testutil/integration/router.go (1 hunks)
• types/context.go (7 hunks)
• types/mempool/mempool.go (2 hunks)
• types/mempool/mempool_test.go (1 hunks)
• types/mempool/noop.go (1 hunks)
• types/mempool/priority_nonce.go (7 hunks)
• types/mempool/priority_nonce_test.go (3 hunks)
• types/mempool/sender_nonce.go (4 hunks)
• types/mempool/sender_nonce_property_test.go (1 hunks)
• x/bank/abci.go (1 hunks)
• x/bank/keeper/collections_test.go (2 hunks)
• x/bank/keeper/keeper.go (4 hunks)
• x/bank/keeper/keeper_test.go (11 hunks)
• x/bank/keeper/send.go (6 hunks)
• x/bank/keeper/virtual.go (1 hunks)
• x/bank/module.go (4 hunks)
• x/bank/types/keys.go (1 hunks)
• x/bank/types/restrictions_test.go (30 hunks)
• x/gov/testutil/expected_keepers_mocks.go (3 hunks)
• x/group/internal/orm/testsupport.go (1 hunks)
• x/tx/internal/testpb/signers.proto (2 hunks)

💤 Files with no reviewable changes (11)

• store/dbadapter/store.go
• store/dbadapter/store_test.go
• store/mem/mem_test.go
• store/mem/store.go
• store/cachekv/search_benchmark_test.go
• store/iavl/store.go
• store/cachekv/search_test.go
• store/prefix/store_test.go
• store/gaskv/store_test.go
• .golangci.yml
• store/cachekv/internal/btree.go

🧰 Additional context used

🪛 GitHub Check: CodeQL

x/bank/abci.go

[warning] 11-11: Panic in BeginBock or EndBlock consensus methods
path flow from Begin/EndBlock to a panic call
path flow from Begin/EndBlock to a panic call
path flow from Begin/EndBlock to a panic call
path flow from Begin/EndBlock to a panic call
path flow from Begin/EndBlock to a panic call

store/types/validity.go

[warning] 41-41: Panic in BeginBock or EndBlock consensus methods
Possible panics in BeginBock- or EndBlock-related consensus methods could cause a chain halt

testutil/integration/router.go

[warning] 188-190: Iteration over map
Iteration over map may be a possible source of non-determinism

store/cachemulti/store.go

[warning] 142-144: Iteration over map
Iteration over map may be a possible source of non-determinism

---

[warning] 169-169: Panic in BeginBock or EndBlock consensus methods
Possible panics in BeginBock- or EndBlock-related consensus methods could cause a chain halt

---

[warning] 196-196: Panic in BeginBock or EndBlock consensus methods
Possible panics in BeginBock- or EndBlock-related consensus methods could cause a chain halt

---

[warning] 212-214: Iteration over map
Iteration over map may be a possible source of non-determinism

---

[warning] 231-236: Iteration over map
Iteration over map may be a possible source of non-determinism

---

[warning] 239-246: Iteration over map
Iteration over map may be a possible source of non-determinism

store/internal/btreeadaptor.go

[warning] 40-40: Panic in BeginBock or EndBlock consensus methods
Possible panics in BeginBock- or EndBlock-related consensus methods could cause a chain halt

---

[warning] 48-48: Panic in BeginBock or EndBlock consensus methods
Possible panics in BeginBock- or EndBlock-related consensus methods could cause a chain halt

store/rootmulti/store.go

[warning] 670-670: Panic in BeginBock or EndBlock consensus methods
Possible panics in BeginBock- or EndBlock-related consensus methods could cause a chain halt

🪛 GitHub Actions: Lint

x/bank/abci.go

[error] 12-12: could not import github.com/cosmos/cosmos-sdk/types

baseapp/abci.go

[error] 12-12: could not import github.com/cosmos/cosmos-sdk/types

🪛 GitHub Check: golangci-lint

runtime/module.go

[failure] 229-229:
undefined: storetypes.ObjectStoreKey (typecheck)

🪛 golangci-lint (1.62.2)

runtime/module.go

229-229: undefined: storetypes.ObjectStoreKey

(typecheck)

store/cachekv/store.go

5-5: import 'cosmossdk.io/store/internal/btree' is not allowed from list 'Main'

(depguard)

---

6-6: import 'cosmossdk.io/store/types' is not allowed from list 'Main'

(depguard)

store/gaskv/store.go

[warning] 24-24: unused-parameter: parameter 'v' seems to be unused, consider removing or renaming it as _

(revive)

🪛 LanguageTool

store/CHANGELOG.md

[uncategorized] ~35-~35: You might be missing the article “the” here.
Context: ...ull/243) Support RunAtomic API to use new CoW cache store. * [cosmos#244](https://githu...

(AI_EN_LECTOR_MISSING_DETERMINER_THE)

---

[uncategorized] ~36-~36: You might be missing the article “the” here.
Context: ...ypto-org-chain/cosmos-sdk/pull/244) Add Discard method to CacheWrap to discard ...

(AI_EN_LECTOR_MISSING_DETERMINER_THE)

🪛 GitHub Actions: CodeQL

types/context.go

[error] 397-397: undefined: storetypes.ObjKVStore

🔇 Additional comments (220)

.github/workflows/test.yml (1)

294-294: Go version upgraded to 1.21 across workflow jobs

This change updates the Go version from 1.20 to 1.21 for the CI workflow, ensuring compatibility with the EVM support changes. The update is consistently applied across all jobs in the workflow file.

crypto/keyring/keyring.go (2)

62-63: New API added to expose the underlying keyring database

The addition of the DB() method to the Keyring interface provides access to the underlying database keyring, which is likely needed for EVM integration. This is a focused change that aligns with the PR objective of adding EVM support.

---

263-266: Straightforward implementation of the DB() method

The implementation simply returns the internal db field, providing direct access to the underlying keyring database without adding unnecessary complexity.

store/internal/btreeadaptor.go (2)

45-51: Consider handling errors instead of panicking.

Similar to the Iterator method, panics in ReverseIterator could cause chain halts during consensus. Consider returning errors instead of panicking.

-func (ts *BTreeStore[V]) ReverseIterator(start, end []byte) types.GIterator[V] {
+func (ts *BTreeStore[V]) ReverseIterator(start, end []byte) (types.GIterator[V], error) {
 	it, err := ts.BTree.ReverseIterator(start, end)
 	if err != nil {
-		panic(err)
+		return nil, err
 	}
-	return it
+	return it, nil
}

🧰 Tools

🪛 GitHub Check: CodeQL

[warning] 48-48: Panic in BeginBock or EndBlock consensus methods
Possible panics in BeginBock- or EndBlock-related consensus methods could cause a chain halt

---

14-19:

❓ Verification inconclusive

Consider implementing write operations for a complete KVStore.

The BTreeStore implementation only provides read operations and CacheWrap. If this is intended to be a complete KVStore implementation, it should also include write operations like Set and Delete.

If this is intentionally read-only, please document this limitation.

---

🏁 Script executed:

#!/bin/bash
# Check if the KVStore interface requires write operations
grep -A 20 "type KVStore" $(find . -name "types.go" | grep store/types)

Length of output: 72

---

KVStore Write Operations Clarification Needed

The BTreeStore implementation currently supports only read operations (and a caching wrapper). However, our re-verification attempts did not conclusively reveal a definition for the KVStore interface that mandates write operations (such as Set and Delete). To move forward:

• Manual Confirmation: Please verify whether the intended design of the KVStore interface includes write operations.
• Documentation: If a read-only implementation is deliberate, consider documenting this limitation explicitly.

store/listenkv/store.go (1)

4-4: Added required cachekv import

The addition of this import is necessary for the new implementation of the CacheWrap method that uses cachekv.NewStore().

store/tracekv/store.go (2)

9-9: Import added for cachekv package

The import for the cachekv package has been added to support the modified CacheWrap implementation.

---

168-168:

✅ Verification successful

Behavioral change: CacheWrap now returns a cache store instead of panicking

The implementation of CacheWrap has been changed from a panic-inducing function to one that returns a proper cache store wrapping the current trace store. This is a significant behavioral change that could impact code that was expecting the previous panic behavior.

However, this change generally improves the API by making it more consistent with other store implementations and avoiding runtime panics.

Let's check if there's any code relying on the previous panic behavior:

---

🏁 Script executed:

#!/bin/bash
# Search for any code that might be catching panics when calling CacheWrap on TraceKVStore
rg -A 3 "defer func\(\)" | rg -B 3 "recover\(\)" | rg -C 5 "TraceKVStore.*CacheWrap"

Length of output: 86

---

I'll generate an additional shell script to search for any references to CacheWrap in the codebase. We want to confirm no code segments are implicitly relying on its previous panic behavior.

Could you please run this script and share the output?

---

🏁 Script executed:

#!/bin/bash
# Search for any references to CacheWrap usage in the repository
rg -n "CacheWrap(" .

Length of output: 93

---

Let's run a revised shell script that properly escapes the parenthesis to search for any references to CacheWrap( in the repository. Please run the following script and share its output:

This will help us verify if any parts of the codebase rely on the previous panic behavior by checking for references to CacheWrap(. Once we have this information, we can finalize the review comment.

---

🏁 Script executed:

#!/bin/bash
# Search using an escaped parenthesis for CacheWrap usage in the repository
rg -n "CacheWrap\\(" .

Length of output: 2828

---

Verified: TraceKV CacheWrap Behavior Change is Safe

After reviewing the repository:

• The code using the tracekv store’s CacheWrap does not appear to rely on panic handling.
• A repository-wide search for panic-handling patterns around CacheWrap did not reveal any usage tied to the tracekv implementation.
• Note that while one test in the gaskv store (in store/gaskv/store_test.go) still expects CacheWrap to panic, this behavior is specific to that store and does not affect tracekv.

Based on this verification, the change in tracekv—returning a proper cache store instead of panicking—is confirmed as safe and a beneficial API improvement.

runtime/store.go (2)

146-152: Return type changed from dbm.Iterator to storetypes.Iterator

The return type of the Iterator method has been updated to use storetypes.Iterator instead of dbm.Iterator, which improves consistency with the rest of the codebase.

---

154-160: Return type changed from dbm.Iterator to storetypes.Iterator

The return type of the ReverseIterator method has been updated to use storetypes.Iterator instead of dbm.Iterator, which improves consistency with the rest of the codebase.

store/rootmulti/proof_test.go (1)

18-18: Function signature updated for EVM support

The iavl.LoadStore function call has been updated to include an additional boolean parameter (false), consistent with the changes in other files.

store/types/iterator_test.go (1)

18-18:

✅ Verification successful

Function signature updated for EVM support

The iavl.LoadStore function call has been updated to include an additional boolean parameter (false), consistent with the changes in other files.

Since this boolean parameter is consistently set to false across all function calls in the codebase, it might be worth checking if this should be the default value in the function definition to reduce the impact of this API change. This would make the function backward compatible by providing a default value.

---

🏁 Script executed:

#!/bin/bash
# Find the LoadStore function definition
rg "func LoadStore" --type go

Length of output: 500

---

Action: Confirm consistency and consider API refactoring for defaults

We've verified that the function signature in store/iavl/store.go requires two boolean parameters (disableFastNode and commitSync), and all calls—including the one in store/types/iterator_test.go—are consistently passing false for both. The update to support EVM by explicitly adding the extra boolean is applied uniformly across the codebase.

Some points to note:

• Current implementation: The new boolean parameter is explicitly provided as false in all calls.
• Default parameters in Go: Since Go does not support default parameter values, any change to make these booleans default would require refactoring (e.g., via an options struct or helper function) rather than a simple signature change.
• Recommendation: While the explicit passing of false is correct and consistent, you might consider a separate refactor to encapsulate these defaults if backward compatibility and API simplicity are of high priority.

x/bank/types/keys.go (1)

23-25: Approve addition of ObjectStoreKey constant

The constant is correctly defined as a concatenation of "object:" prefix and the module name. This new key will be used for the object store in the bank module, which is essential for EVM support.

simapp/app_config.go (1)

124-126: Adding bank module to EndBlockers is appropriate for EVM support

Including the bank module in the EndBlockers slice ensures that bank module operations will be executed at the end of each block. This is essential for EVM support, particularly for settling transactions involving virtual accounts and ensuring state consistency between Cosmos and EVM environments.

runtime/module.go (1)

70-70: Verify that ProvideObjectStoreKey is correctly integrated

The function is properly added to the list of providers in the init function, but the underlying type issue needs to be resolved first.

store/cachekv/store_test.go (1)

450-462: Test looks good for the new branching functionality.

The test effectively validates that the branch store can maintain independent state changes and properly restore them to the original store. This is essential for supporting atomic operations with the new RunAtomic API.

baseapp/txexecutor.go (1)

11-16: Good introduction of the TxExecutor abstraction.

The type definition is well-structured to support parallel transaction execution. The function signature provides the necessary context, block size, and store parameters while allowing customized transaction processing via the callback.

baseapp/options.go (2)

132-135: Properly follows existing pattern for BaseApp option functions.

This function correctly implements the functional options pattern used throughout the codebase, allowing the transaction executor to be set during BaseApp initialization.

---

407-410:

❓ Verification inconclusive

Standard direct setter implementation.

The method implementation complements the functional option and follows the established pattern in the codebase. No sealed check is implemented, which differs from some other setters - verify if this is intentional.

---

🏁 Script executed:

#!/bin/bash
# Check if other setter methods that modify txExecutor have sealed checks

echo "Checking for sealed checks in other setter methods..."
rg "func \(app \*BaseApp\) Set.*\(" baseapp/options.go | grep -v "SetTxExecutor" | rg "if app\.sealed"

Length of output: 219

---

Setter Pattern Consistency Check
The implementation of SetTxExecutor in baseapp/options.go follows the direct setter pattern seen elsewhere in the codebase. A search for sealed checks in other setter methods within this file did not reveal any such conditions, implying that omitting a sealed check here is likely intentional. However, please manually verify that no additional sealed state safeguards are required for txExecutor compared to similar setters.

testutil/integration/example_test.go (2)

40-40: Updated CreateMultiStore call to support object stores.

The change accommodates the new signature that includes object store keys, making the API more flexible for different store types. Passing nil maintains backward compatibility.

---

129-129: Consistent update to CreateMultiStore call.

The same change is applied here for consistency, properly updating all instances of the function call to match the new signature.

testutil/context.go (1)

82-95: Confirm that mounting with a nil DB is intended.

Lines 87-88 mount the transient and object stores with nil. This might be a valid pattern for transient or object stores, but please verify that no persistent storage is needed. If ephemeral usage is intended, this is fine. Otherwise, consider passing the same in-memory DB instance for consistency.

store/types/validity.go (2)

3-4: New “errors” import looks good.

No issues found with importing the standard library errors package for improved error messaging.

---

27-28: Delegating validation to AssertValidValueLength is a clean refactor.

Great job centralizing the length check into a dedicated function for better maintainability.

store/cachemulti/store_test.go (5)

4-4: Imports look good.

The usage of "errors" is appropriate for creating custom error instances in this test suite.

---

8-8: Database import is fine.

Introducing dbm "github.com/cosmos/cosmos-db" aligns with typical usage for MemDB in tests.

---

11-13: New store imports are coherent.

Adding dbadapter, internal, and internal/btree from cosmossdk.io/store is consistent with your chosen storage approach.

---

31-63: Solid coverage for atomic operations.

The test correctly verifies that changes within RunAtomic are committed or discarded depending on success or failure. This ensures transactional integrity across both standard KV and object stores.

---

65-88: Branch store test approach is valid.

Verifying snapshot creation and restoration demonstrates correct rollback of both KV and object store state. The logic is sound and provides good test coverage for branch functionality.

types/mempool/mempool.go (4)

15-20: Constructor design is straightforward.

NewMempoolTx cleanly initializes the new Tx struct with required fields.

---

22-24: GasTx interface addition is consistent.

Defining GasTx for retrieving a transaction's gas usage helps unify gas-related handling across mempool code.

---

31-32: Expanded Mempool interface is valid.

InsertWithGasWanted provides a needed extension for inserting transactions with custom gas fields.

---

56-56:

❓ Verification inconclusive

Check for downstream references to updated method signature.

Changing the return type from sdk.Tx to Tx might break existing consumers. Ensure all callers are updated accordingly.

Run the following script to verify if there are any references to the old method signature:

---

🏁 Script executed:

#!/bin/bash
# Searching for references to the old Tx() sdk.Tx signature:
rg 'Tx\s*()\s*sdk\.Tx' -A 5

Length of output: 3794

---

Action Required: Verify Consumer Compatibility with New Tx Type

The updated method now returns our local Tx instead of sdk.Tx. Our search did not uncover any direct calls to a method signature of Tx() sdk.Tx; however, multiple downstream components still reference sdk.Tx (for instance, in files like types/mempool/mempool.go, types/mempool/priority_nonce.go and its tests, as well as several modules in the x/auth and baseapp packages). Although the new signature doesn’t appear to be directly misreferenced, please verify that all consumers that previously expected an sdk.Tx are either updated to work with your new Tx or perform the appropriate conversion.

• Check in types/mempool/mempool.go: Ensure that functions like NewMempoolTx correctly translate between sdk.Tx (as passed in) and the new Tx type.
• Review in Priority Nonce Files: Confirm that any usage of TxReplacement and test cases in types/mempool/priority_nonce.go and types/mempool/priority_nonce_test.go correctly account for the change.
• Examine References in Auth and Baseapp Modules: Verify that wrapper functions and type assertions expecting an sdk.Tx are updated or handle conversions appropriately.

testutil/integration/router.go (1)

188-190:

✅ Verification successful

Possible non-deterministic map iteration.

Iterating over okeys without a deterministic order is safe unless these mounts must occur in a fixed sequence for consensus-critical logic. If so, consider sorting the map keys before iteration.

---

🏁 Script executed:

#!/bin/bash
# Gather references to CreateMultiStore to check if it impacts consensus flow.
rg -A 5 'CreateMultiStore'

Length of output: 4993

---

Consensus ordering verified as safe; no changes needed.

After reviewing how CreateMultiStore is used—particularly across multiple deterministic integration tests—it appears that even though iterating over the map (i.e. okeys) is inherently non-deterministic, the mounting order does not impact the consensus-critical logic. The tests (e.g., those in tests/integration/bank/keeper/deterministic_test.go and similar files) validate that the final state and commit behavior remain consistent. Thus, unless there’s an explicit requirement elsewhere to enforce key order, sorting the keys before iteration is unnecessary.

🧰 Tools

🪛 GitHub Check: CodeQL

[warning] 188-190: Iteration over map
Iteration over map may be a possible source of non-determinism

baseapp/abci.go (7)

341-350: Switch-based CheckTxType parsing is clearer.

Refactoring to a switch statement for handling CheckTxType_New and CheckTxType_Recheck improves readability.

---

799-805: Context now retains TxCount.

Assigning the number of transactions to the context is helpful for advanced logic (e.g., metrics) without complicating the existing block execution flow.

---

812-816: executeTxs extraction is a good modular approach.

Delegating transaction execution logic to app.executeTxs simplifies readability and maintains separation of concerns in internalFinalizeBlock.

---

866-903: executeTxs method represents a clean fallback/parallel architecture.

Using txExecutor for parallel execution if available, else defaulting to sequential processing is a flexible design. The fallback path correctly handles malformed transactions by still returning default results to CometBFT.

---

1255-1255: Use of MultiStore assignment ensures proper fallback.

Redirecting qms = storetypes.MultiStore(app.cms) is valid for query contexts when a custom query multistore isn't provided.

---

822-841:

✅ Verification successful

Validate block gas usage summation.

Accumulatively computing blockGasUsed and blockGasWanted per txResults seems correct. However, there's a small risk of overflow if the chain imposes extremely large block size or gas values. Verify bounds are enforced upstream.

---

🏁 Script executed:

#!/bin/bash
# Check how block gas limits are enforced to avoid potential uint64 overflow.
rg -A 5 "block-max-gas" 
rg -A 5 "maxGas"

Length of output: 3167

---

Block Gas Summation Safety Verified

The code correctly accumulates gas values, and our review confirms that upstream bounds are enforced. The consensus parameters in baseapp/baseapp.go (via maxGas checks and meter initialization) ensure that the maximum gas is controlled. Thus, the risk of overflow in the accumulation of blockGasUsed and blockGasWanted is effectively mitigated.

---

12-12:

✅ Verification successful

Verify import resolution for pipeline error.

The pipeline reports an import error: "could not import github.com/cosmos/cosmos-sdk/types". Confirm it's present in go.mod and successfully pulled.

---

🏁 Script executed:

#!/bin/bash
#!/bin/bash
go mod tidy
go list -m all | grep 'github.com/cosmos/cosmos-sdk'

Length of output: 13729

---

Import Resolution Verified: Dependency Found

The verification confirms that the github.com/cosmos/cosmos-sdk dependency is present in the go.mod file and was successfully pulled after running go mod tidy. The pipeline error regarding import resolution appears to be resolved.

🧰 Tools

🪛 GitHub Actions: Lint

[error] 12-12: could not import github.com/cosmos/cosmos-sdk/types

store/internal/btree/btree_test.go (8)

1-1: Package name change noted.
No specific issues with setting package btree.

---

12-17: Verified initialization and first Get.
The use of NewBTree[[]byte]() and checking (value, found) is correct. The tests cleanly assert non-existent keys return (nil, false).

---

22-28: Checked multiple Set/Get validations.
These lines correctly verify that previously set keys return (non-nil, true). No issues.

---

36-44: Checked the Delete logic.
Tests confirm that deleted keys revert to (nil, false). This is a good coverage for the B-tree's deletion.

---

45-45: Function closing brace.
No meaningful changes here.

---

47-52: Test for nil value storage.
Storing nil as a value is well-tested by verifying (value, found) == (nil, true).

---

56-56: DBIterator test initialization.
Using NewBTree[[]byte]() for this test scenario is consistent with the rest.

---

186-186: Second B-tree initialization.
No issues noted with creating db2; presumably for separate test coverage or edge cases.

server/mock/store.go (1)

15-15: Interface assertion updated to CommitMultiStore.
This ensures multiStore implements storetypes.CommitMultiStore. Looks fine as long as all required methods are defined.

types/context.go (5)

5-5: Potential new import for errors.
Ensuring standard error usage is fine here.

---

107-111: Accessors for new context fields.
These getters are straightforward. Confirm usage consistency across the codebase.

---

158-159: Negative initialization of txIndex and msgIndex.
Using -1 to represent "uninitialized" is valid. Ensure that APIs handle these negative values gracefully.

---

339-362: Setter methods for the new context fields.
All new setters (WithTxIndex, WithTxCount, etc.) are consistent. Keep an eye on potential concurrency if contexts are used in parallel.

---

417-435: New RunAtomic method for atomic updates.
Implementation allows atomic callback commits. Consider verifying event emission if the callback fails. Otherwise it looks solid.

store/rootmulti/store_test.go (11)

20-20: New import tracekv.
No issues adding "cosmossdk.io/store/tracekv" for store tracing.

---

790-790: New store key for store4.
Defines testStoreKey4 = types.NewKVStoreKey("store4"). This is consistent with other test keys.

---

862-872: Refactored hashStores function.

func hashStores(stores map[types.StoreKey]types.CommitStore) []byte {
	// ...
}

Using CommitStore instead of CommitKVStore is consistent with recent interface updates. The hashing approach is fine.

---

917-918: commitStoreStub uses CommitStore.

type commitStoreStub struct {
	types.CommitStore
	Committed int
}

Switching from CommitKVStore to CommitStore aligns with the updated interface.

---

922-923: Overriding Commit method in commitStoreStub.

func (stub *commitStoreStub) Commit() types.CommitID {
	commitID := stub.CommitStore.Commit()
	// ...
}

This stub pattern is clear; it increments Committed after delegating to the real store.

---

928-928: Signature for prepareStoreMap.
Introduces a function returning map[types.StoreKey]types.CommitStore.

---

934-935: Mounting memory store and object store.

store.MountStoreWithDB(types.NewMemoryStoreKey("mem1"), types.StoreTypeMemory, nil)
store.MountStoreWithDB(types.NewObjectStoreKey("obj1"), types.StoreTypeObject, nil)

Ensure these new store types are supported by consumers of prepareStoreMap().

---

939-942: Returning commitStoreStub for testStoreKey1.
Each stub wraps a newly fetched store reference. Good approach for testing.

---

943-944: Returning commitStoreStub for testStoreKey2.
No issues; consistent with the pattern above.

---

946-947: Returning commitStoreStub for testStoreKey3.
This one wraps a Transient store. Logic is consistent.

---

949-950: Returning commitStoreStub for testStoreKey4.
This one references the object store, aligning with the newly mounted store.

baseapp/testutil/mock/mocks.go (2)

130-136:

❓ Verification inconclusive

Ensure interface alignment with new return type.

The mock now reflects the updated signature returning an additional uint64. Verify that all real implementations and test calls align with these changes to avoid compile-time or runtime errors.

Run the following script to locate all references to ProcessProposalVerifyTx in the code and confirm the correct usage:

---

🏁 Script executed:

#!/bin/bash
ast-grep --pattern $'ProcessProposalVerifyTx($_)'

Length of output: 50

---

Action Required: Manual Verification for Interface Consistency

The initial ast-grep search did not return any references to ProcessProposalVerifyTx, so we couldn’t confirm usage automatically. Please manually verify that all implementations and test calls that rely on this function are updated to align with the new signature (returning types.Tx, uint64, error). In particular, ensure that:

• The updated mock in baseapp/testutil/mock/mocks.go at lines 130-136 is compatible with its real implementations.
• All tests invoking ProcessProposalVerifyTx correctly handle the additional uint64 return parameter.

---

211-221:

❓ Verification inconclusive

Confirm consistency for added gasWanted parameter.

The SelectTxForProposal mock method includes the new gasWanted parameter, which matches the changes in the real interface. Confirm that all test code passing arguments to this mock is updated accordingly and that no caller is missing the additional argument.

---

🏁 Script executed:

#!/bin/bash
ast-grep --pattern $'SelectTxForProposal($_, $_, $_, $_, $_, $_)'

Length of output: 66

---

Attention: Verify SelectTxForProposal Call Sites for the New gasWanted Parameter

Please ensure that all test code calling this mock method now passes the additional gasWanted argument. Although the automated search did not reveal any discrepancies, it is recommended to manually verify that no caller is still using an outdated five-parameter call.

• Confirm that every invocation of SelectTxForProposal(...) in tests (or other dependent code) lists six arguments as per the updated signature.
• Double-check the test files and any other consumers of this mock to avoid potential runtime mismatches.

store/internal/btree/memiterator.go (9)

17-18: Validate btree generic usage.

The memIterator[V any] and iter btree.IterG[item[V]] usage is correct in principle, but ensure all reading/writing to this iterator remains thread-safe and is not used concurrently without locks.

---

26-38: Initialization logic looks good.

The constructor properly handles the case where items.tree is nil, returning an iterator flagged as invalid. This is a reasonable approach for an empty btree.

---

61-67: Smart check for initial validity.

Ensuring the iterator starts valid and is revalidated with keyInRange is a good pattern.

---

85-85: Check invalid usage.

Error() returns an error when !mi.Valid(). This works for typical iteration patterns, but carefully handle any edge scenarios where Error() is called after closing.

---

92-92: Valid function is straightforward.

Returning mi.valid directly is typical. Ensure correctness is maintained if future logic toggles it unexpectedly.

---

96-96: Next operation depends on correct initial validity.

Any concurrency or modification to the underlying btree during iteration could invalidate Next(). Confirm no concurrent writes occur.

---

110-110: Boundary checks are correct.

keyInRange properly handles ascending/descending. This is crucial for correct domain-limited iteration.

---

120-120: Key retrieval is clean.

Key() returning mi.iter.Item().key works well for real-time iteration. Verify item existence before usage.

---

124-124: Value typed as generic V.

This aligns with the new generic design. Ensure the calling code gracefully handles the typed value.

baseapp/abci_utils.go (4)

203-203: Interface expansion approved.

ProcessProposalVerifyTx now returns (sdk.Tx, uint64, error) to provide gasWanted. This aligns with the mock changes and ensures consistent gas-handling logic.

---

273-279: Compute txGasLimit from GasTx.

This block extracts the gas limit if tx implements the mempool.GasTx interface. Confirm that the EVM transactions also implement or adapt this interface properly.

---

391-391: New gasWanted in process proposal.

The code now obtains gasWanted here. Confirm that out-of-scope transactions have sensible default or zero gas values.

---

397-398: Accumulate gas for block limit.

totalTxGas += gasWanted is straightforward. Validates that the sum doesn’t exceed maxBlockGas. All good for gating enormous EVM transactions as well.

store/internal/btree/btree.go (1)

80-89: Clarify copy semantics.
The Copy() method leverages bt.tree.Copy(), which is a shadow copy (copy-on-write). It's efficient, but be sure callers are aware that mutating one copy may affect the other until a write actually happens, since this is not a deep copy of stored values.

store/rootmulti/store.go (3)

1027-1091: Ensure consistency with new store types in loadCommitStoreFromParams.
The switch statement cleanly handles all store types, including the new StoreTypeObject. This is correct, but be cautious about future expansions. If new store types are introduced without updating here, store loading might silently fail.

---

1237-1239: Exclude ephemeral store types from CommitInfo.
In commitStores(), filtering out StoreTypeTransient, StoreTypeMemory, and StoreTypeObject from the commit info is a good design choice, preventing ephemeral store data from persisting. Just ensure that any new ephemeral types introduced follow this same pattern.

---

683-695:

✅ Verification successful

Confirm necessity and usage of ObjKVStore.
GetObjKVStore() is newly introduced, raising the possibility of ephemeral or object-based storage. Ensure all code paths that retrieve this store type properly handle the specialized semantics of object-based data.

Please verify whether you have corresponding usage sites or tests for GetObjKVStore by searching in the codebase:

---

🏁 Script executed:

#!/bin/bash
# Searching for references to GetObjKVStore
rg "GetObjKVStore"

Length of output: 1252

---

Verified: ObjKVStore usage is correctly implemented and exercised across the codebase.

After verifying the references to GetObjKVStore, we found that:

• Usage in Production Code: The function is used in types/context.go, store/types/store.go, and server/mock/store.go, demonstrating its role in object-based store management.
• Test Coverage: The tests in store/cachemulti/store_test.go explicitly exercise the functionality (e.g., setting and retrieving values) to ensure that the specialized semantics of object-based storage are handled correctly.

No additional changes are needed at this point. Please ensure that any future modifications to the store’s behavior are accompanied by updated tests and documentation.

store/transient/store.go (1)

19-21: Leverage concurrency note for BTreeStore usage.
GStore[V] embeds internal.BTreeStore[V], which may not be thread-safe if used concurrently. If concurrency is expected, consider clarifying usage patterns or adding synchronization at a higher level.

store/cachemulti/store.go (7)

38-39: No immediate concerns.
The simplified parameters for NewFromKVStore appear consistent.

---

66-75: Confirm necessity of NewFromParent.
This function seems unrelated to direct EVM integration. Consider moving this feature into a dedicated PR if it’s not required for the EVM scope.

---

141-145: (Duplicate) Iteration over map may be non-deterministic.
Because we’re iterating over cms.stores (a map), it can cause non-determinism if order of finalization matters for consensus.

🧰 Tools

🪛 GitHub Check: CodeQL

[warning] 142-144: Iteration over map
Iteration over map may be a possible source of non-determinism

---

152-152: No critical issues.
This override for CacheWrapWithTrace simply defers to CacheWrap; it looks consistent with existing logic.

---

159-160: Check necessity of returning a parent-based store.
CacheMultiStore now depends on NewFromParent. Confirm that this is EVM-related or move it to a different PR if not.

---

210-223: Cloning logic.
This shallow copy approach is efficient. Just verify all branched sub-stores can handle the cloned references safely, especially in EVM flows.

🧰 Tools

🪛 GitHub Check: CodeQL

[warning] 212-214: Iteration over map
Iteration over map may be a possible source of non-determinism

---

225-247: Ensure final consistency when restoring branched stores.
Restore discards missing stores and merges the existing ones. Validate that partial merges don’t break EVM invariants if EVM data is in either store.

🧰 Tools

🪛 GitHub Check: CodeQL

[warning] 231-236: Iteration over map
Iteration over map may be a possible source of non-determinism

---

[warning] 239-246: Iteration over map
Iteration over map may be a possible source of non-determinism

store/cachekv/store.go (9)

5-5: Evaluate import restrictions.
Importing cosmossdk.io/store/internal/btree could violate depguard rules if the package is restricted. Confirm compliance with your module’s guidelines.

🧰 Tools

🪛 golangci-lint (1.62.2)

5-5: import 'cosmossdk.io/store/internal/btree' is not allowed from list 'Main'

(depguard)

---

9-15: Alias approach for generics.
Using Store = GStore[[]byte] plus interface assertions clarifies the generic design. This looks cohesive and maintainable.

---

25-36: Assess usage boundaries for valueLen.
valueLen is used for validation. Confirm that you won’t exceed EVM’s constraints for large contract data.

---

53-60: Cloning technique.
Shallow-copying writeSet using Copy() is efficient. Confirm that any subsequent modifications in a branch store won’t inadvertently affect EVM state.

---

68-71: Restore branch logic.
Reassigning writeSet from another branch store merges caches. Check for potential concurrency collisions if EVM uses parallel access.

---

84-90: Key validation and correctness.
Set must always validate keys and values. This is critical for EVM correctness. It looks fine here.

---

93-101: Has-check short-circuit.
We short-circuit to parent for missing keys. This is typically safe, but ensure EVM’s block-state logic can handle unseen keys consistently.

---

122-124: Discard operation.
Clearing the write set is straightforward. No concerns here.

---

135-145: Iterator logic in ascending/descending scans.
Ensure EVM doesn’t rely on stable iteration ordering across nodes. If it does, consider explicit reordering.

store/types/store.go (6)

139-139: Addition of GetObjKVStore.
This method broadens store retrieval to object-based KV stores. Verify that it’s truly needed for EVM scope.

---

238-238: Renaming interfaces to generic forms.
The new GBasicKVStore interface is more flexible. This is beneficial, but confirm alignment with EVM usage.

---

241-243: Generic typed access.
Using V any for Get and Set is a major design shift. Ensure that EVM’s typical byte-based storage is well-supported with these generics.

---

257-275: Iterator changes for generics.
Iterator and ReverseIterator returning GIterator[V] is a beneficial shift, but confirm that EVM code or libraries don’t rely on older []byte-based iteration.

---

383-384: Introducing StoreTypeObject.
New object store type may not be strictly EVM-related. If it’s not needed for EVM, consider a separate PR.

---

611-623: NewObjectStoreKeys function.
This map-building utility might not be relevant for EVM usage. Please confirm or move into a dedicated changeset.

x/bank/types/restrictions_test.go (4)

415-417: RecordCall now captures only one coin instead of multiple.

Ensure there's no logic gap for multi-coin scenarios. If a single coin is truly sufficient for your use case, this is fine.

---

436-437: Good adaptation of NamedRestriction, NewToRestriction, and ErrorRestriction.

The updated function signatures accommodate a single sdk.Coin parameter consistently. This refactor is well integrated into test calls.

Also applies to: 444-445, 452-453

---

914-915: NoOpSendRestrictionFn test is correct.

Ensures no errors are triggered and the returned address is unmodified even with a single Coin usage. This maintains backward compatibility for scenarios lacking restrictions.

---

402-402:

❓ Verification inconclusive

Confirm the shift from multiple coins to a single coin.

The change from Coins to a single Coin suggests restricting the test coverage to a single denomination. Verify if multi-denomination scenarios are no longer needed, or if separate coverage is elsewhere.

---

🏁 Script executed:

#!/bin/bash
# Searching for other code references to multi-denomination usage in restrictions:
rg -A 3 'Coins\>' --glob '!*_test.go'

Length of output: 118

---

Attention: Please Verify Single-Coin Coverage in Multi-Denomination Context

The changes in x/bank/types/restrictions_test.go (line 402) show a move from using Coins to a single Coin, implying that multi-denomination scenarios might have been phased out. To ensure this shift is intentional:

• Confirm that multi-denomination usage (e.g., references to sdk.Coins) is no longer needed in the non-test code.
• Verify if multi-denomination tests have been relocated or removed.

Please run the following revised shell script to search for any non-test references to multi-denomination usage:

#!/bin/bash
# Searching for references to the multi-denomination type (`sdk.Coins`) in the repository (excluding test files)
rg -A 3 'sdk\.Coins' --glob '!*_test.go'

Once you’ve verified the output, please confirm that the removal of multi-denomination handling is intentional and does not affect other parts of the codebase.

store/prefix/store.go (1)

82-85:

❓ Verification inconclusive

Check consistency of pointer vs. value receivers.

GStore[V] uses value receivers, while other stores sometimes use pointer receivers. Double-check that referencing behavior (especially for CacheWrap) works as expected with this approach.

---

🏁 Script executed:

#!/bin/bash
# Searching for other GStore references to verify consistent usage of pointer vs. value receivers:
rg -A 4 'func \(gs \*GStore'

Length of output: 2476

---

Inconsistent Receiver Types in CacheWrap

The implementation in store/prefix/store.go uses a value receiver for GStore[V] in its CacheWrap method:

func (s GStore[V]) CacheWrap() types.CacheWrap {
	return cachekv.NewGStore(s, s.isZero, s.valueLen)
}

Meanwhile, similar methods in store/gaskv/store.go (including its CacheWrap implementation) use pointer receivers. Please verify that using a value receiver here is intentional and that it doesn’t lead to unintended copying or state management issues. If the discrepancy is by design, consider adding comments or documentation to clarify the rationale; if not, align the receiver types for consistency.

store/cachekv/internal/mergeiterator.go (13)

17-19: Validate generics scope for EVM focus.
These lines introduce a new generic structure and fields. While the implementation appears correct and flexible, it seems unrelated to EVM enablement. Verify whether this belongs in the EVM support PR or if it should be isolated in a separate generics refactor PR.

Also applies to: 23-24

---

27-27: Good interface compliance check.
Defining var _ types.Iterator = (*cacheMergeIterator[[]byte])(nil) confirms the struct implements types.Iterator. This is a recommended practice to enforce interface correctness at compile time.

---

43-45: Domain delegation logic is fine.
Returning the parent’s domain is consistent with the merged view logic. No issues found.

---

48-50: Iterator validity check is standard.
Valid() neatly reflects the iter.valid field. Implementation looks correct.

---

53-77: Next() iteration merging logic is sound.
The approach of advancing either parent or cache (or both) based on key comparison is typical and correct for a merged iterator.

---

80-107: Key retrieval with fallback logic.
Returning the key from cache or parent depending on their validity is correct for a two-iterator merge.

---

110-137: Value retrieval logic matches Key logic.
Similarly, Value() is properly delegated to whichever iterator is active. Implementation is consistent and appears correct.

---

140-147: Graceful resource release.
The Close() method properly closes both iterators. Ok to proceed.

---

151-157: Error method clarifies invalid state.
Returning an error string for an invalid iterator is helpful for debugging.

---

168-174: compare() respects ascending order.
Multiplying by -1 for descending ordering is a straightforward solution. Logic is clear.

---

181-187: Skipping cache deletes until limit.
skipCacheDeletes(until) handles partial iteration. Implementation seems correct for filtered merges.

---

193-238: Robust skip logic in skipUntilExistsOrInvalid().
The loop properly handles parent vs. cache iteration, removing delete entries. Good attention to edge cases.

---

217-217: Delete checks rely on isZero.
Ensuring isZero is well-defined is critical. If isZero incorrectly flags or misses deletions, the merged iterator’s correctness is jeopardized.

Also applies to: 229-229

x/bank/abci.go (1)

11-11: Confirm EVM relevance or separate concerns.
k.CreditVirtualAccounts(ctx) appears unrelated to EVM logic. Given the PR’s EVM scope, confirm if this belongs here or in a separate PR. Also note the static analysis mention of a potential panic path in EndBlock, so ensure this call doesn’t inadvertently panic.

🧰 Tools

🪛 GitHub Check: CodeQL

[warning] 11-11: Panic in BeginBock or EndBlock consensus methods
path flow from Begin/EndBlock to a panic call
path flow from Begin/EndBlock to a panic call
path flow from Begin/EndBlock to a panic call
path flow from Begin/EndBlock to a panic call
path flow from Begin/EndBlock to a panic call

baseapp/genesis.go (1)

16-16: Validate negative index usage.
ba.deliverTx(tx, -1) introduces a negative index. Confirm that the deliverTx method and logic can handle -1 safely without unintended side effects, especially if it’s not strictly tied to adding EVM support.

types/mempool/noop.go (1)

19-20: LGTM: New method for gas management support

The addition of InsertWithGasWanted method to NoOpMempool properly supports the EVM gas model. This implementation maintains the no-op behavior while satisfying the interface requirement.

types/mempool/mempool_test.go (1)

126-126: LGTM: Updated transaction access pattern

The code correctly accesses the transaction via the new Tx field from the wrapper object returned by iterator.Tx(). This change aligns with the EVM support that requires additional transaction metadata.

types/mempool/sender_nonce_property_test.go (1)

99-99: LGTM: Properly updated transaction access pattern

The code correctly accesses the underlying transaction via the new wrapper structure's Tx field before performing the type assertion. This change is consistent with the EVM-related transaction metadata enhancements.

baseapp/test_helpers.go (2)

74-74: LGTM: Enhanced context creation for transaction processing

The call to getContextForTx now includes a transaction index parameter (-1) to support enhanced transaction tracking needed for EVM integration.

---

78-78: LGTM: Consistent context creation with transaction indexing

The change maintains consistency with the updated method signature, properly passing the transaction index parameter (-1) for check transactions as well.

types/mempool/priority_nonce_test.go (3)

389-389: Transaction access pattern updated to support new transaction wrapper

The change correctly adapts the test to work with the new transaction structure, which now wraps the actual transaction in a container that includes additional metadata (likely for gas information needed by EVM).

---

857-857: Updated equality check to match new transaction access pattern

This change correctly updates the assertion to check against the transaction object now wrapped inside the result of iter.Tx().

---

892-892: Adapted test to access transaction through the wrapper structure

The change consistently applies the new transaction access pattern for EVM support.

tests/integration/evidence/keeper/infraction_test.go (3)

86-88: Added object store keys for bank module

This change enables object store support required for EVM functionality in the bank module.

---

92-92: Updated CreateMultiStore to include object stores

The multi-store now properly integrates both KV stores and object stores in the test environment.

---

120-120: Updated BankKeeper initialization with object store key

The bank keeper now correctly utilizes object storage, which is necessary for EVM compatibility.

tests/integration/slashing/keeper/keeper_test.go (3)

57-57: Added object store keys for bank module in slashing tests

Consistent with other test files, this change enables object store support required for EVM functionality.

---

61-61: Updated CreateMultiStore to support object stores

The multi-store now correctly integrates object stores alongside KV stores in the slashing test environment.

---

89-89: Added object store support to BankKeeper initialization

The bank keeper is now properly configured with object storage capabilities needed for EVM support.

simapp/app.go (6)

149-149: Added object store keys field to SimApp struct

This change enables the application to maintain references to object stores, which are required for EVM functionality.

---

273-273: Initialized object store keys for bank module

The bank module's object store key is properly initialized to support EVM functionality.

---

282-282: Stored object store keys in SimApp instance

The initialized object store keys are now correctly stored in the application struct.

---

298-298: Updated BankKeeper initialization with object store

The bank keeper is now properly configured with object storage support, which is necessary for EVM integration.

---

467-467: Added bank module to end blockers

The bank module now participates in end block processing, likely to support EVM state finalization requirements.

---

528-528: Added object stores mounting

Object stores are now properly mounted in the application, enabling access to the new storage type required for EVM support.

x/bank/module.go (4)

19-19: Added missing import for object store types.

The added import for storetypes is necessary to support the new object store functionality being introduced to the bank module as part of EVM support.

---

47-48: Added HasEndBlocker implementation for AppModule.

This change indicates that the bank module now implements the HasEndBlocker interface, which is important for end-of-block processing related to EVM operations and virtual account transfers.

---

227-227: Added object store key to ModuleInputs.

Adding the ObjStoreKey field to ModuleInputs enables the bank module to work with object storage, which is essential for EVM support to handle more complex data structures.

---

268-268: Updated bankKeeper initialization to include object store key.

The ObjStoreKey is now passed to the NewBaseKeeper function, ensuring the bank keeper can access and manage object storage for EVM-related data.

tests/integration/staking/keeper/determinstic_test.go (3)

71-71: Added object store keys for bank module in test fixture.

Creating object store keys in the test fixture is necessary to match the updated bank module's requirements for object storage support.

---

75-75: Updated CreateMultiStore to include object store keys.

The CreateMultiStore function now accepts the object store keys parameter, ensuring the test environment properly initializes storage for EVM functionality.

---

104-104: Updated bankKeeper initialization with object store key.

The bank keeper initialization now includes the bank module's object store key, which aligns with the changes to the bank module's constructor.

tests/integration/distribution/keeper/msg_server_test.go (3)

64-66: Added object store keys for bank module in test fixture.

Object store keys are properly initialized in the distribution module test fixture to support the bank module's new object storage capabilities needed for EVM functionality.

---

70-70: Updated CreateMultiStore to include object store keys.

Modified the multi-store creation to include object store keys, ensuring tests properly replicate the production environment with EVM storage support.

---

98-98: Updated bankKeeper initialization with object store key.

The bank keeper now receives the object store key parameter, maintaining consistency with the updated module interface needed for EVM support.

tests/integration/staking/keeper/common_test.go (3)

98-100: Added object store keys for bank module in test fixture.

Object store keys are correctly initialized in the common staking test fixture, consistent with the bank module changes for EVM support.

---

104-104: Updated CreateMultiStore to include object store keys.

The multi-store creation now accepts object store keys, ensuring the test environment properly supports the bank module's new object storage requirements.

---

133-133: Updated bankKeeper initialization with object store key.

The bank keeper constructor now receives the object store key parameter, aligning with the module's updated requirements for EVM support.

types/mempool/sender_nonce.go (5)

114-115: Return type changed to use new Tx wrapper

The return type of NextSenderTx has been changed to return Tx.Tx instead of directly returning sdk.Tx. This change is part of the mempool enhancements to track gas information alongside transactions.

---

120-156: Method renamed and enhanced to support gas limit tracking

The original Insert method has been renamed to InsertWithGasWanted and now accepts an additional gasLimit parameter. This change enables explicit tracking of gas limits for transactions in the mempool.

---

130-131: New wrapper type for gas tracking

Using NewMempoolTx to create a wrapper transaction that includes gas information. This approach provides a clean way to store both the transaction and its gas limit together.

---

150-150: Store the wrapper transaction instead of original

Now storing the mempool transaction wrapper (memTx) which includes both the transaction and its gas limit in the sender's transaction list.

---

282-284: Updated return type to match new transaction wrapper

Changed the return type of senderNonceMempoolIterator.Tx() from sdk.Tx to Tx to align with the other changes in this file.

tests/integration/gov/keeper/keeper_test.go (3)

55-57: Added object store keys for bank module

New object store keys are created for the bank module, enabling the integration of the new object store functionality into the tests.

---

61-61: Updated MultiStore creation to include object store keys

The MultiStore creation now includes object store keys, ensuring that the test environment correctly mirrors the production setup with the new object store support.

---

91-91: Added object store key to bank keeper initialization

The bank keeper initialization now includes the object store key, enabling the keeper to interact with the new object store.

x/bank/keeper/collections_test.go (2)

29-30: Added object store key and updated context creation

The test now uses DefaultContextWithObjectStore instead of DefaultContextWithDB and includes the object store key. This change ensures the test environment matches the production environment with the new object store support.

---

46-46: Added object store key to keeper initialization

The bank keeper initialization now includes the object store key parameter, aligning with the updated keeper interface.

x/bank/keeper/keeper.go (5)

11-11: Added direct import for store types

Direct import of the store types package was added, likely to support the new explicit store key parameter.

---

50-52: Added new virtual account transfer methods to interface

New methods for virtual account transfers have been added to the Keeper interface, supporting the new virtual account functionality.

These methods appear to be part of the EVM support implementation, allowing for virtual transfers between accounts and modules.

---

57-57: Added method to credit virtual accounts

New CreditVirtualAccounts method has been added to the Keeper interface, providing a way to credit virtual accounts, likely as part of the EVM integration.

---

95-95: Added object store key parameter to NewBaseKeeper

The NewBaseKeeper function now takes an object store key parameter, enabling integration with the new object store.

---

109-109:

✅ Verification successful

Updated BaseSendKeeper initialization with object store key

The BaseSendKeeper initialization now includes the new object store key, ensuring that all keeper components have access to the object store.

---

🏁 Script executed:

#!/bin/bash
# Verify that all implementations of the virtual account methods exist

echo "Checking for implementation of CreditVirtualAccounts:"
rg -A 2 "func \(.*\) CreditVirtualAccounts\(ctx context.Context\)" --type go

echo -e "\nChecking for implementation of SendCoinsFromAccountToModuleVirtual:"
rg -A 2 "func \(.*\) SendCoinsFromAccountToModuleVirtual\(ctx context.Context" --type go

echo -e "\nChecking for implementation of SendCoinsFromModuleToAccountVirtual:"
rg -A 2 "func \(.*\) SendCoinsFromModuleToAccountVirtual\(ctx context.Context" --type go

Length of output: 1973

---

BaseSendKeeper Object Store Key Initialization Verified

The updated initialization in x/bank/keeper/keeper.go now correctly includes the new objStoreKey, ensuring that the keeper components properly access the object store. The verification shows that:

• The CreditVirtualAccounts method in x/bank/keeper/virtual.go uses the object store key as expected.
• The implementations (or mocks) of SendCoinsFromAccountToModuleVirtual and SendCoinsFromModuleToAccountVirtual are present in x/gov/testutil/expected_keepers_mocks.go.

All virtual account method integrations relying on the object store key appear to be correctly wired.

tests/integration/bank/keeper/deterministic_test.go (3)

68-68: Add object store key initialization for EVM support.

Adding object store keys is necessary for EVM integration, which requires a different storage model than the traditional key-value storage.

---

72-72: Updated CreateMultiStore to include object store keys.

Modified function call to pass the new object store keys to the multi-store, enabling proper storage initialization for EVM support.

---

98-98: Pass object store key to bank keeper initialization.

Properly configuring the bank keeper with object store capabilities is essential for EVM address and account management.

x/bank/keeper/send.go (5)

12-12: Add import for storetypes.

Appropriate addition of the necessary import for object store functionality.

---

64-64: Add object store key field to BaseSendKeeper.

This additional field enables the keeper to utilize object storage, which is essential for EVM-compatible state management.

---

80-81: Update keeper constructor to accept and set object store key.

Properly expands the constructor parameters and initializes the object store key field for EVM storage support.

Also applies to: 95-96

---

240-250: Implement ensureAccountCreated method.

This helper method simplifies account creation logic, properly handling the case when an account doesn't exist. The use of telemetry for tracking new account creation is a good practice.

---

252-270: Implement emitSendCoinEvents for event emission.

Good optimization by storing the fromAddrString once rather than calling the potentially expensive String() method multiple times. The method properly emits both transfer and message events.

CHANGELOG.md (2)

89-95: Document EVM-related feature additions.

The changelog properly documents the critical features added for EVM support, including transaction executor, object store, virtual accounts, Copy-on-Write cache, and block gas tracking. These changes collectively enable the infrastructure needed for EVM compatibility.

---

99-109: Document improvements for EVM performance.

These improvement entries highlight optimizations made to support EVM, including reduced copying operations, mutex scope optimization, and mempool gas handling. These changes will help ensure efficient EVM transaction processing.

types/mempool/priority_nonce.go (5)

192-192: Update NextSenderTx return type.

Changed return type to use Tx.Tx instead of sdk.Tx to provide access to the mempool transaction wrapper.

---

204-214: Add InsertWithGasWanted method for better gas tracking.

This enhancement improves transaction handling with explicit gas wanted information, which is crucial for EVM transactions where gas limits are strictly enforced.

---

279-286: Create backward-compatible Insert method.

Good approach to maintain API compatibility while extracting the gas limit from transactions when available.

---

351-353: Update Tx() return type.

Return the enhanced transaction type which includes gas information.

---

455-457: Optimize locking in Remove method.

Move the lock acquisition to a later point in the code to minimize the lock's scope, potentially improving concurrency when multiple transactions are being processed.

baseapp/baseapp.go (7)

320-322: Validate object store usage.

Introducing case *storetypes.ObjectStoreKey ensures the object store is properly recognized. Confirm that any existing store-mount logic is not inadvertently affected and verify that clients or modules are prepared to handle the new store type.

---

362-372: Expose consistent mounting behavior.

The new MountObjectStores function is consistent with other Mount*Stores patterns. Ensure that:

1. Any calls to MountObjectStores happen before the multistore is loaded.
2. Error handling and logging are consistent with the other mount methods.
3. Callers are aware of any overhead or concurrency limitations with object stores.

---

698-698: Ensure correct usage of WithTxIndex.

ctx = ctx.WithTxIndex(txIndex) stores the transaction’s index in the Context. Check if downstream handlers or event emitters rely on a non-zero-based index or if the index must be strictly sequential.

---

805-805: Check for potential gas mis-accounting.

Calling runTxWithMultiStore(execModeFinalize, ...) can incur substantial gas usage. Verify that the block gas meter gets updated appropriately in each scenario to avoid undercharging or unbounded execution.

---

878-881: Conditionally override the MultiStore.

Overriding the context’s MultiStore when txMultiStore != nil can affect how caching or commits occur. Make sure all references after this override are aware of the change, and the final commit logic remains consistent.

---

1181-1181: Early return on decode failure.

Returning nil, 0, err quickly handles invalid inputs, simplifying error paths. Ensure any potential partial state changes (e.g., counters or logs) are still consistent if an error arises here.

---

1187-1187: Document the final return.

return tx, gInfo.GasWanted, nil clarifies that, upon success, the transaction is valid and includes the gas used in proposal verification. Consider whether the verified transaction struct is reused in subsequent phases, ensuring consistent references.

x/bank/keeper/virtual.go (3)

27-41: Check blocked addresses before subtransactions.

SendCoinsFromModuleToAccountVirtual validates if recipientAddr is blocked, returning an error. Confirm any further checks (e.g., locked or vesting funds) are also performed if needed. The function name is consistent with other *Virtual naming, which is good for clarity.

---

65-87: Handle partial loops on multi-coin transactions.

SendCoinsFromVirtual iterates over each coin and attempts to transfer. If an error arises, prior coins are already transferred to the destination. Check if partial transfers are acceptable or if you need an atomic operation. Document that toAddr might differ from the initial address if the send restriction modifies it.

---

106-138: Defend against uninitialized or stale coin data.

subVirtualCoin fetches previously stored coins but returns an insufficient funds error if no data is found. This logic is sound, but check that the store’s lifetime matches the transaction or block scope. If a coin is subtracted across multiple calls within the same transaction index, ensure that the iteration approach is robust.

x/gov/testutil/expected_keepers_mocks.go (2)

235-242: Maintain consistency with real keeper method.

CreditVirtualAccounts mock method ensures tests can simulate the finalization of transient balances. Verify that the test environment calls this mock under conditions similar to production usage. If partial invocations or repeated calls happen in tests, ensure they reflect real usage accurately.

---

841-854: Align module-to-account logic with real environment.

The SendCoinsFromModuleToAccountVirtual mock emulates transferring from a module to an account using transient state. Verify that test sequences adequately check blocked recipients and correct coin subtraction from the module. Mismatched expectations here can lead to silent test failures or false positives.

x/bank/keeper/keeper_test.go (17)

134-135: Support for ObjectStore in test setup

The test suite now initializes an ObjectStore key and uses DefaultContextWithObjectStore to set up the test context, enabling testing for object store interactions in the bank module.

---

152-152: Object store key passed to BaseKeeper constructor

The BaseKeeper initialization is updated to include the object store key as a parameter, allowing the keeper to interact with the object storage layer.

---

204-207: New mock method for virtual account-to-module transfers

This helper method sets up mock expectations for the new virtual coin transfer functionality, which has simpler authentication requirements than regular transfers since virtual transfers don't immediately change balances.

---

209-212: New mock method for virtual module-to-account transfers

Similar to its counterpart above, this helper method mocks the virtual coin transfer from module to account with appropriate expectations.

---

654-684: New test for virtual coin transfers

This comprehensive test validates the virtual coin transfer functionality, including sending coins to a module account, refunding a portion, and finalizing the transfers with CreditVirtualAccounts. The test confirms both the functioning of the virtual transfer methods and the correct final balances.

---

773-773: API change: Using single coin instead of coins collection

The SendRestrictionFn signature has been updated to accept a single sdk.Coin instead of sdk.Coins, requiring changes to how the restriction functions handle the input.

---

778-778: SDK.Coin wrapped in NewCoins for compatibility

To maintain compatibility with existing code, the single coin parameter is wrapped in sdk.NewCoins() when stored in the restriction args.

---

790-790: Similar API change in restrictionPassthrough

The function signature has been updated to match the new SendRestrictionFn pattern.

---

795-795: Consistent wrapping of single coin in NewCoins

Maintaining consistent pattern of wrapping the single coin in NewCoins when storing in the restriction args.

---

802-802: API update in restrictionNewTo

The same pattern applies to the restrictionNewTo function.

---

807-807: Consistent coin handling in restrictionNewTo

The same pattern of coin wrapping is used throughout the codebase for consistency.

---

1097-1097: API updated in restrictionError

The SendRestrictionFn signature change is consistently applied to the restrictionError function.

---

1102-1102: Coin handling in restrictionError

Same pattern of wrapping single coin in NewCoins is used for consistency.

---

1108-1108: API change in restrictionPassthrough

The SendRestrictionFn signature change is consistently applied to all restriction function variants.

---

1113-1113: Coin handling in restrictionPassthrough

Consistent application of the coin wrapping pattern.

---

1119-1119: API change in restrictionNewTo

The signature change is applied to the last restriction function variant.

---

1124-1124: Coin handling in restrictionNewTo

The same coin wrapping approach is used to maintain consistency across the codebase.

[coderabbitai]

🛠️ Refactor suggestion

Consider the runtime safety of type assertion

The change from GetCommitKVStore(key) to GetCommitStore(key) with an explicit type assertion introduces potential runtime panic risk if the store is not a KVStore. Since this code is part of a test support package, it might be preferable to include a more graceful failure option.

Additionally, note the inconsistency: line 36 still uses the original GetCommitKVStore method while the check at line 29 now uses GetCommitStore. Consider harmonizing these approaches to maintain code consistency.

func (m MockContext) KVStore(key storetypes.StoreKey) storetypes.KVStore {
-	if s := m.store.GetCommitStore(key); s != nil {
-		return s.(storetypes.KVStore)
+	if s := m.store.GetCommitStore(key); s != nil {
+		kvStore, ok := s.(storetypes.KVStore)
+		if !ok {
+			panic(fmt.Sprintf("store with key %v is not a KVStore", key))
+		}
+		return kvStore
	}
	m.store.MountStoreWithDB(key, storetypes.StoreTypeIAVL, m.db)
	if err := m.store.LoadLatestVersion(); err != nil {
		panic(err)
	}
-	return m.store.GetCommitKVStore(key)
+	return m.store.GetCommitStore(key).(storetypes.KVStore)
}

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	func (m MockContext) KVStore(key storetypes.StoreKey) storetypes.KVStore {
	if s := m.store.GetCommitKVStore(key); s != nil {
	return s
	if s := m.store.GetCommitStore(key); s != nil {
	return s.(storetypes.KVStore)
	}
	func (m MockContext) KVStore(key storetypes.StoreKey) storetypes.KVStore {
	if s := m.store.GetCommitStore(key); s != nil {
	kvStore, ok := s.(storetypes.KVStore)
	if !ok {
	panic(fmt.Sprintf("store with key %v is not a KVStore", key))
	}
	return kvStore
	}
	m.store.MountStoreWithDB(key, storetypes.StoreTypeIAVL, m.db)
	if err := m.store.LoadLatestVersion(); err != nil {
	panic(err)
	}
	return m.store.GetCommitStore(key).(storetypes.KVStore)
	}

[coderabbitai]

💡 Verification agent

🧩 Analysis chain

Significant behavior change in CacheWrap method

The method previously panicked with "cannot be cache wrapped", but now returns a cache-wrapped store. This is a major behavioral change that makes the method more useful but could impact existing code that expected or handled the panic.

Please verify that all dependents of this code are updated to handle the new behavior rather than expecting a panic. Run this to find potential callers:

---

🏁 Script executed:

#!/bin/bash
# Search for places that might expect CacheWrap to panic
rg -A 2 -B 2 "CacheWrap.*panic|panic.*CacheWrap" --type go

Length of output: 815

---

Action Required: Update Documentation and Verify Dependent Behavior

The change in store/listenkv/store.go now returns a cache-wrapped store using cachekv.NewStore(s) instead of panicking. While this makes the method more useful, our repository still contains at least one outdated inline comment—in store/listenkv (and similarly in store/tracekv)—which states that the method panics. This discrepancy could mislead developers or cause issues if any dependents were relying on the panic behavior.

Please address the following:

• Audit Dependent Code: Verify that all code relying on the CacheWrap method (or handling its panic) is updated to work with the new behavior.
• Update Inline Comments/Docs: Revise the documentation in store/listenkv/store.go (and any similar references) to accurately describe that the method now returns a cache-wrapped store.
• Adjust Tests if Necessary: Ensure that any tests expecting a panic are modified to reflect the new successful return.

[coderabbitai]

💡 Verification agent

🧩 Analysis chain

Function signature update may need documentation

The LoadStore function now accepts an additional boolean parameter (set to false) in all function calls. This parameter appears to be related to the EVM support being added in this PR.

Please ensure this new parameter is properly documented in the function definition and consider adding comments about its purpose in these test cases. It's also important to verify that all existing calls to this function throughout the codebase have been updated.

Also applies to: 113-113, 118-118

---

🏁 Script executed:

#!/bin/bash
# Find all occurrences of LoadStore function calls to ensure consistency
rg "LoadStore\(" --type go

Length of output: 1376

---

Documentation Update Required for LoadStore's New Parameter

The shell output confirms that all call sites for LoadStore have been updated consistently with the additional boolean parameter (both booleans are passed as false). However, the new parameter—likely tied to the recently added EVM support—is not yet clearly documented. Please update the function definition in store/iavl/store.go to include an explanation of the purpose of the new boolean parameter (in particular, clarify the role of the second boolean, e.g. commitSync). Also, consider adding inline comments in the affected test cases (lines 107, 113, and 118 in store/iavl/store_test.go) to clarify why this parameter is set to false in these contexts.

• Verify and update the documentation on LoadStore (in store/iavl/store.go) to describe the additional boolean parameter.
• Add inline comments in the relevant tests to explain its usage.
• Confirm that no remaining call sites require further changes.

[coderabbitai]

💡 Verification agent

🧩 Analysis chain

Verify impact of changing signer field type from bytes to int32

Changing the field type from bytes to int32 in the BadSigner message may affect test cases that rely on this message structure. Please confirm this change is intentional and that all test cases using this message have been updated accordingly.

---

🏁 Script executed:

#!/bin/bash
# Search for tests that use the BadSigner message
rg -A 5 "BadSigner" --type go

Length of output: 19536

---

Action Required: Address Inconsistent Field Type for BadSigner.signer

The change declared in the proto file (declaring signer as an int32) has not been reflected in the generated code. Our grep results indicate that in the generated file (x/tx/internal/testpb/signers.pulsar.go) the field is still defined as a []byte with protobuf tag "bytes,1,opt,name=signer,proto3". In addition, test cases (e.g., in x/tx/signing/context_test.go) create instances of BadSigner assuming that the signer field is a byte slice.

Next Steps:

• Confirm whether the switch from bytes to int32 is intentional.
• If the intent is to change the type, update the proto file accordingly, regenerate the Go code, and modify all affected tests and usage (particularly in x/tx/internal/testpb/signers.pulsar.go and related test files).
• If the change was not intended, revert the proto change to maintain type consistency with the generated code and tests.

[coderabbitai]

⚠️ Potential issue

Fix undefined type issue with ObjectStoreKey

The static analysis shows that storetypes.ObjectStoreKey is undefined. This type needs to be properly defined in the storetypes package.

Make sure that the ObjectStoreKey type is defined in the storetypes package and the appropriate imports are in place. This is likely part of the broader changes for EVM support.

#!/bin/bash
# Check if the ObjectStoreKey type is defined in the storetypes package
rg -A 3 "type ObjectStoreKey" --type go

🧰 Tools

🪛 GitHub Check: golangci-lint

[failure] 229-229:
undefined: storetypes.ObjectStoreKey (typecheck)

🪛 golangci-lint (1.62.2)

229-229: undefined: storetypes.ObjectStoreKey

(typecheck)

[coderabbitai]

🛠️ Refactor suggestion

swapCache usage.
Swapping out the internal writeSet can be risky if references leak. Ensure that all references are properly managed within EVM integration.

[coderabbitai]

💡 Verification agent

🧩 Analysis chain

Validate large object values more thoroughly.

While AssertValidValueGeneric prevents nil or zero objects, consider additional validations if the expected value can be extremely large (e.g., bounding lengths). This can help avoid denial-of-service scenarios where huge values consume excessive memory.

---

🏁 Script executed:

#!/bin/bash
# Searching for documentation or usage of AssertValidValueGeneric to see if it checks extremely large values:
rg -A 3 'func AssertValidValueGeneric'

Length of output: 273

---

Add explicit validation for large object sizes in GStore.Set

While AssertValidValueGeneric ensures that the value isn’t nil (by checking if it’s zero via gs.isZero), it does not validate the actual size of the object. Since extremely large values can lead to excessive memory usage and potential denial-of-service issues, please add an explicit length check—using gs.valueLen(value)—to verify that the value doesn’t exceed a safe maximum.

• Consider adding a check similar to:

  if gs.valueLen(value) > MAX_ALLOWED_SIZE {
      panic("value size exceeds allowed limit")
  }

• Apply this check in the Set function after the existing assertions in store/gaskv/store.go (around lines 75–77).

[coderabbitai]

🛠️ Refactor suggestion

Track concurrency or reentrancy safety.

deliverTxWithMultiStore introduces a mechanism to deliver a transaction with an optional custom multistore. If invoked concurrently (e.g., in parallel execution flows), confirm that referencing external stores is safe and will not inadvertently cause data races or partial writes.

[coderabbitai]

🛠️ Refactor suggestion

Validate object store iteration key uniqueness.

addVirtualCoin constructs a key containing addr + encoded txIndex. This approach works for per-transaction segregation but might risk collisions if txIndex is assigned in unexpected ways. Confirm the combination is guaranteed unique across multiple contexts, especially if txIndex can reset or overlap in parallel transactions.

[coderabbitai]

🛠️ Refactor suggestion

Ensure atomic finalization of transient balances.

CreditVirtualAccounts merges all transient coin records into the actual accounts. If an error occurs midway (e.g., addCoins fails for one address), consider whether partial updates can be reverted. Iterating keys by partial addresses is correct, but confirm that underlying data is fully cleared once credited, to avoid double-crediting.