Skip to content

Commit

Permalink
Fix internal tests. (#1969)
Browse files Browse the repository at this point in the history 
* [1760]: Fix the min gas prices decorator tests.

* [1760]: In GetFeePayerUsingFeeGrant, treat the fee payer and granter as AccAddresses since they're now being given to us as byte slices which don't convert to strings properly in formats.

* [1760]: Fix the feegrant tests.

* [1760]: Fix signmode stuff in the testutil tests.

* [1760]: Fix signing stuff in the msg-fee invoker test.

* [1760]: Remove TODO about whether something was needed, it was, so I kept it.

* [1760]: Re-enable the internal/antewrapper tests in the github action.
  • Loading branch information
@SpicyLemon
SpicyLemon authored May 13, 2024
1 parent 5e61431 commit 6bda36f
Show file tree
Hide file tree
Showing 6 changed files with 117 additions and 94 deletions.
2 changes: 0 additions & 2 deletions .github/workflows/test.yml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,6 @@ jobs:
run: |
grep -vF \
-e 'github.com/provenance-io/provenance/cmd/provenanced/cmd' \
-e 'github.com/provenance-io/provenance/internal/antewrapper' \
-e 'github.com/provenance-io/provenance/x/ibchooks' \
-e 'github.com/provenance-io/provenance/x/ibcratelimit/module' \
-e 'github.com/provenance-io/provenance/x/ibcratelimit/simulation' \
Expand All @@ -62,7 +61,6 @@ jobs:
split -d -n l/3 pkgs.txt.tmp pkgs.txt.part.
printf '%s\n' \
'github.com/provenance-io/provenance/cmd/provenanced/cmd' \
'github.com/provenance-io/provenance/internal/antewrapper' \
'github.com/provenance-io/provenance/x/ibchooks' \
'github.com/provenance-io/provenance/x/ibcratelimit/module' \
'github.com/provenance-io/provenance/x/ibcratelimit/simulation' \
Expand Down
79 changes: 47 additions & 32 deletions internal/antewrapper/min_gas_prices_decorator_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,17 @@ import (
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
protov2 "google.golang.org/protobuf/proto"
"google.golang.org/protobuf/protoadapt"

cmtproto "github.com/cometbft/cometbft/proto/tendermint/types"

sdkmath "cosmossdk.io/math"

"github.com/cosmos/cosmos-sdk/codec"
sdk "github.com/cosmos/cosmos-sdk/types"
txtypes "github.com/cosmos/cosmos-sdk/types/tx"

"github.com/provenance-io/provenance/app"
"github.com/provenance-io/provenance/internal/antewrapper"
)

Expand Down Expand Up @@ -48,42 +51,54 @@ func (t NonFeeTx) GetMsgsV2() ([]protov2.Message, error) {
return nil, nil
}

var _ sdk.Tx = &FeeTxWrapper{}
var _ sdk.Tx = (*FeeTxWrapper)(nil)
var _ sdk.FeeTx = (*FeeTxWrapper)(nil)

// FeeTxWrapper is a wrapper on a txtypes.Tx that also has the GetMsgsV2 func so it satisfies the sdk.Tx interface.
type FeeTxWrapper struct {
txtypes.Tx
Codec codec.Codec
}

func (t FeeTxWrapper) GetMsgsV2() ([]protov2.Message, error) {
return nil, nil
msgs := t.Tx.GetMsgs()
rv := make([]protov2.Message, len(msgs))
for i, msg := range msgs {
rv[i] = protoadapt.MessageV2Of(msg)
}
return rv, nil
}

func NewFeeTxWrapper(tx txtypes.Tx) *FeeTxWrapper {
return &FeeTxWrapper{Tx: tx}
func (t FeeTxWrapper) FeePayer() []byte {
return t.Tx.FeePayer(t.Codec)
}

func NewFeeTx(gasLimit uint64, fee sdk.Coins) sdk.Tx {
return NewFeeTxWrapper(txtypes.Tx{
AuthInfo: &txtypes.AuthInfo{
Fee: &txtypes.Fee{
Amount: fee,
GasLimit: gasLimit,
},
},
})
func (t FeeTxWrapper) FeeGranter() []byte {
return t.Tx.FeeGranter(t.Codec)
}

func NewFeeTxWrapper(tx txtypes.Tx, codec codec.Codec) *FeeTxWrapper {
return &FeeTxWrapper{Tx: tx, Codec: codec}
}

func NewFeeTx(gasLimit uint64, fee sdk.Coins, codec codec.Codec) sdk.Tx {
tx := txtypes.Tx{AuthInfo: &txtypes.AuthInfo{Fee: &txtypes.Fee{Amount: fee, GasLimit: gasLimit}}}
return NewFeeTxWrapper(tx, codec)
}

func TestAnteHandle(tt *testing.T) {
func TestAnteHandle(t *testing.T) {
var dummyTx sdk.Tx
dummyTx = &NonFeeTx{}
_, ok := dummyTx.(sdk.FeeTx)
require.False(tt, ok, "NonFeeTx should not implement FeeTx.")
require.False(t, ok, "NonFeeTx should not implement FeeTx.")

testSkipMinGasPrices := sdk.NewDecCoins(sdk.NewInt64DecCoin("simfoo", 1000))
testSkipGas := uint64(5)
testSkipFee := sdk.NewCoins(sdk.NewInt64Coin("simfoo", 4999))

encCfg := app.MakeTestEncodingConfig(t)
codec := encCfg.Marshaler

tests := []struct {
name string
simulate bool
Expand All @@ -99,23 +114,23 @@ func TestAnteHandle(tt *testing.T) {
simulate: true,
isCheckTx: true,
minGasPrices: testSkipMinGasPrices,
tx: NewFeeTx(testSkipGas, testSkipFee),
tx: NewFeeTx(testSkipGas, testSkipFee, codec),
expectedInError: nil,
},
{
name: "skip because not isCheckTx",
simulate: false,
isCheckTx: false,
minGasPrices: testSkipMinGasPrices,
tx: NewFeeTx(testSkipGas, testSkipFee),
tx: NewFeeTx(testSkipGas, testSkipFee, codec),
expectedInError: nil,
},
{
name: "skip fails when not skipping",
simulate: false,
isCheckTx: true,
minGasPrices: testSkipMinGasPrices,
tx: NewFeeTx(testSkipGas, testSkipFee),
tx: NewFeeTx(testSkipGas, testSkipFee, codec),
expectedInError: []string{"insufficient fee", "min-gas-prices not met", "got: 4999simfoo", "required: 5000simfoo"},
},
// end of skip tests.
Expand All @@ -125,103 +140,103 @@ func TestAnteHandle(tt *testing.T) {
simulate: false,
isCheckTx: true,
minGasPrices: sdk.NewDecCoins(sdk.NewInt64DecCoin("gascoin", 100)),
tx: NewFeeTx(5, sdk.NewCoins(sdk.NewInt64Coin("feecoin", 500_000))),
tx: NewFeeTx(5, sdk.NewCoins(sdk.NewInt64Coin("feecoin", 500_000)), codec),
expectedInError: []string{"insufficient fee", "min-gas-prices not met", "got: 500000feecoin", "required: 500gascoin"},
},
{
name: "two gas denoms only first in fee not enough",
simulate: false,
isCheckTx: true,
minGasPrices: sdk.NewDecCoins(sdk.NewInt64DecCoin("onefoo", 10), sdk.NewInt64DecCoin("twofoo", 100)),
tx: NewFeeTx(5, sdk.NewCoins(sdk.NewInt64Coin("onefoo", 49))),
tx: NewFeeTx(5, sdk.NewCoins(sdk.NewInt64Coin("onefoo", 49)), codec),
expectedInError: []string{"insufficient fee", "min-gas-prices not met", "got: 49onefoo", "required: 50onefoo,500twofoo"},
},
{
name: "two gas denoms only first in fee barely enough",
simulate: false,
isCheckTx: true,
minGasPrices: sdk.NewDecCoins(sdk.NewInt64DecCoin("threefoo", 10), sdk.NewInt64DecCoin("fourfoo", 100)),
tx: NewFeeTx(5, sdk.NewCoins(sdk.NewInt64Coin("threefoo", 50))),
tx: NewFeeTx(5, sdk.NewCoins(sdk.NewInt64Coin("threefoo", 50)), codec),
expectedInError: nil,
},
{
name: "two gas denoms only second in fee not enough",
simulate: false,
isCheckTx: true,
minGasPrices: sdk.NewDecCoins(sdk.NewInt64DecCoin("fivefoo", 10), sdk.NewInt64DecCoin("sixfoo", 100)),
tx: NewFeeTx(5, sdk.NewCoins(sdk.NewInt64Coin("sixfoo", 499))),
tx: NewFeeTx(5, sdk.NewCoins(sdk.NewInt64Coin("sixfoo", 499)), codec),
expectedInError: []string{"insufficient fee", "min-gas-prices not met", "got: 499sixfoo", "required: 50fivefoo,500sixfoo"},
},
{
name: "two gas denoms only second in fee barely enough",
simulate: false,
isCheckTx: true,
minGasPrices: sdk.NewDecCoins(sdk.NewInt64DecCoin("sevenfoo", 10), sdk.NewInt64DecCoin("eightfoo", 100)),
tx: NewFeeTx(5, sdk.NewCoins(sdk.NewInt64Coin("eightfoo", 500))),
tx: NewFeeTx(5, sdk.NewCoins(sdk.NewInt64Coin("eightfoo", 500)), codec),
expectedInError: nil,
},
{
name: "two gas denoms not enough both",
simulate: false,
isCheckTx: true,
minGasPrices: sdk.NewDecCoins(sdk.NewInt64DecCoin("ninefoo", 10), sdk.NewInt64DecCoin("tenfoo", 100)),
tx: NewFeeTx(10, sdk.NewCoins(sdk.NewInt64Coin("ninefoo", 99), sdk.NewInt64Coin("tenfoo", 999))),
tx: NewFeeTx(10, sdk.NewCoins(sdk.NewInt64Coin("ninefoo", 99), sdk.NewInt64Coin("tenfoo", 999)), codec),
expectedInError: []string{"insufficient fee", "min-gas-prices not met", "got: 99ninefoo,999tenfoo", "required: 100ninefoo,1000tenfoo"},
},
{
name: "two gas denoms not enough first",
simulate: false,
isCheckTx: true,
minGasPrices: sdk.NewDecCoins(sdk.NewInt64DecCoin("elevenfoo", 10), sdk.NewInt64DecCoin("twelvefoo", 100)),
tx: NewFeeTx(10, sdk.NewCoins(sdk.NewInt64Coin("elevenfoo", 99), sdk.NewInt64Coin("twelvefoo", 1000))),
tx: NewFeeTx(10, sdk.NewCoins(sdk.NewInt64Coin("elevenfoo", 99), sdk.NewInt64Coin("twelvefoo", 1000)), codec),
expectedInError: nil,
},
{
name: "two gas denoms not enough second",
simulate: false,
isCheckTx: true,
minGasPrices: sdk.NewDecCoins(sdk.NewInt64DecCoin("thirteenfoo", 10), sdk.NewInt64DecCoin("fourteenfoo", 100)),
tx: NewFeeTx(10, sdk.NewCoins(sdk.NewInt64Coin("thirteenfoo", 100), sdk.NewInt64Coin("fourteenfoo", 999))),
tx: NewFeeTx(10, sdk.NewCoins(sdk.NewInt64Coin("thirteenfoo", 100), sdk.NewInt64Coin("fourteenfoo", 999)), codec),
expectedInError: nil,
},
{
name: "one gas denoms two coins in fee not enough",
simulate: false,
isCheckTx: true,
minGasPrices: sdk.NewDecCoins(sdk.NewInt64DecCoin("fifteenfoo", 10)),
tx: NewFeeTx(7, sdk.NewCoins(sdk.NewInt64Coin("fifteenfoo", 69), sdk.NewInt64Coin("sixteenfoo", 420))),
tx: NewFeeTx(7, sdk.NewCoins(sdk.NewInt64Coin("fifteenfoo", 69), sdk.NewInt64Coin("sixteenfoo", 420)), codec),
expectedInError: []string{"insufficient fee", "min-gas-prices not met", "got: 69fifteenfoo,420sixteenfoo", "required: 70fifteenfoo"},
},
{
name: "one gas denoms two coins in fee enough",
simulate: false,
isCheckTx: true,
minGasPrices: sdk.NewDecCoins(sdk.NewInt64DecCoin("seventeenfoo", 10)),
tx: NewFeeTx(1, sdk.NewCoins(sdk.NewInt64Coin("seventeenfoo", 420), sdk.NewInt64Coin("eighteenfoo", 69))),
tx: NewFeeTx(1, sdk.NewCoins(sdk.NewInt64Coin("seventeenfoo", 420), sdk.NewInt64Coin("eighteenfoo", 69)), codec),
expectedInError: nil,
},
{
name: "one gas denom more than enough fee",
simulate: false,
isCheckTx: true,
minGasPrices: sdk.NewDecCoins(sdk.NewInt64DecCoin("nineteenfoo", 50)),
tx: NewFeeTx(2, sdk.NewCoins(sdk.NewInt64Coin("nineteenfoo", 1_000_000))),
tx: NewFeeTx(2, sdk.NewCoins(sdk.NewInt64Coin("nineteenfoo", 1_000_000)), codec),
expectedInError: nil,
},
{
name: "min gas zero and no fee",
simulate: false,
isCheckTx: true,
minGasPrices: sdk.NewDecCoins(sdk.NewInt64DecCoin("twentyfoo", 0), sdk.NewInt64DecCoin("twentybar", 0)),
tx: NewFeeTx(100, sdk.NewCoins()),
tx: NewFeeTx(100, sdk.NewCoins(), codec),
expectedInError: nil,
},
{
name: "decimal min gas rounded up",
simulate: false,
isCheckTx: true,
minGasPrices: sdk.NewDecCoins(sdk.NewDecCoinFromDec("pcoin", sdkmath.LegacyMustNewDecFromStr("0.15"))),
tx: NewFeeTx(7, sdk.NewCoins(sdk.NewInt64Coin("pcoin", 1))),
tx: NewFeeTx(7, sdk.NewCoins(sdk.NewInt64Coin("pcoin", 1)), codec),
expectedInError: []string{"required: 2pcoin"},
},

Expand Down Expand Up @@ -261,7 +276,7 @@ func TestAnteHandle(tt *testing.T) {
}

for _, tc := range tests {
tt.Run(tc.name, func(t *testing.T) {
t.Run(tc.name, func(t *testing.T) {
ctx := sdk.NewContext(nil, cmtproto.Header{}, tc.isCheckTx, nil).WithMinGasPrices(tc.minGasPrices)
terminator := NewTestTerminator()
decorator := antewrapper.NewMinGasPricesDecorator()
Expand Down
45 changes: 30 additions & 15 deletions internal/antewrapper/prov_feegrant_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
sdk "github.com/cosmos/cosmos-sdk/types"
simtypes "github.com/cosmos/cosmos-sdk/types/simulation"
sdksigning "github.com/cosmos/cosmos-sdk/types/tx/signing"
authsigning "github.com/cosmos/cosmos-sdk/x/auth/signing"
"github.com/cosmos/cosmos-sdk/x/auth/tx"
"github.com/cosmos/cosmos-sdk/x/bank/testutil"

Expand Down Expand Up @@ -90,32 +91,37 @@ func (s *AnteTestSuite) TestDeductFeesNoDelegation() {
signer: addr1,
fee: defaultGas,
expInErr: []string{"10stake", defaultGasStr, "insufficient funds"},
}, {
},
{
name: "paying with good funds",
signerKey: priv2,
signer: addr2,
fee: defaultGas,
expInErr: nil,
}, {
},
{
name: "paying with no account",
signerKey: priv3,
signer: addr3,
fee: defaultGas,
expInErr: []string{"0stake", defaultGasStr, "insufficient funds"},
}, {
},
{
name: "no fee with no account",
signerKey: priv5,
signer: addr5,
fee: 0,
expInErr: []string{"fee payer address", addr5.String(), "does not exist"},
}, {
},
{
name: "valid fee grant without account",
signerKey: priv3,
signer: addr3,
feeAccount: addr2,
fee: defaultGas,
expInErr: nil,
}, {
},
{
name: "no fee grant",
signerKey: priv3,
signer: addr3,
Expand All @@ -127,10 +133,11 @@ func (s *AnteTestSuite) TestDeductFeesNoDelegation() {
fmt.Sprintf("granter: %s", addr1),
fmt.Sprintf("grantee: %s", addr3),
fmt.Sprintf(`fee: "%s"`, defaultGasStr),
`msgs: ["/testdata.TestMsg"]`,
`msgs: ["/testpb.TestMsg"]`,
"fee-grant not found",
},
}, {
},
{
name: "allowance smaller than requested fee",
signerKey: priv4,
signer: addr4,
Expand All @@ -142,10 +149,11 @@ func (s *AnteTestSuite) TestDeductFeesNoDelegation() {
fmt.Sprintf("granter: %s", addr2),
fmt.Sprintf("grantee: %s", addr4),
fmt.Sprintf(`fee: "%s"`, defaultGasStr),
`msgs: ["/testdata.TestMsg"]`,
`msgs: ["/testpb.TestMsg"]`,
"fee limit exceeded",
},
}, {
},
{
name: "granter cannot cover allowed fee grant",
signerKey: priv4,
signer: addr4,
Expand All @@ -157,7 +165,7 @@ func (s *AnteTestSuite) TestDeductFeesNoDelegation() {
fmt.Sprintf("granter: %s", addr1),
fmt.Sprintf("grantee: %s", addr4),
fmt.Sprintf(`fee: "%s"`, defaultGasStr),
`msgs: ["/testdata.TestMsg"]`,
`msgs: ["/testpb.TestMsg"]`,
"fee-grant not found",
},
},
Expand All @@ -169,7 +177,8 @@ func (s *AnteTestSuite) TestDeductFeesNoDelegation() {
msgs := []sdk.Msg{testdata.NewTestMsg(tc.signer)}

acc := app.AccountKeeper.GetAccount(ctx, tc.signer)
privs, accNums, seqs := []cryptotypes.PrivKey{tc.signerKey}, []uint64{0}, []uint64{0}
privs := []cryptotypes.PrivKey{tc.signerKey}
accNums, seqs := []uint64{0}, []uint64{0}
if acc != nil {
accNums, seqs = []uint64{acc.GetAccountNumber()}, []uint64{acc.GetSequence()}
}
Expand Down Expand Up @@ -243,19 +252,25 @@ func genTxWithFeeGranter(ctx context.Context, gen client.TxConfig, msgs []sdk.Ms
AccountNumber: accNums[i],
Sequence: accSeqs[i],
}
txData := signing.TxData{} // TODO[1760]: signing: Base this off of txb.GetTx().

theTx := txb.GetTx()
adaptableTx, ok := theTx.(authsigning.V2AdaptableTx)
if !ok {
return nil, fmt.Errorf("%T does not implement the authsigning.V2AdaptableTx interface", theTx)
}
txData := adaptableTx.GetSigningTxData()
signBytes, err := gen.SignModeHandler().GetSignBytes(ctx, signMode, signerData, txData)
if err != nil {
panic(err)
return nil, err
}
sig, err := p.Sign(signBytes)
if err != nil {
panic(err)
return nil, err
}
sigs[i].Data.(*sdksigning.SingleSignatureData).Signature = sig
err = txb.SetSignatures(sigs...)
if err != nil {
panic(err)
return nil, err
}
}

Expand Down
Loading

0 comments on commit 6bda36f

Please sign in to comment.