From faf70cb7fb4858504b1e125f40f5614412373509 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Milo=C5=A1=20=C5=BDivkovi=C4=87?= Date: Tue, 7 Jan 2025 17:14:12 +0100 Subject: [PATCH 1/2] fix: handle p2p events properly in cluster tests (#3452) ## Description This PR fixes an issue with the p2p event stream where events can be missed due to not being actively scooped from the channel, causing tests to hang. This non-blocking nature is by design, the p2p event stream should be non-blocking for subscribers at all times when new events are added. The PR increases the buffer size for events (from 1 to a reasonable value). For larger event loads, subscribers can handle channel draining caller side without any issues --- tm2/pkg/internal/p2p/p2p.go | 9 ++++++--- tm2/pkg/p2p/events/events.go | 12 ++++++++++-- 2 files changed, 16 insertions(+), 5 deletions(-) diff --git a/tm2/pkg/internal/p2p/p2p.go b/tm2/pkg/internal/p2p/p2p.go index 2a16646a159..1e650e0cd25 100644 --- a/tm2/pkg/internal/p2p/p2p.go +++ b/tm2/pkg/internal/p2p/p2p.go @@ -131,10 +131,13 @@ func MakeConnectedPeers( multiplexSwitch.DialPeers(addrs...) // Set up an exit timer - timer := time.NewTimer(5 * time.Second) + timer := time.NewTimer(1 * time.Minute) defer timer.Stop() - connectedPeers := make(map[p2pTypes.ID]struct{}) + var ( + connectedPeers = make(map[p2pTypes.ID]struct{}) + targetPeers = cfg.Count - 1 + ) for { select { @@ -143,7 +146,7 @@ func MakeConnectedPeers( connectedPeers[ev.PeerID] = struct{}{} - if len(connectedPeers) == cfg.Count-1 { + if len(connectedPeers) == targetPeers { return nil } case <-timer.C: diff --git a/tm2/pkg/p2p/events/events.go b/tm2/pkg/p2p/events/events.go index bf76e27d91e..1eb4699fb45 100644 --- a/tm2/pkg/p2p/events/events.go +++ b/tm2/pkg/p2p/events/events.go @@ -68,7 +68,12 @@ type ( func (s *subscriptions) add(filterFn EventFilter) (string, chan Event) { var ( id = xid.New().String() - ch = make(chan Event, 1) + // Since the event stream is non-blocking, + // the event buffer should be sufficiently + // large for most use-cases. Subscribers can + // handle large event load caller-side to mitigate + // events potentially being missed + ch = make(chan Event, 100) ) (*s)[id] = subscription{ @@ -99,6 +104,9 @@ func (s *subscriptions) notify(event Event) { continue } - sub.ch <- event + select { + case sub.ch <- event: + default: // non-blocking + } } } From 27073ecb721239358c55e5aad68866472ce1355e Mon Sep 17 00:00:00 2001 From: piux2 <90544084+piux2@users.noreply.github.com> Date: Tue, 7 Jan 2025 23:23:32 -0800 Subject: [PATCH 2/2] feat: min gas prices config (#3340) The minimum gas price configuration is a mechanism to prevent spam and ensure that only transactions with sufficient fees are processed. It achieves this by setting a minimum cost per unit of gas that a transaction must meet to be included in the mempool. The following condition is checked by the validator: the gas and fees provided by the user must meet this condition before the transaction can be included in the mempool and subsequently processed by the VM. Gas Fee => Gas Wanted x Min Gas Prices A node operator can set the min gas prices as following `gnoland config set application.min_gas_prices "1000ugnot/1gas" ` Co-authored-by: Nathan Toups <612924+n2p5@users.noreply.github.com> --- gno.land/cmd/gnoland/start.go | 4 +++- gno.land/pkg/gnoland/app.go | 11 ++++++++-- gno.land/pkg/gnoland/app_test.go | 2 +- tm2/pkg/bft/config/config.go | 7 ++++++ tm2/pkg/sdk/auth/ante.go | 3 ++- tm2/pkg/sdk/config/config.go | 35 ++++++++++++++++++++++++++++++ tm2/pkg/sdk/config/config_test.go | 36 +++++++++++++++++++++++++++++++ tm2/pkg/std/gasprice.go | 6 ++++-- 8 files changed, 97 insertions(+), 7 deletions(-) create mode 100644 tm2/pkg/sdk/config/config.go create mode 100644 tm2/pkg/sdk/config/config_test.go diff --git a/gno.land/cmd/gnoland/start.go b/gno.land/cmd/gnoland/start.go index a420e652810..eaaf7293986 100644 --- a/gno.land/cmd/gnoland/start.go +++ b/gno.land/cmd/gnoland/start.go @@ -26,6 +26,7 @@ import ( "github.com/gnolang/gno/tm2/pkg/crypto" "github.com/gnolang/gno/tm2/pkg/events" osm "github.com/gnolang/gno/tm2/pkg/os" + "github.com/gnolang/gno/tm2/pkg/std" "github.com/gnolang/gno/tm2/pkg/telemetry" "go.uber.org/zap" @@ -234,9 +235,10 @@ func execStart(ctx context.Context, c *startCfg, io commands.IO) error { // Create a top-level shared event switch evsw := events.NewEventSwitch() + minGasPrices := cfg.Application.MinGasPrices // Create application and node - cfg.LocalApp, err = gnoland.NewApp(nodeDir, c.skipFailingGenesisTxs, evsw, logger) + cfg.LocalApp, err = gnoland.NewApp(nodeDir, c.skipFailingGenesisTxs, evsw, logger, minGasPrices) if err != nil { return fmt.Errorf("unable to create the Gnoland app, %w", err) } diff --git a/gno.land/pkg/gnoland/app.go b/gno.land/pkg/gnoland/app.go index 9e8f2163441..0de26defad6 100644 --- a/gno.land/pkg/gnoland/app.go +++ b/gno.land/pkg/gnoland/app.go @@ -39,6 +39,7 @@ type AppOptions struct { EventSwitch events.EventSwitch // required VMOutput io.Writer // optional InitChainerConfig // options related to InitChainer + MinGasPrices string // optional } // TestAppOptions provides a "ready" default [AppOptions] for use with @@ -79,9 +80,13 @@ func NewAppWithOptions(cfg *AppOptions) (abci.Application, error) { mainKey := store.NewStoreKey("main") baseKey := store.NewStoreKey("base") + // set sdk app options + var appOpts []func(*sdk.BaseApp) + if cfg.MinGasPrices != "" { + appOpts = append(appOpts, sdk.SetMinGasPrices(cfg.MinGasPrices)) + } // Create BaseApp. - // TODO: Add a consensus based min gas prices for the node, by default it does not check - baseApp := sdk.NewBaseApp("gnoland", cfg.Logger, cfg.DB, baseKey, mainKey) + baseApp := sdk.NewBaseApp("gnoland", cfg.Logger, cfg.DB, baseKey, mainKey, appOpts...) baseApp.SetAppVersion("dev") // Set mounts for BaseApp's MultiStore. @@ -181,6 +186,7 @@ func NewApp( skipFailingGenesisTxs bool, evsw events.EventSwitch, logger *slog.Logger, + minGasPrices string, ) (abci.Application, error) { var err error @@ -191,6 +197,7 @@ func NewApp( GenesisTxResultHandler: PanicOnFailingTxResultHandler, StdlibDir: filepath.Join(gnoenv.RootDir(), "gnovm", "stdlibs"), }, + MinGasPrices: minGasPrices, } if skipFailingGenesisTxs { cfg.GenesisTxResultHandler = NoopGenesisTxResultHandler diff --git a/gno.land/pkg/gnoland/app_test.go b/gno.land/pkg/gnoland/app_test.go index 375602cfa4a..56a15fed5a9 100644 --- a/gno.land/pkg/gnoland/app_test.go +++ b/gno.land/pkg/gnoland/app_test.go @@ -134,7 +134,7 @@ func TestNewApp(t *testing.T) { // NewApp should have good defaults and manage to run InitChain. td := t.TempDir() - app, err := NewApp(td, true, events.NewEventSwitch(), log.NewNoopLogger()) + app, err := NewApp(td, true, events.NewEventSwitch(), log.NewNoopLogger(), "") require.NoError(t, err, "NewApp should be successful") resp := app.InitChain(abci.RequestInitChain{ diff --git a/tm2/pkg/bft/config/config.go b/tm2/pkg/bft/config/config.go index b745c815886..1a01686f4bd 100644 --- a/tm2/pkg/bft/config/config.go +++ b/tm2/pkg/bft/config/config.go @@ -18,6 +18,7 @@ import ( "github.com/gnolang/gno/tm2/pkg/errors" osm "github.com/gnolang/gno/tm2/pkg/os" p2p "github.com/gnolang/gno/tm2/pkg/p2p/config" + sdk "github.com/gnolang/gno/tm2/pkg/sdk/config" telemetry "github.com/gnolang/gno/tm2/pkg/telemetry/config" ) @@ -55,6 +56,7 @@ type Config struct { Consensus *cns.ConsensusConfig `json:"consensus" toml:"consensus" comment:"##### consensus configuration options #####"` TxEventStore *eventstore.Config `json:"tx_event_store" toml:"tx_event_store" comment:"##### event store #####"` Telemetry *telemetry.Config `json:"telemetry" toml:"telemetry" comment:"##### node telemetry #####"` + Application *sdk.AppConfig `json:"application" toml:"application" comment:"##### app settings #####"` } // DefaultConfig returns a default configuration for a Tendermint node @@ -67,6 +69,7 @@ func DefaultConfig() *Config { Consensus: cns.DefaultConsensusConfig(), TxEventStore: eventstore.DefaultEventStoreConfig(), Telemetry: telemetry.DefaultTelemetryConfig(), + Application: sdk.DefaultAppConfig(), } } @@ -183,6 +186,7 @@ func TestConfig() *Config { Consensus: cns.TestConsensusConfig(), TxEventStore: eventstore.DefaultEventStoreConfig(), Telemetry: telemetry.DefaultTelemetryConfig(), + Application: sdk.DefaultAppConfig(), } } @@ -238,6 +242,9 @@ func (cfg *Config) ValidateBasic() error { if err := cfg.Consensus.ValidateBasic(); err != nil { return errors.Wrap(err, "Error in [consensus] section") } + if err := cfg.Application.ValidateBasic(); err != nil { + return errors.Wrap(err, "Error in [application] section") + } return nil } diff --git a/tm2/pkg/sdk/auth/ante.go b/tm2/pkg/sdk/auth/ante.go index 997478fe4b5..bec2c501f61 100644 --- a/tm2/pkg/sdk/auth/ante.go +++ b/tm2/pkg/sdk/auth/ante.go @@ -387,9 +387,10 @@ func EnsureSufficientMempoolFees(ctx sdk.Context, fee std.Fee) sdk.Result { if prod1.Cmp(prod2) >= 0 { return sdk.Result{} } else { + fee := new(big.Int).Quo(prod2, gpg) return abciResult(std.ErrInsufficientFee( fmt.Sprintf( - "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, + "insufficient fees; got: {Gas-Wanted: %d, Gas-Fee %s}, fee required: %d with %+v as minimum gas price set by the node", feeGasPrice.Gas, feeGasPrice.Price, fee, gp, ), )) } diff --git a/tm2/pkg/sdk/config/config.go b/tm2/pkg/sdk/config/config.go new file mode 100644 index 00000000000..6e5ededf9a4 --- /dev/null +++ b/tm2/pkg/sdk/config/config.go @@ -0,0 +1,35 @@ +package config + +import ( + "github.com/gnolang/gno/tm2/pkg/errors" + "github.com/gnolang/gno/tm2/pkg/std" +) + +// ----------------------------------------------------------------------------- +// Application Config + +// AppConfig defines the configuration options for the Application +type AppConfig struct { + // Lowest gas prices accepted by a validator in the form of "100tokenA/3gas;10tokenB/5gas" separated by semicolons + MinGasPrices string `json:"min_gas_prices" toml:"min_gas_prices" comment:"Lowest gas prices accepted by a validator"` +} + +// DefaultAppConfig returns a default configuration for the application +func DefaultAppConfig() *AppConfig { + return &AppConfig{ + MinGasPrices: "", + } +} + +// ValidateBasic performs basic validation, checking format and param bounds, etc., and +// returns an error if any check fails. +func (cfg *AppConfig) ValidateBasic() error { + if cfg.MinGasPrices == "" { + return nil + } + if _, err := std.ParseGasPrices(cfg.MinGasPrices); err != nil { + return errors.Wrap(err, "invalid min gas prices") + } + + return nil +} diff --git a/tm2/pkg/sdk/config/config_test.go b/tm2/pkg/sdk/config/config_test.go new file mode 100644 index 00000000000..dd0c391849b --- /dev/null +++ b/tm2/pkg/sdk/config/config_test.go @@ -0,0 +1,36 @@ +package config + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestValidateAppConfig(t *testing.T) { + c := DefaultAppConfig() + c.MinGasPrices = "" // empty + + testCases := []struct { + testName string + minGasPrices string + expectErr bool + }{ + {"invalid min gas prices invalid gas", "10token/1", true}, + {"invalid min gas prices invalid gas denom", "9token/0gs", true}, + {"invalid min gas prices zero gas", "10token/0gas", true}, + {"invalid min gas prices no gas", "10token/gas", true}, + {"invalid min gas prices negtive gas", "10token/-1gas", true}, + {"invalid min gas prices invalid denom", "10$token/2gas", true}, + {"invalid min gas prices invalid second denom", "10token/2gas;10/3gas", true}, + {"valid min gas prices", "10foo/3gas;5bar/3gas", false}, + } + + cfg := DefaultAppConfig() + for _, tc := range testCases { + tc := tc + t.Run(tc.testName, func(t *testing.T) { + cfg.MinGasPrices = tc.minGasPrices + assert.Equal(t, tc.expectErr, cfg.ValidateBasic() != nil) + }) + } +} diff --git a/tm2/pkg/std/gasprice.go b/tm2/pkg/std/gasprice.go index fd082a93371..82d236c1d04 100644 --- a/tm2/pkg/std/gasprice.go +++ b/tm2/pkg/std/gasprice.go @@ -29,9 +29,11 @@ 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) + + if gas.Amount <= 0 { + return GasPrice{}, errors.New("invalid gas price: %s (invalid gas amount)", gasprice) } + return GasPrice{ Gas: gas.Amount, Price: price,