From 612c3de5273bf7aeb4bdeeab92fa843b756f5614 Mon Sep 17 00:00:00 2001 From: Nina Barbakadze Date: Thu, 1 Aug 2024 16:13:34 +0200 Subject: [PATCH 01/20] refactor: make versioned gas consts version specific --- app/ante/ante.go | 2 +- app/ante/tx_size.go | 149 +++++++++++++++++++ app/ante/tx_size_test.go | 190 ++++++++++++++++++++++++ app/app.go | 14 +- app/modules.go | 20 +++ pkg/appconsts/v3/app_consts.go | 7 + pkg/appconsts/versioned_consts.go | 10 ++ pkg/appconsts/versioned_consts_test.go | 68 +++++---- specs/src/specs/ante_handler_v1.md | 2 +- x/blob/ante/ante.go | 11 +- x/blob/ante/ante_test.go | 10 +- x/blob/keeper/gas_test.go | 37 +---- x/blob/keeper/genesis_test.go | 3 +- x/blob/keeper/grpc_query_params_test.go | 22 ++- x/blob/keeper/keeper.go | 11 +- x/blob/keeper/keeper_test.go | 68 +++++---- x/blob/keeper/params_test.go | 3 +- 17 files changed, 516 insertions(+), 111 deletions(-) create mode 100644 app/ante/tx_size.go create mode 100644 app/ante/tx_size_test.go create mode 100644 pkg/appconsts/v3/app_consts.go diff --git a/app/ante/ante.go b/app/ante/ante.go index dd65fdd120..89b6c1d050 100644 --- a/app/ante/ante.go +++ b/app/ante/ante.go @@ -42,7 +42,7 @@ func NewAnteHandler( ante.NewValidateMemoDecorator(accountKeeper), // Ensure the tx's gas limit is > the gas consumed based on the tx size. // Side effect: consumes gas from the gas meter. - ante.NewConsumeGasForTxSizeDecorator(accountKeeper), + NewConsumeGasForTxSizeDecorator(accountKeeper), // Ensure the feepayer (fee granter or first signer) has enough funds to pay for the tx. // Ensure the gas price >= network min gas price if app version >= 2. // Side effect: deducts fees from the fee payer. Sets the tx priority in context. diff --git a/app/ante/tx_size.go b/app/ante/tx_size.go new file mode 100644 index 0000000000..fa578d073a --- /dev/null +++ b/app/ante/tx_size.go @@ -0,0 +1,149 @@ +package ante + +import ( + "encoding/hex" + + "cosmossdk.io/errors" + "github.com/celestiaorg/celestia-app/v3/pkg/appconsts" + v2 "github.com/celestiaorg/celestia-app/v3/pkg/appconsts/v2" + v3 "github.com/celestiaorg/celestia-app/v3/pkg/appconsts/v3" + "github.com/cosmos/cosmos-sdk/codec/legacy" + "github.com/cosmos/cosmos-sdk/crypto/keys/multisig" + "github.com/cosmos/cosmos-sdk/crypto/keys/secp256k1" + cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types" + sdk "github.com/cosmos/cosmos-sdk/types" + sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" + "github.com/cosmos/cosmos-sdk/types/tx/signing" + ante "github.com/cosmos/cosmos-sdk/x/auth/ante" + "github.com/cosmos/cosmos-sdk/x/auth/migrations/legacytx" + authsigning "github.com/cosmos/cosmos-sdk/x/auth/signing" + auth "github.com/cosmos/cosmos-sdk/x/auth/types" +) + +// simulation signature values used to estimate gas consumption +var ( + // simulation signature values used to estimate gas consumption + key = make([]byte, secp256k1.PubKeySize) + simSecp256k1Pubkey = &secp256k1.PubKey{Key: key} + simSecp256k1Sig [64]byte + + _ authsigning.SigVerifiableTx = (*legacytx.StdTx)(nil) // assert StdTx implements SigVerifiableTx +) + +func init() { + // This decodes a valid hex string into a sepc256k1Pubkey for use in transaction simulation + bz, _ := hex.DecodeString("035AD6810A47F073553FF30D2FCC7E0D3B1C0B74B61A1AAA2582344037151E143A") + copy(key, bz) + simSecp256k1Pubkey.Key = key +} + +// ConsumeTxSizeGasDecorator will take in parameters and consume gas proportional +// to the size of tx before calling next AnteHandler. Note, the gas costs will be +// slightly over estimated due to the fact that any given signing account may need +// to be retrieved from state. +// +// CONTRACT: If simulate=true, then signatures must either be completely filled +// in or empty. +// CONTRACT: To use this decorator, signatures of transaction must be represented +// as legacytx.StdSignature otherwise simulate mode will incorrectly estimate gas cost. +type ConsumeTxSizeGasDecorator struct { + ak ante.AccountKeeper +} + +func NewConsumeGasForTxSizeDecorator(ak ante.AccountKeeper) ConsumeTxSizeGasDecorator { + return ConsumeTxSizeGasDecorator{ + ak: ak, + } +} + +func (cgts ConsumeTxSizeGasDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bool, next sdk.AnteHandler) (sdk.Context, error) { + sigTx, ok := tx.(authsigning.SigVerifiableTx) + if !ok { + return ctx, errors.Wrap(sdkerrors.ErrTxDecode, "invalid tx type") + } + params := cgts.ak.GetParams(ctx) + + consumeGasForTxSize(ctx, sdk.Gas(len(ctx.TxBytes())), params) + + // simulate gas cost for signatures in simulate mode + if simulate { + // in simulate mode, each element should be a nil signature + sigs, err := sigTx.GetSignaturesV2() + if err != nil { + return ctx, err + } + n := len(sigs) + + for i, signer := range sigTx.GetSigners() { + // if signature is already filled in, no need to simulate gas cost + if i < n && !isIncompleteSignature(sigs[i].Data) { + continue + } + + var pubkey cryptotypes.PubKey + + acc := cgts.ak.GetAccount(ctx, signer) + + // use placeholder simSecp256k1Pubkey if sig is nil + if acc == nil || acc.GetPubKey() == nil { + pubkey = simSecp256k1Pubkey + } else { + pubkey = acc.GetPubKey() + } + + // use stdsignature to mock the size of a full signature + simSig := legacytx.StdSignature{ //nolint:staticcheck // this will be removed when proto is ready + Signature: simSecp256k1Sig[:], + PubKey: pubkey, + } + + sigBz := legacy.Cdc.MustMarshal(simSig) + cost := sdk.Gas(len(sigBz) + 6) + + // If the pubkey is a multi-signature pubkey, then we estimate for the maximum + // number of signers. + if _, ok := pubkey.(*multisig.LegacyAminoPubKey); ok { + cost *= params.TxSigLimit + } + + consumeGasForTxSize(ctx, cost, params) + } + } + + return next(ctx, tx, simulate) +} + +// isIncompleteSignature tests whether SignatureData is fully filled in for simulation purposes +func isIncompleteSignature(data signing.SignatureData) bool { + if data == nil { + return true + } + + switch data := data.(type) { + case *signing.SingleSignatureData: + return len(data.Signature) == 0 + case *signing.MultiSignatureData: + if len(data.Signatures) == 0 { + return true + } + for _, s := range data.Signatures { + if isIncompleteSignature(s) { + return true + } + } + } + + return false +} + +func consumeGasForTxSize(ctx sdk.Context, cost uint64, params auth.Params) { + // For app v2 and below we should get txSizeCostPerByte from auth module + if ctx.BlockHeader().Version.App <= v2.Version { + // fmt.Println("HERE <= 2") + ctx.GasMeter().ConsumeGas(params.TxSizeCostPerByte*cost, "txSize") + } else { + // fmt.Println("HERE < 2") + // From v3 onwards, we should get txSizeCostPerByte from appconsts + ctx.GasMeter().ConsumeGas(appconsts.TxSizeCostPerByte(v3.Version)*cost, "txSize") + } +} diff --git a/app/ante/tx_size_test.go b/app/ante/tx_size_test.go new file mode 100644 index 0000000000..1676d16fa8 --- /dev/null +++ b/app/ante/tx_size_test.go @@ -0,0 +1,190 @@ +package ante_test + +import ( + "fmt" + "strings" + "testing" + + "github.com/celestiaorg/celestia-app/v3/app" + "github.com/celestiaorg/celestia-app/v3/app/ante" + testutil "github.com/celestiaorg/celestia-app/v3/test/util" + "github.com/cosmos/cosmos-sdk/client" + "github.com/cosmos/cosmos-sdk/client/tx" + cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types" + "github.com/cosmos/cosmos-sdk/crypto/types/multisig" + "github.com/cosmos/cosmos-sdk/simapp" + "github.com/cosmos/cosmos-sdk/testutil/testdata" + sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/cosmos/cosmos-sdk/types/tx/signing" + authante "github.com/cosmos/cosmos-sdk/x/auth/ante" + xauthsigning "github.com/cosmos/cosmos-sdk/x/auth/signing" + authtypes "github.com/cosmos/cosmos-sdk/x/auth/types" + require "github.com/stretchr/testify/require" + tmproto "github.com/tendermint/tendermint/proto/tendermint/types" +) + +func setup() (*app.App, sdk.Context, client.Context, sdk.AnteHandler, error) { + app, _, _ := testutil.NewTestAppWithGenesisSet(app.DefaultConsensusParams()) + ctx := app.NewContext(false, tmproto.Header{}) + app.AccountKeeper.SetParams(ctx, authtypes.DefaultParams()) + ctx = ctx.WithBlockHeight(1) + + // Set up TxConfig. + encodingConfig := simapp.MakeTestEncodingConfig() + // We're using TestMsg encoding in some tests, so register it here. + encodingConfig.Amino.RegisterConcrete(&testdata.TestMsg{}, "testdata.TestMsg", nil) + testdata.RegisterInterfaces(encodingConfig.InterfaceRegistry) + + clientCtx := client.Context{}. + WithTxConfig(encodingConfig.TxConfig) + + anteHandler, err := authante.NewAnteHandler( + authante.HandlerOptions{ + AccountKeeper: app.AccountKeeper, + BankKeeper: app.BankKeeper, + FeegrantKeeper: app.FeeGrantKeeper, + SignModeHandler: encodingConfig.TxConfig.SignModeHandler(), + SigGasConsumer: authante.DefaultSigVerificationGasConsumer, + }, + ) + if err != nil { + return nil, sdk.Context{}, client.Context{}, nil, fmt.Errorf("error creating AnteHandler: %v", err) + } + + return app, ctx, clientCtx, anteHandler, nil +} + +func TestConsumeGasForTxSize(t *testing.T) { + app, ctx, clientCtx, _, err := setup() + require.NoError(t, err) + sub, exs := app.ParamsKeeper.GetSubspace(authtypes.ModuleName) + fmt.Println(sub, exs) + var txBuilder client.TxBuilder + + // keys and addresses + priv1, _, addr1 := testdata.KeyTestPubAddr() + + // msg and signatures + msg := testdata.NewTestMsg(addr1) + feeAmount := testdata.NewTestFeeAmount() + gasLimit := testdata.NewTestGasLimit() + + cgtsd := ante.NewConsumeGasForTxSizeDecorator(app.AccountKeeper) + antehandler := sdk.ChainAnteDecorators(cgtsd) + + testCases := []struct { + name string + sigV2 signing.SignatureV2 + }{ + {"SingleSignatureData", signing.SignatureV2{PubKey: priv1.PubKey()}}, + {"MultiSignatureData", signing.SignatureV2{PubKey: priv1.PubKey(), Data: multisig.NewMultisig(2)}}, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + txBuilder = clientCtx.TxConfig.NewTxBuilder() + require.NoError(t, txBuilder.SetMsgs(msg)) + txBuilder.SetFeeAmount(feeAmount) + txBuilder.SetGasLimit(gasLimit) + txBuilder.SetMemo(strings.Repeat("01234567890", 10)) + + privs, accNums, accSeqs := []cryptotypes.PrivKey{priv1}, []uint64{0}, []uint64{0} + tx, err := CreateTestTx(txBuilder, clientCtx, privs, accNums, accSeqs, ctx.ChainID()) + require.NoError(t, err) + + txBytes, err := clientCtx.TxConfig.TxJSONEncoder()(tx) + require.Nil(t, err, "Cannot marshal tx: %v", err) + + params := app.AccountKeeper.GetParams(ctx) + expectedGas := sdk.Gas(len(txBytes)) * params.TxSizeCostPerByte + + // Set suite.ctx with TxBytes manually + ctx = ctx.WithTxBytes(txBytes) + + // track how much gas is necessary to retrieve parameters + beforeGas := ctx.GasMeter().GasConsumed() + app.AccountKeeper.GetParams(ctx) + afterGas := ctx.GasMeter().GasConsumed() + expectedGas += afterGas - beforeGas + + beforeGas = ctx.GasMeter().GasConsumed() + ctx, err = antehandler(ctx, tx, false) + require.Nil(t, err, "ConsumeTxSizeGasDecorator returned error: %v", err) + + // require that decorator consumes expected amount of gas + consumedGas := ctx.GasMeter().GasConsumed() - beforeGas + require.Equal(t, expectedGas, consumedGas, "Decorator did not consume the correct amount of gas") + + // simulation must not underestimate gas of this decorator even with nil signatures + txBuilder, err := clientCtx.TxConfig.WrapTxBuilder(tx) + require.NoError(t, err) + require.NoError(t, txBuilder.SetSignatures(tc.sigV2)) + tx = txBuilder.GetTx() + + simTxBytes, err := clientCtx.TxConfig.TxJSONEncoder()(tx) + require.Nil(t, err, "Cannot marshal tx: %v", err) + // require that simulated tx is smaller than tx with signatures + require.True(t, len(simTxBytes) < len(txBytes), "simulated tx still has signatures") + + // Set suite.ctx with smaller simulated TxBytes manually + ctx = ctx.WithTxBytes(simTxBytes) + + beforeSimGas := ctx.GasMeter().GasConsumed() + + // run antehandler with simulate=true + ctx, err = antehandler(ctx, tx, true) + consumedSimGas := ctx.GasMeter().GasConsumed() - beforeSimGas + + // require that antehandler passes and does not underestimate decorator cost + require.Nil(t, err, "ConsumeTxSizeGasDecorator returned error: %v", err) + require.True(t, consumedSimGas >= expectedGas, "Simulate mode underestimates gas on AnteDecorator. Simulated cost: %d, expected cost: %d", consumedSimGas, expectedGas) + }) + } +} + +// CreateTestTx is a helper function to create a tx given multiple inputs. +func CreateTestTx(txBuilder client.TxBuilder, clientCtx client.Context, privs []cryptotypes.PrivKey, accNums []uint64, accSeqs []uint64, chainID string) (xauthsigning.Tx, error) { + // First round: we gather all the signer infos. We use the "set empty + // signature" hack to do that. + sigsV2 := make([]signing.SignatureV2, 0, len(privs)) + for i, priv := range privs { + sigV2 := signing.SignatureV2{ + PubKey: priv.PubKey(), + Data: &signing.SingleSignatureData{ + SignMode: clientCtx.TxConfig.SignModeHandler().DefaultMode(), + Signature: nil, + }, + Sequence: accSeqs[i], + } + + sigsV2 = append(sigsV2, sigV2) + } + err := txBuilder.SetSignatures(sigsV2...) + if err != nil { + return nil, err + } + + // Second round: all signer infos are set, so each signer can sign. + sigsV2 = []signing.SignatureV2{} + for i, priv := range privs { + signerData := xauthsigning.SignerData{ + ChainID: chainID, + AccountNumber: accNums[i], + Sequence: accSeqs[i], + } + sigV2, err := tx.SignWithPrivKey( + clientCtx.TxConfig.SignModeHandler().DefaultMode(), signerData, + txBuilder, priv, clientCtx.TxConfig, accSeqs[i]) + if err != nil { + return nil, err + } + + sigsV2 = append(sigsV2, sigV2) + } + err = txBuilder.SetSignatures(sigsV2...) + if err != nil { + return nil, err + } + + return txBuilder.GetTx(), nil +} diff --git a/app/app.go b/app/app.go index 2de17f779d..230496f918 100644 --- a/app/app.go +++ b/app/app.go @@ -11,6 +11,7 @@ import ( "github.com/celestiaorg/celestia-app/v3/app/posthandler" appv1 "github.com/celestiaorg/celestia-app/v3/pkg/appconsts/v1" appv2 "github.com/celestiaorg/celestia-app/v3/pkg/appconsts/v2" + appv3 "github.com/celestiaorg/celestia-app/v3/pkg/appconsts/v3" "github.com/celestiaorg/celestia-app/v3/pkg/proof" blobkeeper "github.com/celestiaorg/celestia-app/v3/x/blob/keeper" blobtypes "github.com/celestiaorg/celestia-app/v3/x/blob/types" @@ -107,6 +108,7 @@ var maccPerms = map[string][]string{ const ( v1 = appv1.Version v2 = appv2.Version + v3 = appv3.Version DefaultInitialVersion = v1 ) @@ -165,7 +167,7 @@ type App struct { configurator module.Configurator // upgradeHeightV2 is used as a coordination mechanism for the height-based // upgrade from v1 to v2. - upgradeHeightV2 int64 + upgradeHeightV3 int64 // MsgGateKeeper is used to define which messages are accepted for a given // app version. MsgGateKeeper *ante.MsgVersioningGateKeeper @@ -183,7 +185,7 @@ func New( traceStore io.Writer, invCheckPeriod uint, encodingConfig encoding.Config, - upgradeHeightV2 int64, + upgradeHeightV3 int64, appOpts servertypes.AppOptions, baseAppOptions ...func(*baseapp.BaseApp), ) *App { @@ -209,7 +211,7 @@ func New( keys: keys, tkeys: tkeys, memKeys: memKeys, - upgradeHeightV2: upgradeHeightV2, + upgradeHeightV3: upgradeHeightV3, } app.ParamsKeeper = initParamsKeeper(appCodec, encodingConfig.Amino, keys[paramstypes.StoreKey], tkeys[paramstypes.TStoreKey]) @@ -444,7 +446,7 @@ func (app *App) Name() string { return app.BaseApp.Name() } // BeginBlocker application updates every begin block func (app *App) BeginBlocker(ctx sdk.Context, req abci.RequestBeginBlock) abci.ResponseBeginBlock { - if req.Header.Height == app.upgradeHeightV2 { + if req.Header.Height == app.upgradeHeightV3 { app.BaseApp.Logger().Info("upgraded from app version 1 to 2") } return app.manager.BeginBlock(ctx, req) @@ -457,8 +459,8 @@ func (app *App) EndBlocker(ctx sdk.Context, req abci.RequestEndBlock) abci.Respo // For v1 only we upgrade using a agreed upon height known ahead of time if currentVersion == v1 { // check that we are at the height before the upgrade - if req.Height == app.upgradeHeightV2-1 { - app.BaseApp.Logger().Info(fmt.Sprintf("upgrading from app version %v to 2", currentVersion)) + if req.Height == app.upgradeHeightV3-1 { + app.BaseApp.Logger().Info(fmt.Sprintf("upgrading from app version %v to 3", currentVersion)) app.SetInitialAppVersionInConsensusParams(ctx, v2) app.SetAppVersion(ctx, v2) diff --git a/app/modules.go b/app/modules.go index a89f418205..ea18c66a9e 100644 --- a/app/modules.go +++ b/app/modules.go @@ -341,6 +341,26 @@ func versionedStoreKeys() map[uint64][]string { stakingtypes.StoreKey, upgradetypes.StoreKey, }, + // 3: { + // authtypes.StoreKey, + // authzkeeper.StoreKey, + // banktypes.StoreKey, + // blobtypes.StoreKey, + // capabilitytypes.StoreKey, + // distrtypes.StoreKey, + // evidencetypes.StoreKey, + // feegrant.StoreKey, + // govtypes.StoreKey, + // ibchost.StoreKey, + // ibctransfertypes.StoreKey, + // icahosttypes.StoreKey, // added in v2 + // minttypes.StoreKey, + // packetforwardtypes.StoreKey, // added in v2 + // signaltypes.StoreKey, // added in v2 + // slashingtypes.StoreKey, + // stakingtypes.StoreKey, + // upgradetypes.StoreKey, + // }, } } diff --git a/pkg/appconsts/v3/app_consts.go b/pkg/appconsts/v3/app_consts.go new file mode 100644 index 0000000000..f8ba21bad5 --- /dev/null +++ b/pkg/appconsts/v3/app_consts.go @@ -0,0 +1,7 @@ +package v3 + +const ( + Version uint64 = 3 + TxSizeCostPerByte uint64 = 10 + GasPerBlobByte uint32 = 8 +) diff --git a/pkg/appconsts/versioned_consts.go b/pkg/appconsts/versioned_consts.go index 67c3c8a8f2..8721b8e229 100644 --- a/pkg/appconsts/versioned_consts.go +++ b/pkg/appconsts/versioned_consts.go @@ -3,6 +3,7 @@ package appconsts import ( v1 "github.com/celestiaorg/celestia-app/v3/pkg/appconsts/v1" v2 "github.com/celestiaorg/celestia-app/v3/pkg/appconsts/v2" + v3 "github.com/celestiaorg/celestia-app/v3/pkg/appconsts/v3" ) const ( @@ -26,7 +27,16 @@ func SquareSizeUpperBound(_ uint64) int { return v1.SquareSizeUpperBound } +func TxSizeCostPerByte(_ uint64) uint64 { + return v3.TxSizeCostPerByte +} + +func GasPerBlobByte(_ uint64) uint32 { + return v3.GasPerBlobByte +} + var ( DefaultSubtreeRootThreshold = SubtreeRootThreshold(LatestVersion) DefaultSquareSizeUpperBound = SquareSizeUpperBound(LatestVersion) + DefaultTxSizeCostPerByte = TxSizeCostPerByte(LatestVersion) ) diff --git a/pkg/appconsts/versioned_consts_test.go b/pkg/appconsts/versioned_consts_test.go index 6fb5cfc48d..fdfff95521 100644 --- a/pkg/appconsts/versioned_consts_test.go +++ b/pkg/appconsts/versioned_consts_test.go @@ -1,7 +1,6 @@ package appconsts_test import ( - "fmt" "testing" "github.com/stretchr/testify/require" @@ -9,52 +8,57 @@ import ( "github.com/celestiaorg/celestia-app/v3/pkg/appconsts" v1 "github.com/celestiaorg/celestia-app/v3/pkg/appconsts/v1" v2 "github.com/celestiaorg/celestia-app/v3/pkg/appconsts/v2" + v3 "github.com/celestiaorg/celestia-app/v3/pkg/appconsts/v3" ) -func TestSubtreeRootThreshold(t *testing.T) { +func TestVersionedConsts(t *testing.T) { testCases := []struct { - version uint64 - expected int + name string + version uint64 + expectedConstant interface{} + got interface{} }{ { - version: v1.Version, - expected: v1.SubtreeRootThreshold, + name: "SubtreeRootThreshold v1", + version: v1.Version, + expectedConstant: v1.SubtreeRootThreshold, + got: appconsts.SubtreeRootThreshold(v1.Version), }, { - version: v2.Version, - expected: v2.SubtreeRootThreshold, + name: "SubtreeRootThreshold v2", + version: v2.Version, + expectedConstant: v2.SubtreeRootThreshold, + got: appconsts.SubtreeRootThreshold(v2.Version), + }, + { + name: "SquareSizeUpperBound v1", + version: v1.Version, + expectedConstant: v1.SquareSizeUpperBound, + got: appconsts.SquareSizeUpperBound(v1.Version), + }, + { + name: "SquareSizeUpperBound v2", + version: v2.Version, + expectedConstant: v2.SquareSizeUpperBound, + got: appconsts.SquareSizeUpperBound(v2.Version), }, - } - - for _, tc := range testCases { - name := fmt.Sprintf("version %v", tc.version) - t.Run(name, func(t *testing.T) { - got := appconsts.SubtreeRootThreshold(tc.version) - require.Equal(t, tc.expected, got) - }) - } -} - -func TestSquareSizeUpperBound(t *testing.T) { - testCases := []struct { - version uint64 - expected int - }{ { - version: v1.Version, - expected: v1.SquareSizeUpperBound, + name: "TxSizeCostPerByte v3", + version: v3.Version, + expectedConstant: v3.TxSizeCostPerByte, + got: appconsts.TxSizeCostPerByte(v3.Version), }, { - version: v2.Version, - expected: v2.SquareSizeUpperBound, + name: "GasPerBlobByte v3", + version: v3.Version, + expectedConstant: v3.GasPerBlobByte, + got: appconsts.GasPerBlobByte(v3.Version), }, } for _, tc := range testCases { - name := fmt.Sprintf("version %v", tc.version) - t.Run(name, func(t *testing.T) { - got := appconsts.SquareSizeUpperBound(tc.version) - require.Equal(t, tc.expected, got) + t.Run(tc.name, func(t *testing.T) { + require.Equal(t, tc.expectedConstant, tc.got) }) } } diff --git a/specs/src/specs/ante_handler_v1.md b/specs/src/specs/ante_handler_v1.md index 52109f67f8..3070d2a92c 100644 --- a/specs/src/specs/ante_handler_v1.md +++ b/specs/src/specs/ante_handler_v1.md @@ -11,7 +11,7 @@ The AnteHandler chains together several decorators to ensure the following crite - The tx's count of signatures <= the max number of signatures. The max number of signatures is [`TxSigLimit = 7`](https://github.com/cosmos/cosmos-sdk/blob/a429238fc267da88a8548bfebe0ba7fb28b82a13/x/auth/README.md?plain=1#L231). - The tx's [gas_limit](https://github.com/cosmos/cosmos-sdk/blob/22c28366466e64ebf0df1ce5bec8b1130523552c/proto/cosmos/tx/v1beta1/tx.proto#L211-L213) is > the gas consumed based on the tx's signatures. - The tx's [signatures](https://github.com/cosmos/cosmos-sdk/blob/22c28366466e64ebf0df1ce5bec8b1130523552c/types/tx/signing/signature.go#L10-L26) are valid. For each signature, ensure that the signature's sequence number (a.k.a nonce) matches the account sequence number of the signer. -- The tx's [gas_limit](https://github.com/cosmos/cosmos-sdk/blob/22c28366466e64ebf0df1ce5bec8b1130523552c/proto/cosmos/tx/v1beta1/tx.proto#L211-L213) is > the gas consumed based on the blob size(s). Since blobs are charged based on the number of shares they occupy, the gas consumed is calculated as follows: `gasToConsume = sharesNeeded(blob) * bytesPerShare * gasPerBlobByte`. Where `bytesPerShare` is a global constant (an alias for [`ShareSize = 512`](https://github.com/celestiaorg/celestia-app/blob/c90e61d5a2d0c0bd0e123df4ab416f6f0d141b7f/pkg/appconsts/global_consts.go#L27-L28)) and `gasPerBlobByte` is a governance parameter that can be modified (the [`DefaultGasPerBlobByte = 8`](https://github.com/celestiaorg/celestia-app/blob/c90e61d5a2d0c0bd0e123df4ab416f6f0d141b7f/pkg/appconsts/initial_consts.go#L16-L18)). +- The tx's [gas_limit](https://github.com/cosmos/cosmos-sdk/blob/22c28366466e64ebf0df1ce5bec8b1130523552c/proto/cosmos/tx/v1beta1/tx.proto#L211-L213) is > the gas consumed based on the blob size(s). Since blobs are charged based on the number of shares they occupy, the gas consumed is calculated as follows: `gasToConsume = sharesNeeded(blob) * bytesPerShare * gasPerBlobByte`. Where `bytesPerShare` is a global constant (an alias for [`ShareSize = 512`](https://github.com/celestiaorg/celestia-app/blob/c90e61d5a2d0c0bd0e123df4ab416f6f0d141b7f/pkg/appconsts/global_consts.go#L27-L28)) and `gasPerBlobByte` is an application constant that can be modified through hard fork (the [`DefaultGasPerBlobByte = 8`](https://github.com/celestiaorg/celestia-app/blob/c90e61d5a2d0c0bd0e123df4ab416f6f0d141b7f/pkg/appconsts/initial_consts.go#L16-L18)). - The tx's total blob size is <= the max blob size. The max blob size is derived from the maximum valid square size. The max valid square size is the minimum of: `GovMaxSquareSize` and `SquareSizeUpperBound`. - The tx does not contain a message of type [MsgSubmitProposal](https://github.com/cosmos/cosmos-sdk/blob/d6d929843bbd331b885467475bcb3050788e30ca/proto/cosmos/gov/v1/tx.proto#L33-L43) with zero proposal messages. - The tx is not an IBC packet or update message that has already been processed. diff --git a/x/blob/ante/ante.go b/x/blob/ante/ante.go index fe58eae87c..4906eec386 100644 --- a/x/blob/ante/ante.go +++ b/x/blob/ante/ante.go @@ -1,6 +1,9 @@ package ante import ( + "github.com/celestiaorg/celestia-app/v3/pkg/appconsts" + v2 "github.com/celestiaorg/celestia-app/v3/pkg/appconsts/v2" + v3 "github.com/celestiaorg/celestia-app/v3/pkg/appconsts/v3" "github.com/celestiaorg/celestia-app/v3/x/blob/types" "cosmossdk.io/errors" @@ -33,8 +36,12 @@ func (d MinGasPFBDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bool // NOTE: here we assume only one PFB per transaction if pfb, ok := m.(*types.MsgPayForBlobs); ok { if gasPerByte == 0 { - // lazily fetch the gas per byte param - gasPerByte = d.k.GasPerBlobByte(ctx) + if ctx.BlockHeader().Version.App <= v2.Version { + // lazily fetch the gas per byte param + gasPerByte = d.k.GasPerBlobByte(ctx) + } else { + gasPerByte = appconsts.GasPerBlobByte(v3.Version) + } } gasToConsume := pfb.Gas(gasPerByte) if gasToConsume > txGas { diff --git a/x/blob/ante/ante_test.go b/x/blob/ante/ante_test.go index 0f7bf39b02..48b0018d6d 100644 --- a/x/blob/ante/ante_test.go +++ b/x/blob/ante/ante_test.go @@ -6,11 +6,14 @@ import ( "github.com/celestiaorg/celestia-app/v3/app" "github.com/celestiaorg/celestia-app/v3/app/encoding" "github.com/celestiaorg/celestia-app/v3/pkg/appconsts" + v1 "github.com/celestiaorg/celestia-app/v3/pkg/appconsts/v1" ante "github.com/celestiaorg/celestia-app/v3/x/blob/ante" blob "github.com/celestiaorg/celestia-app/v3/x/blob/types" "github.com/celestiaorg/go-square/shares" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/stretchr/testify/require" + tmproto "github.com/tendermint/tendermint/proto/tendermint/types" + "github.com/tendermint/tendermint/proto/tendermint/version" ) const ( @@ -89,7 +92,12 @@ func TestPFBAnteHandler(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { anteHandler := ante.NewMinGasPFBDecorator(mockBlobKeeper{}) - ctx := sdk.Context{}.WithGasMeter(sdk.NewGasMeter(tc.txGas)).WithIsCheckTx(true) + ctx := sdk.NewContext(nil, tmproto.Header{ + Version: version.Consensus{ + App: v1.Version, + }, + }, true, nil).WithGasMeter(sdk.NewGasMeter(tc.txGas)) + ctx.GasMeter().ConsumeGas(tc.gasConsumed, "test") txBuilder := txConfig.NewTxBuilder() require.NoError(t, txBuilder.SetMsgs(tc.pfb)) diff --git a/x/blob/keeper/gas_test.go b/x/blob/keeper/gas_test.go index 8c3b6c1c8c..c6bf07b75f 100644 --- a/x/blob/keeper/gas_test.go +++ b/x/blob/keeper/gas_test.go @@ -4,6 +4,7 @@ import ( "testing" "github.com/celestiaorg/celestia-app/v3/pkg/appconsts" + v3 "github.com/celestiaorg/celestia-app/v3/pkg/appconsts/v3" "github.com/celestiaorg/celestia-app/v3/x/blob/types" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/stretchr/testify/require" @@ -23,33 +24,33 @@ func TestPayForBlobGas(t *testing.T) { { name: "1 byte blob", // occupies 1 share msg: types.MsgPayForBlobs{BlobSizes: []uint32{1}}, - wantGasConsumed: uint64(1*appconsts.ShareSize*types.DefaultGasPerBlobByte + paramLookUpCost), // 1 share * 512 bytes per share * 8 gas per byte + 1060 gas for fetching param = 5156 gas + wantGasConsumed: uint64(1*appconsts.ShareSize*appconsts.GasPerBlobByte(v3.Version) + paramLookUpCost), // 1 share * 512 bytes per share * 8 gas per byte + 1060 gas for fetching param = 5156 gas }, { name: "100 byte blob", // occupies 1 share msg: types.MsgPayForBlobs{BlobSizes: []uint32{100}}, - wantGasConsumed: uint64(1*appconsts.ShareSize*types.DefaultGasPerBlobByte + paramLookUpCost), + wantGasConsumed: uint64(1*appconsts.ShareSize*appconsts.GasPerBlobByte(v3.Version) + paramLookUpCost), }, { name: "1024 byte blob", // occupies 3 shares because share prefix (e.g. namespace, info byte) msg: types.MsgPayForBlobs{BlobSizes: []uint32{1024}}, - wantGasConsumed: uint64(3*appconsts.ShareSize*types.DefaultGasPerBlobByte + paramLookUpCost), // 3 shares * 512 bytes per share * 8 gas per byte + 1060 gas for fetching param = 13348 gas + wantGasConsumed: uint64(3*appconsts.ShareSize*appconsts.GasPerBlobByte(v3.Version) + paramLookUpCost), // 3 shares * 512 bytes per share * 8 gas per byte + 1060 gas for fetching param = 13348 gas }, { name: "3 blobs, 1 share each", msg: types.MsgPayForBlobs{BlobSizes: []uint32{1, 1, 1}}, - wantGasConsumed: uint64(3*appconsts.ShareSize*types.DefaultGasPerBlobByte + paramLookUpCost), // 3 shares * 512 bytes per share * 8 gas per byte + 1060 gas for fetching param = 13348 gas + wantGasConsumed: uint64(3*appconsts.ShareSize*appconsts.GasPerBlobByte(v3.Version) + paramLookUpCost), // 3 shares * 512 bytes per share * 8 gas per byte + 1060 gas for fetching param = 13348 gas }, { name: "3 blobs, 6 shares total", msg: types.MsgPayForBlobs{BlobSizes: []uint32{1024, 800, 100}}, - wantGasConsumed: uint64(6*appconsts.ShareSize*types.DefaultGasPerBlobByte + paramLookUpCost), // 6 shares * 512 bytes per share * 8 gas per byte + 1060 gas for fetching param = 25636 gas + wantGasConsumed: uint64(6*appconsts.ShareSize*appconsts.GasPerBlobByte(v3.Version) + paramLookUpCost), // 6 shares * 512 bytes per share * 8 gas per byte + 1060 gas for fetching param = 25636 gas }, } for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - k, stateStore, _ := CreateKeeper(t) + k, stateStore, _ := CreateKeeper(t, appconsts.LatestVersion) ctx := sdk.NewContext(stateStore, tmproto.Header{}, false, nil) _, err := k.PayForBlobs(sdk.WrapSDKContext(ctx), &tc.msg) require.NoError(t, err) @@ -59,27 +60,3 @@ func TestPayForBlobGas(t *testing.T) { }) } } - -func TestChangingGasParam(t *testing.T) { - msg := types.MsgPayForBlobs{BlobSizes: []uint32{1024}} - k, stateStore, _ := CreateKeeper(t) - tempCtx := sdk.NewContext(stateStore, tmproto.Header{}, false, nil) - - ctx1 := sdk.NewContext(stateStore, tmproto.Header{}, false, nil) - _, err := k.PayForBlobs(sdk.WrapSDKContext(ctx1), &msg) - require.NoError(t, err) - - params := k.GetParams(tempCtx) - params.GasPerBlobByte++ - k.SetParams(tempCtx, params) - - ctx2 := sdk.NewContext(stateStore, tmproto.Header{}, false, nil) - _, err = k.PayForBlobs(sdk.WrapSDKContext(ctx2), &msg) - require.NoError(t, err) - - if ctx1.GasMeter().GasConsumed() >= ctx2.GasMeter().GasConsumedToLimit() { - t.Errorf("Gas consumed was not increased upon incrementing param, before: %d, after: %d", - ctx1.GasMeter().GasConsumed(), ctx2.GasMeter().GasConsumedToLimit(), - ) - } -} diff --git a/x/blob/keeper/genesis_test.go b/x/blob/keeper/genesis_test.go index d291deebf8..120f9cb21e 100644 --- a/x/blob/keeper/genesis_test.go +++ b/x/blob/keeper/genesis_test.go @@ -3,6 +3,7 @@ package keeper_test import ( "testing" + "github.com/celestiaorg/celestia-app/v3/pkg/appconsts" "github.com/celestiaorg/celestia-app/v3/x/blob" "github.com/celestiaorg/celestia-app/v3/x/blob/types" "github.com/stretchr/testify/require" @@ -13,7 +14,7 @@ func TestGenesis(t *testing.T) { Params: types.DefaultParams(), } - k, _, ctx := CreateKeeper(t) + k, _, ctx := CreateKeeper(t, appconsts.LatestVersion) blob.InitGenesis(ctx, *k, genesisState) got := blob.ExportGenesis(ctx, *k) require.NotNil(t, got) diff --git a/x/blob/keeper/grpc_query_params_test.go b/x/blob/keeper/grpc_query_params_test.go index 70f60860ee..2279e79811 100644 --- a/x/blob/keeper/grpc_query_params_test.go +++ b/x/blob/keeper/grpc_query_params_test.go @@ -1,20 +1,28 @@ package keeper_test import ( + "fmt" "testing" + v1 "github.com/celestiaorg/celestia-app/v3/pkg/appconsts/v1" + v2 "github.com/celestiaorg/celestia-app/v3/pkg/appconsts/v2" "github.com/celestiaorg/celestia-app/v3/x/blob/types" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/stretchr/testify/require" ) func TestParamsQuery(t *testing.T) { - keeper, _, ctx := CreateKeeper(t) - wctx := sdk.WrapSDKContext(ctx) - params := types.DefaultParams() - keeper.SetParams(ctx, params) + versions := []uint64{v1.Version, v2.Version} + for _, version := range versions { + t.Run("AppVersion_"+fmt.Sprint(version), func(t *testing.T) { + keeper, _, ctx := CreateKeeper(t, v1.Version) + wctx := sdk.WrapSDKContext(ctx) + params := types.DefaultParams() + keeper.SetParams(ctx, params) - response, err := keeper.Params(wctx, &types.QueryParamsRequest{}) - require.NoError(t, err) - require.Equal(t, &types.QueryParamsResponse{Params: params}, response) + response, err := keeper.Params(wctx, &types.QueryParamsRequest{}) + require.NoError(t, err) + require.Equal(t, &types.QueryParamsResponse{Params: params}, response) + }) + } } diff --git a/x/blob/keeper/keeper.go b/x/blob/keeper/keeper.go index 72a3fb7605..585b2f49ed 100644 --- a/x/blob/keeper/keeper.go +++ b/x/blob/keeper/keeper.go @@ -4,6 +4,8 @@ import ( "context" "fmt" + "github.com/celestiaorg/celestia-app/v3/pkg/appconsts" + v2 "github.com/celestiaorg/celestia-app/v3/pkg/appconsts/v2" "github.com/celestiaorg/celestia-app/v3/x/blob/types" "github.com/cosmos/cosmos-sdk/codec" sdk "github.com/cosmos/cosmos-sdk/types" @@ -43,7 +45,14 @@ func (k Keeper) Logger(ctx sdk.Context) log.Logger { func (k Keeper) PayForBlobs(goCtx context.Context, msg *types.MsgPayForBlobs) (*types.MsgPayForBlobsResponse, error) { ctx := sdk.UnwrapSDKContext(goCtx) - gasToConsume := types.GasToConsume(msg.BlobSizes, k.GasPerBlobByte(ctx)) + // GasPerBlobByte is a versioned variable from version 3 onwards. + var gasToConsume uint64 + if ctx.BlockHeader().Version.App <= v2.Version { + gasToConsume = types.GasToConsume(msg.BlobSizes, k.GasPerBlobByte(ctx)) + } else { + gasToConsume = types.GasToConsume(msg.BlobSizes, appconsts.GasPerBlobByte(appconsts.LatestVersion)) + } + ctx.GasMeter().ConsumeGas(gasToConsume, payForBlobGasDescriptor) err := ctx.EventManager().EmitTypedEvent( diff --git a/x/blob/keeper/keeper_test.go b/x/blob/keeper/keeper_test.go index 1ad3fbb923..c69b132c85 100644 --- a/x/blob/keeper/keeper_test.go +++ b/x/blob/keeper/keeper_test.go @@ -6,6 +6,8 @@ import ( "testing" "github.com/celestiaorg/celestia-app/v3/pkg/appconsts" + v2 "github.com/celestiaorg/celestia-app/v3/pkg/appconsts/v2" + v3 "github.com/celestiaorg/celestia-app/v3/pkg/appconsts/v3" testutil "github.com/celestiaorg/celestia-app/v3/test/util" "github.com/celestiaorg/celestia-app/v3/x/blob/keeper" "github.com/celestiaorg/celestia-app/v3/x/blob/types" @@ -28,34 +30,44 @@ import ( // TestPayForBlobs verifies the attributes on the emitted event. func TestPayForBlobs(t *testing.T) { - k, _, ctx := CreateKeeper(t) - signer := "celestia15drmhzw5kwgenvemy30rqqqgq52axf5wwrruf7" - namespace := appns.MustNewV0(bytes.Repeat([]byte{1}, appns.NamespaceVersionZeroIDSize)) - namespaces := [][]byte{namespace.Bytes()} - blobData := []byte("blob") - blobSizes := []uint32{uint32(len(blobData))} - - // verify no events exist yet - events := ctx.EventManager().Events().ToABCIEvents() - assert.Len(t, events, 0) - - // emit an event by submitting a PayForBlob - msg := createMsgPayForBlob(t, signer, namespace, blobData) - _, err := k.PayForBlobs(ctx, msg) - require.NoError(t, err) + // Testing transition from v2 -> v3 + // v2 uses the GasPerBlobByte key from the params store + // v3 uses the GasPerBlobByte from the appconsts + // TODO: Replace this with appversion latest + versions := []uint64{v2.Version, v3.Version} - // verify that an event was emitted - events = ctx.EventManager().Events().ToABCIEvents() - assert.Len(t, events, 1) - protoEvent, err := sdk.ParseTypedEvent(events[0]) - require.NoError(t, err) - event, err := convertToEventPayForBlobs(protoEvent) - require.NoError(t, err) + for _, version := range versions { + t.Run("AppVersion_"+fmt.Sprint(version), func(t *testing.T) { + k, _, ctx := CreateKeeper(t, version) + signer := "celestia15drmhzw5kwgenvemy30rqqqgq52axf5wwrruf7" + namespace := appns.MustNewV0(bytes.Repeat([]byte{1}, appns.NamespaceVersionZeroIDSize)) + namespaces := [][]byte{namespace.Bytes()} + blobData := []byte("blob") + blobSizes := []uint32{uint32(len(blobData))} + + // verify no events exist yet + events := ctx.EventManager().Events().ToABCIEvents() + assert.Len(t, events, 0) - // verify the attributes of the event - assert.Equal(t, signer, event.Signer) - assert.Equal(t, namespaces, event.Namespaces) - assert.Equal(t, blobSizes, event.BlobSizes) + // emit an event by submitting a PayForBlob + msg := createMsgPayForBlob(t, signer, namespace, blobData) + _, err := k.PayForBlobs(ctx, msg) + require.NoError(t, err) + + // verify that an event was emitted + events = ctx.EventManager().Events().ToABCIEvents() + assert.Len(t, events, 1) + protoEvent, err := sdk.ParseTypedEvent(events[0]) + require.NoError(t, err) + event, err := convertToEventPayForBlobs(protoEvent) + require.NoError(t, err) + + // verify the attributes of the event + assert.Equal(t, signer, event.Signer) + assert.Equal(t, namespaces, event.Namespaces) + assert.Equal(t, blobSizes, event.BlobSizes) + }) + } } func convertToEventPayForBlobs(message proto.Message) (*types.EventPayForBlobs, error) { @@ -72,7 +84,7 @@ func createMsgPayForBlob(t *testing.T, signer string, namespace appns.Namespace, return msg } -func CreateKeeper(t *testing.T) (*keeper.Keeper, store.CommitMultiStore, sdk.Context) { +func CreateKeeper(t *testing.T, version uint64) (*keeper.Keeper, store.CommitMultiStore, sdk.Context) { storeKey := sdk.NewKVStoreKey(paramtypes.StoreKey) tStoreKey := storetypes.NewTransientStoreKey(paramtypes.TStoreKey) @@ -87,7 +99,7 @@ func CreateKeeper(t *testing.T) (*keeper.Keeper, store.CommitMultiStore, sdk.Con ctx := sdk.NewContext(stateStore, tmproto.Header{ Version: tmversion.Consensus{ Block: 1, - App: 1, + App: version, }, }, false, nil) diff --git a/x/blob/keeper/params_test.go b/x/blob/keeper/params_test.go index 9431ef3828..53f5dbca20 100644 --- a/x/blob/keeper/params_test.go +++ b/x/blob/keeper/params_test.go @@ -3,12 +3,13 @@ package keeper_test import ( "testing" + "github.com/celestiaorg/celestia-app/v3/pkg/appconsts" "github.com/celestiaorg/celestia-app/v3/x/blob/types" "github.com/stretchr/testify/require" ) func TestGetParams(t *testing.T) { - k, _, ctx := CreateKeeper(t) + k, _, ctx := CreateKeeper(t, appconsts.LatestVersion) params := types.DefaultParams() k.SetParams(ctx, params) From cb261f770bec5d8e9e9107237d42712acd5f6281 Mon Sep 17 00:00:00 2001 From: Nina Barbakadze Date: Thu, 1 Aug 2024 17:35:44 +0200 Subject: [PATCH 02/20] refactor: nits --- app/ante/tx_size.go | 5 ++--- app/ante/tx_size_test.go | 5 +++-- app/app.go | 16 +++++++--------- app/modules.go | 20 -------------------- pkg/appconsts/initial_consts.go | 4 ---- pkg/appconsts/versioned_consts.go | 1 + specs/src/specs/ante_handler_v1.md | 2 +- x/blob/README.md | 2 +- x/blob/ante/ante_test.go | 3 +-- x/blob/keeper/gas_test.go | 25 +++++++++++++++++++++++++ x/blob/keeper/grpc_query_params_test.go | 21 +++++++-------------- x/blob/types/payforblob.go | 3 +-- 12 files changed, 49 insertions(+), 58 deletions(-) diff --git a/app/ante/tx_size.go b/app/ante/tx_size.go index fa578d073a..08f5df9af7 100644 --- a/app/ante/tx_size.go +++ b/app/ante/tx_size.go @@ -22,7 +22,6 @@ import ( // simulation signature values used to estimate gas consumption var ( - // simulation signature values used to estimate gas consumption key = make([]byte, secp256k1.PubKeySize) simSecp256k1Pubkey = &secp256k1.PubKey{Key: key} simSecp256k1Sig [64]byte @@ -136,13 +135,13 @@ func isIncompleteSignature(data signing.SignatureData) bool { return false } +// consumeGasForTxSize consumes gas based on the size of the transaction. +// It uses different parameters depending on the app version. func consumeGasForTxSize(ctx sdk.Context, cost uint64, params auth.Params) { // For app v2 and below we should get txSizeCostPerByte from auth module if ctx.BlockHeader().Version.App <= v2.Version { - // fmt.Println("HERE <= 2") ctx.GasMeter().ConsumeGas(params.TxSizeCostPerByte*cost, "txSize") } else { - // fmt.Println("HERE < 2") // From v3 onwards, we should get txSizeCostPerByte from appconsts ctx.GasMeter().ConsumeGas(appconsts.TxSizeCostPerByte(v3.Version)*cost, "txSize") } diff --git a/app/ante/tx_size_test.go b/app/ante/tx_size_test.go index 1676d16fa8..98a2df8eeb 100644 --- a/app/ante/tx_size_test.go +++ b/app/ante/tx_size_test.go @@ -7,6 +7,8 @@ import ( "github.com/celestiaorg/celestia-app/v3/app" "github.com/celestiaorg/celestia-app/v3/app/ante" + "github.com/celestiaorg/celestia-app/v3/pkg/appconsts" + v3 "github.com/celestiaorg/celestia-app/v3/pkg/appconsts/v3" testutil "github.com/celestiaorg/celestia-app/v3/test/util" "github.com/cosmos/cosmos-sdk/client" "github.com/cosmos/cosmos-sdk/client/tx" @@ -95,8 +97,7 @@ func TestConsumeGasForTxSize(t *testing.T) { txBytes, err := clientCtx.TxConfig.TxJSONEncoder()(tx) require.Nil(t, err, "Cannot marshal tx: %v", err) - params := app.AccountKeeper.GetParams(ctx) - expectedGas := sdk.Gas(len(txBytes)) * params.TxSizeCostPerByte + expectedGas := sdk.Gas(len(txBytes)) * appconsts.TxSizeCostPerByte(v3.Version) // Set suite.ctx with TxBytes manually ctx = ctx.WithTxBytes(txBytes) diff --git a/app/app.go b/app/app.go index 230496f918..a15d6c8d54 100644 --- a/app/app.go +++ b/app/app.go @@ -11,7 +11,6 @@ import ( "github.com/celestiaorg/celestia-app/v3/app/posthandler" appv1 "github.com/celestiaorg/celestia-app/v3/pkg/appconsts/v1" appv2 "github.com/celestiaorg/celestia-app/v3/pkg/appconsts/v2" - appv3 "github.com/celestiaorg/celestia-app/v3/pkg/appconsts/v3" "github.com/celestiaorg/celestia-app/v3/pkg/proof" blobkeeper "github.com/celestiaorg/celestia-app/v3/x/blob/keeper" blobtypes "github.com/celestiaorg/celestia-app/v3/x/blob/types" @@ -108,7 +107,6 @@ var maccPerms = map[string][]string{ const ( v1 = appv1.Version v2 = appv2.Version - v3 = appv3.Version DefaultInitialVersion = v1 ) @@ -167,7 +165,7 @@ type App struct { configurator module.Configurator // upgradeHeightV2 is used as a coordination mechanism for the height-based // upgrade from v1 to v2. - upgradeHeightV3 int64 + upgradeHeightV2 int64 // MsgGateKeeper is used to define which messages are accepted for a given // app version. MsgGateKeeper *ante.MsgVersioningGateKeeper @@ -185,7 +183,7 @@ func New( traceStore io.Writer, invCheckPeriod uint, encodingConfig encoding.Config, - upgradeHeightV3 int64, + upgradeHeightV2 int64, appOpts servertypes.AppOptions, baseAppOptions ...func(*baseapp.BaseApp), ) *App { @@ -211,7 +209,7 @@ func New( keys: keys, tkeys: tkeys, memKeys: memKeys, - upgradeHeightV3: upgradeHeightV3, + upgradeHeightV2: upgradeHeightV2, } app.ParamsKeeper = initParamsKeeper(appCodec, encodingConfig.Amino, keys[paramstypes.StoreKey], tkeys[paramstypes.TStoreKey]) @@ -446,7 +444,7 @@ func (app *App) Name() string { return app.BaseApp.Name() } // BeginBlocker application updates every begin block func (app *App) BeginBlocker(ctx sdk.Context, req abci.RequestBeginBlock) abci.ResponseBeginBlock { - if req.Header.Height == app.upgradeHeightV3 { + if req.Header.Height == app.upgradeHeightV2 { app.BaseApp.Logger().Info("upgraded from app version 1 to 2") } return app.manager.BeginBlock(ctx, req) @@ -459,8 +457,8 @@ func (app *App) EndBlocker(ctx sdk.Context, req abci.RequestEndBlock) abci.Respo // For v1 only we upgrade using a agreed upon height known ahead of time if currentVersion == v1 { // check that we are at the height before the upgrade - if req.Height == app.upgradeHeightV3-1 { - app.BaseApp.Logger().Info(fmt.Sprintf("upgrading from app version %v to 3", currentVersion)) + if req.Height == app.upgradeHeightV2-1 { + app.BaseApp.Logger().Info(fmt.Sprintf("upgrading from app version %v to 2", currentVersion)) app.SetInitialAppVersionInConsensusParams(ctx, v2) app.SetAppVersion(ctx, v2) @@ -470,7 +468,7 @@ func (app *App) EndBlocker(ctx sdk.Context, req abci.RequestEndBlock) abci.Respo panic(err) } } - // from v2 to v3 and onwards we use a signalling mechanism + // from v1 to v1 and onwards we use a signalling mechanism } else if shouldUpgrade, newVersion := app.SignalKeeper.ShouldUpgrade(ctx); shouldUpgrade { // Version changes must be increasing. Downgrades are not permitted if newVersion > currentVersion { diff --git a/app/modules.go b/app/modules.go index ea18c66a9e..a89f418205 100644 --- a/app/modules.go +++ b/app/modules.go @@ -341,26 +341,6 @@ func versionedStoreKeys() map[uint64][]string { stakingtypes.StoreKey, upgradetypes.StoreKey, }, - // 3: { - // authtypes.StoreKey, - // authzkeeper.StoreKey, - // banktypes.StoreKey, - // blobtypes.StoreKey, - // capabilitytypes.StoreKey, - // distrtypes.StoreKey, - // evidencetypes.StoreKey, - // feegrant.StoreKey, - // govtypes.StoreKey, - // ibchost.StoreKey, - // ibctransfertypes.StoreKey, - // icahosttypes.StoreKey, // added in v2 - // minttypes.StoreKey, - // packetforwardtypes.StoreKey, // added in v2 - // signaltypes.StoreKey, // added in v2 - // slashingtypes.StoreKey, - // stakingtypes.StoreKey, - // upgradetypes.StoreKey, - // }, } } diff --git a/pkg/appconsts/initial_consts.go b/pkg/appconsts/initial_consts.go index 09e3a164a4..f2187f8165 100644 --- a/pkg/appconsts/initial_consts.go +++ b/pkg/appconsts/initial_consts.go @@ -13,10 +13,6 @@ const ( // maximum number of bytes allowed in a valid block. DefaultMaxBytes = DefaultGovMaxSquareSize * DefaultGovMaxSquareSize * ContinuationSparseShareContentSize - // DefaultGasPerBlobByte is the default gas cost deducted per byte of blob - // included in a PayForBlobs txn - DefaultGasPerBlobByte = 8 - // DefaultMinGasPrice is the default min gas price that gets set in the app.toml file. // The min gas price acts as a filter. Transactions below that limit will not pass // a nodes `CheckTx` and thus not be proposed by that node. diff --git a/pkg/appconsts/versioned_consts.go b/pkg/appconsts/versioned_consts.go index 8721b8e229..715200418b 100644 --- a/pkg/appconsts/versioned_consts.go +++ b/pkg/appconsts/versioned_consts.go @@ -39,4 +39,5 @@ var ( DefaultSubtreeRootThreshold = SubtreeRootThreshold(LatestVersion) DefaultSquareSizeUpperBound = SquareSizeUpperBound(LatestVersion) DefaultTxSizeCostPerByte = TxSizeCostPerByte(LatestVersion) + DefaultGasPerBlobByte = GasPerBlobByte(LatestVersion) ) diff --git a/specs/src/specs/ante_handler_v1.md b/specs/src/specs/ante_handler_v1.md index 3070d2a92c..52109f67f8 100644 --- a/specs/src/specs/ante_handler_v1.md +++ b/specs/src/specs/ante_handler_v1.md @@ -11,7 +11,7 @@ The AnteHandler chains together several decorators to ensure the following crite - The tx's count of signatures <= the max number of signatures. The max number of signatures is [`TxSigLimit = 7`](https://github.com/cosmos/cosmos-sdk/blob/a429238fc267da88a8548bfebe0ba7fb28b82a13/x/auth/README.md?plain=1#L231). - The tx's [gas_limit](https://github.com/cosmos/cosmos-sdk/blob/22c28366466e64ebf0df1ce5bec8b1130523552c/proto/cosmos/tx/v1beta1/tx.proto#L211-L213) is > the gas consumed based on the tx's signatures. - The tx's [signatures](https://github.com/cosmos/cosmos-sdk/blob/22c28366466e64ebf0df1ce5bec8b1130523552c/types/tx/signing/signature.go#L10-L26) are valid. For each signature, ensure that the signature's sequence number (a.k.a nonce) matches the account sequence number of the signer. -- The tx's [gas_limit](https://github.com/cosmos/cosmos-sdk/blob/22c28366466e64ebf0df1ce5bec8b1130523552c/proto/cosmos/tx/v1beta1/tx.proto#L211-L213) is > the gas consumed based on the blob size(s). Since blobs are charged based on the number of shares they occupy, the gas consumed is calculated as follows: `gasToConsume = sharesNeeded(blob) * bytesPerShare * gasPerBlobByte`. Where `bytesPerShare` is a global constant (an alias for [`ShareSize = 512`](https://github.com/celestiaorg/celestia-app/blob/c90e61d5a2d0c0bd0e123df4ab416f6f0d141b7f/pkg/appconsts/global_consts.go#L27-L28)) and `gasPerBlobByte` is an application constant that can be modified through hard fork (the [`DefaultGasPerBlobByte = 8`](https://github.com/celestiaorg/celestia-app/blob/c90e61d5a2d0c0bd0e123df4ab416f6f0d141b7f/pkg/appconsts/initial_consts.go#L16-L18)). +- The tx's [gas_limit](https://github.com/cosmos/cosmos-sdk/blob/22c28366466e64ebf0df1ce5bec8b1130523552c/proto/cosmos/tx/v1beta1/tx.proto#L211-L213) is > the gas consumed based on the blob size(s). Since blobs are charged based on the number of shares they occupy, the gas consumed is calculated as follows: `gasToConsume = sharesNeeded(blob) * bytesPerShare * gasPerBlobByte`. Where `bytesPerShare` is a global constant (an alias for [`ShareSize = 512`](https://github.com/celestiaorg/celestia-app/blob/c90e61d5a2d0c0bd0e123df4ab416f6f0d141b7f/pkg/appconsts/global_consts.go#L27-L28)) and `gasPerBlobByte` is a governance parameter that can be modified (the [`DefaultGasPerBlobByte = 8`](https://github.com/celestiaorg/celestia-app/blob/c90e61d5a2d0c0bd0e123df4ab416f6f0d141b7f/pkg/appconsts/initial_consts.go#L16-L18)). - The tx's total blob size is <= the max blob size. The max blob size is derived from the maximum valid square size. The max valid square size is the minimum of: `GovMaxSquareSize` and `SquareSizeUpperBound`. - The tx does not contain a message of type [MsgSubmitProposal](https://github.com/cosmos/cosmos-sdk/blob/d6d929843bbd331b885467475bcb3050788e30ca/proto/cosmos/gov/v1/tx.proto#L33-L43) with zero proposal messages. - The tx is not an IBC packet or update message that has already been processed. diff --git a/x/blob/README.md b/x/blob/README.md index e72c12db44..bd32dd9c0f 100644 --- a/x/blob/README.md +++ b/x/blob/README.md @@ -46,7 +46,7 @@ message Params { #### `GasPerBlobByte` -`GasPerBlobByte` is the amount of gas that is consumed per byte of blob data +in v1 and v2 `GasPerBlobByte` is the amount of gas that is consumed per byte of blob data when a `MsgPayForBlobs` is processed. Currently, the default value is 8. This value is set below that of normal transaction gas consumption, which is 10. diff --git a/x/blob/ante/ante_test.go b/x/blob/ante/ante_test.go index 48b0018d6d..eea631948c 100644 --- a/x/blob/ante/ante_test.go +++ b/x/blob/ante/ante_test.go @@ -6,7 +6,6 @@ import ( "github.com/celestiaorg/celestia-app/v3/app" "github.com/celestiaorg/celestia-app/v3/app/encoding" "github.com/celestiaorg/celestia-app/v3/pkg/appconsts" - v1 "github.com/celestiaorg/celestia-app/v3/pkg/appconsts/v1" ante "github.com/celestiaorg/celestia-app/v3/x/blob/ante" blob "github.com/celestiaorg/celestia-app/v3/x/blob/types" "github.com/celestiaorg/go-square/shares" @@ -94,7 +93,7 @@ func TestPFBAnteHandler(t *testing.T) { anteHandler := ante.NewMinGasPFBDecorator(mockBlobKeeper{}) ctx := sdk.NewContext(nil, tmproto.Header{ Version: version.Consensus{ - App: v1.Version, + App: appconsts.LatestVersion, }, }, true, nil).WithGasMeter(sdk.NewGasMeter(tc.txGas)) diff --git a/x/blob/keeper/gas_test.go b/x/blob/keeper/gas_test.go index c6bf07b75f..470b53d1b0 100644 --- a/x/blob/keeper/gas_test.go +++ b/x/blob/keeper/gas_test.go @@ -4,6 +4,7 @@ import ( "testing" "github.com/celestiaorg/celestia-app/v3/pkg/appconsts" + v2 "github.com/celestiaorg/celestia-app/v3/pkg/appconsts/v2" v3 "github.com/celestiaorg/celestia-app/v3/pkg/appconsts/v3" "github.com/celestiaorg/celestia-app/v3/x/blob/types" sdk "github.com/cosmos/cosmos-sdk/types" @@ -60,3 +61,27 @@ func TestPayForBlobGas(t *testing.T) { }) } } + +func TestChangingGasParam(t *testing.T) { + msg := types.MsgPayForBlobs{BlobSizes: []uint32{1024}} + k, stateStore, _ := CreateKeeper(t, v2.Version) + tempCtx := sdk.NewContext(stateStore, tmproto.Header{}, false, nil) + + ctx1 := sdk.NewContext(stateStore, tmproto.Header{}, false, nil) + _, err := k.PayForBlobs(sdk.WrapSDKContext(ctx1), &msg) + require.NoError(t, err) + + params := k.GetParams(tempCtx) + params.GasPerBlobByte++ + k.SetParams(tempCtx, params) + + ctx2 := sdk.NewContext(stateStore, tmproto.Header{}, false, nil) + _, err = k.PayForBlobs(sdk.WrapSDKContext(ctx2), &msg) + require.NoError(t, err) + + if ctx1.GasMeter().GasConsumed() >= ctx2.GasMeter().GasConsumedToLimit() { + t.Errorf("Gas consumed was not increased upon incrementing param, before: %d, after: %d", + ctx1.GasMeter().GasConsumed(), ctx2.GasMeter().GasConsumedToLimit(), + ) + } +} diff --git a/x/blob/keeper/grpc_query_params_test.go b/x/blob/keeper/grpc_query_params_test.go index 2279e79811..ae821ee807 100644 --- a/x/blob/keeper/grpc_query_params_test.go +++ b/x/blob/keeper/grpc_query_params_test.go @@ -1,10 +1,8 @@ package keeper_test import ( - "fmt" "testing" - v1 "github.com/celestiaorg/celestia-app/v3/pkg/appconsts/v1" v2 "github.com/celestiaorg/celestia-app/v3/pkg/appconsts/v2" "github.com/celestiaorg/celestia-app/v3/x/blob/types" sdk "github.com/cosmos/cosmos-sdk/types" @@ -12,17 +10,12 @@ import ( ) func TestParamsQuery(t *testing.T) { - versions := []uint64{v1.Version, v2.Version} - for _, version := range versions { - t.Run("AppVersion_"+fmt.Sprint(version), func(t *testing.T) { - keeper, _, ctx := CreateKeeper(t, v1.Version) - wctx := sdk.WrapSDKContext(ctx) - params := types.DefaultParams() - keeper.SetParams(ctx, params) + keeper, _, ctx := CreateKeeper(t, v2.Version) + wctx := sdk.WrapSDKContext(ctx) + params := types.DefaultParams() + keeper.SetParams(ctx, params) - response, err := keeper.Params(wctx, &types.QueryParamsRequest{}) - require.NoError(t, err) - require.Equal(t, &types.QueryParamsResponse{Params: params}, response) - }) - } + response, err := keeper.Params(wctx, &types.QueryParamsRequest{}) + require.NoError(t, err) + require.Equal(t, &types.QueryParamsResponse{Params: params}, response) } diff --git a/x/blob/types/payforblob.go b/x/blob/types/payforblob.go index e1db0c7cf6..8fbbdf7e7f 100644 --- a/x/blob/types/payforblob.go +++ b/x/blob/types/payforblob.go @@ -13,7 +13,6 @@ import ( appshares "github.com/celestiaorg/go-square/shares" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/x/auth/migrations/legacytx" - auth "github.com/cosmos/cosmos-sdk/x/auth/types" "golang.org/x/exp/slices" ) @@ -175,7 +174,7 @@ func EstimateGas(blobSizes []uint32, gasPerByte uint32, txSizeCost uint64) uint6 // DefaultEstimateGas runs EstimateGas with the system defaults. The network may change these values // through governance, thus this function should predominantly be used in testing. func DefaultEstimateGas(blobSizes []uint32) uint64 { - return EstimateGas(blobSizes, appconsts.DefaultGasPerBlobByte, auth.DefaultTxSizeCostPerByte) + return EstimateGas(blobSizes, appconsts.DefaultGasPerBlobByte, appconsts.DefaultTxSizeCostPerByte) } // ValidateBlobNamespace returns an error if the provided namespace is an From 3645e1b1a8a964ff4192c5dc8844bbcc152bed70 Mon Sep 17 00:00:00 2001 From: Nina Barbakadze Date: Thu, 1 Aug 2024 17:44:30 +0200 Subject: [PATCH 03/20] test: remove print statements --- app/ante/tx_size_test.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/app/ante/tx_size_test.go b/app/ante/tx_size_test.go index 98a2df8eeb..2d82014df9 100644 --- a/app/ante/tx_size_test.go +++ b/app/ante/tx_size_test.go @@ -59,8 +59,6 @@ func setup() (*app.App, sdk.Context, client.Context, sdk.AnteHandler, error) { func TestConsumeGasForTxSize(t *testing.T) { app, ctx, clientCtx, _, err := setup() require.NoError(t, err) - sub, exs := app.ParamsKeeper.GetSubspace(authtypes.ModuleName) - fmt.Println(sub, exs) var txBuilder client.TxBuilder // keys and addresses From e061c2011230604fb47adfbb6fc92fa0d7d5d085 Mon Sep 17 00:00:00 2001 From: Nina Barbakadze Date: Fri, 2 Aug 2024 06:59:47 +0200 Subject: [PATCH 04/20] refactor: cleanup more --- app/ante/tx_size.go | 8 +++----- app/ante/tx_size_test.go | 8 ++++---- app/app.go | 2 +- x/blob/README.md | 2 +- x/blob/keeper/gas_test.go | 3 +-- x/blob/keeper/grpc_query_params_test.go | 4 ++-- 6 files changed, 12 insertions(+), 15 deletions(-) diff --git a/app/ante/tx_size.go b/app/ante/tx_size.go index 08f5df9af7..2ea1d8cd47 100644 --- a/app/ante/tx_size.go +++ b/app/ante/tx_size.go @@ -14,23 +14,21 @@ import ( sdk "github.com/cosmos/cosmos-sdk/types" sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" "github.com/cosmos/cosmos-sdk/types/tx/signing" - ante "github.com/cosmos/cosmos-sdk/x/auth/ante" + "github.com/cosmos/cosmos-sdk/x/auth/ante" "github.com/cosmos/cosmos-sdk/x/auth/migrations/legacytx" authsigning "github.com/cosmos/cosmos-sdk/x/auth/signing" auth "github.com/cosmos/cosmos-sdk/x/auth/types" ) -// simulation signature values used to estimate gas consumption var ( + // Simulation signature values used to estimate gas consumption. key = make([]byte, secp256k1.PubKeySize) simSecp256k1Pubkey = &secp256k1.PubKey{Key: key} simSecp256k1Sig [64]byte - - _ authsigning.SigVerifiableTx = (*legacytx.StdTx)(nil) // assert StdTx implements SigVerifiableTx ) func init() { - // This decodes a valid hex string into a sepc256k1Pubkey for use in transaction simulation + // Decodes a valid hex string into a sepc256k1Pubkey for use in transaction simulation bz, _ := hex.DecodeString("035AD6810A47F073553FF30D2FCC7E0D3B1C0B74B61A1AAA2582344037151E143A") copy(key, bz) simSecp256k1Pubkey.Key = key diff --git a/app/ante/tx_size_test.go b/app/ante/tx_size_test.go index 2d82014df9..dfe28c5af6 100644 --- a/app/ante/tx_size_test.go +++ b/app/ante/tx_size_test.go @@ -21,7 +21,7 @@ import ( authante "github.com/cosmos/cosmos-sdk/x/auth/ante" xauthsigning "github.com/cosmos/cosmos-sdk/x/auth/signing" authtypes "github.com/cosmos/cosmos-sdk/x/auth/types" - require "github.com/stretchr/testify/require" + "github.com/stretchr/testify/require" tmproto "github.com/tendermint/tendermint/proto/tendermint/types" ) @@ -33,7 +33,7 @@ func setup() (*app.App, sdk.Context, client.Context, sdk.AnteHandler, error) { // Set up TxConfig. encodingConfig := simapp.MakeTestEncodingConfig() - // We're using TestMsg encoding in some tests, so register it here. + // We're using TestMsg encoding in the test, so register it here. encodingConfig.Amino.RegisterConcrete(&testdata.TestMsg{}, "testdata.TestMsg", nil) testdata.RegisterInterfaces(encodingConfig.InterfaceRegistry) @@ -97,7 +97,7 @@ func TestConsumeGasForTxSize(t *testing.T) { expectedGas := sdk.Gas(len(txBytes)) * appconsts.TxSizeCostPerByte(v3.Version) - // Set suite.ctx with TxBytes manually + // set suite.ctx with TxBytes manually ctx = ctx.WithTxBytes(txBytes) // track how much gas is necessary to retrieve parameters @@ -141,7 +141,7 @@ func TestConsumeGasForTxSize(t *testing.T) { } } -// CreateTestTx is a helper function to create a tx given multiple inputs. +// CreateTestTx creates a test tx given multiple inputs. func CreateTestTx(txBuilder client.TxBuilder, clientCtx client.Context, privs []cryptotypes.PrivKey, accNums []uint64, accSeqs []uint64, chainID string) (xauthsigning.Tx, error) { // First round: we gather all the signer infos. We use the "set empty // signature" hack to do that. diff --git a/app/app.go b/app/app.go index a15d6c8d54..2de17f779d 100644 --- a/app/app.go +++ b/app/app.go @@ -468,7 +468,7 @@ func (app *App) EndBlocker(ctx sdk.Context, req abci.RequestEndBlock) abci.Respo panic(err) } } - // from v1 to v1 and onwards we use a signalling mechanism + // from v2 to v3 and onwards we use a signalling mechanism } else if shouldUpgrade, newVersion := app.SignalKeeper.ShouldUpgrade(ctx); shouldUpgrade { // Version changes must be increasing. Downgrades are not permitted if newVersion > currentVersion { diff --git a/x/blob/README.md b/x/blob/README.md index bd32dd9c0f..620717dcf0 100644 --- a/x/blob/README.md +++ b/x/blob/README.md @@ -46,7 +46,7 @@ message Params { #### `GasPerBlobByte` -in v1 and v2 `GasPerBlobByte` is the amount of gas that is consumed per byte of blob data +In v1 and v2`GasPerBlobByte` is the amount of gas that is consumed per byte of blob data when a `MsgPayForBlobs` is processed. Currently, the default value is 8. This value is set below that of normal transaction gas consumption, which is 10. diff --git a/x/blob/keeper/gas_test.go b/x/blob/keeper/gas_test.go index 470b53d1b0..cf866232e1 100644 --- a/x/blob/keeper/gas_test.go +++ b/x/blob/keeper/gas_test.go @@ -4,7 +4,6 @@ import ( "testing" "github.com/celestiaorg/celestia-app/v3/pkg/appconsts" - v2 "github.com/celestiaorg/celestia-app/v3/pkg/appconsts/v2" v3 "github.com/celestiaorg/celestia-app/v3/pkg/appconsts/v3" "github.com/celestiaorg/celestia-app/v3/x/blob/types" sdk "github.com/cosmos/cosmos-sdk/types" @@ -64,7 +63,7 @@ func TestPayForBlobGas(t *testing.T) { func TestChangingGasParam(t *testing.T) { msg := types.MsgPayForBlobs{BlobSizes: []uint32{1024}} - k, stateStore, _ := CreateKeeper(t, v2.Version) + k, stateStore, _ := CreateKeeper(t, appconsts.LatestVersion) tempCtx := sdk.NewContext(stateStore, tmproto.Header{}, false, nil) ctx1 := sdk.NewContext(stateStore, tmproto.Header{}, false, nil) diff --git a/x/blob/keeper/grpc_query_params_test.go b/x/blob/keeper/grpc_query_params_test.go index ae821ee807..e8c367cd35 100644 --- a/x/blob/keeper/grpc_query_params_test.go +++ b/x/blob/keeper/grpc_query_params_test.go @@ -3,14 +3,14 @@ package keeper_test import ( "testing" - v2 "github.com/celestiaorg/celestia-app/v3/pkg/appconsts/v2" + "github.com/celestiaorg/celestia-app/v3/pkg/appconsts" "github.com/celestiaorg/celestia-app/v3/x/blob/types" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/stretchr/testify/require" ) func TestParamsQuery(t *testing.T) { - keeper, _, ctx := CreateKeeper(t, v2.Version) + keeper, _, ctx := CreateKeeper(t, appconsts.LatestVersion) wctx := sdk.WrapSDKContext(ctx) params := types.DefaultParams() keeper.SetParams(ctx, params) From be83e244d716fea653c5ee605339e7b1d2166bf5 Mon Sep 17 00:00:00 2001 From: Nina Barbakadze Date: Tue, 6 Aug 2024 12:23:52 +0200 Subject: [PATCH 05/20] refactor: address review comments --- app/ante/min_fee_test.go | 11 +++++------ app/ante/tx_size_test.go | 21 +++------------------ app/test/testnode_test.go | 4 ++-- app/test/upgrade_test.go | 3 ++- pkg/appconsts/initial_consts.go | 4 ++++ pkg/appconsts/v2/app_consts.go | 3 --- pkg/appconsts/v3/app_consts.go | 8 +++++--- pkg/appconsts/versioned_consts.go | 5 ++--- pkg/appconsts/versioned_consts_test.go | 12 ++++++++++++ x/blob/ante/ante.go | 3 +-- x/blob/keeper/gas_test.go | 11 +++++------ x/blob/keeper/keeper.go | 2 +- x/minfee/grpc_query_test.go | 4 ++-- x/minfee/params.go | 4 ++-- 14 files changed, 46 insertions(+), 49 deletions(-) diff --git a/app/ante/min_fee_test.go b/app/ante/min_fee_test.go index 30912813c4..fc66e11a67 100644 --- a/app/ante/min_fee_test.go +++ b/app/ante/min_fee_test.go @@ -9,7 +9,6 @@ import ( "github.com/celestiaorg/celestia-app/v3/app/ante" "github.com/celestiaorg/celestia-app/v3/app/encoding" "github.com/celestiaorg/celestia-app/v3/pkg/appconsts" - v2 "github.com/celestiaorg/celestia-app/v3/pkg/appconsts/v2" "github.com/celestiaorg/celestia-app/v3/test/util/testnode" "github.com/celestiaorg/celestia-app/v3/x/minfee" "github.com/cosmos/cosmos-sdk/codec" @@ -58,7 +57,7 @@ func TestValidateTxFee(t *testing.T) { { name: "bad tx; fee below required minimum", fee: sdk.NewCoins(sdk.NewInt64Coin(appconsts.BondDenom, feeAmount-1)), - gasLimit: uint64(float64(feeAmount) / v2.NetworkMinGasPrice), + gasLimit: uint64(float64(feeAmount) / appconsts.DefaultNetworkMinGasPrice), appVersion: uint64(2), isCheckTx: false, expErr: true, @@ -66,7 +65,7 @@ func TestValidateTxFee(t *testing.T) { { name: "good tx; fee equal to required minimum", fee: sdk.NewCoins(sdk.NewInt64Coin(appconsts.BondDenom, feeAmount)), - gasLimit: uint64(float64(feeAmount) / v2.NetworkMinGasPrice), + gasLimit: uint64(float64(feeAmount) / appconsts.DefaultNetworkMinGasPrice), appVersion: uint64(2), isCheckTx: false, expErr: false, @@ -74,7 +73,7 @@ func TestValidateTxFee(t *testing.T) { { name: "good tx; fee above required minimum", fee: sdk.NewCoins(sdk.NewInt64Coin(appconsts.BondDenom, feeAmount+1)), - gasLimit: uint64(float64(feeAmount) / v2.NetworkMinGasPrice), + gasLimit: uint64(float64(feeAmount) / appconsts.DefaultNetworkMinGasPrice), appVersion: uint64(2), isCheckTx: false, expErr: false, @@ -82,7 +81,7 @@ func TestValidateTxFee(t *testing.T) { { name: "good tx; with no fee (v1)", fee: sdk.NewCoins(sdk.NewInt64Coin(appconsts.BondDenom, feeAmount)), - gasLimit: uint64(float64(feeAmount) / v2.NetworkMinGasPrice), + gasLimit: uint64(float64(feeAmount) / appconsts.DefaultNetworkMinGasPrice), appVersion: uint64(1), isCheckTx: false, expErr: false, @@ -143,7 +142,7 @@ func TestValidateTxFee(t *testing.T) { ctx = ctx.WithMinGasPrices(sdk.DecCoins{validatorMinGasPriceCoin}) - networkMinGasPriceDec, err := sdk.NewDecFromStr(fmt.Sprintf("%f", v2.NetworkMinGasPrice)) + networkMinGasPriceDec, err := sdk.NewDecFromStr(fmt.Sprintf("%f", appconsts.DefaultNetworkMinGasPrice)) require.NoError(t, err) subspace, _ := paramsKeeper.GetSubspace(minfee.ModuleName) diff --git a/app/ante/tx_size_test.go b/app/ante/tx_size_test.go index dfe28c5af6..bdc8e8fe62 100644 --- a/app/ante/tx_size_test.go +++ b/app/ante/tx_size_test.go @@ -1,7 +1,6 @@ package ante_test import ( - "fmt" "strings" "testing" @@ -18,14 +17,13 @@ import ( "github.com/cosmos/cosmos-sdk/testutil/testdata" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/types/tx/signing" - authante "github.com/cosmos/cosmos-sdk/x/auth/ante" xauthsigning "github.com/cosmos/cosmos-sdk/x/auth/signing" authtypes "github.com/cosmos/cosmos-sdk/x/auth/types" "github.com/stretchr/testify/require" tmproto "github.com/tendermint/tendermint/proto/tendermint/types" ) -func setup() (*app.App, sdk.Context, client.Context, sdk.AnteHandler, error) { +func setup() (*app.App, sdk.Context, client.Context, error) { app, _, _ := testutil.NewTestAppWithGenesisSet(app.DefaultConsensusParams()) ctx := app.NewContext(false, tmproto.Header{}) app.AccountKeeper.SetParams(ctx, authtypes.DefaultParams()) @@ -40,24 +38,11 @@ func setup() (*app.App, sdk.Context, client.Context, sdk.AnteHandler, error) { clientCtx := client.Context{}. WithTxConfig(encodingConfig.TxConfig) - anteHandler, err := authante.NewAnteHandler( - authante.HandlerOptions{ - AccountKeeper: app.AccountKeeper, - BankKeeper: app.BankKeeper, - FeegrantKeeper: app.FeeGrantKeeper, - SignModeHandler: encodingConfig.TxConfig.SignModeHandler(), - SigGasConsumer: authante.DefaultSigVerificationGasConsumer, - }, - ) - if err != nil { - return nil, sdk.Context{}, client.Context{}, nil, fmt.Errorf("error creating AnteHandler: %v", err) - } - - return app, ctx, clientCtx, anteHandler, nil + return app, ctx, clientCtx, nil } func TestConsumeGasForTxSize(t *testing.T) { - app, ctx, clientCtx, _, err := setup() + app, ctx, clientCtx, err := setup() require.NoError(t, err) var txBuilder client.TxBuilder diff --git a/app/test/testnode_test.go b/app/test/testnode_test.go index 88e18564e3..0d584cfbd7 100644 --- a/app/test/testnode_test.go +++ b/app/test/testnode_test.go @@ -3,7 +3,7 @@ package app_test import ( "testing" - v2 "github.com/celestiaorg/celestia-app/v3/pkg/appconsts/v2" + "github.com/celestiaorg/celestia-app/v3/pkg/appconsts" "github.com/celestiaorg/celestia-app/v3/test/util/testnode" "github.com/celestiaorg/celestia-app/v3/x/minfee" nodeservice "github.com/cosmos/cosmos-sdk/client/grpc/node" @@ -29,7 +29,7 @@ func Test_testnode(t *testing.T) { require.NoError(t, err) got, err := resp.NetworkMinGasPrice.Float64() require.NoError(t, err) - assert.Equal(t, v2.NetworkMinGasPrice, got) + assert.Equal(t, appconsts.DefaultNetworkMinGasPrice, got) }) t.Run("testnode can query local min gas price", func(t *testing.T) { config := testnode.DefaultConfig() diff --git a/app/test/upgrade_test.go b/app/test/upgrade_test.go index 55de8fc486..1ebb32dd6d 100644 --- a/app/test/upgrade_test.go +++ b/app/test/upgrade_test.go @@ -9,6 +9,7 @@ import ( app "github.com/celestiaorg/celestia-app/v3/app" "github.com/celestiaorg/celestia-app/v3/app/encoding" + "github.com/celestiaorg/celestia-app/v3/pkg/appconsts" v1 "github.com/celestiaorg/celestia-app/v3/pkg/appconsts/v1" v2 "github.com/celestiaorg/celestia-app/v3/pkg/appconsts/v2" "github.com/celestiaorg/celestia-app/v3/test/util" @@ -30,7 +31,7 @@ import ( // TestAppUpgrades verifies that the all module's params are overridden during an // upgrade from v1 -> v2 and the app version changes correctly. func TestAppUpgrades(t *testing.T) { - NetworkMinGasPriceDec, err := sdk.NewDecFromStr(fmt.Sprintf("%f", v2.NetworkMinGasPrice)) + NetworkMinGasPriceDec, err := sdk.NewDecFromStr(fmt.Sprintf("%f", appconsts.DefaultNetworkMinGasPrice)) require.NoError(t, err) tests := []struct { diff --git a/pkg/appconsts/initial_consts.go b/pkg/appconsts/initial_consts.go index f2187f8165..62188c485b 100644 --- a/pkg/appconsts/initial_consts.go +++ b/pkg/appconsts/initial_consts.go @@ -22,4 +22,8 @@ const ( // to unbond in a proof of stake system. Any validator within this // time can be subject to slashing under conditions of misbehavior. DefaultUnbondingTime = 3 * 7 * 24 * time.Hour + + // NetworkMinGasPrice is used by x/minfee to prevent transactions from being + // included in a block if they specify a gas price lower than this. + DefaultNetworkMinGasPrice = 0.000001 // utia ) diff --git a/pkg/appconsts/v2/app_consts.go b/pkg/appconsts/v2/app_consts.go index 2ef7a4075c..d5829e84c5 100644 --- a/pkg/appconsts/v2/app_consts.go +++ b/pkg/appconsts/v2/app_consts.go @@ -4,7 +4,4 @@ const ( Version uint64 = 2 SquareSizeUpperBound int = 128 SubtreeRootThreshold int = 64 - // NetworkMinGasPrice is used by x/minfee to prevent transactions from being - // included in a block if they specify a gas price lower than this. - NetworkMinGasPrice float64 = 0.000001 // utia ) diff --git a/pkg/appconsts/v3/app_consts.go b/pkg/appconsts/v3/app_consts.go index f8ba21bad5..3fa92f49a0 100644 --- a/pkg/appconsts/v3/app_consts.go +++ b/pkg/appconsts/v3/app_consts.go @@ -1,7 +1,9 @@ package v3 const ( - Version uint64 = 3 - TxSizeCostPerByte uint64 = 10 - GasPerBlobByte uint32 = 8 + Version uint64 = 3 + SquareSizeUpperBound int = 128 + SubtreeRootThreshold int = 64 + TxSizeCostPerByte uint64 = 10 + GasPerBlobByte uint32 = 8 ) diff --git a/pkg/appconsts/versioned_consts.go b/pkg/appconsts/versioned_consts.go index 715200418b..a0634f3efd 100644 --- a/pkg/appconsts/versioned_consts.go +++ b/pkg/appconsts/versioned_consts.go @@ -1,7 +1,6 @@ package appconsts import ( - v1 "github.com/celestiaorg/celestia-app/v3/pkg/appconsts/v1" v2 "github.com/celestiaorg/celestia-app/v3/pkg/appconsts/v2" v3 "github.com/celestiaorg/celestia-app/v3/pkg/appconsts/v3" ) @@ -19,12 +18,12 @@ const ( // // The rationale for this value is described in more detail in ADR-013. func SubtreeRootThreshold(_ uint64) int { - return v1.SubtreeRootThreshold + return v3.SubtreeRootThreshold } // SquareSizeUpperBound imposes an upper bound on the max effective square size. func SquareSizeUpperBound(_ uint64) int { - return v1.SquareSizeUpperBound + return v3.SquareSizeUpperBound } func TxSizeCostPerByte(_ uint64) uint64 { diff --git a/pkg/appconsts/versioned_consts_test.go b/pkg/appconsts/versioned_consts_test.go index fdfff95521..da29d71241 100644 --- a/pkg/appconsts/versioned_consts_test.go +++ b/pkg/appconsts/versioned_consts_test.go @@ -30,6 +30,12 @@ func TestVersionedConsts(t *testing.T) { expectedConstant: v2.SubtreeRootThreshold, got: appconsts.SubtreeRootThreshold(v2.Version), }, + { + name: "SubtreeRootThreshold v3", + version: v3.Version, + expectedConstant: v3.SubtreeRootThreshold, + got: appconsts.SubtreeRootThreshold(v3.Version), + }, { name: "SquareSizeUpperBound v1", version: v1.Version, @@ -42,6 +48,12 @@ func TestVersionedConsts(t *testing.T) { expectedConstant: v2.SquareSizeUpperBound, got: appconsts.SquareSizeUpperBound(v2.Version), }, + { + name: "SquareSizeUpperBound v3", + version: v3.Version, + expectedConstant: v3.SquareSizeUpperBound, + got: appconsts.SquareSizeUpperBound(v3.Version), + }, { name: "TxSizeCostPerByte v3", version: v3.Version, diff --git a/x/blob/ante/ante.go b/x/blob/ante/ante.go index 4906eec386..3c5249b291 100644 --- a/x/blob/ante/ante.go +++ b/x/blob/ante/ante.go @@ -3,7 +3,6 @@ package ante import ( "github.com/celestiaorg/celestia-app/v3/pkg/appconsts" v2 "github.com/celestiaorg/celestia-app/v3/pkg/appconsts/v2" - v3 "github.com/celestiaorg/celestia-app/v3/pkg/appconsts/v3" "github.com/celestiaorg/celestia-app/v3/x/blob/types" "cosmossdk.io/errors" @@ -40,7 +39,7 @@ func (d MinGasPFBDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bool // lazily fetch the gas per byte param gasPerByte = d.k.GasPerBlobByte(ctx) } else { - gasPerByte = appconsts.GasPerBlobByte(v3.Version) + gasPerByte = appconsts.GasPerBlobByte(ctx.BlockHeader().Version.App) } } gasToConsume := pfb.Gas(gasPerByte) diff --git a/x/blob/keeper/gas_test.go b/x/blob/keeper/gas_test.go index cf866232e1..61fe6d75eb 100644 --- a/x/blob/keeper/gas_test.go +++ b/x/blob/keeper/gas_test.go @@ -4,7 +4,6 @@ import ( "testing" "github.com/celestiaorg/celestia-app/v3/pkg/appconsts" - v3 "github.com/celestiaorg/celestia-app/v3/pkg/appconsts/v3" "github.com/celestiaorg/celestia-app/v3/x/blob/types" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/stretchr/testify/require" @@ -24,27 +23,27 @@ func TestPayForBlobGas(t *testing.T) { { name: "1 byte blob", // occupies 1 share msg: types.MsgPayForBlobs{BlobSizes: []uint32{1}}, - wantGasConsumed: uint64(1*appconsts.ShareSize*appconsts.GasPerBlobByte(v3.Version) + paramLookUpCost), // 1 share * 512 bytes per share * 8 gas per byte + 1060 gas for fetching param = 5156 gas + wantGasConsumed: uint64(1*appconsts.ShareSize*appconsts.GasPerBlobByte(appconsts.LatestVersion) + paramLookUpCost), // 1 share * 512 bytes per share * 8 gas per byte + 1060 gas for fetching param = 5156 gas }, { name: "100 byte blob", // occupies 1 share msg: types.MsgPayForBlobs{BlobSizes: []uint32{100}}, - wantGasConsumed: uint64(1*appconsts.ShareSize*appconsts.GasPerBlobByte(v3.Version) + paramLookUpCost), + wantGasConsumed: uint64(1*appconsts.ShareSize*appconsts.GasPerBlobByte(appconsts.LatestVersion) + paramLookUpCost), }, { name: "1024 byte blob", // occupies 3 shares because share prefix (e.g. namespace, info byte) msg: types.MsgPayForBlobs{BlobSizes: []uint32{1024}}, - wantGasConsumed: uint64(3*appconsts.ShareSize*appconsts.GasPerBlobByte(v3.Version) + paramLookUpCost), // 3 shares * 512 bytes per share * 8 gas per byte + 1060 gas for fetching param = 13348 gas + wantGasConsumed: uint64(3*appconsts.ShareSize*appconsts.GasPerBlobByte(appconsts.LatestVersion) + paramLookUpCost), // 3 shares * 512 bytes per share * 8 gas per byte + 1060 gas for fetching param = 13348 gas }, { name: "3 blobs, 1 share each", msg: types.MsgPayForBlobs{BlobSizes: []uint32{1, 1, 1}}, - wantGasConsumed: uint64(3*appconsts.ShareSize*appconsts.GasPerBlobByte(v3.Version) + paramLookUpCost), // 3 shares * 512 bytes per share * 8 gas per byte + 1060 gas for fetching param = 13348 gas + wantGasConsumed: uint64(3*appconsts.ShareSize*appconsts.GasPerBlobByte(appconsts.LatestVersion) + paramLookUpCost), // 3 shares * 512 bytes per share * 8 gas per byte + 1060 gas for fetching param = 13348 gas }, { name: "3 blobs, 6 shares total", msg: types.MsgPayForBlobs{BlobSizes: []uint32{1024, 800, 100}}, - wantGasConsumed: uint64(6*appconsts.ShareSize*appconsts.GasPerBlobByte(v3.Version) + paramLookUpCost), // 6 shares * 512 bytes per share * 8 gas per byte + 1060 gas for fetching param = 25636 gas + wantGasConsumed: uint64(6*appconsts.ShareSize*appconsts.GasPerBlobByte(appconsts.LatestVersion) + paramLookUpCost), // 6 shares * 512 bytes per share * 8 gas per byte + 1060 gas for fetching param = 25636 gas }, } diff --git a/x/blob/keeper/keeper.go b/x/blob/keeper/keeper.go index 585b2f49ed..a3279ddb0e 100644 --- a/x/blob/keeper/keeper.go +++ b/x/blob/keeper/keeper.go @@ -50,7 +50,7 @@ func (k Keeper) PayForBlobs(goCtx context.Context, msg *types.MsgPayForBlobs) (* if ctx.BlockHeader().Version.App <= v2.Version { gasToConsume = types.GasToConsume(msg.BlobSizes, k.GasPerBlobByte(ctx)) } else { - gasToConsume = types.GasToConsume(msg.BlobSizes, appconsts.GasPerBlobByte(appconsts.LatestVersion)) + gasToConsume = types.GasToConsume(msg.BlobSizes, appconsts.GasPerBlobByte(ctx.BlockHeader().Version.App)) } ctx.GasMeter().ConsumeGas(gasToConsume, payForBlobGasDescriptor) diff --git a/x/minfee/grpc_query_test.go b/x/minfee/grpc_query_test.go index a7ab0fdfef..9e797ee828 100644 --- a/x/minfee/grpc_query_test.go +++ b/x/minfee/grpc_query_test.go @@ -4,7 +4,7 @@ import ( "testing" "github.com/celestiaorg/celestia-app/v3/app" - v2 "github.com/celestiaorg/celestia-app/v3/pkg/appconsts/v2" + "github.com/celestiaorg/celestia-app/v3/pkg/appconsts" testutil "github.com/celestiaorg/celestia-app/v3/test/util" "github.com/celestiaorg/celestia-app/v3/x/minfee" sdk "github.com/cosmos/cosmos-sdk/types" @@ -24,5 +24,5 @@ func TestQueryNetworkMinGasPrice(t *testing.T) { require.NoError(t, err) // Check the response - require.Equal(t, v2.NetworkMinGasPrice, resp.NetworkMinGasPrice.MustFloat64()) + require.Equal(t, appconsts.DefaultNetworkMinGasPrice, resp.NetworkMinGasPrice.MustFloat64()) } diff --git a/x/minfee/params.go b/x/minfee/params.go index ef17044d7c..2f1edc3ee8 100644 --- a/x/minfee/params.go +++ b/x/minfee/params.go @@ -3,7 +3,7 @@ package minfee import ( "fmt" - v2 "github.com/celestiaorg/celestia-app/v3/pkg/appconsts/v2" + "github.com/celestiaorg/celestia-app/v3/pkg/appconsts" sdk "github.com/cosmos/cosmos-sdk/types" paramtypes "github.com/cosmos/cosmos-sdk/x/params/types" ) @@ -18,7 +18,7 @@ var ( ) func init() { - DefaultNetworkMinGasPriceDec, err := sdk.NewDecFromStr(fmt.Sprintf("%f", v2.NetworkMinGasPrice)) + DefaultNetworkMinGasPriceDec, err := sdk.NewDecFromStr(fmt.Sprintf("%f", appconsts.DefaultNetworkMinGasPrice)) if err != nil { panic(err) } From 9c48d61d46a4ece3c472e8520eacb0acc86323ef Mon Sep 17 00:00:00 2001 From: Nina Barbakadze Date: Wed, 7 Aug 2024 10:46:55 +0200 Subject: [PATCH 06/20] refactor: address review comments --- app/ante/tx_size.go | 12 ++++++------ app/ante/tx_size_test.go | 9 ++++----- pkg/appconsts/initial_consts.go | 3 ++- x/blob/keeper/keeper.go | 2 +- 4 files changed, 13 insertions(+), 13 deletions(-) diff --git a/app/ante/tx_size.go b/app/ante/tx_size.go index 2ea1d8cd47..4f43a6cde6 100644 --- a/app/ante/tx_size.go +++ b/app/ante/tx_size.go @@ -95,15 +95,15 @@ func (cgts ConsumeTxSizeGasDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, sim } sigBz := legacy.Cdc.MustMarshal(simSig) - cost := sdk.Gas(len(sigBz) + 6) + txBytes := sdk.Gas(len(sigBz) + 6) // If the pubkey is a multi-signature pubkey, then we estimate for the maximum // number of signers. if _, ok := pubkey.(*multisig.LegacyAminoPubKey); ok { - cost *= params.TxSigLimit + txBytes *= params.TxSigLimit } - consumeGasForTxSize(ctx, cost, params) + consumeGasForTxSize(ctx, txBytes, params) } } @@ -135,12 +135,12 @@ func isIncompleteSignature(data signing.SignatureData) bool { // consumeGasForTxSize consumes gas based on the size of the transaction. // It uses different parameters depending on the app version. -func consumeGasForTxSize(ctx sdk.Context, cost uint64, params auth.Params) { +func consumeGasForTxSize(ctx sdk.Context, txBytes uint64, params auth.Params) { // For app v2 and below we should get txSizeCostPerByte from auth module if ctx.BlockHeader().Version.App <= v2.Version { - ctx.GasMeter().ConsumeGas(params.TxSizeCostPerByte*cost, "txSize") + ctx.GasMeter().ConsumeGas(params.TxSizeCostPerByte*txBytes, "txSize") } else { // From v3 onwards, we should get txSizeCostPerByte from appconsts - ctx.GasMeter().ConsumeGas(appconsts.TxSizeCostPerByte(v3.Version)*cost, "txSize") + ctx.GasMeter().ConsumeGas(appconsts.TxSizeCostPerByte(ctx.BlockHeader().Version.App)*txBytes, "txSize") } } diff --git a/app/ante/tx_size_test.go b/app/ante/tx_size_test.go index bdc8e8fe62..a2956573d6 100644 --- a/app/ante/tx_size_test.go +++ b/app/ante/tx_size_test.go @@ -7,7 +7,6 @@ import ( "github.com/celestiaorg/celestia-app/v3/app" "github.com/celestiaorg/celestia-app/v3/app/ante" "github.com/celestiaorg/celestia-app/v3/pkg/appconsts" - v3 "github.com/celestiaorg/celestia-app/v3/pkg/appconsts/v3" testutil "github.com/celestiaorg/celestia-app/v3/test/util" "github.com/cosmos/cosmos-sdk/client" "github.com/cosmos/cosmos-sdk/client/tx" @@ -74,13 +73,13 @@ func TestConsumeGasForTxSize(t *testing.T) { txBuilder.SetMemo(strings.Repeat("01234567890", 10)) privs, accNums, accSeqs := []cryptotypes.PrivKey{priv1}, []uint64{0}, []uint64{0} - tx, err := CreateTestTx(txBuilder, clientCtx, privs, accNums, accSeqs, ctx.ChainID()) + tx, err := createTestTx(txBuilder, clientCtx, privs, accNums, accSeqs, ctx.ChainID()) require.NoError(t, err) txBytes, err := clientCtx.TxConfig.TxJSONEncoder()(tx) require.Nil(t, err, "Cannot marshal tx: %v", err) - expectedGas := sdk.Gas(len(txBytes)) * appconsts.TxSizeCostPerByte(v3.Version) + expectedGas := sdk.Gas(len(txBytes)) * appconsts.TxSizeCostPerByte(appconsts.LatestVersion) // set suite.ctx with TxBytes manually ctx = ctx.WithTxBytes(txBytes) @@ -126,8 +125,8 @@ func TestConsumeGasForTxSize(t *testing.T) { } } -// CreateTestTx creates a test tx given multiple inputs. -func CreateTestTx(txBuilder client.TxBuilder, clientCtx client.Context, privs []cryptotypes.PrivKey, accNums []uint64, accSeqs []uint64, chainID string) (xauthsigning.Tx, error) { +// createTestTx creates a test tx given multiple inputs. +func createTestTx(txBuilder client.TxBuilder, clientCtx client.Context, privs []cryptotypes.PrivKey, accNums []uint64, accSeqs []uint64, chainID string) (xauthsigning.Tx, error) { // First round: we gather all the signer infos. We use the "set empty // signature" hack to do that. sigsV2 := make([]signing.SignatureV2, 0, len(privs)) diff --git a/pkg/appconsts/initial_consts.go b/pkg/appconsts/initial_consts.go index 62188c485b..f4724552b5 100644 --- a/pkg/appconsts/initial_consts.go +++ b/pkg/appconsts/initial_consts.go @@ -23,7 +23,8 @@ const ( // time can be subject to slashing under conditions of misbehavior. DefaultUnbondingTime = 3 * 7 * 24 * time.Hour - // NetworkMinGasPrice is used by x/minfee to prevent transactions from being + // DefaultNetworkMinGasPrice is used by x/minfee to prevent transactions from being // included in a block if they specify a gas price lower than this. + // Only applies to app version >= 2 DefaultNetworkMinGasPrice = 0.000001 // utia ) diff --git a/x/blob/keeper/keeper.go b/x/blob/keeper/keeper.go index a3279ddb0e..a7e0f3cc71 100644 --- a/x/blob/keeper/keeper.go +++ b/x/blob/keeper/keeper.go @@ -45,7 +45,7 @@ func (k Keeper) Logger(ctx sdk.Context) log.Logger { func (k Keeper) PayForBlobs(goCtx context.Context, msg *types.MsgPayForBlobs) (*types.MsgPayForBlobsResponse, error) { ctx := sdk.UnwrapSDKContext(goCtx) - // GasPerBlobByte is a versioned variable from version 3 onwards. + // GasPerBlobByte is a versioned param from version 3 onwards. var gasToConsume uint64 if ctx.BlockHeader().Version.App <= v2.Version { gasToConsume = types.GasToConsume(msg.BlobSizes, k.GasPerBlobByte(ctx)) From b92566e5338e1dd2ce152d843664f524023601ec Mon Sep 17 00:00:00 2001 From: Nina Barbakadze Date: Wed, 7 Aug 2024 11:07:30 +0200 Subject: [PATCH 07/20] docs: update blob readme --- x/blob/README.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/x/blob/README.md b/x/blob/README.md index 620717dcf0..0a7c174124 100644 --- a/x/blob/README.md +++ b/x/blob/README.md @@ -46,9 +46,10 @@ message Params { #### `GasPerBlobByte` -In v1 and v2`GasPerBlobByte` is the amount of gas that is consumed per byte of blob data +`GasPerBlobByte` is the amount of gas that is consumed per byte of blob data when a `MsgPayForBlobs` is processed. Currently, the default value is 8. This value is set below that of normal transaction gas consumption, which is 10. +`GasPerBlobByte` was a governance-modifiable parameter in v1 and v2. In app v3 and above, it is a versioned parameter, meaning it can only be changed through hard fork upgrades. #### `GovMaxSquareSize` From a1d15e8c854c4c462cc917041f240270a34e8235 Mon Sep 17 00:00:00 2001 From: Nina Barbakadze Date: Wed, 7 Aug 2024 11:35:54 +0200 Subject: [PATCH 08/20] fix: build issues after merge --- app/ante/tx_size.go | 1 - app/test/std_sdk_test.go | 4 ++-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/app/ante/tx_size.go b/app/ante/tx_size.go index 4f43a6cde6..c918b850fd 100644 --- a/app/ante/tx_size.go +++ b/app/ante/tx_size.go @@ -6,7 +6,6 @@ import ( "cosmossdk.io/errors" "github.com/celestiaorg/celestia-app/v3/pkg/appconsts" v2 "github.com/celestiaorg/celestia-app/v3/pkg/appconsts/v2" - v3 "github.com/celestiaorg/celestia-app/v3/pkg/appconsts/v3" "github.com/cosmos/cosmos-sdk/codec/legacy" "github.com/cosmos/cosmos-sdk/crypto/keys/multisig" "github.com/cosmos/cosmos-sdk/crypto/keys/secp256k1" diff --git a/app/test/std_sdk_test.go b/app/test/std_sdk_test.go index 85be2e9d7d..7ec3afed38 100644 --- a/app/test/std_sdk_test.go +++ b/app/test/std_sdk_test.go @@ -8,7 +8,7 @@ import ( "github.com/celestiaorg/celestia-app/v3/app" "github.com/celestiaorg/celestia-app/v3/app/encoding" "github.com/celestiaorg/celestia-app/v3/app/grpc/tx" - v2 "github.com/celestiaorg/celestia-app/v3/pkg/appconsts/v2" + "github.com/celestiaorg/celestia-app/v3/pkg/appconsts" "github.com/celestiaorg/celestia-app/v3/pkg/user" "github.com/celestiaorg/celestia-app/v3/test/util/blobfactory" "github.com/celestiaorg/celestia-app/v3/test/util/testfactory" @@ -339,7 +339,7 @@ func (s *StandardSDKIntegrationTestSuite) TestGRPCQueries() { require.NoError(t, err) got, err := resp.NetworkMinGasPrice.Float64() require.NoError(t, err) - assert.Equal(t, v2.NetworkMinGasPrice, got) + assert.Equal(t, appconsts.DefaultNetworkMinGasPrice, got) }) t.Run("testnode can query local min gas price", func(t *testing.T) { serviceClient := nodeservice.NewServiceClient(s.cctx.GRPCClient) From a3a09b8c47f3326f114a13036b627290f1123ea6 Mon Sep 17 00:00:00 2001 From: Nina Barbakadze Date: Thu, 8 Aug 2024 18:52:46 +0200 Subject: [PATCH 09/20] docs: add a comment in tx_size.go --- app/ante/tx_size.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/app/ante/tx_size.go b/app/ante/tx_size.go index c918b850fd..0f6c1ba919 100644 --- a/app/ante/tx_size.go +++ b/app/ante/tx_size.go @@ -42,6 +42,11 @@ func init() { // in or empty. // CONTRACT: To use this decorator, signatures of transaction must be represented // as legacytx.StdSignature otherwise simulate mode will incorrectly estimate gas cost. + +// The code was copied from celestia's fork of the cosmos-sdk: +// https://github.com/celestiaorg/cosmos-sdk/blob/release/v0.46.x-celestia/x/auth/ante/basic.go +// In app versions v2 and below, the txSizeCostPerByte used for gas cost estimation is taken from the auth module. +// In app v3 and above, the versioned constant appconsts.TxSizeCostPerByte is used. type ConsumeTxSizeGasDecorator struct { ak ante.AccountKeeper } From 055b70608a7559ac66f8a8777138b62f36bd9410 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?nina=20/=20=E1=83=9C=E1=83=98=E1=83=9C=E1=83=90?= Date: Thu, 8 Aug 2024 19:05:12 +0200 Subject: [PATCH 10/20] Update keeper_test.go --- x/blob/keeper/keeper_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/x/blob/keeper/keeper_test.go b/x/blob/keeper/keeper_test.go index 5a5f825d4d..9c9aba2764 100644 --- a/x/blob/keeper/keeper_test.go +++ b/x/blob/keeper/keeper_test.go @@ -43,6 +43,7 @@ func TestPayForBlobs(t *testing.T) { namespaces := [][]byte{namespace.Bytes()} blobData := []byte("blob") blobSizes := []uint32{uint32(len(blobData))} + // verify no events exist yet events := ctx.EventManager().Events().ToABCIEvents() assert.Len(t, events, 0) From a6e69cd48c526394b40df0624df340ac1d3ffe2a Mon Sep 17 00:00:00 2001 From: Nina Barbakadze Date: Thu, 8 Aug 2024 19:07:12 +0200 Subject: [PATCH 11/20] style: make linter happy --- x/blob/keeper/keeper_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x/blob/keeper/keeper_test.go b/x/blob/keeper/keeper_test.go index 9c9aba2764..1f36d1727d 100644 --- a/x/blob/keeper/keeper_test.go +++ b/x/blob/keeper/keeper_test.go @@ -43,7 +43,7 @@ func TestPayForBlobs(t *testing.T) { namespaces := [][]byte{namespace.Bytes()} blobData := []byte("blob") blobSizes := []uint32{uint32(len(blobData))} - + // verify no events exist yet events := ctx.EventManager().Events().ToABCIEvents() assert.Len(t, events, 0) From 2ddaa9f97df64e703e04ce12378b607963ef6265 Mon Sep 17 00:00:00 2001 From: Nina Barbakadze Date: Thu, 8 Aug 2024 22:35:56 +0200 Subject: [PATCH 12/20] test: add v3 version in tx size test --- app/ante/tx_size_test.go | 173 +++++++++++++++++++++------------------ 1 file changed, 92 insertions(+), 81 deletions(-) diff --git a/app/ante/tx_size_test.go b/app/ante/tx_size_test.go index a2956573d6..ae60760914 100644 --- a/app/ante/tx_size_test.go +++ b/app/ante/tx_size_test.go @@ -1,12 +1,15 @@ package ante_test import ( + "fmt" "strings" "testing" "github.com/celestiaorg/celestia-app/v3/app" "github.com/celestiaorg/celestia-app/v3/app/ante" "github.com/celestiaorg/celestia-app/v3/pkg/appconsts" + v2 "github.com/celestiaorg/celestia-app/v3/pkg/appconsts/v2" + v3 "github.com/celestiaorg/celestia-app/v3/pkg/appconsts/v3" testutil "github.com/celestiaorg/celestia-app/v3/test/util" "github.com/cosmos/cosmos-sdk/client" "github.com/cosmos/cosmos-sdk/client/tx" @@ -20,11 +23,14 @@ import ( authtypes "github.com/cosmos/cosmos-sdk/x/auth/types" "github.com/stretchr/testify/require" tmproto "github.com/tendermint/tendermint/proto/tendermint/types" + "github.com/tendermint/tendermint/proto/tendermint/version" ) -func setup() (*app.App, sdk.Context, client.Context, error) { +func setup(appVersion uint64) (*app.App, sdk.Context, client.Context, error) { app, _, _ := testutil.NewTestAppWithGenesisSet(app.DefaultConsensusParams()) - ctx := app.NewContext(false, tmproto.Header{}) + ctx := app.NewContext(false, tmproto.Header{Version: version.Consensus{ + App: appVersion, + }}) app.AccountKeeper.SetParams(ctx, authtypes.DefaultParams()) ctx = ctx.WithBlockHeight(1) @@ -41,86 +47,91 @@ func setup() (*app.App, sdk.Context, client.Context, error) { } func TestConsumeGasForTxSize(t *testing.T) { - app, ctx, clientCtx, err := setup() - require.NoError(t, err) - var txBuilder client.TxBuilder - - // keys and addresses - priv1, _, addr1 := testdata.KeyTestPubAddr() - - // msg and signatures - msg := testdata.NewTestMsg(addr1) - feeAmount := testdata.NewTestFeeAmount() - gasLimit := testdata.NewTestGasLimit() - - cgtsd := ante.NewConsumeGasForTxSizeDecorator(app.AccountKeeper) - antehandler := sdk.ChainAnteDecorators(cgtsd) - - testCases := []struct { - name string - sigV2 signing.SignatureV2 - }{ - {"SingleSignatureData", signing.SignatureV2{PubKey: priv1.PubKey()}}, - {"MultiSignatureData", signing.SignatureV2{PubKey: priv1.PubKey(), Data: multisig.NewMultisig(2)}}, - } - - for _, tc := range testCases { - t.Run(tc.name, func(t *testing.T) { - txBuilder = clientCtx.TxConfig.NewTxBuilder() - require.NoError(t, txBuilder.SetMsgs(msg)) - txBuilder.SetFeeAmount(feeAmount) - txBuilder.SetGasLimit(gasLimit) - txBuilder.SetMemo(strings.Repeat("01234567890", 10)) - - privs, accNums, accSeqs := []cryptotypes.PrivKey{priv1}, []uint64{0}, []uint64{0} - tx, err := createTestTx(txBuilder, clientCtx, privs, accNums, accSeqs, ctx.ChainID()) - require.NoError(t, err) - - txBytes, err := clientCtx.TxConfig.TxJSONEncoder()(tx) - require.Nil(t, err, "Cannot marshal tx: %v", err) - - expectedGas := sdk.Gas(len(txBytes)) * appconsts.TxSizeCostPerByte(appconsts.LatestVersion) - - // set suite.ctx with TxBytes manually - ctx = ctx.WithTxBytes(txBytes) - - // track how much gas is necessary to retrieve parameters - beforeGas := ctx.GasMeter().GasConsumed() - app.AccountKeeper.GetParams(ctx) - afterGas := ctx.GasMeter().GasConsumed() - expectedGas += afterGas - beforeGas - - beforeGas = ctx.GasMeter().GasConsumed() - ctx, err = antehandler(ctx, tx, false) - require.Nil(t, err, "ConsumeTxSizeGasDecorator returned error: %v", err) - - // require that decorator consumes expected amount of gas - consumedGas := ctx.GasMeter().GasConsumed() - beforeGas - require.Equal(t, expectedGas, consumedGas, "Decorator did not consume the correct amount of gas") - - // simulation must not underestimate gas of this decorator even with nil signatures - txBuilder, err := clientCtx.TxConfig.WrapTxBuilder(tx) + versions := []uint64{v2.Version, v3.Version} + for _, version := range versions { + t.Run("AppVersion-"+fmt.Sprint(version), func(t *testing.T) { + app, ctx, clientCtx, err := setup(version) require.NoError(t, err) - require.NoError(t, txBuilder.SetSignatures(tc.sigV2)) - tx = txBuilder.GetTx() - - simTxBytes, err := clientCtx.TxConfig.TxJSONEncoder()(tx) - require.Nil(t, err, "Cannot marshal tx: %v", err) - // require that simulated tx is smaller than tx with signatures - require.True(t, len(simTxBytes) < len(txBytes), "simulated tx still has signatures") - - // Set suite.ctx with smaller simulated TxBytes manually - ctx = ctx.WithTxBytes(simTxBytes) - - beforeSimGas := ctx.GasMeter().GasConsumed() - - // run antehandler with simulate=true - ctx, err = antehandler(ctx, tx, true) - consumedSimGas := ctx.GasMeter().GasConsumed() - beforeSimGas - - // require that antehandler passes and does not underestimate decorator cost - require.Nil(t, err, "ConsumeTxSizeGasDecorator returned error: %v", err) - require.True(t, consumedSimGas >= expectedGas, "Simulate mode underestimates gas on AnteDecorator. Simulated cost: %d, expected cost: %d", consumedSimGas, expectedGas) + var txBuilder client.TxBuilder + + // keys and addresses + priv1, _, addr1 := testdata.KeyTestPubAddr() + + // msg and signatures + msg := testdata.NewTestMsg(addr1) + feeAmount := testdata.NewTestFeeAmount() + gasLimit := testdata.NewTestGasLimit() + + cgtsd := ante.NewConsumeGasForTxSizeDecorator(app.AccountKeeper) + antehandler := sdk.ChainAnteDecorators(cgtsd) + + testCases := []struct { + name string + sigV2 signing.SignatureV2 + }{ + {"SingleSignatureData", signing.SignatureV2{PubKey: priv1.PubKey()}}, + {"MultiSignatureData", signing.SignatureV2{PubKey: priv1.PubKey(), Data: multisig.NewMultisig(2)}}, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + txBuilder = clientCtx.TxConfig.NewTxBuilder() + require.NoError(t, txBuilder.SetMsgs(msg)) + txBuilder.SetFeeAmount(feeAmount) + txBuilder.SetGasLimit(gasLimit) + txBuilder.SetMemo(strings.Repeat("01234567890", 10)) + + privs, accNums, accSeqs := []cryptotypes.PrivKey{priv1}, []uint64{0}, []uint64{0} + tx, err := createTestTx(txBuilder, clientCtx, privs, accNums, accSeqs, ctx.ChainID()) + require.NoError(t, err) + + txBytes, err := clientCtx.TxConfig.TxJSONEncoder()(tx) + require.Nil(t, err, "Cannot marshal tx: %v", err) + + expectedGas := sdk.Gas(len(txBytes)) * appconsts.TxSizeCostPerByte(appconsts.LatestVersion) + + // set suite.ctx with TxBytes manually + ctx = ctx.WithTxBytes(txBytes) + + // track how much gas is necessary to retrieve parameters + beforeGas := ctx.GasMeter().GasConsumed() + app.AccountKeeper.GetParams(ctx) + afterGas := ctx.GasMeter().GasConsumed() + expectedGas += afterGas - beforeGas + + beforeGas = ctx.GasMeter().GasConsumed() + ctx, err = antehandler(ctx, tx, false) + require.Nil(t, err, "ConsumeTxSizeGasDecorator returned error: %v", err) + + // require that decorator consumes expected amount of gas + consumedGas := ctx.GasMeter().GasConsumed() - beforeGas + require.Equal(t, expectedGas, consumedGas, "Decorator did not consume the correct amount of gas") + + // simulation must not underestimate gas of this decorator even with nil signatures + txBuilder, err := clientCtx.TxConfig.WrapTxBuilder(tx) + require.NoError(t, err) + require.NoError(t, txBuilder.SetSignatures(tc.sigV2)) + tx = txBuilder.GetTx() + + simTxBytes, err := clientCtx.TxConfig.TxJSONEncoder()(tx) + require.Nil(t, err, "Cannot marshal tx: %v", err) + // require that simulated tx is smaller than tx with signatures + require.True(t, len(simTxBytes) < len(txBytes), "simulated tx still has signatures") + + // Set suite.ctx with smaller simulated TxBytes manually + ctx = ctx.WithTxBytes(simTxBytes) + + beforeSimGas := ctx.GasMeter().GasConsumed() + + // run antehandler with simulate=true + ctx, err = antehandler(ctx, tx, true) + consumedSimGas := ctx.GasMeter().GasConsumed() - beforeSimGas + + // require that antehandler passes and does not underestimate decorator cost + require.Nil(t, err, "ConsumeTxSizeGasDecorator returned error: %v", err) + require.True(t, consumedSimGas >= expectedGas, "Simulate mode underestimates gas on AnteDecorator. Simulated cost: %d, expected cost: %d", consumedSimGas, expectedGas) + }) + } }) } } From 21ec97919f929d7b4e1f3180cc0cd59c9f663039 Mon Sep 17 00:00:00 2001 From: Nina Barbakadze Date: Fri, 9 Aug 2024 09:43:30 +0200 Subject: [PATCH 13/20] test: make tx size test better --- app/ante/tx_size_test.go | 178 +++++++++++++++++++-------------------- 1 file changed, 89 insertions(+), 89 deletions(-) diff --git a/app/ante/tx_size_test.go b/app/ante/tx_size_test.go index ae60760914..0ce8fd66d6 100644 --- a/app/ante/tx_size_test.go +++ b/app/ante/tx_size_test.go @@ -1,7 +1,6 @@ package ante_test import ( - "fmt" "strings" "testing" @@ -26,11 +25,9 @@ import ( "github.com/tendermint/tendermint/proto/tendermint/version" ) -func setup(appVersion uint64) (*app.App, sdk.Context, client.Context, error) { +func setup() (*app.App, sdk.Context, client.Context, error) { app, _, _ := testutil.NewTestAppWithGenesisSet(app.DefaultConsensusParams()) - ctx := app.NewContext(false, tmproto.Header{Version: version.Consensus{ - App: appVersion, - }}) + ctx := app.NewContext(false, tmproto.Header{}) app.AccountKeeper.SetParams(ctx, authtypes.DefaultParams()) ctx = ctx.WithBlockHeight(1) @@ -47,91 +44,94 @@ func setup(appVersion uint64) (*app.App, sdk.Context, client.Context, error) { } func TestConsumeGasForTxSize(t *testing.T) { - versions := []uint64{v2.Version, v3.Version} - for _, version := range versions { - t.Run("AppVersion-"+fmt.Sprint(version), func(t *testing.T) { - app, ctx, clientCtx, err := setup(version) + app, ctx, clientCtx, err := setup() + require.NoError(t, err) + var txBuilder client.TxBuilder + + // keys and addresses + priv1, _, addr1 := testdata.KeyTestPubAddr() + + // msg and signatures + msg := testdata.NewTestMsg(addr1) + feeAmount := testdata.NewTestFeeAmount() + gasLimit := testdata.NewTestGasLimit() + + cgtsd := ante.NewConsumeGasForTxSizeDecorator(app.AccountKeeper) + antehandler := sdk.ChainAnteDecorators(cgtsd) + + testCases := []struct { + version uint64 + name string + sigV2 signing.SignatureV2 + }{ + {v2.Version, "SingleSignatureData", signing.SignatureV2{PubKey: priv1.PubKey()}}, + {v2.Version, "MultiSignatureData", signing.SignatureV2{PubKey: priv1.PubKey(), Data: multisig.NewMultisig(2)}}, + {v3.Version, "SingleSignatureData", signing.SignatureV2{PubKey: priv1.PubKey()}}, + {v3.Version, "MultiSignatureData", signing.SignatureV2{PubKey: priv1.PubKey(), Data: multisig.NewMultisig(2)}}, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + // set the version + ctx = app.NewContext(false, tmproto.Header{Version: version.Consensus{ + App: tc.version, + }}) + + txBuilder = clientCtx.TxConfig.NewTxBuilder() + require.NoError(t, txBuilder.SetMsgs(msg)) + txBuilder.SetFeeAmount(feeAmount) + txBuilder.SetGasLimit(gasLimit) + txBuilder.SetMemo(strings.Repeat("01234567890", 10)) + + privs, accNums, accSeqs := []cryptotypes.PrivKey{priv1}, []uint64{0}, []uint64{0} + tx, err := createTestTx(txBuilder, clientCtx, privs, accNums, accSeqs, ctx.ChainID()) require.NoError(t, err) - var txBuilder client.TxBuilder - - // keys and addresses - priv1, _, addr1 := testdata.KeyTestPubAddr() - - // msg and signatures - msg := testdata.NewTestMsg(addr1) - feeAmount := testdata.NewTestFeeAmount() - gasLimit := testdata.NewTestGasLimit() - - cgtsd := ante.NewConsumeGasForTxSizeDecorator(app.AccountKeeper) - antehandler := sdk.ChainAnteDecorators(cgtsd) - - testCases := []struct { - name string - sigV2 signing.SignatureV2 - }{ - {"SingleSignatureData", signing.SignatureV2{PubKey: priv1.PubKey()}}, - {"MultiSignatureData", signing.SignatureV2{PubKey: priv1.PubKey(), Data: multisig.NewMultisig(2)}}, - } - - for _, tc := range testCases { - t.Run(tc.name, func(t *testing.T) { - txBuilder = clientCtx.TxConfig.NewTxBuilder() - require.NoError(t, txBuilder.SetMsgs(msg)) - txBuilder.SetFeeAmount(feeAmount) - txBuilder.SetGasLimit(gasLimit) - txBuilder.SetMemo(strings.Repeat("01234567890", 10)) - - privs, accNums, accSeqs := []cryptotypes.PrivKey{priv1}, []uint64{0}, []uint64{0} - tx, err := createTestTx(txBuilder, clientCtx, privs, accNums, accSeqs, ctx.ChainID()) - require.NoError(t, err) - - txBytes, err := clientCtx.TxConfig.TxJSONEncoder()(tx) - require.Nil(t, err, "Cannot marshal tx: %v", err) - - expectedGas := sdk.Gas(len(txBytes)) * appconsts.TxSizeCostPerByte(appconsts.LatestVersion) - - // set suite.ctx with TxBytes manually - ctx = ctx.WithTxBytes(txBytes) - - // track how much gas is necessary to retrieve parameters - beforeGas := ctx.GasMeter().GasConsumed() - app.AccountKeeper.GetParams(ctx) - afterGas := ctx.GasMeter().GasConsumed() - expectedGas += afterGas - beforeGas - - beforeGas = ctx.GasMeter().GasConsumed() - ctx, err = antehandler(ctx, tx, false) - require.Nil(t, err, "ConsumeTxSizeGasDecorator returned error: %v", err) - - // require that decorator consumes expected amount of gas - consumedGas := ctx.GasMeter().GasConsumed() - beforeGas - require.Equal(t, expectedGas, consumedGas, "Decorator did not consume the correct amount of gas") - - // simulation must not underestimate gas of this decorator even with nil signatures - txBuilder, err := clientCtx.TxConfig.WrapTxBuilder(tx) - require.NoError(t, err) - require.NoError(t, txBuilder.SetSignatures(tc.sigV2)) - tx = txBuilder.GetTx() - - simTxBytes, err := clientCtx.TxConfig.TxJSONEncoder()(tx) - require.Nil(t, err, "Cannot marshal tx: %v", err) - // require that simulated tx is smaller than tx with signatures - require.True(t, len(simTxBytes) < len(txBytes), "simulated tx still has signatures") - - // Set suite.ctx with smaller simulated TxBytes manually - ctx = ctx.WithTxBytes(simTxBytes) - - beforeSimGas := ctx.GasMeter().GasConsumed() - - // run antehandler with simulate=true - ctx, err = antehandler(ctx, tx, true) - consumedSimGas := ctx.GasMeter().GasConsumed() - beforeSimGas - - // require that antehandler passes and does not underestimate decorator cost - require.Nil(t, err, "ConsumeTxSizeGasDecorator returned error: %v", err) - require.True(t, consumedSimGas >= expectedGas, "Simulate mode underestimates gas on AnteDecorator. Simulated cost: %d, expected cost: %d", consumedSimGas, expectedGas) - }) - } + + txBytes, err := clientCtx.TxConfig.TxJSONEncoder()(tx) + require.Nil(t, err, "Cannot marshal tx: %v", err) + + expectedGas := sdk.Gas(len(txBytes)) * appconsts.TxSizeCostPerByte(appconsts.LatestVersion) + + // set suite.ctx with TxBytes manually + ctx = ctx.WithTxBytes(txBytes) + + // track how much gas is necessary to retrieve parameters + beforeGas := ctx.GasMeter().GasConsumed() + app.AccountKeeper.GetParams(ctx) + afterGas := ctx.GasMeter().GasConsumed() + expectedGas += afterGas - beforeGas + + beforeGas = ctx.GasMeter().GasConsumed() + ctx, err = antehandler(ctx, tx, false) + require.Nil(t, err, "ConsumeTxSizeGasDecorator returned error: %v", err) + + // require that decorator consumes expected amount of gas + consumedGas := ctx.GasMeter().GasConsumed() - beforeGas + require.Equal(t, expectedGas, consumedGas, "Decorator did not consume the correct amount of gas") + + // simulation must not underestimate gas of this decorator even with nil signatures + txBuilder, err := clientCtx.TxConfig.WrapTxBuilder(tx) + require.NoError(t, err) + require.NoError(t, txBuilder.SetSignatures(tc.sigV2)) + tx = txBuilder.GetTx() + + simTxBytes, err := clientCtx.TxConfig.TxJSONEncoder()(tx) + require.Nil(t, err, "Cannot marshal tx: %v", err) + // require that simulated tx is smaller than tx with signatures + require.True(t, len(simTxBytes) < len(txBytes), "simulated tx still has signatures") + + // Set suite.ctx with smaller simulated TxBytes manually + ctx = ctx.WithTxBytes(simTxBytes) + + beforeSimGas := ctx.GasMeter().GasConsumed() + + // run antehandler with simulate=true + ctx, err = antehandler(ctx, tx, true) + consumedSimGas := ctx.GasMeter().GasConsumed() - beforeSimGas + + // require that antehandler passes and does not underestimate decorator cost + require.Nil(t, err, "ConsumeTxSizeGasDecorator returned error: %v", err) + require.True(t, consumedSimGas >= expectedGas, "Simulate mode underestimates gas on AnteDecorator. Simulated cost: %d, expected cost: %d", consumedSimGas, expectedGas) }) } } From 8547221c6bceaa8090b8d02c8b7b832a3cbd0117 Mon Sep 17 00:00:00 2001 From: Nina Barbakadze Date: Tue, 17 Sep 2024 20:29:12 +0200 Subject: [PATCH 14/20] feat: apply requested changes --- app/ante/tx_size_test.go | 11 +++--- specs/src/parameters_v3.md | 72 ++++++++++++++++++++++++++++++++++++ x/blob/keeper/keeper_test.go | 65 ++++++++++++++------------------ 3 files changed, 104 insertions(+), 44 deletions(-) create mode 100644 specs/src/parameters_v3.md diff --git a/app/ante/tx_size_test.go b/app/ante/tx_size_test.go index 0ce8fd66d6..326cd3f2b2 100644 --- a/app/ante/tx_size_test.go +++ b/app/ante/tx_size_test.go @@ -1,14 +1,13 @@ package ante_test import ( + "fmt" "strings" "testing" - "github.com/celestiaorg/celestia-app/v3/app" "github.com/celestiaorg/celestia-app/v3/app/ante" "github.com/celestiaorg/celestia-app/v3/pkg/appconsts" v2 "github.com/celestiaorg/celestia-app/v3/pkg/appconsts/v2" - v3 "github.com/celestiaorg/celestia-app/v3/pkg/appconsts/v3" testutil "github.com/celestiaorg/celestia-app/v3/test/util" "github.com/cosmos/cosmos-sdk/client" "github.com/cosmos/cosmos-sdk/client/tx" @@ -64,10 +63,10 @@ func TestConsumeGasForTxSize(t *testing.T) { name string sigV2 signing.SignatureV2 }{ - {v2.Version, "SingleSignatureData", signing.SignatureV2{PubKey: priv1.PubKey()}}, - {v2.Version, "MultiSignatureData", signing.SignatureV2{PubKey: priv1.PubKey(), Data: multisig.NewMultisig(2)}}, - {v3.Version, "SingleSignatureData", signing.SignatureV2{PubKey: priv1.PubKey()}}, - {v3.Version, "MultiSignatureData", signing.SignatureV2{PubKey: priv1.PubKey(), Data: multisig.NewMultisig(2)}}, + {v2.Version, "SingleSignatureData v2", signing.SignatureV2{PubKey: priv1.PubKey()}}, + {v2.Version, "MultiSignatureData v2", signing.SignatureV2{PubKey: priv1.PubKey(), Data: multisig.NewMultisig(2)}}, + {appconsts.LatestVersion, fmt.Sprintf("SingleSignatureData v%d", appconsts.LatestVersion), signing.SignatureV2{PubKey: priv1.PubKey()}}, + {appconsts.LatestVersion, fmt.Sprintf("MultiSignatureData v%d", appconsts.LatestVersion), signing.SignatureV2{PubKey: priv1.PubKey(), Data: multisig.NewMultisig(2)}}, } for _, tc := range testCases { diff --git a/specs/src/parameters_v3.md b/specs/src/parameters_v3.md new file mode 100644 index 0000000000..491e4834c1 --- /dev/null +++ b/specs/src/parameters_v3.md @@ -0,0 +1,72 @@ +# Parameters v3 + +The parameters below represent the parameters for app version 3. + +Note that not all of these parameters are changeable via governance. This list +also includes parameter that require a hardfork to change due to being manually +hardcoded in the application or they are blocked by the `x/paramfilter` module. + +## Global parameters + +| Parameter | Default | Summary | Changeable via Governance | +|-------------------|---------|------------------------------------------------------------------------------------------------------------------------|---------------------------| +| MaxBlockSizeBytes | 100MiB | Hardcoded value in CometBFT for the protobuf encoded block. | False | +| MaxSquareSize | 128 | Hardcoded maximum square size determined per shares per row or column for the original data square (not yet extended). | False | + +## Module parameters + +| Module.Parameter | Default | Summary | Changeable via Governance | +|-----------------------------------------------|---------------------------------------------|-------------------------------------------------------------------------------------------------------------------------------------|---------------------------| +| auth.MaxMemoCharacters | 256 | Largest allowed size for a memo in bytes. | True | +| auth.SigVerifyCostED25519 | 590 | Gas used to verify Ed25519 signature. | True | +| auth.SigVerifyCostSecp256k1 | 1000 | Gas used to verify secp256k1 signature. | True | +| auth.TxSigLimit | 7 | Max number of signatures allowed in a multisig transaction. | True | +| auth.TxSizeCostPerByte | 10 | Gas used per transaction byte. | False | +| bank.SendEnabled | true | Allow transfers. | False | +| blob.GasPerBlobByte | 8 | Gas used per blob byte. | False | +| blob.GovMaxSquareSize | 64 | Governance parameter for the maximum square size of the original data square. | True | +| consensus.block.MaxBytes | 1974272 bytes (~1.88 MiB) | Governance parameter for the maximum size of the protobuf encoded block. | True | +| consensus.block.MaxGas | -1 | Maximum gas allowed per block (-1 is infinite). | True | +| consensus.block.TimeIotaMs | 1000 | Minimum time added to the time in the header each block. | False | +| consensus.evidence.MaxAgeDuration | 1814400000000000 (21 days) | The maximum age of evidence before it is considered invalid in nanoseconds. This value should be identical to the unbonding period. | True | +| consensus.evidence.MaxAgeNumBlocks | 120960 | The maximum number of blocks before evidence is considered invalid. This value will stop CometBFT from pruning block data. | True | +| consensus.evidence.MaxBytes | 1MiB | Maximum size in bytes used by evidence in a given block. | True | +| consensus.validator.PubKeyTypes | Ed25519 | The type of public key used by validators. | False | +| consensus.Version.AppVersion | 2 | Determines protocol rules used for a given height. Incremented by the application upon an upgrade. | True | +| distribution.BaseProposerReward | 0 | Reward in the mint denomination for proposing a block. | True | +| distribution.BonusProposerReward | 0 | Extra reward in the mint denomination for proposers based on the voting power included in the commit. | True | +| distribution.CommunityTax | 0.02 (2%) | Percentage of the inflation sent to the community pool. | True | +| distribution.WithdrawAddrEnabled | true | Enables delegators to withdraw funds to a different address. | True | +| gov.DepositParams.MaxDepositPeriod | 604800000000000 (1 week) | Maximum period for token holders to deposit on a proposal in nanoseconds. | True | +| gov.DepositParams.MinDeposit | 10_000_000_000 utia (10,000 TIA) | Minimum deposit for a proposal to enter voting period. | True | +| gov.TallyParams.Quorum | 0.334 (33.4%) | Minimum percentage of total stake needed to vote for a result to be considered valid. | True | +| gov.TallyParams.Threshold | 0.50 (50%) | Minimum proportion of Yes votes for proposal to pass. | True | +| gov.TallyParams.VetoThreshold | 0.334 (33.4%) | Minimum value of Veto votes to Total votes ratio for proposal to be vetoed. | True | +| gov.VotingParams.VotingPeriod | 604800000000000 (1 week) | Duration of the voting period in nanoseconds. | True | +| ibc.ClientGenesis.AllowedClients | []string{"06-solomachine", "07-tendermint"} | List of allowed IBC light clients. | True | +| ibc.ConnectionGenesis.MaxExpectedTimePerBlock | 7500000000000 (75 seconds) | Maximum expected time per block in nanoseconds under normal operation. | True | +| ibc.Transfer.ReceiveEnabled | true | Enable receiving tokens via IBC. | True | +| ibc.Transfer.SendEnabled | true | Enable sending tokens via IBC. | True | +| icahost.HostEnabled | True | Enables or disables the Inter-Chain Accounts host module. | True | +| icahost.AllowMessages | [icaAllowMessages] | Defines a list of sdk message typeURLs allowed to be executed on a host chain. | True | +| minfee.NetworkMinGasPrice | 0.000001 utia | All transactions must have a gas price greater than or equal to this value. | True | +| mint.BondDenom | utia | Denomination that is inflated and sent to the distribution module account. | False | +| mint.DisinflationRate | 0.10 (10%) | The rate at which the inflation rate decreases each year. | False | +| mint.InitialInflationRate | 0.08 (8%) | The inflation rate the network starts at. | False | +| mint.TargetInflationRate | 0.015 (1.5%) | The inflation rate that the network aims to stabilize at. | False | +| packetfowardmiddleware.FeePercentage | 0 | % of the forwarded packet amount which will be subtracted and distributed to the community pool. | True | +| slashing.DowntimeJailDuration | 1 min | Duration of time a validator must stay jailed. | True | +| slashing.MinSignedPerWindow | 0.75 (75%) | The percentage of SignedBlocksWindow that must be signed not to get jailed. | True | +| slashing.SignedBlocksWindow | 5000 | The range of blocks used to count for downtime. | True | +| slashing.SlashFractionDoubleSign | 0.02 (2%) | Percentage slashed after a validator is jailed for double signing. | True | +| slashing.SlashFractionDowntime | 0.00 (0%) | Percentage slashed after a validator is jailed for downtime. | True | +| staking.BondDenom | utia | Bondable coin denomination. | False | +| staking.HistoricalEntries | 10000 | Number of historical entries to persist in store. | True | +| staking.MaxEntries | 7 | Maximum number of entries in the redelegation queue. | True | +| staking.MaxValidators | 100 | Maximum number of validators. | True | +| staking.MinCommissionRate | 0.05 (5%) | Minimum commission rate used by all validators. | True | +| staking.UnbondingTime | 1814400 (21 days) | Duration of time for unbonding in seconds. | False | + +Note: none of the mint module parameters are governance modifiable because they have been converted into hardcoded constants. See the x/mint README.md for more details. + +[icaAllowMessages]: https://github.com/rootulp/celestia-app/blob/8caa5807df8d15477554eba953bd056ae72d4503/app/ica_host.go#L3-L18 diff --git a/x/blob/keeper/keeper_test.go b/x/blob/keeper/keeper_test.go index 79a0b23511..6aedaa3a66 100644 --- a/x/blob/keeper/keeper_test.go +++ b/x/blob/keeper/keeper_test.go @@ -6,8 +6,6 @@ import ( "testing" "github.com/celestiaorg/celestia-app/v3/pkg/appconsts" - v2 "github.com/celestiaorg/celestia-app/v3/pkg/appconsts/v2" - v3 "github.com/celestiaorg/celestia-app/v3/pkg/appconsts/v3" testutil "github.com/celestiaorg/celestia-app/v3/test/util" "github.com/celestiaorg/celestia-app/v3/x/blob/keeper" "github.com/celestiaorg/celestia-app/v3/x/blob/types" @@ -29,44 +27,34 @@ import ( // TestPayForBlobs verifies the attributes on the emitted event. func TestPayForBlobs(t *testing.T) { - // Testing transition from v2 -> v3 - // v2 uses the GasPerBlobByte key from the params store - // v3 uses the GasPerBlobByte from the appconsts - // TODO: Replace this with appversion latest - versions := []uint64{v2.Version, v3.Version} - - for _, version := range versions { - t.Run("AppVersion_"+fmt.Sprint(version), func(t *testing.T) { - k, _, ctx := CreateKeeper(t, version) - signer := "celestia15drmhzw5kwgenvemy30rqqqgq52axf5wwrruf7" - namespace := share.MustNewV0Namespace(bytes.Repeat([]byte{1}, share.NamespaceVersionZeroIDSize)) - namespaces := [][]byte{namespace.Bytes()} - blobData := []byte("blob") - blobSizes := []uint32{uint32(len(blobData))} - - // verify no events exist yet - events := ctx.EventManager().Events().ToABCIEvents() - assert.Len(t, events, 0) - - // emit an event by submitting a PayForBlob - msg := createMsgPayForBlob(t, signer, namespace, blobData) - _, err := k.PayForBlobs(ctx, msg) - require.NoError(t, err) + k, _, ctx := CreateKeeper(t, appconsts.LatestVersion) + signer := "celestia15drmhzw5kwgenvemy30rqqqgq52axf5wwrruf7" + namespace := share.MustNewV0Namespace(bytes.Repeat([]byte{1}, share.NamespaceVersionZeroIDSize)) + namespaces := [][]byte{namespace.Bytes()} + blobData := []byte("blob") + blobSizes := []uint32{uint32(len(blobData))} + + // verify no events exist yet + events := ctx.EventManager().Events().ToABCIEvents() + assert.Len(t, events, 0) + + // emit an event by submitting a PayForBlob + msg := createMsgPayForBlob(t, signer, namespace, blobData) + _, err := k.PayForBlobs(ctx, msg) + require.NoError(t, err) - // verify that an event was emitted - events = ctx.EventManager().Events().ToABCIEvents() - assert.Len(t, events, 1) - protoEvent, err := sdk.ParseTypedEvent(events[0]) - require.NoError(t, err) - event, err := convertToEventPayForBlobs(protoEvent) - require.NoError(t, err) + // verify that an event was emitted + events = ctx.EventManager().Events().ToABCIEvents() + assert.Len(t, events, 1) + protoEvent, err := sdk.ParseTypedEvent(events[0]) + require.NoError(t, err) + event, err := convertToEventPayForBlobs(protoEvent) + require.NoError(t, err) - // verify the attributes of the event - assert.Equal(t, signer, event.Signer) - assert.Equal(t, namespaces, event.Namespaces) - assert.Equal(t, blobSizes, event.BlobSizes) - }) - } + // verify the attributes of the event + assert.Equal(t, signer, event.Signer) + assert.Equal(t, namespaces, event.Namespaces) + assert.Equal(t, blobSizes, event.BlobSizes) } func convertToEventPayForBlobs(message proto.Message) (*types.EventPayForBlobs, error) { @@ -113,6 +101,7 @@ func CreateKeeper(t *testing.T, version uint64) (*keeper.Keeper, store.CommitMul cdc, paramsSubspace, ) + // change the params to a non default value k.SetParams(ctx, types.DefaultParams()) return k, stateStore, ctx From 0f3282af6efc6e41c4742363d44135c79e3a62ae Mon Sep 17 00:00:00 2001 From: Nina Barbakadze Date: Tue, 17 Sep 2024 20:32:06 +0200 Subject: [PATCH 15/20] style: lint --- app/ante/tx_size_test.go | 1 + x/blob/ante/ante_test.go | 1 + 2 files changed, 2 insertions(+) diff --git a/app/ante/tx_size_test.go b/app/ante/tx_size_test.go index 326cd3f2b2..0756524bd7 100644 --- a/app/ante/tx_size_test.go +++ b/app/ante/tx_size_test.go @@ -4,6 +4,7 @@ import ( "fmt" "strings" "testing" + "github.com/celestiaorg/celestia-app/v3/app" "github.com/celestiaorg/celestia-app/v3/app/ante" "github.com/celestiaorg/celestia-app/v3/pkg/appconsts" diff --git a/x/blob/ante/ante_test.go b/x/blob/ante/ante_test.go index 78103fbbbb..5b4b7d79fa 100644 --- a/x/blob/ante/ante_test.go +++ b/x/blob/ante/ante_test.go @@ -5,6 +5,7 @@ import ( "github.com/celestiaorg/celestia-app/v3/app" "github.com/celestiaorg/celestia-app/v3/app/encoding" + "github.com/celestiaorg/celestia-app/v3/pkg/appconsts" ante "github.com/celestiaorg/celestia-app/v3/x/blob/ante" blob "github.com/celestiaorg/celestia-app/v3/x/blob/types" "github.com/celestiaorg/go-square/v2/share" From bc08f02d133a1bebb58f5b1f87338989b60a636a Mon Sep 17 00:00:00 2001 From: Nina Barbakadze Date: Wed, 18 Sep 2024 13:46:36 +0200 Subject: [PATCH 16/20] test: expand ante test to account for different version logics --- x/blob/ante/ante_test.go | 80 +++++++++++++++++++++++++++------------- 1 file changed, 55 insertions(+), 25 deletions(-) diff --git a/x/blob/ante/ante_test.go b/x/blob/ante/ante_test.go index 5b4b7d79fa..6be76d77d9 100644 --- a/x/blob/ante/ante_test.go +++ b/x/blob/ante/ante_test.go @@ -1,11 +1,13 @@ package ante_test import ( + "fmt" "testing" "github.com/celestiaorg/celestia-app/v3/app" "github.com/celestiaorg/celestia-app/v3/app/encoding" "github.com/celestiaorg/celestia-app/v3/pkg/appconsts" + v2 "github.com/celestiaorg/celestia-app/v3/pkg/appconsts/v2" ante "github.com/celestiaorg/celestia-app/v3/x/blob/ante" blob "github.com/celestiaorg/celestia-app/v3/x/blob/types" "github.com/celestiaorg/go-square/v2/share" @@ -25,8 +27,9 @@ func TestPFBAnteHandler(t *testing.T) { testCases := []struct { name string pfb *blob.MsgPayForBlobs - txGas uint64 + txGas func(uint32) uint32 gasConsumed uint64 + versions []uint64 wantErr bool }{ { @@ -35,8 +38,11 @@ func TestPFBAnteHandler(t *testing.T) { // 1 share = 512 bytes = 5120 gas BlobSizes: []uint32{uint32(share.AvailableBytesFromSparseShares(1))}, }, - txGas: share.ShareSize * testGasPerBlobByte, + txGas: func(testGasPerBlobByte uint32) uint32 { + return share.ShareSize * testGasPerBlobByte + }, gasConsumed: 0, + versions: []uint64{v2.Version, appconsts.LatestVersion}, wantErr: false, }, { @@ -44,8 +50,11 @@ func TestPFBAnteHandler(t *testing.T) { pfb: &blob.MsgPayForBlobs{ BlobSizes: []uint32{uint32(share.AvailableBytesFromSparseShares(1)), uint32(share.AvailableBytesFromSparseShares(2))}, }, - txGas: 3 * share.ShareSize * testGasPerBlobByte, + txGas: func(testGasPerBlobByte uint32) uint32 { + return 3 * share.ShareSize * testGasPerBlobByte + }, gasConsumed: 0, + versions: []uint64{v2.Version, appconsts.LatestVersion}, wantErr: false, }, { @@ -54,8 +63,11 @@ func TestPFBAnteHandler(t *testing.T) { // 2 share = 1024 bytes = 10240 gas BlobSizes: []uint32{uint32(share.AvailableBytesFromSparseShares(1) + 1)}, }, - txGas: 2*share.ShareSize*testGasPerBlobByte - 1, + txGas: func(testGasPerBlobByte uint32) uint32 { + return 2*share.ShareSize*testGasPerBlobByte - 1 + }, gasConsumed: 0, + versions: []uint64{v2.Version, appconsts.LatestVersion}, wantErr: true, }, { @@ -63,8 +75,11 @@ func TestPFBAnteHandler(t *testing.T) { pfb: &blob.MsgPayForBlobs{ BlobSizes: []uint32{uint32(share.AvailableBytesFromSparseShares(1)), uint32(share.AvailableBytesFromSparseShares(2))}, }, - txGas: 3*share.ShareSize*testGasPerBlobByte - 1, + txGas: func(testGasPerBlobByte uint32) uint32 { + return 3*share.ShareSize*testGasPerBlobByte - 1 + }, gasConsumed: 0, + versions: []uint64{v2.Version, appconsts.LatestVersion}, wantErr: true, }, { @@ -73,8 +88,11 @@ func TestPFBAnteHandler(t *testing.T) { // 1 share = 512 bytes = 5120 gas BlobSizes: []uint32{uint32(share.AvailableBytesFromSparseShares(1))}, }, - txGas: share.ShareSize*testGasPerBlobByte + 10000 - 1, + txGas: func(testGasPerBlobByte uint32) uint32 { + return share.ShareSize*testGasPerBlobByte + 10000 - 1 + }, gasConsumed: 10000, + versions: []uint64{v2.Version, appconsts.LatestVersion}, wantErr: true, }, { @@ -83,31 +101,43 @@ func TestPFBAnteHandler(t *testing.T) { // 1 share = 512 bytes = 5120 gas BlobSizes: []uint32{uint32(share.AvailableBytesFromSparseShares(10))}, }, - txGas: 1000000, + txGas: func(_ uint32) uint32 { + return 1000000 + }, gasConsumed: 10000, + versions: []uint64{v2.Version, appconsts.LatestVersion}, wantErr: false, }, } for _, tc := range testCases { - t.Run(tc.name, func(t *testing.T) { - anteHandler := ante.NewMinGasPFBDecorator(mockBlobKeeper{}) - ctx := sdk.NewContext(nil, tmproto.Header{ - Version: version.Consensus{ - App: appconsts.LatestVersion, - }, - }, true, nil).WithGasMeter(sdk.NewGasMeter(tc.txGas)) + for _, currentVersion := range tc.versions { + t.Run(fmt.Sprintf("%s v%d", tc.name, currentVersion), func(t *testing.T) { + anteHandler := ante.NewMinGasPFBDecorator(mockBlobKeeper{}) + var gasPerBlobByte uint32 + if currentVersion == v2.Version { + gasPerBlobByte = testGasPerBlobByte + } else { + gasPerBlobByte = appconsts.GasPerBlobByte(currentVersion) + } + + ctx := sdk.NewContext(nil, tmproto.Header{ + Version: version.Consensus{ + App: currentVersion, + }, + }, true, nil).WithGasMeter(sdk.NewGasMeter(uint64(tc.txGas(gasPerBlobByte)))).WithIsCheckTx(true) - ctx.GasMeter().ConsumeGas(tc.gasConsumed, "test") - txBuilder := txConfig.NewTxBuilder() - require.NoError(t, txBuilder.SetMsgs(tc.pfb)) - tx := txBuilder.GetTx() - _, err := anteHandler.AnteHandle(ctx, tx, false, func(ctx sdk.Context, _ sdk.Tx, _ bool) (sdk.Context, error) { return ctx, nil }) - if tc.wantErr { - require.Error(t, err) - } else { - require.NoError(t, err) - } - }) + ctx.GasMeter().ConsumeGas(tc.gasConsumed, "test") + txBuilder := txConfig.NewTxBuilder() + require.NoError(t, txBuilder.SetMsgs(tc.pfb)) + tx := txBuilder.GetTx() + _, err := anteHandler.AnteHandle(ctx, tx, false, func(ctx sdk.Context, _ sdk.Tx, _ bool) (sdk.Context, error) { return ctx, nil }) + if tc.wantErr { + require.Error(t, err) + } else { + require.NoError(t, err) + } + }) + } } } From a033364623ddb65bb9e22186bd292f27d02d9fbe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?nina=20/=20=E1=83=9C=E1=83=98=E1=83=9C=E1=83=90?= Date: Wed, 18 Sep 2024 13:47:55 +0200 Subject: [PATCH 17/20] Update app/test/upgrade_test.go Co-authored-by: Rootul P --- app/test/upgrade_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/test/upgrade_test.go b/app/test/upgrade_test.go index bdb35c641e..459a33aefc 100644 --- a/app/test/upgrade_test.go +++ b/app/test/upgrade_test.go @@ -145,7 +145,7 @@ func TestAppUpgradeV3(t *testing.T) { // TestAppUpgradeV2 verifies that the all module's params are overridden during an // upgrade from v1 -> v2 and the app version changes correctly. -func TestAppUpgrades(t *testing.T) { +func TestAppUpgradeV2(t *testing.T) { NetworkMinGasPriceDec, err := sdk.NewDecFromStr(fmt.Sprintf("%f", appconsts.DefaultNetworkMinGasPrice)) require.NoError(t, err) From 9603fd2ce7053cd32192a10cee93b74642959a47 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?nina=20/=20=E1=83=9C=E1=83=98=E1=83=9C=E1=83=90?= Date: Wed, 18 Sep 2024 13:48:02 +0200 Subject: [PATCH 18/20] Update x/blob/keeper/keeper_test.go Co-authored-by: Rootul P --- x/blob/keeper/keeper_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/x/blob/keeper/keeper_test.go b/x/blob/keeper/keeper_test.go index 6aedaa3a66..8fee7f81a8 100644 --- a/x/blob/keeper/keeper_test.go +++ b/x/blob/keeper/keeper_test.go @@ -101,7 +101,6 @@ func CreateKeeper(t *testing.T, version uint64) (*keeper.Keeper, store.CommitMul cdc, paramsSubspace, ) - // change the params to a non default value k.SetParams(ctx, types.DefaultParams()) return k, stateStore, ctx From f6f08c67103a7118112d4a317f43a28f6ba5fd0f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?nina=20/=20=E1=83=9C=E1=83=98=E1=83=9C=E1=83=90?= Date: Wed, 18 Sep 2024 13:48:10 +0200 Subject: [PATCH 19/20] Update x/blob/types/payforblob.go Co-authored-by: Rootul P --- x/blob/types/payforblob.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/x/blob/types/payforblob.go b/x/blob/types/payforblob.go index 4ce9331559..88f583ba80 100644 --- a/x/blob/types/payforblob.go +++ b/x/blob/types/payforblob.go @@ -161,8 +161,7 @@ func EstimateGas(blobSizes []uint32, gasPerByte uint32, txSizeCost uint64) uint6 return GasToConsume(blobSizes, gasPerByte) + (txSizeCost * BytesPerBlobInfo * uint64(len(blobSizes))) + PFBGasFixedCost } -// DefaultEstimateGas runs EstimateGas with the system defaults. The network may change these values -// through governance, thus this function should predominantly be used in testing. +// DefaultEstimateGas runs EstimateGas with the system defaults. func DefaultEstimateGas(blobSizes []uint32) uint64 { return EstimateGas(blobSizes, appconsts.DefaultGasPerBlobByte, appconsts.DefaultTxSizeCostPerByte) } From 4832d1b0f32f025b3bcbbcbed688bf51493696db Mon Sep 17 00:00:00 2001 From: Nina Barbakadze Date: Wed, 18 Sep 2024 14:40:27 +0200 Subject: [PATCH 20/20] test: expand tx size test to test different values --- app/ante/tx_size.go | 3 ++- app/ante/tx_size_test.go | 17 +++++++++++++++-- x/blob/types/payforblob.go | 2 +- 3 files changed, 18 insertions(+), 4 deletions(-) diff --git a/app/ante/tx_size.go b/app/ante/tx_size.go index 0f6c1ba919..356f6f2646 100644 --- a/app/ante/tx_size.go +++ b/app/ante/tx_size.go @@ -145,6 +145,7 @@ func consumeGasForTxSize(ctx sdk.Context, txBytes uint64, params auth.Params) { ctx.GasMeter().ConsumeGas(params.TxSizeCostPerByte*txBytes, "txSize") } else { // From v3 onwards, we should get txSizeCostPerByte from appconsts - ctx.GasMeter().ConsumeGas(appconsts.TxSizeCostPerByte(ctx.BlockHeader().Version.App)*txBytes, "txSize") + txSizeCostPerByte := appconsts.TxSizeCostPerByte(ctx.BlockHeader().Version.App) + ctx.GasMeter().ConsumeGas(txSizeCostPerByte*txBytes, "txSize") } } diff --git a/app/ante/tx_size_test.go b/app/ante/tx_size_test.go index 0756524bd7..6b6bdbd80b 100644 --- a/app/ante/tx_size_test.go +++ b/app/ante/tx_size_test.go @@ -25,10 +25,15 @@ import ( "github.com/tendermint/tendermint/proto/tendermint/version" ) +const TxSizeCostPerByte = 8 + func setup() (*app.App, sdk.Context, client.Context, error) { app, _, _ := testutil.NewTestAppWithGenesisSet(app.DefaultConsensusParams()) ctx := app.NewContext(false, tmproto.Header{}) - app.AccountKeeper.SetParams(ctx, authtypes.DefaultParams()) + params := authtypes.DefaultParams() + // Override default with a different TxSizeCostPerByte value for testing + params.TxSizeCostPerByte = TxSizeCostPerByte + app.AccountKeeper.SetParams(ctx, params) ctx = ctx.WithBlockHeight(1) // Set up TxConfig. @@ -90,7 +95,15 @@ func TestConsumeGasForTxSize(t *testing.T) { txBytes, err := clientCtx.TxConfig.TxJSONEncoder()(tx) require.Nil(t, err, "Cannot marshal tx: %v", err) - expectedGas := sdk.Gas(len(txBytes)) * appconsts.TxSizeCostPerByte(appconsts.LatestVersion) + // expected TxSizeCostPerByte is different for each version + var txSizeCostPerByte uint64 + if tc.version == v2.Version { + txSizeCostPerByte = TxSizeCostPerByte + } else { + txSizeCostPerByte = appconsts.TxSizeCostPerByte(tc.version) + } + + expectedGas := sdk.Gas(len(txBytes)) * txSizeCostPerByte // set suite.ctx with TxBytes manually ctx = ctx.WithTxBytes(txBytes) diff --git a/x/blob/types/payforblob.go b/x/blob/types/payforblob.go index 88f583ba80..b49eb05394 100644 --- a/x/blob/types/payforblob.go +++ b/x/blob/types/payforblob.go @@ -161,7 +161,7 @@ func EstimateGas(blobSizes []uint32, gasPerByte uint32, txSizeCost uint64) uint6 return GasToConsume(blobSizes, gasPerByte) + (txSizeCost * BytesPerBlobInfo * uint64(len(blobSizes))) + PFBGasFixedCost } -// DefaultEstimateGas runs EstimateGas with the system defaults. +// DefaultEstimateGas runs EstimateGas with the system defaults. func DefaultEstimateGas(blobSizes []uint32) uint64 { return EstimateGas(blobSizes, appconsts.DefaultGasPerBlobByte, appconsts.DefaultTxSizeCostPerByte) }