From a56c4421081db13d06e12d3a1ba466ee7d8d5896 Mon Sep 17 00:00:00 2001 From: Reece Williams <31943163+Reecepbcups@users.noreply.github.com> Date: Tue, 21 Nov 2023 21:52:25 -0600 Subject: [PATCH] FeeShare: Authz payout support (#845) * use BinaryCodec for Unpacking Authz msgs * Recursively Search Authz Msgs * Add Ante Unit Tests * Lint * Add Interchain Tests for Authz * Remove Debug Statement * Fix Type Issues * Fix Math Int Conversion * mv -> `ExecuteAuthzGrantMsg` & rm unused args --------- Co-authored-by: Joel Smith --- go.mod | 2 +- interchaintest/helpers/authz.go | 100 ++++++++++++++++ interchaintest/helpers/cosmwasm.go | 5 - interchaintest/module_feeshare_test.go | 23 +++- x/feeshare/ante/ante.go | 78 ++++++------ x/feeshare/ante/ante_test.go | 160 ++++++++++++++++++++++++- 6 files changed, 316 insertions(+), 52 deletions(-) create mode 100644 interchaintest/helpers/authz.go diff --git a/go.mod b/go.mod index 56f77f883..7c7e913fd 100644 --- a/go.mod +++ b/go.mod @@ -180,7 +180,7 @@ require ( google.golang.org/appengine v1.6.7 // indirect google.golang.org/genproto v0.0.0-20230711160842-782d3b101e98 // indirect google.golang.org/genproto/googleapis/rpc v0.0.0-20230711160842-782d3b101e98 // indirect - google.golang.org/protobuf v1.31.0 // indirect + google.golang.org/protobuf v1.31.0 gopkg.in/ini.v1 v1.67.0 // indirect gopkg.in/yaml.v3 v3.0.1 // indirect nhooyr.io/websocket v1.8.7 // indirect diff --git a/interchaintest/helpers/authz.go b/interchaintest/helpers/authz.go new file mode 100644 index 000000000..65359547c --- /dev/null +++ b/interchaintest/helpers/authz.go @@ -0,0 +1,100 @@ +package helpers + +import ( + "context" + "strings" + "testing" + + "github.com/cosmos/cosmos-sdk/crypto/keyring" + "github.com/strangelove-ventures/interchaintest/v7/chain/cosmos" + "github.com/strangelove-ventures/interchaintest/v7/ibc" + "github.com/strangelove-ventures/interchaintest/v7/testutil" + "github.com/stretchr/testify/require" +) + +func ExecuteAuthzGrantMsg(t *testing.T, ctx context.Context, chain *cosmos.CosmosChain, granter ibc.Wallet, grantee ibc.Wallet, msgType string) { + if !strings.HasPrefix(msgType, "/") { + msgType = "/" + msgType + } + + cmd := []string{ + "junod", "tx", "authz", "grant", grantee.FormattedAddress(), "generic", + "--msg-type", msgType, + "--node", chain.GetRPCAddress(), + "--home", chain.HomeDir(), + "--chain-id", chain.Config().ChainID, + "--from", granter.KeyName(), + "--gas", "500000", + "--keyring-dir", chain.HomeDir(), + "--keyring-backend", keyring.BackendTest, + "-y", + } + + stdout, _, err := chain.Exec(ctx, cmd, nil) + require.NoError(t, err) + + debugOutput(t, string(stdout)) + + if err := testutil.WaitForBlocks(ctx, 2, chain); err != nil { + t.Fatal(err) + } +} + +func ExecuteAuthzExecMsgWithFee(t *testing.T, ctx context.Context, chain *cosmos.CosmosChain, grantee ibc.Wallet, contractAddr, amount, feeCoin, message string) { + // Get the node to execute the command & write output to file + node := chain.Nodes()[0] + filePath := "authz.json" + generateMsg := []string{ + "junod", "tx", "wasm", "execute", contractAddr, message, + "--home", chain.HomeDir(), + "--chain-id", chain.Config().ChainID, + "--from", grantee.KeyName(), + "--gas", "500000", + "--fees", feeCoin, + "--keyring-dir", chain.HomeDir(), + "--keyring-backend", keyring.BackendTest, + "--generate-only", + } + + // Generate msg output + res, resErr, err := node.Exec(ctx, generateMsg, nil) + if resErr != nil { + t.Fatal(resErr) + } + if err != nil { + t.Fatal(err) + } + + // Write output to file + err = node.WriteFile(ctx, res, filePath) + if err != nil { + t.Fatal(err) + } + + // Execute the command + cmd := []string{ + "junod", "tx", "authz", "exec", node.HomeDir() + "/" + filePath, + "--node", chain.GetRPCAddress(), + "--home", chain.HomeDir(), + "--chain-id", chain.Config().ChainID, + "--from", grantee.KeyName(), + "--gas", "500000", + "--fees", feeCoin, + "--keyring-dir", chain.HomeDir(), + "--keyring-backend", keyring.BackendTest, + "-y", + } + + if amount != "" { + cmd = append(cmd, "--amount", amount) + } + + stdout, _, err := chain.Exec(ctx, cmd, nil) + require.NoError(t, err) + + debugOutput(t, string(stdout)) + + if err := testutil.WaitForBlocks(ctx, 2, chain); err != nil { + t.Fatal(err) + } +} diff --git a/interchaintest/helpers/cosmwasm.go b/interchaintest/helpers/cosmwasm.go index 26e0bfd3d..1df24584f 100644 --- a/interchaintest/helpers/cosmwasm.go +++ b/interchaintest/helpers/cosmwasm.go @@ -122,10 +122,5 @@ func ExecuteMsgWithFeeReturn(t *testing.T, ctx context.Context, chain *cosmos.Co // convert stdout into a TxResponse txRes, err := chain.GetTransaction(txHash) - - if err := testutil.WaitForBlocks(ctx, 2, chain); err != nil { - t.Fatal(err) - } - return txRes, err } diff --git a/interchaintest/module_feeshare_test.go b/interchaintest/module_feeshare_test.go index 924286e8d..2bb43ace6 100644 --- a/interchaintest/module_feeshare_test.go +++ b/interchaintest/module_feeshare_test.go @@ -24,14 +24,15 @@ func TestJunoFeeShare(t *testing.T) { // Users users := interchaintest.GetAndFundTestUsers(t, ctx, "default", int64(10_000_000), juno, juno) - user := users[0] + granter := users[0] + grantee := users[1] feeRcvAddr := "juno1v75wlkccpv7le3560zw32v2zjes5n0e7csr4qh" // Upload & init contract payment to another address - _, contractAddr := helpers.SetupContract(t, ctx, juno, user.KeyName(), "contracts/cw_template.wasm", `{"count":0}`) + _, contractAddr := helpers.SetupContract(t, ctx, juno, granter.KeyName(), "contracts/cw_template.wasm", `{"count":0}`) // register contract to a random address (since we are the creator, though not the admin) - helpers.RegisterFeeShare(t, ctx, juno, user, contractAddr, feeRcvAddr) + helpers.RegisterFeeShare(t, ctx, juno, granter, contractAddr, feeRcvAddr) if balance, err := juno.GetBalance(ctx, feeRcvAddr, nativeDenom); err != nil { t.Fatal(err) } else if balance.Int64() != 0 { @@ -39,7 +40,7 @@ func TestJunoFeeShare(t *testing.T) { } // execute with a 10000 fee (so 5000 denom should be in the contract now with 50% feeshare default) - helpers.ExecuteMsgWithFee(t, ctx, juno, user, contractAddr, "", "10000"+nativeDenom, `{"increment":{}}`) + helpers.ExecuteMsgWithFee(t, ctx, juno, granter, contractAddr, "", "10000"+nativeDenom, `{"increment":{}}`) // check balance of nativeDenom now if balance, err := juno.GetBalance(ctx, feeRcvAddr, nativeDenom); err != nil { @@ -48,6 +49,20 @@ func TestJunoFeeShare(t *testing.T) { t.Fatal("balance not 5,000. it is ", balance, nativeDenom) } + // Test authz message execution: + // Grant contract execute permission to grantee + helpers.ExecuteAuthzGrantMsg(t, ctx, juno, granter, grantee, "/cosmos.authz.v1beta1.MsgExec") + + // Execute authz msg as grantee + helpers.ExecuteAuthzExecMsgWithFee(t, ctx, juno, grantee, contractAddr, "", "10000"+nativeDenom, `{"increment":{}}`) + + // check balance of nativeDenom now + if balance, err := juno.GetBalance(ctx, feeRcvAddr, nativeDenom); err != nil { + t.Fatal(err) + } else if balance.Int64() != 10000 { + t.Fatal("balance not 10,000. it is ", balance, nativeDenom) + } + t.Cleanup(func() { _ = ic.Close() }) diff --git a/x/feeshare/ante/ante.go b/x/feeshare/ante/ante.go index 88d5475ed..2a942dd1c 100644 --- a/x/feeshare/ante/ante.go +++ b/x/feeshare/ante/ante.go @@ -35,7 +35,7 @@ func (fsd FeeSharePayoutDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simula return ctx, errorsmod.Wrap(sdkerrors.ErrTxDecode, "Tx must be a FeeTx") } - err = FeeSharePayout(ctx, fsd.bankKeeper, feeTx.GetFee(), fsd.feesharekeeper, tx.GetMsgs()) + err = fsd.FeeSharePayout(ctx, fsd.bankKeeper, feeTx.GetFee(), fsd.feesharekeeper, tx.GetMsgs()) if err != nil { return ctx, errorsmod.Wrapf(sdkerrors.ErrInsufficientFunds, err.Error()) } @@ -63,19 +63,42 @@ type FeeSharePayoutEventOutput struct { FeesPaid sdk.Coins `json:"fees_paid"` } -func addNewFeeSharePayoutsForMsg(ctx sdk.Context, fsk FeeShareKeeper, toPay *[]sdk.AccAddress, m sdk.Msg) error { - if msg, ok := m.(*wasmtypes.MsgExecuteContract); ok { - contractAddr, err := sdk.AccAddressFromBech32(msg.Contract) - if err != nil { - return err +// Loop through all messages and add the withdraw address to the list of addresses to pay +// if the contract opted-in to fee sharing +func addNewFeeSharePayoutsForMsgs(ctx sdk.Context, fsk FeeShareKeeper, toPay *[]sdk.AccAddress, msgs []sdk.Msg) error { + for _, msg := range msgs { + + // Check if an authz message, loop through all inner messages, and recursively call this function + if authzMsg, ok := msg.(*authz.MsgExec); ok { + + innerMsgs, err := authzMsg.GetMessages() + if err != nil { + return errorsmod.Wrapf(sdkerrors.ErrUnauthorized, "cannot unmarshal authz exec msgs") + } + + // Recursively call this function with the inner messages + err = addNewFeeSharePayoutsForMsgs(ctx, fsk, toPay, innerMsgs) + if err != nil { + return err + } } - shareData, _ := fsk.GetFeeShare(ctx, contractAddr) + // If an execute contract message, check if the contract opted-in to fee sharing, + // and if so, add the withdraw address to the list of addresses to pay + if execContractMsg, ok := msg.(*wasmtypes.MsgExecuteContract); ok { + contractAddr, err := sdk.AccAddressFromBech32(execContractMsg.Contract) + if err != nil { + return err + } + + shareData, _ := fsk.GetFeeShare(ctx, contractAddr) - withdrawAddr := shareData.GetWithdrawerAddr() - if withdrawAddr != nil && !withdrawAddr.Empty() { - *toPay = append(*toPay, withdrawAddr) + withdrawAddr := shareData.GetWithdrawerAddr() + if withdrawAddr != nil && !withdrawAddr.Empty() { + *toPay = append(*toPay, withdrawAddr) + } } + } return nil @@ -83,7 +106,7 @@ func addNewFeeSharePayoutsForMsg(ctx sdk.Context, fsk FeeShareKeeper, toPay *[]s // FeeSharePayout takes the total fees and redistributes 50% (or param set) to the contract developers // provided they opted-in to payments. -func FeeSharePayout(ctx sdk.Context, bankKeeper BankKeeper, totalFees sdk.Coins, fsk FeeShareKeeper, msgs []sdk.Msg) error { +func (fsd FeeSharePayoutDecorator) FeeSharePayout(ctx sdk.Context, bankKeeper BankKeeper, totalFees sdk.Coins, fsk FeeShareKeeper, msgs []sdk.Msg) error { params := fsk.GetParams(ctx) if !params.EnableFeeShare { return nil @@ -92,35 +115,10 @@ func FeeSharePayout(ctx sdk.Context, bankKeeper BankKeeper, totalFees sdk.Coins, // Get valid withdraw addresses from contracts toPay := make([]sdk.AccAddress, 0) - // validAuthz checks if the msg is an authz exec msg and if so, call the validExecuteMsg for each - // inner msg. If it is a CosmWasm execute message, that logic runs for nested functions. - validAuthz := func(execMsg *authz.MsgExec) error { - for _, v := range execMsg.Msgs { - var innerMsg sdk.Msg - if err := json.Unmarshal(v.Value, &innerMsg); err != nil { - return errorsmod.Wrapf(sdkerrors.ErrUnauthorized, "cannot unmarshal authz exec msgs") - } - - err := addNewFeeSharePayoutsForMsg(ctx, fsk, &toPay, innerMsg) - if err != nil { - return err - } - } - - return nil - } - - for _, m := range msgs { - if msg, ok := m.(*authz.MsgExec); ok { - if err := validAuthz(msg); err != nil { - return err - } - continue - } - - if err := addNewFeeSharePayoutsForMsg(ctx, fsk, &toPay, m); err != nil { - return err - } + // Add fee share payouts for each msg + err := addNewFeeSharePayoutsForMsgs(ctx, fsk, &toPay, msgs) + if err != nil { + return err } // Do nothing if no one needs payment diff --git a/x/feeshare/ante/ante_test.go b/x/feeshare/ante/ante_test.go index 0705bfe71..eb08bbb91 100644 --- a/x/feeshare/ante/ante_test.go +++ b/x/feeshare/ante/ante_test.go @@ -2,23 +2,123 @@ package ante_test import ( "testing" + "time" + wasmtypes "github.com/CosmWasm/wasmd/x/wasm/types" "github.com/stretchr/testify/suite" + protov2 "google.golang.org/protobuf/proto" + tmproto "github.com/cometbft/cometbft/proto/tendermint/types" + + "github.com/cosmos/cosmos-sdk/testutil/testdata" sdk "github.com/cosmos/cosmos-sdk/types" + authtypes "github.com/cosmos/cosmos-sdk/x/auth/types" + "github.com/cosmos/cosmos-sdk/x/authz" + bankkeeper "github.com/cosmos/cosmos-sdk/x/bank/keeper" + minttypes "github.com/cosmos/cosmos-sdk/x/mint/types" + "github.com/CosmosContracts/juno/v18/app" ante "github.com/CosmosContracts/juno/v18/x/feeshare/ante" + feesharekeeper "github.com/CosmosContracts/juno/v18/x/feeshare/keeper" + feesharetypes "github.com/CosmosContracts/juno/v18/x/feeshare/types" +) + +// Define an empty ante handle +var ( + EmptyAnte = func(ctx sdk.Context, tx sdk.Tx, simulate bool) (sdk.Context, error) { + return ctx, nil + } ) type AnteTestSuite struct { suite.Suite + + ctx sdk.Context + app *app.App + bankKeeper bankkeeper.Keeper + feeshareKeeper feesharekeeper.Keeper +} + +func (s *AnteTestSuite) SetupTest() { + isCheckTx := false + s.app = app.Setup(s.T()) + + s.ctx = s.app.BaseApp.NewContext(isCheckTx, tmproto.Header{ + ChainID: "testing", + Height: 10, + Time: time.Now().UTC(), + }) + + s.bankKeeper = s.app.AppKeepers.BankKeeper + s.feeshareKeeper = s.app.AppKeepers.FeeShareKeeper } func TestAnteSuite(t *testing.T) { suite.Run(t, new(AnteTestSuite)) } -func (suite *AnteTestSuite) TestFeeLogic() { +func (s *AnteTestSuite) TestAnteHandle() { + // Mint coins to FeeCollector to cover fees + err := s.FundModule(s.ctx, authtypes.FeeCollectorName, sdk.NewCoins(sdk.NewCoin("ujuno", sdk.NewInt(1_000_000)))) + s.Require().NoError(err) + + // Create & fund deployer + _, _, deployer := testdata.KeyTestPubAddr() + err = s.FundAccount(s.ctx, deployer, sdk.NewCoins(sdk.NewCoin("ujuno", sdk.NewInt(100_000_000)))) + s.Require().NoError(err) + + // Create funds receiver account + _, _, receiver := testdata.KeyTestPubAddr() + + // Address used to mock a contract + _, _, contractAddr := testdata.KeyTestPubAddr() + + // Register contract with Fee Share + registerMsg := feesharetypes.FeeShare{ + ContractAddress: contractAddr.String(), + DeployerAddress: deployer.String(), + WithdrawerAddress: receiver.String(), + } + s.feeshareKeeper.SetFeeShare(s.ctx, registerMsg) + + // Create execute msg + executeMsg := &wasmtypes.MsgExecuteContract{ + Sender: deployer.String(), + Contract: contractAddr.String(), + Msg: []byte("{}"), + Funds: sdk.NewCoins(sdk.NewCoin("ujuno", sdk.NewInt(0))), + } + tx := NewMockTx(deployer, executeMsg) + + // Run normal msg through ante handle + ante := ante.NewFeeSharePayoutDecorator(s.bankKeeper, s.feeshareKeeper) + _, err = ante.AnteHandle(s.ctx, tx, false, EmptyAnte) + s.Require().NoError(err) + + // Check that the receiver account was paid + receiverBal := s.bankKeeper.GetBalance(s.ctx, receiver, "ujuno") + s.Require().Equal(sdk.NewInt(250).Int64(), receiverBal.Amount.Int64()) + + // Create & handle authz msg + authzMsg := authz.NewMsgExec(deployer, []sdk.Msg{executeMsg}) + _, err = ante.AnteHandle(s.ctx, NewMockTx(deployer, &authzMsg), false, EmptyAnte) + s.Require().NoError(err) + + // Check that the receiver account was paid + receiverBal = s.bankKeeper.GetBalance(s.ctx, receiver, "ujuno") + s.Require().Equal(sdk.NewInt(500).Int64(), receiverBal.Amount.Int64()) + + // Create & handle authz msg with nested authz msg + nestedAuthzMsg := authz.NewMsgExec(deployer, []sdk.Msg{&authzMsg}) + _, err = ante.AnteHandle(s.ctx, NewMockTx(deployer, &nestedAuthzMsg), false, EmptyAnte) + s.Require().NoError(err) + + // Check that the receiver account was paid + receiverBal = s.bankKeeper.GetBalance(s.ctx, receiver, "ujuno") + s.Require().Equal(sdk.NewInt(750).Int64(), receiverBal.Amount.Int64()) +} + +func (s *AnteTestSuite) TestFeeLogic() { // We expect all to pass feeCoins := sdk.NewCoins(sdk.NewCoin("ujuno", sdk.NewInt(500)), sdk.NewCoin("utoken", sdk.NewInt(250))) @@ -107,9 +207,65 @@ func (suite *AnteTestSuite) TestFeeLogic() { for _, coin := range coins { for _, expectedCoin := range tc.expectedFeePayment { if coin.Denom == expectedCoin.Denom { - suite.Require().Equal(expectedCoin.Amount.Int64(), coin.Amount.Int64(), tc.name) + s.Require().Equal(expectedCoin.Amount.Int64(), coin.Amount.Int64(), tc.name) } } } } } + +func (s *AnteTestSuite) FundAccount(ctx sdk.Context, addr sdk.AccAddress, amounts sdk.Coins) error { + if err := s.bankKeeper.MintCoins(ctx, minttypes.ModuleName, amounts); err != nil { + return err + } + + return s.bankKeeper.SendCoinsFromModuleToAccount(ctx, minttypes.ModuleName, addr, amounts) +} + +func (s *AnteTestSuite) FundModule(ctx sdk.Context, moduleName string, amounts sdk.Coins) error { + if err := s.bankKeeper.MintCoins(ctx, minttypes.ModuleName, amounts); err != nil { + return err + } + + return s.bankKeeper.SendCoinsFromModuleToModule(ctx, minttypes.ModuleName, moduleName, amounts) +} + +type MockTx struct { + feePayer sdk.AccAddress + msgs []sdk.Msg +} + +func NewMockTx(feePayer sdk.AccAddress, msgs ...sdk.Msg) MockTx { + return MockTx{ + feePayer: feePayer, + msgs: msgs, + } +} + +func (tx MockTx) GetGas() uint64 { + return 200000 +} + +func (tx MockTx) GetFee() sdk.Coins { + return sdk.NewCoins(sdk.NewCoin("ujuno", sdk.NewInt(500))) +} + +func (tx MockTx) FeePayer() sdk.AccAddress { + return tx.feePayer +} + +func (tx MockTx) FeeGranter() sdk.AccAddress { + return nil +} + +func (tx MockTx) GetMsgs() []sdk.Msg { + return tx.msgs +} + +func (tx MockTx) GetMsgsV2() ([]protov2.Message, error) { + return nil, nil +} + +func (tx MockTx) ValidateBasic() error { + return nil +}