Skip to content

ICS20 Transfer precompile does not correctly handle UnboundedSpendLimit cases #221

@zsystm

Description

@zsystm

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.

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions