From d7c5bc5f35505e1d64e9b3521067871947d55bfa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?colin=20axn=C3=A9r?= <25233464+colin-axner@users.noreply.github.com> Date: Tue, 1 Jun 2021 09:48:19 +0200 Subject: [PATCH 1/6] fix typo, v0.42 -> v0.43 (#9430) Co-authored-by: Marko --- x/bank/legacy/v043/store.go | 2 +- x/distribution/legacy/v043/store.go | 2 +- x/gov/legacy/v043/store.go | 2 +- x/slashing/legacy/v043/store.go | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/x/bank/legacy/v043/store.go b/x/bank/legacy/v043/store.go index f22901b9cc8f..a57907b20560 100644 --- a/x/bank/legacy/v043/store.go +++ b/x/bank/legacy/v043/store.go @@ -69,7 +69,7 @@ func migrateBalanceKeys(store sdk.KVStore) { } } -// MigrateStore performs in-place store migrations from v0.40 to v0.42. The +// MigrateStore performs in-place store migrations from v0.40 to v0.43. The // migration includes: // // - Change addresses to be length-prefixed. diff --git a/x/distribution/legacy/v043/store.go b/x/distribution/legacy/v043/store.go index 1cb78ea0be85..4448c59b75ff 100644 --- a/x/distribution/legacy/v043/store.go +++ b/x/distribution/legacy/v043/store.go @@ -5,7 +5,7 @@ import ( v040distribution "github.com/cosmos/cosmos-sdk/x/distribution/legacy/v040" ) -// MigrateStore performs in-place store migrations from v0.40 to v0.42. The +// MigrateStore performs in-place store migrations from v0.40 to v0.43. The // migration includes: // // - Change addresses to be length-prefixed. diff --git a/x/gov/legacy/v043/store.go b/x/gov/legacy/v043/store.go index 1658af56c8fa..c5da33d951c4 100644 --- a/x/gov/legacy/v043/store.go +++ b/x/gov/legacy/v043/store.go @@ -65,7 +65,7 @@ func migrateStoreWeightedVotes(store sdk.KVStore, cdc codec.BinaryCodec) error { return nil } -// MigrateStore performs in-place store migrations from v0.40 to v0.42. The +// MigrateStore performs in-place store migrations from v0.40 to v0.43. The // migration includes: // // - Change addresses to be length-prefixed. diff --git a/x/slashing/legacy/v043/store.go b/x/slashing/legacy/v043/store.go index 4737315e98f3..daee1d3fc157 100644 --- a/x/slashing/legacy/v043/store.go +++ b/x/slashing/legacy/v043/store.go @@ -6,7 +6,7 @@ import ( v040slashing "github.com/cosmos/cosmos-sdk/x/slashing/legacy/v040" ) -// MigrateStore performs in-place store migrations from v0.40 to v0.42. The +// MigrateStore performs in-place store migrations from v0.40 to v0.43. The // migration includes: // // - Change addresses to be length-prefixed. From 98fbdbd5b81a6f7b687dd7fc960b6d92272f1a3a Mon Sep 17 00:00:00 2001 From: Sunny Aggarwal Date: Tue, 1 Jun 2021 09:20:16 -0400 Subject: [PATCH 2/6] fix blond typo (#9422) Co-authored-by: Marko --- x/slashing/types/genesis.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x/slashing/types/genesis.go b/x/slashing/types/genesis.go index ee765c8c37e6..9b427e725ac3 100644 --- a/x/slashing/types/genesis.go +++ b/x/slashing/types/genesis.go @@ -55,7 +55,7 @@ func ValidateGenesis(data GenesisState) error { downtimeJail := data.Params.DowntimeJailDuration if downtimeJail < 1*time.Minute { - return fmt.Errorf("downtime unblond duration must be at least 1 minute, is %s", downtimeJail.String()) + return fmt.Errorf("downtime unjail duration must be at least 1 minute, is %s", downtimeJail.String()) } signedWindow := data.Params.SignedBlocksWindow From da87ab0d365819545eda18c8f3f4ea5e7be47105 Mon Sep 17 00:00:00 2001 From: likhita-809 <78951027+likhita-809@users.noreply.github.com> Date: Tue, 1 Jun 2021 20:47:44 +0530 Subject: [PATCH 3/6] fix: Staking delegations should return empty list instead of rpc error when no records found (#9423) * return empty list instead of rpc error when no records found for staking delegations * fix grpc query DelegatorDelegations tests * remove response code tests for staking delegations * fix failing tests * change staking delegations response code to 200 in grpc test * add staking delegations response code tests to TestQueryDelegatorDelegationsGRPC * add address without delegations testcase * add changes to grpc query tests of delegatorDelegations * remove getRequest unused function from x/staking/client/rest/grpc_query_test.go * minor fixes * add testcases for request with no delegations * address review comments * add changelog Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> --- CHANGELOG.md | 1 + x/staking/client/rest/grpc_query_test.go | 45 +++----- x/staking/keeper/grpc_query.go | 5 - x/staking/keeper/grpc_query_test.go | 129 +++++++++++++++-------- 4 files changed, 102 insertions(+), 78 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0249a1d31d4e..1e5e1843296a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -127,6 +127,7 @@ if input key is empty, or input data contains empty key. ### Improvements +* (x/staking) [\#9423](https://github.com/cosmos/cosmos-sdk/pull/9423) Staking delegations now returns empty list instead of rpc error when no records found. * (baseapp, types) [#\9390](https://github.com/cosmos/cosmos-sdk/pull/9390) Add current block header hash to `Context` * (x/bank) [\#8614](https://github.com/cosmos/cosmos-sdk/issues/8614) Add `Name` and `Symbol` fields to denom metadata * (x/auth) [\#8522](https://github.com/cosmos/cosmos-sdk/pull/8522) Allow to query all stored accounts diff --git a/x/staking/client/rest/grpc_query_test.go b/x/staking/client/rest/grpc_query_test.go index 651931d8d559..7ead75b8df73 100644 --- a/x/staking/client/rest/grpc_query_test.go +++ b/x/staking/client/rest/grpc_query_test.go @@ -4,8 +4,6 @@ package rest_test import ( "fmt" - "io/ioutil" - "net/http" "testing" "github.com/gogo/protobuf/proto" @@ -414,39 +412,15 @@ func (s *IntegrationTestSuite) TestQueryUnbondingDelegationGRPC() { } } -func (s *IntegrationTestSuite) TestQueryDelegationsResponseCode() { +func (s *IntegrationTestSuite) TestQueryDelegatorDelegationsGRPC() { val := s.network.Validators[0] + baseURL := val.APIAddress - // Create new account in the keyring. + // Create new account in the keyring for address without delegations. info, _, err := val.ClientCtx.Keyring.NewMnemonic("test", keyring.English, sdk.FullFundraiserPath, keyring.DefaultBIP39Passphrase, hd.Secp256k1) s.Require().NoError(err) newAddr := sdk.AccAddress(info.GetPubKey().Address()) - s.T().Log("expect 404 error for address without delegations") - res, statusCode, err := getRequest(fmt.Sprintf("%s/cosmos/staking/v1beta1/delegations/%s", val.APIAddress, newAddr.String())) - s.Require().NoError(err) - s.Require().Contains(string(res), "\"code\": 5") - s.Require().Equal(404, statusCode) -} - -func getRequest(url string) ([]byte, int, error) { - res, err := http.Get(url) // nolint:gosec - body, err := ioutil.ReadAll(res.Body) - if err != nil { - return nil, res.StatusCode, err - } - - if err = res.Body.Close(); err != nil { - return nil, res.StatusCode, err - } - - return body, res.StatusCode, nil -} - -func (s *IntegrationTestSuite) TestQueryDelegatorDelegationsGRPC() { - val := s.network.Validators[0] - baseURL := val.APIAddress - testCases := []struct { name string url string @@ -486,6 +460,19 @@ func (s *IntegrationTestSuite) TestQueryDelegatorDelegationsGRPC() { Pagination: &query.PageResponse{Total: 1}, }, }, + { + "address without delegations", + fmt.Sprintf("%s/cosmos/staking/v1beta1/delegations/%s", baseURL, newAddr.String()), + map[string]string{ + grpctypes.GRPCBlockHeightHeader: "1", + }, + false, + &types.QueryDelegatorDelegationsResponse{}, + &types.QueryDelegatorDelegationsResponse{ + DelegationResponses: types.DelegationResponses{}, + Pagination: &query.PageResponse{Total: 0}, + }, + }, } for _, tc := range testCases { diff --git a/x/staking/keeper/grpc_query.go b/x/staking/keeper/grpc_query.go index 4852848255c0..8fa16354d48e 100644 --- a/x/staking/keeper/grpc_query.go +++ b/x/staking/keeper/grpc_query.go @@ -281,11 +281,6 @@ func (k Querier) DelegatorDelegations(c context.Context, req *types.QueryDelegat return nil, status.Error(codes.Internal, err.Error()) } - if delegations == nil { - return nil, status.Errorf( - codes.NotFound, - "unable to find delegations for address %s", req.DelegatorAddr) - } delegationResps, err := DelegationsToDelegationResponses(ctx, k.Keeper, delegations) if err != nil { return nil, status.Error(codes.Internal, err.Error()) diff --git a/x/staking/keeper/grpc_query_test.go b/x/staking/keeper/grpc_query_test.go index dc47dc34c17a..ba43269e2e32 100644 --- a/x/staking/keeper/grpc_query_test.go +++ b/x/staking/keeper/grpc_query_test.go @@ -35,7 +35,8 @@ func (suite *KeeperTestSuite) TestGRPCQueryValidators() { len(vals), false, }, - {"empty status returns all the validators", + { + "empty status returns all the validators", func() { req = &types.QueryValidatorsRequest{Status: ""} }, @@ -52,7 +53,8 @@ func (suite *KeeperTestSuite) TestGRPCQueryValidators() { 0, false, }, - {"valid request", + { + "valid request", func() { req = &types.QueryValidatorsRequest{Status: types.Bonded.String(), Pagination: &query.PageRequest{Limit: 1, CountTotal: true}} @@ -101,7 +103,8 @@ func (suite *KeeperTestSuite) TestGRPCQueryValidator() { }, false, }, - {"valid request", + { + "valid request", func() { req = &types.QueryValidatorRequest{ValidatorAddr: vals[0].OperatorAddress} }, @@ -141,7 +144,8 @@ func (suite *KeeperTestSuite) TestGRPCQueryDelegatorValidators() { }, false, }, - {"valid request", + { + "valid request", func() { req = &types.QueryDelegatorValidatorsRequest{ DelegatorAddr: addrs[0].String(), @@ -185,7 +189,8 @@ func (suite *KeeperTestSuite) TestGRPCQueryDelegatorValidator() { }, false, }, - {"invalid delegator, validator pair", + { + "invalid delegator, validator pair", func() { req = &types.QueryDelegatorValidatorRequest{ DelegatorAddr: addr.String(), @@ -194,7 +199,8 @@ func (suite *KeeperTestSuite) TestGRPCQueryDelegatorValidator() { }, false, }, - {"valid request", + { + "valid request", func() { req = &types.QueryDelegatorValidatorRequest{ DelegatorAddr: addr.String(), @@ -235,13 +241,15 @@ func (suite *KeeperTestSuite) TestGRPCQueryDelegation() { malleate func() expPass bool }{ - {"empty request", + { + "empty request", func() { req = &types.QueryDelegationRequest{} }, false, }, - {"invalid validator, delegator pair", + { + "invalid validator, delegator pair", func() { req = &types.QueryDelegationRequest{ DelegatorAddr: addrAcc1.String(), @@ -250,7 +258,8 @@ func (suite *KeeperTestSuite) TestGRPCQueryDelegation() { }, false, }, - {"valid request", + { + "valid request", func() { req = &types.QueryDelegationRequest{DelegatorAddr: addrAcc.String(), ValidatorAddr: addrVal} }, @@ -285,27 +294,42 @@ func (suite *KeeperTestSuite) TestGRPCQueryDelegatorDelegations() { var req *types.QueryDelegatorDelegationsRequest testCases := []struct { - msg string - malleate func() - expPass bool + msg string + malleate func() + onSuccess func(suite *KeeperTestSuite, response *types.QueryDelegatorDelegationsResponse) + expErr bool }{ - {"empty request", + { + "empty request", func() { req = &types.QueryDelegatorDelegationsRequest{} }, - false, - }, {"invalid request", + func(suite *KeeperTestSuite, response *types.QueryDelegatorDelegationsResponse) {}, + true, + }, + { + "valid request with no delegations", func() { req = &types.QueryDelegatorDelegationsRequest{DelegatorAddr: addrs[4].String()} }, + func(suite *KeeperTestSuite, response *types.QueryDelegatorDelegationsResponse) { + suite.Equal(uint64(0), response.Pagination.Total) + suite.Len(response.DelegationResponses, 0) + }, false, }, - {"valid request", + { + "valid request", func() { req = &types.QueryDelegatorDelegationsRequest{DelegatorAddr: addrAcc.String(), Pagination: &query.PageRequest{Limit: 1, CountTotal: true}} }, - true, + func(suite *KeeperTestSuite, response *types.QueryDelegatorDelegationsResponse) { + suite.Equal(uint64(2), response.Pagination.Total) + suite.Len(response.DelegationResponses, 1) + suite.Equal(sdk.NewCoin(sdk.DefaultBondDenom, delegation.Shares.TruncateInt()), response.DelegationResponses[0].Balance) + }, + false, }, } @@ -313,14 +337,11 @@ func (suite *KeeperTestSuite) TestGRPCQueryDelegatorDelegations() { suite.Run(fmt.Sprintf("Case %s", tc.msg), func() { tc.malleate() res, err := queryClient.DelegatorDelegations(gocontext.Background(), req) - if tc.expPass { - suite.Equal(uint64(2), res.Pagination.Total) - suite.Len(res.DelegationResponses, 1) - suite.Equal(1, len(res.DelegationResponses)) - suite.Equal(sdk.NewCoin(sdk.DefaultBondDenom, delegation.Shares.TruncateInt()), res.DelegationResponses[0].Balance) - } else { + if tc.expErr { suite.Error(err) - suite.Nil(res) + } else { + suite.NoError(err) + tc.onSuccess(suite, res) } }) } @@ -344,21 +365,24 @@ func (suite *KeeperTestSuite) TestGRPCQueryValidatorDelegations() { expPass bool expErr bool }{ - {"empty request", + { + "empty request", func() { req = &types.QueryValidatorDelegationsRequest{} }, false, true, }, - {"invalid validator delegator pair", + { + "invalid validator delegator pair", func() { req = &types.QueryValidatorDelegationsRequest{ValidatorAddr: addrVal2.String()} }, false, false, }, - {"valid request", + { + "valid request", func() { req = &types.QueryValidatorDelegationsRequest{ValidatorAddr: addrVal1, Pagination: &query.PageRequest{Limit: 1, CountTotal: true}} @@ -409,19 +433,22 @@ func (suite *KeeperTestSuite) TestGRPCQueryUnbondingDelegation() { malleate func() expPass bool }{ - {"empty request", + { + "empty request", func() { req = &types.QueryUnbondingDelegationRequest{} }, false, }, - {"invalid request", + { + "invalid request", func() { req = &types.QueryUnbondingDelegationRequest{} }, false, }, - {"valid request", + { + "valid request", func() { req = &types.QueryUnbondingDelegationRequest{ DelegatorAddr: addrAcc2.String(), ValidatorAddr: addrVal2} @@ -469,21 +496,24 @@ func (suite *KeeperTestSuite) TestGRPCQueryDelegatorUnbondingDelegations() { expPass bool expErr bool }{ - {"empty request", + { + "empty request", func() { req = &types.QueryDelegatorUnbondingDelegationsRequest{} }, false, true, }, - {"invalid request", + { + "invalid request", func() { req = &types.QueryDelegatorUnbondingDelegationsRequest{DelegatorAddr: addrAcc1.String()} }, false, false, }, - {"valid request", + { + "valid request", func() { req = &types.QueryDelegatorUnbondingDelegationsRequest{DelegatorAddr: addrAcc.String(), Pagination: &query.PageRequest{Limit: 1, CountTotal: true}} @@ -544,25 +574,29 @@ func (suite *KeeperTestSuite) TestGRPCQueryHistoricalInfo() { malleate func() expPass bool }{ - {"empty request", + { + "empty request", func() { req = &types.QueryHistoricalInfoRequest{} }, false, }, - {"invalid request with negative height", + { + "invalid request with negative height", func() { req = &types.QueryHistoricalInfoRequest{Height: -1} }, false, }, - {"valid request with old height", + { + "valid request with old height", func() { req = &types.QueryHistoricalInfoRequest{Height: 4} }, false, }, - {"valid request with current height", + { + "valid request with current height", func() { req = &types.QueryHistoricalInfoRequest{Height: 5} }, @@ -612,14 +646,16 @@ func (suite *KeeperTestSuite) TestGRPCQueryRedelegations() { expPass bool expErr bool }{ - {"request redelegations for non existent addr", + { + "request redelegations for non existent addr", func() { req = &types.QueryRedelegationsRequest{DelegatorAddr: addrAcc.String()} }, false, false, }, - {"request redelegations with non existent pairs", + { + "request redelegations with non existent pairs", func() { req = &types.QueryRedelegationsRequest{DelegatorAddr: addrAcc.String(), SrcValidatorAddr: val3.String(), DstValidatorAddr: val4.String()} @@ -627,7 +663,8 @@ func (suite *KeeperTestSuite) TestGRPCQueryRedelegations() { false, true, }, - {"request redelegations with delegatoraddr, sourceValAddr, destValAddr", + { + "request redelegations with delegatoraddr, sourceValAddr, destValAddr", func() { req = &types.QueryRedelegationsRequest{ DelegatorAddr: addrAcc1.String(), SrcValidatorAddr: val1.OperatorAddress, @@ -636,7 +673,8 @@ func (suite *KeeperTestSuite) TestGRPCQueryRedelegations() { true, false, }, - {"request redelegations with delegatoraddr and sourceValAddr", + { + "request redelegations with delegatoraddr and sourceValAddr", func() { req = &types.QueryRedelegationsRequest{ DelegatorAddr: addrAcc1.String(), SrcValidatorAddr: val1.OperatorAddress, @@ -645,7 +683,8 @@ func (suite *KeeperTestSuite) TestGRPCQueryRedelegations() { true, false, }, - {"query redelegations with sourceValAddr only", + { + "query redelegations with sourceValAddr only", func() { req = &types.QueryRedelegationsRequest{ SrcValidatorAddr: val1.GetOperator().String(), @@ -695,13 +734,15 @@ func (suite *KeeperTestSuite) TestGRPCQueryValidatorUnbondingDelegations() { malleate func() expPass bool }{ - {"empty request", + { + "empty request", func() { req = &types.QueryValidatorUnbondingDelegationsRequest{} }, false, }, - {"valid request", + { + "valid request", func() { req = &types.QueryValidatorUnbondingDelegationsRequest{ ValidatorAddr: val1.GetOperator().String(), From 2ae78754889203a6611b478ae31c9e29aef1b45b Mon Sep 17 00:00:00 2001 From: yys Date: Wed, 2 Jun 2021 05:03:32 +0900 Subject: [PATCH 4/6] fix: Bank module init genesis optimization (#9428) * optimize the bank module genesis initialization * remove k.setBalances & k.clearBalances and update changelog * fix lint Co-authored-by: Aaron Craelius --- CHANGELOG.md | 1 + x/bank/keeper/genesis.go | 2 +- x/bank/keeper/send.go | 35 ++++++++++++----------------------- 3 files changed, 14 insertions(+), 24 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1e5e1843296a..f38304d2d01d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -36,6 +36,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ ## [Unreleased] +* [\#9428](https://github.com/cosmos/cosmos-sdk/pull/9428) Optimize bank InitGenesis. Removed bank keeper's `k.setBalances` and `k.clearBalances`. Added `k.initBalances`. * [\#9231](https://github.com/cosmos/cosmos-sdk/pull/9231) Remove redundant staking errors. * [\#9205](https://github.com/cosmos/cosmos-sdk/pull/9205) Improve readability in `abci` handleQueryP2P * [\#9235](https://github.com/cosmos/cosmos-sdk/pull/9235) CreateMembershipProof/CreateNonMembershipProof now returns an error diff --git a/x/bank/keeper/genesis.go b/x/bank/keeper/genesis.go index 712e2f5d19ed..24b5ef44f9fc 100644 --- a/x/bank/keeper/genesis.go +++ b/x/bank/keeper/genesis.go @@ -21,7 +21,7 @@ func (k BaseKeeper) InitGenesis(ctx sdk.Context, genState *types.GenesisState) { panic(err) } - if err := k.setBalances(ctx, addr, balance.Coins); err != nil { + if err := k.initBalances(ctx, addr, balance.Coins); err != nil { panic(fmt.Errorf("error on setting balances %w", err)) } diff --git a/x/bank/keeper/send.go b/x/bank/keeper/send.go index 675eb67735e7..369fa631c447 100644 --- a/x/bank/keeper/send.go +++ b/x/bank/keeper/send.go @@ -227,31 +227,20 @@ func (k BaseSendKeeper) addCoins(ctx sdk.Context, addr sdk.AccAddress, amt sdk.C return nil } -// clearBalances removes all balances for a given account by address. -func (k BaseSendKeeper) clearBalances(ctx sdk.Context, addr sdk.AccAddress) { - keys := [][]byte{} - k.IterateAccountBalances(ctx, addr, func(balance sdk.Coin) bool { - keys = append(keys, []byte(balance.Denom)) - return false - }) - +// initBalances sets the balance (multiple coins) for an account by address. +// An error is returned upon failure. +func (k BaseSendKeeper) initBalances(ctx sdk.Context, addr sdk.AccAddress, balances sdk.Coins) error { accountStore := k.getAccountStore(ctx, addr) + for i := range balances { + balance := balances[i] + if !balance.IsValid() { + return sdkerrors.Wrap(sdkerrors.ErrInvalidCoins, balance.String()) + } - for _, key := range keys { - accountStore.Delete(key) - } -} - -// setBalances sets the balance (multiple coins) for an account by address. It will -// clear out all balances prior to setting the new coins as to set existing balances -// to zero if they don't exist in amt. An error is returned upon failure. -func (k BaseSendKeeper) setBalances(ctx sdk.Context, addr sdk.AccAddress, balances sdk.Coins) error { - k.clearBalances(ctx, addr) - - for _, balance := range balances { - err := k.setBalance(ctx, addr, balance) - if err != nil { - return err + // Bank invariants require to not store zero balances. + if !balance.IsZero() { + bz := k.cdc.MustMarshal(&balance) + accountStore.Set([]byte(balance.Denom), bz) } } From 33c045c3c9376344c7691937b529ae6a4e7e726f Mon Sep 17 00:00:00 2001 From: Ryan Christoffersen <12519942+ryanchristo@users.noreply.github.com> Date: Wed, 2 Jun 2021 06:50:59 -0700 Subject: [PATCH 5/6] fix memo flag description (#9436) Co-authored-by: ryanchrypto <12519942+ryanchrypto@users.noreply.github.com> Co-authored-by: Amaury <1293565+amaurym@users.noreply.github.com> --- client/flags/flags.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client/flags/flags.go b/client/flags/flags.go index 629929bb08ba..14e4c8155ac4 100644 --- a/client/flags/flags.go +++ b/client/flags/flags.go @@ -97,7 +97,7 @@ func AddTxFlagsToCmd(cmd *cobra.Command) { cmd.Flags().String(FlagFrom, "", "Name or address of private key with which to sign") cmd.Flags().Uint64P(FlagAccountNumber, "a", 0, "The account number of the signing account (offline mode only)") cmd.Flags().Uint64P(FlagSequence, "s", 0, "The sequence number of the signing account (offline mode only)") - cmd.Flags().String(FlagNote, "", "Note to add a description to the transaction (previously `--memo`)") + cmd.Flags().String(FlagNote, "", "Note to add a description to the transaction (previously --memo)") cmd.Flags().String(FlagFees, "", "Fees to pay along with transaction; eg: 10uatom") cmd.Flags().String(FlagGasPrices, "", "Gas prices in decimal format to determine the transaction fee (e.g. 0.1uatom)") cmd.Flags().String(FlagNode, "tcp://localhost:26657", ": to tendermint rpc interface for this chain") From 90edeb67e29eb5335e406ab3ad56c5b4020740a0 Mon Sep 17 00:00:00 2001 From: Federico Kunze <31522760+fedekunze@users.noreply.github.com> Date: Wed, 2 Jun 2021 11:14:15 -0400 Subject: [PATCH 6/6] feat: add `RefundGas` function to `GasMeter` (#9403) * feat: add RefundGas function to GasMeter * changelog * add comment about use case * Apply suggestions from code review Co-authored-by: Alessio Treglia Co-authored-by: Amaury <1293565+amaurym@users.noreply.github.com> --- CHANGELOG.md | 3 ++- store/types/gas.go | 34 ++++++++++++++++++++++++++++++++++ store/types/gas_test.go | 8 ++++++++ 3 files changed, 44 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f38304d2d01d..0bc5902a837e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -128,8 +128,9 @@ if input key is empty, or input data contains empty key. ### Improvements +* (store) [\#9403](https://github.com/cosmos/cosmos-sdk/pull/9403) Add `RefundGas` function to `GasMeter` interface +* (baseapp, types) [\#9390](https://github.com/cosmos/cosmos-sdk/pull/9390) Add current block header hash to `Context` * (x/staking) [\#9423](https://github.com/cosmos/cosmos-sdk/pull/9423) Staking delegations now returns empty list instead of rpc error when no records found. -* (baseapp, types) [#\9390](https://github.com/cosmos/cosmos-sdk/pull/9390) Add current block header hash to `Context` * (x/bank) [\#8614](https://github.com/cosmos/cosmos-sdk/issues/8614) Add `Name` and `Symbol` fields to denom metadata * (x/auth) [\#8522](https://github.com/cosmos/cosmos-sdk/pull/8522) Allow to query all stored accounts * (crypto/types) [\#8600](https://github.com/cosmos/cosmos-sdk/pull/8600) `CompactBitArray`: optimize the `NumTrueBitsBefore` method and add an `Equal` method. diff --git a/store/types/gas.go b/store/types/gas.go index 028edc350ecd..bf44e717d095 100644 --- a/store/types/gas.go +++ b/store/types/gas.go @@ -20,6 +20,12 @@ const ( // Gas measured by the SDK type Gas = uint64 +// ErrorNegativeGasConsumed defines an error thrown when the amount of gas refunded results in a +// negative gas consumed amount. +type ErrorNegativeGasConsumed struct { + Descriptor string +} + // ErrorOutOfGas defines an error thrown when an action results in out of gas. type ErrorOutOfGas struct { Descriptor string @@ -37,6 +43,7 @@ type GasMeter interface { GasConsumedToLimit() Gas Limit() Gas ConsumeGas(amount Gas, descriptor string) + RefundGas(amount Gas, descriptor string) IsPastLimit() bool IsOutOfGas() bool String() string @@ -91,7 +98,20 @@ func (g *basicGasMeter) ConsumeGas(amount Gas, descriptor string) { if g.consumed > g.limit { panic(ErrorOutOfGas{descriptor}) } +} + +// RefundGas will deduct the given amount from the gas consumed. If the amount is greater than the +// gas consumed, the function will panic. +// +// Use case: This functionality enables refunding gas to the transaction or block gas pools so that +// EVM-compatible chains can fully support the go-ethereum StateDb interface. +// See https://github.com/cosmos/cosmos-sdk/pull/9403 for reference. +func (g *basicGasMeter) RefundGas(amount Gas, descriptor string) { + if g.consumed < amount { + panic(ErrorNegativeGasConsumed{Descriptor: descriptor}) + } + g.consumed -= amount } func (g *basicGasMeter) IsPastLimit() bool { @@ -138,6 +158,20 @@ func (g *infiniteGasMeter) ConsumeGas(amount Gas, descriptor string) { } } +// RefundGas will deduct the given amount from the gas consumed. If the amount is greater than the +// gas consumed, the function will panic. +// +// Use case: This functionality enables refunding gas to the trasaction or block gas pools so that +// EVM-compatible chains can fully support the go-ethereum StateDb interface. +// See https://github.com/cosmos/cosmos-sdk/pull/9403 for reference. +func (g *infiniteGasMeter) RefundGas(amount Gas, descriptor string) { + if g.consumed < amount { + panic(ErrorNegativeGasConsumed{Descriptor: descriptor}) + } + + g.consumed -= amount +} + func (g *infiniteGasMeter) IsPastLimit() bool { return false } diff --git a/store/types/gas_test.go b/store/types/gas_test.go index f4490747135b..8a02df4cfa18 100644 --- a/store/types/gas_test.go +++ b/store/types/gas_test.go @@ -16,10 +16,13 @@ func TestInfiniteGasMeter(t *testing.T) { meter.ConsumeGas(10, "consume 10") require.Equal(t, uint64(10), meter.GasConsumed()) require.Equal(t, uint64(10), meter.GasConsumedToLimit()) + meter.RefundGas(1, "refund 1") + require.Equal(t, uint64(9), meter.GasConsumed()) require.False(t, meter.IsPastLimit()) require.False(t, meter.IsOutOfGas()) meter.ConsumeGas(Gas(math.MaxUint64/2), "consume half max uint64") require.Panics(t, func() { meter.ConsumeGas(Gas(math.MaxUint64/2)+2, "panic") }) + require.Panics(t, func() { meter.RefundGas(meter.GasConsumed()+1, "refund greater than consumed") }) } func TestGasMeter(t *testing.T) { @@ -57,6 +60,11 @@ func TestGasMeter(t *testing.T) { require.Panics(t, func() { meter.ConsumeGas(1, "") }, "Exceeded but not panicked. tc #%d", tcnum) require.Equal(t, meter.GasConsumedToLimit(), meter.Limit(), "Gas consumption (to limit) not match limit") require.Equal(t, meter.GasConsumed(), meter.Limit()+1, "Gas consumption not match limit+1") + + require.NotPanics(t, func() { meter.RefundGas(1, "refund 1") }) + require.Equal(t, meter.GasConsumed(), meter.Limit(), "Gas consumption not match limit+1") + require.Panics(t, func() { meter.RefundGas(meter.GasConsumed()+1, "refund greater than consumed") }) + meter2 := NewGasMeter(math.MaxUint64) meter2.ConsumeGas(Gas(math.MaxUint64/2), "consume half max uint64") require.Panics(t, func() { meter2.ConsumeGas(Gas(math.MaxUint64/2)+2, "panic") })