Skip to content

Commit

Permalink
Estimate overhead required by authz execution (#687)
Browse files Browse the repository at this point in the history
# Description

# Reviewers checklist:
- [ ] Try to write more meaningful comments with clear actions to be taken.
- [ ] Nit-picking should be unblocking. Focus on core issues.

# Authors checklist
- [x] Provide a concise and meaningful description
- [x] Review the code yourself first, before making the PR.
- [x] Annotate your PR in places that require explanation.
- [x] Think and try to split the PR to smaller PR if it is big.
  • Loading branch information
wojtek-coreum authored Oct 26, 2023
1 parent d09f797 commit 6b189ce
Show file tree
Hide file tree
Showing 3 changed files with 85 additions and 7 deletions.
83 changes: 83 additions & 0 deletions integration-tests/modules/gas_estimation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,13 @@ package modules
import (
"fmt"
"testing"
"time"

sdkmath "cosmossdk.io/math"
sdk "github.com/cosmos/cosmos-sdk/types"
authztypes "github.com/cosmos/cosmos-sdk/x/authz"
banktypes "github.com/cosmos/cosmos-sdk/x/bank/types"
"github.com/samber/lo"
"github.com/stretchr/testify/require"

integrationtests "github.com/CoreumFoundation/coreum/v3/integration-tests"
Expand Down Expand Up @@ -114,3 +117,83 @@ func TestBankSendEstimation(t *testing.T) {
fmt.Printf("%d\t%d\n", n, txRes.GasUsed)
}
}

// TestAuthzEstimation it estimates gas overhead required by authz message execution.
// It executes regular message first. Then the same message is executed using authz. By subtracting those values
// we know what the overhead of authz is.
// To get correct results, both authz and bank send must be temporarily configured as non-deterministic messages,
// to get real results.
func TestAuthzEstimation(t *testing.T) {
t.Parallel()

ctx, chain := integrationtests.NewCoreumTestingContext(t)

requireT := require.New(t)

granter := chain.GenAccount()
grantee := chain.GenAccount()
recipient1 := chain.GenAccount()
recipient2 := chain.GenAccount()

chain.Faucet.FundAccounts(ctx, t,
integration.FundedAccount{
Address: granter,
Amount: chain.NewCoin(sdk.NewInt(50000000)),
},
integration.FundedAccount{
Address: grantee,
Amount: chain.NewCoin(sdk.NewInt(50000000)),
},
)

// grant the authorization
grantMsg, err := authztypes.NewMsgGrant(
granter,
grantee,
authztypes.NewGenericAuthorization(sdk.MsgTypeURL(&banktypes.MsgSend{})),
lo.ToPtr(time.Now().Add(time.Minute)),
)
require.NoError(t, err)

_, err = client.BroadcastTx(
ctx,
chain.ClientContext.WithFromAddress(granter),
chain.TxFactory().WithGas(chain.GasLimitByMsgs(grantMsg)),
grantMsg,
)
requireT.NoError(err)

// execute regular message
amountToSend := sdkmath.NewInt(2_000)
txf := chain.TxFactory().WithSimulateAndExecute(true)
resRegular, err := client.BroadcastTx(
ctx,
chain.ClientContext.WithFromAddress(granter),
txf,
&banktypes.MsgSend{
FromAddress: granter.String(),
ToAddress: recipient1.String(),
Amount: sdk.NewCoins(chain.NewCoin(amountToSend)),
},
)
requireT.NoError(err)

// execute authz message
execMsg := authztypes.NewMsgExec(grantee, []sdk.Msg{
&banktypes.MsgSend{
FromAddress: granter.String(),
ToAddress: recipient2.String(),
Amount: sdk.NewCoins(chain.NewCoin(amountToSend)),
},
})

resAuthZ, err := client.BroadcastTx(
ctx,
chain.ClientContext.WithFromAddress(grantee),
txf,
&execMsg,
)
requireT.NoError(err)

fmt.Printf("Authz gas overhead: %d\n", resAuthZ.GasUsed-resRegular.GasUsed)
}
7 changes: 1 addition & 6 deletions x/deterministicgas/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ import (
const (
BankSendPerCoinGas = 50000
BankMultiSendPerOperationsGas = 35000
AuthzExecOverhead = 2000
AuthzExecOverhead = 1500
)

type (
Expand Down Expand Up @@ -95,11 +95,6 @@ func DefaultConfig() Config {
MsgToMsgURL(&assetnfttypes.MsgRemoveFromClassWhitelist{}): constantGasFunc(3500),

// authz
// FIXME (v47-deterministic): We need a procedure to estimate the overhead of the authz. Proposal:
// 1. Estimate normal message
// 2. Estimate the same message executed using authz
// 3. Subtract one from the other
// We should have an integration test doing this.
MsgToMsgURL(&authz.MsgExec{}): cfg.authzMsgExecGasFunc(AuthzExecOverhead),
MsgToMsgURL(&authz.MsgGrant{}): constantGasFunc(28000),
MsgToMsgURL(&authz.MsgRevoke{}): constantGasFunc(8000),
Expand Down
2 changes: 1 addition & 1 deletion x/deterministicgas/spec/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ Real examples of special case tests could be found [here](https://github.com/Cor

`DeterministicGasForMsg = authzMsgExecOverhead + Sum(DeterministicGas(ChildMsg))`

`authzMsgExecOverhead` is currently equal to `2000`.
`authzMsgExecOverhead` is currently equal to `1500`.

### Nondeterministic messages

Expand Down

0 comments on commit 6b189ce

Please sign in to comment.