op-challenger: Tidy up TxSender (#9931)

Don't return receipts as they always ignored
Return an error when a transaction publishes but reverts rather than logging and ignoring
Add a method that returns errors for each individual transaction

op-challenger/game/fault/claims/claimer.go

@@ -12,6 +12,10 @@ import (
 	"github.com/ethereum/go-ethereum/log"
 )
 
+type TxSender interface {
+	SendAndWaitSimple(txPurpose string, txs ...txmgr.TxCandidate) error
+}
+
 type BondClaimMetrics interface {
 	RecordBondClaimed(amount uint64)
 }
@@ -27,13 +31,13 @@ type Claimer struct {
 	logger          log.Logger
 	metrics         BondClaimMetrics
 	contractCreator BondContractCreator
-	txSender        types.TxSender
+	txSender        TxSender
 	claimants       []common.Address
 }
 
 var _ BondClaimer = (*Claimer)(nil)
 
-func NewBondClaimer(l log.Logger, m BondClaimMetrics, contractCreator BondContractCreator, txSender types.TxSender, claimants ...common.Address) *Claimer {
+func NewBondClaimer(l log.Logger, m BondClaimMetrics, contractCreator BondContractCreator, txSender TxSender, claimants ...common.Address) *Claimer {
 	return &Claimer{
 		logger:          l,
 		metrics:         m,
@@ -78,7 +82,7 @@ func (c *Claimer) claimBond(ctx context.Context, game types.GameMetadata, addr c
 		return fmt.Errorf("failed to create credit claim tx: %w", err)
 	}
 
-	if _, err = c.txSender.SendAndWait("claim credit", candidate); err != nil {
+	if err = c.txSender.SendAndWaitSimple("claim credit", candidate); err != nil {
 		return fmt.Errorf("failed to claim credit: %w", err)
 	}
 

op-challenger/game/fault/claims/claimer_test.go

@@ -10,7 +10,6 @@ import (
 	"github.com/ethereum-optimism/optimism/op-service/testlog"
 	"github.com/ethereum-optimism/optimism/op-service/txmgr"
 	"github.com/ethereum/go-ethereum/common"
-	ethtypes "github.com/ethereum/go-ethereum/core/types"
 	"github.com/ethereum/go-ethereum/log"
 	"github.com/stretchr/testify/require"
 )
@@ -132,15 +131,15 @@ func (s *mockTxSender) From() common.Address {
 	return common.HexToAddress("0x33333")
 }
 
-func (s *mockTxSender) SendAndWait(_ string, _ ...txmgr.TxCandidate) ([]*ethtypes.Receipt, error) {
+func (s *mockTxSender) SendAndWaitSimple(_ string, _ ...txmgr.TxCandidate) error {
 	s.sends++
 	if s.sendFails {
-		return nil, mockTxMgrSendError
+		return mockTxMgrSendError
 	}
 	if s.statusFail {
-		return []*ethtypes.Receipt{{Status: ethtypes.ReceiptStatusFailed}}, nil
+		return errors.New("transaction reverted")
 	}
-	return []*ethtypes.Receipt{{Status: ethtypes.ReceiptStatusSuccessful}}, nil
+	return nil
 }
 
 type stubBondContract struct {

op-challenger/game/fault/player.go

@@ -14,6 +14,7 @@ import (
 	"github.com/ethereum-optimism/optimism/op-challenger/metrics"
 	"github.com/ethereum-optimism/optimism/op-service/clock"
 	"github.com/ethereum-optimism/optimism/op-service/eth"
+	"github.com/ethereum-optimism/optimism/op-service/txmgr"
 	"github.com/ethereum/go-ethereum/common"
 	gethTypes "github.com/ethereum/go-ethereum/core/types"
 	"github.com/ethereum/go-ethereum/log"
@@ -34,6 +35,11 @@ type L1HeaderSource interface {
 	HeaderByHash(context.Context, common.Hash) (*gethTypes.Header, error)
 }
 
+type TxSender interface {
+	From() common.Address
+	SendAndWaitSimple(txPurpose string, txs ...txmgr.TxCandidate) error
+}
+
 type GamePlayer struct {
 	act                actor
 	loader             GameInfo
@@ -66,7 +72,7 @@ func NewGamePlayer(
 	m metrics.Metricer,
 	dir string,
 	addr common.Address,
-	txSender gameTypes.TxSender,
+	txSender TxSender,
 	loader GameContract,
 	syncValidator SyncValidator,
 	validators []Validator,

op-challenger/game/fault/preimages/direct.go

@@ -5,7 +5,6 @@ import (
 	"fmt"
 
 	"github.com/ethereum-optimism/optimism/op-challenger/game/fault/types"
-	gameTypes "github.com/ethereum-optimism/optimism/op-challenger/game/types"
 	"github.com/ethereum-optimism/optimism/op-service/txmgr"
 	"github.com/ethereum/go-ethereum/log"
 )
@@ -21,11 +20,11 @@ type PreimageGameContract interface {
 type DirectPreimageUploader struct {
 	log log.Logger
 
-	txSender gameTypes.TxSender
+	txSender TxSender
 	contract PreimageGameContract
 }
 
-func NewDirectPreimageUploader(logger log.Logger, txSender gameTypes.TxSender, contract PreimageGameContract) *DirectPreimageUploader {
+func NewDirectPreimageUploader(logger log.Logger, txSender TxSender, contract PreimageGameContract) *DirectPreimageUploader {
 	return &DirectPreimageUploader{logger, txSender, contract}
 }
 
@@ -38,7 +37,7 @@ func (d *DirectPreimageUploader) UploadPreimage(ctx context.Context, claimIdx ui
 	if err != nil {
 		return fmt.Errorf("failed to create pre-image oracle tx: %w", err)
 	}
-	if _, err := d.txSender.SendAndWait("populate pre-image oracle", candidate); err != nil {
+	if err := d.txSender.SendAndWaitSimple("populate pre-image oracle", candidate); err != nil {
 		return fmt.Errorf("failed to populate pre-image oracle: %w", err)
 	}
 	return nil

op-challenger/game/fault/preimages/direct_test.go

@@ -9,7 +9,6 @@ import (
 	"github.com/ethereum-optimism/optimism/op-service/testlog"
 	"github.com/ethereum-optimism/optimism/op-service/txmgr"
 	"github.com/ethereum/go-ethereum/common"
-	ethtypes "github.com/ethereum/go-ethereum/core/types"
 	"github.com/ethereum/go-ethereum/log"
 	"github.com/stretchr/testify/require"
 )
@@ -83,13 +82,13 @@ func (s *mockTxSender) From() common.Address {
 	return common.Address{}
 }
 
-func (s *mockTxSender) SendAndWait(_ string, _ ...txmgr.TxCandidate) ([]*ethtypes.Receipt, error) {
+func (s *mockTxSender) SendAndWaitSimple(_ string, _ ...txmgr.TxCandidate) error {
 	s.sends++
 	if s.sendFails {
-		return nil, mockTxMgrSendError
+		return mockTxMgrSendError
 	}
 	if s.statusFail {
-		return []*ethtypes.Receipt{{Status: ethtypes.ReceiptStatusFailed}}, nil
+		return errors.New("transaction reverted")
 	}
-	return []*ethtypes.Receipt{{Status: ethtypes.ReceiptStatusSuccessful}}, nil
+	return nil
 }

op-challenger/game/fault/preimages/large.go

@@ -12,7 +12,6 @@ import (
 	"github.com/ethereum-optimism/optimism/op-challenger/game/fault/types"
 	"github.com/ethereum-optimism/optimism/op-challenger/game/keccak/matrix"
 	keccakTypes "github.com/ethereum-optimism/optimism/op-challenger/game/keccak/types"
-	gameTypes "github.com/ethereum-optimism/optimism/op-challenger/game/types"
 	"github.com/ethereum-optimism/optimism/op-service/sources/batching/rpcblock"
 	"github.com/ethereum-optimism/optimism/op-service/txmgr"
 	"github.com/ethereum/go-ethereum/common"
@@ -41,11 +40,11 @@ type LargePreimageUploader struct {
 	log log.Logger
 
 	clock    types.ClockReader
-	txSender gameTypes.TxSender
+	txSender TxSender
 	contract PreimageOracleContract
 }
 
-func NewLargePreimageUploader(logger log.Logger, cl types.ClockReader, txSender gameTypes.TxSender, contract PreimageOracleContract) *LargePreimageUploader {
+func NewLargePreimageUploader(logger log.Logger, cl types.ClockReader, txSender TxSender, contract PreimageOracleContract) *LargePreimageUploader {
 	return &LargePreimageUploader{logger, cl, txSender, contract}
 }
 
@@ -151,7 +150,7 @@ func (p *LargePreimageUploader) Squeeze(ctx context.Context, uuid *big.Int, stat
 	if err != nil {
 		return fmt.Errorf("failed to create pre-image oracle tx: %w", err)
 	}
-	if _, err := p.txSender.SendAndWait("squeeze large preimage", tx); err != nil {
+	if err := p.txSender.SendAndWaitSimple("squeeze large preimage", tx); err != nil {
 		return fmt.Errorf("failed to populate pre-image oracle: %w", err)
 	}
 	return nil
@@ -170,7 +169,7 @@ func (p *LargePreimageUploader) initLargePreimage(uuid *big.Int, partOffset uint
 		return fmt.Errorf("failed to get min bond for large preimage proposal: %w", err)
 	}
 	candidate.Value = bond
-	if _, err := p.txSender.SendAndWait("init large preimage", candidate); err != nil {
+	if err := p.txSender.SendAndWaitSimple("init large preimage", candidate); err != nil {
 		return fmt.Errorf("failed to populate pre-image oracle: %w", err)
 	}
 	return nil
@@ -191,6 +190,5 @@ func (p *LargePreimageUploader) addLargePreimageData(uuid *big.Int, chunks []kec
 		txs[i] = tx
 	}
 	p.log.Info("Adding large preimage leaves", "uuid", uuid, "blocksProcessed", blocksProcessed, "txs", len(txs))
-	_, err := p.txSender.SendAndWait("add leaf to large preimage", txs...)
-	return err
+	return p.txSender.SendAndWaitSimple("add leaf to large preimage", txs...)
 }

op-challenger/game/fault/preimages/types.go

@@ -21,6 +21,11 @@ type PreimageUploader interface {
 	UploadPreimage(ctx context.Context, claimIdx uint64, data *types.PreimageOracleData) error
 }
 
+type TxSender interface {
+	From() common.Address
+	SendAndWaitSimple(txPurpose string, txs ...txmgr.TxCandidate) error
+}
+
 // PreimageOracleContract is the interface for interacting with the PreimageOracle contract.
 type PreimageOracleContract interface {
 	InitLargePreimage(uuid *big.Int, partOffset uint32, claimedSize uint32) (txmgr.TxCandidate, error)

op-challenger/game/fault/register.go

@@ -49,7 +49,7 @@ func RegisterGameTypes(
 	registry Registry,
 	oracles OracleRegistry,
 	rollupClient RollupClient,
-	txSender types.TxSender,
+	txSender TxSender,
 	gameFactory *contracts.DisputeGameFactoryContract,
 	caller *batching.MultiCaller,
 	l1HeaderSource L1HeaderSource,
@@ -96,7 +96,7 @@ func registerAlphabet(
 	m metrics.Metricer,
 	syncValidator SyncValidator,
 	rollupClient RollupClient,
-	txSender types.TxSender,
+	txSender TxSender,
 	gameFactory *contracts.DisputeGameFactoryContract,
 	caller *batching.MultiCaller,
 	l1HeaderSource L1HeaderSource,
@@ -179,7 +179,7 @@ func registerCannon(
 	cfg *config.Config,
 	syncValidator SyncValidator,
 	rollupClient outputs.OutputRollupClient,
-	txSender types.TxSender,
+	txSender TxSender,
 	gameFactory *contracts.DisputeGameFactoryContract,
 	caller *batching.MultiCaller,
 	l2Client cannon.L2HeaderSource,

op-challenger/game/fault/responder/responder.go

@@ -30,17 +30,21 @@ type Oracle interface {
 	GlobalDataExists(ctx context.Context, data *types.PreimageOracleData) (bool, error)
 }
 
+type TxSender interface {
+	SendAndWaitSimple(txPurpose string, txs ...txmgr.TxCandidate) error
+}
+
 // FaultResponder implements the [Responder] interface to send onchain transactions.
 type FaultResponder struct {
 	log      log.Logger
-	sender   gameTypes.TxSender
+	sender   TxSender
 	contract GameContract
 	uploader preimages.PreimageUploader
 	oracle   Oracle
 }
 
 // NewFaultResponder returns a new [FaultResponder].
-func NewFaultResponder(logger log.Logger, sender gameTypes.TxSender, contract GameContract, uploader preimages.PreimageUploader, oracle Oracle) (*FaultResponder, error) {
+func NewFaultResponder(logger log.Logger, sender TxSender, contract GameContract, uploader preimages.PreimageUploader, oracle Oracle) (*FaultResponder, error) {
 	return &FaultResponder{
 		log:      logger,
 		sender:   sender,
@@ -63,7 +67,7 @@ func (r *FaultResponder) Resolve() error {
 		return err
 	}
 
-	return r.sendTxAndWait("resolve game", candidate)
+	return r.sender.SendAndWaitSimple("resolve game", candidate)
 }
 
 // CallResolveClaim determines if the resolveClaim function on the fault dispute game contract
@@ -78,7 +82,7 @@ func (r *FaultResponder) ResolveClaim(claimIdx uint64) error {
 	if err != nil {
 		return err
 	}
-	return r.sendTxAndWait("resolve claim", candidate)
+	return r.sender.SendAndWaitSimple("resolve claim", candidate)
 }
 
 func (r *FaultResponder) PerformAction(ctx context.Context, action types.Action) error {
@@ -126,12 +130,5 @@ func (r *FaultResponder) PerformAction(ctx context.Context, action types.Action)
 	if err != nil {
 		return err
 	}
-	return r.sendTxAndWait("perform action", candidate)
-}
-
-// sendTxAndWait sends a transaction through the [txmgr] and waits for a receipt.
-// This sets the tx GasLimit to 0, performing gas estimation online through the [txmgr].
-func (r *FaultResponder) sendTxAndWait(purpose string, candidate txmgr.TxCandidate) error {
-	_, err := r.sender.SendAndWait(purpose, candidate)
-	return err
+	return r.sender.SendAndWaitSimple("perform action", candidate)
 }

op-challenger/game/fault/responder/responder_test.go

@@ -12,7 +12,6 @@ import (
 	"github.com/ethereum-optimism/optimism/op-service/txmgr"
 
 	"github.com/ethereum/go-ethereum/common"
-	ethtypes "github.com/ethereum/go-ethereum/core/types"
 	"github.com/ethereum/go-ethereum/log"
 
 	"github.com/stretchr/testify/require"
@@ -325,21 +324,15 @@ type mockTxManager struct {
 	sendFails bool
 }
 
-func (m *mockTxManager) SendAndWait(_ string, txs ...txmgr.TxCandidate) ([]*ethtypes.Receipt, error) {
-	rcpts := make([]*ethtypes.Receipt, 0, len(txs))
+func (m *mockTxManager) SendAndWaitSimple(_ string, txs ...txmgr.TxCandidate) error {
 	for _, tx := range txs {
 		if m.sendFails {
-			return nil, mockSendError
+			return mockSendError
 		}
 		m.sends++
 		m.sent = append(m.sent, tx)
-		rcpts = append(rcpts, ethtypes.NewReceipt(
-			[]byte{},
-			false,
-			0,
-		))
 	}
-	return rcpts, nil
+	return nil
 }
 
 func (m *mockTxManager) BlockNumber(_ context.Context) (uint64, error) {

op-challenger/game/keccak/challenger.go

@@ -10,7 +10,6 @@ import (
 	keccakTypes "github.com/ethereum-optimism/optimism/op-challenger/game/keccak/types"
 	"github.com/ethereum-optimism/optimism/op-service/txmgr"
 	"github.com/ethereum/go-ethereum/common"
-	"github.com/ethereum/go-ethereum/core/types"
 	"github.com/ethereum/go-ethereum/log"
 )
 
@@ -29,7 +28,7 @@ type Verifier interface {
 }
 
 type Sender interface {
-	SendAndWait(txPurpose string, txs ...txmgr.TxCandidate) ([]*types.Receipt, error)
+	SendAndWaitSimple(txPurpose string, txs ...txmgr.TxCandidate) error
 }
 
 type PreimageChallenger struct {
@@ -80,7 +79,7 @@ func (c *PreimageChallenger) Challenge(ctx context.Context, blockHash common.Has
 	wg.Wait()
 	c.log.Debug("Created preimage challenge transactions", "count", len(txs))
 	if len(txs) > 0 {
-		_, err := c.sender.SendAndWait("challenge preimages", txs...)
+		err := c.sender.SendAndWaitSimple("challenge preimages", txs...)
 		if err != nil {
 			c.metrics.RecordPreimageChallengeFailed()
 			return fmt.Errorf("failed to send challenge txs: %w", err)

op-challenger/game/keccak/challenger_test.go

@@ -11,7 +11,6 @@ import (
 	"github.com/ethereum-optimism/optimism/op-service/testlog"
 	"github.com/ethereum-optimism/optimism/op-service/txmgr"
 	"github.com/ethereum/go-ethereum/common"
-	"github.com/ethereum/go-ethereum/core/types"
 	"github.com/ethereum/go-ethereum/log"
 	"github.com/stretchr/testify/require"
 )
@@ -151,12 +150,12 @@ type stubSender struct {
 	sent [][]txmgr.TxCandidate
 }
 
-func (s *stubSender) SendAndWait(_ string, txs ...txmgr.TxCandidate) ([]*types.Receipt, error) {
+func (s *stubSender) SendAndWaitSimple(_ string, txs ...txmgr.TxCandidate) error {
 	if s.err != nil {
-		return nil, s.err
+		return s.err
 	}
 	s.sent = append(s.sent, txs)
-	return nil, nil
+	return nil
 }
 
 type stubChallengerOracle struct {

op-challenger/game/types/types.go

@@ -4,9 +4,7 @@ import (
 	"errors"
 	"fmt"
 
-	"github.com/ethereum-optimism/optimism/op-service/txmgr"
 	"github.com/ethereum/go-ethereum/common"
-	ethtypes "github.com/ethereum/go-ethereum/core/types"
 )
 
 var ErrInvalidPrestate = errors.New("absolute prestate does not match")
@@ -46,8 +44,3 @@ type GameMetadata struct {
 	Timestamp uint64
 	Proxy     common.Address
 }
-
-type TxSender interface {
-	From() common.Address
-	SendAndWait(txPurpose string, txs ...txmgr.TxCandidate) ([]*ethtypes.Receipt, error)
-}

op-challenger/sender/sender.go

@@ -3,6 +3,7 @@ package sender
 import (
 	"context"
 	"errors"
+	"fmt"
 
 	"github.com/ethereum-optimism/optimism/op-service/txmgr"
 	"github.com/ethereum/go-ethereum/common"
@@ -10,6 +11,8 @@ import (
 	"github.com/ethereum/go-ethereum/log"
 )
 
+var ErrTransactionReverted = errors.New("transaction published but reverted")
+
 type TxSender struct {
 	log log.Logger
 
@@ -30,27 +33,30 @@ func (s *TxSender) From() common.Address {
 	return s.txMgr.From()
 }
 
-func (s *TxSender) SendAndWait(txPurpose string, txs ...txmgr.TxCandidate) ([]*types.Receipt, error) {
+func (s *TxSender) SendAndWaitDetailed(txPurpose string, txs ...txmgr.TxCandidate) []error {
 	receiptsCh := make(chan txmgr.TxReceipt[int], len(txs))
 	for i, tx := range txs {
 		s.queue.Send(i, tx, receiptsCh)
 	}
-	receipts := make([]*types.Receipt, len(txs))
 	completed := 0
-	var errs []error
+	errs := make([]error, len(txs))
 	for completed < len(txs) {
 		rcpt := <-receiptsCh
-		receipts[rcpt.ID] = rcpt.Receipt
 		completed++
 		if rcpt.Err != nil {
-			errs = append(errs, rcpt.Err)
+			errs[rcpt.ID] = rcpt.Err
 		} else if rcpt.Receipt != nil {
 			if rcpt.Receipt.Status != types.ReceiptStatusSuccessful {
-				s.log.Error("Transaction published but reverted", "tx_hash", rcpt.Receipt.TxHash, "purpose", txPurpose)
+				errs[rcpt.ID] = fmt.Errorf("%w purpose: %v hash: %v", ErrTransactionReverted, txPurpose, rcpt.Receipt.TxHash)
 			} else {
 				s.log.Debug("Transaction successfully published", "tx_hash", rcpt.Receipt.TxHash, "purpose", txPurpose)
 			}
 		}
 	}
-	return receipts, errors.Join(errs...)
+	return errs
+}
+
+func (s *TxSender) SendAndWaitSimple(txPurpose string, txs ...txmgr.TxCandidate) error {
+	errs := s.SendAndWaitDetailed(txPurpose, txs...)
+	return errors.Join(errs...)
 }

op-challenger/sender/sender_test.go

@@ -16,7 +16,7 @@ import (
 	"golang.org/x/exp/maps"
 )
 
-func TestSendAndWait(t *testing.T) {
+func TestSendAndWaitQueueWithMaxPending(t *testing.T) {
 	ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
 	defer cancel()
 	txMgr := &stubTxMgr{sending: make(map[byte]chan *types.Receipt)}
@@ -26,18 +26,18 @@ func TestSendAndWait(t *testing.T) {
 		return txmgr.TxCandidate{TxData: []byte{i}}
 	}
 
-	sendAsync := func(txs ...txmgr.TxCandidate) chan []*types.Receipt {
-		ch := make(chan []*types.Receipt, 1)
+	sendAsync := func(txs ...txmgr.TxCandidate) chan []txmgr.TxCandidate {
+		ch := make(chan []txmgr.TxCandidate, 1)
 		go func() {
-			rcpts, err := sender.SendAndWait("testing", txs...)
+			err := sender.SendAndWaitSimple("testing", txs...)
 			require.NoError(t, err)
-			ch <- rcpts
+			ch <- txs
 			close(ch)
 		}()
 		return ch
 	}
 
-	wait := func(ch chan []*types.Receipt) []*types.Receipt {
+	wait := func(ch chan []txmgr.TxCandidate) []txmgr.TxCandidate {
 		select {
 		case rcpts := <-ch:
 			return rcpts
@@ -88,9 +88,34 @@ func TestSendAndWait(t *testing.T) {
 	require.Len(t, wait(batch2), 2, "Batch2 should complete")
 }
 
+func TestSendAndWaitReturnIndividualErrors(t *testing.T) {
+	ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
+	defer cancel()
+	txMgr := &stubTxMgr{
+		sending: make(map[byte]chan *types.Receipt),
+		syncStatus: map[byte]uint64{
+			0: types.ReceiptStatusSuccessful,
+			1: types.ReceiptStatusFailed,
+			2: types.ReceiptStatusSuccessful,
+		},
+	}
+	sender := NewTxSender(ctx, testlog.Logger(t, log.LevelInfo), txMgr, 500)
+
+	tx := func(i byte) txmgr.TxCandidate {
+		return txmgr.TxCandidate{TxData: []byte{i}}
+	}
+
+	errs := sender.SendAndWaitDetailed("testing", tx(0), tx(1), tx(2))
+	require.Len(t, errs, 3)
+	require.NoError(t, errs[0])
+	require.ErrorIs(t, errs[1], ErrTransactionReverted)
+	require.NoError(t, errs[2])
+}
+
 type stubTxMgr struct {
-	m       sync.Mutex
-	sending map[byte]chan *types.Receipt
+	m          sync.Mutex
+	sending    map[byte]chan *types.Receipt
+	syncStatus map[byte]uint64
 }
 
 func (s *stubTxMgr) IsClosed() bool {
@@ -111,7 +136,11 @@ func (s *stubTxMgr) recordTx(candidate txmgr.TxCandidate) chan *types.Receipt {
 		panic("Sending duplicate transaction")
 	}
 	ch := make(chan *types.Receipt, 1)
-	s.sending[id] = ch
+	if status, ok := s.syncStatus[id]; ok {
+		ch <- &types.Receipt{Status: status}
+	} else {
+		s.sending[id] = ch
+	}
 	return ch
 }