From 149a032ea0b6f413ab34a7a8b436f1385c8b4977 Mon Sep 17 00:00:00 2001 From: Paolo Grisoli Date: Wed, 2 Nov 2022 10:26:59 +0100 Subject: [PATCH] Etherman optionally readonly. (#1321) Closes: https://github.com/0xPolygonHermez/zkevm-node/issues/1227 This PR implements the option to set Etherman to be read only (a.k.a. without an account). This is useful for nodes that don't send transactions. The behavior is controlled by the new ReadOnly field in the Etherman config (false by default). Methods accessing the account are now safeguarded by a check on the readOnly field in the Etherman struct. Bonus: added the Etherman test cases in config/config_test.go Includes: * No need to supply RPC with account (#1337) Co-authored-by: Konrad Iturbe --- Makefile | 4 ++- aggregator/aggregator.go | 8 ++++-- aggregator/aggregator_internal_test.go | 4 +-- aggregator/interfaces.go | 2 +- aggregator/mocks/mock_etherman.go | 11 +++++++-- cmd/run.go | 7 ++++-- config/config_test.go | 33 +++++++++++++++++++++++++ config/default.go | 2 -- docker-compose.yml | 2 -- etherman/etherman.go | 34 +++++++++++++++++++++++--- etherman/etherman_test.go | 2 +- etherman/simulated.go | 4 +++ ethtxmanager/ethtxmanager_test.go | 27 +++++++++++++++++--- test/config/test.node.config.toml | 3 +++ 14 files changed, 121 insertions(+), 22 deletions(-) diff --git a/Makefile b/Makefile index 108e4acf6d..2234137770 100644 --- a/Makefile +++ b/Makefile @@ -28,7 +28,9 @@ run-rpc: ## Runs all the services need to run a local zkEMV RPC node sleep 2 docker-compose up -d zkevm-prover sleep 5 - docker-compose up -d zkevm-sync zkevm-rpc + docker-compose up -d zkevm-sync + sleep 2 + docker-compose up -d zkevm-rpc .PHONY: stop stop: ## Stops all services diff --git a/aggregator/aggregator.go b/aggregator/aggregator.go index 2b2a7c9596..855ac93a81 100644 --- a/aggregator/aggregator.go +++ b/aggregator/aggregator.go @@ -353,7 +353,7 @@ func (a *Aggregator) getBatchToVerify(ctx context.Context) (*state.Batch, error) func (a *Aggregator) buildInputProver(ctx context.Context, batchToVerify *state.Batch) (*pb.InputProver, error) { previousBatch, err := a.State.GetBatchByNumber(ctx, batchToVerify.BatchNumber-1, nil) if err != nil && err != state.ErrStateNotSynchronized { - return nil, fmt.Errorf("failed to get previous batch, err: %v", err) + return nil, fmt.Errorf("failed to get previous batch, err: %w", err) } blockTimestampByte := make([]byte, 8) //nolint:gomnd @@ -364,6 +364,10 @@ func (a *Aggregator) buildInputProver(ctx context.Context, batchToVerify *state. blockTimestampByte, batchToVerify.Coinbase[:], )) + pubAddr, err := a.Ethman.GetPublicAddress() + if err != nil { + return nil, fmt.Errorf("failed to get public address, err: %w", err) + } inputProver := &pb.InputProver{ PublicInputs: &pb.PublicInputs{ OldStateRoot: previousBatch.StateRoot.String(), @@ -374,7 +378,7 @@ func (a *Aggregator) buildInputProver(ctx context.Context, batchToVerify *state. BatchHashData: batchHashData.String(), BatchNum: uint32(batchToVerify.BatchNumber), EthTimestamp: uint64(batchToVerify.Timestamp.Unix()), - AggregatorAddr: a.Ethman.GetPublicAddress().String(), + AggregatorAddr: pubAddr.String(), ChainId: a.cfg.ChainID, }, GlobalExitRoot: batchToVerify.GlobalExitRoot.String(), diff --git a/aggregator/aggregator_internal_test.go b/aggregator/aggregator_internal_test.go index 610a9e91d2..67e850e1c0 100644 --- a/aggregator/aggregator_internal_test.go +++ b/aggregator/aggregator_internal_test.go @@ -125,7 +125,7 @@ func TestBuildInputProver(t *testing.T) { BatchL2Data: hex.EncodeToString(batchL2Data), } - etherman.On("GetPublicAddress").Return(common.HexToAddress("0x123")) + etherman.On("GetPublicAddress").Return(common.HexToAddress("0x123"), nil) ip, err := a.buildInputProver(ctx, batchToVerify) require.NoError(t, err) require.NotNil(t, ip) @@ -271,7 +271,7 @@ func TestAggregatorFlow(t *testing.T) { st.On("AddGeneratedProof", mock.Anything, mock.Anything, nil).Return(nil) st.On("UpdateGeneratedProof", mock.Anything, mock.Anything, nil).Return(nil) etherman.On("GetLatestVerifiedBatchNum").Return(uint64(1), nil) - etherman.On("GetPublicAddress").Return(aggrAddress) + etherman.On("GetPublicAddress").Return(aggrAddress, nil) lastVerifiedBatch, err := a.State.GetLastVerifiedBatch(context.Background(), nil) require.NoError(t, err) diff --git a/aggregator/interfaces.go b/aggregator/interfaces.go index e77e4e49d4..b21de1016f 100644 --- a/aggregator/interfaces.go +++ b/aggregator/interfaces.go @@ -21,7 +21,7 @@ type ethTxManager interface { // etherman contains the methods required to interact with ethereum type etherman interface { GetLatestVerifiedBatchNum() (uint64, error) - GetPublicAddress() common.Address + GetPublicAddress() (common.Address, error) } // aggregatorTxProfitabilityChecker interface for different profitability diff --git a/aggregator/mocks/mock_etherman.go b/aggregator/mocks/mock_etherman.go index 1daff5f0d9..551028faab 100644 --- a/aggregator/mocks/mock_etherman.go +++ b/aggregator/mocks/mock_etherman.go @@ -34,7 +34,7 @@ func (_m *Etherman) GetLatestVerifiedBatchNum() (uint64, error) { } // GetPublicAddress provides a mock function with given fields: -func (_m *Etherman) GetPublicAddress() common.Address { +func (_m *Etherman) GetPublicAddress() (common.Address, error) { ret := _m.Called() var r0 common.Address @@ -46,7 +46,14 @@ func (_m *Etherman) GetPublicAddress() common.Address { } } - return r0 + var r1 error + if rf, ok := ret.Get(1).(func() error); ok { + r1 = rf() + } else { + r1 = ret.Error(1) + } + + return r0, r1 } type mockConstructorTestingTNewEtherman interface { diff --git a/cmd/run.go b/cmd/run.go index a7c641fa5d..ba67a0bc42 100644 --- a/cmd/run.go +++ b/cmd/run.go @@ -257,12 +257,15 @@ func newKeyFromKeystore(path, password string) (*keystore.Key, error) { func newAuthFromKeystore(path, password string, chainID uint64) (*bind.TransactOpts, error) { key, err := newKeyFromKeystore(path, password) if err != nil { - log.Fatal(err) + return nil, err + } + if key == nil { + return nil, nil } log.Info("addr: ", key.Address.Hex()) auth, err := bind.NewKeyedTransactorWithChainID(key.PrivateKey, new(big.Int).SetUint64(chainID)) if err != nil { - log.Fatal(err) + return nil, err } return auth, nil } diff --git a/config/config_test.go b/config/config_test.go index 0762cfb933..6b6161fc5c 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -14,6 +14,7 @@ import ( "github.com/0xPolygonHermez/zkevm-node/config/types" "github.com/0xPolygonHermez/zkevm-node/pricegetter" "github.com/0xPolygonHermez/zkevm-node/sequencer" + "github.com/ethereum/go-ethereum/common" "github.com/stretchr/testify/require" "github.com/urfave/cli/v2" ) @@ -111,6 +112,38 @@ func Test_Defaults(t *testing.T) { path: "Sequencer.MaxAllowedFailedCounter", expectedValue: uint64(50), }, + { + path: "Etherman.URL", + expectedValue: "http://localhost:8545", + }, + { + path: "Etherman.L1ChainID", + expectedValue: uint64(1337), + }, + { + path: "Etherman.PrivateKeyPath", + expectedValue: "", + }, + { + path: "Etherman.PrivateKeyPassword", + expectedValue: "", + }, + { + path: "Etherman.PoEAddr", + expectedValue: common.HexToAddress("0x2279B7A0a67DB372996a5FaB50D91eAA73d2eBe6"), + }, + { + path: "Etherman.MaticAddr", + expectedValue: common.HexToAddress("0x5FbDB2315678afecb367f032d93F642f64180aa3"), + }, + { + path: "Etherman.GlobalExitRootManagerAddr", + expectedValue: common.HexToAddress("0xDc64a140Aa3E981100a9becA4E685f962f0cF6C9"), + }, + { + path: "Etherman.MultiGasProvider", + expectedValue: true, + }, { path: "EthTxManager.MaxSendBatchTxRetries", expectedValue: uint32(10), diff --git a/config/default.go b/config/default.go index 67336fd9a9..e8db30bb2f 100644 --- a/config/default.go +++ b/config/default.go @@ -29,8 +29,6 @@ MaxConns = 200 [Etherman] URL = "http://localhost:8545" L1ChainID = 1337 -PrivateKeyPath = "/pk/keystore" -PrivateKeyPassword = "testonly" PoEAddr = "0x2279B7A0a67DB372996a5FaB50D91eAA73d2eBe6" MaticAddr = "0x5FbDB2315678afecb367f032d93F642f64180aa3" GlobalExitRootManagerAddr = "0xDc64a140Aa3E981100a9becA4E685f962f0cF6C9" diff --git a/docker-compose.yml b/docker-compose.yml index bc067b064b..1f8091cc0a 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -14,7 +14,6 @@ services: - ZKEVM_NODE_RPC_DB_HOST=zkevm-rpc-db - ZKEVM_NODE_RPC_BROADCASTURI=zkevm-broadcast:61090 volumes: - - ./config/example.keystore:/pk/keystore - ./config/environments/public/public.node.config.toml:/app/config.toml - ./config/environments/public/public.genesis.config.json:/app/genesis.json command: @@ -28,7 +27,6 @@ services: environment: - ZKEVM_NODE_STATEDB_HOST=zkevm-state-db volumes: - - ./config/example.keystore:/pk/keystore - ./config/environments/public/public.node.config.toml:/app/config.toml - ./config/environments/public/public.genesis.config.json:/app/genesis.json command: diff --git a/etherman/etherman.go b/etherman/etherman.go index 843fdb9b24..a3ffd3e101 100644 --- a/etherman/etherman.go +++ b/etherman/etherman.go @@ -48,6 +48,9 @@ var ( // ErrNotFound is used when the object is not found ErrNotFound = errors.New("Not found") + // ErrIsReadOnlyMode is used when the EtherMan client is in read-only mode. + ErrIsReadOnlyMode = errors.New("Etherman client in read-only mode: no account configured to send transactions to L1. " + + "Please check the [Etherman] PrivateKeyPath and PrivateKeyPassword configuration.") ) // EventOrder is the the type used to identify the events order @@ -90,7 +93,7 @@ type Client struct { GasProviders externalGasProviders - auth *bind.TransactOpts + auth *bind.TransactOpts // nil in case of read-only client } // NewClient creates a new etherman. @@ -138,9 +141,14 @@ func NewClient(cfg Config, auth *bind.TransactOpts) (*Client, error) { MultiGasProvider: cfg.MultiGasProvider, Providers: gProviders, }, - auth: auth}, nil + auth: auth, + }, nil } +// IsReadOnly returns whether the EtherMan client is in read-only mode. +// Call this before trying to access the `auth` field. +func (c *Client) IsReadOnly() bool { return c.auth == nil } + // GetRollupInfoByBlockRange function retrieves the Rollup information that are included in all this ethereum blocks // from block x to block y. func (etherMan *Client) GetRollupInfoByBlockRange(ctx context.Context, fromBlock uint64, toBlock *uint64) ([]Block, map[common.Hash][]Order, error) { @@ -265,6 +273,9 @@ func (etherMan *Client) WaitTxToBeMined(ctx context.Context, tx *types.Transacti // EstimateGasSequenceBatches estimates gas for sending batches func (etherMan *Client) EstimateGasSequenceBatches(sequences []ethmanTypes.Sequence) (*types.Transaction, error) { + if etherMan.IsReadOnly() { + return nil, ErrIsReadOnlyMode + } noSendOpts := *etherMan.auth noSendOpts.NoSend = true tx, err := etherMan.sequenceBatches(&noSendOpts, sequences) @@ -277,6 +288,9 @@ func (etherMan *Client) EstimateGasSequenceBatches(sequences []ethmanTypes.Seque // SequenceBatches send sequences of batches to the ethereum func (etherMan *Client) SequenceBatches(ctx context.Context, sequences []ethmanTypes.Sequence, gasLimit uint64, gasPrice, nonce *big.Int) (*types.Transaction, error) { + if etherMan.IsReadOnly() { + return nil, ErrIsReadOnlyMode + } sendSequencesOpts := *etherMan.auth sendSequencesOpts.GasLimit = gasLimit if gasPrice != nil { @@ -319,6 +333,9 @@ func (etherMan *Client) sequenceBatches(opts *bind.TransactOpts, sequences []eth // EstimateGasForVerifyBatch estimates gas for verify batch smart contract call func (etherMan *Client) EstimateGasForVerifyBatch(batchNumber uint64, resGetProof *pb.GetProofResponse) (uint64, error) { + if etherMan.IsReadOnly() { + return 0, ErrIsReadOnlyMode + } verifyBatchOpts := *etherMan.auth verifyBatchOpts.NoSend = true tx, err := etherMan.verifyBatch(&verifyBatchOpts, batchNumber, resGetProof) @@ -330,6 +347,9 @@ func (etherMan *Client) EstimateGasForVerifyBatch(batchNumber uint64, resGetProo // VerifyBatch send verifyBatch request to the ethereum func (etherMan *Client) VerifyBatch(ctx context.Context, batchNumber uint64, resGetProof *pb.GetProofResponse, gasLimit uint64, gasPrice, nonce *big.Int) (*types.Transaction, error) { + if etherMan.IsReadOnly() { + return nil, ErrIsReadOnlyMode + } verifyBatchOpts := *etherMan.auth verifyBatchOpts.GasLimit = gasLimit if gasPrice != nil { @@ -689,6 +709,9 @@ func (etherMan *Client) GetTxReceipt(ctx context.Context, txHash common.Hash) (* // ApproveMatic function allow to approve tokens in matic smc func (etherMan *Client) ApproveMatic(ctx context.Context, maticAmount *big.Int, to common.Address) (*types.Transaction, error) { + if etherMan.IsReadOnly() { + return nil, ErrIsReadOnlyMode + } opts := *etherMan.auth if etherMan.GasProviders.MultiGasProvider { opts.GasPrice = etherMan.getGasPrice(ctx) @@ -710,8 +733,11 @@ func (etherMan *Client) GetTrustedSequencerURL() (string, error) { } // GetPublicAddress returns eth client public address -func (etherMan *Client) GetPublicAddress() common.Address { - return etherMan.auth.From +func (etherMan *Client) GetPublicAddress() (common.Address, error) { + if etherMan.IsReadOnly() { + return common.Address{}, ErrIsReadOnlyMode + } + return etherMan.auth.From, nil } // GetL2ChainID returns L2 Chain ID diff --git a/etherman/etherman_test.go b/etherman/etherman_test.go index eda7c07b54..4182c9cc37 100644 --- a/etherman/etherman_test.go +++ b/etherman/etherman_test.go @@ -29,7 +29,7 @@ func init() { }) } -//This function prepare the blockchain, the wallet with funds and deploy the smc +// This function prepare the blockchain, the wallet with funds and deploy the smc func newTestingEnv() (ethman *Client, ethBackend *backends.SimulatedBackend, maticAddr common.Address, br *bridge.Bridge) { privateKey, err := crypto.GenerateKey() if err != nil { diff --git a/etherman/simulated.go b/etherman/simulated.go index 3de1b39185..24d724f503 100644 --- a/etherman/simulated.go +++ b/etherman/simulated.go @@ -20,6 +20,10 @@ import ( // NewSimulatedEtherman creates an etherman that uses a simulated blockchain. It's important to notice that the ChainID of the auth // must be 1337. The address that holds the auth will have an initial balance of 10 ETH func NewSimulatedEtherman(cfg Config, auth *bind.TransactOpts) (etherman *Client, ethBackend *backends.SimulatedBackend, maticAddr common.Address, br *bridge.Bridge, err error) { + if auth == nil { + // read only client + return &Client{}, nil, common.Address{}, nil, nil + } // 10000000 ETH in wei balance, _ := new(big.Int).SetString("10000000000000000000000000", 10) //nolint:gomnd address := auth.From diff --git a/ethtxmanager/ethtxmanager_test.go b/ethtxmanager/ethtxmanager_test.go index 92132a8ce0..da3e7ee461 100644 --- a/ethtxmanager/ethtxmanager_test.go +++ b/ethtxmanager/ethtxmanager_test.go @@ -1,18 +1,39 @@ package ethtxmanager import ( + "context" "math/big" "testing" + ethman "github.com/0xPolygonHermez/zkevm-node/etherman" + ethmanTypes "github.com/0xPolygonHermez/zkevm-node/etherman/types" "github.com/stretchr/testify/assert" ) +func TestIncreaseGasLimit(t *testing.T) { + actual := increaseGasLimit(100, 1) + assert.Equal(t, uint64(101), actual) +} + func TestIncreaseGasPrice(t *testing.T) { actual := increaseGasPrice(big.NewInt(100), 1) assert.Equal(t, big.NewInt(101), actual) } -func TestIncreaseGasLimit(t *testing.T) { - actual := increaseGasLimit(100, 1) - assert.Equal(t, uint64(101), actual) +func TestSequenceBatchesWithROEthman(t *testing.T) { + ethManRO, _, _, _, _ := ethman.NewSimulatedEtherman(ethman.Config{}, nil) + txMan := New(Config{MaxSendBatchTxRetries: 2}, ethManRO) // 3 executions in total + + err := txMan.SequenceBatches(context.Background(), []ethmanTypes.Sequence{}) + + assert.ErrorIs(t, err, ethman.ErrIsReadOnlyMode) +} + +func TestVerifyBatchWithROEthman(t *testing.T) { + ethManRO, _, _, _, _ := ethman.NewSimulatedEtherman(ethman.Config{}, nil) + txMan := New(Config{MaxVerifyBatchTxRetries: 2}, ethManRO) // 3 executions in total + + err := txMan.VerifyBatch(context.Background(), 42, nil) + + assert.ErrorIs(t, err, ethman.ErrIsReadOnlyMode) } diff --git a/test/config/test.node.config.toml b/test/config/test.node.config.toml index 2e0ca4ac02..1a4235821b 100644 --- a/test/config/test.node.config.toml +++ b/test/config/test.node.config.toml @@ -26,6 +26,9 @@ MaxConns = 200 URL = "http://zkevm-mock-l1-network:8545" PrivateKeyPath = "/pk/keystore" PrivateKeyPassword = "testonly" +PoEAddr = "0x2279B7A0a67DB372996a5FaB50D91eAA73d2eBe6" +MaticAddr = "0x5FbDB2315678afecb367f032d93F642f64180aa3" +GlobalExitRootManagerAddr = "0xDc64a140Aa3E981100a9becA4E685f962f0cF6C9" [EthTxManager] MaxSendBatchTxRetries = 10