feat: Skip no-op contract state changes

missing cosmos store changes still

x/evm/keeper/statedb.go

@@ -22,6 +22,8 @@ import (
 	sdkmath "cosmossdk.io/math"
 
 	errorsmod "cosmossdk.io/errors"
+	"github.com/cosmos/cosmos-sdk/store/cachekv"
+	"github.com/cosmos/cosmos-sdk/store/gaskv"
 	"github.com/cosmos/cosmos-sdk/store/prefix"
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	authtypes "github.com/cosmos/cosmos-sdk/x/auth/types"
@@ -186,6 +188,35 @@ func (k *Keeper) SetState(ctx sdk.Context, addr common.Address, key, value commo
 	)
 }
 
+func (k *Keeper) UnsetState(ctx sdk.Context, addr common.Address, key common.Hash) error {
+	ctxStore := ctx.KVStore(k.storeKey)
+	gasKVStore, ok := ctxStore.(*gaskv.Store)
+	if !ok {
+		return fmt.Errorf("expected gaskv.Store, got %T", ctxStore)
+	}
+
+	// Use parent of store and try as cachekv.Store
+	ctxStore = gasKVStore.GetParent()
+
+	cacheKVStore, ok := ctxStore.(*cachekv.Store)
+	if !ok {
+		return fmt.Errorf("expected cachekv.Store, got %T", ctxStore)
+	}
+
+	storeKey := types.AddressStoragePrefix(addr)
+	storeKey = append(storeKey, key.Bytes()...)
+
+	cacheKVStore.Unset(storeKey)
+
+	k.Logger(ctx).Debug(
+		"state unset",
+		"ethereum-address", addr.Hex(),
+		"key", key.Hex(),
+	)
+
+	return nil
+}
+
 // SetCode set contract code, delete if code is empty.
 func (k *Keeper) SetCode(ctx sdk.Context, codeHash, code []byte) {
 	store := prefix.NewStore(ctx.KVStore(k.storeKey), types.KeyPrefixCode)

x/evm/statedb/ctx.go

@@ -29,7 +29,7 @@ func NewSnapshotCtx(initialCtx sdk.Context) *SnapshotCommitCtx {
 	// Create an initial snapshot of the initialCtx so no state is written until
 	// Commit() is called. The ID is -1 but disregarded along with the
 	// StoreRevertKey indices as this is only to branch the ctx.
-	_ = sCtx.Snapshot(0, StoreRevertKey{0, 0, 0})
+	_ = sCtx.Snapshot(0, StoreRevertKey{0, 0, 0, 0})
 
 	return sCtx
 }

x/evm/statedb/interfaces.go

@@ -45,6 +45,8 @@ type Keeper interface {
 	// Write methods, only called by `StateDB.Commit()`
 	SetAccount(ctx sdk.Context, addr common.Address, account Account) error
 	SetState(ctx sdk.Context, addr common.Address, key, value common.Hash)
+	UnsetState(ctx sdk.Context, addr common.Address, key common.Hash) error
+
 	SetCode(ctx sdk.Context, codeHash []byte, code []byte)
 	SetBalance(ctx sdk.Context, addr common.Address, amount *big.Int) error
 	DeleteAccount(ctx sdk.Context, addr common.Address) error

x/evm/statedb/statedb.go

@@ -253,12 +253,9 @@ func (s *StateDB) ForEachStorage(addr common.Address, cb func(key, value common.
 
 // AddBalance adds amount to the account associated with addr.
 func (s *StateDB) AddBalance(addr common.Address, amount *big.Int) {
-	// Only allow positive amounts.
-	// TODO: Geth apparently allows negative amounts, but can cause negative
-	// balance which is not allowed in bank keeper
-	if amount.Sign() != 1 {
-		return
-	}
+	// Geth apparently allows negative amounts, but can cause negative
+	// balance which is not allowed in bank keeper. However, we need to create
+	// the account still.
 
 	account := s.getOrNewAccount(addr)
 
@@ -321,6 +318,9 @@ func (s *StateDB) SetState(addr common.Address, key, value common.Hash) {
 	//
 	// End result: 0x0, but we cannot skip step 3 or it will be incorrectly 0x1
 	s.keeper.SetState(s.ctx.CurrentCtx(), addr, key, value)
+
+	// Keep track of the key that had a state change
+	s.ephemeralStore.AddContractStateKey(addr, key)
 }
 
 // Suicide marks the given account as suicided.
@@ -447,6 +447,34 @@ func (s *StateDB) Commit() error {
 		}
 	}
 
+	// Check for any state no-op changes and skip committing
+	for _, change := range s.ephemeralStore.GetContractStateKeys() {
+		committedValue := s.keeper.GetState(s.ctx.initialCtx, change.Addr, change.Key)
+		dirtyValue := s.keeper.GetState(s.ctx.CurrentCtx(), change.Addr, change.Key)
+
+		// Different committed and dirty value, so we need to commit
+		if committedValue != dirtyValue {
+			continue
+		}
+
+		// Remove no-op change from ALL snapshot contexts to prevent writing it
+		// to the store. This is necessary since the same key/value will still
+		// update the internal IAVL node version and thus the hash.
+
+		// We can't just remove it from the latest snapshot context, since all
+		// snapshots are written to the store. A snapshot in the "middle" may
+		// contain a state change that isn't just a no-op change but a
+		// completely different value.
+		// Example: A -(snapshot)-> B -(snapshot)-> A
+		// -> We need to remove BOTH the B -> A change and the A -> B change,
+		//    otherwise B will be written to the store.
+		for _, snapshot := range s.ctx.snapshots {
+			if err := s.keeper.UnsetState(snapshot.ctx, change.Addr, change.Key); err != nil {
+				return err
+			}
+		}
+	}
+
 	// Commit after account deletions
 	s.ctx.Commit()
 

x/evm/statedb/store.go

@@ -12,6 +12,13 @@ type StoreRevertKey struct {
 	RefundIndex           int
 	SuicidedAccountsIndex int
 	LogsIndex             int
+	ContractStatesIndex   int
+}
+
+// ContractStateKey represents the state key of a contract.
+type ContractStateKey struct {
+	Addr common.Address
+	Key  common.Hash
 }
 
 // EphemeralStore provides in-memory state of the refund and suicided accounts
@@ -20,6 +27,7 @@ type EphemeralStore struct {
 	RefundStates          []uint64
 	SuicidedAccountStates []common.Address
 	Logs                  []*ethtypes.Log
+	ContractStates        []ContractStateKey
 }
 
 // NewEphemeralStore creates a new EphemeralStore.
@@ -28,6 +36,7 @@ func NewEphemeralStore() *EphemeralStore {
 		RefundStates:          nil,
 		SuicidedAccountStates: nil,
 		Logs:                  nil,
+		ContractStates:        nil,
 	}
 }
 
@@ -37,6 +46,7 @@ func (es *EphemeralStore) GetRevertKey() StoreRevertKey {
 		RefundIndex:           len(es.RefundStates),
 		SuicidedAccountsIndex: len(es.SuicidedAccountStates),
 		LogsIndex:             len(es.Logs),
+		ContractStatesIndex:   len(es.ContractStates),
 	}
 }
 
@@ -49,6 +59,7 @@ func (es *EphemeralStore) Revert(key StoreRevertKey) {
 	es.RefundStates = es.RefundStates[:key.RefundIndex]
 	es.SuicidedAccountStates = es.SuicidedAccountStates[:key.SuicidedAccountsIndex]
 	es.Logs = es.Logs[:key.LogsIndex]
+	es.ContractStates = es.ContractStates[:key.ContractStatesIndex]
 }
 
 func (es *EphemeralStore) ValidateRevertKey(key StoreRevertKey) error {
@@ -73,6 +84,13 @@ func (es *EphemeralStore) ValidateRevertKey(key StoreRevertKey) error {
 		)
 	}
 
+	if key.ContractStatesIndex > len(es.ContractStates) {
+		return fmt.Errorf(
+			"invalid ContractStatesIndex, %d is greater than the length of the contract states (%d)",
+			key.ContractStatesIndex, len(es.ContractStates),
+		)
+	}
+
 	return nil
 }
 
@@ -147,3 +165,30 @@ func (es *EphemeralStore) GetAccountSuicided(addr common.Address) bool {
 func (es *EphemeralStore) GetAllSuicided() []common.Address {
 	return es.SuicidedAccountStates
 }
+
+// -----------------------------------------------------------------------------
+// Contract states
+
+// AddContractState adds a contract state to the store.
+func (es *EphemeralStore) AddContractStateKey(addr common.Address, key common.Hash) {
+	if es.HasContractStateKey(addr, key) {
+		return
+	}
+
+	es.ContractStates = append(es.ContractStates, ContractStateKey{Addr: addr, Key: key})
+}
+
+func (es *EphemeralStore) HasContractStateKey(addr common.Address, key common.Hash) bool {
+	for _, state := range es.ContractStates {
+		if state.Addr == addr && state.Key == key {
+			return true
+		}
+	}
+
+	return false
+}
+
+// GetContractStateKeys returns all contract state keys.
+func (es *EphemeralStore) GetContractStateKeys() []ContractStateKey {
+	return es.ContractStates
+}