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 <[email protected]>

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

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)
+	}
+}

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
 }

interchaintest/module_feeshare_test.go

@@ -24,22 +24,23 @@ 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 {
 		t.Fatal("balance not 0")
 	}
 
 	// 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()
 	})

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,27 +63,50 @@ 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
 }
 
 // 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