Skip to content

Conversation

@aaronc
Copy link
Member

@aaronc aaronc commented Oct 20, 2025

Description

Closes: #XXXX

io "io"
"os"
"path/filepath"
"runtime"

Check notice

Code scanning / CodeQL

Sensitive package import Note

Certain system packages contain functions which may be a possible source of non-determinism

root, err := c.root.Resolve()
if err != nil {
panic(err)

Check warning

Code scanning / CodeQL

Panic in BeginBock or EndBlock consensus methods Warning

Possible panics in BeginBock- or EndBlock-related consensus methods could cause a chain halt

value, _, err := root.Get(key)
if err != nil {
panic(err)

Check warning

Code scanning / CodeQL

Panic in BeginBock or EndBlock consensus methods Warning

Possible panics in BeginBock- or EndBlock-related consensus methods could cause a chain halt
// compute hash and assign node IDs
var err error
hash, err = commitTraverse(commitCtx, c.root, 0)
rootPtr, err = c.store.ResolveRoot(uint32(version))

Check failure

Code scanning / CodeQL

Incorrect conversion between integer types High

Incorrect conversion of a signed 64-bit integer from
strconv.ParseInt
to a lower bit size type uint32 without an upper bound check.

Copilot Autofix

AI 14 days ago

To solve this issue, we must ensure that when casting from an int64 to a smaller (unsigned) integer type (uint32), we properly verify that the value fits within the representable domain of uint32. That means the value must be in the range [0, math.MaxUint32]. If the value is outside this range, it should be rejected or a safe default should be returned (or an error raised).

The best way to implement this is to add an explicit check on the range of version in CommitTree.GetImmutable. If version < 0 or version > math.MaxUint32, return an error. This should be done just before the call to ResolveRoot(uint32(version)).

Because this is Go, we need to import the standard math package to obtain math.MaxUint32. The changes required are:

  • In iavl/commit_tree.go, in the GetImmutable method, add a range check for version.
  • If the bounds check fails, return an error indicating the version is invalid or out of range.
  • Import "math" at the top of the file if not already present.

No other files need to be changed, since the conversion only happens in iavl/commit_tree.go.


Suggested changeset 1
iavl/commit_tree.go

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/iavl/commit_tree.go b/iavl/commit_tree.go
--- a/iavl/commit_tree.go
+++ b/iavl/commit_tree.go
@@ -3,6 +3,7 @@
 import (
 	"fmt"
 	"io"
+	"math"
 	"sync"
 	"sync/atomic"
 
@@ -302,8 +303,12 @@
 }
 
 func (c *CommitTree) GetImmutable(version int64) (storetypes.CacheKVStore, error) {
+	// Check for valid range before converting from int64 to uint32.
+	if version < 0 || version > int64(math.MaxUint32) {
+		return nil, fmt.Errorf("invalid version (out of uint32 bounds): %d", version)
+	}
 	var rootPtr *NodePointer
-	if version == c.lastCommitId.Version {
+	if version == int64(c.lastCommitId.Version) {
 		rootPtr = c.root
 	} else {
 		var err error
EOF
@@ -3,6 +3,7 @@
import (
"fmt"
"io"
"math"
"sync"
"sync/atomic"

@@ -302,8 +303,12 @@
}

func (c *CommitTree) GetImmutable(version int64) (storetypes.CacheKVStore, error) {
// Check for valid range before converting from int64 to uint32.
if version < 0 || version > int64(math.MaxUint32) {
return nil, fmt.Errorf("invalid version (out of uint32 bounds): %d", version)
}
var rootPtr *NodePointer
if version == c.lastCommitId.Version {
if version == int64(c.lastCommitId.Version) {
rootPtr = c.root
} else {
var err error
Copilot is powered by AI and may make mistakes. Always verify output.
root, err := tree.root.Resolve()
if err != nil {
return nil, err
panic(err)

Check warning

Code scanning / CodeQL

Panic in BeginBock or EndBlock consensus methods Warning

Possible panics in BeginBock- or EndBlock-related consensus methods could cause a chain halt
value, _, err := root.Get(key)
if err != nil {
return nil, err
panic(err)

Check warning

Code scanning / CodeQL

Panic in BeginBock or EndBlock consensus methods Warning

Possible panics in BeginBock- or EndBlock-related consensus methods could cause a chain halt
newRoot, _, err := setRecursive(tree.root, leafNode, ctx)
if err != nil {
return err
panic(err)

Check warning

Code scanning / CodeQL

Panic in BeginBock or EndBlock consensus methods Warning

Possible panics in BeginBock- or EndBlock-related consensus methods could cause a chain halt
_, newRoot, _, err := removeRecursive(tree.root, key, ctx)
if err != nil {
return err
panic(err)

Check warning

Code scanning / CodeQL

Panic in BeginBock or EndBlock consensus methods Warning

Possible panics in BeginBock- or EndBlock-related consensus methods could cause a chain halt
func (db *CommitMultiTree) GetKVStore(key storetypes.StoreKey) storetypes.KVStore {
index, ok := db.treesByKey[key]
if !ok {
panic(fmt.Sprintf("store not found for key: %s (key type: %T)", key.Name(), key))

Check warning

Code scanning / CodeQL

Panic in BeginBock or EndBlock consensus methods Warning

Possible panics in BeginBock- or EndBlock-related consensus methods could cause a chain halt
if err != nil {
return nil, fmt.Errorf("failed to commit trees: %w", err)
if index >= len(db.trees) {
panic(fmt.Sprintf("store index %d out of bounds for key %s (trees length: %d)", index, key.Name(), len(db.trees)))

Check warning

Code scanning / CodeQL

Panic in BeginBock or EndBlock consensus methods Warning

Possible panics in BeginBock- or EndBlock-related consensus methods could cause a chain halt
index, ok := t.treesByKey[key]
if !ok {
return nil
panic(fmt.Sprintf("store not found for key: %s (key type: %T)", key.Name(), key))

Check warning

Code scanning / CodeQL

Panic in BeginBock or EndBlock consensus methods Warning

Possible panics in BeginBock- or EndBlock-related consensus methods could cause a chain halt
}
return t.trees[idx]
if index >= len(t.trees) {
panic(fmt.Sprintf("store index %d out of bounds for key %s (trees length: %d)", index, key.Name(), len(t.trees)))

Check warning

Code scanning / CodeQL

Panic in BeginBock or EndBlock consensus methods Warning

Possible panics in BeginBock- or EndBlock-related consensus methods could cause a chain halt

func (tree *Tree) applyChangesToParent(origRoot, newRoot *NodePointer, updateBatch KVUpdateBatch) error {
if tree.root != origRoot {
panic("cannot apply changes: root has changed")

Check warning

Code scanning / CodeQL

Panic in BeginBock or EndBlock consensus methods Warning

Possible panics in BeginBock- or EndBlock-related consensus methods could cause a chain halt

func (tree *Tree) Write() {
if tree.parent == nil {
panic("cannot write: tree is immutable")

Check warning

Code scanning / CodeQL

Panic in BeginBock or EndBlock consensus methods Warning

Possible panics in BeginBock- or EndBlock-related consensus methods could cause a chain halt
}
err := tree.parent.applyChangesToParent(tree.origRoot, tree.root, tree.updateBatch)
if err != nil {
panic(err)

Check warning

Code scanning / CodeQL

Panic in BeginBock or EndBlock consensus methods Warning

Possible panics in BeginBock- or EndBlock-related consensus methods could cause a chain halt
@aaronc aaronc marked this pull request as ready for review October 21, 2025 22:37
@github-actions
Copy link
Contributor

🔒 PR closed: unsigned commits detected

This pull request contains 13 commit(s) without a verified signature.

How to fix:

  1. Set up commit signing (GPG or SSH).
  2. Amend/rebase so every commit in this PR is verified-signed.
  3. Push the updated branch and open a new PR, or ask a maintainer to reopen once fixed.

Docs: https://docs.github.com/authentication/managing-commit-signature-verification

Unsigned commits:

  • a3f1cf2 — feat!: adapt iavlx to store
  • 1041204 — fixes
  • 2cbdf7b — build fixes
  • dc876a4 — WIP on system tests
  • c3d9665 — WIP on system tests
  • 7f154ba — Merge branch 'main' of github.com:cosmos/cosmos-sdk into aaronc/iavlx-store
  • 26a09d8 — Merge branch 'aaronc/iavlx' into aaronc/iavlx-store
  • 29edd54 — WIP on system tests
  • 80b98c6 — fix bug with multi-store isolation
  • fbf7209 — attempting to fix cachewrap write behavior
  • 13aa9fa — fixing bugs and fixing working hash implementation
  • 8b61055 — revert unneeded makefile changes
  • 245982d — fix bug

@github-actions github-actions bot closed this Oct 21, 2025
@github-actions
Copy link
Contributor

@aaronc your pull request is missing a changelog!

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants