Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make MsgSubmitProposal a nondeterministic message #377

Merged
merged 8 commits into from
Feb 9, 2023
Merged
9 changes: 5 additions & 4 deletions integration-tests/chain.go
Original file line number Diff line number Diff line change
Expand Up @@ -121,9 +121,10 @@ func (c ChainContext) GasLimitByMultiSendMsgs(msgs ...sdk.Msg) uint64 {

// BalancesOptions is the input type for the ComputeNeededBalanceFromOptions.
type BalancesOptions struct {
Messages []sdk.Msg
GasPrice sdk.Dec
Amount sdk.Int
Messages []sdk.Msg
NondeterministicMessagesGas uint64
GasPrice sdk.Dec
Amount sdk.Int
}

// ComputeNeededBalanceFromOptions computes the required balance based on the input options.
Expand All @@ -148,7 +149,7 @@ func (c ChainContext) ComputeNeededBalanceFromOptions(options BalancesOptions) s
totalAmount = totalAmount.Add(amt)
}

return totalAmount.Add(options.Amount)
return totalAmount.Add(options.GasPrice.Mul(sdk.NewDec(int64(options.NondeterministicMessagesGas))).Ceil().RoundInt()).Add(options.Amount)
}

// ChainConfig defines the config arguments required for the test chain initialisation.
Expand Down
9 changes: 5 additions & 4 deletions integration-tests/gov.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ import (
"github.com/CoreumFoundation/coreum/testutil/event"
)

const submitProposalGas = 200_000

// Governance keep the test chain predefined account for the governance operations.
type Governance struct {
chainCtx ChainContext
Expand Down Expand Up @@ -53,8 +55,8 @@ func (g Governance) ComputeProposerBalance(ctx context.Context) (sdk.Coin, error

minDeposit := govParams.DepositParams.MinDeposit[0]
proposerInitialBalance := g.chainCtx.ComputeNeededBalanceFromOptions(BalancesOptions{
Messages: []sdk.Msg{&govtypes.MsgSubmitProposal{}},
Amount: minDeposit.Amount,
NondeterministicMessagesGas: submitProposalGas, // to cover MsgSubmitProposal
Amount: minDeposit.Amount,
})

return g.chainCtx.NewCoin(proposerInitialBalance), nil
Expand Down Expand Up @@ -125,8 +127,7 @@ func (g Governance) ProposeAndVote(ctx context.Context, proposer sdk.AccAddress,

// Propose creates a new proposal.
func (g Governance) Propose(ctx context.Context, msg *govtypes.MsgSubmitProposal) (uint64, error) {
txf := g.chainCtx.TxFactory().
WithGas(g.chainCtx.GasLimitByMsgs(&govtypes.MsgSubmitProposal{}))
txf := g.chainCtx.TxFactory().WithGas(submitProposalGas)
result, err := client.BroadcastTx(
ctx,
g.chainCtx.ClientContext.WithFromAddress(msg.GetProposer()),
Expand Down
5 changes: 3 additions & 2 deletions integration-tests/modules/bank_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,8 +80,9 @@ func TestBankMultiSendBatchOutputs(t *testing.T) {
}

requireT.NoError(chain.Faucet.FundAccountsWithOptions(ctx, issuer, integrationtests.BalancesOptions{
Messages: append([]sdk.Msg{issueMsg}, multiSendMsgs...),
Amount: chain.NetworkConfig.AssetFTConfig.IssueFee.Add(sdk.NewInt(10_000_000)), // add more coins to cover extra bytes because of the message size
Messages: append([]sdk.Msg{issueMsg}, multiSendMsgs...),
NondeterministicMessagesGas: 10_000_000, // to cover extra bytes because of the message size
Amount: chain.NetworkConfig.AssetFTConfig.IssueFee,
}))

// issue fungible tokens
Expand Down
1 change: 0 additions & 1 deletion integration-tests/modules/staking_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -367,7 +367,6 @@ func setUnbondingTimeViaGovernance(ctx context.Context, t *testing.T, chain inte
err = chain.Faucet.FundAccounts(ctx, integrationtests.NewFundedAccount(proposer, proposerBalance))
requireT.NoError(err)

// TODO(dhil) refactor other tests to use that func for the standard propose + vote action.
// Create proposition to change max the unbonding time value.
err = chain.Governance.ProposeAndVote(ctx, proposer,
paramproposal.NewParameterChangeProposal(
Expand Down
5 changes: 5 additions & 0 deletions pkg/client/tx.go
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,11 @@ func BroadcastTx(ctx context.Context, clientCtx Context, txf Factory, msgs ...sd
// CalculateGas simulates the execution of a transaction and returns the
// simulation response obtained by the query and the adjusted gas amount.
func CalculateGas(ctx context.Context, clientCtx Context, txf Factory, msgs ...sdk.Msg) (*sdktx.SimulateResponse, uint64, error) {
txf, err := prepareFactory(ctx, clientCtx, txf)
if err != nil {
return nil, 0, err
}

txBytes, err := tx.BuildSimTx(txf, msgs...)
if err != nil {
return nil, 0, err
Expand Down
14 changes: 10 additions & 4 deletions x/deterministicgas/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@ type Config struct {
}

// DefaultConfig returns default config for deterministic gas.
//
//nolint:funlen
func DefaultConfig() Config {
cfg := Config{
FixedGas: 50000,
Expand Down Expand Up @@ -84,10 +86,9 @@ func DefaultConfig() Config {
MsgType(&feegranttypes.MsgRevokeAllowance{}): constantGasFunc(2500),

// gov
MsgType(&govtypes.MsgSubmitProposal{}): constantGasFunc(65000),
MsgType(&govtypes.MsgVote{}): constantGasFunc(7000),
MsgType(&govtypes.MsgVoteWeighted{}): constantGasFunc(9000),
MsgType(&govtypes.MsgDeposit{}): constantGasFunc(52000),
MsgType(&govtypes.MsgVote{}): constantGasFunc(7000),
MsgType(&govtypes.MsgVoteWeighted{}): constantGasFunc(9000),
MsgType(&govtypes.MsgDeposit{}): constantGasFunc(52000),

// nft
MsgType(&nfttypes.MsgSend{}): constantGasFunc(16000),
Expand All @@ -113,6 +114,11 @@ func DefaultConfig() Config {
registerUndeterministicGasFuncs(
&cfg,
[]sdk.Msg{
// gov
// MsgSubmitProposal is defined as undeterministic because it runs a proposal handler function
// specific for each proposal and those functions consume unknown amount of gas.
&govtypes.MsgSubmitProposal{},

// crisis
// MsgVerifyInvariant is defined as undeterministic since fee
// charged by this tx type is defined as param inside module.
Expand Down
7 changes: 5 additions & 2 deletions x/deterministicgas/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,9 @@ func TestDeterministicGas_DeterministicMessages(t *testing.T) {

// WASM messages will be added here
undetermMsgTypes := []string{
// gov
"/cosmos.gov.v1beta1.MsgSubmitProposal",

// crisis
"/cosmos.crisis.v1beta1.MsgVerifyInvariant",

Expand Down Expand Up @@ -146,8 +149,8 @@ func TestDeterministicGas_DeterministicMessages(t *testing.T) {
// To make sure we do not increase/decrease deterministic types accidentally
// we assert length to be equal to exact number, so each change requires
// explicit adjustment of tests.
assert.Equal(t, 9, len(undetermMsgs))
assert.Equal(t, 40, len(determMsgs))
assert.Equal(t, 10, len(undetermMsgs))
assert.Equal(t, 39, len(determMsgs))

for _, sdkMsg := range determMsgs {
sdkMsg := sdkMsg
Expand Down