fix(x/rewards): create block rewards for flat fees

CHANGELOG.md

@@ -28,6 +28,12 @@ Contains bug fixes.
 Contains all the PRs that improved the code without changing the behaviours. 
 -->
 
+## [Unreleased]
+
+### Fixed
+
+- [#338](https://github.com/archway-network/archway/pull/338) - fixed issue where contract premium was not completly being sent to the rewards address
+
 ## [v0.3.1]
 
 ### Fixed

app/ante.go

@@ -76,7 +76,7 @@ func NewAnteHandler(options HandlerOptions) (sdk.AnteHandler, error) {
 		// Custom Archway interceptor to track new transactions
 		trackingAnte.NewTxGasTrackingDecorator(options.TrackingKeeper),
 		// Custom Archway fee deduction, which splits fees between x/rewards and x/auth fee collector
-		rewardsAnte.NewDeductFeeDecorator(options.AccountKeeper, options.BankKeeper, options.FeegrantKeeper, options.RewardsKeeper),
+		rewardsAnte.NewDeductFeeDecorator(options.Codec, options.AccountKeeper, options.BankKeeper, options.FeegrantKeeper, options.RewardsKeeper),
 		// SetPubKeyDecorator must be called before all signature verification decorators
 		ante.NewSetPubKeyDecorator(options.AccountKeeper),
 		ante.NewValidateSigCountDecorator(options.AccountKeeper),

e2e/rewards_test.go

@@ -414,9 +414,10 @@ func (s *E2ETestSuite) TestTXFailsAfterAnteHandler() {
 		RewardsAddress:  contractAddr.String(),
 	})
 
-	err := chain.GetApp().RewardsKeeper.SetFlatFee(chain.GetContext(), senderAcc.Address, rewardsTypes.FlatFee{
+	flatFees := sdk.NewInt64Coin("stake", 1000)
+	err := rewardsKeeper.SetFlatFee(chain.GetContext(), senderAcc.Address, rewardsTypes.FlatFee{
 		ContractAddress: contractAddr.String(),
-		FlatFee:         sdk.NewInt64Coin("stake", 1000),
+		FlatFee:         flatFees,
 	})
 	require.NoError(s.T(), err)
 
@@ -446,7 +447,6 @@ func (s *E2ETestSuite) TestTXFailsAfterAnteHandler() {
 
 		return
 	}
-	rk := chain.GetApp().RewardsKeeper
 
 	// send a message that passes the ante handler but not the wasm execution step
 	sendMsg(&wasmdTypes.MsgExecuteContract{
@@ -458,9 +458,154 @@ func (s *E2ETestSuite) TestTXFailsAfterAnteHandler() {
 
 	chain.NextBlock(1 * time.Second)
 
-	// no rewards because the TX failed.
-	rewards := rk.GetState().RewardsRecord(chain.GetContext()).GetRewardsRecordByRewardsAddress(contractAddr)
-	require.Empty(s.T(), rewards)
+	// only rewards record for contract premiums. no rewards record for feerebaes/inflation because because the TX failed.
+	rewards := rewardsKeeper.GetState().RewardsRecord(chain.GetContext()).GetRewardsRecordByRewardsAddress(contractAddr)
+	require.Len(s.T(), rewards, 1)
+	require.Equal(s.T(), flatFees, rewards[0].Rewards[0])
+}
+
+// TestRewardsFlatFees tests that a contract which has flatfees set, on a successful execution against
+// the contract the relevant rewards records have been created
+func (s *E2ETestSuite) TestRewardsFlatFees() {
+	// Create a custom chain with "close to mainnet" params
+	chain := e2eTesting.NewTestChain(s.T(), 1,
+		// Set 1B total supply (10^9 * 10^6)
+		e2eTesting.WithGenAccounts(2),
+		e2eTesting.WithGenDefaultCoinBalance("1000000000000000"),
+		// Set bonded ratio to 30%
+		e2eTesting.WithBondAmount("300000000000000"),
+		// Override the default Tx fee
+		e2eTesting.WithDefaultFeeAmount("10000000"),
+		// Set block gas limit (Archway mainnet param)
+		e2eTesting.WithBlockGasLimit(100_000_000),
+		// x/rewards distribution params
+		e2eTesting.WithTxFeeRebatesRewardsRatio(sdk.NewDecWithPrec(5, 1)),
+		e2eTesting.WithInflationRewardsRatio(sdk.NewDecWithPrec(2, 1)),
+		// Set constant inflation rate
+		e2eTesting.WithMintParams(
+			sdk.NewDecWithPrec(10, 2), // 10%
+			sdk.NewDecWithPrec(10, 2), // 10%
+			uint64(60*60*8766/1),      // 1 seconds block time
+		),
+	)
+	rewardsKeeper := chain.GetApp().RewardsKeeper
+
+	// Upload a new contract and set its address as the rewardsAddress
+	senderAcc := chain.GetAccount(0)
+	contractAddr := s.VoterUploadAndInstantiate(chain, senderAcc)
+
+	// Setting contract metadata with rewards address to be itself
+	chain.SetContractMetadata(senderAcc, contractAddr, rewardsTypes.ContractMetadata{
+		ContractAddress: contractAddr.String(),
+		OwnerAddress:    senderAcc.Address.String(),
+		RewardsAddress:  contractAddr.String(),
+	})
+
+	// Setting contract flatfee to be 1000 stake
+	flatFees := sdk.NewInt64Coin("stake", 1000)
+	err := rewardsKeeper.SetFlatFee(chain.GetContext(), senderAcc.Address, rewardsTypes.FlatFee{
+		ContractAddress: contractAddr.String(),
+		FlatFee:         flatFees,
+	})
+	require.NoError(s.T(), err)
+
+	// contract execution to trigger rewards distribution
+	req := voterTypes.MsgExecute{
+		NewVoting: &voterTypes.NewVotingRequest{
+			Name:        "Test",
+			VoteOptions: []string{"Yes", "No"},
+			Duration:    uint64(time.Minute),
+		},
+	}
+	reqBz, err := req.MarshalJSON()
+	s.Require().NoError(err)
+
+	msg := wasmdTypes.MsgExecuteContract{
+		Sender:   senderAcc.Address.String(),
+		Contract: contractAddr.String(),
+		Msg:      reqBz,
+		Funds:    sdk.NewCoins(sdk.NewInt64Coin(sdk.DefaultBondDenom, DefNewVotingCostAmt)),
+	}
+	_, _, _, err = chain.SendMsgs(senderAcc, true, []sdk.Msg{&msg})
+	require.NoError(s.T(), err)
+
+	chain.NextBlock(1 * time.Second)
+
+	// should find two rewards records
+	// 1. Flatfee rewards record
+	// 2. InflationaryRewards + FeeRewards rewards record
+	rewards := rewardsKeeper.GetState().RewardsRecord(chain.GetContext()).GetRewardsRecordByRewardsAddress(contractAddr)
+	require.Len(s.T(), rewards, 2)
+	require.Equal(s.T(), flatFees, rewards[0].Rewards[0]) // the first rewards record matches our set flat fees
+	require.Equal(s.T(), sdk.NewInt64Coin("stake", 4999724), rewards[1].Rewards[0])
+
+	// Setting up a second contract which also has flat fees enabled
+	sender2Acc := chain.GetAccount(1)
+	contract2Addr := s.VoterUploadAndInstantiate(chain, sender2Acc)
+	chain.SetContractMetadata(sender2Acc, contract2Addr, rewardsTypes.ContractMetadata{
+		ContractAddress: contract2Addr.String(),
+		OwnerAddress:    sender2Acc.Address.String(),
+		RewardsAddress:  contract2Addr.String(),
+	})
+	flatFees2 := sdk.NewInt64Coin("stake", 20)
+	err = rewardsKeeper.SetFlatFee(chain.GetContext(), sender2Acc.Address, rewardsTypes.FlatFee{
+		ContractAddress: contract2Addr.String(),
+		FlatFee:         flatFees2,
+	})
+	require.NoError(s.T(), err)
+
+	// Lets now do the same operations a bunch of times - and by a bunch of times i mean ten times
+	// this should generate quite a few rewards records - and by quite a few i mean 50 times
+	// each loop the following are executed
+	// 1. execute contract1 and move to next block
+	// 2. execute contract2 and move to next block
+	// 3. execute contract1,contract1(again),contarct2 in a single msg and move to the next block
+	for i := 0; i < 10; i++ {
+		// execute contract1 and move to next block
+		_, _, _, err = chain.SendMsgs(senderAcc, true, []sdk.Msg{&wasmdTypes.MsgExecuteContract{
+			Sender:   senderAcc.Address.String(),
+			Contract: contractAddr.String(),
+			Msg:      reqBz,
+			Funds:    sdk.NewCoins(sdk.NewInt64Coin(sdk.DefaultBondDenom, DefNewVotingCostAmt)),
+		}})
+		require.NoError(s.T(), err)
+		chain.NextBlock(1 * time.Second)
+
+		// execute contract2 and move to next block
+		_, _, _, err = chain.SendMsgs(sender2Acc, true, []sdk.Msg{&wasmdTypes.MsgExecuteContract{
+			Sender:   sender2Acc.Address.String(),
+			Contract: contract2Addr.String(),
+			Msg:      reqBz,
+			Funds:    sdk.NewCoins(sdk.NewInt64Coin(sdk.DefaultBondDenom, DefNewVotingCostAmt)),
+		}})
+		require.NoError(s.T(), err)
+		chain.NextBlock(1 * time.Second)
+
+		// execute contract1,contract1(again),contarct2 in a single msg and move to the next block
+		_, _, _, err = chain.SendMsgs(senderAcc, true, []sdk.Msg{&wasmdTypes.MsgExecuteContract{
+			Sender:   senderAcc.Address.String(),
+			Contract: contractAddr.String(),
+			Msg:      reqBz,
+			Funds:    sdk.NewCoins(sdk.NewInt64Coin(sdk.DefaultBondDenom, DefNewVotingCostAmt)),
+		}, &wasmdTypes.MsgExecuteContract{
+			Sender:   senderAcc.Address.String(),
+			Contract: contractAddr.String(),
+			Msg:      reqBz,
+			Funds:    sdk.NewCoins(sdk.NewInt64Coin(sdk.DefaultBondDenom, DefNewVotingCostAmt)),
+		}, &wasmdTypes.MsgExecuteContract{
+			Sender:   senderAcc.Address.String(),
+			Contract: contract2Addr.String(),
+			Msg:      reqBz,
+			Funds:    sdk.NewCoins(sdk.NewInt64Coin(sdk.DefaultBondDenom, DefNewVotingCostAmt)),
+		}})
+		require.NoError(s.T(), err)
+		chain.NextBlock(1 * time.Second)
+	}
+	rewards = rewardsKeeper.GetState().RewardsRecord(chain.GetContext()).GetRewardsRecordByRewardsAddress(contractAddr)
+	require.Len(s.T(), rewards, 52) // why 52? cuz we already had 2 rewards record. we made 10 loops with 2 txs for this contract. And second txs contains 2 msgs. so 2 + (10 * (2 + 3)) = 52
+
+	rewards = rewardsKeeper.GetState().RewardsRecord(chain.GetContext()).GetRewardsRecordByRewardsAddress(contract2Addr)
+	require.Len(s.T(), rewards, 40) // why 40? cuz we made 10 loops with 2 txs for this contract. and each msg creates two records. so 10 * 2 * 2 = 40
 }
 
 // TestSubMsgRevert tests when a contract calls another contract but the sub message reverts,

x/rewards/ante/ante_utils.go

@@ -0,0 +1,65 @@
+package ante
+
+import (
+	wasmTypes "github.com/CosmWasm/wasmd/x/wasm/types"
+	"github.com/cosmos/cosmos-sdk/codec"
+	sdk "github.com/cosmos/cosmos-sdk/types"
+	sdkErrors "github.com/cosmos/cosmos-sdk/types/errors"
+	"github.com/cosmos/cosmos-sdk/x/authz"
+)
+
+// RewardsKeeperExpected defines the expected interface for the x/rewards keeper.
+type RewardsKeeperExpected interface {
+	// Used in MinFeeDecorator
+	GetMinConsensusFee(ctx sdk.Context) (sdk.DecCoin, bool)
+	GetFlatFee(ctx sdk.Context, contractAddr sdk.AccAddress) (sdk.Coin, bool)
+	CreateFlatFeeRewardsRecords(ctx sdk.Context, contractAddress sdk.AccAddress, flatfee sdk.Coins)
+
+	// Used in DeductFeeDecorator
+	TxFeeRebateRatio(ctx sdk.Context) sdk.Dec
+	TrackFeeRebatesRewards(ctx sdk.Context, rewards sdk.Coins)
+}
+
+type contractFlatFee struct {
+	ContractAddress sdk.AccAddress
+	FlatFees        sdk.Coins
+}
+
+func GetContractFlatFees(ctx sdk.Context, rk RewardsKeeperExpected, codec codec.BinaryCodec, m sdk.Msg) (contractFlatFees []contractFlatFee, hasWasmMsgs bool, err error) {
+	switch msg := m.(type) {
+	case *wasmTypes.MsgMigrateContract:
+		{
+			return nil, true, nil
+		}
+	case *wasmTypes.MsgExecuteContract: // if msg is contract execute, fetch flatfee for msg.Contract address
+		{
+			ca, err := sdk.AccAddressFromBech32(msg.Contract)
+			if err != nil {
+				return nil, true, err
+			}
+			fee, found := rk.GetFlatFee(ctx, ca)
+			if found {
+				contractFlatFees = append(contractFlatFees, contractFlatFee{ContractAddress: ca, FlatFees: sdk.NewCoins(fee)})
+				return contractFlatFees, true, nil
+			}
+			return nil, true, nil
+		}
+	case *authz.MsgExec: // if msg is authz msg, unwrap the msg and check if any are wasmTypes.MsgExecuteContract
+		{
+			for _, v := range msg.Msgs {
+				var wrappedMsg sdk.Msg
+				err := codec.UnpackAny(v, &wrappedMsg)
+				if err != nil {
+					return nil, false, sdkErrors.Wrapf(sdkErrors.ErrUnauthorized, "error decoding authz messages")
+				}
+				cff, hasWasmMsgs, err := GetContractFlatFees(ctx, rk, codec, wrappedMsg)
+				if err != nil {
+					return nil, hasWasmMsgs, err
+				}
+				contractFlatFees = append(contractFlatFees, cff...)
+			}
+			return contractFlatFees, hasWasmMsgs, nil
+		}
+	}
+	return nil, false, nil
+}

x/rewards/ante/fee_deduction.go

@@ -3,39 +3,34 @@ package ante
 import (
 	"fmt"
 
+	"github.com/cosmos/cosmos-sdk/codec"
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	sdkErrors "github.com/cosmos/cosmos-sdk/types/errors"
 	"github.com/cosmos/cosmos-sdk/x/auth/ante"
 	authTypes "github.com/cosmos/cosmos-sdk/x/auth/types"
 
-	wasmdTypes "github.com/CosmWasm/wasmd/x/wasm/types"
-
 	"github.com/archway-network/archway/pkg"
 	rewardsTypes "github.com/archway-network/archway/x/rewards/types"
 )
 
 var _ sdk.AnteDecorator = DeductFeeDecorator{}
 
-// TxFeeRewardsKeeperExpected defines the expected interface for the x/rewards keeper.
-type TxFeeRewardsKeeperExpected interface {
-	TxFeeRebateRatio(ctx sdk.Context) sdk.Dec
-	TrackFeeRebatesRewards(ctx sdk.Context, rewards sdk.Coins)
-}
-
 // DeductFeeDecorator deducts fees from the first signer of the tx.
 // If the first signer does not have the funds to pay for the fees, return with InsufficientFunds error.
 // Call next AnteHandler if fees successfully deducted.
 // CONTRACT: Tx must implement FeeTx interface to use DeductFeeDecorator.
 type DeductFeeDecorator struct {
+	codec          codec.BinaryCodec
 	ak             ante.AccountKeeper
 	bankKeeper     authTypes.BankKeeper
 	feegrantKeeper ante.FeegrantKeeper
-	rewardsKeeper  TxFeeRewardsKeeperExpected
+	rewardsKeeper  RewardsKeeperExpected
 }
 
 // NewDeductFeeDecorator returns a new DeductFeeDecorator instance.
-func NewDeductFeeDecorator(ak ante.AccountKeeper, bk authTypes.BankKeeper, fk ante.FeegrantKeeper, rk TxFeeRewardsKeeperExpected) DeductFeeDecorator {
+func NewDeductFeeDecorator(codec codec.BinaryCodec, ak ante.AccountKeeper, bk authTypes.BankKeeper, fk ante.FeegrantKeeper, rk RewardsKeeperExpected) DeductFeeDecorator {
 	return DeductFeeDecorator{
+		codec:          codec,
 		ak:             ak,
 		bankKeeper:     bk,
 		feegrantKeeper: fk,
@@ -103,17 +98,19 @@ func (dfd DeductFeeDecorator) deductFees(ctx sdk.Context, tx sdk.Tx, acc authTyp
 		return sdkErrors.Wrapf(sdkErrors.ErrInsufficientFee, "invalid fee amount: %s", fees)
 	}
 
+	var flatFees sdk.Coins
 	// Check if transaction has wasmd operations
 	hasWasmMsgs := false
-	for _, msg := range tx.GetMsgs() {
-		// We can use switch here, but breaking the for loop from switch is less readable
-		if _, ok := msg.(*wasmdTypes.MsgExecuteContract); ok {
-			hasWasmMsgs = true
-			break
+	for _, m := range tx.GetMsgs() {
+		contractFlatFees, hwm, err := GetContractFlatFees(ctx, dfd.rewardsKeeper, dfd.codec, m)
+		if err != nil {
+			return err
+		}
+		if !hasWasmMsgs {
+			hasWasmMsgs = hwm //set hasWasmMsgs as true if its false. if its true, do nothing
 		}
-		if _, ok := msg.(*wasmdTypes.MsgMigrateContract); ok {
-			hasWasmMsgs = true
-			break
+		for _, cff := range contractFlatFees {
+			flatFees = flatFees.Add(cff.FlatFees...)
 		}
 	}
 
@@ -126,6 +123,13 @@ func (dfd DeductFeeDecorator) deductFees(ctx sdk.Context, tx sdk.Tx, acc authTyp
 		return nil
 	}
 
+	if !flatFees.Empty() {
+		if err := dfd.bankKeeper.SendCoinsFromAccountToModule(ctx, acc.GetAddress(), rewardsTypes.ContractRewardCollector, flatFees); err != nil {
+			return sdkErrors.Wrapf(sdkErrors.ErrInsufficientFunds, err.Error())
+		}
+		fees = fees.Sub(flatFees) // reduce flatfees from the sent fees amount
+	}
+
 	// Split the fees between the fee collector account and the rewards collector account
 	rewardsFees, authFees := pkg.SplitCoins(fees, rebateRatio)
 

x/rewards/ante/fee_deduction_test.go

@@ -30,7 +30,9 @@ func TestRewardsFeeDeductionAnteHandler(t *testing.T) {
 		rewardsBalanceDiffExpected      string // expected x/rewards module balance diff [sdk.Coins]
 	}
 
-	mockWasmExecuteMsg := &wasmdTypes.MsgExecuteContract{}
+	mockWasmExecuteMsg := &wasmdTypes.MsgExecuteContract{
+		Contract: e2eTesting.GenContractAddresses(1)[0].String(),
+	}
 
 	newStakeCoin := func(amt uint64) sdk.Coin {
 		return sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewIntFromUint64(amt))
@@ -132,7 +134,7 @@ func TestRewardsFeeDeductionAnteHandler(t *testing.T) {
 			)
 
 			// Call the deduction Ante handler manually
-			anteHandler := ante.NewDeductFeeDecorator(chain.GetApp().AccountKeeper, chain.GetApp().BankKeeper, chain.GetApp().FeeGrantKeeper, chain.GetApp().RewardsKeeper)
+			anteHandler := ante.NewDeductFeeDecorator(chain.GetAppCodec(), chain.GetApp().AccountKeeper, chain.GetApp().BankKeeper, chain.GetApp().FeeGrantKeeper, chain.GetApp().RewardsKeeper)
 			_, err = anteHandler.AnteHandle(ctx, tx, false, testutils.NoopAnteHandler)
 			if tc.errExpected {
 				require.Error(t, err)