From 51be26f3986bf161d2fc1e2a49e2c358800e928f Mon Sep 17 00:00:00 2001 From: Simon Noetzlin Date: Mon, 15 Jan 2024 08:44:19 +0100 Subject: [PATCH] address comments --- app/keepers/keepers.go | 5 -- buf.work.yaml | 3 - tests/e2e/e2e_globalfee_proposal_test.go | 6 +- tests/e2e/e2e_gov_test.go | 16 ++---- tests/e2e/e2e_ics_test.go | 4 +- tests/e2e/e2e_lsm_test.go | 2 +- tests/e2e/query.go | 13 ----- x/globalfee/ante/fee.go | 8 --- x/globalfee/querier_test.go | 73 ------------------------ 9 files changed, 12 insertions(+), 118 deletions(-) delete mode 100644 x/globalfee/querier_test.go diff --git a/app/keepers/keepers.go b/app/keepers/keepers.go index b7b2ffb5337..032ee60ecdb 100644 --- a/app/keepers/keepers.go +++ b/app/keepers/keepers.go @@ -388,12 +388,8 @@ func NewAppKeeper( ibcRouter := porttypes.NewRouter() // Middleware Stacks - - // TODO: Move inline appKeepers.TransferModule = transfer.NewAppModule(appKeepers.TransferKeeper) - // TODO: Move inline appKeepers.ICAModule = ica.NewAppModule(nil, &appKeepers.ICAHostKeeper) - // TODO: Move inline appKeepers.PFMRouterModule = pfmrouter.NewAppModule(appKeepers.PFMRouterKeeper, appKeepers.GetSubspace(pfmroutertypes.ModuleName)) // create IBC module from bottom to top of stack @@ -449,7 +445,6 @@ func initParamsKeeper(appCodec codec.BinaryCodec, legacyAmino *codec.LegacyAmino paramsKeeper.Subspace(ibctransfertypes.ModuleName) paramsKeeper.Subspace(ibcexported.ModuleName) paramsKeeper.Subspace(icahosttypes.SubModuleName) - paramsKeeper.Subspace(pfmroutertypes.ModuleName).WithKeyTable(pfmroutertypes.ParamKeyTable()) paramsKeeper.Subspace(globalfee.ModuleName) paramsKeeper.Subspace(providertypes.ModuleName) diff --git a/buf.work.yaml b/buf.work.yaml index 1b4a0d95c29..9de52861e22 100644 --- a/buf.work.yaml +++ b/buf.work.yaml @@ -1,6 +1,3 @@ -# Generated by "buf config migrate-v1beta1". Edit as necessary, and -# remove this comment when you're finished. -# # This workspace file points to the roots found in your # previous "buf.yaml" configuration. version: v1 diff --git a/tests/e2e/e2e_globalfee_proposal_test.go b/tests/e2e/e2e_globalfee_proposal_test.go index df26e7a376a..58fe2197d9b 100644 --- a/tests/e2e/e2e_globalfee_proposal_test.go +++ b/tests/e2e/e2e_globalfee_proposal_test.go @@ -20,7 +20,7 @@ func (s *IntegrationTestSuite) govProposeNewGlobalfee(newGlobalfee sdk.DecCoins, // gov proposing new fees s.T().Logf("Proposal number: %d", proposalCounter) s.T().Logf("Submitting, deposit and vote legacy Gov Proposal: change global fee to %s", newGlobalfee.String()) - s.runGovProcess(chainAAPIEndpoint, submitter, proposalCounter, paramtypes.ProposalTypeChange, submitGovFlags, depositGovFlags, voteGovFlags, "vote", false) + s.submitLegacyGovProposal(chainAAPIEndpoint, submitter, proposalCounter, paramtypes.ProposalTypeChange, submitGovFlags, depositGovFlags, voteGovFlags, "vote", false) // query the proposal status and new fee s.Require().Eventually( @@ -57,7 +57,7 @@ func (s *IntegrationTestSuite) govProposeNewBypassMsgs(newBypassMsgs []string, p // gov proposing new fees s.T().Logf("Proposal number: %d", proposalCounter) s.T().Logf("Submitting, deposit and vote legacy Gov Proposal: change bypass min fee msg types to %s", newBypassMsgs) - s.runGovProcess(chainAAPIEndpoint, submitter, proposalCounter, paramtypes.ProposalTypeChange, submitGovFlags, depositGovFlags, voteGovFlags, "vote", false) + s.submitLegacyGovProposal(chainAAPIEndpoint, submitter, proposalCounter, paramtypes.ProposalTypeChange, submitGovFlags, depositGovFlags, voteGovFlags, "vote", false) // query the proposal status and new fee s.Require().Eventually( @@ -95,7 +95,7 @@ func (s *IntegrationTestSuite) govProposeNewMaxTotalBypassMinFeeMsgGasUsage(newG // gov proposing new max gas usage for bypass msgs s.T().Logf("Proposal number: %d", proposalCounter) s.T().Logf("Submitting, deposit and vote legacy Gov Proposal: change maxTotalBypassMinFeeMsgGasUsage to %d", newGas) - s.runGovProcess(chainAAPIEndpoint, submitter, proposalCounter, paramtypes.ProposalTypeChange, submitGovFlags, depositGovFlags, voteGovFlags, "vote", false) + s.submitLegacyGovProposal(chainAAPIEndpoint, submitter, proposalCounter, paramtypes.ProposalTypeChange, submitGovFlags, depositGovFlags, voteGovFlags, "vote", false) // query the proposal status and max gas usage for bypass msgs s.Require().Eventually( diff --git a/tests/e2e/e2e_gov_test.go b/tests/e2e/e2e_gov_test.go index 316590e8e3f..8ace820c9ae 100644 --- a/tests/e2e/e2e_gov_test.go +++ b/tests/e2e/e2e_gov_test.go @@ -42,7 +42,7 @@ func (s *IntegrationTestSuite) GovSoftwareUpgrade() { depositGovFlags := []string{strconv.Itoa(proposalCounter), depositAmount.String()} voteGovFlags := []string{strconv.Itoa(proposalCounter), "yes=0.8,no=0.1,abstain=0.05,no_with_veto=0.05"} - s.runGovProcess(chainAAPIEndpoint, sender, proposalCounter, upgradetypes.ProposalTypeSoftwareUpgrade, submitGovFlags, depositGovFlags, voteGovFlags, "weighted-vote", true) + s.submitLegacyGovProposal(chainAAPIEndpoint, sender, proposalCounter, upgradetypes.ProposalTypeSoftwareUpgrade, submitGovFlags, depositGovFlags, voteGovFlags, "weighted-vote", true) s.verifyChainHaltedAtUpgradeHeight(s.chainA, 0, proposalHeight) s.T().Logf("Successfully halted chain at height %d", proposalHeight) @@ -94,13 +94,13 @@ func (s *IntegrationTestSuite) GovCancelSoftwareUpgrade() { depositGovFlags := []string{strconv.Itoa(proposalCounter), depositAmount.String()} voteGovFlags := []string{strconv.Itoa(proposalCounter), "yes"} - s.runGovProcess(chainAAPIEndpoint, sender, proposalCounter, upgradetypes.ProposalTypeSoftwareUpgrade, submitGovFlags, depositGovFlags, voteGovFlags, "vote", true) + s.submitLegacyGovProposal(chainAAPIEndpoint, sender, proposalCounter, upgradetypes.ProposalTypeSoftwareUpgrade, submitGovFlags, depositGovFlags, voteGovFlags, "vote", true) proposalCounter++ submitGovFlags = []string{"cancel-software-upgrade", "--title='Upgrade V1'", "--description='Software Upgrade'"} depositGovFlags = []string{strconv.Itoa(proposalCounter), depositAmount.String()} voteGovFlags = []string{strconv.Itoa(proposalCounter), "yes"} - s.runGovProcess(chainAAPIEndpoint, sender, proposalCounter, upgradetypes.ProposalTypeCancelSoftwareUpgrade, submitGovFlags, depositGovFlags, voteGovFlags, "vote", true) + s.submitLegacyGovProposal(chainAAPIEndpoint, sender, proposalCounter, upgradetypes.ProposalTypeCancelSoftwareUpgrade, submitGovFlags, depositGovFlags, voteGovFlags, "vote", true) s.verifyChainPassesUpgradeHeight(s.chainA, 0, proposalHeight) s.T().Logf("Successfully canceled upgrade at height %d", proposalHeight) @@ -132,7 +132,7 @@ func (s *IntegrationTestSuite) GovCommunityPoolSpend() { depositGovFlags := []string{strconv.Itoa(proposalCounter), depositAmount.String()} voteGovFlags := []string{strconv.Itoa(proposalCounter), "yes"} // TODO: replace proposal type by distrtypes.ProposalTypeCommunityPoolSpend equivalent in SDK v0.47 - s.runGovProcessV1(chainAAPIEndpoint, sender, proposalCounter, "CommunityPoolSpend", submitGovFlags, depositGovFlags, voteGovFlags, "vote", false) + s.submitGovProposal(chainAAPIEndpoint, sender, proposalCounter, "CommunityPoolSpend", submitGovFlags, depositGovFlags, voteGovFlags, "vote", false) s.Require().Eventually( func() bool { @@ -146,9 +146,7 @@ func (s *IntegrationTestSuite) GovCommunityPoolSpend() { ) } -// NOTE: @MSalopek -// rename to runGovLegacyProcess or submitLegacyGovProposal -func (s *IntegrationTestSuite) runGovProcess(chainAAPIEndpoint, sender string, proposalID int, proposalType string, submitFlags []string, depositFlags []string, voteFlags []string, voteCommand string, withDeposit bool) { +func (s *IntegrationTestSuite) submitLegacyGovProposal(chainAAPIEndpoint, sender string, proposalID int, proposalType string, submitFlags []string, depositFlags []string, voteFlags []string, voteCommand string, withDeposit bool) { s.T().Logf("Submitting Gov Proposal: %s", proposalType) // min deposit of 1000uatom is required in e2e tests, otherwise the gov antehandler causes the proposal to be dropped sflags := submitFlags @@ -162,9 +160,7 @@ func (s *IntegrationTestSuite) runGovProcess(chainAAPIEndpoint, sender string, p s.submitGovCommand(chainAAPIEndpoint, sender, proposalID, voteCommand, voteFlags, govtypesv1beta1.StatusPassed) } -// NOTE: @MSalopek -// rename to runGovProcess or submitGovProposal -func (s *IntegrationTestSuite) runGovProcessV1(chainAAPIEndpoint, sender string, proposalID int, proposalType string, submitFlags []string, depositFlags []string, voteFlags []string, voteCommand string, withDeposit bool) { +func (s *IntegrationTestSuite) submitGovProposal(chainAAPIEndpoint, sender string, proposalID int, proposalType string, submitFlags []string, depositFlags []string, voteFlags []string, voteCommand string, withDeposit bool) { s.T().Logf("Submitting Gov Proposal: %s", proposalType) // min deposit of 1000uatom is required in e2e tests, otherwise the gov antehandler causes the proposal to be dropped sflags := submitFlags diff --git a/tests/e2e/e2e_ics_test.go b/tests/e2e/e2e_ics_test.go index b30ac32311f..6442b8366ee 100644 --- a/tests/e2e/e2e_ics_test.go +++ b/tests/e2e/e2e_ics_test.go @@ -102,7 +102,7 @@ func (s *IntegrationTestSuite) AddRemoveConsumerChain() { submitGovFlags := []string{"consumer-addition", configFile(proposalAddConsumerChainFilename)} depositGovFlags := []string{strconv.Itoa(proposalCounter), depositAmount.String()} voteGovFlags := []string{strconv.Itoa(proposalCounter), "yes"} - s.runGovProcess(chainAAPIEndpoint, sender, proposalCounter, providertypes.ProposalTypeConsumerAddition, submitGovFlags, depositGovFlags, voteGovFlags, "vote", false) + s.submitLegacyGovProposal(chainAAPIEndpoint, sender, proposalCounter, providertypes.ProposalTypeConsumerAddition, submitGovFlags, depositGovFlags, voteGovFlags, "vote", false) // Query and assert consumer has been added s.execQueryConsumerChains(s.chainA, 0, gaiaHomePath, validateConsumerAddition, consumerChainID) @@ -112,7 +112,7 @@ func (s *IntegrationTestSuite) AddRemoveConsumerChain() { submitGovFlags = []string{"consumer-removal", configFile(proposalRemoveConsumerChainFilename)} depositGovFlags = []string{strconv.Itoa(proposalCounter), depositAmount.String()} voteGovFlags = []string{strconv.Itoa(proposalCounter), "yes"} - s.runGovProcess(chainAAPIEndpoint, sender, proposalCounter, providertypes.ProposalTypeConsumerRemoval, submitGovFlags, depositGovFlags, voteGovFlags, "vote", false) + s.submitLegacyGovProposal(chainAAPIEndpoint, sender, proposalCounter, providertypes.ProposalTypeConsumerRemoval, submitGovFlags, depositGovFlags, voteGovFlags, "vote", false) // Query and assert consumer has been removed s.execQueryConsumerChains(s.chainA, 0, gaiaHomePath, validateConsumerRemoval, consumerChainID) } diff --git a/tests/e2e/e2e_lsm_test.go b/tests/e2e/e2e_lsm_test.go index cb30a9427b7..079941be254 100644 --- a/tests/e2e/e2e_lsm_test.go +++ b/tests/e2e/e2e_lsm_test.go @@ -30,7 +30,7 @@ func (s *IntegrationTestSuite) testLSM() { // gov proposing LSM parameters (global liquid staking cap, validator liquid staking cap, validator bond factor) s.T().Logf("Proposal number: %d", proposalCounter) s.T().Logf("Submitting, deposit and vote legacy Gov Proposal: Set parameters (global liquid staking cap, validator liquid staking cap, validator bond factor)") - s.runGovProcessV1(chainEndpoint, validatorAAddr.String(), proposalCounter, "stakingtypes.MsgUpdateProposal", submitGovFlags, depositGovFlags, voteGovFlags, "vote", false) + s.submitGovProposal(chainEndpoint, validatorAAddr.String(), proposalCounter, "stakingtypes.MsgUpdateProposal", submitGovFlags, depositGovFlags, voteGovFlags, "vote", false) // query the proposal status and new fee s.Require().Eventually( diff --git a/tests/e2e/query.go b/tests/e2e/query.go index 4696154cea8..8661776a180 100644 --- a/tests/e2e/query.go +++ b/tests/e2e/query.go @@ -235,19 +235,6 @@ func queryContinuousVestingAccount(endpoint, address string) (authvesting.Contin return *acc, nil } -// func queryPermanentLockedAccount(endpoint, address string) (authvesting.PermanentLockedAccount, error) { //nolint:unused // this is called during e2e tests -// baseAcc, err := queryAccount(endpoint, address) -// if err != nil { -// return authvesting.PermanentLockedAccount{}, err -// } -// acc, ok := baseAcc.(*authvesting.PermanentLockedAccount) -// if !ok { -// return authvesting.PermanentLockedAccount{}, -// fmt.Errorf("cannot cast %v to PermanentLockedAccount", baseAcc) -// } -// return *acc, nil -// } - func queryPeriodicVestingAccount(endpoint, address string) (authvesting.PeriodicVestingAccount, error) { //nolint:unused // this is called during e2e tests baseAcc, err := queryAccount(endpoint, address) if err != nil { diff --git a/x/globalfee/ante/fee.go b/x/globalfee/ante/fee.go index 0679b0aa317..b3b4c4d39ab 100644 --- a/x/globalfee/ante/fee.go +++ b/x/globalfee/ante/fee.go @@ -61,9 +61,6 @@ func (mfd FeeDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bool, ne return ctx, err } - ctx.Logger().Info(fmt.Sprintf("FeeCoins: %v", feeTx.GetFee())) - ctx.Logger().Info(fmt.Sprintf("FeeRequired: %v", feeRequired)) - // reject the transaction early if the feeCoins have more denoms than the fee requirement // feeRequired cannot be empty @@ -107,11 +104,6 @@ func (mfd FeeDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bool, ne doesNotExceedMaxGasUsage := gas <= maxTotalBypassMinFeeMsgGasUsage allBypassMsgs := mfd.ContainsOnlyBypassMinFeeMsgs(ctx, msgs) allowedToBypassMinFee := allBypassMsgs && doesNotExceedMaxGasUsage - ctx.Logger().Info(fmt.Sprintf("gas: %v", gas)) - ctx.Logger().Info(fmt.Sprintf("allBypassMsgs: %v", allBypassMsgs)) - ctx.Logger().Info(fmt.Sprintf("maxTotalBypassMinFeeMsgGasUsage: %v", maxTotalBypassMinFeeMsgGasUsage)) - ctx.Logger().Info(fmt.Sprintf("doesNotExceedMaxGasUsage: %v", doesNotExceedMaxGasUsage)) - ctx.Logger().Info(fmt.Sprintf("allowedToBypassMinFee: %v", allowedToBypassMinFee)) if allowedToBypassMinFee { ctx.Logger().Info("Pass allowedToBypassMinFee check") diff --git a/x/globalfee/querier_test.go b/x/globalfee/querier_test.go deleted file mode 100644 index 2b314a08663..00000000000 --- a/x/globalfee/querier_test.go +++ /dev/null @@ -1,73 +0,0 @@ -package globalfee - -// import ( -// "testing" - -// sdk "github.com/cosmos/cosmos-sdk/types" -// paramtypes "github.com/cosmos/cosmos-sdk/x/params/types" -// "github.com/stretchr/testify/assert" -// "github.com/stretchr/testify/require" - -// "github.com/cosmos/gaia/v11/x/globalfee/types" -// ) -// "github.com/stretchr/testify/assert" -// "github.com/stretchr/testify/require" - -// sdk "github.com/cosmos/cosmos-sdk/types" -// paramtypes "github.com/cosmos/cosmos-sdk/x/params/types" - -// "github.com/cosmos/gaia/v15/x/globalfee/types" -// ) - -// "github.com/stretchr/testify/assert" -// "github.com/stretchr/testify/require" - -// sdk "github.com/cosmos/cosmos-sdk/types" -// paramtypes "github.com/cosmos/cosmos-sdk/x/params/types" - -// "github.com/cosmos/gaia/v15/x/globalfee/types" -// ) - -// func TestQueryMinimumGasPrices(t *testing.T) { -// specs := map[string]struct { -// setupStore func(ctx sdk.Context, s paramtypes.Subspace) -// expMin sdk.DecCoins -// }{ -// "one coin": { -// setupStore: func(ctx sdk.Context, s paramtypes.Subspace) { -// s.SetParamSet(ctx, &types.Params{ -// MinimumGasPrices: sdk.NewDecCoins(sdk.NewDecCoin("ALX", sdk.OneInt())), -// }) -// }, -// expMin: sdk.NewDecCoins(sdk.NewDecCoin("ALX", sdk.OneInt())), -// }, -// "multiple coins": { -// setupStore: func(ctx sdk.Context, s paramtypes.Subspace) { -// s.SetParamSet(ctx, &types.Params{ -// MinimumGasPrices: sdk.NewDecCoins(sdk.NewDecCoin("ALX", sdk.OneInt()), sdk.NewDecCoin("BLX", sdk.NewInt(2))), -// }) -// }, -// expMin: sdk.NewDecCoins(sdk.NewDecCoin("ALX", sdk.OneInt()), sdk.NewDecCoin("BLX", sdk.NewInt(2))), -// }, -// "no min gas price set": { -// setupStore: func(ctx sdk.Context, s paramtypes.Subspace) { -// s.SetParamSet(ctx, &types.Params{}) -// }, -// }, -// "no param set": { -// setupStore: func(ctx sdk.Context, s paramtypes.Subspace) { -// }, -// }, -// } -// for name, spec := range specs { -// t.Run(name, func(t *testing.T) { -// ctx, _, subspace := setupTestStore(t) -// spec.setupStore(ctx, subspace) -// q := NewGrpcQuerier(subspace) -// gotResp, gotErr := q.Params(sdk.WrapSDKContext(ctx), nil) -// require.NoError(t, gotErr) -// require.NotNil(t, gotResp) -// assert.Equal(t, spec.expMin, gotResp.Params.MinimumGasPrices) -// }) -// } -// }