Merge pull request #2000 from OffchainLabs/external_signer_for_validator

Fix Validator when using external signer to use correct From address, don't require wallet for validator if external signer is enabled

arbnode/dataposter/data_poster.go

@@ -53,7 +53,7 @@ type DataPoster struct {
 	stopwaiter.StopWaiter
 	headerReader      *headerreader.HeaderReader
 	client            arbutil.L1Interface
-	sender            common.Address
+	auth              *bind.TransactOpts
 	signer            signerFn
 	redisLock         AttemptLocker
 	config            ConfigFetcher
@@ -162,7 +162,7 @@ func NewDataPoster(ctx context.Context, opts *DataPosterOpts) (*DataPoster, erro
 	dp := &DataPoster{
 		headerReader: opts.HeaderReader,
 		client:       opts.HeaderReader.Client(),
-		sender:       opts.Auth.From,
+		auth:         opts.Auth,
 		signer: func(_ context.Context, addr common.Address, tx *types.Transaction) (*types.Transaction, error) {
 			return opts.Auth.Signer(addr, tx)
 		},
@@ -180,7 +180,13 @@ func NewDataPoster(ctx context.Context, opts *DataPosterOpts) (*DataPoster, erro
 		if err != nil {
 			return nil, err
 		}
-		dp.signer, dp.sender = signer, sender
+		dp.signer = signer
+		dp.auth = &bind.TransactOpts{
+			From: sender,
+			Signer: func(address common.Address, tx *types.Transaction) (*types.Transaction, error) {
+				return signer(context.TODO(), address, tx)
+			},
+		}
 	}
 	return dp, nil
 }
@@ -259,8 +265,12 @@ func externalSigner(ctx context.Context, opts *ExternalSignerCfg) (signerFn, com
 	}, sender, nil
 }
 
+func (p *DataPoster) Auth() *bind.TransactOpts {
+	return p.auth
+}
+
 func (p *DataPoster) Sender() common.Address {
-	return p.sender
+	return p.auth.From
 }
 
 func (p *DataPoster) MaxMempoolTransactions() uint64 {
@@ -287,7 +297,7 @@ func (p *DataPoster) canPostWithNonce(ctx context.Context, nextNonce uint64) err
 	// Check that posting a new transaction won't exceed maximum pending
 	// transactions in mempool.
 	if cfg.MaxMempoolTransactions > 0 {
-		unconfirmedNonce, err := p.client.NonceAt(ctx, p.sender, nil)
+		unconfirmedNonce, err := p.client.NonceAt(ctx, p.Sender(), nil)
 		if err != nil {
 			return fmt.Errorf("getting nonce of a dataposter sender: %w", err)
 		}
@@ -330,7 +340,7 @@ func (p *DataPoster) getNextNonceAndMaybeMeta(ctx context.Context) (uint64, []by
 		// Fall back to using a recent block to get the nonce. This is safe because there's nothing in the queue.
 		nonceQueryBlock := arbmath.UintToBig(arbmath.SaturatingUSub(blockNum, 1))
 		log.Warn("failed to update nonce with queue empty; falling back to using a recent block", "recentBlock", nonceQueryBlock, "err", err)
-		nonce, err := p.client.NonceAt(ctx, p.sender, nonceQueryBlock)
+		nonce, err := p.client.NonceAt(ctx, p.Sender(), nonceQueryBlock)
 		if err != nil {
 			return 0, nil, false, fmt.Errorf("failed to get nonce at block %v: %w", nonceQueryBlock, err)
 		}
@@ -387,7 +397,7 @@ func (p *DataPoster) feeAndTipCaps(ctx context.Context, nonce uint64, gasLimit u
 		return nil, nil, fmt.Errorf("latest parent chain block %v missing BaseFee (either the parent chain does not have EIP-1559 or the parent chain node is not synced)", latestHeader.Number)
 	}
 	softConfBlock := arbmath.BigSubByUint(latestHeader.Number, config.NonceRbfSoftConfs)
-	softConfNonce, err := p.client.NonceAt(ctx, p.sender, softConfBlock)
+	softConfNonce, err := p.client.NonceAt(ctx, p.Sender(), softConfBlock)
 	if err != nil {
 		return nil, nil, fmt.Errorf("failed to get latest nonce %v blocks ago (block %v): %w", config.NonceRbfSoftConfs, softConfBlock, err)
 	}
@@ -500,7 +510,7 @@ func (p *DataPoster) PostTransaction(ctx context.Context, dataCreatedAt time.Tim
 		Data:       calldata,
 		AccessList: accessList,
 	}
-	fullTx, err := p.signer(ctx, p.sender, types.NewTx(&inner))
+	fullTx, err := p.signer(ctx, p.Sender(), types.NewTx(&inner))
 	if err != nil {
 		return nil, fmt.Errorf("signing transaction: %w", err)
 	}
@@ -579,7 +589,7 @@ func (p *DataPoster) replaceTx(ctx context.Context, prevTx *storage.QueuedTransa
 	newTx.Sent = false
 	newTx.Data.GasFeeCap = newFeeCap
 	newTx.Data.GasTipCap = newTipCap
-	newTx.FullTx, err = p.signer(ctx, p.sender, types.NewTx(&newTx.Data))
+	newTx.FullTx, err = p.signer(ctx, p.Sender(), types.NewTx(&newTx.Data))
 	if err != nil {
 		return err
 	}
@@ -602,7 +612,7 @@ func (p *DataPoster) updateNonce(ctx context.Context) error {
 	if p.lastBlock != nil && arbmath.BigEquals(p.lastBlock, header.Number) {
 		return nil
 	}
-	nonce, err := p.client.NonceAt(ctx, p.sender, header.Number)
+	nonce, err := p.client.NonceAt(ctx, p.Sender(), header.Number)
 	if err != nil {
 		if p.lastBlock != nil {
 			log.Warn("Failed to get current nonce", "lastBlock", p.lastBlock, "newBlock", header.Number, "err", err)
@@ -640,7 +650,7 @@ func (p *DataPoster) updateNonce(ctx context.Context) error {
 func (p *DataPoster) updateBalance(ctx context.Context) error {
 	// Use the pending (representated as -1) balance because we're looking at batches we'd post,
 	// so we want to see how much gas we could afford with our pending state.
-	balance, err := p.client.BalanceAt(ctx, p.sender, big.NewInt(-1))
+	balance, err := p.client.BalanceAt(ctx, p.Sender(), big.NewInt(-1))
 	if err != nil {
 		return err
 	}
@@ -695,7 +705,7 @@ func (p *DataPoster) Start(ctxIn context.Context) {
 		if maxTxsToRbf == 0 {
 			maxTxsToRbf = 512
 		}
-		unconfirmedNonce, err := p.client.NonceAt(ctx, p.sender, nil)
+		unconfirmedNonce, err := p.client.NonceAt(ctx, p.Sender(), nil)
 		if err != nil {
 			log.Warn("Failed to get latest nonce", "err", err)
 			return minWait

arbnode/node.go

@@ -302,10 +302,10 @@ func StakerDataposter(
 	ctx context.Context, db ethdb.Database, l1Reader *headerreader.HeaderReader,
 	transactOpts *bind.TransactOpts, cfgFetcher ConfigFetcher, syncMonitor *SyncMonitor,
 ) (*dataposter.DataPoster, error) {
-	if transactOpts == nil {
+	cfg := cfgFetcher.Get()
+	if transactOpts == nil && cfg.Staker.DataPoster.ExternalSigner.URL == "" {
 		return nil, nil
 	}
-	cfg := cfgFetcher.Get()
 	mdRetriever := func(ctx context.Context, blockNum *big.Int) ([]byte, error) {
 		return nil, nil
 	}
@@ -323,6 +323,12 @@ func StakerDataposter(
 	dpCfg := func() *dataposter.DataPosterConfig {
 		return &cfg.Staker.DataPoster
 	}
+	var sender string
+	if transactOpts != nil {
+		sender = transactOpts.From.String()
+	} else {
+		sender = cfg.Staker.DataPoster.ExternalSigner.Address
+	}
 	return dataposter.NewDataPoster(ctx,
 		&dataposter.DataPosterOpts{
 			Database:          db,
@@ -332,8 +338,7 @@ func StakerDataposter(
 			RedisLock:         redisLock,
 			Config:            dpCfg,
 			MetadataRetriever: mdRetriever,
-			// transactOpts is non-nil, it's checked at the beginning.
-			RedisKey: transactOpts.From.String() + ".staker-data-poster.queue",
+			RedisKey:          sender + ".staker-data-poster.queue",
 		})
 }
 
@@ -580,7 +585,7 @@ func createNodeImpl(
 		// creation into multiple helpers.
 		var wallet staker.ValidatorWalletInterface = validatorwallet.NewNoOp(l1client, deployInfo.Rollup)
 		if !strings.EqualFold(config.Staker.Strategy, "watchtower") {
-			if config.Staker.UseSmartContractWallet || txOptsValidator == nil {
+			if config.Staker.UseSmartContractWallet || (txOptsValidator == nil && config.Staker.DataPoster.ExternalSigner.URL == "") {
 				var existingWalletAddress *common.Address
 				if len(config.Staker.ContractWalletAddress) > 0 {
 					if !common.IsHexAddress(config.Staker.ContractWalletAddress) {
@@ -598,7 +603,7 @@ func createNodeImpl(
 				if len(config.Staker.ContractWalletAddress) > 0 {
 					return nil, errors.New("validator contract wallet specified but flag to use a smart contract wallet was not specified")
 				}
-				wallet, err = validatorwallet.NewEOA(dp, deployInfo.Rollup, l1client, txOptsValidator, getExtraGas)
+				wallet, err = validatorwallet.NewEOA(dp, deployInfo.Rollup, l1client, getExtraGas)
 				if err != nil {
 					return nil, err
 				}
@@ -618,21 +623,23 @@ func createNodeImpl(
 		if err := wallet.Initialize(ctx); err != nil {
 			return nil, err
 		}
-		var txValidatorSenderPtr *common.Address
+		var validatorAddr string
 		if txOptsValidator != nil {
-			txValidatorSenderPtr = &txOptsValidator.From
+			validatorAddr = txOptsValidator.From.String()
+		} else {
+			validatorAddr = config.Staker.DataPoster.ExternalSigner.Address
 		}
 		whitelisted, err := stakerObj.IsWhitelisted(ctx)
 		if err != nil {
 			return nil, err
 		}
-		log.Info("running as validator", "txSender", txValidatorSenderPtr, "actingAsWallet", wallet.Address(), "whitelisted", whitelisted, "strategy", config.Staker.Strategy)
+		log.Info("running as validator", "txSender", validatorAddr, "actingAsWallet", wallet.Address(), "whitelisted", whitelisted, "strategy", config.Staker.Strategy)
 	}
 
 	var batchPoster *BatchPoster
 	var delayedSequencer *DelayedSequencer
 	if config.BatchPoster.Enable {
-		if txOptsBatchPoster == nil {
+		if txOptsBatchPoster == nil && config.BatchPoster.DataPoster.ExternalSigner.URL == "" {
 			return nil, errors.New("batchposter, but no TxOpts")
 		}
 		batchPoster, err = NewBatchPoster(ctx, &BatchPosterOpts{

cmd/nitro/nitro.go

@@ -238,7 +238,8 @@ func mainImpl() int {
 	// external signing sequencer will need a key.
 	sequencerNeedsKey := (nodeConfig.Node.Sequencer && !nodeConfig.Node.Feed.Output.DisableSigning) ||
 		(nodeConfig.Node.BatchPoster.Enable && nodeConfig.Node.BatchPoster.DataPoster.ExternalSigner.URL == "")
-	validatorNeedsKey := nodeConfig.Node.Staker.OnlyCreateWalletContract || nodeConfig.Node.Staker.Enable && !strings.EqualFold(nodeConfig.Node.Staker.Strategy, "watchtower")
+	validatorNeedsKey := nodeConfig.Node.Staker.OnlyCreateWalletContract ||
+		(nodeConfig.Node.Staker.Enable && !strings.EqualFold(nodeConfig.Node.Staker.Strategy, "watchtower") && nodeConfig.Node.Staker.DataPoster.ExternalSigner.URL == "")
 
 	l1Wallet.ResolveDirectoryNames(nodeConfig.Persistent.Chain)
 	defaultL1WalletConfig := conf.DefaultL1WalletConfig

staker/validatorwallet/eoa.go

@@ -28,9 +28,9 @@ type EOA struct {
 	getExtraGas             func() uint64
 }
 
-func NewEOA(dataPoster *dataposter.DataPoster, rollupAddress common.Address, l1Client arbutil.L1Interface, auth *bind.TransactOpts, getExtraGas func() uint64) (*EOA, error) {
+func NewEOA(dataPoster *dataposter.DataPoster, rollupAddress common.Address, l1Client arbutil.L1Interface, getExtraGas func() uint64) (*EOA, error) {
 	return &EOA{
-		auth:          auth,
+		auth:          dataPoster.Auth(),
 		client:        l1Client,
 		rollupAddress: rollupAddress,
 		dataPoster:    dataPoster,

system_tests/batch_poster_test.go

@@ -61,7 +61,6 @@ func addNewBatchPoster(ctx context.Context, t *testing.T, builder *NodeBuilder,
 }
 
 func testBatchPosterParallel(t *testing.T, useRedis bool) {
-	t.Parallel()
 	ctx, cancel := context.WithCancel(context.Background())
 	defer cancel()
 	httpSrv, srv := newServer(ctx, t)

system_tests/staker_test.go

@@ -12,6 +12,7 @@ import (
 	"errors"
 	"fmt"
 	"math/big"
+	"net/http"
 	"strings"
 	"testing"
 	"time"
@@ -59,6 +60,19 @@ func stakerTestImpl(t *testing.T, faultyStaker bool, honestStakerInactive bool)
 	t.Parallel()
 	ctx, cancelCtx := context.WithCancel(context.Background())
 	defer cancelCtx()
+	httpSrv, srv := newServer(ctx, t)
+	t.Cleanup(func() {
+		if err := httpSrv.Shutdown(ctx); err != nil {
+			t.Fatalf("Error shutting down http server: %v", err)
+		}
+	})
+	go func() {
+		log.Debug("Server is listening on port 1234...")
+		if err := httpSrv.ListenAndServeTLS(signerServerCert, signerServerKey); err != nil && err != http.ErrServerClosed {
+			log.Debug("ListenAndServeTLS() failed", "error", err)
+			return
+		}
+	}()
 	var transferGas = util.NormalizeL2GasForL1GasInitial(800_000, params.GWei) // include room for aggregator L1 costs
 
 	builder := NewNodeBuilder(ctx).DefaultConfig(t, true)
@@ -72,6 +86,11 @@ func stakerTestImpl(t *testing.T, faultyStaker bool, honestStakerInactive bool)
 	cleanupA := builder.Build(t)
 	defer cleanupA()
 
+	addNewBatchPoster(ctx, t, builder, srv.address)
+
+	builder.L1.SendWaitTestTransactions(t, []*types.Transaction{
+		builder.L1Info.PrepareTxTo("Faucet", &srv.address, 30000, big.NewInt(1).Mul(big.NewInt(1e18), big.NewInt(1e18)), nil)})
+
 	l2nodeA := builder.L2.ConsensusNode
 	execNodeA := builder.L2.ExecNode
 
@@ -133,7 +152,7 @@ func stakerTestImpl(t *testing.T, faultyStaker bool, honestStakerInactive bool)
 	rollupABI, err := abi.JSON(strings.NewReader(rollupgen.RollupAdminLogicABI))
 	Require(t, err, "unable to parse rollup ABI")
 
-	setValidatorCalldata, err := rollupABI.Pack("setValidator", []common.Address{valWalletAddrA, l1authB.From}, []bool{true, true})
+	setValidatorCalldata, err := rollupABI.Pack("setValidator", []common.Address{valWalletAddrA, l1authB.From, srv.address}, []bool{true, true, true})
 	Require(t, err, "unable to generate setValidator calldata")
 	tx, err := upgradeExecutor.ExecuteCall(&deployAuth, l2nodeA.DeployInfo.Rollup, setValidatorCalldata)
 	Require(t, err, "unable to set validators")
@@ -199,12 +218,13 @@ func stakerTestImpl(t *testing.T, faultyStaker bool, honestStakerInactive bool)
 		Require(t, err)
 	}
 	Require(t, err)
-
-	dpB, err := arbnode.StakerDataposter(ctx, rawdb.NewTable(l2nodeB.ArbDB, storage.StakerPrefix), l2nodeB.L1Reader, &l1authB, NewFetcherFromConfig(arbnode.ConfigDefaultL1NonSequencerTest()), nil)
+	cfg := arbnode.ConfigDefaultL1NonSequencerTest()
+	cfg.Staker.DataPoster.ExternalSigner = *externalSignerTestCfg(srv.address)
+	dpB, err := arbnode.StakerDataposter(ctx, rawdb.NewTable(l2nodeB.ArbDB, storage.StakerPrefix), l2nodeB.L1Reader, &l1authB, NewFetcherFromConfig(cfg), nil)
 	if err != nil {
 		t.Fatalf("Error creating validator dataposter: %v", err)
 	}
-	valWalletB, err := validatorwallet.NewEOA(dpB, l2nodeB.DeployInfo.Rollup, l2nodeB.L1Reader.Client(), &l1authB, func() uint64 { return 0 })
+	valWalletB, err := validatorwallet.NewEOA(dpB, l2nodeB.DeployInfo.Rollup, l2nodeB.L1Reader.Client(), func() uint64 { return 0 })
 	Require(t, err)
 	valConfig.Strategy = "MakeNodes"
 	statelessB, err := staker.NewStatelessBlockValidator(
@@ -365,14 +385,14 @@ func stakerTestImpl(t *testing.T, faultyStaker bool, honestStakerInactive bool)
 			Require(t, err, "EnsureTxSucceeded failed for staker", stakerName, "tx")
 		}
 		if faultyStaker {
-			conflictInfo, err := validatorUtils.FindStakerConflict(&bind.CallOpts{}, l2nodeA.DeployInfo.Rollup, l1authA.From, l1authB.From, big.NewInt(1024))
+			conflictInfo, err := validatorUtils.FindStakerConflict(&bind.CallOpts{}, l2nodeA.DeployInfo.Rollup, l1authA.From, srv.address, big.NewInt(1024))
 			Require(t, err)
 			if staker.ConflictType(conflictInfo.Ty) == staker.CONFLICT_TYPE_FOUND {
 				cancelBackgroundTxs()
 			}
 		}
 		if faultyStaker && !sawStakerZombie {
-			sawStakerZombie, err = rollup.IsZombie(&bind.CallOpts{}, l1authB.From)
+			sawStakerZombie, err = rollup.IsZombie(&bind.CallOpts{}, srv.address)
 			Require(t, err)
 		}
 		isHonestZombie, err := rollup.IsZombie(&bind.CallOpts{}, valWalletAddrA)
@@ -393,7 +413,7 @@ func stakerTestImpl(t *testing.T, faultyStaker bool, honestStakerInactive bool)
 			Require(t, err)
 		}
 		if !stakerBWasStaked {
-			stakerBWasStaked, err = rollup.IsStaked(&bind.CallOpts{}, l1authB.From)
+			stakerBWasStaked, err = rollup.IsStaked(&bind.CallOpts{}, srv.address)
 			Require(t, err)
 		}
 		for j := 0; j < 5; j++ {