-
Notifications
You must be signed in to change notification settings - Fork 119
Description
Description
Currently, the ICS20 precompile does not correctly reflect balance changes to the stateDB when the transfer amount is equal to UnboundedSpendLimit.
This causes a mismatch between bankKeeper state and stateDB when the actual amount transferred is adjusted to the spendable amount inside the IBC transfer logic.
The core issue is that the balance change entries are always set based on the original msg.Token.Amount, which can be unbounded. However, when UnboundedSpendLimit is detected, the actual send amount is reduced to the spendable amount:
https://github.com/cosmos/ibc-go/blob/main/modules/apps/transfer/keeper/msg_server.go#L37-L43
// Using types.UnboundedSpendLimit allows us to send the entire balance of a given denom.
if coin.Amount.Equal(types.UnboundedSpendLimit()) {
coin.Amount = k.BankKeeper.SpendableCoin(ctx, sender, coin.Denom).Amount
if coin.Amount.IsZero() {
return nil, errorsmod.Wrapf(types.ErrInvalidAmount, "empty spendable balance for %s", coin.Denom)
}
}Thus, the following logic:
evmDenom := evmtypes.GetEVMCoinDenom()
sendAmt := msg.Token.Amount
if sendAmt.GTE(transfertypes.UnboundedSpendLimit()) {
spendable := p.bankKeeper.SpendableCoin(ctx, sender.Bytes(), evmDenom)
sendAmt = spendable.Amount
}
res, err := p.transferKeeper.Transfer(ctx, msg)
if err != nil {
return nil, err
}
if msg.Token.Denom == evmDenom {
// escrow address is also changed on this tx, and it is not a module account
// so we need to account for this on the UpdateDirties
escrowAccAddress := transfertypes.GetEscrowAddress(msg.SourcePort, msg.SourceChannel)
escrowHexAddr := common.BytesToAddress(escrowAccAddress)
// NOTE: This ensures that the changes in the bank keeper are correctly mirrored to the EVM stateDB
// when calling the precompile from another smart contract.
// This prevents the stateDB from overwriting the changed balance in the bank keeper when committing the EVM state.
amt, err := utils.Uint256FromBigInt(sendAmt.BigInt())
if err != nil {
return nil, err
}
p.SetBalanceChangeEntries(
cmn.NewBalanceChangeEntry(sender, amt, cmn.Sub),
cmn.NewBalanceChangeEntry(escrowHexAddr, amt, cmn.Add),
)
}must be applied to current implementation for reflecting the adjusted sendAmt value to avoid state inconsistency.
Fix Status
I’ve already reproduced the issue and currently working on a fix to ensure balance changes reflect the final transferred amount after UnboundedSpendLimit adjustment.