From 214aff9bbb2b225b7a870c26c6f7cd8b559f9c8b Mon Sep 17 00:00:00 2001 From: Bartek Tofel Date: Tue, 29 Oct 2024 11:48:26 +0100 Subject: [PATCH] add extra validations to client creation in read-only mode (#1280) --- seth/.changeset/v1.50.6.md | 1 + seth/README.md | 12 ++++- seth/client.go | 29 +++++++++++- seth/client_builder.go | 1 + seth/config.go | 1 + seth/config_test.go | 95 ++++++++++++++++++++++++++++++++++++++ seth/seth.toml | 5 ++ 7 files changed, 140 insertions(+), 4 deletions(-) create mode 100644 seth/.changeset/v1.50.6.md diff --git a/seth/.changeset/v1.50.6.md b/seth/.changeset/v1.50.6.md new file mode 100644 index 000000000..f123bc629 --- /dev/null +++ b/seth/.changeset/v1.50.6.md @@ -0,0 +1 @@ +- Adds `read-only` mode that can be useful if we are interested only in tracing and want to make sure that no write operations can be executed diff --git a/seth/README.md b/seth/README.md index c09724313..86fb8d484 100644 --- a/seth/README.md +++ b/seth/README.md @@ -813,11 +813,12 @@ With `SETH_LOG_LEVEL=trace` we will also log to console all traffic between Seth ### Read-only mode It's possible to use Seth in read-only mode only for transaction confirmation and tracing. Following operations will fail: -* contract deployment -* gas estimations (we need the pk/address to check nonce) +* contract deployment (we need a pk to sign the transaction) +* new transaction options (we need the pk/address to check nonce) * RPC health check (we need a pk to send a transaction to ourselves) * pending nonce protection (we need an address to check pending transactions) * ephemeral keys (we need a pk to fund them) +* gas bumping (we need a pk to sign the transaction) The easiest way to enable read-only mode is to client via `ClientBuilder`: ```go @@ -831,3 +832,10 @@ The easiest way to enable read-only mode is to client via `ClientBuilder`: ``` when builder is called with `WithReadOnlyMode()` it will disable all the operations mentioned above and all the configuration settings related to them. + +Additionally, when the client is build anc `cfg.ReadOnly = true` is set, we will validate that: +* no addresses and private keys are passed +* no ephemeral addresses are to be created +* RPC health check is disabled +* pending nonce protection is disabled +* gas bumping is disabled diff --git a/seth/client.go b/seth/client.go index 89d302aa3..27fa78040 100644 --- a/seth/client.go +++ b/seth/client.go @@ -31,9 +31,15 @@ const ( ErrCreateNonceManager = "failed to create nonce manager" ErrCreateTracer = "failed to create tracer" ErrReadContractMap = "failed to read deployed contract map" - ErrNoKeyLoaded = "failed to load private key" ErrRpcHealthCheckFailed = "RPC health check failed ¯\\_(ツ)_/¯" ErrContractDeploymentFailed = "contract deployment failed" + ErrNoPksEphemeralMode = "no private keys loaded, cannot fund ephemeral addresses" + + ErrReadOnlyWithPrivateKeys = "read-only mode is enabled, but you tried to load private keys" + ErrReadOnlyEphemeralKeys = "ephemeral mode is not supported in read-only mode" + ErrReadOnlyGasBumping = "gas bumping is not supported in read-only mode" + ErrReadOnlyRpcHealth = "RPC health check is not supported in read-only mode" + ErrReadOnlyPendingNonce = "pending nonce protection is not supported in read-only mode" ContractMapFilePattern = "deployed_contracts_%s_%s.toml" RevertedTransactionsFilePattern = "reverted_transactions_%s_%s.json" @@ -231,6 +237,11 @@ func NewClientRaw( if len(cfg.Network.URLs) > 1 { L.Warn().Msg("Multiple RPC URLs provided, only the first one will be used") } + + if cfg.ReadOnly && (len(addrs) > 0 || len(pkeys) > 0) { + return nil, errors.New(ErrReadOnlyWithPrivateKeys) + } + ctx, cancel := context.WithTimeout(context.Background(), cfg.Network.DialTimeout.Duration()) defer cancel() rpcClient, err := rpc.DialOptions(ctx, @@ -303,6 +314,9 @@ func NewClientRaw( } if cfg.CheckRpcHealthOnStart { + if cfg.ReadOnly { + return nil, errors.New(ErrReadOnlyRpcHealth) + } if c.NonceManager == nil { L.Debug().Msg("Nonce manager is not set, RPC health check will be skipped. Client will most probably fail on first transaction") } else { @@ -312,6 +326,10 @@ func NewClientRaw( } } + if cfg.PendingNonceProtectionEnabled && cfg.ReadOnly { + return nil, errors.New(ErrReadOnlyPendingNonce) + } + cfg.setEphemeralAddrs() L.Info(). @@ -324,7 +342,10 @@ func NewClientRaw( if cfg.ephemeral { if len(c.Addresses) == 0 { - return nil, errors.New("no private keys loaded, cannot fund ephemeral addresses") + return nil, errors.New(ErrNoPksEphemeralMode) + } + if cfg.ReadOnly { + return nil, errors.New(ErrReadOnlyEphemeralKeys) } gasPrice, err := c.GetSuggestedLegacyFees(context.Background(), Priority_Standard) if err != nil { @@ -392,6 +413,10 @@ func NewClientRaw( } } + if c.Cfg.GasBump != nil && c.Cfg.GasBump.Retries != 0 && c.Cfg.ReadOnly { + return nil, errors.New(ErrReadOnlyGasBumping) + } + // if gas bumping is enabled, but no strategy is set, we set the default one; otherwise we set the no-op strategy (defensive programming to avoid NPE) if c.Cfg.GasBump != nil && c.Cfg.GasBump.StrategyFn == nil { if c.Cfg.GasBumpRetries() != 0 { diff --git a/seth/client_builder.go b/seth/client_builder.go index 75dff2a7e..52b8b79c5 100644 --- a/seth/client_builder.go +++ b/seth/client_builder.go @@ -393,6 +393,7 @@ func (c *ClientBuilder) handleReadOnlyMode() { c.config.PendingNonceProtectionEnabled = false c.config.CheckRpcHealthOnStart = false c.config.EphemeralAddrs = nil + c.readonly = true if c.config.Network != nil { c.config.Network.GasPriceEstimationEnabled = false c.config.Network.PrivateKeys = []string{} diff --git a/seth/config.go b/seth/config.go index 6f1d254f2..d97b4cfc7 100644 --- a/seth/config.go +++ b/seth/config.go @@ -67,6 +67,7 @@ type Config struct { CheckRpcHealthOnStart bool `toml:"check_rpc_health_on_start"` BlockStatsConfig *BlockStatsConfig `toml:"block_stats"` GasBump *GasBumpConfig `toml:"gas_bump"` + ReadOnly bool `toml:"read_only"` } type GasBumpConfig struct { diff --git a/seth/config_test.go b/seth/config_test.go index 3f300bacf..b329745be 100644 --- a/seth/config_test.go +++ b/seth/config_test.go @@ -465,3 +465,98 @@ func TestConfigAppendPkToInactiveNetwork(t *testing.T) { require.Equal(t, 0, len(cfg.Networks[0].PrivateKeys), "network should have 0 pks") require.Equal(t, []string{"pk"}, cfg.Networks[1].PrivateKeys, "network should have 1 pk") } + +func TestConfig_ReadOnly_WithPk(t *testing.T) { + cfg := seth.Config{ + ReadOnly: true, + Network: &seth.Network{ + Name: "some_other", + URLs: []string{"ws://localhost:8546"}, + }, + } + + addrs := []common.Address{common.HexToAddress("0xb794f5ea0ba39494ce839613fffba74279579268")} + + _, err := seth.NewClientRaw(&cfg, addrs, nil) + require.Error(t, err, "succeeded in creating client") + require.Equal(t, seth.ErrReadOnlyWithPrivateKeys, err.Error(), "expected different error message") + + privateKey, err := crypto.HexToECDSA("ac0974bec39a17e36ba4a6b4d238ff944bacb478cbed5efcae784d7bf4f2ff80") + require.NoError(t, err, "failed to parse private key") + + pks := []*ecdsa.PrivateKey{privateKey} + _, err = seth.NewClientRaw(&cfg, nil, pks) + require.Error(t, err, "succeeded in creating client") + require.Equal(t, seth.ErrReadOnlyWithPrivateKeys, err.Error(), "expected different error message") + + _, err = seth.NewClientRaw(&cfg, addrs, pks) + require.Error(t, err, "succeeded in creating client") + require.Equal(t, seth.ErrReadOnlyWithPrivateKeys, err.Error(), "expected different error message") +} + +func TestConfig_ReadOnly_GasBumping(t *testing.T) { + cfg := seth.Config{ + ReadOnly: true, + Network: &seth.Network{ + Name: "some_other", + URLs: []string{"ws://localhost:8546"}, + DialTimeout: &seth.Duration{D: 10 * time.Second}, + }, + GasBump: &seth.GasBumpConfig{ + Retries: uint(1), + }, + } + + _, err := seth.NewClientRaw(&cfg, nil, nil) + require.Error(t, err, "succeeded in creating client") + require.Equal(t, seth.ErrReadOnlyGasBumping, err.Error(), "expected different error message") +} + +func TestConfig_ReadOnly_RpcHealth(t *testing.T) { + cfg := seth.Config{ + ReadOnly: true, + CheckRpcHealthOnStart: true, + Network: &seth.Network{ + Name: "some_other", + URLs: []string{"ws://localhost:8546"}, + DialTimeout: &seth.Duration{D: 10 * time.Second}, + }, + } + + _, err := seth.NewClientRaw(&cfg, nil, nil) + require.Error(t, err, "succeeded in creating client") + require.Equal(t, seth.ErrReadOnlyRpcHealth, err.Error(), "expected different error message") +} + +func TestConfig_ReadOnly_PendingNonce(t *testing.T) { + cfg := seth.Config{ + ReadOnly: true, + PendingNonceProtectionEnabled: true, + Network: &seth.Network{ + Name: "some_other", + URLs: []string{"ws://localhost:8546"}, + DialTimeout: &seth.Duration{D: 10 * time.Second}, + }, + } + + _, err := seth.NewClientRaw(&cfg, nil, nil) + require.Error(t, err, "succeeded in creating client") + require.Equal(t, seth.ErrReadOnlyPendingNonce, err.Error(), "expected different error message") +} + +func TestConfig_ReadOnly_EphemeralKeys(t *testing.T) { + ten := int64(10) + cfg := seth.Config{ + ReadOnly: true, + EphemeralAddrs: &ten, + Network: &seth.Network{ + Name: "some_other", + URLs: []string{"ws://localhost:8546"}, + DialTimeout: &seth.Duration{D: 10 * time.Second}, + }, + } + + _, err := seth.NewClientRaw(&cfg, nil, nil) + require.Error(t, err, "succeeded in creating client") + require.Equal(t, seth.ErrNoPksEphemeralMode, err.Error(), "expected different error message") +} diff --git a/seth/seth.toml b/seth/seth.toml index 9124f3fe7..f4ac7ab4f 100644 --- a/seth/seth.toml +++ b/seth/seth.toml @@ -55,6 +55,11 @@ experiments_enabled = ["slow_funds_return", "eip_1559_fee_equalizer"] # to make sure transaction can be submitted and mined check_rpc_health_on_start = false +# when enabled, upon creation Seth will validate that there are no private keys set, that node RPC health check is disabled +# and that gas bumping is disabled, since all of these operations are "write" operations. This is useful for running Seth +# only for tracing, when you want to make sure that no transactions are sent to the network. +read_only = false + [gas_bumps] # when > 0 then we will bump gas price for transactions that are stuck in the mempool # by default the bump step is controlled by gas_price_estimation_tx_priority (check readme.md for more details)