Merge pull request ethereum#99 from mdehoog/accesslist-fix

access lists & versioned hashes on blob txs should always be non-nil
marioevz · Feb 7, 2023 · 83bc851 · 83bc851
2 parents cbe4964 + 367f3d9
commit 83bc851
Show file tree

Hide file tree

Showing 3 changed files with 40 additions and 3 deletions.
diff --git a/core/types/data_blob_tx.go b/core/types/data_blob_tx.go
@@ -138,6 +138,8 @@ func (tdv *TxDataView) UnmarshalText(text []byte) error {
 	return conv.DynamicBytesUnmarshalText((*[]byte)(tdv), text[:])
 }
 
+// ReadHashes reads length hashes from dr and returns them through hashes, reusing existing capacity if possible. Hashes will always be
+// non-nil on return.
 func ReadHashes(dr *codec.DecodingReader, hashes *[]common.Hash, length uint64) error {
 	if uint64(len(*hashes)) != length {
 		// re-use space if available (for recycling old state objects)
@@ -146,6 +148,9 @@ func ReadHashes(dr *codec.DecodingReader, hashes *[]common.Hash, length uint64)
 		} else {
 			*hashes = make([]common.Hash, length)
 		}
+	} else if *hashes == nil {
+		// make sure the output is never nil
+		*hashes = []common.Hash{}
 	}
 	dst := *hashes
 	for i := uint64(0); i < length; i++ {
@@ -234,6 +239,7 @@ func (atv *AccessTupleView) FixedLength() uint64 {
 type AccessListView AccessList
 
 func (alv *AccessListView) Deserialize(dr *codec.DecodingReader) error {
+	*alv = AccessListView([]AccessTuple{})
 	return dr.List(func() codec.Deserializable {
 		i := len(*alv)
 		*alv = append(*alv, AccessTuple{})

diff --git a/core/types/transaction_marshalling.go b/core/types/transaction_marshalling.go
@@ -296,10 +296,11 @@ func (tx *Transaction) UnmarshalJSON(input []byte) error {
 	case BlobTxType:
 		var itx SignedBlobTx
 		inner = &itx
-		// Access list is optional for now.
-		if dec.AccessList != nil {
-			itx.Message.AccessList = AccessListView(*dec.AccessList)
+		// Access list should always be non-nil
+		if dec.AccessList == nil {
+			return errors.New("found nil access list in blob tx")
 		}
+		itx.Message.AccessList = AccessListView(*dec.AccessList)
 		if dec.ChainID == nil {
 			return errors.New("missing required field 'chainId' in transaction")
 		}

diff --git a/core/types/transaction_test.go b/core/types/transaction_test.go
@@ -524,6 +524,36 @@ func TestTransactionCoding(t *testing.T) {
 	}
 }
 
+// Make sure deserialized blob transactions never have nil access lists or versioned hash lists,
+// even when empty.
+func TestBlobTransactionEmptyLists(t *testing.T) {
+	txdata := &SignedBlobTx{
+		Message: BlobTxMessage{
+			ChainID:          view.Uint256View(*uint256.NewInt(1)),
+			Nonce:            view.Uint64View(1),
+			Gas:              view.Uint64View(123457),
+			GasTipCap:        view.Uint256View(*uint256.NewInt(42)),
+			GasFeeCap:        view.Uint256View(*uint256.NewInt(10)),
+			MaxFeePerDataGas: view.Uint256View(*uint256.NewInt(10000000)),
+		},
+	}
+	tx := NewTx(txdata)
+	data, err := tx.MarshalMinimal()
+	if err != nil {
+		t.Fatalf("ssz encoding failed: %v", err)
+	}
+	var parsedTx = &Transaction{}
+	if err := parsedTx.UnmarshalMinimal(data); err != nil {
+		t.Fatalf("ssz decoding failed: %v", err)
+	}
+	if parsedTx.AccessList() == nil {
+		t.Fatal("Deserialized blob txs should have non-nil access lists")
+	}
+	if parsedTx.DataHashes() == nil {
+		t.Fatal("Deserialized blob txs should have non-nil versioned hash lists")
+	}
+}
+
 func TestBlobTransactionMinimalCodec(t *testing.T) {
 	key, err := crypto.GenerateKey()
 	if err != nil {