Skip to content

Commit

Permalink
Refactor MultiSend interceptor to work only with 1 input (#660)
Browse files Browse the repository at this point in the history
# Description
With a change introduced to bank Multisend in Cosmos SDK v0.47, only a single sender can be specified in Multisend. In this PR we take advantage of this change to make the logic of our interceptor much simpler. 
# Reviewers checklist:
- [ ] Try to write more meaningful comments with clear actions to be taken.
- [ ] Nit-picking should be unblocking. Focus on core issues.

# Authors checklist
- [x] Provide a concise and meaningful description
- [x] Review the code yourself first, before making the PR.
- [x] Annotate your PR in places that require explanation.
- [x] Think and try to split the PR to smaller PR if it is big.
  • Loading branch information
miladz68 authored Sep 26, 2023
1 parent 9617a20 commit 52c8d17
Show file tree
Hide file tree
Showing 3 changed files with 228 additions and 457 deletions.
161 changes: 59 additions & 102 deletions x/asset/ft/keeper/before_send.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import (
wasmtypes "github.com/CosmWasm/wasmd/x/wasm/types"
sdk "github.com/cosmos/cosmos-sdk/types"
banktypes "github.com/cosmos/cosmos-sdk/x/bank/types"
"github.com/pkg/errors"

"github.com/CoreumFoundation/coreum/v3/x/asset/ft/types"
wibctransfertypes "github.com/CoreumFoundation/coreum/v3/x/wibctransfer/types"
Expand All @@ -18,14 +17,20 @@ import (
func (k Keeper) BeforeSendCoins(ctx sdk.Context, fromAddress, toAddress sdk.AccAddress, coins sdk.Coins) error {
return k.applyFeatures(
ctx,
[]banktypes.Input{{Address: fromAddress.String(), Coins: coins}},
banktypes.Input{Address: fromAddress.String(), Coins: coins},
[]banktypes.Output{{Address: toAddress.String(), Coins: coins}},
)
}

// BeforeInputOutputCoins extends InputOutputCoins method of the bank keeper.
func (k Keeper) BeforeInputOutputCoins(ctx sdk.Context, inputs []banktypes.Input, outputs []banktypes.Output) error {
return k.applyFeatures(ctx, inputs, outputs)
if len(inputs) > 1 {
return banktypes.ErrMultipleSenders
}
if len(inputs) == 0 {
return banktypes.ErrNoInputs
}
return k.applyFeatures(ctx, inputs[0], outputs)
}

type accountOperationMap map[string]sdkmath.Int
Expand All @@ -49,93 +54,73 @@ func (g groupedByDenomAccountOperations) add(address string, coins sdk.Coins) {
}
}

func (k Keeper) applyFeatures(ctx sdk.Context, inputs []banktypes.Input, outputs []banktypes.Output) error {
// TODO: Starting from version v0.47 Cosmos SDK accepts single input only, so we may greatly simplify the logic here.
groupInputs := make(groupedByDenomAccountOperations)
for _, in := range inputs {
groupInputs.add(in.Address, in.Coins)
}

func (k Keeper) applyFeatures(ctx sdk.Context, input banktypes.Input, outputs []banktypes.Output) error {
groupOutputs := make(groupedByDenomAccountOperations)
for _, out := range outputs {
groupOutputs.add(out.Address, out.Coins)
}

return k.applyRules(ctx, groupInputs, groupOutputs)
return k.applyRules(ctx, input, groupOutputs)
}

func (k Keeper) applyRules(ctx sdk.Context, inputs, outputs groupedByDenomAccountOperations) error {
return iterateMapDeterministic(inputs, func(denom string, inOps accountOperationMap) error {
def, err := k.GetDefinition(ctx, denom)
func (k Keeper) applyRules(ctx sdk.Context, input banktypes.Input, outputs groupedByDenomAccountOperations) error {
sender, err := sdk.AccAddressFromBech32(input.Address)
if err != nil {
return sdkerrors.Wrapf(err, "invalid address %s", input.Address)
}

for _, coin := range input.Coins {
def, err := k.GetDefinition(ctx, coin.Denom)
if types.ErrInvalidDenom.Is(err) || types.ErrTokenNotFound.Is(err) {
return nil
continue
}

outOps := outputs[denom]
outOps := outputs[coin.Denom]

burnShares, err := k.CalculateRateShares(ctx, def.BurnRate, def.Issuer, inOps, outOps)
issuer, err := sdk.AccAddressFromBech32(def.Issuer)
if err != nil {
return err
return sdkerrors.Wrapf(err, "invalid address %s", def.Issuer)
}

if err := iterateMapDeterministic(burnShares, func(account string, amount sdkmath.Int) error {
accountAddr, err := sdk.AccAddressFromBech32(account)
if err != nil {
return sdkerrors.Wrapf(err, "invalid address %s", account)
}
return k.burnIfSpendable(ctx, accountAddr, def, amount)
}); err != nil {
burnAmount := k.ApplyRate(ctx, def.BurnRate, issuer, sender, outOps)
if err := k.burnIfSpendable(ctx, sender, def, burnAmount); err != nil {
return err
}

commissionShares, err := k.CalculateRateShares(ctx, def.SendCommissionRate, def.Issuer, inOps, outOps)
if err != nil {
commissionAmount := k.ApplyRate(ctx, def.SendCommissionRate, issuer, sender, outOps)
commissionCoin := sdk.NewCoins(sdk.NewCoin(def.Denom, commissionAmount))
if err := k.bankKeeper.SendCoins(ctx, sender, issuer, commissionCoin); err != nil {
return err
}

issuer, err := sdk.AccAddressFromBech32(def.Issuer)
if err != nil {
return sdkerrors.Wrapf(err, "invalid address %s", def.Issuer)
if err := k.isCoinSpendable(ctx, sender, def, coin.Amount); err != nil {
return err
}

if err := iterateMapDeterministic(commissionShares, func(account string, amount sdkmath.Int) error {
if err := iterateMapDeterministic(outOps, func(account string, amount sdkmath.Int) error {
accountAddr, err := sdk.AccAddressFromBech32(account)
if err != nil {
return sdkerrors.Wrapf(err, "invalid address %s", account)
}
coins := sdk.NewCoins(sdk.NewCoin(def.Denom, amount))
return k.bankKeeper.SendCoins(ctx, accountAddr, issuer, coins)
return k.isCoinReceivable(ctx, accountAddr, def, amount)
}); err != nil {
return err
}

if err := iterateMapDeterministic(inOps, func(account string, amount sdkmath.Int) error {
accountAddr, err := sdk.AccAddressFromBech32(account)
if err != nil {
return sdkerrors.Wrapf(err, "invalid address %s", account)
}
return k.isCoinSpendable(ctx, accountAddr, def, amount)
}); err != nil {
if err != nil {
return err
}
}

return iterateMapDeterministic(outOps, func(account string, amount sdkmath.Int) error {
accountAddr, err := sdk.AccAddressFromBech32(account)
if err != nil {
return sdkerrors.Wrapf(err, "invalid address %s", account)
}
return k.isCoinReceivable(ctx, accountAddr, def, amount)
})
})
return nil
}

// CalculateRateShares calculates how the burn or commission share amount should be split between different parties.
func (k Keeper) CalculateRateShares(ctx sdk.Context, rate sdk.Dec, issuer string, inOps, outOps accountOperationMap) (map[string]sdkmath.Int, error) {
// ApplyRate calculates how the burn or commission amount should be calculated.
func (k Keeper) ApplyRate(ctx sdk.Context, rate sdk.Dec, issuer, sender sdk.AccAddress, outOps accountOperationMap) sdkmath.Int {
// We decided that rates should not be charged on incoming IBC transfers.
// According to our current protocol, it cannot be done because sender pays the rates, meaning that escrow address
// would be charged leading to breaking the IBC mechanics.
if wibctransfertypes.IsPurposeIn(ctx) {
return nil, nil //nolint:nilnil
return sdk.ZeroInt()
}

// Context is marked with ACK purpose in two cases:
Expand All @@ -144,81 +129,53 @@ func (k Keeper) CalculateRateShares(ctx sdk.Context, rate sdk.Dec, issuer string
// This function is called only in the negative case, when the IBC transfer must be rolled back and funds
// must be sent back to the sender. In this case we should not charge the rates.
if wibctransfertypes.IsPurposeAck(ctx) {
return nil, nil //nolint:nilnil
return sdk.ZeroInt()
}

// Same thing as above just in case of IBC timeout.
if wibctransfertypes.IsPurposeTimeout(ctx) {
return nil, nil //nolint:nilnil
return sdk.ZeroInt()
}
// Since burning & send commissions are not applied when sending to/from token issuer or from any smart contract,
// we can't simply apply original burn rate or send commission rates when bank multisend contains issuer or smart contract in
// inputs or issuer in outputs. To recalculate new adjusted amount we split whole "commission" between all non-issuer
// and non-smart-contract senders proportionally to amount they send.
// input or issuer in outputs. To recalculate new adjusted amount we exclude amount sent to issuers.

// Examples
// burn_rate: 10%

// inputs:
// 75, 75
// 25 <-- issuer
// 100

// outputs:
// 50
// 100 <-- issuer
// 25
// 75
// 25 <-- issuer

// In this case commissioned amount is: min(non_issuer_inputs, non_issuer_outputs) = min(75+75, 50+25) = 75
// In this case commissioned amount is: 75
// Expected commission: 75 * 10% = 7.5
// And now we divide it proportionally between all input sender: 7.5 / 150 * 75 = 3.75
// As result each sender is expected to pay 3.75 of commission.
// Note that if we used original rate it would be 75 * 10% = 7.5
// Here is the final formula we use to calculate adjusted burn/commission amount for multisend txs:
// amount * rate * min(non_issuer_inputs_sum, non_issuer_outputs_sum) / non_issuer_inputs_sum
// which is deduces from the sender account.
if rate.IsNil() || !rate.IsPositive() {
return nil, nil //nolint:nilnil
}

taxableInputSum := sdkmath.ZeroInt()
shares := accountOperationMap{}
for account, amount := range inOps {
if account == issuer {
continue
}
acc, err := sdk.AccAddressFromBech32(account)
if err != nil {
return nil, errors.WithStack(err)
}
if len(acc) == wasmtypes.ContractAddrLen && k.wasmKeeper.HasContractInfo(ctx, acc) {
continue
}
taxableInputSum = taxableInputSum.Add(amount)
shares[account] = amount
return sdk.ZeroInt()
}

taxableOutputSum := sdkmath.ZeroInt()
for account, amount := range outOps {
if account != issuer {
taxableOutputSum = taxableOutputSum.Add(amount)
}
}

taxableSum := taxableInputSum
if taxableOutputSum.LT(taxableInputSum) {
taxableSum = taxableOutputSum
if sender.String() == issuer.String() {
return sdk.ZeroInt()
}

if !taxableSum.IsPositive() {
return nil, nil //nolint:nilnil
// we do not apply burn and commission rate if sender is an smart contract address.
if len(sender) == wasmtypes.ContractAddrLen && k.wasmKeeper.HasContractInfo(ctx, sender) {
return sdk.ZeroInt()
}

for account, amount := range shares {
// in order to reduce precision errors, we first multiply all sdkmath.Ints, and then multiply sdk.Decs, and then divide
finalShare := rate.MulInt(taxableSum.Mul(amount)).QuoInt(taxableInputSum).Ceil().RoundInt()
shares[account] = finalShare
taxableOutputSum := sdk.NewInt(0)
issuerStr := issuer.String()
for account, amount := range outOps {
if account == issuerStr {
continue
}
taxableOutputSum = taxableOutputSum.Add(amount)
}

return shares, nil
return rate.MulInt(taxableOutputSum).Ceil().RoundInt()
}

func sortedKeys[V any](m map[string]V) []string {
Expand Down
Loading

0 comments on commit 52c8d17

Please sign in to comment.