Ensure to test all TX types including unprotected tx

op-node/rollup/derive/batch_test.go

@@ -19,7 +19,7 @@ import (
 )
 
 func RandomRawSpanBatch(rng *rand.Rand, chainId *big.Int) *RawSpanBatch {
-	blockCount := uint64(1 + rng.Int()&0xFF)
+	blockCount := uint64(4 + rng.Int()&0xFF) // at least 4
 	originBits := new(big.Int)
 	for i := 0; i < int(blockCount); i++ {
 		bit := uint(0)
@@ -30,11 +30,8 @@ func RandomRawSpanBatch(rng *rand.Rand, chainId *big.Int) *RawSpanBatch {
 	}
 	var blockTxCounts []uint64
 	totalblockTxCounts := uint64(0)
-
-	blockTxCounts = append(blockTxCounts, 4)
-	totalblockTxCounts += 4
-	for i := 1; i < int(blockCount); i++ {
-		blockTxCount := uint64(rng.Intn(16))
+	for i := 0; i < int(blockCount); i++ {
+		blockTxCount := 1 + uint64(rng.Intn(16))
 		blockTxCounts = append(blockTxCounts, blockTxCount)
 		totalblockTxCounts += blockTxCount
 	}
@@ -42,11 +39,15 @@ func RandomRawSpanBatch(rng *rand.Rand, chainId *big.Int) *RawSpanBatch {
 	var txs [][]byte
 	for i := 0; i < int(totalblockTxCounts); i++ {
 		var tx *types.Transaction
-		// ensure unprotected txs are included
-		if i < 4 || testutils.RandomBool(rng) {
-			tx = testutils.RandomLegacyTxNotProtected(rng)
-		} else {
-			tx = testutils.RandomTx(rng, new(big.Int).SetUint64(rng.Uint64()), londonSigner)
+		switch i % 4 {
+		case 0:
+			tx = testutils.RandomLegacyTx(rng, types.HomesteadSigner{})
+		case 1:
+			tx = testutils.RandomLegacyTx(rng, londonSigner)
+		case 2:
+			tx = testutils.RandomAccessListTx(rng, londonSigner)
+		case 3:
+			tx = testutils.RandomDynamicFeeTx(rng, londonSigner)
 		}
 		rawTx, err := tx.MarshalBinary()
 		if err != nil {

op-node/rollup/derive/span_batch_test.go

@@ -447,9 +447,10 @@ func TestSpanBatchToSingularBatch(t *testing.T) {
 
 func TestSpanBatchReadTxData(t *testing.T) {
 	cases := []spanBatchTxTest{
-		{"legacy tx", 32, testutils.RandomLegacyTx},
-		{"access list tx", 32, testutils.RandomAccessListTx},
-		{"dynamic fee tx", 32, testutils.RandomDynamicFeeTx},
+		{"unprotected legacy tx", 32, testutils.RandomLegacyTx, false},
+		{"legacy tx", 32, testutils.RandomLegacyTx, true},
+		{"access list tx", 32, testutils.RandomAccessListTx, true},
+		{"dynamic fee tx", 32, testutils.RandomDynamicFeeTx, true},
 	}
 
 	for i, testCase := range cases {
@@ -457,6 +458,9 @@ func TestSpanBatchReadTxData(t *testing.T) {
 			rng := rand.New(rand.NewSource(int64(0x109550 + i)))
 			chainID := new(big.Int).SetUint64(rng.Uint64())
 			signer := types.NewLondonSigner(chainID)
+			if !testCase.protected {
+				signer = types.HomesteadSigner{}
+			}
 
 			var rawTxs [][]byte
 			var txs []*types.Transaction

op-node/rollup/derive/span_batch_tx_test.go

@@ -12,16 +12,18 @@ import (
 )
 
 type spanBatchTxTest struct {
-	name   string
-	trials int
-	mkTx   func(rng *rand.Rand, signer types.Signer) *types.Transaction
+	name      string
+	trials    int
+	mkTx      func(rng *rand.Rand, signer types.Signer) *types.Transaction
+	protected bool
 }
 
 func TestSpanBatchTxConvert(t *testing.T) {
 	cases := []spanBatchTxTest{
-		{"legacy tx", 32, testutils.RandomLegacyTx},
-		{"access list tx", 32, testutils.RandomAccessListTx},
-		{"dynamic fee tx", 32, testutils.RandomDynamicFeeTx},
+		{"unprotected legacy tx", 32, testutils.RandomLegacyTx, false},
+		{"legacy tx", 32, testutils.RandomLegacyTx, true},
+		{"access list tx", 32, testutils.RandomAccessListTx, true},
+		{"dynamic fee tx", 32, testutils.RandomDynamicFeeTx, true},
 	}
 
 	for i, testCase := range cases {
@@ -29,6 +31,9 @@ func TestSpanBatchTxConvert(t *testing.T) {
 			rng := rand.New(rand.NewSource(int64(0x1331 + i)))
 			chainID := big.NewInt(rng.Int63n(1000))
 			signer := types.NewLondonSigner(chainID)
+			if !testCase.protected {
+				signer = types.HomesteadSigner{}
+			}
 
 			for txIdx := 0; txIdx < testCase.trials; txIdx++ {
 				tx := testCase.mkTx(rng, signer)
@@ -54,9 +59,10 @@ func TestSpanBatchTxConvert(t *testing.T) {
 
 func TestSpanBatchTxRoundTrip(t *testing.T) {
 	cases := []spanBatchTxTest{
-		{"legacy tx", 32, testutils.RandomLegacyTx},
-		{"access list tx", 32, testutils.RandomAccessListTx},
-		{"dynamic fee tx", 32, testutils.RandomDynamicFeeTx},
+		{"unprotected legacy tx", 32, testutils.RandomLegacyTx, false},
+		{"legacy tx", 32, testutils.RandomLegacyTx, true},
+		{"access list tx", 32, testutils.RandomAccessListTx, true},
+		{"dynamic fee tx", 32, testutils.RandomDynamicFeeTx, true},
 	}
 
 	for i, testCase := range cases {
@@ -64,6 +70,9 @@ func TestSpanBatchTxRoundTrip(t *testing.T) {
 			rng := rand.New(rand.NewSource(int64(0x1332 + i)))
 			chainID := big.NewInt(rng.Int63n(1000))
 			signer := types.NewLondonSigner(chainID)
+			if !testCase.protected {
+				signer = types.HomesteadSigner{}
+			}
 
 			for txIdx := 0; txIdx < testCase.trials; txIdx++ {
 				tx := testCase.mkTx(rng, signer)

op-node/rollup/derive/span_batch_txs_test.go

@@ -14,6 +14,12 @@ import (
 	"github.com/ethereum-optimism/optimism/op-service/testutils"
 )
 
+type txTypeTest struct {
+	name   string
+	mkTx   func(rng *rand.Rand, signer types.Signer) *types.Transaction
+	signer types.Signer
+}
+
 func TestSpanBatchTxsContractCreationBits(t *testing.T) {
 	rng := rand.New(rand.NewSource(0x1234567))
 	chainID := big.NewInt(rng.Int63n(1000))
@@ -304,57 +310,61 @@ func TestSpanBatchTxsRecoverV(t *testing.T) {
 	londonSigner := types.NewLondonSigner(chainID)
 	totalblockTxCount := 20 + rng.Intn(100)
 
-	var spanBatchTxs spanBatchTxs
-	var txTypes []int
-	var txSigs []spanBatchSignature
-	var originalVs []uint64
-	yParityBits := new(big.Int)
-	protectedBits := new(big.Int)
-	totalLegacyTxCount := 0
-	for idx := 0; idx < totalblockTxCount; idx++ {
-		var tx *types.Transaction
-		// ensure unprotected txs are included
-		if idx < 4 || testutils.RandomBool(rng) {
-			tx = testutils.RandomLegacyTxNotProtected(rng)
-		} else {
-			tx = testutils.RandomTx(rng, new(big.Int).SetUint64(rng.Uint64()), londonSigner)
-		}
-		txType := tx.Type()
-		txTypes = append(txTypes, int(txType))
-		var txSig spanBatchSignature
-		v, r, s := tx.RawSignatureValues()
-		if txType == types.LegacyTxType {
-			protectedBit := uint(0)
-			if tx.Protected() {
-				protectedBit = uint(1)
-			}
-			protectedBits.SetBit(protectedBits, int(totalLegacyTxCount), protectedBit)
-			totalLegacyTxCount++
-		}
-		// Do not fill in txSig.V
-		txSig.r, _ = uint256.FromBig(r)
-		txSig.s, _ = uint256.FromBig(s)
-		txSigs = append(txSigs, txSig)
-		originalVs = append(originalVs, v.Uint64())
-		yParityBit, err := convertVToYParity(v.Uint64(), int(tx.Type()))
-		require.NoError(t, err)
-		yParityBits.SetBit(yParityBits, idx, yParityBit)
+	cases := []txTypeTest{
+		{"unprotected legacy tx", testutils.RandomLegacyTx, types.HomesteadSigner{}},
+		{"legacy tx", testutils.RandomLegacyTx, londonSigner},
+		{"access list tx", testutils.RandomAccessListTx, londonSigner},
+		{"dynamic fee tx", testutils.RandomDynamicFeeTx, londonSigner},
 	}
 
-	spanBatchTxs.yParityBits = yParityBits
-	spanBatchTxs.txSigs = txSigs
-	spanBatchTxs.txTypes = txTypes
-	spanBatchTxs.protectedBits = protectedBits
-	// recover txSig.v
-	err := spanBatchTxs.recoverV(chainID)
-	require.NoError(t, err)
+	for _, testCase := range cases {
+		t.Run(testCase.name, func(t *testing.T) {
+			var spanBatchTxs spanBatchTxs
+			var txTypes []int
+			var txSigs []spanBatchSignature
+			var originalVs []uint64
+			yParityBits := new(big.Int)
+			protectedBits := new(big.Int)
+			totalLegacyTxCount := 0
+			for idx := 0; idx < totalblockTxCount; idx++ {
+				tx := testCase.mkTx(rng, testCase.signer)
+				txType := tx.Type()
+				txTypes = append(txTypes, int(txType))
+				var txSig spanBatchSignature
+				v, r, s := tx.RawSignatureValues()
+				if txType == types.LegacyTxType {
+					protectedBit := uint(0)
+					if tx.Protected() {
+						protectedBit = uint(1)
+					}
+					protectedBits.SetBit(protectedBits, int(totalLegacyTxCount), protectedBit)
+					totalLegacyTxCount++
+				}
+				// Do not fill in txSig.V
+				txSig.r, _ = uint256.FromBig(r)
+				txSig.s, _ = uint256.FromBig(s)
+				txSigs = append(txSigs, txSig)
+				originalVs = append(originalVs, v.Uint64())
+				yParityBit, err := convertVToYParity(v.Uint64(), int(tx.Type()))
+				require.NoError(t, err)
+				yParityBits.SetBit(yParityBits, idx, yParityBit)
+			}
 
-	var recoveredVs []uint64
-	for _, txSig := range spanBatchTxs.txSigs {
-		recoveredVs = append(recoveredVs, txSig.v)
-	}
+			spanBatchTxs.yParityBits = yParityBits
+			spanBatchTxs.txSigs = txSigs
+			spanBatchTxs.txTypes = txTypes
+			spanBatchTxs.protectedBits = protectedBits
+			// recover txSig.v
+			err := spanBatchTxs.recoverV(chainID)
+			require.NoError(t, err)
 
-	require.Equal(t, originalVs, recoveredVs, "recovered v mismatch")
+			var recoveredVs []uint64
+			for _, txSig := range spanBatchTxs.txSigs {
+				recoveredVs = append(recoveredVs, txSig.v)
+			}
+			require.Equal(t, originalVs, recoveredVs, "recovered v mismatch")
+		})
+	}
 }
 
 func TestSpanBatchTxsRoundTrip(t *testing.T) {
@@ -388,24 +398,35 @@ func TestSpanBatchTxsRoundTrip(t *testing.T) {
 func TestSpanBatchTxsRoundTripFullTxs(t *testing.T) {
 	rng := rand.New(rand.NewSource(0x13377331))
 	chainID := big.NewInt(rng.Int63n(1000))
-	signer := types.NewLondonSigner(chainID)
-
-	for i := 0; i < 4; i++ {
-		totalblockTxCounts := uint64(1 + rng.Int()&0xFF)
-		var txs [][]byte
-		for i := 0; i < int(totalblockTxCounts); i++ {
-			tx := testutils.RandomTx(rng, new(big.Int).SetUint64(rng.Uint64()), signer)
-			rawTx, err := tx.MarshalBinary()
-			require.NoError(t, err)
-			txs = append(txs, rawTx)
-		}
-		sbt, err := newSpanBatchTxs(txs, chainID)
-		require.NoError(t, err)
+	londonSigner := types.NewLondonSigner(chainID)
 
-		txs2, err := sbt.fullTxs(chainID)
-		require.NoError(t, err)
+	cases := []txTypeTest{
+		{"unprotected legacy tx", testutils.RandomLegacyTx, types.HomesteadSigner{}},
+		{"legacy tx", testutils.RandomLegacyTx, londonSigner},
+		{"access list tx", testutils.RandomAccessListTx, londonSigner},
+		{"dynamic fee tx", testutils.RandomDynamicFeeTx, londonSigner},
+	}
 
-		require.Equal(t, txs, txs2)
+	for _, testCase := range cases {
+		t.Run(testCase.name, func(t *testing.T) {
+			for i := 0; i < 4; i++ {
+				totalblockTxCounts := uint64(1 + rng.Int()&0xFF)
+				var txs [][]byte
+				for i := 0; i < int(totalblockTxCounts); i++ {
+					tx := testCase.mkTx(rng, testCase.signer)
+					rawTx, err := tx.MarshalBinary()
+					require.NoError(t, err)
+					txs = append(txs, rawTx)
+				}
+				sbt, err := newSpanBatchTxs(txs, chainID)
+				require.NoError(t, err)
+
+				txs2, err := sbt.fullTxs(chainID)
+				require.NoError(t, err)
+
+				require.Equal(t, txs, txs2)
+			}
+		})
 	}
 }
 
@@ -427,24 +448,35 @@ func TestSpanBatchTxsRecoverVInvalidTxType(t *testing.T) {
 func TestSpanBatchTxsFullTxNotEnoughTxTos(t *testing.T) {
 	rng := rand.New(rand.NewSource(0x13572468))
 	chainID := big.NewInt(rng.Int63n(1000))
-	signer := types.NewLondonSigner(chainID)
+	londonSigner := types.NewLondonSigner(chainID)
 
-	totalblockTxCounts := uint64(1 + rng.Int()&0xFF)
-	var txs [][]byte
-	for i := 0; i < int(totalblockTxCounts); i++ {
-		tx := testutils.RandomTx(rng, new(big.Int).SetUint64(rng.Uint64()), signer)
-		rawTx, err := tx.MarshalBinary()
-		require.NoError(t, err)
-		txs = append(txs, rawTx)
+	cases := []txTypeTest{
+		{"unprotected legacy tx", testutils.RandomLegacyTx, types.HomesteadSigner{}},
+		{"legacy tx", testutils.RandomLegacyTx, londonSigner},
+		{"access list tx", testutils.RandomAccessListTx, londonSigner},
+		{"dynamic fee tx", testutils.RandomDynamicFeeTx, londonSigner},
 	}
-	sbt, err := newSpanBatchTxs(txs, chainID)
-	require.NoError(t, err)
 
-	// drop single to field
-	sbt.txTos = sbt.txTos[:len(sbt.txTos)-2]
+	for _, testCase := range cases {
+		t.Run(testCase.name, func(t *testing.T) {
+			totalblockTxCounts := uint64(1 + rng.Int()&0xFF)
+			var txs [][]byte
+			for i := 0; i < int(totalblockTxCounts); i++ {
+				tx := testCase.mkTx(rng, testCase.signer)
+				rawTx, err := tx.MarshalBinary()
+				require.NoError(t, err)
+				txs = append(txs, rawTx)
+			}
+			sbt, err := newSpanBatchTxs(txs, chainID)
+			require.NoError(t, err)
+
+			// drop single to field
+			sbt.txTos = sbt.txTos[:len(sbt.txTos)-2]
 
-	_, err = sbt.fullTxs(chainID)
-	require.EqualError(t, err, "tx to not enough")
+			_, err = sbt.fullTxs(chainID)
+			require.EqualError(t, err, "tx to not enough")
+		})
+	}
 }
 
 func TestSpanBatchTxsMaxContractCreationBitsLength(t *testing.T) {