Skip to content

Commit

Permalink
fix review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
pythonberg1997 committed Jan 4, 2023
1 parent ba37304 commit 8ee9745
Show file tree
Hide file tree
Showing 13 changed files with 242 additions and 1,228 deletions.
26 changes: 4 additions & 22 deletions proto/cosmos/gashub/v1alpha1/gashub.proto
Original file line number Diff line number Diff line change
Expand Up @@ -12,26 +12,8 @@ message Params {

uint64 max_tx_size = 1 [(gogoproto.customname) = "MaxTxSize"];
uint64 min_gas_per_byte = 2 [(gogoproto.customname) = "MinGasPerByte"];
uint64 msg_grant_gas = 3 [(gogoproto.customname) = "MsgGrantGas"];
uint64 msg_revoke_gas = 4 [(gogoproto.customname) = "MsgRevokeGas"];
uint64 msg_exec_gas = 5 [(gogoproto.customname) = "MsgExecGas"];
uint64 msg_send_gas = 6 [(gogoproto.customname) = "MsgSendGas"];
uint64 msg_multi_send_gas = 7 [(gogoproto.customname) = "MsgMultiSendGas"];
uint64 msg_withdraw_delegator_reward_gas = 8 [(gogoproto.customname) = "MsgWithdrawDelegatorRewardGas"];
uint64 msg_withdraw_validator_commission_gas = 9 [(gogoproto.customname) = "MsgWithdrawValidatorCommissionGas"];
uint64 msg_set_withdraw_address_gas = 10 [(gogoproto.customname) = "MsgSetWithdrawAddressGas"];
uint64 msg_fund_community_pool_gas = 11 [(gogoproto.customname) = "MsgFundCommunityPoolGas"];
uint64 msg_grant_allowance_gas = 12 [(gogoproto.customname) = "MsgGrantAllowanceGas"];
uint64 msg_revoke_allowance_gas = 13 [(gogoproto.customname) = "MsgRevokeAllowanceGas"];
uint64 msg_submit_proposal_gas = 14 [(gogoproto.customname) = "MsgSubmitProposalGas"];
uint64 msg_vote_gas = 15 [(gogoproto.customname) = "MsgVoteGas"];
uint64 msg_vote_weighted_gas = 16 [(gogoproto.customname) = "MsgVoteWeightedGas"];
uint64 msg_deposit_gas = 17 [(gogoproto.customname) = "MsgDepositGas"];
uint64 msg_unjail_gas = 18 [(gogoproto.customname) = "MsgUnjailGas"];
uint64 msg_impeach_gas = 19 [(gogoproto.customname) = "MsgImpeachGas"];
uint64 msg_edit_validator_gas = 20 [(gogoproto.customname) = "MsgEditValidatorGas"];
uint64 msg_delegate_gas = 21 [(gogoproto.customname) = "MsgDelegateGas"];
uint64 msg_undelegate_gas = 22 [(gogoproto.customname) = "MsgUndelegateGas"];
uint64 msg_begin_redelegate_gas = 23 [(gogoproto.customname) = "MsgBeginRedelegateGas"];
uint64 msg_cancel_unbonding_delegation_gas = 24 [(gogoproto.customname) = "MsgCancelUnbondingDelegationGas"];
uint64 fixed_msg_gas = 3 [(gogoproto.customname) = "FixedMsgGas"];
uint64 msg_grant_per_item_gas = 4 [(gogoproto.customname) = "MsgGrantPerItemGas"];
uint64 msg_multi_send_per_item_gas = 5 [(gogoproto.customname) = "MsgMultiSendPerItemGas"];
uint64 msg_grant_allowance_per_item_gas = 6 [(gogoproto.customname) = "MsgGrantAllowancePerItemGas"];
}
11 changes: 7 additions & 4 deletions x/auth/ante/msg_gas.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import (
"fmt"

"cosmossdk.io/errors"

"github.com/cosmos/cosmos-sdk/codec/legacy"
cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types"
sdk "github.com/cosmos/cosmos-sdk/types"
Expand Down Expand Up @@ -113,7 +112,7 @@ func (cmfg ConsumeMsgGasDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simula
return ctx, err
}

if gasByTxSize >= gasByMsgType {
if gasByTxSize > gasByMsgType {
ctx.GasMeter().ConsumeGas(gasByTxSize, "tx bytes length")
} else {
ctx.GasMeter().ConsumeGas(gasByMsgType, "msg type")
Expand All @@ -124,14 +123,18 @@ func (cmfg ConsumeMsgGasDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simula

func (cmfg ConsumeMsgGasDecorator) getMsgGas(params types.Params, tx sdk.Tx) (uint64, error) {
msgs := tx.GetMsgs()
var totalGas uint64
totalGas := uint64(0)
for _, msg := range msgs {
feeCalcGen := types.GetGasCalculatorGen(sdk.MsgTypeURL(msg))
if feeCalcGen == nil {
return 0, fmt.Errorf("failed to find fee calculator")
}
feeCalc := feeCalcGen(params)
totalGas += feeCalc(msg)
gas, err := feeCalc(msg)
if err != nil {
return 0, err
}
totalGas = totalGas + gas
}
return totalGas, nil
}
Expand Down
2 changes: 1 addition & 1 deletion x/auth/ante/msg_gas_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ func (suite *AnteTestSuite) TestMsgGas() {
}
testCases := []testCase{
{"MsgSend", msgSend, 100000},
{"MsgMultiSend", msgMultiSend, 300000},
{"MsgMultiSend", msgMultiSend, 400000},
}
for _, tc := range testCases {
suite.txBuilder = suite.clientCtx.TxConfig.NewTxBuilder() // Create new txBuilder for each test
Expand Down
10 changes: 5 additions & 5 deletions x/authz/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,10 @@ import (
"github.com/cosmos/cosmos-sdk/x/authz"
)

// // TODO: Revisit this once we have propoer gas fee framework.
// // Tracking issues https://github.com/cosmos/cosmos-sdk/issues/9054,
// // https://github.com/cosmos/cosmos-sdk/discussions/9072
// const gasCostPerIteration = uint64(20)
// TODO: Revisit this once we have propoer gas fee framework.
// Tracking issues https://github.com/cosmos/cosmos-sdk/issues/9054,
// https://github.com/cosmos/cosmos-sdk/discussions/9072
const gasCostPerIteration = uint64(20)

type Keeper struct {
storeKey storetypes.StoreKey
Expand Down Expand Up @@ -359,7 +359,7 @@ func (keeper Keeper) removeFromGrantQueue(ctx sdk.Context, grantKey []byte, gran
queueItems := queueItem.MsgTypeUrls

for index, typeURL := range queueItems {
// ctx.GasMeter().ConsumeGas(gasCostPerIteration, "grant queue")
ctx.GasMeter().ConsumeGas(gasCostPerIteration, "grant queue")

if typeURL == msgType {
end := len(queueItem.MsgTypeUrls) - 1
Expand Down
2 changes: 1 addition & 1 deletion x/feegrant/fees.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import (
sdk "github.com/cosmos/cosmos-sdk/types"
)

// FeeAllowance implementations are tied to a given fee delegator and delegatee,
// FeeAllowanceI implementations are tied to a given fee delegator and delegatee,
// and are used to enforce fee grant limits.
type FeeAllowanceI interface {
// Accept can use fee payment requested as well as timestamp of the current block
Expand Down
16 changes: 8 additions & 8 deletions x/feegrant/filtered_fee.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,11 @@ import (
sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
)

// // TODO: Revisit this once we have propoer gas fee framework.
// // Tracking issues https://github.com/cosmos/cosmos-sdk/issues/9054, https://github.com/cosmos/cosmos-sdk/discussions/9072
// const (
// gasCostPerIteration = uint64(10)
// )
// TODO: Revisit this once we have propoer gas fee framework.
// Tracking issues https://github.com/cosmos/cosmos-sdk/issues/9054, https://github.com/cosmos/cosmos-sdk/discussions/9072
const (
gasCostPerIteration = uint64(10)
)

var (
_ FeeAllowanceI = (*AllowedMsgAllowance)(nil)
Expand All @@ -27,7 +27,7 @@ func (a *AllowedMsgAllowance) UnpackInterfaces(unpacker types.AnyUnpacker) error
return unpacker.UnpackAny(a.Allowance, &allowance)
}

// NewAllowedMsgFeeAllowance creates new filtered fee allowance.
// NewAllowedMsgAllowance creates new filtered fee allowance.
func NewAllowedMsgAllowance(allowance FeeAllowanceI, allowedMsgs []string) (*AllowedMsgAllowance, error) {
msg, ok := allowance.(proto.Message)
if !ok {
Expand Down Expand Up @@ -88,7 +88,7 @@ func (a *AllowedMsgAllowance) Accept(ctx sdk.Context, fee sdk.Coins, msgs []sdk.
func (a *AllowedMsgAllowance) allowedMsgsToMap(ctx sdk.Context) map[string]bool {
msgsMap := make(map[string]bool, len(a.AllowedMessages))
for _, msg := range a.AllowedMessages {
// ctx.GasMeter().ConsumeGas(gasCostPerIteration, "check msg")
ctx.GasMeter().ConsumeGas(gasCostPerIteration, "check msg")
msgsMap[msg] = true
}

Expand All @@ -99,7 +99,7 @@ func (a *AllowedMsgAllowance) allMsgTypesAllowed(ctx sdk.Context, msgs []sdk.Msg
msgsMap := a.allowedMsgsToMap(ctx)

for _, msg := range msgs {
// ctx.GasMeter().ConsumeGas(gasCostPerIteration, "check msg")
ctx.GasMeter().ConsumeGas(gasCostPerIteration, "check msg")
if !msgsMap[sdk.MsgTypeURL(msg)] {
return false
}
Expand Down
10 changes: 5 additions & 5 deletions x/gashub/simulation/genesis.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import (
const (
MaxTxSize = "max_tx_size"
MinGasPerByte = "min_gas_per_byte"
MsgGas = "msg_gas"
FixedMsgGas = "fixed_msg_gas"
)

// GenMaxTxSize randomized MaxTxSize
Expand Down Expand Up @@ -46,13 +46,13 @@ func RandomizedGenState(simState *module.SimulationState) {
func(r *rand.Rand) { minGasPerByte = GenMinGasPerByte(r) },
)

var msgGas uint64
var fixedMsgGas uint64
simState.AppParams.GetOrGenerate(
simState.Cdc, MsgGas, &msgGas, simState.Rand,
func(r *rand.Rand) { msgGas = GenMsgGas(r) },
simState.Cdc, FixedMsgGas, &fixedMsgGas, simState.Rand,
func(r *rand.Rand) { fixedMsgGas = GenMsgGas(r) },
)

params := types.NewParams(maxTxSize, minGasPerByte, msgGas, msgGas, msgGas, msgGas, msgGas, msgGas, msgGas, msgGas, msgGas, msgGas, msgGas, msgGas, msgGas, msgGas, msgGas, msgGas, msgGas, msgGas, msgGas, msgGas, msgGas, msgGas)
params := types.NewParams(maxTxSize, minGasPerByte, fixedMsgGas, fixedMsgGas, fixedMsgGas, fixedMsgGas)

gashubGenesis := types.NewGenesisState(params)

Expand Down
3 changes: 1 addition & 2 deletions x/gashub/simulation/genesis_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,5 @@ func TestRandomizedGenState(t *testing.T) {

require.Equal(t, uint64(2540), gashubGenesis.Params.MaxTxSize)
require.Equal(t, uint64(2956), gashubGenesis.Params.MinGasPerByte)
require.Equal(t, uint64(2803300), gashubGenesis.Params.MsgSendGas)
require.Equal(t, uint64(2803300), gashubGenesis.Params.MsgMultiSendGas)
require.Equal(t, uint64(2803300), gashubGenesis.Params.FixedMsgGas)
}
4 changes: 2 additions & 2 deletions x/gashub/simulation/params.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,14 @@ import (
)

const (
keyMsgSendGas = "MsgSendGas"
keyFixedMsgGas = "FixedMsgGas"
)

// ParamChanges defines the parameters that can be modified by param change proposals
// on the simulation
func ParamChanges(r *rand.Rand) []simtypes.ParamChange {
return []simtypes.ParamChange{
simulation.NewSimParamChange(types.ModuleName, keyMsgSendGas,
simulation.NewSimParamChange(types.ModuleName, keyFixedMsgGas,
func(r *rand.Rand) string {
return fmt.Sprintf("%d", GenMsgGas(r))
},
Expand Down
Loading

0 comments on commit 8ee9745

Please sign in to comment.