From cb8cc2f23857c427d9200f10d502e469de90f3b7 Mon Sep 17 00:00:00 2001 From: piux2 <90544084+piux2@users.noreply.github.com> Date: Tue, 24 Sep 2024 02:18:40 -0700 Subject: [PATCH 01/10] feat: dynamic gas price, keeper implementation --- gno.land/cmd/gnoland/start.go | 8 +- gno.land/pkg/gnoclient/integration_test.go | 18 +- gno.land/pkg/gnoland/app.go | 34 +- gno.land/pkg/gnoland/app_test.go | 394 ++++++++++++++++-- gno.land/pkg/gnoland/genesis.go | 20 + gno.land/pkg/gnoland/types.go | 6 +- .../pkg/integration/testing_integration.go | 15 +- gno.land/pkg/integration/testing_node.go | 26 +- gno.land/pkg/sdk/vm/common_test.go | 4 +- tm2/pkg/sdk/auth/ante.go | 23 +- tm2/pkg/sdk/auth/ante_test.go | 52 +++ tm2/pkg/sdk/auth/consts.go | 3 +- tm2/pkg/sdk/auth/keeper.go | 157 ++++++- tm2/pkg/sdk/auth/keeper_test.go | 26 ++ tm2/pkg/sdk/auth/params.go | 99 ++++- tm2/pkg/sdk/auth/test_common.go | 9 +- tm2/pkg/sdk/auth/types.go | 29 ++ tm2/pkg/sdk/bank/common_test.go | 5 +- tm2/pkg/sdk/baseapp.go | 18 +- tm2/pkg/sdk/baseapp_test.go | 10 +- tm2/pkg/std/gasprice.go | 23 + tm2/pkg/std/package.go | 4 + tm2/pkg/std/package_test.go | 53 +++ tm2/pkg/telemetry/metrics/metrics.go | 11 + 24 files changed, 939 insertions(+), 108 deletions(-) diff --git a/gno.land/cmd/gnoland/start.go b/gno.land/cmd/gnoland/start.go index 21f0cb4b1a6..bef0dea6ebb 100644 --- a/gno.land/cmd/gnoland/start.go +++ b/gno.land/cmd/gnoland/start.go @@ -418,10 +418,10 @@ func generateGenesisFile(genesisFile string, pk crypto.PubKey, c *startCfg) erro genesisTxs = append(pkgsTxs, genesisTxs...) // Construct genesis AppState. - gen.AppState = gnoland.GnoGenesisState{ - Balances: balances, - Txs: genesisTxs, - } + defaultGenState := gnoland.DefaultGenState() + defaultGenState.Balances = balances + defaultGenState.Txs = genesisTxs + gen.AppState = defaultGenState // Write genesis state if err := gen.SaveAs(genesisFile); err != nil { diff --git a/gno.land/pkg/gnoclient/integration_test.go b/gno.land/pkg/gnoclient/integration_test.go index ea068e0680b..9918cd9ffe5 100644 --- a/gno.land/pkg/gnoclient/integration_test.go +++ b/gno.land/pkg/gnoclient/integration_test.go @@ -39,7 +39,7 @@ func TestCallSingle_Integration(t *testing.T) { // Make Tx config baseCfg := BaseTxCfg{ - GasFee: ugnot.ValueString(10000), + GasFee: ugnot.ValueString(800000), GasWanted: 8000000, AccountNumber: 0, SequenceNumber: 0, @@ -92,7 +92,7 @@ func TestCallMultiple_Integration(t *testing.T) { // Make Tx config baseCfg := BaseTxCfg{ - GasFee: ugnot.ValueString(10000), + GasFee: ugnot.ValueString(800000), GasWanted: 8000000, AccountNumber: 0, SequenceNumber: 0, @@ -154,7 +154,7 @@ func TestSendSingle_Integration(t *testing.T) { // Make Tx config baseCfg := BaseTxCfg{ - GasFee: ugnot.ValueString(10000), + GasFee: ugnot.ValueString(800000), GasWanted: 8000000, AccountNumber: 0, SequenceNumber: 0, @@ -218,7 +218,7 @@ func TestSendMultiple_Integration(t *testing.T) { // Make Tx config baseCfg := BaseTxCfg{ - GasFee: ugnot.ValueString(10000), + GasFee: ugnot.ValueString(800000), GasWanted: 8000000, AccountNumber: 0, SequenceNumber: 0, @@ -290,7 +290,7 @@ func TestRunSingle_Integration(t *testing.T) { // Make Tx config baseCfg := BaseTxCfg{ - GasFee: ugnot.ValueString(10000), + GasFee: ugnot.ValueString(800000), GasWanted: 8000000, AccountNumber: 0, SequenceNumber: 0, @@ -358,7 +358,7 @@ func TestRunMultiple_Integration(t *testing.T) { // Make Tx config baseCfg := BaseTxCfg{ - GasFee: ugnot.ValueString(10000), + GasFee: ugnot.ValueString(800000), GasWanted: 8000000, AccountNumber: 0, SequenceNumber: 0, @@ -451,7 +451,7 @@ func TestAddPackageSingle_Integration(t *testing.T) { // Make Tx config baseCfg := BaseTxCfg{ - GasFee: ugnot.ValueString(10000), + GasFee: ugnot.ValueString(800000), GasWanted: 8000000, AccountNumber: 0, SequenceNumber: 0, @@ -536,7 +536,7 @@ func TestAddPackageMultiple_Integration(t *testing.T) { // Make Tx config baseCfg := BaseTxCfg{ - GasFee: ugnot.ValueString(10000), + GasFee: ugnot.ValueString(800000), GasWanted: 8000000, AccountNumber: 0, SequenceNumber: 0, @@ -556,7 +556,7 @@ func Echo(str string) string { body2 := `package hello func Hello(str string) string { - return "Hello " + str + "!" + return "Hello " + str + "!" }` caller, err := client.Signer.Info() diff --git a/gno.land/pkg/gnoland/app.go b/gno.land/pkg/gnoland/app.go index 2380658c6e9..3e5e44d9c2f 100644 --- a/gno.land/pkg/gnoland/app.go +++ b/gno.land/pkg/gnoland/app.go @@ -19,6 +19,7 @@ import ( "github.com/gnolang/gno/tm2/pkg/sdk" "github.com/gnolang/gno/tm2/pkg/sdk/auth" "github.com/gnolang/gno/tm2/pkg/sdk/bank" + "github.com/gnolang/gno/tm2/pkg/sdk/params" "github.com/gnolang/gno/tm2/pkg/std" "github.com/gnolang/gno/tm2/pkg/store" "github.com/gnolang/gno/tm2/pkg/store/dbadapter" @@ -86,14 +87,16 @@ func NewAppWithOptions(cfg *AppOptions) (abci.Application, error) { baseApp.MountStoreWithDB(baseKey, dbadapter.StoreConstructor, cfg.DB) // Construct keepers. - acctKpr := auth.NewAccountKeeper(mainKey, ProtoGnoAccount) + paramsKpr := params.NewKeeper(mainKey, nil) + acctKpr := auth.NewAccountKeeper(mainKey, paramsKpr, ProtoGnoAccount) + gpKpr := auth.NewGasPriceKeeper(mainKey) bankKpr := bank.NewBankKeeper(acctKpr) vmk := vm.NewVMKeeper(baseKey, mainKey, acctKpr, bankKpr, cfg.MaxCycles) // Set InitChainer icc := cfg.InitChainerConfig icc.baseApp = baseApp - icc.acctKpr, icc.bankKpr, icc.vmKpr = acctKpr, bankKpr, vmk + icc.acctKpr, icc.bankKpr, icc.vmKpr, icc.gpKpr = acctKpr, bankKpr, vmk, gpKpr baseApp.SetInitChainer(icc.InitChainer) // Set AnteHandler @@ -107,9 +110,11 @@ func NewAppWithOptions(cfg *AppOptions) (abci.Application, error) { func(ctx sdk.Context, tx std.Tx, simulate bool) ( newCtx sdk.Context, res sdk.Result, abort bool, ) { + // Add last gas price in the context + ctx = ctx.WithValue(auth.GasPriceContextKey{}, gpKpr.LastGasPrice(ctx)) + // Override auth params. - ctx = ctx. - WithValue(auth.AuthParamsContextKey{}, auth.DefaultParams()) + ctx = ctx.WithValue(auth.AuthParamsContextKey{}, acctKpr.GetParams(ctx)) // Continue on with default auth ante handler. newCtx, res, abort = authAnteHandler(ctx, tx, simulate) return @@ -140,6 +145,8 @@ func NewAppWithOptions(cfg *AppOptions) (abci.Application, error) { baseApp.SetEndBlocker( EndBlocker( c, + acctKpr, + gpKpr, vmk, baseApp, ), @@ -229,6 +236,7 @@ type InitChainerConfig struct { vmKpr vm.VMKeeperI acctKpr auth.AccountKeeperI bankKpr bank.BankKeeperI + gpKpr auth.GasPriceKeeperI } // InitChainer is the function that can be used as a [sdk.InitChainer]. @@ -286,6 +294,10 @@ func (cfg InitChainerConfig) loadAppState(ctx sdk.Context, appState any) ([]abci if !ok { return nil, fmt.Errorf("invalid AppState of type %T", appState) } + cfg.acctKpr.InitGenesis(ctx, state.Auth) + params := cfg.acctKpr.GetParams(ctx) + ctx = ctx.WithValue(auth.AuthParamsContextKey{}, params) + auth.InitChainer(ctx, cfg.gpKpr.(auth.GasPriceKeeper), params.InitialGasPrice) // Parse and set genesis state balances for _, bal := range state.Balances { @@ -335,6 +347,8 @@ type endBlockerApp interface { // validator set changes func EndBlocker( collector *collector[validatorUpdate], + acctKpr auth.AccountKeeperI, + gpKpr auth.GasPriceKeeperI, vmk vm.VMKeeperI, app endBlockerApp, ) func( @@ -342,12 +356,24 @@ func EndBlocker( req abci.RequestEndBlock, ) abci.ResponseEndBlock { return func(ctx sdk.Context, _ abci.RequestEndBlock) abci.ResponseEndBlock { + // set the auth params value in the ctx. The EndBlocker will use InitialGasPrice in + // the params to calculate the updated gas price. + if acctKpr != nil { + ctx = ctx.WithValue(auth.AuthParamsContextKey{}, acctKpr.GetParams(ctx)) + } + if acctKpr != nil && gpKpr != nil { + auth.EndBlocker(ctx, gpKpr.(auth.GasPriceKeeper)) + } // Check if there was a valset change if len(collector.getEvents()) == 0 { // No valset updates return abci.ResponseEndBlock{} } + if vmk == nil { + return abci.ResponseEndBlock{} + } + // Run the VM to get the updates from the chain response, err := vmk.QueryEval( ctx, diff --git a/gno.land/pkg/gnoland/app_test.go b/gno.land/pkg/gnoland/app_test.go index 193ff0b0b14..f698f18c13c 100644 --- a/gno.land/pkg/gnoland/app_test.go +++ b/gno.land/pkg/gnoland/app_test.go @@ -18,6 +18,10 @@ import ( "github.com/gnolang/gno/tm2/pkg/events" "github.com/gnolang/gno/tm2/pkg/log" "github.com/gnolang/gno/tm2/pkg/sdk" + "github.com/gnolang/gno/tm2/pkg/sdk/auth" + "github.com/gnolang/gno/tm2/pkg/sdk/bank" + "github.com/gnolang/gno/tm2/pkg/sdk/params" + "github.com/gnolang/gno/tm2/pkg/sdk/testutils" "github.com/gnolang/gno/tm2/pkg/std" "github.com/gnolang/gno/tm2/pkg/store" "github.com/gnolang/gno/tm2/pkg/store/dbadapter" @@ -37,6 +41,27 @@ func TestNewAppWithOptions(t *testing.T) { assert.Equal(t, "gnoland", bapp.Name()) addr := crypto.AddressFromPreimage([]byte("test1")) + + appState := DefaultGenState() + appState.Balances = []Balance{ + { + Address: addr, + Amount: []std.Coin{{Amount: 1e15, Denom: "ugnot"}}, + }, + } + appState.Txs = []std.Tx{ + { + Msgs: []std.Msg{vm.NewMsgAddPackage(addr, "gno.land/r/demo", []*std.MemFile{ + { + Name: "demo.gno", + Body: "package demo; func Hello() string { return `hello`; }", + }, + })}, + Fee: std.Fee{GasWanted: 1e6, GasFee: std.Coin{Amount: 1e6, Denom: "ugnot"}}, + Signatures: []std.Signature{{}}, // one empty signature + }, + } + resp := bapp.InitChain(abci.RequestInitChain{ Time: time.Now(), ChainID: "dev", @@ -44,26 +69,7 @@ func TestNewAppWithOptions(t *testing.T) { Block: defaultBlockParams(), }, Validators: []abci.ValidatorUpdate{}, - AppState: GnoGenesisState{ - Balances: []Balance{ - { - Address: addr, - Amount: []std.Coin{{Amount: 1e15, Denom: "ugnot"}}, - }, - }, - Txs: []std.Tx{ - { - Msgs: []std.Msg{vm.NewMsgAddPackage(addr, "gno.land/r/demo", []*std.MemFile{ - { - Name: "demo.gno", - Body: "package demo; func Hello() string { return `hello`; }", - }, - })}, - Fee: std.Fee{GasWanted: 1e6, GasFee: std.Coin{Amount: 1e6, Denom: "ugnot"}}, - Signatures: []std.Signature{{}}, // one empty signature - }, - }, - }, + AppState: appState, }) require.True(t, resp.IsOK(), "InitChain response: %v", resp) @@ -111,7 +117,7 @@ func TestNewApp(t *testing.T) { }, }, Validators: []abci.ValidatorUpdate{}, - AppState: GnoGenesisState{}, + AppState: DefaultGenState(), }) assert.True(t, resp.IsOK(), "resp is not OK: %v", resp) } @@ -181,8 +187,12 @@ func testInitChainerLoadStdlib(t *testing.T, cached bool) { //nolint:thelper vmKpr: mock, CacheStdlibLoad: cached, } + // Construct keepers. + paramsKpr := params.NewKeeper(iavlCapKey, nil) + cfg.acctKpr = auth.NewAccountKeeper(iavlCapKey, paramsKpr, ProtoGnoAccount) + cfg.gpKpr = auth.NewGasPriceKeeper(iavlCapKey) cfg.InitChainer(testCtx, abci.RequestInitChain{ - AppState: GnoGenesisState{}, + AppState: DefaultGenState(), }) // assert number of calls @@ -271,7 +281,7 @@ func TestEndBlocker(t *testing.T) { c := newCollector[validatorUpdate](&mockEventSwitch{}, noFilter) // Create the EndBlocker - eb := EndBlocker(c, nil, &mockEndBlockerApp{}) + eb := EndBlocker(c, nil, nil, nil, &mockEndBlockerApp{}) // Run the EndBlocker res := eb(sdk.Context{}, abci.RequestEndBlock{}) @@ -311,7 +321,7 @@ func TestEndBlocker(t *testing.T) { mockEventSwitch.FireEvent(gnostd.GnoEvent{}) // Create the EndBlocker - eb := EndBlocker(c, mockVMKeeper, &mockEndBlockerApp{}) + eb := EndBlocker(c, nil, nil, mockVMKeeper, &mockEndBlockerApp{}) // Run the EndBlocker res := eb(sdk.Context{}, abci.RequestEndBlock{}) @@ -354,7 +364,7 @@ func TestEndBlocker(t *testing.T) { mockEventSwitch.FireEvent(gnostd.GnoEvent{}) // Create the EndBlocker - eb := EndBlocker(c, mockVMKeeper, &mockEndBlockerApp{}) + eb := EndBlocker(c, nil, nil, mockVMKeeper, &mockEndBlockerApp{}) // Run the EndBlocker res := eb(sdk.Context{}, abci.RequestEndBlock{}) @@ -422,7 +432,7 @@ func TestEndBlocker(t *testing.T) { mockEventSwitch.FireEvent(txEvent) // Create the EndBlocker - eb := EndBlocker(c, mockVMKeeper, &mockEndBlockerApp{}) + eb := EndBlocker(c, nil, nil, mockVMKeeper, &mockEndBlockerApp{}) // Run the EndBlocker res := eb(sdk.Context{}, abci.RequestEndBlock{}) @@ -437,3 +447,335 @@ func TestEndBlocker(t *testing.T) { } }) } + +func TestGasPriceUpdate(t *testing.T) { + app, err := newGasPriceTestApp() + require.NoError(t, err) + // with default initial gas price 0.1 ugnot per gas + gnoGen, err := gnoGenesisState() + require.NoError(t, err) + + // abci inintChain + app.InitChain(abci.RequestInitChain{ + AppState: gnoGen, + ChainID: "test-chain", + ConsensusParams: &abci.ConsensusParams{ + Block: &abci.BlockParams{ + MaxGas: 10000, + }, + }, + }) + baseApp := app.(*sdk.BaseApp) + require.Equal(t, int64(0), baseApp.LastBlockHeight()) + // Case 1 + // CheckTx failed because the GasFee is less than the initial gas price. + + tx := newCounterTx(100) + tx.Fee = std.Fee{ + GasWanted: 100, + GasFee: sdk.Coin{ + Amount: 9, + Denom: "ugnot", + }, + } + txBytes, err := amino.Marshal(tx) + require.NoError(t, err) + r := app.CheckTx(abci.RequestCheckTx{Tx: txBytes}) + assert.False(t, r.IsOK(), fmt.Sprintf("%v", r)) + + // Case 2: + // A previously successful CheckTx failed after the block gas price increased. + // Check Tx Ok + tx2 := newCounterTx(100) + tx2.Fee = std.Fee{ + GasWanted: 1000, + GasFee: sdk.Coin{ + Amount: 100, + Denom: "ugnot", + }, + } + txBytes2, err := amino.Marshal(tx2) + require.NoError(t, err) + r = app.CheckTx(abci.RequestCheckTx{Tx: txBytes2}) + assert.True(t, r.IsOK(), fmt.Sprintf("%v", r)) + + // After replaying a block, the gas price increased. + header := &bft.Header{ChainID: "test-chain", Height: 1} + app.BeginBlock(abci.RequestBeginBlock{Header: header}) + // Delvier Tx consumes more than that target block gas 6000. + + tx6001 := newCounterTx(6001) + tx6001.Fee = std.Fee{ + GasWanted: 20000, + GasFee: sdk.Coin{ + Amount: 200, + Denom: "ugnot", + }, + } + txBytes6001, err := amino.Marshal(tx6001) + require.NoError(t, err) + res := app.DeliverTx(abci.RequestDeliverTx{Tx: txBytes6001}) + require.True(t, res.IsOK(), fmt.Sprintf("%v", res)) + app.EndBlock(abci.RequestEndBlock{}) + app.Commit() + + // CheckTx failed because gas price increased + r = app.CheckTx(abci.RequestCheckTx{Tx: txBytes2}) + assert.False(t, r.IsOK(), fmt.Sprintf("%v", r)) + + // Case 3: + // A previously failed CheckTx successed after block gas price reduced. + + // CheckTx Failed + r = app.CheckTx(abci.RequestCheckTx{Tx: txBytes2}) + assert.False(t, r.IsOK(), fmt.Sprintf("%v", r)) + // Replayed a Block, the gas price decrease + header = &bft.Header{ChainID: "test-chain", Height: 2} + app.BeginBlock(abci.RequestBeginBlock{Header: header}) + // Delvier Tx consumes less than that target block gas 6000. + + tx200 := newCounterTx(200) + tx200.Fee = std.Fee{ + GasWanted: 20000, + GasFee: sdk.Coin{ + Amount: 200, + Denom: "ugnot", + }, + } + txBytes200, err := amino.Marshal(tx200) + require.NoError(t, err) + + res = app.DeliverTx(abci.RequestDeliverTx{Tx: txBytes200}) + require.True(t, res.IsOK(), fmt.Sprintf("%v", res)) + + app.EndBlock(abci.RequestEndBlock{}) + app.Commit() + + // CheckTx earlier failed tx, now is OK + r = app.CheckTx(abci.RequestCheckTx{Tx: txBytes2}) + assert.True(t, r.IsOK(), fmt.Sprintf("%v", r)) + + // Case 4 + // require matching expected GasPrice after three blocks ( increase case) + replayBlock(t, baseApp, 8000, 3) + replayBlock(t, baseApp, 8000, 4) + replayBlock(t, baseApp, 6000, 5) + + key := []byte("gasPrice") + query := abci.RequestQuery{ + Path: ".store/main/key", + Data: key, + } + qr := app.Query(query) + var gp std.GasPrice + err = amino.Unmarshal(qr.Value, &gp) + require.NoError(t, err) + require.Equal(t, "108ugnot", gp.Price.String()) + + // Case 5, + // require matching expected GasPrice after low gas blocks ( decrease below initial gas price case) + + replayBlock(t, baseApp, 5000, 6) + replayBlock(t, baseApp, 5000, 7) + replayBlock(t, baseApp, 5000, 8) + + qr = app.Query(query) + err = amino.Unmarshal(qr.Value, &gp) + require.NoError(t, err) + require.Equal(t, "102ugnot", gp.Price.String()) + + replayBlock(t, baseApp, 5000, 9) + + qr = app.Query(query) + err = amino.Unmarshal(qr.Value, &gp) + require.NoError(t, err) + require.Equal(t, "100ugnot", gp.Price.String()) +} + +func newGasPriceTestApp() (abci.Application, error) { + cfg := TestAppOptions(memdb.NewMemDB()) + cfg.EventSwitch = events.NewEventSwitch() + + // Capabilities keys. + mainKey := store.NewStoreKey("main") + baseKey := store.NewStoreKey("base") + + baseApp := sdk.NewBaseApp("gnoland", cfg.Logger, cfg.DB, baseKey, mainKey) + baseApp.SetAppVersion("test") + + // Set mounts for BaseApp's MultiStore. + baseApp.MountStoreWithDB(mainKey, iavl.StoreConstructor, cfg.DB) + baseApp.MountStoreWithDB(baseKey, dbadapter.StoreConstructor, cfg.DB) + + // Construct keepers. + paramsKpr := params.NewKeeper(mainKey, nil) + acctKpr := auth.NewAccountKeeper(mainKey, paramsKpr, ProtoGnoAccount) + gpKpr := auth.NewGasPriceKeeper(mainKey) + bankKpr := bank.NewBankKeeper(acctKpr) + vmk := vm.NewVMKeeper(baseKey, mainKey, acctKpr, bankKpr, cfg.MaxCycles) + + // Set InitChainer + icc := cfg.InitChainerConfig + icc.baseApp = baseApp + icc.acctKpr, icc.bankKpr, icc.vmKpr, icc.gpKpr = acctKpr, bankKpr, vmk, gpKpr + baseApp.SetInitChainer(icc.InitChainer) + + // Set AnteHandler + baseApp.SetAnteHandler( + // Override default AnteHandler with custom logic. + func(ctx sdk.Context, tx std.Tx, simulate bool) ( + newCtx sdk.Context, res sdk.Result, abort bool, + ) { + // Add last gas price in the context + ctx = ctx.WithValue(auth.GasPriceContextKey{}, gpKpr.LastGasPrice(ctx)) + + // Override auth params. + ctx = ctx.WithValue(auth.AuthParamsContextKey{}, acctKpr.GetParams(ctx)) + // Continue on with default auth ante handler. + if ctx.IsCheckTx() { + res := auth.EnsureSufficientMempoolFees(ctx, tx.Fee) + if !res.IsOK() { + return ctx, res, true + } + } + + newCtx = auth.SetGasMeter(false, ctx, tx.Fee.GasWanted) + + count := getTotalCount(tx) + + newCtx.GasMeter().ConsumeGas(count, "counter-ante") + res = sdk.Result{ + GasWanted: getTotalCount(tx), + } + return + }, + ) + + // Set up the event collector + c := newCollector[validatorUpdate]( + cfg.EventSwitch, // global event switch filled by the node + validatorEventFilter, // filter fn that keeps the collector valid + ) + + // Set EndBlocker + baseApp.SetEndBlocker( + EndBlocker( + c, + acctKpr, + gpKpr, + nil, + baseApp, + ), + ) + + // Set a handler Route. + baseApp.Router().AddRoute("auth", auth.NewHandler(acctKpr)) + baseApp.Router().AddRoute("bank", bank.NewHandler(bankKpr)) + baseApp.Router().AddRoute( + testutils.RouteMsgCounter, + newTestHandler( + func(ctx sdk.Context, msg sdk.Msg) sdk.Result { return sdk.Result{} }, + ), + ) + + baseApp.Router().AddRoute("vm", vm.NewHandler(vmk)) + + // Load latest version. + if err := baseApp.LoadLatestVersion(); err != nil { + return nil, err + } + + // Initialize the VMKeeper. + ms := baseApp.GetCacheMultiStore() + vmk.Initialize(cfg.Logger, ms) + ms.MultiWrite() // XXX why was't this needed? + + return baseApp, nil +} + +// newTx constructs a tx with multiple counter messages. +// we can use the counter as the gas used for the message. + +func newCounterTx(counters ...int64) sdk.Tx { + msgs := make([]sdk.Msg, len(counters)) + + for i, c := range counters { + msgs[i] = testutils.MsgCounter{Counter: c} + } + tx := sdk.Tx{Msgs: msgs} + return tx +} + +func getTotalCount(tx sdk.Tx) int64 { + var c int64 + for _, m := range tx.Msgs { + c = +m.(testutils.MsgCounter).Counter + } + return c +} + +func gnoGenesisState() (GnoGenesisState, error) { + gen := GnoGenesisState{} + genBytes := []byte(`{ + "@type": "/gno.GenesisState", + "auth": { + "params": { + "gas_price_change_compressor": "8", + "initial_gasprice": { + "gas": "1000", + "price": "100ugnot" + }, + "max_memo_bytes": "65536", + "sig_verify_cost_ed25519": "590", + "sig_verify_cost_secp256k1": "1000", + "target_gas_ratio": "60", + "tx_sig_limit": "7", + "tx_size_cost_per_byte": "10" + } + } + }`) + err := amino.UnmarshalJSON(genBytes, &gen) + + return gen, err +} + +func replayBlock(t *testing.T, app *sdk.BaseApp, gas int64, hight int64) { + t.Helper() + tx := newCounterTx(gas) + tx.Fee = std.Fee{ + GasWanted: 20000, + GasFee: sdk.Coin{ + Amount: 1000, + Denom: "ugnot", + }, + } + txBytes, err := amino.Marshal(tx) + require.NoError(t, err) + + header := &bft.Header{ChainID: "test-chain", Height: hight} + app.BeginBlock(abci.RequestBeginBlock{Header: header}) + // consume gas in the block + res := app.DeliverTx(abci.RequestDeliverTx{Tx: txBytes}) + require.True(t, res.IsOK(), fmt.Sprintf("%v", res)) + app.EndBlock(abci.RequestEndBlock{}) + app.Commit() +} + +type testHandler struct { + process func(sdk.Context, sdk.Msg) sdk.Result + query func(sdk.Context, abci.RequestQuery) abci.ResponseQuery +} + +func (th testHandler) Process(ctx sdk.Context, msg sdk.Msg) sdk.Result { + return th.process(ctx, msg) +} + +func (th testHandler) Query(ctx sdk.Context, req abci.RequestQuery) abci.ResponseQuery { + return th.query(ctx, req) +} + +func newTestHandler(proc func(sdk.Context, sdk.Msg) sdk.Result) sdk.Handler { + return testHandler{ + process: proc, + } +} diff --git a/gno.land/pkg/gnoland/genesis.go b/gno.land/pkg/gnoland/genesis.go index f5f0aa56758..cae2c7ab989 100644 --- a/gno.land/pkg/gnoland/genesis.go +++ b/gno.land/pkg/gnoland/genesis.go @@ -12,9 +12,12 @@ import ( bft "github.com/gnolang/gno/tm2/pkg/bft/types" "github.com/gnolang/gno/tm2/pkg/crypto" osm "github.com/gnolang/gno/tm2/pkg/os" + "github.com/gnolang/gno/tm2/pkg/sdk/auth" "github.com/gnolang/gno/tm2/pkg/std" ) +const InitGasPrice = "10ugnot/100gas" + // LoadGenesisBalancesFile loads genesis balances from the provided file path. func LoadGenesisBalancesFile(path string) ([]Balance, error) { // each balance is in the form: g1xxxxxxxxxxxxxxxx=100000ugnot @@ -138,3 +141,20 @@ func LoadPackage(pkg gnomod.Pkg, creator bft.Address, fee std.Fee, deposit std.C return tx, nil } + +func DefaultGenState() GnoGenesisState { + authGen := auth.DefaultGenesisState() + gp, err := std.ParseGasPrice(InitGasPrice) + if err != nil { + panic(err) + } + authGen.Params.InitialGasPrice = gp + + gs := GnoGenesisState{ + Balances: []Balance{}, + Txs: []std.Tx{}, + Auth: authGen, + } + + return gs +} diff --git a/gno.land/pkg/gnoland/types.go b/gno.land/pkg/gnoland/types.go index 016f3279dbd..bbb6ca0dceb 100644 --- a/gno.land/pkg/gnoland/types.go +++ b/gno.land/pkg/gnoland/types.go @@ -3,6 +3,7 @@ package gnoland import ( "errors" + "github.com/gnolang/gno/tm2/pkg/sdk/auth" "github.com/gnolang/gno/tm2/pkg/std" ) @@ -20,6 +21,7 @@ func ProtoGnoAccount() std.Account { } type GnoGenesisState struct { - Balances []Balance `json:"balances"` - Txs []std.Tx `json:"txs"` + Balances []Balance `json:"balances"` + Txs []std.Tx `json:"txs"` + Auth auth.GenesisState `json:"auth"` } diff --git a/gno.land/pkg/integration/testing_integration.go b/gno.land/pkg/integration/testing_integration.go index d3f55cfadf7..b936bf0470b 100644 --- a/gno.land/pkg/integration/testing_integration.go +++ b/gno.land/pkg/integration/testing_integration.go @@ -132,10 +132,9 @@ func setupGnolandTestScript(t *testing.T, txtarDir string) testscript.Params { // Track new user balances added via the `adduser` // command and packages added with the `loadpkg` command. // This genesis will be use when node is started. - genesis := &gnoland.GnoGenesisState{ - Balances: LoadDefaultGenesisBalanceFile(t, gnoRootDir), - Txs: []std.Tx{}, - } + genesis := gnoland.DefaultGenState() + genesis.Balances = LoadDefaultGenesisBalanceFile(t, gnoRootDir) + genesis.Auth.Params.InitialGasPrice = std.GasPrice{Gas: 0, Price: std.Coin{Amount: 0, Denom: "ugnot"}} // test1 must be created outside of the loop below because it is already included in genesis so // attempting to recreate results in it getting overwritten and breaking existing tests that @@ -144,7 +143,7 @@ func setupGnolandTestScript(t *testing.T, txtarDir string) testscript.Params { env.Setenv("USER_SEED_"+DefaultAccount_Name, DefaultAccount_Seed) env.Setenv("USER_ADDR_"+DefaultAccount_Name, DefaultAccount_Address) - env.Values[envKeyGenesis] = genesis + env.Values[envKeyGenesis] = &genesis env.Values[envKeyPkgsLoader] = newPkgsLoader() env.Setenv("GNOROOT", gnoRootDir) @@ -184,8 +183,10 @@ func setupGnolandTestScript(t *testing.T, txtarDir string) testscript.Params { pkgs := ts.Value(envKeyPkgsLoader).(*pkgsLoader) // grab logger creator := crypto.MustAddressFromString(DefaultAccount_Address) // test1 defaultFee := std.NewFee(50000, std.MustParseCoin(ugnot.ValueString(1000000))) - pkgsTxs, err := pkgs.LoadPackages(creator, defaultFee, nil) - if err != nil { + // we need to define a new err1 otherwise the out err would be shadowed in the case "start": + pkgsTxs, err1 := pkgs.LoadPackages(creator, defaultFee, nil) + + if err1 != nil { ts.Fatalf("unable to load packages txs: %s", err) } diff --git a/gno.land/pkg/integration/testing_node.go b/gno.land/pkg/integration/testing_node.go index 5e9e2272049..fb6524722a8 100644 --- a/gno.land/pkg/integration/testing_node.go +++ b/gno.land/pkg/integration/testing_node.go @@ -63,11 +63,10 @@ func TestingNodeConfig(t TestingTS, gnoroot string, additionalTxs ...std.Tx) (*g txs := []std.Tx{} txs = append(txs, LoadDefaultPackages(t, creator, gnoroot)...) txs = append(txs, additionalTxs...) - - cfg.Genesis.AppState = gnoland.GnoGenesisState{ - Balances: balances, - Txs: txs, - } + ggs := cfg.Genesis.AppState.(gnoland.GnoGenesisState) + ggs.Balances = balances + ggs.Txs = txs + cfg.Genesis.AppState = ggs return cfg, creator } @@ -95,6 +94,13 @@ func TestingMinimalNodeConfig(t TestingTS, gnoroot string) *gnoland.InMemoryNode } func DefaultTestingGenesisConfig(t TestingTS, gnoroot string, self crypto.PubKey, tmconfig *tmcfg.Config) *bft.GenesisDoc { + genState := gnoland.DefaultGenState() + genState.Balances = []gnoland.Balance{ + { + Address: crypto.MustAddressFromString(DefaultAccount_Address), + Amount: std.MustParseCoins(ugnot.ValueString(10000000000000)), + }, + } return &bft.GenesisDoc{ GenesisTime: time.Now(), ChainID: tmconfig.ChainID(), @@ -114,15 +120,7 @@ func DefaultTestingGenesisConfig(t TestingTS, gnoroot string, self crypto.PubKey Name: "self", }, }, - AppState: gnoland.GnoGenesisState{ - Balances: []gnoland.Balance{ - { - Address: crypto.MustAddressFromString(DefaultAccount_Address), - Amount: std.MustParseCoins(ugnot.ValueString(10000000000000)), - }, - }, - Txs: []std.Tx{}, - }, + AppState: genState, } } diff --git a/gno.land/pkg/sdk/vm/common_test.go b/gno.land/pkg/sdk/vm/common_test.go index 43a8fe1fbec..6f35b958674 100644 --- a/gno.land/pkg/sdk/vm/common_test.go +++ b/gno.land/pkg/sdk/vm/common_test.go @@ -11,6 +11,7 @@ import ( "github.com/gnolang/gno/tm2/pkg/sdk" authm "github.com/gnolang/gno/tm2/pkg/sdk/auth" bankm "github.com/gnolang/gno/tm2/pkg/sdk/bank" + "github.com/gnolang/gno/tm2/pkg/sdk/params" "github.com/gnolang/gno/tm2/pkg/std" "github.com/gnolang/gno/tm2/pkg/store" "github.com/gnolang/gno/tm2/pkg/store/dbadapter" @@ -45,7 +46,8 @@ func _setupTestEnv(cacheStdlibs bool) testEnv { ms.LoadLatestVersion() ctx := sdk.NewContext(sdk.RunTxModeDeliver, ms, &bft.Header{ChainID: "test-chain-id"}, log.NewNoopLogger()) - acck := authm.NewAccountKeeper(iavlCapKey, std.ProtoBaseAccount) + paramk := params.NewKeeper(iavlCapKey, nil) + acck := authm.NewAccountKeeper(iavlCapKey, paramk, std.ProtoBaseAccount) bank := bankm.NewBankKeeper(acck) vmk := NewVMKeeper(baseCapKey, iavlCapKey, acck, bank, 100_000_000) diff --git a/tm2/pkg/sdk/auth/ante.go b/tm2/pkg/sdk/auth/ante.go index 49662b47a55..2af9eb0545f 100644 --- a/tm2/pkg/sdk/auth/ante.go +++ b/tm2/pkg/sdk/auth/ante.go @@ -379,6 +379,25 @@ func DeductFees(bank BankKeeperI, ctx sdk.Context, acc std.Account, fees std.Coi // consensus. func EnsureSufficientMempoolFees(ctx sdk.Context, fee std.Fee) sdk.Result { minGasPrices := ctx.MinGasPrices() + blockGasPrice := ctx.Value(GasPriceContextKey{}).(std.GasPrice) + feeGasPrice := std.GasPrice{ + Gas: fee.GasWanted, + Price: std.Coin{ + Amount: fee.GasFee.Amount, + Denom: fee.GasFee.Denom, + }, + } + // check the block gas price + if blockGasPrice.Price.IsValid() && !blockGasPrice.Price.IsZero() { + if !feeGasPrice.IsGTE(blockGasPrice) { + return abciResult(std.ErrInsufficientFee( + fmt.Sprintf( + "insufficient fees; got: {Gas-Wanted: %d, Gas-Fee %s}, fee required: %+v as block gas price", feeGasPrice.Gas, feeGasPrice.Price, blockGasPrice, + ), + )) + } + } + // check min gas price set by the node. if len(minGasPrices) == 0 { // no minimum gas price (not recommended) // TODO: allow for selective filtering of 0 fee txs. @@ -405,7 +424,7 @@ func EnsureSufficientMempoolFees(ctx sdk.Context, fee std.Fee) sdk.Result { } else { return abciResult(std.ErrInsufficientFee( fmt.Sprintf( - "insufficient fees; got: %q required: %q", fee.GasFee, gp, + "insufficient fees; got: {Gas-Wanted: %d, Gas-Fee %s}, fee required: %+v as minimum gas price set by the node", feeGasPrice.Gas, feeGasPrice.Price, gp, ), )) } @@ -415,7 +434,7 @@ func EnsureSufficientMempoolFees(ctx sdk.Context, fee std.Fee) sdk.Result { return abciResult(std.ErrInsufficientFee( fmt.Sprintf( - "insufficient fees; got: %q required (one of): %q", fee.GasFee, minGasPrices, + "insufficient fees; got: {Gas-Wanted: %d, Gas-Fee %s}, required (one of): %q", feeGasPrice.Gas, feeGasPrice.Price, minGasPrices, ), )) } diff --git a/tm2/pkg/sdk/auth/ante_test.go b/tm2/pkg/sdk/auth/ante_test.go index be4167a6238..bb91f833653 100644 --- a/tm2/pkg/sdk/auth/ante_test.go +++ b/tm2/pkg/sdk/auth/ante_test.go @@ -809,6 +809,8 @@ func TestEnsureSufficientMempoolFees(t *testing.T) { {std.NewFee(200000, std.NewCoin("stake", 2)), true}, {std.NewFee(200000, std.NewCoin("atom", 5)), false}, } + // Do not set the block gas price + ctx = ctx.WithValue(GasPriceContextKey{}, std.GasPrice{}) for i, tc := range testCases { res := EnsureSufficientMempoolFees(ctx, tc.input) @@ -866,3 +868,53 @@ func TestCustomSignatureVerificationGasConsumer(t *testing.T) { tx = tu.NewTestTx(t, ctx.ChainID(), msgs, privs, accnums, seqs, fee) checkValidTx(t, anteHandler, ctx, tx, false) } + +func TestEnsureBlockGasPrice(t *testing.T) { + p1, err := std.ParseGasPrice("3ugnot/10gas") // 0.3ugnot + require.NoError(t, err) + + p2, err := std.ParseGasPrice("400ugnot/2000gas") // 0.2ugnot + require.NoError(t, err) + + userFeeCases := []struct { + minGasPrice std.GasPrice + blockGasPrice std.GasPrice + input std.Fee + expectedOK bool + }{ + // user's gas wanted and gas fee: 0.1ugnot to 0.5ugnot + // validator's minGasPrice: 0.3 ugnot + // block gas price: 0.2ugnot + + {p1, p2, std.NewFee(100, std.NewCoin("ugnot", 10)), false}, + {p1, p2, std.NewFee(100, std.NewCoin("ugnot", 20)), false}, + {p1, p2, std.NewFee(100, std.NewCoin("ugnot", 30)), true}, + {p1, p2, std.NewFee(100, std.NewCoin("ugnot", 40)), true}, + {p1, p2, std.NewFee(100, std.NewCoin("ugnot", 50)), true}, + + // validator's minGasPrice: 0.2 ugnot + // block gas price2: 0.3ugnot + {p2, p1, std.NewFee(100, std.NewCoin("ugnot", 10)), false}, + {p2, p1, std.NewFee(100, std.NewCoin("ugnot", 20)), false}, + {p2, p1, std.NewFee(100, std.NewCoin("ugnot", 30)), true}, + {p2, p1, std.NewFee(100, std.NewCoin("ugnot", 40)), true}, + {p2, p1, std.NewFee(100, std.NewCoin("ugnot", 50)), true}, + } + + // setup + env := setupTestEnv() + ctx := env.ctx + // validator min gas price // 0.3 ugnot per gas + for i, c := range userFeeCases { + ctx = ctx.WithMinGasPrices( + []std.GasPrice{c.minGasPrice}, + ) + ctx = ctx.WithValue(GasPriceContextKey{}, c.blockGasPrice) + + res := EnsureSufficientMempoolFees(ctx, c.input) + require.Equal( + t, c.expectedOK, res.IsOK(), + "unexpected result; case #%d, input: %v, log: %v", i, c.input, res.Log, + ) + } +} diff --git a/tm2/pkg/sdk/auth/consts.go b/tm2/pkg/sdk/auth/consts.go index 09bbb15cdbc..462ca0cd64d 100644 --- a/tm2/pkg/sdk/auth/consts.go +++ b/tm2/pkg/sdk/auth/consts.go @@ -19,7 +19,8 @@ const ( // AddressStoreKeyPrefix prefix for account-by-address store AddressStoreKeyPrefix = "/a/" - + // key for gas price + GasPriceKey = "gasPrice" // param key for global account number GlobalAccountNumberKey = "globalAccountNumber" ) diff --git a/tm2/pkg/sdk/auth/keeper.go b/tm2/pkg/sdk/auth/keeper.go index e43b5389844..7c93a21e935 100644 --- a/tm2/pkg/sdk/auth/keeper.go +++ b/tm2/pkg/sdk/auth/keeper.go @@ -3,10 +3,12 @@ package auth import ( "fmt" "log/slog" + "math/big" "github.com/gnolang/gno/tm2/pkg/amino" "github.com/gnolang/gno/tm2/pkg/crypto" "github.com/gnolang/gno/tm2/pkg/sdk" + "github.com/gnolang/gno/tm2/pkg/sdk/params" "github.com/gnolang/gno/tm2/pkg/std" "github.com/gnolang/gno/tm2/pkg/store" ) @@ -15,7 +17,8 @@ import ( type AccountKeeper struct { // The (unexposed) key used to access the store from the Context. key store.StoreKey - + // The keeper used to store auth parameters + paramk params.Keeper // The prototypical Account constructor. proto func() std.Account } @@ -23,11 +26,12 @@ type AccountKeeper struct { // NewAccountKeeper returns a new AccountKeeper that uses go-amino to // (binary) encode and decode concrete std.Accounts. func NewAccountKeeper( - key store.StoreKey, proto func() std.Account, + key store.StoreKey, pk params.Keeper, proto func() std.Account, ) AccountKeeper { return AccountKeeper{ - key: key, - proto: proto, + key: key, + paramk: pk, + proto: proto, } } @@ -160,3 +164,148 @@ func (ak AccountKeeper) decodeAccount(bz []byte) (acc std.Account) { } return } + +type GasPriceContextKey struct{} + +type GasPriceKeeper struct { + key store.StoreKey +} + +// GasPriceKeeper +// The GasPriceKeeper stores the history of gas prices and calculates +// new gas price with formula parameters +func NewGasPriceKeeper(key store.StoreKey) GasPriceKeeper { + return GasPriceKeeper{ + key: key, + } +} + +// setGasPrice is called in InitChainer to store initial gas price set in the genesis + +func (gk GasPriceKeeper) setGasPrice(ctx sdk.Context, gp std.GasPrice) { + if (gp == std.GasPrice{}) { + return + } + stor := ctx.Store(gk.key) + bz, err := amino.Marshal(gp) + if err != nil { + panic(err) + } + stor.Set([]byte(GasPriceKey), bz) +} + +// We store the history. If the formula changes, we can replay blocks +// and apply the formula to a specific block range. The new gas price is +// calculated in EndBlock(). +func (gk GasPriceKeeper) updateGasPrice(ctx sdk.Context) { + params := ctx.Value(AuthParamsContextKey{}).(Params) + gasUsed := ctx.BlockGasMeter().GasConsumed() + maxBlockGas := ctx.ConsensusParams().Block.MaxGas + lgp := gk.LastGasPrice(ctx) + newGasPrice := gk.calcBlockGasPrice(lgp, gasUsed, maxBlockGas, params) + gk.setGasPrice(ctx, newGasPrice) +} + +// calcBlockGasPrice calculates the minGasPrice for the txs to be included in the next block. +// newGasPrice = lastPrice + lastPrice*(gasUsed-TargetBlockGas)/TargetBlockGas/GasCompressor) +// +// The math formula is an abstraction of a simple solution for the underlying problem we're trying to solve. +// 1. What do we do if the gas used is less than the target gas in a block? +// 2. How do we bring the gas used back to the target level, if gas used is more than the target? +// We simplify the solution with a one-line formula to explain the idea. However, in reality, we need to treat +// two scenarios differently. For example, in the first case, we need to increase the gas by at least 1 unit, +// instead of round down for the integer divisions, and in the second case, we should set a floor +// as the target gas price. This is just a starting point. Down the line, the solution might not be even +// representable by one simple formula + +func (gk GasPriceKeeper) calcBlockGasPrice(lastGasPrice std.GasPrice, gasUsed int64, maxGas int64, params Params) std.GasPrice { + // If no block gas price is set, there is no need to change the last gas price. + if lastGasPrice.Price.Amount == 0 { + return lastGasPrice + } + + // This is also a configuration to indicate that there is no need to change the last gas price. + if params.TargetGasRatio == 0 { + return lastGasPrice + } + // if no gas used, no need to change the lastPrice + if gasUsed == 0 { + return lastGasPrice + } + var ( + num = new(big.Int) + denom = new(big.Int) + ) + + // targetGas = maxGax*TargetGasRatio/100 + + num.Mul(big.NewInt(maxGas), big.NewInt(params.TargetGasRatio)) + num.Div(num, big.NewInt(int64(100))) + targetGasInt := new(big.Int).Set(num) + + // if used gas is right on target, no need to change + gasUsedInt := big.NewInt(gasUsed) + if targetGasInt.Cmp(gasUsedInt) == 0 { + return lastGasPrice + } + + c := params.GasPricesChangeCompressor + lastPriceInt := big.NewInt(lastGasPrice.Price.Amount) + + if gasUsedInt.Cmp(targetGasInt) == 1 { // gas used is more than the target + // increase gas price + num = num.Sub(gasUsedInt, targetGasInt) + num.Mul(num, lastPriceInt) + num.Div(num, targetGasInt) + num.Div(num, denom.SetInt64(c)) + // increase at least 1 + diff := max(num, big.NewInt(1)) + num.Add(lastPriceInt, diff) + // XXX should we cap it with a max gas price? + } else { // gas used is less than the target + // decrease gas price down to initial gas price + initPriceInt := big.NewInt(params.InitialGasPrice.Price.Amount) + if lastPriceInt.Cmp(initPriceInt) == -1 { + return params.InitialGasPrice + } + num.Sub(targetGasInt, gasUsedInt) + num.Mul(num, lastPriceInt) + num.Div(num, targetGasInt) + num.Div(num, denom.SetInt64(c)) + + num.Sub(lastPriceInt, num) + // gas price should not be less than the initial gas price, + num = max(num, initPriceInt) + } + + if !num.IsInt64() { + panic("The min gas price is out of int64 range") + } + + lastGasPrice.Price.Amount = num.Int64() + return lastGasPrice +} + +// max returns the larger of x or y. +func max(x, y *big.Int) *big.Int { + if x.Cmp(y) < 0 { + return y + } + return x +} + +// It returns the gas price for the last block. +func (gk GasPriceKeeper) LastGasPrice(ctx sdk.Context) std.GasPrice { + stor := ctx.Store(gk.key) + bz := stor.Get([]byte(GasPriceKey)) + if bz == nil { + return std.GasPrice{} + } + + gp := std.GasPrice{} + err := amino.Unmarshal(bz, &gp) + if err != nil { + panic(err) + } + return gp +} diff --git a/tm2/pkg/sdk/auth/keeper_test.go b/tm2/pkg/sdk/auth/keeper_test.go index d40d96cdb4b..29cfcf3fdae 100644 --- a/tm2/pkg/sdk/auth/keeper_test.go +++ b/tm2/pkg/sdk/auth/keeper_test.go @@ -6,6 +6,7 @@ import ( "github.com/stretchr/testify/require" "github.com/gnolang/gno/tm2/pkg/crypto" + "github.com/gnolang/gno/tm2/pkg/std" ) func TestAccountMapperGetSet(t *testing.T) { @@ -71,3 +72,28 @@ func TestAccountMapperRemoveAccount(t *testing.T) { require.NotNil(t, acc2) require.Equal(t, accSeq2, acc2.GetSequence()) } + +func TestAccountKeeperParams(t *testing.T) { + env := setupTestEnv() + + dp := DefaultParams() + err := env.acck.SetParams(env.ctx, dp) + require.NoError(t, err) + + dp2 := env.acck.GetParams(env.ctx) + require.True(t, dp.Equals(dp2)) +} + +func TestGasPrice(t *testing.T) { + env := setupTestEnv() + gp := std.GasPrice{ + Gas: 100, + Price: std.Coin{ + Denom: "token", + Amount: 10, + }, + } + env.gk.setGasPrice(env.ctx, gp) + gp2 := env.gk.LastGasPrice(env.ctx) + require.True(t, gp == gp2) +} diff --git a/tm2/pkg/sdk/auth/params.go b/tm2/pkg/sdk/auth/params.go index dfeaa73af71..7cf5c8f69ed 100644 --- a/tm2/pkg/sdk/auth/params.go +++ b/tm2/pkg/sdk/auth/params.go @@ -5,38 +5,47 @@ import ( "strings" "github.com/gnolang/gno/tm2/pkg/amino" + "github.com/gnolang/gno/tm2/pkg/sdk" + "github.com/gnolang/gno/tm2/pkg/std" ) type AuthParamsContextKey struct{} // Default parameter values const ( - DefaultMaxMemoBytes int64 = 65536 - DefaultTxSigLimit int64 = 7 - DefaultTxSizeCostPerByte int64 = 10 - DefaultSigVerifyCostED25519 int64 = 590 - DefaultSigVerifyCostSecp256k1 int64 = 1000 + DefaultMaxMemoBytes int64 = 65536 + DefaultTxSigLimit int64 = 7 + DefaultTxSizeCostPerByte int64 = 10 + DefaultSigVerifyCostED25519 int64 = 590 + DefaultSigVerifyCostSecp256k1 int64 = 1000 + DefaultGasPricesChangeCompressor int64 = 10 + DefaultTargetGasRatio int64 = 70 // 70% of the MaxGas in a block ) // Params defines the parameters for the auth module. type Params struct { - MaxMemoBytes int64 `json:"max_memo_bytes" yaml:"max_memo_bytes"` - TxSigLimit int64 `json:"tx_sig_limit" yaml:"tx_sig_limit"` - TxSizeCostPerByte int64 `json:"tx_size_cost_per_byte" yaml:"tx_size_cost_per_byte"` - SigVerifyCostED25519 int64 `json:"sig_verify_cost_ed25519" yaml:"sig_verify_cost_ed25519"` - SigVerifyCostSecp256k1 int64 `json:"sig_verify_cost_secp256k1" yaml:"sig_verify_cost_secp256k1"` + MaxMemoBytes int64 `json:"max_memo_bytes" yaml:"max_memo_bytes"` + TxSigLimit int64 `json:"tx_sig_limit" yaml:"tx_sig_limit"` + TxSizeCostPerByte int64 `json:"tx_size_cost_per_byte" yaml:"tx_size_cost_per_byte"` + SigVerifyCostED25519 int64 `json:"sig_verify_cost_ed25519" yaml:"sig_verify_cost_ed25519"` + SigVerifyCostSecp256k1 int64 `json:"sig_verify_cost_secp256k1" yaml:"sig_verify_cost_secp256k1"` + GasPricesChangeCompressor int64 `json:"gas_price_change_compressor" yaml:"gas_price_change_compressor"` + TargetGasRatio int64 `json:"target_gas_ratio" yaml:"target_gas_ratio"` + InitialGasPrice std.GasPrice `json:"initial_gasprice"` } // NewParams creates a new Params object func NewParams(maxMemoBytes, txSigLimit, txSizeCostPerByte, - sigVerifyCostED25519, sigVerifyCostSecp256k1 int64, + sigVerifyCostED25519, sigVerifyCostSecp256k1, gasPricesChangeCompressor, targetGasRatio int64, ) Params { return Params{ - MaxMemoBytes: maxMemoBytes, - TxSigLimit: txSigLimit, - TxSizeCostPerByte: txSizeCostPerByte, - SigVerifyCostED25519: sigVerifyCostED25519, - SigVerifyCostSecp256k1: sigVerifyCostSecp256k1, + MaxMemoBytes: maxMemoBytes, + TxSigLimit: txSigLimit, + TxSizeCostPerByte: txSizeCostPerByte, + SigVerifyCostED25519: sigVerifyCostED25519, + SigVerifyCostSecp256k1: sigVerifyCostSecp256k1, + GasPricesChangeCompressor: gasPricesChangeCompressor, + TargetGasRatio: targetGasRatio, } } @@ -48,11 +57,13 @@ func (p Params) Equals(p2 Params) bool { // DefaultParams returns a default set of parameters. func DefaultParams() Params { return Params{ - MaxMemoBytes: DefaultMaxMemoBytes, - TxSigLimit: DefaultTxSigLimit, - TxSizeCostPerByte: DefaultTxSizeCostPerByte, - SigVerifyCostED25519: DefaultSigVerifyCostED25519, - SigVerifyCostSecp256k1: DefaultSigVerifyCostSecp256k1, + MaxMemoBytes: DefaultMaxMemoBytes, + TxSigLimit: DefaultTxSigLimit, + TxSizeCostPerByte: DefaultTxSizeCostPerByte, + SigVerifyCostED25519: DefaultSigVerifyCostED25519, + SigVerifyCostSecp256k1: DefaultSigVerifyCostSecp256k1, + GasPricesChangeCompressor: DefaultGasPricesChangeCompressor, + TargetGasRatio: DefaultTargetGasRatio, } } @@ -65,5 +76,51 @@ func (p Params) String() string { sb.WriteString(fmt.Sprintf("TxSizeCostPerByte: %d\n", p.TxSizeCostPerByte)) sb.WriteString(fmt.Sprintf("SigVerifyCostED25519: %d\n", p.SigVerifyCostED25519)) sb.WriteString(fmt.Sprintf("SigVerifyCostSecp256k1: %d\n", p.SigVerifyCostSecp256k1)) + sb.WriteString(fmt.Sprintf("GasPricesChangeCompressor: %d\n", p.GasPricesChangeCompressor)) + sb.WriteString(fmt.Sprintf("TargetGasRatio: %d\n", p.TargetGasRatio)) return sb.String() } + +func (p Params) Validate() error { + if p.TxSigLimit == 0 { + return fmt.Errorf("invalid tx signature limit: %d", p.TxSigLimit) + } + if p.SigVerifyCostED25519 == 0 { + return fmt.Errorf("invalid ED25519 signature verification cost: %d", p.SigVerifyCostED25519) + } + if p.SigVerifyCostSecp256k1 == 0 { + return fmt.Errorf("invalid SECK256k1 signature verification cost: %d", p.SigVerifyCostSecp256k1) + } + if p.TxSizeCostPerByte == 0 { + return fmt.Errorf("invalid tx size cost per byte: %d", p.TxSizeCostPerByte) + } + if p.GasPricesChangeCompressor <= 0 { + return fmt.Errorf("invalid gas prices change compressor: %d, it should be larger or equal to 1", p.GasPricesChangeCompressor) + } + if p.TargetGasRatio < 0 || p.TargetGasRatio > 100 { + return fmt.Errorf("invalid target block gas ratio: %d, it should be between 0 and 100, 0 is unlimited", p.TargetGasRatio) + } + return nil +} + +func (ak AccountKeeper) SetParams(ctx sdk.Context, params Params) error { + if err := params.Validate(); err != nil { + return err + } + ak.paramk.SetParam(ctx, ModuleName, params) + return nil +} + +func (ak AccountKeeper) GetParams(ctx sdk.Context) Params { + params := &Params{} + + ok, err := ak.paramk.GetParam(ctx, ModuleName, params) + + if !ok { + panic("params key " + ModuleName + " does not exist") + } + if err != nil { + panic(err.Error()) + } + return *params +} diff --git a/tm2/pkg/sdk/auth/test_common.go b/tm2/pkg/sdk/auth/test_common.go index f833a0b0564..aac65adc69d 100644 --- a/tm2/pkg/sdk/auth/test_common.go +++ b/tm2/pkg/sdk/auth/test_common.go @@ -6,8 +6,8 @@ import ( "github.com/gnolang/gno/tm2/pkg/crypto" "github.com/gnolang/gno/tm2/pkg/db/memdb" "github.com/gnolang/gno/tm2/pkg/log" - "github.com/gnolang/gno/tm2/pkg/sdk" + "github.com/gnolang/gno/tm2/pkg/sdk/params" "github.com/gnolang/gno/tm2/pkg/std" "github.com/gnolang/gno/tm2/pkg/store" "github.com/gnolang/gno/tm2/pkg/store/iavl" @@ -17,6 +17,7 @@ type testEnv struct { ctx sdk.Context acck AccountKeeper bank BankKeeperI + gk GasPriceKeeper } func setupTestEnv() testEnv { @@ -28,8 +29,10 @@ func setupTestEnv() testEnv { ms.MountStoreWithDB(authCapKey, iavl.StoreConstructor, db) ms.LoadLatestVersion() - acck := NewAccountKeeper(authCapKey, std.ProtoBaseAccount) + paramk := params.NewKeeper(authCapKey, nil) + acck := NewAccountKeeper(authCapKey, paramk, std.ProtoBaseAccount) bank := NewDummyBankKeeper(acck) + gk := NewGasPriceKeeper(authCapKey) ctx := sdk.NewContext(sdk.RunTxModeDeliver, ms, &bft.Header{Height: 1, ChainID: "test-chain-id"}, log.NewNoopLogger()) ctx = ctx.WithValue(AuthParamsContextKey{}, DefaultParams()) @@ -46,7 +49,7 @@ func setupTestEnv() testEnv { }, }) - return testEnv{ctx: ctx, acck: acck, bank: bank} + return testEnv{ctx: ctx, acck: acck, bank: bank, gk: gk} } // DummyBankKeeper defines a supply keeper used only for testing to avoid diff --git a/tm2/pkg/sdk/auth/types.go b/tm2/pkg/sdk/auth/types.go index 8bbc5e39e3b..e73581c0129 100644 --- a/tm2/pkg/sdk/auth/types.go +++ b/tm2/pkg/sdk/auth/types.go @@ -13,6 +13,8 @@ type AccountKeeperI interface { GetAllAccounts(ctx sdk.Context) []std.Account SetAccount(ctx sdk.Context, acc std.Account) IterateAccounts(ctx sdk.Context, process func(std.Account) bool) + InitGenesis(ctx sdk.Context, data GenesisState) + GetParams(ctx sdk.Context) Params } var _ AccountKeeperI = AccountKeeper{} @@ -21,3 +23,30 @@ var _ AccountKeeperI = AccountKeeper{} type BankKeeperI interface { SendCoins(ctx sdk.Context, fromAddr crypto.Address, toAddr crypto.Address, amt std.Coins) error } + +type GasPriceKeeperI interface { + LastGasPrice(ctx sdk.Context) std.GasPrice +} + +var _ GasPriceKeeperI = GasPriceKeeper{} + +// GenesisState - all auth state that must be provided at genesis +type GenesisState struct { + Params Params `json:"params"` +} + +// NewGenesisState - Create a new genesis state +func NewGenesisState(params Params) GenesisState { + return GenesisState{params} +} + +// DefaultGenesisState - Return a default genesis state +func DefaultGenesisState() GenesisState { + return NewGenesisState(DefaultParams()) +} + +// ValidateGenesis performs basic validation of auth genesis data returning an +// error for any failed validation criteria. +func ValidateGenesis(data GenesisState) error { + return data.Params.Validate() +} diff --git a/tm2/pkg/sdk/bank/common_test.go b/tm2/pkg/sdk/bank/common_test.go index 95b93157165..1c843bcce4f 100644 --- a/tm2/pkg/sdk/bank/common_test.go +++ b/tm2/pkg/sdk/bank/common_test.go @@ -9,6 +9,7 @@ import ( "github.com/gnolang/gno/tm2/pkg/sdk" "github.com/gnolang/gno/tm2/pkg/sdk/auth" + "github.com/gnolang/gno/tm2/pkg/sdk/params" "github.com/gnolang/gno/tm2/pkg/std" "github.com/gnolang/gno/tm2/pkg/store" "github.com/gnolang/gno/tm2/pkg/store/iavl" @@ -28,10 +29,10 @@ func setupTestEnv() testEnv { ms := store.NewCommitMultiStore(db) ms.MountStoreWithDB(authCapKey, iavl.StoreConstructor, db) ms.LoadLatestVersion() - + paramk := params.NewKeeper(authCapKey, nil) ctx := sdk.NewContext(sdk.RunTxModeDeliver, ms, &bft.Header{ChainID: "test-chain-id"}, log.NewNoopLogger()) acck := auth.NewAccountKeeper( - authCapKey, std.ProtoBaseAccount, + authCapKey, paramk, std.ProtoBaseAccount, ) bank := NewBankKeeper(acck) diff --git a/tm2/pkg/sdk/baseapp.go b/tm2/pkg/sdk/baseapp.go index 671f18cf058..7bbed023558 100644 --- a/tm2/pkg/sdk/baseapp.go +++ b/tm2/pkg/sdk/baseapp.go @@ -179,7 +179,13 @@ func (app *BaseApp) initFromMainStore() error { // Load the consensus params from the main store. If the consensus params are // nil, it will be saved later during InitChain. // - // TODO: assert that InitChain hasn't yet been called. + // assert that InitChain hasn't yet been called. + // the app.checkState will be set in InitChain. + // We assert that InitChain hasn't yet been called so + // we don't over write the consensus params in the app. + if app.checkState != nil { + panic("Consensus Params are already set in app, we should not overwrite it here") + } consensusParamsBz := mainStore.Get(mainConsensusParamsKey) if consensusParamsBz != nil { consensusParams := &abci.ConsensusParams{} @@ -353,7 +359,10 @@ func (app *BaseApp) InitChain(req abci.RequestInitChain) (res abci.ResponseInitC } } } - + // In initChainer(), We set the initial parameter values in the params keeper. + // These values need to be written to the multi-store so that CheckTx + // can access the information stored in the params keeper. + app.deliverState.ctx.ms.MultiWrite() // NOTE: We don't commit, but BeginBlock for block 1 starts from this // deliverState. return @@ -856,7 +865,10 @@ func (app *BaseApp) runTx(mode RunTxMode, txBytes []byte, tx Tx) (result Result) // EndBlock implements the ABCI interface. func (app *BaseApp) EndBlock(req abci.RequestEndBlock) (res abci.ResponseEndBlock) { if app.endBlocker != nil { - res = app.endBlocker(app.deliverState.ctx, req) + // we need to load consensusParams to the end blocker Context + // end blocker use consensusParams to calculat the gas price changes. + ctx := app.deliverState.ctx.WithConsensusParams(app.consensusParams) + res = app.endBlocker(ctx, req) } return diff --git a/tm2/pkg/sdk/baseapp_test.go b/tm2/pkg/sdk/baseapp_test.go index c8884533b30..54d9d401595 100644 --- a/tm2/pkg/sdk/baseapp_test.go +++ b/tm2/pkg/sdk/baseapp_test.go @@ -353,11 +353,6 @@ func TestInitChainer(t *testing.T) { Data: key, } - // initChainer is nil - nothing happens - app.InitChain(abci.RequestInitChain{ChainID: "test-chain"}) - res := app.Query(query) - require.Equal(t, 0, len(res.Value)) - // set initChainer and try again - should see the value app.SetInitChainer(initChainer) @@ -366,6 +361,11 @@ func TestInitChainer(t *testing.T) { require.Nil(t, err) require.Equal(t, int64(0), app.LastBlockHeight()) + // initChainer is nil - nothing happens + app.InitChain(abci.RequestInitChain{ChainID: "test-chain"}) + res := app.Query(query) + require.Equal(t, 0, len(res.Value)) + app.InitChain(abci.RequestInitChain{AppState: nil, ChainID: "test-chain-id"}) // must have valid JSON genesis file, even if empty // assert that chainID is set correctly in InitChain diff --git a/tm2/pkg/std/gasprice.go b/tm2/pkg/std/gasprice.go index 5acc934fb71..edfe6d50ae7 100644 --- a/tm2/pkg/std/gasprice.go +++ b/tm2/pkg/std/gasprice.go @@ -1,6 +1,7 @@ package std import ( + "math/big" "strings" "github.com/gnolang/gno/tm2/pkg/errors" @@ -48,3 +49,25 @@ func ParseGasPrices(gasprices string) (res []GasPrice, err error) { } return res, nil } + +// IsGTE compares the GasPrice with another gas price B. If coin denom matches AND fee per gas is +// greater or equal to the gas price B return true, other wise return false, +func (gp GasPrice) IsGTE(gpB GasPrice) bool { + gpg := big.NewInt(gp.Gas) + gpa := big.NewInt(gp.Price.Amount) + gpd := gp.Price.Denom + + gpBg := big.NewInt(gpB.Gas) + gpBa := big.NewInt(gpB.Price.Amount) + gpBd := gpB.Price.Denom + if gpd != gpBd { + return false + } + prod1 := big.NewInt(0).Mul(gpa, gpBg) // gp's price amount * gpB's gas + prod2 := big.NewInt(0).Mul(gpg, gpBa) // gpB's gas * pg's price amount + // This is equivalent to checking + // That the Fee / GasWanted ratio is greater than or equal to the minimum GasPrice per gas. + // This approach helps us avoid dealing with configurations where the value of + // the minimum gas price is set to 0.00001ugnot/gas. + return prod1.Cmp(prod2) >= 0 +} diff --git a/tm2/pkg/std/package.go b/tm2/pkg/std/package.go index 76e1f9fc4ad..056aba5a6bd 100644 --- a/tm2/pkg/std/package.go +++ b/tm2/pkg/std/package.go @@ -12,6 +12,10 @@ var Package = amino.RegisterPackage(amino.NewPackage( // Account &BaseAccount{}, "BaseAccount", + // Coin + &Coin{}, "Coin", + // GasPrice + &GasPrice{}, "GasPrice", // MemFile/MemPackage MemFile{}, "MemFile", diff --git a/tm2/pkg/std/package_test.go b/tm2/pkg/std/package_test.go index 0a21188737b..2659d9b6955 100644 --- a/tm2/pkg/std/package_test.go +++ b/tm2/pkg/std/package_test.go @@ -24,3 +24,56 @@ func TestAminoBaseAccount(t *testing.T) { err := amino.UnmarshalJSON(b, &acc) require.NoError(t, err) } + +func TestAminoGasPrice(t *testing.T) { + gp := std.GasPrice{ + Gas: 100, + Price: std.Coin{ + Denom: "token", + Amount: 10, + }, + } + // Binary + bz, err := amino.Marshal(gp) + require.NoError(t, err) + err = amino.Unmarshal(bz, &gp) + require.NoError(t, err) + + // JSON + bz, err = amino.MarshalJSON(gp) + require.NoError(t, err) + + err = amino.UnmarshalJSON(bz, &gp) + require.NoError(t, err) + + bz = []byte(`{ + "gas": "10", + "price": "100token" + }`) + err = amino.UnmarshalJSON(bz, &gp) + require.NoError(t, err) +} + +func TestAminoCoin(t *testing.T) { + coin := std.Coin{ + Denom: "token", + Amount: 10, + } + + // Binary + bz, err := amino.Marshal(coin) + require.NoError(t, err) + + err = amino.Unmarshal(bz, &coin) + require.NoError(t, err) + + // JSON + bz, err = amino.MarshalJSON(coin) + require.NoError(t, err) + err = amino.UnmarshalJSON(bz, &coin) + require.NoError(t, err) + + bz = []byte(`"10token"`) + err = amino.UnmarshalJSON(bz, &coin) + require.NoError(t, err) +} diff --git a/tm2/pkg/telemetry/metrics/metrics.go b/tm2/pkg/telemetry/metrics/metrics.go index 2b04769fe0c..541eee92788 100644 --- a/tm2/pkg/telemetry/metrics/metrics.go +++ b/tm2/pkg/telemetry/metrics/metrics.go @@ -37,6 +37,7 @@ const ( blockIntervalKey = "block_interval_hist" blockTxsKey = "block_txs_hist" blockSizeKey = "block_size_hist" + gasPriceKey = "block_gas_price_hist" httpRequestTimeKey = "http_request_time_hist" wsRequestTimeKey = "ws_request_time_hist" @@ -104,6 +105,9 @@ var ( // BlockSizeBytes measures the size of the latest block in bytes BlockSizeBytes metric.Int64Histogram + // BlockGasPriceAmount measures the block gas price of the last block + BlockGasPriceAmount metric.Int64Histogram + // RPC // // HTTPRequestTime measures the HTTP request response time @@ -287,6 +291,13 @@ func Init(config config.Config) error { return fmt.Errorf("unable to create histogram, %w", err) } + if BlockGasPriceAmount, err = meter.Int64Histogram( + gasPriceKey, + metric.WithDescription("block gas price"), + metric.WithUnit("token"), + ); err != nil { + return fmt.Errorf("unable to create histogram, %w", err) + } // RPC // if HTTPRequestTime, err = meter.Int64Histogram( From e7cf2961b59deb36a7dcc2e7623d8def95432670 Mon Sep 17 00:00:00 2001 From: piux2 <90544084+piux2@users.noreply.github.com> Date: Tue, 24 Sep 2024 03:03:39 -0700 Subject: [PATCH 02/10] added new files --- tm2/pkg/sdk/auth/abci.go | 19 +++++++++ tm2/pkg/sdk/auth/genesis.go | 19 +++++++++ tm2/pkg/sdk/params/keeper.go | 72 +++++++++++++++++++++++++++++++++ tm2/pkg/sdk/params/keymapper.go | 8 ++++ 4 files changed, 118 insertions(+) create mode 100644 tm2/pkg/sdk/auth/abci.go create mode 100644 tm2/pkg/sdk/auth/genesis.go create mode 100644 tm2/pkg/sdk/params/keeper.go create mode 100644 tm2/pkg/sdk/params/keymapper.go diff --git a/tm2/pkg/sdk/auth/abci.go b/tm2/pkg/sdk/auth/abci.go new file mode 100644 index 00000000000..11dee4d48e6 --- /dev/null +++ b/tm2/pkg/sdk/auth/abci.go @@ -0,0 +1,19 @@ +package auth + +import ( + "github.com/gnolang/gno/tm2/pkg/sdk" + "github.com/gnolang/gno/tm2/pkg/std" +) + +// EndBlocker is called in the EndBlock(), it calcuates the minimum gas price +// for the next gas price +func EndBlocker(ctx sdk.Context, gk GasPriceKeeper) { + gk.updateGasPrice(ctx) +} + +// InitChainer is called in the InitChain(), it set the initial gas price in the +// GasPriceKeeper store +// for the next gas price +func InitChainer(ctx sdk.Context, gk GasPriceKeeper, gp std.GasPrice) { + gk.setGasPrice(ctx, gp) +} diff --git a/tm2/pkg/sdk/auth/genesis.go b/tm2/pkg/sdk/auth/genesis.go new file mode 100644 index 00000000000..881f803bc77 --- /dev/null +++ b/tm2/pkg/sdk/auth/genesis.go @@ -0,0 +1,19 @@ +package auth + +import ( + "github.com/gnolang/gno/tm2/pkg/sdk" +) + +// InitGenesis - Init store state from genesis data +func (ak AccountKeeper) InitGenesis(ctx sdk.Context, data GenesisState) { + if err := ak.SetParams(ctx, data.Params); err != nil { + panic(err) + } +} + +// ExportGenesis returns a GenesisState for a given context and keeper +func (ak AccountKeeper) ExportGenesis(ctx sdk.Context) GenesisState { + params := ak.GetParams(ctx) + + return NewGenesisState(params) +} diff --git a/tm2/pkg/sdk/params/keeper.go b/tm2/pkg/sdk/params/keeper.go new file mode 100644 index 00000000000..2e3a4d0675b --- /dev/null +++ b/tm2/pkg/sdk/params/keeper.go @@ -0,0 +1,72 @@ +package params + +import ( + "log/slog" + + "github.com/gnolang/gno/tm2/pkg/amino" + "github.com/gnolang/gno/tm2/pkg/sdk" + "github.com/gnolang/gno/tm2/pkg/store" +) + +const ( + ModuleName = "params" + + StoreKey = ModuleName + + // ValueStorePrevfix is "/pv/" for param value. + ValueStoreKeyPrefix = "/pv/" +) + +func ValueStoreKey(key string) []byte { + return append([]byte(ValueStoreKeyPrefix), []byte(key)...) +} + +// Keeper of the global param store. +type Keeper struct { + key store.StoreKey + keyMapper KeyMapper +} + +// NewKeeper constructs a params keeper +func NewKeeper(key store.StoreKey, keyMapper KeyMapper) Keeper { + return Keeper{ + key: key, + keyMapper: keyMapper, + } +} + +// Logger returns a module-specific logger. +func (k Keeper) Logger(ctx sdk.Context) *slog.Logger { + return ctx.Logger().With("module", ModuleName) +} + +// GetParam gets a param value from the global param store. +func (k Keeper) GetParam(ctx sdk.Context, key string, target interface{}) (bool, error) { + stor := ctx.Store(k.key) + if k.keyMapper != nil { + key = k.keyMapper.Map(key) + } + + bz := stor.Get(ValueStoreKey(key)) + if bz == nil { + return false, nil + } + + return true, amino.Unmarshal(bz, target) +} + +// SetParam sets a param value to the global param store. +func (k Keeper) SetParam(ctx sdk.Context, key string, param interface{}) error { + bz, err := amino.Marshal(param) + if err != nil { + return err + } + + stor := ctx.Store(k.key) + if k.keyMapper != nil { + key = k.keyMapper.Map(key) + } + + stor.Set(ValueStoreKey(key), bz) + return nil +} diff --git a/tm2/pkg/sdk/params/keymapper.go b/tm2/pkg/sdk/params/keymapper.go new file mode 100644 index 00000000000..22bd0ddd820 --- /dev/null +++ b/tm2/pkg/sdk/params/keymapper.go @@ -0,0 +1,8 @@ +package params + +// KeyMapper is used to map one key string to another. +type KeyMapper interface { + // Map does a transformation on an input key to produce the key + // appropriate for accessing a param keeper's storage instance. + Map(key string) string +} From 7696e584a1e9feb597d854dabb2da77f538950e3 Mon Sep 17 00:00:00 2001 From: piux2 <90544084+piux2@users.noreply.github.com> Date: Sat, 28 Sep 2024 00:25:10 -0700 Subject: [PATCH 03/10] fix dev node genesis AppState --- contribs/gnodev/go.mod | 2 +- contribs/gnodev/pkg/dev/node.go | 23 +++++++++-------------- contribs/gnodev/pkg/dev/node_state.go | 16 ++++++++-------- 3 files changed, 18 insertions(+), 23 deletions(-) diff --git a/contribs/gnodev/go.mod b/contribs/gnodev/go.mod index f4859889a16..c419f968d4a 100644 --- a/contribs/gnodev/go.mod +++ b/contribs/gnodev/go.mod @@ -1,6 +1,6 @@ module github.com/gnolang/gno/contribs/gnodev -go 1.22 +go 1.22.0 replace github.com/gnolang/gno => ../.. diff --git a/contribs/gnodev/pkg/dev/node.go b/contribs/gnodev/pkg/dev/node.go index c3e70366fb2..d43fb1b5b0e 100644 --- a/contribs/gnodev/pkg/dev/node.go +++ b/contribs/gnodev/pkg/dev/node.go @@ -113,12 +113,9 @@ func NewDevNode(ctx context.Context, cfg *NodeConfig) (*Node, error) { initialState: cfg.InitialTxs, currentStateIndex: len(cfg.InitialTxs), } - - // generate genesis state - genesis := gnoland.GnoGenesisState{ - Balances: cfg.BalancesList, - Txs: append(pkgsTxs, cfg.InitialTxs...), - } + genesis := gnoland.DefaultGenState() + genesis.Balances = cfg.BalancesList + genesis.Txs = append(pkgsTxs, cfg.InitialTxs...) if err := devnode.rebuildNode(ctx, genesis); err != nil { return nil, fmt.Errorf("unable to initialize the node: %w", err) @@ -270,10 +267,9 @@ func (n *Node) Reset(ctx context.Context) error { // Append initialTxs txs := append(pkgsTxs, n.initialState...) - genesis := gnoland.GnoGenesisState{ - Balances: n.config.BalancesList, - Txs: txs, - } + genesis := gnoland.DefaultGenState() + genesis.Balances = n.config.BalancesList + genesis.Txs = txs // Reset the node with the new genesis state. err = n.rebuildNode(ctx, genesis) @@ -410,10 +406,9 @@ func (n *Node) rebuildNodeFromState(ctx context.Context) error { } // Create genesis with loaded pkgs + previous state - genesis := gnoland.GnoGenesisState{ - Balances: n.config.BalancesList, - Txs: append(pkgsTxs, state...), - } + genesis := gnoland.DefaultGenState() + genesis.Balances = n.config.BalancesList + genesis.Txs = append(pkgsTxs, state...) // Reset the node with the new genesis state. err = n.rebuildNode(ctx, genesis) diff --git a/contribs/gnodev/pkg/dev/node_state.go b/contribs/gnodev/pkg/dev/node_state.go index 846c4857784..af4301f8ad1 100644 --- a/contribs/gnodev/pkg/dev/node_state.go +++ b/contribs/gnodev/pkg/dev/node_state.go @@ -93,10 +93,9 @@ func (n *Node) MoveBy(ctx context.Context, x int) error { newState := n.state[:newIndex] // Create genesis with loaded pkgs + previous state - genesis := gnoland.GnoGenesisState{ - Balances: n.config.BalancesList, - Txs: append(pkgsTxs, newState...), - } + genesis := gnoland.DefaultGenState() + genesis.Balances = n.config.BalancesList + genesis.Txs = append(pkgsTxs, newState...) // Reset the node with the new genesis state. if err = n.rebuildNode(ctx, genesis); err != nil { @@ -133,10 +132,11 @@ func (n *Node) ExportStateAsGenesis(ctx context.Context) (*bft.GenesisDoc, error // Get current blockstore state doc := *n.Node.GenesisDoc() // copy doc - doc.AppState = gnoland.GnoGenesisState{ - Balances: n.config.BalancesList, - Txs: state, - } + + genState := doc.AppState.(gnoland.GnoGenesisState) + genState.Balances = n.config.BalancesList + genState.Txs = state + doc.AppState = genState return &doc, nil } From ae25c58c49c90e417333a00e6011b04dbca56fe9 Mon Sep 17 00:00:00 2001 From: piux2 <90544084+piux2@users.noreply.github.com> Date: Mon, 7 Oct 2024 14:44:37 -0700 Subject: [PATCH 04/10] bug fix: textar non-validator restart failure --- tm2/pkg/sdk/baseapp.go | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/tm2/pkg/sdk/baseapp.go b/tm2/pkg/sdk/baseapp.go index 7bbed023558..6f537ffc335 100644 --- a/tm2/pkg/sdk/baseapp.go +++ b/tm2/pkg/sdk/baseapp.go @@ -359,10 +359,13 @@ func (app *BaseApp) InitChain(req abci.RequestInitChain) (res abci.ResponseInitC } } } - // In initChainer(), We set the initial parameter values in the params keeper. - // These values need to be written to the multi-store so that CheckTx - // can access the information stored in the params keeper. - app.deliverState.ctx.ms.MultiWrite() + // In app.initChainer(), we set the initial parameter values in the params keeper. + // The params keeper store needs to be accessible in the CheckTx state so that + // the first CheckTx can verify the gas price set right after the chain is initialized + // with the genesis state. + app.checkState.ctx.ms = app.deliverState.ctx.ms + app.checkState.ms = app.deliverState.ms + // NOTE: We don't commit, but BeginBlock for block 1 starts from this // deliverState. return From 6da36bba8b3e5853409a134be246b736559f4bce Mon Sep 17 00:00:00 2001 From: piux2 <90544084+piux2@users.noreply.github.com> Date: Thu, 10 Oct 2024 17:48:54 -0700 Subject: [PATCH 05/10] more testing coverage --- tm2/pkg/sdk/auth/keeper_test.go | 79 +++++++++++++++++++++++++++++++++ 1 file changed, 79 insertions(+) diff --git a/tm2/pkg/sdk/auth/keeper_test.go b/tm2/pkg/sdk/auth/keeper_test.go index 29cfcf3fdae..1655c3afd63 100644 --- a/tm2/pkg/sdk/auth/keeper_test.go +++ b/tm2/pkg/sdk/auth/keeper_test.go @@ -1,6 +1,7 @@ package auth import ( + "math/big" "testing" "github.com/stretchr/testify/require" @@ -97,3 +98,81 @@ func TestGasPrice(t *testing.T) { gp2 := env.gk.LastGasPrice(env.ctx) require.True(t, gp == gp2) } + +func TestMax(t *testing.T) { + tests := []struct { + name string + x, y *big.Int + expected *big.Int + }{ + { + name: "X is less than Y", + x: big.NewInt(5), + y: big.NewInt(10), + expected: big.NewInt(10), + }, + { + name: "X is greater than Y", + x: big.NewInt(15), + y: big.NewInt(10), + expected: big.NewInt(15), + }, + { + name: "X is equal to Y", + x: big.NewInt(10), + y: big.NewInt(10), + expected: big.NewInt(10), + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + result := max(tc.x, tc.y) + require.Equal(t, tc.expected, result) + }) + } +} + +func TestCalcBlockGasPrice(t *testing.T) { + gk := GasPriceKeeper{} + + lastGasPrice := std.GasPrice{ + Price: std.Coin{ + Amount: 100, + Denom: "atom", + }, + } + gasUsed := int64(5000) + maxGas := int64(10000) + params := Params{ + TargetGasRatio: 50, + GasPricesChangeCompressor: 2, + } + + // Test with normal parameters + newGasPrice := gk.calcBlockGasPrice(lastGasPrice, gasUsed, maxGas, params) + expectedAmount := big.NewInt(100) + num := big.NewInt(gasUsed - maxGas*params.TargetGasRatio/100) + num.Mul(num, expectedAmount) + num.Div(num, big.NewInt(maxGas*params.TargetGasRatio/100)) + num.Div(num, big.NewInt(params.GasPricesChangeCompressor)) + expectedAmount.Add(expectedAmount, num) + require.Equal(t, expectedAmount.Int64(), newGasPrice.Price.Amount) + + // Test with lastGasPrice amount as 0 + lastGasPrice.Price.Amount = 0 + newGasPrice = gk.calcBlockGasPrice(lastGasPrice, gasUsed, maxGas, params) + require.Equal(t, int64(0), newGasPrice.Price.Amount) + + // Test with TargetGasRatio as 0 (should not change the last price) + params.TargetGasRatio = 0 + newGasPrice = gk.calcBlockGasPrice(lastGasPrice, gasUsed, maxGas, params) + require.Equal(t, int64(0), newGasPrice.Price.Amount) + + // Test with gasUsed as 0 (should not change the last price) + params.TargetGasRatio = 50 + lastGasPrice.Price.Amount = 100 + gasUsed = 0 + newGasPrice = gk.calcBlockGasPrice(lastGasPrice, gasUsed, maxGas, params) + require.Equal(t, int64(100), newGasPrice.Price.Amount) +} From 6f89b6bd3a126bf1487d0a59f6a9890b1ca08944 Mon Sep 17 00:00:00 2001 From: piux2 <90544084+piux2@users.noreply.github.com> Date: Thu, 10 Oct 2024 17:57:43 -0700 Subject: [PATCH 06/10] more test --- tm2/pkg/sdk/auth/params_test.go | 68 +++++++++++++++++++++++++++++++++ tm2/pkg/std/gasprice.go | 10 ++--- 2 files changed, 73 insertions(+), 5 deletions(-) create mode 100644 tm2/pkg/sdk/auth/params_test.go diff --git a/tm2/pkg/sdk/auth/params_test.go b/tm2/pkg/sdk/auth/params_test.go new file mode 100644 index 00000000000..ab21a5c9ccc --- /dev/null +++ b/tm2/pkg/sdk/auth/params_test.go @@ -0,0 +1,68 @@ +package auth + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +func TestValidate(t *testing.T) { + tests := []struct { + name string + params Params + expectsError bool + }{ + { + name: "Valid Params", + params: Params{ + MaxMemoBytes: 256, + TxSigLimit: 10, + TxSizeCostPerByte: 1, + SigVerifyCostED25519: 100, + SigVerifyCostSecp256k1: 200, + GasPricesChangeCompressor: 1, + TargetGasRatio: 50, + }, + expectsError: false, + }, + { + name: "Invalid TxSigLimit", + params: Params{ + TxSigLimit: 0, + }, + expectsError: true, + }, + { + name: "Invalid SigVerifyCostED25519", + params: Params{ + SigVerifyCostED25519: 0, + }, + expectsError: true, + }, + { + name: "Invalid GasPricesChangeCompressor", + params: Params{ + GasPricesChangeCompressor: 0, + }, + expectsError: true, + }, + { + name: "Invalid TargetGasRatio", + params: Params{ + TargetGasRatio: 150, + }, + expectsError: true, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + err := tc.params.Validate() + if tc.expectsError { + require.Error(t, err) + } else { + require.NoError(t, err) + } + }) + } +} diff --git a/tm2/pkg/std/gasprice.go b/tm2/pkg/std/gasprice.go index edfe6d50ae7..8293af56916 100644 --- a/tm2/pkg/std/gasprice.go +++ b/tm2/pkg/std/gasprice.go @@ -53,16 +53,16 @@ func ParseGasPrices(gasprices string) (res []GasPrice, err error) { // IsGTE compares the GasPrice with another gas price B. If coin denom matches AND fee per gas is // greater or equal to the gas price B return true, other wise return false, func (gp GasPrice) IsGTE(gpB GasPrice) bool { + if gp.Price.Denom != gpB.Price.Denom { + return false + } + gpg := big.NewInt(gp.Gas) gpa := big.NewInt(gp.Price.Amount) - gpd := gp.Price.Denom gpBg := big.NewInt(gpB.Gas) gpBa := big.NewInt(gpB.Price.Amount) - gpBd := gpB.Price.Denom - if gpd != gpBd { - return false - } + prod1 := big.NewInt(0).Mul(gpa, gpBg) // gp's price amount * gpB's gas prod2 := big.NewInt(0).Mul(gpg, gpBa) // gpB's gas * pg's price amount // This is equivalent to checking From 8b71aa3638c7e0dcee37b4afc8e4731c2dbaedf7 Mon Sep 17 00:00:00 2001 From: piux2 <90544084+piux2@users.noreply.github.com> Date: Thu, 10 Oct 2024 22:07:08 -0700 Subject: [PATCH 07/10] export methods --- gno.land/pkg/gnoland/app.go | 2 +- tm2/pkg/sdk/auth/abci.go | 6 +++--- tm2/pkg/sdk/auth/keeper.go | 8 ++++---- tm2/pkg/sdk/auth/keeper_test.go | 2 +- tm2/pkg/sdk/auth/types.go | 2 ++ 5 files changed, 11 insertions(+), 9 deletions(-) diff --git a/gno.land/pkg/gnoland/app.go b/gno.land/pkg/gnoland/app.go index 3e5e44d9c2f..6d8bd6105d7 100644 --- a/gno.land/pkg/gnoland/app.go +++ b/gno.land/pkg/gnoland/app.go @@ -362,7 +362,7 @@ func EndBlocker( ctx = ctx.WithValue(auth.AuthParamsContextKey{}, acctKpr.GetParams(ctx)) } if acctKpr != nil && gpKpr != nil { - auth.EndBlocker(ctx, gpKpr.(auth.GasPriceKeeper)) + auth.EndBlocker(ctx, gpKpr) } // Check if there was a valset change if len(collector.getEvents()) == 0 { diff --git a/tm2/pkg/sdk/auth/abci.go b/tm2/pkg/sdk/auth/abci.go index 11dee4d48e6..86cbf962fad 100644 --- a/tm2/pkg/sdk/auth/abci.go +++ b/tm2/pkg/sdk/auth/abci.go @@ -7,13 +7,13 @@ import ( // EndBlocker is called in the EndBlock(), it calcuates the minimum gas price // for the next gas price -func EndBlocker(ctx sdk.Context, gk GasPriceKeeper) { - gk.updateGasPrice(ctx) +func EndBlocker(ctx sdk.Context, gk GasPriceKeeperI) { + gk.UpdateGasPrice(ctx) } // InitChainer is called in the InitChain(), it set the initial gas price in the // GasPriceKeeper store // for the next gas price func InitChainer(ctx sdk.Context, gk GasPriceKeeper, gp std.GasPrice) { - gk.setGasPrice(ctx, gp) + gk.SetGasPrice(ctx, gp) } diff --git a/tm2/pkg/sdk/auth/keeper.go b/tm2/pkg/sdk/auth/keeper.go index 7c93a21e935..0b959bb88e5 100644 --- a/tm2/pkg/sdk/auth/keeper.go +++ b/tm2/pkg/sdk/auth/keeper.go @@ -180,9 +180,9 @@ func NewGasPriceKeeper(key store.StoreKey) GasPriceKeeper { } } -// setGasPrice is called in InitChainer to store initial gas price set in the genesis +// SetGasPrice is called in InitChainer to store initial gas price set in the genesis -func (gk GasPriceKeeper) setGasPrice(ctx sdk.Context, gp std.GasPrice) { +func (gk GasPriceKeeper) SetGasPrice(ctx sdk.Context, gp std.GasPrice) { if (gp == std.GasPrice{}) { return } @@ -197,13 +197,13 @@ func (gk GasPriceKeeper) setGasPrice(ctx sdk.Context, gp std.GasPrice) { // We store the history. If the formula changes, we can replay blocks // and apply the formula to a specific block range. The new gas price is // calculated in EndBlock(). -func (gk GasPriceKeeper) updateGasPrice(ctx sdk.Context) { +func (gk GasPriceKeeper) UpdateGasPrice(ctx sdk.Context) { params := ctx.Value(AuthParamsContextKey{}).(Params) gasUsed := ctx.BlockGasMeter().GasConsumed() maxBlockGas := ctx.ConsensusParams().Block.MaxGas lgp := gk.LastGasPrice(ctx) newGasPrice := gk.calcBlockGasPrice(lgp, gasUsed, maxBlockGas, params) - gk.setGasPrice(ctx, newGasPrice) + gk.SetGasPrice(ctx, newGasPrice) } // calcBlockGasPrice calculates the minGasPrice for the txs to be included in the next block. diff --git a/tm2/pkg/sdk/auth/keeper_test.go b/tm2/pkg/sdk/auth/keeper_test.go index 1655c3afd63..8124ee9c46b 100644 --- a/tm2/pkg/sdk/auth/keeper_test.go +++ b/tm2/pkg/sdk/auth/keeper_test.go @@ -94,7 +94,7 @@ func TestGasPrice(t *testing.T) { Amount: 10, }, } - env.gk.setGasPrice(env.ctx, gp) + env.gk.SetGasPrice(env.ctx, gp) gp2 := env.gk.LastGasPrice(env.ctx) require.True(t, gp == gp2) } diff --git a/tm2/pkg/sdk/auth/types.go b/tm2/pkg/sdk/auth/types.go index e73581c0129..3fb2d10fbb5 100644 --- a/tm2/pkg/sdk/auth/types.go +++ b/tm2/pkg/sdk/auth/types.go @@ -26,6 +26,8 @@ type BankKeeperI interface { type GasPriceKeeperI interface { LastGasPrice(ctx sdk.Context) std.GasPrice + SetGasPrice(ctx sdk.Context, gp std.GasPrice) + UpdateGasPrice(ctx sdk.Context) } var _ GasPriceKeeperI = GasPriceKeeper{} From 5d6be56b7aaac3c9d92e659955c3156e290ddff0 Mon Sep 17 00:00:00 2001 From: piux2 <90544084+piux2@users.noreply.github.com> Date: Thu, 10 Oct 2024 23:45:43 -0700 Subject: [PATCH 08/10] more tests --- tm2/pkg/std/gasprice.go | 9 +- tm2/pkg/std/gasprice_test.go | 165 +++++++++++++++++++++++++++++++++++ 2 files changed, 173 insertions(+), 1 deletion(-) create mode 100644 tm2/pkg/std/gasprice_test.go diff --git a/tm2/pkg/std/gasprice.go b/tm2/pkg/std/gasprice.go index 8293af56916..1bd09a6a199 100644 --- a/tm2/pkg/std/gasprice.go +++ b/tm2/pkg/std/gasprice.go @@ -1,6 +1,7 @@ package std import ( + "fmt" "math/big" "strings" @@ -29,6 +30,9 @@ func ParseGasPrice(gasprice string) (GasPrice, error) { if gas.Denom != "gas" { return GasPrice{}, errors.New("invalid gas price: %s (invalid gas denom)", gasprice) } + if gas.Amount == 0 { + return GasPrice{}, errors.New("invalid gas price: %s (gas can not be zero)", gasprice) + } return GasPrice{ Gas: gas.Amount, Price: price, @@ -54,7 +58,10 @@ func ParseGasPrices(gasprices string) (res []GasPrice, err error) { // greater or equal to the gas price B return true, other wise return false, func (gp GasPrice) IsGTE(gpB GasPrice) bool { if gp.Price.Denom != gpB.Price.Denom { - return false + panic(fmt.Sprintf("gas price denominations should be equal; %s, %s", gp.Price.Denom, gpB.Price.Denom)) + } + if gp.Gas == 0 || gpB.Gas == 0 { + panic(fmt.Sprintf("GasPrice.Gas cannot be zero; %+v, %+v", gp, gpB)) } gpg := big.NewInt(gp.Gas) diff --git a/tm2/pkg/std/gasprice_test.go b/tm2/pkg/std/gasprice_test.go new file mode 100644 index 00000000000..587be4b8e61 --- /dev/null +++ b/tm2/pkg/std/gasprice_test.go @@ -0,0 +1,165 @@ +package std + +import ( + "strings" + "testing" +) + +func TestGasPriceGTE(t *testing.T) { + tests := []struct { + name string + gp GasPrice + gpB GasPrice + expectPanic bool + panicMsg string + expected bool // for non-panic cases: whether gp.IsGTE(gpB) should return true or false + }{ + // Panic cases: Different denominations + { + name: "Different Denominations Panic", + gp: GasPrice{ + Gas: 100, + Price: Coin{ + Denom: "atom", + Amount: 500, + }, + }, + gpB: GasPrice{ + Gas: 100, + Price: Coin{ + Denom: "btc", // Different denomination + Amount: 500, + }, + }, + expectPanic: true, + panicMsg: "gas price denominations should be equal", + }, + // Panic cases: Zero Gas values + { + name: "Zero Gas in gp Panic", + gp: GasPrice{ + Gas: 0, // Zero Gas in gp + Price: Coin{ + Denom: "atom", + Amount: 500, + }, + }, + gpB: GasPrice{ + Gas: 100, + Price: Coin{ + Denom: "atom", + Amount: 500, + }, + }, + expectPanic: true, + panicMsg: "GasPrice.Gas cannot be zero", + }, + { + name: "Zero Gas in gpB Panic", + gp: GasPrice{ + Gas: 100, + Price: Coin{ + Denom: "atom", + Amount: 500, + }, + }, + gpB: GasPrice{ + Gas: 0, // Zero Gas in gpB + Price: Coin{ + Denom: "atom", + Amount: 500, + }, + }, + expectPanic: true, + panicMsg: "GasPrice.Gas cannot be zero", + }, + // Valid cases: No panic, just compare gas prices + { + name: "Greater Gas Price", + gp: GasPrice{ + Gas: 100, + Price: Coin{ + Denom: "atom", + Amount: 600, // Greater price + }, + }, + gpB: GasPrice{ + Gas: 100, + Price: Coin{ + Denom: "atom", + Amount: 500, + }, + }, + expectPanic: false, + expected: true, + }, + { + name: "Equal Gas Price", + gp: GasPrice{ + Gas: 100, + Price: Coin{ + Denom: "atom", + Amount: 500, + }, + }, + gpB: GasPrice{ + Gas: 100, + Price: Coin{ + Denom: "atom", + Amount: 500, + }, + }, + expectPanic: false, + expected: true, + }, + { + name: "Lesser Gas Price", + gp: GasPrice{ + Gas: 100, + Price: Coin{ + Denom: "atom", + Amount: 400, // Lesser price + }, + }, + gpB: GasPrice{ + Gas: 100, + Price: Coin{ + Denom: "atom", + Amount: 500, + }, + }, + expectPanic: false, + expected: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + defer func() { + if r := recover(); r != nil { + if !tt.expectPanic { + t.Errorf("Test %s failed: expected no panic, but got panic: %v", tt.name, r) + } else if tt.expectPanic && r != nil { + // Check if the panic message contains the expected substring + panicMsg := r.(string) + if tt.expectPanic && !strings.Contains(panicMsg, tt.panicMsg) { + t.Errorf("Test %s failed: expected panic message containing %q, but got %q", tt.name, tt.panicMsg, panicMsg) + } + } + } else if tt.expectPanic { + t.Errorf("Test %s failed: expected panic, but no panic occurred", tt.name) + } + }() + + if !tt.expectPanic { + got := tt.gp.IsGTE(tt.gpB) + if got != tt.expected { + t.Errorf("Test %s failed: expected result %v, got %v", tt.name, tt.expected, got) + } + } else { + // This will panic, but we handle it in the defer/recover above. + _ = tt.gp.IsGTE(tt.gpB) + } + }) + } +} From 1504d8f0cd0a9390ec60ee67e2350a48fba91898 Mon Sep 17 00:00:00 2001 From: piux2 <90544084+piux2@users.noreply.github.com> Date: Tue, 5 Nov 2024 21:37:18 -0800 Subject: [PATCH 09/10] more param test --- tm2/pkg/sdk/auth/params_test.go | 39 +++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/tm2/pkg/sdk/auth/params_test.go b/tm2/pkg/sdk/auth/params_test.go index ab21a5c9ccc..4b5a6b15789 100644 --- a/tm2/pkg/sdk/auth/params_test.go +++ b/tm2/pkg/sdk/auth/params_test.go @@ -1,6 +1,7 @@ package auth import ( + "reflect" "testing" "github.com/stretchr/testify/require" @@ -66,3 +67,41 @@ func TestValidate(t *testing.T) { }) } } + +func TestNewParams(t *testing.T) { + // Define expected values for each parameter + maxMemoBytes := int64(256) + txSigLimit := int64(10) + txSizeCostPerByte := int64(5) + sigVerifyCostED25519 := int64(100) + sigVerifyCostSecp256k1 := int64(200) + gasPricesChangeCompressor := int64(50) + targetGasRatio := int64(75) + + // Call NewParams with the values + params := NewParams( + maxMemoBytes, + txSigLimit, + txSizeCostPerByte, + sigVerifyCostED25519, + sigVerifyCostSecp256k1, + gasPricesChangeCompressor, + targetGasRatio, + ) + + // Create an expected Params struct with the same values + expectedParams := Params{ + MaxMemoBytes: maxMemoBytes, + TxSigLimit: txSigLimit, + TxSizeCostPerByte: txSizeCostPerByte, + SigVerifyCostED25519: sigVerifyCostED25519, + SigVerifyCostSecp256k1: sigVerifyCostSecp256k1, + GasPricesChangeCompressor: gasPricesChangeCompressor, + TargetGasRatio: targetGasRatio, + } + + // Check if the returned params struct matches the expected struct + if !reflect.DeepEqual(params, expectedParams) { + t.Errorf("NewParams() = %+v, want %+v", params, expectedParams) + } +} From 53dfbd3b8189f729fe0294e354fa3e86c4ce75b5 Mon Sep 17 00:00:00 2001 From: piux2 <90544084+piux2@users.noreply.github.com> Date: Wed, 27 Nov 2024 20:53:44 -0800 Subject: [PATCH 10/10] Refactor gasprice.IsGTE to return an error instead of causing a panic. --- contribs/gnodev/go.mod | 4 +- contribs/gnodev/pkg/dev/node.go | 8 ++-- tm2/pkg/sdk/auth/ante.go | 8 +++- tm2/pkg/sdk/auth/ante_test.go | 28 ++++++++++++++ tm2/pkg/std/gasprice.go | 13 +++---- tm2/pkg/std/gasprice_test.go | 67 ++++++++++++++------------------- 6 files changed, 76 insertions(+), 52 deletions(-) diff --git a/contribs/gnodev/go.mod b/contribs/gnodev/go.mod index c419f968d4a..bde7884cf81 100644 --- a/contribs/gnodev/go.mod +++ b/contribs/gnodev/go.mod @@ -1,6 +1,8 @@ module github.com/gnolang/gno/contribs/gnodev -go 1.22.0 +go 1.22 + +toolchain go1.22.4 replace github.com/gnolang/gno => ../.. diff --git a/contribs/gnodev/pkg/dev/node.go b/contribs/gnodev/pkg/dev/node.go index d43fb1b5b0e..4b255f19198 100644 --- a/contribs/gnodev/pkg/dev/node.go +++ b/contribs/gnodev/pkg/dev/node.go @@ -388,10 +388,10 @@ func (n *Node) rebuildNodeFromState(ctx context.Context) error { if err != nil { return fmt.Errorf("unable to load pkgs: %w", err) } - - return n.rebuildNode(ctx, gnoland.GnoGenesisState{ - Balances: n.config.BalancesList, Txs: txs, - }) + genesis := gnoland.DefaultGenState() + genesis.Balances = n.config.BalancesList + genesis.Txs = txs + return n.rebuildNode(ctx, genesis) } state, err := n.getBlockStoreState(ctx) diff --git a/tm2/pkg/sdk/auth/ante.go b/tm2/pkg/sdk/auth/ante.go index 2af9eb0545f..6f2807b3d34 100644 --- a/tm2/pkg/sdk/auth/ante.go +++ b/tm2/pkg/sdk/auth/ante.go @@ -389,7 +389,13 @@ func EnsureSufficientMempoolFees(ctx sdk.Context, fee std.Fee) sdk.Result { } // check the block gas price if blockGasPrice.Price.IsValid() && !blockGasPrice.Price.IsZero() { - if !feeGasPrice.IsGTE(blockGasPrice) { + ok, err := feeGasPrice.IsGTE(blockGasPrice) + if err != nil { + return abciResult(std.ErrInsufficientFee( + err.Error(), + )) + } + if !ok { return abciResult(std.ErrInsufficientFee( fmt.Sprintf( "insufficient fees; got: {Gas-Wanted: %d, Gas-Fee %s}, fee required: %+v as block gas price", feeGasPrice.Gas, feeGasPrice.Price, blockGasPrice, diff --git a/tm2/pkg/sdk/auth/ante_test.go b/tm2/pkg/sdk/auth/ante_test.go index bb91f833653..d959adbc9e3 100644 --- a/tm2/pkg/sdk/auth/ante_test.go +++ b/tm2/pkg/sdk/auth/ante_test.go @@ -7,6 +7,7 @@ import ( "strings" "testing" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/gnolang/gno/tm2/pkg/amino" @@ -918,3 +919,30 @@ func TestEnsureBlockGasPrice(t *testing.T) { ) } } + +func TestInvalidUserFee(t *testing.T) { + minGasPrice, err := std.ParseGasPrice("3ugnot/10gas") // 0.3ugnot + require.NoError(t, err) + + blockGasPrice, err := std.ParseGasPrice("400ugnot/2000gas") // 0.2ugnot + require.NoError(t, err) + + userFee1 := std.NewFee(0, std.NewCoin("ugnot", 50)) + userFee2 := std.NewFee(100, std.NewCoin("uatom", 50)) + + // setup + env := setupTestEnv() + ctx := env.ctx + + ctx = ctx.WithMinGasPrices( + []std.GasPrice{minGasPrice}, + ) + ctx = ctx.WithValue(GasPriceContextKey{}, blockGasPrice) + res1 := EnsureSufficientMempoolFees(ctx, userFee1) + require.False(t, res1.IsOK()) + assert.Contains(t, res1.Log, "GasPrice.Gas cannot be zero;") + + res2 := EnsureSufficientMempoolFees(ctx, userFee2) + require.False(t, res2.IsOK()) + assert.Contains(t, res2.Log, "Gas price denominations should be equal;") +} diff --git a/tm2/pkg/std/gasprice.go b/tm2/pkg/std/gasprice.go index 1bd09a6a199..5a62eb2654a 100644 --- a/tm2/pkg/std/gasprice.go +++ b/tm2/pkg/std/gasprice.go @@ -1,7 +1,6 @@ package std import ( - "fmt" "math/big" "strings" @@ -54,14 +53,14 @@ func ParseGasPrices(gasprices string) (res []GasPrice, err error) { return res, nil } -// IsGTE compares the GasPrice with another gas price B. If coin denom matches AND fee per gas is -// greater or equal to the gas price B return true, other wise return false, -func (gp GasPrice) IsGTE(gpB GasPrice) bool { +// IsGTE compares the GasPrice with another gas price B. If the coin denom matches AND the fee per gas +// is greater than or equal to gas price B, return true; otherwise, return false. +func (gp GasPrice) IsGTE(gpB GasPrice) (bool, error) { if gp.Price.Denom != gpB.Price.Denom { - panic(fmt.Sprintf("gas price denominations should be equal; %s, %s", gp.Price.Denom, gpB.Price.Denom)) + return false, errors.New("Gas price denominations should be equal; %s, %s", gp.Price.Denom, gpB.Price.Denom) } if gp.Gas == 0 || gpB.Gas == 0 { - panic(fmt.Sprintf("GasPrice.Gas cannot be zero; %+v, %+v", gp, gpB)) + return false, errors.New("GasPrice.Gas cannot be zero; %+v, %+v", gp, gpB) } gpg := big.NewInt(gp.Gas) @@ -76,5 +75,5 @@ func (gp GasPrice) IsGTE(gpB GasPrice) bool { // That the Fee / GasWanted ratio is greater than or equal to the minimum GasPrice per gas. // This approach helps us avoid dealing with configurations where the value of // the minimum gas price is set to 0.00001ugnot/gas. - return prod1.Cmp(prod2) >= 0 + return prod1.Cmp(prod2) >= 0, nil } diff --git a/tm2/pkg/std/gasprice_test.go b/tm2/pkg/std/gasprice_test.go index 587be4b8e61..4151ee32f80 100644 --- a/tm2/pkg/std/gasprice_test.go +++ b/tm2/pkg/std/gasprice_test.go @@ -1,8 +1,10 @@ package std import ( - "strings" "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestGasPriceGTE(t *testing.T) { @@ -10,13 +12,13 @@ func TestGasPriceGTE(t *testing.T) { name string gp GasPrice gpB GasPrice - expectPanic bool - panicMsg string - expected bool // for non-panic cases: whether gp.IsGTE(gpB) should return true or false + expectError bool + errorMsg string + expected bool // for non-error cases: whether gp.IsGTE(gpB) should return true or false }{ - // Panic cases: Different denominations + // Error cases: Different denominations { - name: "Different Denominations Panic", + name: "Different denominations error", gp: GasPrice{ Gas: 100, Price: Coin{ @@ -31,12 +33,12 @@ func TestGasPriceGTE(t *testing.T) { Amount: 500, }, }, - expectPanic: true, - panicMsg: "gas price denominations should be equal", + expectError: true, + errorMsg: "Gas price denominations should be equal;", }, - // Panic cases: Zero Gas values + // Error cases: Zero Gas values { - name: "Zero Gas in gp Panic", + name: "Zero Gas in gp error", gp: GasPrice{ Gas: 0, // Zero Gas in gp Price: Coin{ @@ -51,11 +53,11 @@ func TestGasPriceGTE(t *testing.T) { Amount: 500, }, }, - expectPanic: true, - panicMsg: "GasPrice.Gas cannot be zero", + expectError: true, + errorMsg: "GasPrice.Gas cannot be zero;", }, { - name: "Zero Gas in gpB Panic", + name: "Zero Gas in gpB error", gp: GasPrice{ Gas: 100, Price: Coin{ @@ -70,10 +72,10 @@ func TestGasPriceGTE(t *testing.T) { Amount: 500, }, }, - expectPanic: true, - panicMsg: "GasPrice.Gas cannot be zero", + expectError: true, + errorMsg: "GasPrice.Gas cannot be zero;", }, - // Valid cases: No panic, just compare gas prices + // Valid cases: No errors, just compare gas prices { name: "Greater Gas Price", gp: GasPrice{ @@ -90,7 +92,7 @@ func TestGasPriceGTE(t *testing.T) { Amount: 500, }, }, - expectPanic: false, + expectError: false, expected: true, }, { @@ -109,7 +111,7 @@ func TestGasPriceGTE(t *testing.T) { Amount: 500, }, }, - expectPanic: false, + expectError: false, expected: true, }, { @@ -128,37 +130,24 @@ func TestGasPriceGTE(t *testing.T) { Amount: 500, }, }, - expectPanic: false, + expectError: false, expected: false, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - defer func() { - if r := recover(); r != nil { - if !tt.expectPanic { - t.Errorf("Test %s failed: expected no panic, but got panic: %v", tt.name, r) - } else if tt.expectPanic && r != nil { - // Check if the panic message contains the expected substring - panicMsg := r.(string) - if tt.expectPanic && !strings.Contains(panicMsg, tt.panicMsg) { - t.Errorf("Test %s failed: expected panic message containing %q, but got %q", tt.name, tt.panicMsg, panicMsg) - } - } - } else if tt.expectPanic { - t.Errorf("Test %s failed: expected panic, but no panic occurred", tt.name) - } - }() - - if !tt.expectPanic { - got := tt.gp.IsGTE(tt.gpB) + got, err := tt.gp.IsGTE(tt.gpB) + if !tt.expectError { + require.NoError(t, err) + assert.Equal(t, tt.expected, got, "Expect that %v is less than %v", tt.gp, tt.gpB) if got != tt.expected { t.Errorf("Test %s failed: expected result %v, got %v", tt.name, tt.expected, got) } } else { - // This will panic, but we handle it in the defer/recover above. - _ = tt.gp.IsGTE(tt.gpB) + require.Error(t, err) + errorMsg := err.Error() + assert.Contains(t, errorMsg, tt.errorMsg, "expected error message containing %q, but got %q", tt.errorMsg, errorMsg) } }) }