Made before_send interceptor deterministic (#590)

* Made before_send interceptor deterministic * pass linter * added test * fixed PR comments
CoreumFoundation · Aug 4, 2023 · dc41e55 · dc41e55
1 parent b630933
commit dc41e55
Show file tree

Hide file tree

Showing 2 changed files with 143 additions and 21 deletions.
diff --git a/integration-tests/modules/wasm_test.go b/integration-tests/modules/wasm_test.go
@@ -7,6 +7,7 @@ import (
 	_ "embed"
 	"encoding/base64"
 	"encoding/json"
+	"math/rand"
 	"testing"
 	"time"
 
@@ -1615,6 +1616,95 @@ func TestWASMNonFungibleTokenInContract(t *testing.T) {
 	})
 }
 
+// TestWASMBankSendContractWithMultipleFundsAttached tests sending multiple ft funds and core token to smart contract.
+// TODO: remove this test after this task is implemented. https://app.clickup.com/t/86857vqra
+func TestWASMBankSendContractWithMultipleFundsAttached(t *testing.T) {
+	t.Parallel()
+
+	ctx, chain := integrationtests.NewCoreumTestingContext(t)
+
+	admin := chain.GenAccount()
+	recipient := chain.GenAccount()
+	nativeDenom := chain.ChainSettings.Denom
+
+	requireT := require.New(t)
+	chain.Faucet.FundAccounts(ctx, t,
+		integrationtests.NewFundedAccount(admin, chain.NewCoin(sdk.NewInt(5000_000_000))),
+	)
+
+	// deployWASMContract and init contract with the initial coins amount
+	initialPayload, err := json.Marshal(struct{}{})
+	requireT.NoError(err)
+	contractAddr, _, err := chain.Wasm.DeployAndInstantiateWASMContract(
+		ctx,
+		chain.TxFactory().
+			WithSimulateAndExecute(true),
+		admin,
+		moduleswasm.BankSendWASM,
+		integrationtests.InstantiateConfig{
+			AccessType: wasmtypes.AccessTypeUnspecified,
+			Payload:    initialPayload,
+			Amount:     chain.NewCoin(sdk.NewInt(10000)),
+			Label:      "bank_send",
+		},
+	)
+	requireT.NoError(err)
+
+	issueMsgs := make([]sdk.Msg, 0)
+	coinsToSend := make([]sdk.Coin, 0)
+	for i := 0; i < 20; i++ {
+		// Issue the new fungible token
+		msgIssue := &assetfttypes.MsgIssue{
+			Issuer:        admin.String(),
+			Symbol:        randStringWithLength(20),
+			Subunit:       randStringWithLength(20),
+			Precision:     6,
+			InitialAmount: sdk.NewInt(10000000000000),
+		}
+		denom := assetfttypes.BuildDenom(msgIssue.Subunit, admin)
+		coinsToSend = append(coinsToSend, sdk.NewInt64Coin(denom, 1_000_000))
+		issueMsgs = append(issueMsgs, msgIssue)
+	}
+	// issue tokens
+	_, err = client.BroadcastTx(
+		ctx,
+		chain.ClientContext.WithFromAddress(admin),
+		chain.TxFactory().WithGas(chain.GasLimitByMsgs(issueMsgs...)),
+		issueMsgs...,
+	)
+	requireT.NoError(err)
+
+	// add additional native coins
+	coinsToSend = append(coinsToSend, chain.NewCoin(sdk.NewInt(10000)))
+
+	// send coin from the contract to test wallet
+	withdrawPayload, err := json.Marshal(map[bankMethod]bankWithdrawRequest{
+		withdraw: {
+			Amount:    "5000",
+			Denom:     nativeDenom,
+			Recipient: recipient.String(),
+		},
+	})
+	requireT.NoError(err)
+
+	executeMsg := &wasmtypes.MsgExecuteContract{
+		Sender:   admin.String(),
+		Contract: contractAddr,
+		Msg:      wasmtypes.RawContractMessage(withdrawPayload),
+		Funds:    sdk.NewCoins(coinsToSend...),
+	}
+	_, err = client.BroadcastTx(
+		ctx,
+		chain.ClientContext.WithFromAddress(admin),
+		chain.TxFactory().WithGasAdjustment(1.5).WithSimulateAndExecute(true),
+		executeMsg,
+	)
+	requireT.NoError(err)
+	waitCtx, cancel := context.WithTimeout(ctx, 10*time.Second)
+	t.Cleanup(cancel)
+	requireT.NoError(client.AwaitNextBlocks(waitCtx, chain.ClientContext, 2))
+}
+
 func methodToEmptyBodyPayload(methodName simpleStateMethod) (json.RawMessage, error) {
 	return json.Marshal(map[simpleStateMethod]struct{}{
 		methodName: {},
@@ -1648,3 +1738,12 @@ func incrementSimpleStateAndVerify(
 
 	return gasUsed
 }
+
+func randStringWithLength(n int) string {
+	var letterRunes = []rune("abcdefghijklmnopqrstuvwxyz")
+	b := make([]rune, n)
+	for i := range b {
+		b[i] = letterRunes[rand.Intn(len(letterRunes))]
+	}
+	return string(b)
+}
diff --git a/x/asset/ft/keeper/before_send.go b/x/asset/ft/keeper/before_send.go
@@ -1,6 +1,8 @@
 package keeper
 
 import (
+	"sort"
+
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	banktypes "github.com/cosmos/cosmos-sdk/x/bank/types"
 
@@ -58,7 +60,7 @@ func (k Keeper) applyFeatures(ctx sdk.Context, inputs []banktypes.Input, outputs
 }
 
 func (k Keeper) applyRules(ctx sdk.Context, inputs, outputs groupedByDenomAccountOperations) error {
-	for denom, inOps := range inputs {
+	return iterateMapDeterministic(inputs, func(denom string, inOps accountOperationMap) error {
 		def, err := k.GetDefinition(ctx, denom)
 		if types.ErrInvalidDenom.Is(err) || types.ErrTokenNotFound.Is(err) {
 			return nil
@@ -67,35 +69,33 @@ func (k Keeper) applyRules(ctx sdk.Context, inputs, outputs groupedByDenomAccoun
 		outOps := outputs[denom]
 
 		burnShares := k.CalculateRateShares(ctx, def.BurnRate, def.Issuer, inOps, outOps)
-		for account, amount := range burnShares {
-			if err := k.burnIfSpendable(ctx, sdk.MustAccAddressFromBech32(account), def, amount); err != nil {
-				return err
-			}
+
+		if err := iterateMapDeterministic(burnShares, func(account string, amount sdk.Int) error {
+			return k.burnIfSpendable(ctx, sdk.MustAccAddressFromBech32(account), def, amount)
+		}); err != nil {
+			return err
 		}
 
 		commissionShares := k.CalculateRateShares(ctx, def.SendCommissionRate, def.Issuer, inOps, outOps)
 		issuer := sdk.MustAccAddressFromBech32(def.Issuer)
-		for account, amount := range commissionShares {
-			coins := sdk.NewCoins(sdk.NewCoin(def.Denom, amount))
-			if err := k.bankKeeper.SendCoins(ctx, sdk.MustAccAddressFromBech32(account), issuer, coins); err != nil {
-				return err
-			}
-		}
 
-		for account, amount := range inOps {
-			if err := k.isCoinSpendable(ctx, sdk.MustAccAddressFromBech32(account), def, amount); err != nil {
-				return err
-			}
+		if err := iterateMapDeterministic(commissionShares, func(account string, amount sdk.Int) error {
+			coins := sdk.NewCoins(sdk.NewCoin(def.Denom, amount))
+			return k.bankKeeper.SendCoins(ctx, sdk.MustAccAddressFromBech32(account), issuer, coins)
+		}); err != nil {
+			return err
 		}
 
-		for account, amount := range outOps {
-			if err := k.isCoinReceivable(ctx, sdk.MustAccAddressFromBech32(account), def, amount); err != nil {
-				return err
-			}
+		if err := iterateMapDeterministic(inOps, func(account string, amount sdk.Int) error {
+			return k.isCoinSpendable(ctx, sdk.MustAccAddressFromBech32(account), def, amount)
+		}); err != nil {
+			return err
 		}
-	}
 
-	return nil
+		return iterateMapDeterministic(outOps, func(account string, amount sdk.Int) error {
+			return k.isCoinReceivable(ctx, sdk.MustAccAddressFromBech32(account), def, amount)
+		})
+	})
 }
 
 func nonIssuerSum(ops accountOperationMap, issuer string) sdk.Int {
@@ -182,3 +182,26 @@ func (k Keeper) CalculateRateShares(ctx sdk.Context, rate sdk.Dec, issuer string
 
 	return shares
 }
+
+func sortedKeys[V any](m map[string]V) []string {
+	keys := make([]string, len(m))
+	i := 0
+	for k := range m {
+		keys[i] = k
+		i++
+	}
+	sort.Slice(keys, func(i, j int) bool { return keys[i] < keys[j] })
+	return keys
+}
+
+func iterateMapDeterministic[V any](m map[string]V, fn func(key string, value V) error) error {
+	keys := sortedKeys(m)
+	for _, key := range keys {
+		v := m[key]
+		if err := fn(key, v); err != nil {
+			return err
+		}
+	}
+
+	return nil
+}