op-proposer: retry failed output proposals (#11291)

* op-proposer: add retries to output proposal

* op-proposer: proposeOutput returns err to help trigger retry

* op-proposer: use retry.Do for FetchOutput, add unit tests

* op-proposer: improve output fetching retry impl

* op-proposer: move done signal check into inner loop

---------
Co-authored-by: Sebastian Stammler <seb@oplabs.co>

op-e2e/actions/l2_proposer.go

@@ -3,6 +3,7 @@ package actions
 import (
 	"context"
 	"crypto/ecdsa"
+	"encoding/binary"
 	"math/big"
 	"time"
 
@@ -31,6 +32,7 @@ type ProposerCfg struct {
 	OutputOracleAddr       *common.Address
 	DisputeGameFactoryAddr *common.Address
 	ProposalInterval       time.Duration
+	ProposalRetryInterval  time.Duration
 	DisputeGameType        uint32
 	ProposerKey            *ecdsa.PrivateKey
 	AllowNonFinalized      bool
@@ -77,6 +79,7 @@ func NewL2Proposer(t Testing, log log.Logger, cfg *ProposerCfg, l1 *ethclient.Cl
 		PollInterval:           time.Second,
 		NetworkTimeout:         time.Second,
 		ProposalInterval:       cfg.ProposalInterval,
+		OutputRetryInterval:    cfg.ProposalRetryInterval,
 		L2OutputOracleAddr:     cfg.OutputOracleAddr,
 		DisputeGameFactoryAddr: cfg.DisputeGameFactoryAddr,
 		DisputeGameType:        cfg.DisputeGameType,
@@ -206,18 +209,12 @@ func toCallArg(msg ethereum.CallMsg) interface{} {
 
 func (p *L2Proposer) fetchNextOutput(t Testing) (*eth.OutputResponse, bool, error) {
 	if e2eutils.UseFaultProofs() {
-		blockNumber, err := p.driver.FetchCurrentBlockNumber(t.Ctx())
+		output, err := p.driver.FetchDGFOutput(t.Ctx())
 		if err != nil {
 			return nil, false, err
 		}
-
-		output, _, err := p.driver.FetchOutput(t.Ctx(), blockNumber)
-		if err != nil {
-			return nil, false, err
-		}
-
 		encodedBlockNumber := make([]byte, 32)
-		copy(encodedBlockNumber[32-len(blockNumber.Bytes()):], blockNumber.Bytes())
+		binary.BigEndian.PutUint64(encodedBlockNumber[24:], output.BlockRef.Number)
 		game, err := p.disputeGameFactory.Games(&bind.CallOpts{}, p.driver.Cfg.DisputeGameType, output.OutputRoot, encodedBlockNumber)
 		if err != nil {
 			return nil, false, err
@@ -228,7 +225,7 @@ func (p *L2Proposer) fetchNextOutput(t Testing) (*eth.OutputResponse, bool, erro
 
 		return output, true, nil
 	} else {
-		return p.driver.FetchNextOutputInfo(t.Ctx())
+		return p.driver.FetchL2OOOutput(t.Ctx())
 	}
 }
 

op-e2e/actions/l2_proposer_test.go

@@ -64,6 +64,7 @@ func RunProposerTest(gt *testing.T, deltaTimeOffset *hexutil.Uint64) {
 		proposer = NewL2Proposer(t, log, &ProposerCfg{
 			DisputeGameFactoryAddr: &sd.DeploymentsL1.DisputeGameFactoryProxy,
 			ProposalInterval:       6 * time.Second,
+			ProposalRetryInterval:  3 * time.Second,
 			DisputeGameType:        respectedGameType,
 			ProposerKey:            dp.Secrets.Proposer,
 			AllowNonFinalized:      true,
@@ -72,6 +73,7 @@ func RunProposerTest(gt *testing.T, deltaTimeOffset *hexutil.Uint64) {
 		proposer = NewL2Proposer(t, log, &ProposerCfg{
 			OutputOracleAddr:      &sd.DeploymentsL1.L2OutputOracleProxy,
 			ProposerKey:           dp.Secrets.Proposer,
+			ProposalRetryInterval: 3 * time.Second,
 			AllowNonFinalized:     false,
 		}, miner.EthClient(), rollupSeqCl)
 	}

op-e2e/actions/user_test.go

@@ -144,6 +144,7 @@ func runCrossLayerUserTest(gt *testing.T, test hardforkScheduledTest) {
 		proposer = NewL2Proposer(t, log, &ProposerCfg{
 			DisputeGameFactoryAddr: &sd.DeploymentsL1.DisputeGameFactoryProxy,
 			ProposalInterval:       6 * time.Second,
+			ProposalRetryInterval:  3 * time.Second,
 			DisputeGameType:        respectedGameType,
 			ProposerKey:            dp.Secrets.Proposer,
 			AllowNonFinalized:      true,
@@ -152,6 +153,7 @@ func runCrossLayerUserTest(gt *testing.T, test hardforkScheduledTest) {
 		proposer = NewL2Proposer(t, log, &ProposerCfg{
 			OutputOracleAddr:      &sd.DeploymentsL1.L2OutputOracleProxy,
 			ProposerKey:           dp.Secrets.Proposer,
+			ProposalRetryInterval: 3 * time.Second,
 			AllowNonFinalized:     true,
 		}, miner.EthClient(), seq.RollupClient())
 	}

op-e2e/setup.go

@@ -851,6 +851,7 @@ func (cfg SystemConfig) Start(t *testing.T, _opts ...SystemConfigOption) (*Syste
 			ProposalInterval:    6 * time.Second,
 			DisputeGameType:     254, // Fast game type
 			PollInterval:        50 * time.Millisecond,
+			OutputRetryInterval: 10 * time.Millisecond,
 			TxMgrConfig:         newTxMgrConfig(sys.EthInstances[RoleL1].WSEndpoint(), cfg.Secrets.Proposer),
 			AllowNonFinalized:   cfg.NonFinalizedProposals,
 			LogConfig: oplog.CLIConfig{
@@ -864,6 +865,7 @@ func (cfg SystemConfig) Start(t *testing.T, _opts ...SystemConfigOption) (*Syste
 			RollupRpc:           sys.RollupNodes[RoleSeq].HTTPEndpoint(),
 			L2OOAddress:         config.L1Deployments.L2OutputOracleProxy.Hex(),
 			PollInterval:        50 * time.Millisecond,
+			OutputRetryInterval: 10 * time.Millisecond,
 			TxMgrConfig:         newTxMgrConfig(sys.EthInstances[RoleL1].WSEndpoint(), cfg.Secrets.Proposer),
 			AllowNonFinalized:   cfg.NonFinalizedProposals,
 			LogConfig: oplog.CLIConfig{

op-proposer/flags/flags.go

@@ -41,8 +41,8 @@ var (
 	}
 	PollIntervalFlag = &cli.DurationFlag{
 		Name:    "poll-interval",
-		Usage:   "How frequently to poll L2 for new blocks",
-		Value:   6 * time.Second,
+		Usage:   "How frequently to poll L2 for new blocks (legacy L2OO)",
+		Value:   12 * time.Second,
 		EnvVars: prefixEnvVars("POLL_INTERVAL"),
 	}
 	AllowNonFinalizedFlag = &cli.BoolFlag{
@@ -60,6 +60,12 @@ var (
 		Usage:   "Interval between submitting L2 output proposals when the dispute game factory address is set",
 		EnvVars: prefixEnvVars("PROPOSAL_INTERVAL"),
 	}
+	OutputRetryIntervalFlag = &cli.DurationFlag{
+		Name:    "output-retry-interval",
+		Usage:   "Interval between retrying output fetching (DGF)",
+		Value:   12 * time.Second,
+		EnvVars: prefixEnvVars("OUTPUT_RETRY_INTERVAL"),
+	}
 	DisputeGameTypeFlag = &cli.UintFlag{
 		Name:    "game-type",
 		Usage:   "Dispute game type to create via the configured DisputeGameFactory",
@@ -95,6 +101,7 @@ var optionalFlags = []cli.Flag{
 	L2OutputHDPathFlag,
 	DisputeGameFactoryAddressFlag,
 	ProposalIntervalFlag,
+	OutputRetryIntervalFlag,
 	DisputeGameTypeFlag,
 	ActiveSequencerCheckDurationFlag,
 	WaitNodeSyncFlag,

op-proposer/proposer/config.go

@@ -53,6 +53,9 @@ type CLIConfig struct {
 	// ProposalInterval is the delay between submitting L2 output proposals when the DGFAddress is set.
 	ProposalInterval time.Duration
 
+	// OutputRetryInterval is the delay between retrying output fetch if one fails.
+	OutputRetryInterval time.Duration
+
 	// DisputeGameType is the type of dispute game to create when submitting an output proposal.
 	DisputeGameType uint32
 
@@ -110,6 +113,7 @@ func NewConfig(ctx *cli.Context) *CLIConfig {
 		PprofConfig:                  oppprof.ReadCLIConfig(ctx),
 		DGFAddress:                   ctx.String(flags.DisputeGameFactoryAddressFlag.Name),
 		ProposalInterval:             ctx.Duration(flags.ProposalIntervalFlag.Name),
+		OutputRetryInterval:          ctx.Duration(flags.OutputRetryIntervalFlag.Name),
 		DisputeGameType:              uint32(ctx.Uint(flags.DisputeGameTypeFlag.Name)),
 		ActiveSequencerCheckDuration: ctx.Duration(flags.ActiveSequencerCheckDurationFlag.Name),
 		WaitNodeSync:                 ctx.Bool(flags.WaitNodeSyncFlag.Name),

op-proposer/proposer/driver_test.go

+package proposer
+
+import (
+	"context"
+	"fmt"
+	"math/big"
+	"testing"
+	"time"
+
+	"github.com/ethereum-optimism/optimism/op-proposer/bindings"
+	"github.com/ethereum-optimism/optimism/op-proposer/metrics"
+	"github.com/ethereum-optimism/optimism/op-service/dial"
+	"github.com/ethereum-optimism/optimism/op-service/eth"
+	"github.com/ethereum-optimism/optimism/op-service/testlog"
+	"github.com/ethereum-optimism/optimism/op-service/testutils"
+	txmgrmocks "github.com/ethereum-optimism/optimism/op-service/txmgr/mocks"
+
+	"github.com/ethereum/go-ethereum/accounts/abi/bind"
+	"github.com/ethereum/go-ethereum/common"
+	"github.com/ethereum/go-ethereum/core/types"
+	"github.com/ethereum/go-ethereum/log"
+	"github.com/stretchr/testify/mock"
+	"github.com/stretchr/testify/require"
+)
+
+type MockL2OOContract struct {
+	mock.Mock
+}
+
+func (m *MockL2OOContract) Version(opts *bind.CallOpts) (string, error) {
+	args := m.Called(opts)
+	return args.String(0), args.Error(1)
+}
+
+func (m *MockL2OOContract) NextBlockNumber(opts *bind.CallOpts) (*big.Int, error) {
+	args := m.Called(opts)
+	return args.Get(0).(*big.Int), args.Error(1)
+}
+
+type mockRollupEndpointProvider struct {
+	rollupClient    *testutils.MockRollupClient
+	rollupClientErr error
+}
+
+func newEndpointProvider() *mockRollupEndpointProvider {
+	return &mockRollupEndpointProvider{
+		rollupClient: new(testutils.MockRollupClient),
+	}
+}
+
+func (p *mockRollupEndpointProvider) RollupClient(context.Context) (dial.RollupClientInterface, error) {
+	return p.rollupClient, p.rollupClientErr
+}
+
+func (p *mockRollupEndpointProvider) Close() {}
+
+func setup(t *testing.T) (*L2OutputSubmitter, *mockRollupEndpointProvider, *MockL2OOContract, *txmgrmocks.TxManager, *testlog.CapturingHandler) {
+	ep := newEndpointProvider()
+
+	l2OutputOracleAddr := common.HexToAddress("0x3F8A862E63E759a77DA22d384027D21BF096bA9E")
+
+	proposerConfig := ProposerConfig{
+		PollInterval:        time.Microsecond,
+		ProposalInterval:    time.Microsecond,
+		OutputRetryInterval: time.Microsecond,
+		L2OutputOracleAddr:  &l2OutputOracleAddr,
+	}
+
+	txmgr := txmgrmocks.NewTxManager(t)
+
+	lgr, logs := testlog.CaptureLogger(t, log.LevelDebug)
+	setup := DriverSetup{
+		Log:            lgr,
+		Metr:           metrics.NoopMetrics,
+		Cfg:            proposerConfig,
+		Txmgr:          txmgr,
+		RollupProvider: ep,
+	}
+
+	parsed, err := bindings.L2OutputOracleMetaData.GetAbi()
+	require.NoError(t, err)
+
+	ctx, cancel := context.WithCancel(context.Background())
+	l2ooContract := new(MockL2OOContract)
+	l2OutputSubmitter := L2OutputSubmitter{
+		DriverSetup:  setup,
+		done:         make(chan struct{}),
+		l2ooContract: l2ooContract,
+		l2ooABI:      parsed,
+		ctx:          ctx,
+		cancel:       cancel,
+	}
+
+	txmgr.On("BlockNumber", mock.Anything).Return(uint64(100), nil).Once()
+	txmgr.On("Send", mock.Anything, mock.Anything).
+		Return(&types.Receipt{Status: uint64(1), TxHash: common.Hash{}}, nil).
+		Once().
+		Run(func(_ mock.Arguments) {
+			// let loops return after first Send call
+			t.Log("Closing proposer.")
+			close(l2OutputSubmitter.done)
+		})
+
+	return &l2OutputSubmitter, ep, l2ooContract, txmgr, logs
+}
+
+func TestL2OutputSubmitter_OutputRetry(t *testing.T) {
+	tests := []struct {
+		name string
+	}{
+		{name: "L2OO"},
+		{name: "DGF"},
+	}
+
+	const numFails = 3
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			ps, ep, l2ooContract, txmgr, logs := setup(t)
+
+			ep.rollupClient.On("SyncStatus").Return(&eth.SyncStatus{FinalizedL2: eth.L2BlockRef{Number: 42}}, nil).Times(numFails + 1)
+			ep.rollupClient.ExpectOutputAtBlock(42, nil, fmt.Errorf("TEST: failed to fetch output")).Times(numFails)
+			ep.rollupClient.ExpectOutputAtBlock(
+				42,
+				&eth.OutputResponse{
+					Version:  supportedL2OutputVersion,
+					BlockRef: eth.L2BlockRef{Number: 42},
+					Status: &eth.SyncStatus{
+						CurrentL1:   eth.L1BlockRef{Hash: common.Hash{}},
+						FinalizedL2: eth.L2BlockRef{Number: 42},
+					},
+				},
+				nil,
+			)
+
+			if tt.name == "DGF" {
+				ps.loopDGF(ps.ctx)
+			} else {
+				txmgr.On("From").Return(common.Address{}).Times(numFails + 1)
+				l2ooContract.On("NextBlockNumber", mock.AnythingOfType("*bind.CallOpts")).Return(big.NewInt(42), nil).Times(numFails + 1)
+				ps.loopL2OO(ps.ctx)
+			}
+
+			ep.rollupClient.AssertExpectations(t)
+			l2ooContract.AssertExpectations(t)
+			require.Len(t, logs.FindLogs(testlog.NewMessageContainsFilter("Error getting "+tt.name)), numFails)
+			require.NotNil(t, logs.FindLog(testlog.NewMessageFilter("Proposer tx successfully published")))
+			require.NotNil(t, logs.FindLog(testlog.NewMessageFilter("loop"+tt.name+" returning")))
+		})
+	}
+}

op-proposer/proposer/service.go

@@ -32,6 +32,9 @@ type ProposerConfig struct {
 	PollInterval   time.Duration
 	NetworkTimeout time.Duration
 
+	// How frequently to retry fetching an output if one fails
+	OutputRetryInterval time.Duration
+
 	// How frequently to post L2 outputs when the DisputeGameFactory is configured
 	ProposalInterval time.Duration
 
@@ -89,6 +92,7 @@ func (ps *ProposerService) initFromCLIConfig(ctx context.Context, version string
 	ps.initMetrics(cfg)
 
 	ps.PollInterval = cfg.PollInterval
+	ps.OutputRetryInterval = cfg.OutputRetryInterval
 	ps.NetworkTimeout = cfg.TxMgrConfig.NetworkTimeout
 	ps.AllowNonFinalized = cfg.AllowNonFinalized
 	ps.WaitNodeSync = cfg.WaitNodeSync

op-service/testutils/mock_rollup_client.go

@@ -18,8 +18,8 @@ func (m *MockRollupClient) OutputAtBlock(ctx context.Context, blockNum uint64) (
 	return out.Get(0).(*eth.OutputResponse), out.Error(1)
 }
 
-func (m *MockRollupClient) ExpectOutputAtBlock(blockNum uint64, response *eth.OutputResponse, err error) {
-	m.Mock.On("OutputAtBlock", blockNum).Once().Return(response, err)
+func (m *MockRollupClient) ExpectOutputAtBlock(blockNum uint64, response *eth.OutputResponse, err error) *mock.Call {
+	return m.Mock.On("OutputAtBlock", blockNum).Once().Return(response, err)
 }
 
 func (m *MockRollupClient) SyncStatus(ctx context.Context) (*eth.SyncStatus, error) {