diff --git a/action/protocol/context.go b/action/protocol/context.go index 247604f875..790242478d 100644 --- a/action/protocol/context.go +++ b/action/protocol/context.go @@ -167,6 +167,10 @@ type ( CandidateBLSPublicKeyNotCopied bool OnlyOwnerCanUpdateBLSPublicKey bool PrePectraEVM bool + // AlwaysWriteCachedContract if true, CommitContracts writes back all cached + // contracts regardless of whether they were modified; if false, only dirty + // contracts are committed and written back + AlwaysWriteCachedContract bool NoCandidateExitQueue bool } @@ -339,6 +343,7 @@ func WithFeatureCtx(ctx context.Context) context.Context { CandidateBLSPublicKeyNotCopied: !g.IsXinguBeta(height), OnlyOwnerCanUpdateBLSPublicKey: !g.IsToBeEnabled(height), PrePectraEVM: !g.IsToBeEnabled(height), + AlwaysWriteCachedContract: !g.IsToBeEnabled(height), NoCandidateExitQueue: !g.IsToBeEnabled(height), }, ) diff --git a/action/protocol/execution/evm/contract.go b/action/protocol/execution/evm/contract.go index 6b0a74a39d..237f197d8a 100644 --- a/action/protocol/execution/evm/contract.go +++ b/action/protocol/execution/evm/contract.go @@ -37,6 +37,7 @@ type ( SetCode(hash.Hash256, []byte) SelfState() *state.Account Commit() error + Dirty() bool LoadRoot() error Iterator() (trie.Iterator, error) Snapshot() Contract @@ -125,6 +126,11 @@ func (c *contract) SelfState() *state.Account { return c.Account } +// Dirty returns whether the contract has been modified +func (c *contract) Dirty() bool { + return c.dirtyCode || c.dirtyState +} + // Commit writes the changes into underlying trie func (c *contract) Commit() error { if c.dirtyState { diff --git a/action/protocol/execution/evm/contract_erigon.go b/action/protocol/execution/evm/contract_erigon.go index 9d17e32c7d..a0a604422e 100644 --- a/action/protocol/execution/evm/contract_erigon.go +++ b/action/protocol/execution/evm/contract_erigon.go @@ -71,6 +71,15 @@ func (c *contractErigon) SelfState() *state.Account { return acc } +// Dirty returns false because contractErigon does not track dirty state itself; +// all mutations go directly into Erigon's IntraBlockState, which is persisted +// separately via IntraBlockState.CommitBlock() at the block level. +// In normal execution mode, contractErigon is wrapped inside contractAdapter +// whose Dirty() is delegated to the embedded mptrie contract. +func (c *contractErigon) Dirty() bool { + return false +} + func (c *contractErigon) Commit() error { return nil } diff --git a/action/protocol/execution/evm/evm.go b/action/protocol/execution/evm/evm.go index 70613f2f1b..848b468d64 100644 --- a/action/protocol/execution/evm/evm.go +++ b/action/protocol/execution/evm/evm.go @@ -525,6 +525,9 @@ func prepareStateDBAdapter(ctx context.Context, sm protocol.StateManager) (*Stat if featureCtx.PrePectraEVM { opts = append(opts, IgnoreBalanceChangeTouchAccountOption()) } + if !featureCtx.AlwaysWriteCachedContract { + opts = append(opts, SkipWriteCleanContractOption()) + } return NewStateDBAdapter( sm, blkCtx.BlockHeight, diff --git a/action/protocol/execution/evm/evmstatedbadapter.go b/action/protocol/execution/evm/evmstatedbadapter.go index 63a392d388..d41aa91bf2 100644 --- a/action/protocol/execution/evm/evmstatedbadapter.go +++ b/action/protocol/execution/evm/evmstatedbadapter.go @@ -93,6 +93,10 @@ type ( fixRevertSnapshot bool // ignoreBalanceChangeTouchAccount indicates whether to ignore balance change touch account ignoreBalanceChangeTouchAccount bool + // skipWriteCleanContract indicates whether to skip writing back read-only + // contracts in CommitContracts; when true, only contracts with dirty state + // or code are committed and written back to the state trie + skipWriteCleanContract bool } ) @@ -221,6 +225,14 @@ func IgnoreBalanceChangeTouchAccountOption() StateDBAdapterOption { } } +// SkipWriteCleanContractOption set skipWriteCleanContract as true +func SkipWriteCleanContractOption() StateDBAdapterOption { + return func(adapter *StateDBAdapter) error { + adapter.skipWriteCleanContract = true + return nil + } +} + // NewStateDBAdapter creates a new state db with iotex blockchain func NewStateDBAdapter( sm protocol.StateManager, @@ -1125,14 +1137,21 @@ func (stateDB *StateDBAdapter) SetState(evmAddr common.Address, k, v common.Hash } func (stateDB *StateDBAdapter) GetStorageRoot(evmAddr common.Address) common.Hash { - contract, err := stateDB.getContractWoCreate(evmAddr) + // check the contract cache first for up-to-date Root + if contract, ok := stateDB.cachedContract[evmAddr]; ok { + return common.BytesToHash(contract.SelfState().Root[:]) + } + // read the account directly without adding to cachedContract, so that + // CommitContracts won't write back an unmodified account with a recomputed + // (non-zero) empty-trie Root + account, err := accountutil.Recorded(stateDB.sm, evmAddr) switch errors.Cause(err) { case nil: - return common.BytesToHash(contract.SelfState().Root[:]) + return common.BytesToHash(account.Root[:]) case state.ErrStateNotExist: return common.Hash{} default: - log.T(stateDB.ctx).Error("Failed to get contract.", zap.Error(err), zap.String("address", evmAddr.Hex())) + log.T(stateDB.ctx).Error("Failed to get account.", zap.Error(err), zap.String("address", evmAddr.Hex())) stateDB.logError(err) return common.Hash{} } @@ -1167,6 +1186,13 @@ func (stateDB *StateDBAdapter) CommitContracts() error { continue } contract := stateDB.cachedContract[addr] + if stateDB.skipWriteCleanContract && !contract.Dirty() { + // a read-only contract (loaded by GetState/GetCommittedState but never + // modified by SetState/SetCode) does not need Commit or PutState; + // skipping it avoids writing back a stale Root that was recomputed by + // Snapshot() on an empty storage trie + continue + } err := contract.Commit() if stateDB.assertError(err, "failed to commit contract", zap.Error(err), zap.String("address", addr.Hex())) { return errors.Wrap(err, "failed to commit contract") @@ -1235,24 +1261,6 @@ func (stateDB *StateDBAdapter) getNewContract(evmAddr common.Address) (Contract, return contract, nil } -func (stateDB *StateDBAdapter) getContractWoCreate(evmAddr common.Address) (Contract, error) { - if contract, ok := stateDB.cachedContract[evmAddr]; ok { - return contract, nil - } - addr := hash.BytesToHash160(evmAddr.Bytes()) - account, err := accountutil.Recorded(stateDB.sm, evmAddr) - if err != nil { - return nil, err - } - contract, err := stateDB.newContract(addr, account) - if err != nil { - return nil, errors.Wrapf(err, "failed to create storage trie for new contract %x", addr) - } - // add to contract cache - stateDB.cachedContract[evmAddr] = contract - return contract, nil -} - // clear clears local changes func (stateDB *StateDBAdapter) clear() { stateDB.refundSnapshot = make(map[int]uint64)