cosmos
[Feature]: x/bank: Remove HasAccount check if the recipient account already has a balance
Summary
The HasAccount check within x/bank SendCoins is a large performance bottleneck right now. It has been a bottleneck in the past as well, with the Osmosis epoch.
This check is currently done, to enforce a guarantee of the form "If the recipient receives coins, it has an associated account".
In Osmosis, we recently had blocks that contained 20k sends (from 10 multisend tx's, each to 2k addresses) due to spam. This is currently 40k sequential reads. This made the block time degrade to 7s. This is applicable to every cosmos chain, up to their gas bounds atm.
Problem Definition
There are a number of ways to speed this up. I'll make issues later for suggestions on how we can do parallelized/bulk loads, to enable parallel data reads. For context of the problem here, a random (uncached anywhere) SSD read is ~50-200 microseconds. So just reading this data, let alone writes, at say 100microseconds/read is 4s. This data is in legacy IAVL node types on ~most chains, so it is relatively safe to assume its an uncached read.
The simplest course of action, is to make this do less I/O. Due to caching and that accounts don't alter as often as balances, its actually the case that the account fetches is more than 50% of this latency on Osmosis.
Notice that the guarantee we are aiming to create is:
This check is currently done, to enforce a guarantee of the form "If the recipient receives coins, it has an associated account".
However we get the recipients balance before the HasAccount check. So we can simply change the code from (pseudocode)
bal, err := recipient.GetBalance(denom)
if err != nil {
return err
}
if !HasAccount(recipient) {
SetAccount(recipient)
}
To instead be
bal, err := recipient.GetBalance(denom)
if err != nil {
return err
}
if bal.IsZero() {
if !HasAccount(recipient) {
SetAccount(recipient)
}
}
This would break gas numbers (lowering them), so we cannot do this in a patch release unfortunately. However this would notably speedup all Token Sends, even outside this spam vector.
Proposed Feature
See above
This definitely makes sense to me. We are mid cutting v53, but we are planning a fast follow up with cometbft v1 support. Could be worthwhile to get this into that release
I can submit a PR for this today if that helps. (My ideal is once tested on testnet for several days, getting it into an Osmosis release to remove the risk of this DOS concern)
