fix: Use an outputproposal struct.

op-challenger/challenger/output.go

@@ -19,49 +19,65 @@ var (
 	ErrUnsupportedL2OOVersion = errors.New("unsupported l2oo version")
 	// ErrInvalidOutputLogTopic is returned when the output log topic is invalid.
 	ErrInvalidOutputLogTopic = errors.New("invalid output log topic")
+	// ErrInvalidOutputTopicLength is returned when the output log topic length is invalid.
+	ErrInvalidOutputTopicLength = errors.New("invalid output log topic length")
 )
 
+// OutputProposal is a proposal for an output root
+// in the L2OutputOracle for a given L2 block number.
+type OutputProposal struct {
+	L2BlockNumber *big.Int
+	OutputRoot    eth.Bytes32
+}
+
 // ParseOutputLog parses a log from the L2OutputOracle contract.
-func (c *Challenger) ParseOutputLog(log *types.Log) (*big.Int, eth.Bytes32, error) {
+func (c *Challenger) ParseOutputLog(log *types.Log) (*OutputProposal, error) {
+	// Check the length of log topics
+	if len(log.Topics) != 4 {
+		return nil, ErrInvalidOutputTopicLength
+	}
 	// Validate the first topic is the output log topic
 	if log.Topics[0] != c.l2ooABI.Events["OutputProposed"].ID {
-		return nil, eth.Bytes32{}, ErrInvalidOutputLogTopic
+		return nil, ErrInvalidOutputLogTopic
 	}
 	l2BlockNumber := new(big.Int).SetBytes(log.Topics[3][:])
 	expected := log.Topics[1]
-	return l2BlockNumber, eth.Bytes32(expected), nil
+	return &OutputProposal{
+		L2BlockNumber: l2BlockNumber,
+		OutputRoot:    eth.Bytes32(expected),
+	}, nil
 }
 
 // ValidateOutput checks that a given output is expected via a trusted rollup node rpc.
 // It returns: if the output is correct, the fetched output, error
-func (c *Challenger) ValidateOutput(ctx context.Context, l2BlockNumber *big.Int, expected eth.Bytes32) (bool, *eth.Bytes32, error) {
+func (c *Challenger) ValidateOutput(ctx context.Context, proposal OutputProposal) (bool, eth.Bytes32, error) {
 	// Fetch the output from the rollup node
 	ctx, cancel := context.WithTimeout(ctx, c.networkTimeout)
 	defer cancel()
-	output, err := c.rollupClient.OutputAtBlock(ctx, l2BlockNumber.Uint64())
+	output, err := c.rollupClient.OutputAtBlock(ctx, proposal.L2BlockNumber.Uint64())
 	if err != nil {
-		c.log.Error("Failed to fetch output", "blockNum", l2BlockNumber, "err", err)
-		return false, nil, err
+		c.log.Error("Failed to fetch output", "blockNum", proposal.L2BlockNumber, "err", err)
+		return false, eth.Bytes32{}, err
 	}
 
 	// Compare the output root to the expected output root
-	equalRoots, err := c.compareOutputRoots(output, expected, l2BlockNumber)
+	equalRoots, err := c.compareOutputRoots(output, proposal)
 	if err != nil {
-		return false, nil, err
+		return false, eth.Bytes32{}, err
 	}
 
-	return equalRoots, &output.OutputRoot, nil
+	return equalRoots, output.OutputRoot, nil
 }
 
 // compareOutputRoots compares the output root of the given block number to the expected output root.
-func (c *Challenger) compareOutputRoots(received *eth.OutputResponse, expected eth.Bytes32, blockNumber *big.Int) (bool, error) {
+func (c *Challenger) compareOutputRoots(received *eth.OutputResponse, expected OutputProposal) (bool, error) {
 	if received.Version != supportedL2OutputVersion {
 		c.log.Error("Unsupported l2 output version", "version", received.Version)
 		return false, ErrUnsupportedL2OOVersion
 	}
-	if received.BlockRef.Number != blockNumber.Uint64() {
-		c.log.Error("Invalid blockNumber", "expected", blockNumber, "actual", received.BlockRef.Number)
+	if received.BlockRef.Number != expected.L2BlockNumber.Uint64() {
+		c.log.Error("Invalid blockNumber", "expected", expected.L2BlockNumber, "actual", received.BlockRef.Number)
 		return false, ErrInvalidBlockNumber
 	}
-	return received.OutputRoot == expected, nil
+	return received.OutputRoot == expected.OutputRoot, nil
 }

op-challenger/challenger/output_test.go

@@ -28,29 +28,27 @@ func TestParseOutputLog_Succeeds(t *testing.T) {
 	log := types.Log{
 		Topics: []common.Hash{logTopic, common.Hash(expectedOutputRoot), {0x03}, common.BigToHash(expectedBlockNumber)},
 	}
-	parsedBlockNumber, parsedOutputRoot, err := challenger.ParseOutputLog(&log)
+	outputProposal, err := challenger.ParseOutputLog(&log)
 	require.NoError(t, err)
-	require.Equal(t, expectedBlockNumber, parsedBlockNumber)
-	require.Equal(t, expectedOutputRoot, parsedOutputRoot)
+	require.Equal(t, expectedBlockNumber, outputProposal.L2BlockNumber)
+	require.Equal(t, expectedOutputRoot, outputProposal.OutputRoot)
 }
 
 func TestParseOutputLog_WrongLogTopic_Errors(t *testing.T) {
 	challenger := newTestChallenger(t, eth.OutputResponse{}, true)
-	_, _, err := challenger.ParseOutputLog(&types.Log{
+	_, err := challenger.ParseOutputLog(&types.Log{
 		Topics: []common.Hash{{0x01}, {0x02}, {0x03}, {0x04}},
 	})
 	require.ErrorIs(t, err, ErrInvalidOutputLogTopic)
 }
 
-func TestParseOutputLog_BadLog_Panics(t *testing.T) {
+func TestParseOutputLog_WrongTopicLength_Errors(t *testing.T) {
 	challenger := newTestChallenger(t, eth.OutputResponse{}, true)
 	logTopic := challenger.l2ooABI.Events["OutputProposed"].ID
-	require.Panics(t, func() {
-		log := types.Log{
-			Topics: []common.Hash{logTopic, {0x02}, {0x03}},
-		}
-		_, _, _ = challenger.ParseOutputLog(&log)
+	_, err := challenger.ParseOutputLog(&types.Log{
+		Topics: []common.Hash{logTopic, {0x02}, {0x03}},
 	})
+	require.ErrorIs(t, err, ErrInvalidOutputTopicLength)
 }
 
 func TestChallenger_ValidateOutput_RollupClientErrors(t *testing.T) {
@@ -62,9 +60,13 @@ func TestChallenger_ValidateOutput_RollupClientErrors(t *testing.T) {
 
 	challenger := newTestChallenger(t, output, true)
 
-	valid, received, err := challenger.ValidateOutput(context.Background(), big.NewInt(0), output.OutputRoot)
+	checked := OutputProposal{
+		L2BlockNumber: big.NewInt(0),
+		OutputRoot:    output.OutputRoot,
+	}
+	valid, received, err := challenger.ValidateOutput(context.Background(), checked)
 	require.False(t, valid)
-	require.Nil(t, received)
+	require.Equal(t, eth.Bytes32{}, received)
 	require.ErrorIs(t, err, mockOutputApiError)
 }
 
@@ -77,9 +79,13 @@ func TestChallenger_ValidateOutput_ErrorsWithWrongVersion(t *testing.T) {
 
 	challenger := newTestChallenger(t, output, false)
 
-	valid, received, err := challenger.ValidateOutput(context.Background(), big.NewInt(0), eth.Bytes32{})
+	checked := OutputProposal{
+		L2BlockNumber: big.NewInt(0),
+		OutputRoot:    output.OutputRoot,
+	}
+	valid, received, err := challenger.ValidateOutput(context.Background(), checked)
 	require.False(t, valid)
-	require.Nil(t, received)
+	require.Equal(t, eth.Bytes32{}, received)
 	require.ErrorIs(t, err, ErrUnsupportedL2OOVersion)
 }
 
@@ -92,9 +98,13 @@ func TestChallenger_ValidateOutput_ErrorsInvalidBlockNumber(t *testing.T) {
 
 	challenger := newTestChallenger(t, output, false)
 
-	valid, received, err := challenger.ValidateOutput(context.Background(), big.NewInt(1), output.OutputRoot)
+	checked := OutputProposal{
+		L2BlockNumber: big.NewInt(1),
+		OutputRoot:    output.OutputRoot,
+	}
+	valid, received, err := challenger.ValidateOutput(context.Background(), checked)
 	require.False(t, valid)
-	require.Nil(t, received)
+	require.Equal(t, eth.Bytes32{}, received)
 	require.ErrorIs(t, err, ErrInvalidBlockNumber)
 }
 
@@ -107,8 +117,12 @@ func TestOutput_ValidateOutput(t *testing.T) {
 
 	challenger := newTestChallenger(t, output, false)
 
-	valid, expected, err := challenger.ValidateOutput(context.Background(), big.NewInt(0), output.OutputRoot)
-	require.Equal(t, *expected, output.OutputRoot)
+	checked := OutputProposal{
+		L2BlockNumber: big.NewInt(0),
+		OutputRoot:    output.OutputRoot,
+	}
+	valid, expected, err := challenger.ValidateOutput(context.Background(), checked)
+	require.Equal(t, expected, output.OutputRoot)
 	require.True(t, valid)
 	require.NoError(t, err)
 }
@@ -122,7 +136,11 @@ func TestChallenger_CompareOutputRoots_ErrorsWithDifferentRoots(t *testing.T) {
 
 	challenger := newTestChallenger(t, output, false)
 
-	valid, err := challenger.compareOutputRoots(&output, output.OutputRoot, big.NewInt(0))
+	checked := OutputProposal{
+		L2BlockNumber: big.NewInt(0),
+		OutputRoot:    output.OutputRoot,
+	}
+	valid, err := challenger.compareOutputRoots(&output, checked)
 	require.False(t, valid)
 	require.ErrorIs(t, err, ErrUnsupportedL2OOVersion)
 }
@@ -136,7 +154,11 @@ func TestChallenger_CompareOutputRoots_ErrInvalidBlockNumber(t *testing.T) {
 
 	challenger := newTestChallenger(t, output, false)
 
-	valid, err := challenger.compareOutputRoots(&output, output.OutputRoot, big.NewInt(1))
+	checked := OutputProposal{
+		L2BlockNumber: big.NewInt(1),
+		OutputRoot:    output.OutputRoot,
+	}
+	valid, err := challenger.compareOutputRoots(&output, checked)
 	require.False(t, valid)
 	require.ErrorIs(t, err, ErrInvalidBlockNumber)
 }
@@ -150,11 +172,19 @@ func TestChallenger_CompareOutputRoots_Succeeds(t *testing.T) {
 
 	challenger := newTestChallenger(t, output, false)
 
-	valid, err := challenger.compareOutputRoots(&output, output.OutputRoot, big.NewInt(0))
+	checked := OutputProposal{
+		L2BlockNumber: big.NewInt(0),
+		OutputRoot:    output.OutputRoot,
+	}
+	valid, err := challenger.compareOutputRoots(&output, checked)
 	require.True(t, valid)
 	require.NoError(t, err)
 
-	valid, err = challenger.compareOutputRoots(&output, eth.Bytes32{0x01}, big.NewInt(0))
+	checked = OutputProposal{
+		L2BlockNumber: big.NewInt(0),
+		OutputRoot:    eth.Bytes32{0x01},
+	}
+	valid, err = challenger.compareOutputRoots(&output, checked)
 	require.False(t, valid)
 	require.NoError(t, err)
 }