Etherman optionally readonly. (0xPolygonHermez#1321)

Closes: 0xPolygonHermez#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 (0xPolygonHermez#1337) Co-authored-by: Konrad Iturbe <[email protected]>
alagbe003 · Nov 2, 2022 · 149a032 · 149a032
1 parent 63a0490
commit 149a032
Show file tree

Hide file tree

Showing 14 changed files with 121 additions and 22 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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