From 271f82736985274d35019f52a40100e8e2680cd3 Mon Sep 17 00:00:00 2001 From: Jonathan Oppenheimer Date: Tue, 23 Sep 2025 15:28:08 -0400 Subject: [PATCH 01/11] Implement correct balance updating logic --- stateupgrade/interfaces.go | 3 + stateupgrade/state_upgrade.go | 22 ++- stateupgrade/state_upgrade_test.go | 225 +++++++++++++++++++++++++++++ 3 files changed, 248 insertions(+), 2 deletions(-) create mode 100644 stateupgrade/state_upgrade_test.go diff --git a/stateupgrade/interfaces.go b/stateupgrade/interfaces.go index e77a7ef408..0f90165870 100644 --- a/stateupgrade/interfaces.go +++ b/stateupgrade/interfaces.go @@ -15,7 +15,10 @@ import ( type StateDB interface { SetState(common.Address, common.Hash, common.Hash, ...stateconf.StateDBStateOption) SetCode(common.Address, []byte) + + GetBalance(common.Address) *uint256.Int AddBalance(common.Address, *uint256.Int) + SubBalance(common.Address, *uint256.Int) GetNonce(common.Address) uint64 SetNonce(common.Address, uint64) diff --git a/stateupgrade/state_upgrade.go b/stateupgrade/state_upgrade.go index 86c43e3d06..8385f9a86a 100644 --- a/stateupgrade/state_upgrade.go +++ b/stateupgrade/state_upgrade.go @@ -4,6 +4,7 @@ package stateupgrade import ( + "fmt" "math/big" "github.com/ava-labs/libevm/common" @@ -30,9 +31,26 @@ func upgradeAccount(account common.Address, upgrade extras.StateUpgradeAccount, state.CreateAccount(account) } + // Balance change detected - update the balance of the account if upgrade.BalanceChange != nil { - balanceChange, _ := uint256.FromBig((*big.Int)(upgrade.BalanceChange)) - state.AddBalance(account, balanceChange) + // BalanceChange is a HexOrDecimal256, which is just a typed big.Int + bigChange := (*big.Int)(upgrade.BalanceChange) + switch bigChange.Sign() { + case 1: // Positive + // ParseBig256 already enforced 256-bit limit during JSON parsing, so no overflow check needed + balanceChange, _ := uint256.FromBig(bigChange) + state.AddBalance(account, balanceChange) + case -1: // Negative + absChange := new(big.Int).Abs(bigChange) + balanceChange, _ := uint256.FromBig(absChange) + currentBalance := state.GetBalance(account) + if currentBalance.Cmp(balanceChange) < 0 { + return fmt.Errorf("insufficient balance for subtraction: account %s has %s but trying to subtract %s", + account.Hex(), currentBalance.ToBig().String(), balanceChange.ToBig().String()) + } + state.SubBalance(account, balanceChange) + } + // If zero (Sign() == 0), do nothing } if len(upgrade.Code) != 0 { // if the nonce is 0, set the nonce to 1 as we would when deploying a contract at diff --git a/stateupgrade/state_upgrade_test.go b/stateupgrade/state_upgrade_test.go new file mode 100644 index 0000000000..4de7963a63 --- /dev/null +++ b/stateupgrade/state_upgrade_test.go @@ -0,0 +1,225 @@ +// Copyright (C) 2019-2025, Ava Labs, Inc. All rights reserved. +// See the file LICENSE for licensing terms. + +package stateupgrade + +import ( + "math/big" + "testing" + + "github.com/ava-labs/libevm/common" + "github.com/ava-labs/libevm/common/math" + "github.com/ava-labs/libevm/core/rawdb" + "github.com/ava-labs/libevm/core/state" + "github.com/ava-labs/libevm/core/types" + "github.com/holiman/uint256" + "github.com/stretchr/testify/require" + + "github.com/ava-labs/subnet-evm/core/extstate" + "github.com/ava-labs/subnet-evm/params/extras" +) + +func TestUpgradeAccount_BalanceChanges(t *testing.T) { + testAddr := common.HexToAddress("0x1234567890123456789012345678901234567890") + + tests := []struct { + name string + initialBalance *uint256.Int + balanceChange *math.HexOrDecimal256 + accountExists bool + wantBalance *uint256.Int + wantError string + }{ + { + name: "positive balance change on existing account", + initialBalance: uint256.NewInt(100), + balanceChange: hexOrDecimal256FromInt64(50), + accountExists: true, + wantBalance: uint256.NewInt(150), + }, + { + name: "negative balance change with sufficient funds", + initialBalance: uint256.NewInt(100), + balanceChange: hexOrDecimal256FromInt64(-30), + accountExists: true, + wantBalance: uint256.NewInt(70), + }, + { + name: "negative balance change with insufficient funds", + initialBalance: uint256.NewInt(50), + balanceChange: hexOrDecimal256FromInt64(-100), + accountExists: true, + wantBalance: uint256.NewInt(50), // unchanged + wantError: "insufficient balance for subtraction", + }, + { + name: "zero balance change", + initialBalance: uint256.NewInt(100), + balanceChange: hexOrDecimal256FromInt64(0), + accountExists: true, + wantBalance: uint256.NewInt(100), + }, + { + name: "nil balance change", + initialBalance: uint256.NewInt(100), + balanceChange: nil, + accountExists: true, + wantBalance: uint256.NewInt(100), + }, + { + name: "new account with positive balance", + initialBalance: uint256.NewInt(0), + balanceChange: hexOrDecimal256FromInt64(1000), + accountExists: false, + wantBalance: uint256.NewInt(1000), + }, + { + name: "new account with negative balance", + initialBalance: uint256.NewInt(0), + balanceChange: hexOrDecimal256FromInt64(-100), + accountExists: false, + wantBalance: uint256.NewInt(0), // unchanged + wantError: "insufficient balance for subtraction", + }, + { + name: "exact balance subtraction", + initialBalance: uint256.NewInt(100), + balanceChange: hexOrDecimal256FromInt64(-100), + accountExists: true, + wantBalance: uint256.NewInt(0), + }, + { + name: "off-by-one underflow", + initialBalance: uint256.NewInt(100), + balanceChange: hexOrDecimal256FromInt64(-101), + accountExists: true, + wantBalance: uint256.NewInt(100), // unchanged + wantError: "insufficient balance for subtraction", + }, + { + name: "large positive balance change", + initialBalance: uint256.MustFromBig(new(big.Int).Sub(new(big.Int).Lsh(big.NewInt(1), 255), big.NewInt(1000))), + balanceChange: hexOrDecimal256FromInt64(500), + accountExists: true, + wantBalance: uint256.MustFromBig(new(big.Int).Sub(new(big.Int).Lsh(big.NewInt(1), 255), big.NewInt(500))), + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Create a fresh state database for each test + statedb := createTestStateDB(t) + + // Set up initial account state + if tt.accountExists { + setAccountBalance(t, statedb, testAddr, tt.initialBalance) + } + + // Create the upgrade configuration + upgrade := extras.StateUpgradeAccount{ + BalanceChange: tt.balanceChange, + } + + // Execute the upgrade + err := upgradeAccount(testAddr, upgrade, statedb, false) + + // Check error expectations + if tt.wantError != "" { + require.Error(t, err) + require.Contains(t, err.Error(), tt.wantError) + require.Contains(t, err.Error(), testAddr.Hex()) + } else { + require.NoError(t, err) + } + + // Check final balance + actualBalance := statedb.GetBalance(testAddr) + require.Equal(t, tt.wantBalance, actualBalance, "final balance mismatch") + + // Verify account was created if it didn't exist + require.True(t, statedb.Exist(testAddr), "account should exist after upgrade") + }) + } +} + +func TestUpgradeAccount_ErrorMessageFormat(t *testing.T) { + statedb := createTestStateDB(t) + addr := common.HexToAddress("0xabcdef1234567890abcdef1234567890abcdef12") + + // Set initial balance to 50 + setAccountBalance(t, statedb, addr, uint256.NewInt(50)) + + upgrade := extras.StateUpgradeAccount{ + BalanceChange: hexOrDecimal256FromInt64(-75), + } + + err := upgradeAccount(addr, upgrade, statedb, false) + require.Error(t, err) + + // Check specific error message components + errorMsg := err.Error() + require.Contains(t, errorMsg, "insufficient balance for subtraction") + require.Contains(t, errorMsg, addr.Hex()) // EIP-55 checksum format + require.Contains(t, errorMsg, "has 50") + require.Contains(t, errorMsg, "trying to subtract 75") +} + +func TestUpgradeAccount_CompleteUpgrade(t *testing.T) { + statedb := createTestStateDB(t) + addr := common.HexToAddress("0x1234567890123456789012345678901234567890") + + // Create a complete state upgrade with balance change, code, and storage + code := []byte{0xde, 0xad, 0xbe, 0xef} + storageKey := common.HexToHash("0x1234") + storageValue := common.HexToHash("0x5678") + + upgrade := extras.StateUpgradeAccount{ + BalanceChange: hexOrDecimal256FromInt64(1000), + Code: code, + Storage: map[common.Hash]common.Hash{storageKey: storageValue}, + } + + // Execute the upgrade + err := upgradeAccount(addr, upgrade, statedb, true) // Test with EIP158 = true + require.NoError(t, err) + + // Verify all changes were applied + require.Equal(t, uint256.NewInt(1000), statedb.GetBalance(addr)) + require.True(t, statedb.Exist(addr)) + require.Equal(t, uint64(1), statedb.GetNonce(addr)) // Should be set to 1 due to EIP158 and code deployment + + // Cast to access code and storage (these aren't in our StateDB interface) + extStateDB := statedb.(*extstate.StateDB) + require.Equal(t, code, extStateDB.GetCode(addr)) + require.Equal(t, storageValue, extStateDB.GetState(addr, storageKey)) +} + +// createTestStateDB creates a real StateDB instance for testing +func createTestStateDB(t *testing.T) StateDB { + t.Helper() + + // Create an in-memory database + db := rawdb.NewMemoryDatabase() + + // Create a new state database + statedb, err := state.New(types.EmptyRootHash, state.NewDatabase(db), nil) + require.NoError(t, err) + + // Wrap it with extstate for predicate support (matching the interface) + return extstate.New(statedb) +} + +// setAccountBalance is a helper to set an account's initial balance for testing +func setAccountBalance(t *testing.T, statedb StateDB, addr common.Address, balance *uint256.Int) { + t.Helper() + + // Cast to extstate.StateDB to access the underlying state + extStateDB := statedb.(*extstate.StateDB) + extStateDB.CreateAccount(addr) + extStateDB.SetBalance(addr, balance) +} + +// Helper function to create HexOrDecimal256 from int64 +func hexOrDecimal256FromInt64(i int64) *math.HexOrDecimal256 { + return (*math.HexOrDecimal256)(big.NewInt(i)) +} From 88fce359f23729b05cba955770b91e822a4a19b1 Mon Sep 17 00:00:00 2001 From: Jonathan Oppenheimer Date: Tue, 23 Sep 2025 15:35:22 -0400 Subject: [PATCH 02/11] Trim non-useful comments --- stateupgrade/state_upgrade_test.go | 8 -------- 1 file changed, 8 deletions(-) diff --git a/stateupgrade/state_upgrade_test.go b/stateupgrade/state_upgrade_test.go index 4de7963a63..4d81481bc4 100644 --- a/stateupgrade/state_upgrade_test.go +++ b/stateupgrade/state_upgrade_test.go @@ -115,12 +115,9 @@ func TestUpgradeAccount_BalanceChanges(t *testing.T) { setAccountBalance(t, statedb, testAddr, tt.initialBalance) } - // Create the upgrade configuration upgrade := extras.StateUpgradeAccount{ BalanceChange: tt.balanceChange, } - - // Execute the upgrade err := upgradeAccount(testAddr, upgrade, statedb, false) // Check error expectations @@ -178,8 +175,6 @@ func TestUpgradeAccount_CompleteUpgrade(t *testing.T) { Code: code, Storage: map[common.Hash]common.Hash{storageKey: storageValue}, } - - // Execute the upgrade err := upgradeAccount(addr, upgrade, statedb, true) // Test with EIP158 = true require.NoError(t, err) @@ -198,10 +193,7 @@ func TestUpgradeAccount_CompleteUpgrade(t *testing.T) { func createTestStateDB(t *testing.T) StateDB { t.Helper() - // Create an in-memory database db := rawdb.NewMemoryDatabase() - - // Create a new state database statedb, err := state.New(types.EmptyRootHash, state.NewDatabase(db), nil) require.NoError(t, err) From 074ccbb9b6e1b50f140eacee92882ce7b95a8d1c Mon Sep 17 00:00:00 2001 From: Jonathan Oppenheimer Date: Tue, 23 Sep 2025 15:44:32 -0400 Subject: [PATCH 03/11] lint --- stateupgrade/state_upgrade_test.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/stateupgrade/state_upgrade_test.go b/stateupgrade/state_upgrade_test.go index 4d81481bc4..c4472c7e10 100644 --- a/stateupgrade/state_upgrade_test.go +++ b/stateupgrade/state_upgrade_test.go @@ -175,8 +175,7 @@ func TestUpgradeAccount_CompleteUpgrade(t *testing.T) { Code: code, Storage: map[common.Hash]common.Hash{storageKey: storageValue}, } - err := upgradeAccount(addr, upgrade, statedb, true) // Test with EIP158 = true - require.NoError(t, err) + require.NoError(t, upgradeAccount(addr, upgrade, statedb, true)) // Test with EIP158 = true // Verify all changes were applied require.Equal(t, uint256.NewInt(1000), statedb.GetBalance(addr)) From 4b321966c841461058191f1e0301d510af8e75c3 Mon Sep 17 00:00:00 2001 From: Jonathan Oppenheimer Date: Tue, 23 Sep 2025 15:57:17 -0400 Subject: [PATCH 04/11] Update stateupgrade/state_upgrade_test.go Co-authored-by: Austin Larson <78000745+alarso16@users.noreply.github.com> Signed-off-by: Jonathan Oppenheimer <147infiniti@gmail.com> --- stateupgrade/state_upgrade_test.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/stateupgrade/state_upgrade_test.go b/stateupgrade/state_upgrade_test.go index c4472c7e10..6eda990cfa 100644 --- a/stateupgrade/state_upgrade_test.go +++ b/stateupgrade/state_upgrade_test.go @@ -121,7 +121,10 @@ func TestUpgradeAccount_BalanceChanges(t *testing.T) { err := upgradeAccount(testAddr, upgrade, statedb, false) // Check error expectations - if tt.wantError != "" { + require.ErrorIs(t, err, tt.wantError) + if tt.wantError != nil { + require.ErrorContains(t, err, testAddr.Hex()) // I'm not even sure this is necessary + } require.Error(t, err) require.Contains(t, err.Error(), tt.wantError) require.Contains(t, err.Error(), testAddr.Hex()) From ce9c10897a3b1d28d79c55a40e319d96f01de20d Mon Sep 17 00:00:00 2001 From: Jonathan Oppenheimer Date: Tue, 23 Sep 2025 16:12:17 -0400 Subject: [PATCH 05/11] Austin suggestions --- stateupgrade/state_upgrade.go | 16 +++++++-- stateupgrade/state_upgrade_test.go | 52 +++++++++++------------------- 2 files changed, 33 insertions(+), 35 deletions(-) diff --git a/stateupgrade/state_upgrade.go b/stateupgrade/state_upgrade.go index 8385f9a86a..b51ec81abb 100644 --- a/stateupgrade/state_upgrade.go +++ b/stateupgrade/state_upgrade.go @@ -4,15 +4,22 @@ package stateupgrade import ( + "errors" "fmt" "math/big" + "github.com/ava-labs/libevm/accounts/abi" "github.com/ava-labs/libevm/common" "github.com/holiman/uint256" "github.com/ava-labs/subnet-evm/params/extras" ) +var ( + ErrInsufficientBalanceForSubtraction = errors.New("insufficient balance for subtraction") + ErrBalanceOverflow = errors.New("balance overflow") +) + // Configure applies the state upgrade to the state. func Configure(stateUpgrade *extras.StateUpgrade, chainConfig ChainContext, state StateDB, blockContext BlockContext) error { isEIP158 := chainConfig.IsEIP158(blockContext.Number()) @@ -37,15 +44,20 @@ func upgradeAccount(account common.Address, upgrade extras.StateUpgradeAccount, bigChange := (*big.Int)(upgrade.BalanceChange) switch bigChange.Sign() { case 1: // Positive - // ParseBig256 already enforced 256-bit limit during JSON parsing, so no overflow check needed balanceChange, _ := uint256.FromBig(bigChange) + currentBalance := state.GetBalance(account) + if new(big.Int).Add(currentBalance.ToBig(), balanceChange.ToBig()).Cmp(abi.MaxUint256) > 0 { + return fmt.Errorf("%w: account %s current balance %s + change %s would exceed maximum uint256", + ErrBalanceOverflow, account.Hex(), currentBalance.ToBig().String(), balanceChange.ToBig().String()) + } state.AddBalance(account, balanceChange) case -1: // Negative absChange := new(big.Int).Abs(bigChange) balanceChange, _ := uint256.FromBig(absChange) currentBalance := state.GetBalance(account) if currentBalance.Cmp(balanceChange) < 0 { - return fmt.Errorf("insufficient balance for subtraction: account %s has %s but trying to subtract %s", + return fmt.Errorf("%w: account %s has %s but trying to subtract %s", + ErrInsufficientBalanceForSubtraction, account.Hex(), currentBalance.ToBig().String(), balanceChange.ToBig().String()) } state.SubBalance(account, balanceChange) diff --git a/stateupgrade/state_upgrade_test.go b/stateupgrade/state_upgrade_test.go index 6eda990cfa..0b875864b5 100644 --- a/stateupgrade/state_upgrade_test.go +++ b/stateupgrade/state_upgrade_test.go @@ -7,6 +7,7 @@ import ( "math/big" "testing" + "github.com/ava-labs/libevm/accounts/abi" "github.com/ava-labs/libevm/common" "github.com/ava-labs/libevm/common/math" "github.com/ava-labs/libevm/core/rawdb" @@ -28,7 +29,7 @@ func TestUpgradeAccount_BalanceChanges(t *testing.T) { balanceChange *math.HexOrDecimal256 accountExists bool wantBalance *uint256.Int - wantError string + wantError error }{ { name: "positive balance change on existing account", @@ -50,7 +51,7 @@ func TestUpgradeAccount_BalanceChanges(t *testing.T) { balanceChange: hexOrDecimal256FromInt64(-100), accountExists: true, wantBalance: uint256.NewInt(50), // unchanged - wantError: "insufficient balance for subtraction", + wantError: ErrInsufficientBalanceForSubtraction, }, { name: "zero balance change", @@ -79,7 +80,7 @@ func TestUpgradeAccount_BalanceChanges(t *testing.T) { balanceChange: hexOrDecimal256FromInt64(-100), accountExists: false, wantBalance: uint256.NewInt(0), // unchanged - wantError: "insufficient balance for subtraction", + wantError: ErrInsufficientBalanceForSubtraction, }, { name: "exact balance subtraction", @@ -94,14 +95,24 @@ func TestUpgradeAccount_BalanceChanges(t *testing.T) { balanceChange: hexOrDecimal256FromInt64(-101), accountExists: true, wantBalance: uint256.NewInt(100), // unchanged - wantError: "insufficient balance for subtraction", + wantError: ErrInsufficientBalanceForSubtraction, }, { - name: "large positive balance change", + name: "large positive balance change", + // Initial balance: 2^255 - 1000 (a very large number near half of max uint256) initialBalance: uint256.MustFromBig(new(big.Int).Sub(new(big.Int).Lsh(big.NewInt(1), 255), big.NewInt(1000))), balanceChange: hexOrDecimal256FromInt64(500), accountExists: true, - wantBalance: uint256.MustFromBig(new(big.Int).Sub(new(big.Int).Lsh(big.NewInt(1), 255), big.NewInt(500))), + // Expected balance: 2^255 - 500 (initial + 500) + wantBalance: uint256.MustFromBig(new(big.Int).Sub(new(big.Int).Lsh(big.NewInt(1), 255), big.NewInt(500))), + }, + { + name: "balance overflow protection", + initialBalance: uint256.MustFromBig(abi.MaxUint256), // Max uint256 + balanceChange: hexOrDecimal256FromInt64(1), + accountExists: true, + wantBalance: uint256.MustFromBig(abi.MaxUint256), // unchanged + wantError: ErrBalanceOverflow, }, } @@ -121,13 +132,10 @@ func TestUpgradeAccount_BalanceChanges(t *testing.T) { err := upgradeAccount(testAddr, upgrade, statedb, false) // Check error expectations - require.ErrorIs(t, err, tt.wantError) if tt.wantError != nil { - require.ErrorContains(t, err, testAddr.Hex()) // I'm not even sure this is necessary - } require.Error(t, err) - require.Contains(t, err.Error(), tt.wantError) - require.Contains(t, err.Error(), testAddr.Hex()) + require.ErrorIs(t, err, tt.wantError) + require.ErrorContains(t, err, testAddr.Hex()) } else { require.NoError(t, err) } @@ -142,28 +150,6 @@ func TestUpgradeAccount_BalanceChanges(t *testing.T) { } } -func TestUpgradeAccount_ErrorMessageFormat(t *testing.T) { - statedb := createTestStateDB(t) - addr := common.HexToAddress("0xabcdef1234567890abcdef1234567890abcdef12") - - // Set initial balance to 50 - setAccountBalance(t, statedb, addr, uint256.NewInt(50)) - - upgrade := extras.StateUpgradeAccount{ - BalanceChange: hexOrDecimal256FromInt64(-75), - } - - err := upgradeAccount(addr, upgrade, statedb, false) - require.Error(t, err) - - // Check specific error message components - errorMsg := err.Error() - require.Contains(t, errorMsg, "insufficient balance for subtraction") - require.Contains(t, errorMsg, addr.Hex()) // EIP-55 checksum format - require.Contains(t, errorMsg, "has 50") - require.Contains(t, errorMsg, "trying to subtract 75") -} - func TestUpgradeAccount_CompleteUpgrade(t *testing.T) { statedb := createTestStateDB(t) addr := common.HexToAddress("0x1234567890123456789012345678901234567890") From 30620887ef6254e29a508904e2980ad5ff433cc1 Mon Sep 17 00:00:00 2001 From: Jonathan Oppenheimer Date: Tue, 23 Sep 2025 16:37:59 -0400 Subject: [PATCH 06/11] Austin suggestions --- stateupgrade/state_upgrade_test.go | 17 ++++------------- 1 file changed, 4 insertions(+), 13 deletions(-) diff --git a/stateupgrade/state_upgrade_test.go b/stateupgrade/state_upgrade_test.go index 0b875864b5..0a010dae5d 100644 --- a/stateupgrade/state_upgrade_test.go +++ b/stateupgrade/state_upgrade_test.go @@ -63,7 +63,6 @@ func TestUpgradeAccount_BalanceChanges(t *testing.T) { { name: "nil balance change", initialBalance: uint256.NewInt(100), - balanceChange: nil, accountExists: true, wantBalance: uint256.NewInt(100), }, @@ -71,14 +70,12 @@ func TestUpgradeAccount_BalanceChanges(t *testing.T) { name: "new account with positive balance", initialBalance: uint256.NewInt(0), balanceChange: hexOrDecimal256FromInt64(1000), - accountExists: false, wantBalance: uint256.NewInt(1000), }, { name: "new account with negative balance", initialBalance: uint256.NewInt(0), balanceChange: hexOrDecimal256FromInt64(-100), - accountExists: false, wantBalance: uint256.NewInt(0), // unchanged wantError: ErrInsufficientBalanceForSubtraction, }, @@ -123,7 +120,7 @@ func TestUpgradeAccount_BalanceChanges(t *testing.T) { // Set up initial account state if tt.accountExists { - setAccountBalance(t, statedb, testAddr, tt.initialBalance) + setAccountBalance(statedb, testAddr, tt.initialBalance) } upgrade := extras.StateUpgradeAccount{ @@ -131,13 +128,9 @@ func TestUpgradeAccount_BalanceChanges(t *testing.T) { } err := upgradeAccount(testAddr, upgrade, statedb, false) - // Check error expectations - if tt.wantError != nil { - require.Error(t, err) - require.ErrorIs(t, err, tt.wantError) + require.ErrorIs(t, err, tt.wantError) + if err != nil { require.ErrorContains(t, err, testAddr.Hex()) - } else { - require.NoError(t, err) } // Check final balance @@ -190,9 +183,7 @@ func createTestStateDB(t *testing.T) StateDB { } // setAccountBalance is a helper to set an account's initial balance for testing -func setAccountBalance(t *testing.T, statedb StateDB, addr common.Address, balance *uint256.Int) { - t.Helper() - +func setAccountBalance(statedb StateDB, addr common.Address, balance *uint256.Int) { // Cast to extstate.StateDB to access the underlying state extStateDB := statedb.(*extstate.StateDB) extStateDB.CreateAccount(addr) From 3f7b7a0a795c1f313bb8862a02cf36b22acaf6fb Mon Sep 17 00:00:00 2001 From: Jonathan Oppenheimer Date: Tue, 23 Sep 2025 16:48:29 -0400 Subject: [PATCH 07/11] Apply suggestions from code review Co-authored-by: Austin Larson <78000745+alarso16@users.noreply.github.com> Signed-off-by: Jonathan Oppenheimer <147infiniti@gmail.com> --- stateupgrade/state_upgrade.go | 3 ++- stateupgrade/state_upgrade_test.go | 1 - 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/stateupgrade/state_upgrade.go b/stateupgrade/state_upgrade.go index b51ec81abb..30af61073a 100644 --- a/stateupgrade/state_upgrade.go +++ b/stateupgrade/state_upgrade.go @@ -16,7 +16,8 @@ import ( ) var ( - ErrInsufficientBalanceForSubtraction = errors.New("insufficient balance for subtraction") + errInsufficientBalanceForSubtraction = errors.New("insufficient balance for subtraction") + errBalanceOverflow = errors.New("balance overflow") ErrBalanceOverflow = errors.New("balance overflow") ) diff --git a/stateupgrade/state_upgrade_test.go b/stateupgrade/state_upgrade_test.go index 0a010dae5d..2eb30f101b 100644 --- a/stateupgrade/state_upgrade_test.go +++ b/stateupgrade/state_upgrade_test.go @@ -127,7 +127,6 @@ func TestUpgradeAccount_BalanceChanges(t *testing.T) { BalanceChange: tt.balanceChange, } err := upgradeAccount(testAddr, upgrade, statedb, false) - require.ErrorIs(t, err, tt.wantError) if err != nil { require.ErrorContains(t, err, testAddr.Hex()) From 9efe53fdf767a64f2f57a9e3d9199bf6ee83a01e Mon Sep 17 00:00:00 2001 From: Jonathan Oppenheimer Date: Tue, 23 Sep 2025 16:49:00 -0400 Subject: [PATCH 08/11] nits --- stateupgrade/state_upgrade.go | 2 +- stateupgrade/state_upgrade_test.go | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/stateupgrade/state_upgrade.go b/stateupgrade/state_upgrade.go index 30af61073a..006dc61d6e 100644 --- a/stateupgrade/state_upgrade.go +++ b/stateupgrade/state_upgrade.go @@ -58,7 +58,7 @@ func upgradeAccount(account common.Address, upgrade extras.StateUpgradeAccount, currentBalance := state.GetBalance(account) if currentBalance.Cmp(balanceChange) < 0 { return fmt.Errorf("%w: account %s has %s but trying to subtract %s", - ErrInsufficientBalanceForSubtraction, + errInsufficientBalanceForSubtraction, account.Hex(), currentBalance.ToBig().String(), balanceChange.ToBig().String()) } state.SubBalance(account, balanceChange) diff --git a/stateupgrade/state_upgrade_test.go b/stateupgrade/state_upgrade_test.go index 2eb30f101b..1459301ba5 100644 --- a/stateupgrade/state_upgrade_test.go +++ b/stateupgrade/state_upgrade_test.go @@ -51,7 +51,7 @@ func TestUpgradeAccount_BalanceChanges(t *testing.T) { balanceChange: hexOrDecimal256FromInt64(-100), accountExists: true, wantBalance: uint256.NewInt(50), // unchanged - wantError: ErrInsufficientBalanceForSubtraction, + wantError: errInsufficientBalanceForSubtraction, }, { name: "zero balance change", @@ -77,7 +77,7 @@ func TestUpgradeAccount_BalanceChanges(t *testing.T) { initialBalance: uint256.NewInt(0), balanceChange: hexOrDecimal256FromInt64(-100), wantBalance: uint256.NewInt(0), // unchanged - wantError: ErrInsufficientBalanceForSubtraction, + wantError: errInsufficientBalanceForSubtraction, }, { name: "exact balance subtraction", @@ -92,7 +92,7 @@ func TestUpgradeAccount_BalanceChanges(t *testing.T) { balanceChange: hexOrDecimal256FromInt64(-101), accountExists: true, wantBalance: uint256.NewInt(100), // unchanged - wantError: ErrInsufficientBalanceForSubtraction, + wantError: errInsufficientBalanceForSubtraction, }, { name: "large positive balance change", From e355e362a122e3839fe6d7f4bafca4be4ac18cb4 Mon Sep 17 00:00:00 2001 From: Jonathan Oppenheimer Date: Wed, 24 Sep 2025 12:17:15 -0400 Subject: [PATCH 09/11] lint --- stateupgrade/state_upgrade.go | 3 +-- stateupgrade/state_upgrade_test.go | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/stateupgrade/state_upgrade.go b/stateupgrade/state_upgrade.go index 006dc61d6e..3b43355efe 100644 --- a/stateupgrade/state_upgrade.go +++ b/stateupgrade/state_upgrade.go @@ -18,7 +18,6 @@ import ( var ( errInsufficientBalanceForSubtraction = errors.New("insufficient balance for subtraction") errBalanceOverflow = errors.New("balance overflow") - ErrBalanceOverflow = errors.New("balance overflow") ) // Configure applies the state upgrade to the state. @@ -49,7 +48,7 @@ func upgradeAccount(account common.Address, upgrade extras.StateUpgradeAccount, currentBalance := state.GetBalance(account) if new(big.Int).Add(currentBalance.ToBig(), balanceChange.ToBig()).Cmp(abi.MaxUint256) > 0 { return fmt.Errorf("%w: account %s current balance %s + change %s would exceed maximum uint256", - ErrBalanceOverflow, account.Hex(), currentBalance.ToBig().String(), balanceChange.ToBig().String()) + errBalanceOverflow, account.Hex(), currentBalance.ToBig().String(), balanceChange.ToBig().String()) } state.AddBalance(account, balanceChange) case -1: // Negative diff --git a/stateupgrade/state_upgrade_test.go b/stateupgrade/state_upgrade_test.go index 1459301ba5..116bb9f01c 100644 --- a/stateupgrade/state_upgrade_test.go +++ b/stateupgrade/state_upgrade_test.go @@ -109,7 +109,7 @@ func TestUpgradeAccount_BalanceChanges(t *testing.T) { balanceChange: hexOrDecimal256FromInt64(1), accountExists: true, wantBalance: uint256.MustFromBig(abi.MaxUint256), // unchanged - wantError: ErrBalanceOverflow, + wantError: errBalanceOverflow, }, } From b40c6a5feec3c495d55112ba254c41bd6df5529d Mon Sep 17 00:00:00 2001 From: Jonathan Oppenheimer Date: Wed, 24 Sep 2025 14:32:46 -0400 Subject: [PATCH 10/11] remove contains check --- stateupgrade/state_upgrade_test.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/stateupgrade/state_upgrade_test.go b/stateupgrade/state_upgrade_test.go index 116bb9f01c..b18d5290e8 100644 --- a/stateupgrade/state_upgrade_test.go +++ b/stateupgrade/state_upgrade_test.go @@ -128,9 +128,6 @@ func TestUpgradeAccount_BalanceChanges(t *testing.T) { } err := upgradeAccount(testAddr, upgrade, statedb, false) require.ErrorIs(t, err, tt.wantError) - if err != nil { - require.ErrorContains(t, err, testAddr.Hex()) - } // Check final balance actualBalance := statedb.GetBalance(testAddr) From 624c067656430a1190519c59b12e95431e3a09fe Mon Sep 17 00:00:00 2001 From: Jonathan Oppenheimer Date: Tue, 14 Oct 2025 14:12:14 -0400 Subject: [PATCH 11/11] Cey suggestions --- params/extras/state_upgrade.go | 15 +++++++ params/extras/state_upgrade_test.go | 62 +++++++++++++++++++++++++++++ stateupgrade/state_upgrade.go | 54 +++++++++++++------------ stateupgrade/state_upgrade_test.go | 47 +++++++--------------- 4 files changed, 120 insertions(+), 58 deletions(-) diff --git a/params/extras/state_upgrade.go b/params/extras/state_upgrade.go index 530e0b8046..ab651d28a7 100644 --- a/params/extras/state_upgrade.go +++ b/params/extras/state_upgrade.go @@ -5,6 +5,7 @@ package extras import ( "fmt" + "math/big" "reflect" "github.com/ava-labs/libevm/common" @@ -37,6 +38,7 @@ func (s *StateUpgrade) Equal(other *StateUpgrade) bool { // verifyStateUpgrades checks [c.StateUpgrades] is well formed: // - the specified blockTimestamps must monotonically increase +// - all BalanceChange values must fit within uint256 func (c *ChainConfig) verifyStateUpgrades() error { var previousUpgradeTimestamp *uint64 for i, upgrade := range c.StateUpgrades { @@ -54,6 +56,19 @@ func (c *ChainConfig) verifyStateUpgrades() error { return fmt.Errorf("StateUpgrade[%d]: config block timestamp (%v) <= previous timestamp (%v)", i, *upgradeTimestamp, *previousUpgradeTimestamp) } previousUpgradeTimestamp = upgradeTimestamp + + // Verify all BalanceChange values fit within uint256 + for account, accountUpgrade := range upgrade.StateUpgradeAccounts { + if accountUpgrade.BalanceChange != nil { + bigChange := (*big.Int)(accountUpgrade.BalanceChange) + // Check if the absolute value fits in uint256 + absChange := new(big.Int).Abs(bigChange) + if absChange.BitLen() > 256 { + return fmt.Errorf("StateUpgrade[%d]: account %s has BalanceChange %s that exceeds uint256 bit length", + i, account.Hex(), bigChange.String()) + } + } + } } return nil } diff --git a/params/extras/state_upgrade_test.go b/params/extras/state_upgrade_test.go index 4b6819641b..12fa515199 100644 --- a/params/extras/state_upgrade_test.go +++ b/params/extras/state_upgrade_test.go @@ -22,6 +22,14 @@ func TestVerifyStateUpgrades(t *testing.T) { BalanceChange: (*math.HexOrDecimal256)(common.Big1), }, } + // Create a value that exceeds uint256 (2^256 + 1) + maxUint256 := new(big.Int).Lsh(big.NewInt(1), 256) + maxUint256.Sub(maxUint256, big.NewInt(1)) // 2^256 - 1 + overflowValue := new(big.Int).Add(maxUint256, big.NewInt(2)) + + // Create a negative value that exceeds uint256 in absolute value + negativeOverflowValue := new(big.Int).Neg(overflowValue) + tests := []struct { name string upgrades []StateUpgrade @@ -57,6 +65,60 @@ func TestVerifyStateUpgrades(t *testing.T) { }, expectedError: "config block timestamp (0) must be greater than 0", }, + { + name: "balance change exceeds uint256", + upgrades: []StateUpgrade{ + { + BlockTimestamp: utils.NewUint64(1), + StateUpgradeAccounts: map[common.Address]StateUpgradeAccount{ + {1}: { + BalanceChange: (*math.HexOrDecimal256)(overflowValue), + }, + }, + }, + }, + expectedError: "exceeds uint256 bit length", + }, + { + name: "negative balance change exceeds uint256", + upgrades: []StateUpgrade{ + { + BlockTimestamp: utils.NewUint64(1), + StateUpgradeAccounts: map[common.Address]StateUpgradeAccount{ + {1}: { + BalanceChange: (*math.HexOrDecimal256)(negativeOverflowValue), + }, + }, + }, + }, + expectedError: "exceeds uint256 bit length", + }, + { + name: "max uint256 balance change is valid", + upgrades: []StateUpgrade{ + { + BlockTimestamp: utils.NewUint64(1), + StateUpgradeAccounts: map[common.Address]StateUpgradeAccount{ + {1}: { + BalanceChange: (*math.HexOrDecimal256)(maxUint256), + }, + }, + }, + }, + }, + { + name: "negative max uint256 balance change is valid", + upgrades: []StateUpgrade{ + { + BlockTimestamp: utils.NewUint64(1), + StateUpgradeAccounts: map[common.Address]StateUpgradeAccount{ + {1}: { + BalanceChange: (*math.HexOrDecimal256)(new(big.Int).Neg(maxUint256)), + }, + }, + }, + }, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { diff --git a/stateupgrade/state_upgrade.go b/stateupgrade/state_upgrade.go index 3b43355efe..32d2fecfa7 100644 --- a/stateupgrade/state_upgrade.go +++ b/stateupgrade/state_upgrade.go @@ -4,35 +4,28 @@ package stateupgrade import ( - "errors" - "fmt" "math/big" - "github.com/ava-labs/libevm/accounts/abi" "github.com/ava-labs/libevm/common" + "github.com/ava-labs/libevm/log" "github.com/holiman/uint256" "github.com/ava-labs/subnet-evm/params/extras" ) -var ( - errInsufficientBalanceForSubtraction = errors.New("insufficient balance for subtraction") - errBalanceOverflow = errors.New("balance overflow") -) - // Configure applies the state upgrade to the state. +// Note: This function should not return errors as any error would break the chain. +// All validation should be done in verifyStateUpgrades at config load time. func Configure(stateUpgrade *extras.StateUpgrade, chainConfig ChainContext, state StateDB, blockContext BlockContext) error { isEIP158 := chainConfig.IsEIP158(blockContext.Number()) for account, upgrade := range stateUpgrade.StateUpgradeAccounts { - if err := upgradeAccount(account, upgrade, state, isEIP158); err != nil { - return err - } + upgradeAccount(account, upgrade, state, isEIP158) } return nil } // upgradeAccount applies the state upgrade to the given account. -func upgradeAccount(account common.Address, upgrade extras.StateUpgradeAccount, state StateDB, isEIP158 bool) error { +func upgradeAccount(account common.Address, upgrade extras.StateUpgradeAccount, state StateDB, isEIP158 bool) { // Create the account if it does not exist if !state.Exist(account) { state.CreateAccount(account) @@ -41,29 +34,41 @@ func upgradeAccount(account common.Address, upgrade extras.StateUpgradeAccount, // Balance change detected - update the balance of the account if upgrade.BalanceChange != nil { // BalanceChange is a HexOrDecimal256, which is just a typed big.Int + // Note: We validate at config load time that the change itself fits in uint256, + // but we still need to check if the resulting balance would overflow/underflow. bigChange := (*big.Int)(upgrade.BalanceChange) + currentBalance := state.GetBalance(account) + switch bigChange.Sign() { - case 1: // Positive + case 1: // Positive - check for overflow balanceChange, _ := uint256.FromBig(bigChange) - currentBalance := state.GetBalance(account) - if new(big.Int).Add(currentBalance.ToBig(), balanceChange.ToBig()).Cmp(abi.MaxUint256) > 0 { - return fmt.Errorf("%w: account %s current balance %s + change %s would exceed maximum uint256", - errBalanceOverflow, account.Hex(), currentBalance.ToBig().String(), balanceChange.ToBig().String()) + _, overflow := new(uint256.Int).AddOverflow(currentBalance, balanceChange) + if overflow { + log.Warn("State upgrade balance change would overflow, clamping to max uint256", + "account", account.Hex(), + "currentBalance", currentBalance.ToBig().String(), + "balanceChange", balanceChange.ToBig().String()) + state.SubBalance(account, currentBalance) + state.AddBalance(account, new(uint256.Int).SetAllOne()) + } else { + state.AddBalance(account, balanceChange) } - state.AddBalance(account, balanceChange) - case -1: // Negative + case -1: // Negative - check for underflow absChange := new(big.Int).Abs(bigChange) balanceChange, _ := uint256.FromBig(absChange) - currentBalance := state.GetBalance(account) if currentBalance.Cmp(balanceChange) < 0 { - return fmt.Errorf("%w: account %s has %s but trying to subtract %s", - errInsufficientBalanceForSubtraction, - account.Hex(), currentBalance.ToBig().String(), balanceChange.ToBig().String()) + log.Warn("State upgrade balance change would underflow, clamping to zero", + "account", account.Hex(), + "currentBalance", currentBalance.ToBig().String(), + "balanceChange", balanceChange.ToBig().String()) + state.SubBalance(account, currentBalance) + } else { + state.SubBalance(account, balanceChange) } - state.SubBalance(account, balanceChange) } // If zero (Sign() == 0), do nothing } + if len(upgrade.Code) != 0 { // if the nonce is 0, set the nonce to 1 as we would when deploying a contract at // the address. @@ -75,5 +80,4 @@ func upgradeAccount(account common.Address, upgrade extras.StateUpgradeAccount, for key, value := range upgrade.Storage { state.SetState(account, key, value) } - return nil } diff --git a/stateupgrade/state_upgrade_test.go b/stateupgrade/state_upgrade_test.go index b18d5290e8..a1679debc6 100644 --- a/stateupgrade/state_upgrade_test.go +++ b/stateupgrade/state_upgrade_test.go @@ -7,7 +7,6 @@ import ( "math/big" "testing" - "github.com/ava-labs/libevm/accounts/abi" "github.com/ava-labs/libevm/common" "github.com/ava-labs/libevm/common/math" "github.com/ava-labs/libevm/core/rawdb" @@ -29,7 +28,6 @@ func TestUpgradeAccount_BalanceChanges(t *testing.T) { balanceChange *math.HexOrDecimal256 accountExists bool wantBalance *uint256.Int - wantError error }{ { name: "positive balance change on existing account", @@ -45,14 +43,6 @@ func TestUpgradeAccount_BalanceChanges(t *testing.T) { accountExists: true, wantBalance: uint256.NewInt(70), }, - { - name: "negative balance change with insufficient funds", - initialBalance: uint256.NewInt(50), - balanceChange: hexOrDecimal256FromInt64(-100), - accountExists: true, - wantBalance: uint256.NewInt(50), // unchanged - wantError: errInsufficientBalanceForSubtraction, - }, { name: "zero balance change", initialBalance: uint256.NewInt(100), @@ -72,13 +62,6 @@ func TestUpgradeAccount_BalanceChanges(t *testing.T) { balanceChange: hexOrDecimal256FromInt64(1000), wantBalance: uint256.NewInt(1000), }, - { - name: "new account with negative balance", - initialBalance: uint256.NewInt(0), - balanceChange: hexOrDecimal256FromInt64(-100), - wantBalance: uint256.NewInt(0), // unchanged - wantError: errInsufficientBalanceForSubtraction, - }, { name: "exact balance subtraction", initialBalance: uint256.NewInt(100), @@ -86,14 +69,6 @@ func TestUpgradeAccount_BalanceChanges(t *testing.T) { accountExists: true, wantBalance: uint256.NewInt(0), }, - { - name: "off-by-one underflow", - initialBalance: uint256.NewInt(100), - balanceChange: hexOrDecimal256FromInt64(-101), - accountExists: true, - wantBalance: uint256.NewInt(100), // unchanged - wantError: errInsufficientBalanceForSubtraction, - }, { name: "large positive balance change", // Initial balance: 2^255 - 1000 (a very large number near half of max uint256) @@ -104,12 +79,19 @@ func TestUpgradeAccount_BalanceChanges(t *testing.T) { wantBalance: uint256.MustFromBig(new(big.Int).Sub(new(big.Int).Lsh(big.NewInt(1), 255), big.NewInt(500))), }, { - name: "balance overflow protection", - initialBalance: uint256.MustFromBig(abi.MaxUint256), // Max uint256 - balanceChange: hexOrDecimal256FromInt64(1), + name: "balance overflow clamped to max uint256", + // Set initial balance to max uint256 - 100 + initialBalance: new(uint256.Int).Sub(new(uint256.Int).SetAllOne(), uint256.NewInt(100)), + balanceChange: hexOrDecimal256FromInt64(200), // This would overflow accountExists: true, - wantBalance: uint256.MustFromBig(abi.MaxUint256), // unchanged - wantError: errBalanceOverflow, + wantBalance: new(uint256.Int).SetAllOne(), // max uint256 + }, + { + name: "balance underflow clamped to zero", + initialBalance: uint256.NewInt(50), + balanceChange: hexOrDecimal256FromInt64(-100), // More than current balance + accountExists: true, + wantBalance: uint256.NewInt(0), }, } @@ -126,8 +108,7 @@ func TestUpgradeAccount_BalanceChanges(t *testing.T) { upgrade := extras.StateUpgradeAccount{ BalanceChange: tt.balanceChange, } - err := upgradeAccount(testAddr, upgrade, statedb, false) - require.ErrorIs(t, err, tt.wantError) + upgradeAccount(testAddr, upgrade, statedb, false) // Check final balance actualBalance := statedb.GetBalance(testAddr) @@ -153,7 +134,7 @@ func TestUpgradeAccount_CompleteUpgrade(t *testing.T) { Code: code, Storage: map[common.Hash]common.Hash{storageKey: storageValue}, } - require.NoError(t, upgradeAccount(addr, upgrade, statedb, true)) // Test with EIP158 = true + upgradeAccount(addr, upgrade, statedb, true) // Test with EIP158 = true // Verify all changes were applied require.Equal(t, uint256.NewInt(1000), statedb.GetBalance(addr))