-
Notifications
You must be signed in to change notification settings - Fork 76
feat: Delete EVM instance in AnteHandler #352
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
ante/evm/07_can_transfer.go
Outdated
|
||
// check that caller has enough balance to cover asset transfer for **topmost** call | ||
// NOTE: here the gas consumed is from the context with the infinite gas meter | ||
convertedValue, err := utils.Uint256FromBigInt(msg.Value) | ||
if err != nil { | ||
return err | ||
} | ||
if msg.Value.Sign() > 0 && !evm.Context.CanTransfer(stateDB, msg.From, convertedValue) { | ||
if msg.Value.Sign() > 0 && stateDB.GetBalance(msg.From).Cmp(convertedValue) < 0 { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
don't event need to allocate a statedb, just call evmKeeper
should be good?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That makes sense. I'll apply it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the current StateDB.GetBalance implementation, if the state object has already been cached, it returns the cached balance:
// GetBalance retrieves the balance from the given address or 0 if object not found
func (s *StateDB) GetBalance(addr common.Address) *uint256.Int {
stateObject := s.getStateObject(addr)
if stateObject != nil {
return stateObject.Balance()
}
return common.U2560
}
So if we switch this to using the keeper instead, it would bypass this caching logic. We should take a moment to consider whether this could introduce any unintended side effects.
That said, in the ante handler, the StateDB is freshly created for each transaction. Since the object cache is bound to each individual StateDB instance, this change should be safe. In other words, we’re shifting from a cached balance lookup in the state object to a direct query via the ctx and keeper, but because the cache would’ve been newly initialized anyway, this won’t cause inconsistencies.
// New creates a new state from a given trie.
func New(ctx sdk.Context, keeper Keeper, txConfig TxConfig) *StateDB {
return &StateDB{
keeper: keeper,
ctx: ctx,
stateObjects: make(map[common.Address]*stateObject),
journal: newJournal(),
accessList: newAccessList(),
transientStorage: newTransientStorage(),
txConfig: txConfig,
}
}
So overall, I don’t think this change introduces an issue, but it’s worth highlighting that we’re now relying on the balance from ctx rather than the potentially-cached balance from StateDB.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the great suggestion.
I applied the revised commit and reran the experiments without any issues.
And the benchmarks show that both allocs/op and B/op have decreased by over 40%.
go test -benchmem -run=^$ -bench ^BenchmarkAnteHandler$ github.com/cosmos/evm/evmd/ante
goos: darwin
goarch: arm64
pkg: github.com/cosmos/evm/evmd/ante
cpu: Apple M1 Pro
BenchmarkAnteHandler/tx_type_evm_transfer_sim-10 17802 70048 ns/op 24593 B/op 447 allocs/op
BenchmarkAnteHandler/tx_type_evm_transfer-10 16951 71668 ns/op 24593 B/op 447 allocs/op
BenchmarkAnteHandler/tx_type_bank_msg_send_sim-10 18535 64953 ns/op 33776 B/op 521 allocs/op
BenchmarkAnteHandler/tx_type_bank_msg_send-10 10000 125510 ns/op 39836 B/op 629 allocs/op
PASS
ok github.com/cosmos/evm/evmd/ante 162.720s
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's make an EVM Keeper call
Description
Closes: #351
Summary
Remove the creation of a Geth EVM instance during the AnteHandler balance check.
Notes
Background
Cosmos EVM currently instantiates a full Geth EVM engine twice per transaction—once for pre‑validation (
CanTransfer
) and once for execution—leading to significant CPU, memory, and GC overhead. By eliminating the unnecessary VM spin‑up in the AnteHandler, we can streamline the pre‑validation path without affecting state integrity.Changes
Before: In
ante/evm/07_can_transfer.go
, callevmKeeper.NewEVM()
on every transaction to create a “first” EVM instance and then invokevm.CanTransfer()
.After: Refactored CanTransfer() to remove the
NewEVM()
call and perform a direct balance lookup viaStateDB.GetBalance()
. Same results, but Compact.(Reference: Geth’s pre-validation path also does not spin up an EVM instance.)
All existing tests pass without modification.
Benchmark
Benchstat
go version go1.24.4
cpu : M1 pro, memory : 16 GB
old benchmark
Date 0724 02:10 AM PST
Branch : [old/delete-evm-instance-ante]
latest benchmark
Date 0724 02:40 AM PST
Branch : [feat/delete-evm-instance-ante]
Average time per operation (ns/op): ↓ 4.54%
Heap allocation bytes (B/op): ↓ 11.76%
Heap allocation calls (allocs/op): ↓ 11.78%
Interpretation:
newEVM()
.Expect shorter and less frequent GC pauses in production, resulting in lower processing latency, although this performance gain isn’t reflected in the current benchmark.
Reproduce this benchmark
Since there was no performance improvement, I mocked tx types bank-msg-send
Author Checklist
All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.
I have...
main
branchReviewers Checklist
All items are required.
Please add a note if the item is not applicable
and please add your handle next to the items reviewed
if you only reviewed selected items.
I have...
Unreleased
section inCHANGELOG.md