From 0329e319fb643d6b7a47a7cfbfcb70d36a6bbc62 Mon Sep 17 00:00:00 2001 From: Roberto Bayardo Date: Tue, 10 Oct 2023 17:00:19 -0700 Subject: [PATCH] post-Canyon receipt-root deposit tx hashing fix --- core/rawdb/accessors_chain_test.go | 38 ++++---- core/state_processor.go | 10 ++- core/types/receipt.go | 41 +++++++-- core/types/receipt_test.go | 140 +++++++++++++++++++++++++---- internal/ethapi/api.go | 9 ++ 5 files changed, 200 insertions(+), 38 deletions(-) diff --git a/core/rawdb/accessors_chain_test.go b/core/rawdb/accessors_chain_test.go index 1226bbbb63..273785e718 100644 --- a/core/rawdb/accessors_chain_test.go +++ b/core/rawdb/accessors_chain_test.go @@ -705,23 +705,30 @@ func TestReadLogs(t *testing.T) { body := &types.Body{Transactions: types.Transactions{tx1, tx2}} - // Create the two receipts to manage afterwards + // Create the three receipts to manage afterwards depositNonce := uint64(math.MaxUint64) - receipt1 := &types.Receipt{ + depositReceipt := types.Receipt{ Status: types.ReceiptStatusFailed, CumulativeGasUsed: 1, Logs: []*types.Log{ {Address: common.BytesToAddress([]byte{0x11})}, {Address: common.BytesToAddress([]byte{0x01, 0x11})}, }, - TxHash: tx1.Hash(), - ContractAddress: common.BytesToAddress([]byte{0x01, 0x11, 0x11}), - GasUsed: 111111, - DepositNonce: &depositNonce, + TxHash: tx1.Hash(), + ContractAddress: common.BytesToAddress([]byte{0x01, 0x11, 0x11}), + GasUsed: 111111, + DepositNonce: &depositNonce, + DepositReceiptVersion: nil, } - receipt1.Bloom = types.CreateBloom(types.Receipts{receipt1}) + depositReceipt.Bloom = types.CreateBloom(types.Receipts{&depositReceipt}) - receipt2 := &types.Receipt{ + // versionedDepositReceipt is same as depositReceipt only it has the Canyon DepositReceiptVersion + versionedDepositReceipt := depositReceipt + receiptVersion := types.CanyonDepositReceiptVersion + versionedDepositReceipt.DepositReceiptVersion = &receiptVersion + versionedDepositReceipt.Bloom = types.CreateBloom(types.Receipts{&versionedDepositReceipt}) + + receipt := types.Receipt{ PostState: common.Hash{2}.Bytes(), CumulativeGasUsed: 2, Logs: []*types.Log{ @@ -732,8 +739,8 @@ func TestReadLogs(t *testing.T) { ContractAddress: common.BytesToAddress([]byte{0x02, 0x22, 0x22}), GasUsed: 222222, } - receipt2.Bloom = types.CreateBloom(types.Receipts{receipt2}) - receipts := []*types.Receipt{receipt1, receipt2} + receipt.Bloom = types.CreateBloom(types.Receipts{&receipt}) + receipts := []*types.Receipt{&receipt, &depositReceipt, &versionedDepositReceipt} hash := common.BytesToHash([]byte{0x03, 0x14}) // Check that no receipt entries are in a pristine database @@ -750,14 +757,13 @@ func TestReadLogs(t *testing.T) { if len(logs) == 0 { t.Fatalf("no logs returned") } - if have, want := len(logs), 2; have != want { + if have, want := len(logs), 3; have != want { t.Fatalf("unexpected number of logs returned, have %d want %d", have, want) } - if have, want := len(logs[0]), 2; have != want { - t.Fatalf("unexpected number of logs[0] returned, have %d want %d", have, want) - } - if have, want := len(logs[1]), 2; have != want { - t.Fatalf("unexpected number of logs[1] returned, have %d want %d", have, want) + for i := range logs { + if have, want := len(logs[i]), 2; have != want { + t.Fatalf("unexpected number of logs[%d] returned, have %d want %d", i, have, want) + } } for i, pr := range receipts { diff --git a/core/state_processor.go b/core/state_processor.go index 139cee7db8..372235e372 100644 --- a/core/state_processor.go +++ b/core/state_processor.go @@ -142,9 +142,15 @@ func applyTransaction(msg *Message, config *params.ChainConfig, gp *GasPool, sta receipt.GasUsed = result.UsedGas if msg.IsDepositTx && config.IsOptimismRegolith(evm.Context.Time) { - // The actual nonce for deposit transactions is only recorded from Regolith onwards. - // Before the Regolith fork the DepositNonce must remain nil + // The actual nonce for deposit transactions is only recorded from Regolith onwards and + // otherwise must be nil. receipt.DepositNonce = &nonce + // The DepositReceiptVersion for deposit transactions is only recorded from Canyon onwards + // and otherwise must be nil. + if config.IsOptimismCanyon(evm.Context.Time) { + receipt.DepositReceiptVersion = new(uint64) + *receipt.DepositReceiptVersion = types.CanyonDepositReceiptVersion + } } if tx.Type() == types.BlobTxType { receipt.BlobGasUsed = uint64(len(tx.BlobHashes()) * params.BlobTxBlobGasPerBlob) diff --git a/core/types/receipt.go b/core/types/receipt.go index 0f02824144..100ac98795 100644 --- a/core/types/receipt.go +++ b/core/types/receipt.go @@ -46,6 +46,9 @@ const ( // ReceiptStatusSuccessful is the status code of a transaction if execution succeeded. ReceiptStatusSuccessful = uint64(1) + + // The version number for post-canyon deposit receipts. + CanyonDepositReceiptVersion = uint64(1) ) // Receipt represents the results of a transaction. @@ -70,6 +73,10 @@ type Receipt struct { // DepositNonce was introduced in Regolith to store the actual nonce used by deposit transactions // The state transition process ensures this is only set for Regolith deposit transactions. DepositNonce *uint64 `json:"depositNonce,omitempty"` + // DepositReceiptVersion was introduced in Canyon to indicate an update to how receipt hashes + // should be computed when set. The state transition process ensures this is only set for + // post-Canyon deposit transactions. + DepositReceiptVersion *uint64 `json:"depositReceiptVersion,omitempty"` // Inclusion information: These fields provide information about the inclusion of the // transaction corresponding to this receipt. @@ -111,7 +118,7 @@ type receiptRLP struct { Logs []*Log } -type depositReceiptRlp struct { +type depositReceiptRLP struct { PostStateOrStatus []byte CumulativeGasUsed uint64 Bloom Bloom @@ -119,6 +126,10 @@ type depositReceiptRlp struct { // DepositNonce was introduced in Regolith to store the actual nonce used by deposit transactions. // Must be nil for any transactions prior to Regolith or that aren't deposit transactions. DepositNonce *uint64 `rlp:"optional"` + // Receipt hash post-Regolith but pre-Canyon inadvertently did not include the above + // DepositNonce. Post Canyon, receipts will have a non-empty DepositReceiptVersion indicating + // which post-Canyon receipt hash function to invoke. + DepositReceiptVersion *uint64 `rlp:"optional"` } // storedReceiptRLP is the storage encoding of a receipt. @@ -129,6 +140,10 @@ type storedReceiptRLP struct { // DepositNonce was introduced in Regolith to store the actual nonce used by deposit transactions. // Must be nil for any transactions prior to Regolith or that aren't deposit transactions. DepositNonce *uint64 `rlp:"optional"` + // Receipt hash post-Regolith but pre-Canyon inadvertently did not include the above + // DepositNonce. Post Canyon, receipts will have a non-empty DepositReceiptVersion indicating + // which post-Canyon receipt hash function to invoke. + DepositReceiptVersion *uint64 `rlp:"optional"` } // LegacyOptimismStoredReceiptRLP is the pre bedrock storage encoding of a @@ -234,7 +249,7 @@ func (r *Receipt) encodeTyped(data *receiptRLP, w *bytes.Buffer) error { w.WriteByte(r.Type) switch r.Type { case DepositTxType: - withNonce := depositReceiptRlp{data.PostStateOrStatus, data.CumulativeGasUsed, data.Bloom, data.Logs, r.DepositNonce} + withNonce := &depositReceiptRLP{data.PostStateOrStatus, data.CumulativeGasUsed, data.Bloom, data.Logs, r.DepositNonce, r.DepositReceiptVersion} return rlp.Encode(w, withNonce) default: return rlp.Encode(w, data) @@ -315,13 +330,14 @@ func (r *Receipt) decodeTyped(b []byte) error { r.Type = b[0] return r.setFromRLP(data) case DepositTxType: - var data depositReceiptRlp + var data depositReceiptRLP err := rlp.DecodeBytes(b[1:], &data) if err != nil { return err } r.Type = b[0] r.DepositNonce = data.DepositNonce + r.DepositReceiptVersion = data.DepositReceiptVersion return r.setFromRLP(receiptRLP{data.PostStateOrStatus, data.CumulativeGasUsed, data.Bloom, data.Logs}) default: return ErrTxTypeNotSupported @@ -388,6 +404,9 @@ func (r *ReceiptForStorage) EncodeRLP(_w io.Writer) error { w.ListEnd(logList) if r.DepositNonce != nil { w.WriteUint64(*r.DepositNonce) + if r.DepositReceiptVersion != nil { + w.WriteUint64(*r.DepositReceiptVersion) + } } w.ListEnd(outerList) return w.Flush() @@ -451,6 +470,7 @@ func decodeStoredReceiptRLP(r *ReceiptForStorage, blob []byte) error { r.Bloom = CreateBloom(Receipts{(*Receipt)(r)}) if stored.DepositNonce != nil { r.DepositNonce = stored.DepositNonce + r.DepositReceiptVersion = stored.DepositReceiptVersion } return nil } @@ -461,7 +481,10 @@ type Receipts []*Receipt // Len returns the number of receipts in this list. func (rs Receipts) Len() int { return len(rs) } -// EncodeIndex encodes the i'th receipt to w. +// EncodeIndex encodes the i'th receipt to w. For DepositTxType receipts with non-nil DepositNonce +// but nil DepositReceiptVersion, the output will differ than calling r.MarshalBinary(); this +// behavior difference should not be changed to preserve backwards compatibility of receipt-root +// hash computation. func (rs Receipts) EncodeIndex(i int, w *bytes.Buffer) { r := rs[i] data := &receiptRLP{r.statusEncoding(), r.CumulativeGasUsed, r.Bloom, r.Logs} @@ -471,8 +494,16 @@ func (rs Receipts) EncodeIndex(i int, w *bytes.Buffer) { } w.WriteByte(r.Type) switch r.Type { - case AccessListTxType, DynamicFeeTxType, BlobTxType, DepositTxType: + case AccessListTxType, DynamicFeeTxType, BlobTxType: rlp.Encode(w, data) + case DepositTxType: + if r.DepositReceiptVersion != nil { + // post-canyon receipt hash computation update + depositData := &depositReceiptRLP{data.PostStateOrStatus, data.CumulativeGasUsed, r.Bloom, r.Logs, r.DepositNonce, r.DepositReceiptVersion} + rlp.Encode(w, depositData) + } else { + rlp.Encode(w, data) + } default: // For unsupported types, write nothing. Since this is for // DeriveSha, the error will be caught matching the derived hash diff --git a/core/types/receipt_test.go b/core/types/receipt_test.go index e6c76a733d..f413ffbc11 100644 --- a/core/types/receipt_test.go +++ b/core/types/receipt_test.go @@ -103,9 +103,30 @@ var ( } nonce = uint64(1234) depositReceiptWithNonce = &Receipt{ - Status: ReceiptStatusFailed, - CumulativeGasUsed: 1, - DepositNonce: &nonce, + Status: ReceiptStatusFailed, + CumulativeGasUsed: 1, + DepositNonce: &nonce, + DepositReceiptVersion: nil, + Logs: []*Log{ + { + Address: common.BytesToAddress([]byte{0x11}), + Topics: []common.Hash{common.HexToHash("dead"), common.HexToHash("beef")}, + Data: []byte{0x01, 0x00, 0xff}, + }, + { + Address: common.BytesToAddress([]byte{0x01, 0x11}), + Topics: []common.Hash{common.HexToHash("dead"), common.HexToHash("beef")}, + Data: []byte{0x01, 0x00, 0xff}, + }, + }, + Type: DepositTxType, + } + version = CanyonDepositReceiptVersion + depositReceiptWithNonceAndVersion = &Receipt{ + Status: ReceiptStatusFailed, + CumulativeGasUsed: 1, + DepositNonce: &nonce, + DepositReceiptVersion: &version, Logs: []*Log{ { Address: common.BytesToAddress([]byte{0x11}), @@ -192,11 +213,18 @@ var ( Value: big.NewInt(6), Gas: 50, }), + NewTx(&DepositTx{ + To: nil, // contract creation + Value: big.NewInt(6), + Gas: 60, + }), } - depNonce = uint64(7) - blockNumber = big.NewInt(1) - blockTime = uint64(2) - blockHash = common.BytesToHash([]byte{0x03, 0x14}) + depNonce1 = uint64(7) + depNonce2 = uint64(8) + blockNumber = big.NewInt(1) + blockTime = uint64(2) + blockHash = common.BytesToHash([]byte{0x03, 0x14}) + canyonDepositReceiptVersion = uint64(CanyonDepositReceiptVersion) // Create the corresponding receipts receipts = Receipts{ @@ -362,14 +390,51 @@ var ( Index: 5, }, }, - TxHash: txs[7].Hash(), - ContractAddress: common.HexToAddress("0x3bb898b4bbe24f68a4e9be46cfe72d1787fd74f4"), - GasUsed: 50, - EffectiveGasPrice: big.NewInt(0), - BlockHash: blockHash, - BlockNumber: blockNumber, - TransactionIndex: 7, - DepositNonce: &depNonce, + TxHash: txs[7].Hash(), + ContractAddress: common.HexToAddress("0x3bb898b4bbe24f68a4e9be46cfe72d1787fd74f4"), + GasUsed: 50, + EffectiveGasPrice: big.NewInt(0), + BlockHash: blockHash, + BlockNumber: blockNumber, + TransactionIndex: 7, + DepositNonce: &depNonce1, + DepositReceiptVersion: nil, + }, + &Receipt{ + Type: DepositTxType, + PostState: common.Hash{5}.Bytes(), + CumulativeGasUsed: 60 + 50 + 28, + Logs: []*Log{ + { + Address: common.BytesToAddress([]byte{0x33}), + Topics: []common.Hash{common.HexToHash("dead"), common.HexToHash("beef")}, + // derived fields: + BlockNumber: blockNumber.Uint64(), + TxHash: txs[8].Hash(), + TxIndex: 8, + BlockHash: blockHash, + Index: 6, + }, + { + Address: common.BytesToAddress([]byte{0x03, 0x33}), + Topics: []common.Hash{common.HexToHash("dead"), common.HexToHash("beef")}, + // derived fields: + BlockNumber: blockNumber.Uint64(), + TxHash: txs[8].Hash(), + TxIndex: 8, + BlockHash: blockHash, + Index: 7, + }, + }, + TxHash: txs[8].Hash(), + ContractAddress: common.HexToAddress("0x117814af22cb83d8ad6e8489e9477d28265bc105"), + GasUsed: 60, + EffectiveGasPrice: big.NewInt(0), + BlockHash: blockHash, + BlockNumber: blockNumber, + TransactionIndex: 8, + DepositNonce: &depNonce2, + DepositReceiptVersion: &canyonDepositReceiptVersion, }, } ) @@ -745,6 +810,46 @@ func TestBedrockDepositReceiptUnchanged(t *testing.T) { require.EqualValues(t, receipt.Logs, parsed.Logs) // And still shouldn't have a nonce require.Nil(t, parsed.DepositNonce) + // ..or a deposit nonce + require.Nil(t, parsed.DepositReceiptVersion) +} + +// Regolith introduced an inconsistency in behavior between EncodeIndex and MarshalBinary for a +// deposit transaction receipt. TestReceiptEncodeIndexBugIsEnshrined makes sure this difference is +// preserved for backwards compatibility purposes, but also that there is no discrepancy for the +// post-Canyon encoding. +func TestReceiptEncodeIndexBugIsEnshrined(t *testing.T) { + // Check that a post-Regolith, pre-Canyon receipt produces the expected difference between + // EncodeIndex and MarshalBinary. + buf := new(bytes.Buffer) + receipts := Receipts{depositReceiptWithNonce} + receipts.EncodeIndex(0, buf) + indexBytes := buf.Bytes() + + regularBytes, _ := receipts[0].MarshalBinary() + + require.NotEqual(t, indexBytes, regularBytes) + + // Confirm the buggy encoding is as expected, which means it should encode as if it had no + // nonce specified (like that of a non-deposit receipt, whose encoding would differ only in the + // type byte). + buf.Reset() + tempReceipt := *depositReceiptWithNonce + tempReceipt.Type = eip1559Receipt.Type + buggyBytes, _ := tempReceipt.MarshalBinary() + + require.Equal(t, indexBytes[1:], buggyBytes[1:]) + + // check that the post-Canyon encoding has no differences between EncodeIndex and + // MarshalBinary. + buf.Reset() + receipts = Receipts{depositReceiptWithNonceAndVersion} + receipts.EncodeIndex(0, buf) + indexBytes = buf.Bytes() + + regularBytes, _ = receipts[0].MarshalBinary() + + require.Equal(t, indexBytes, regularBytes) } func TestRoundTripReceipt(t *testing.T) { @@ -757,6 +862,7 @@ func TestRoundTripReceipt(t *testing.T) { {name: "EIP1559", rcpt: eip1559Receipt}, {name: "DepositNoNonce", rcpt: depositReceiptNoNonce}, {name: "DepositWithNonce", rcpt: depositReceiptWithNonce}, + {name: "DepositWithNonceAndVersion", rcpt: depositReceiptWithNonceAndVersion}, } for _, test := range tests { t.Run(test.name, func(t *testing.T) { @@ -767,6 +873,8 @@ func TestRoundTripReceipt(t *testing.T) { err = d.UnmarshalBinary(data) require.NoError(t, err) require.Equal(t, test.rcpt, d) + require.Equal(t, test.rcpt.DepositNonce, d.DepositNonce) + require.Equal(t, test.rcpt.DepositReceiptVersion, d.DepositReceiptVersion) }) t.Run(fmt.Sprintf("%sRejectExtraData", test.name), func(t *testing.T) { @@ -790,6 +898,7 @@ func TestRoundTripReceiptForStorage(t *testing.T) { {name: "EIP1559", rcpt: eip1559Receipt}, {name: "DepositNoNonce", rcpt: depositReceiptNoNonce}, {name: "DepositWithNonce", rcpt: depositReceiptWithNonce}, + {name: "DepositWithNonceAndVersion", rcpt: depositReceiptWithNonceAndVersion}, } for _, test := range tests { t.Run(test.name, func(t *testing.T) { @@ -804,6 +913,7 @@ func TestRoundTripReceiptForStorage(t *testing.T) { require.Equal(t, test.rcpt.CumulativeGasUsed, d.CumulativeGasUsed) require.Equal(t, test.rcpt.Logs, d.Logs) require.Equal(t, test.rcpt.DepositNonce, d.DepositNonce) + require.Equal(t, test.rcpt.DepositReceiptVersion, d.DepositReceiptVersion) }) } } diff --git a/internal/ethapi/api.go b/internal/ethapi/api.go index 229acc7f2b..ab3b2981c1 100644 --- a/internal/ethapi/api.go +++ b/internal/ethapi/api.go @@ -1567,6 +1567,8 @@ type RPCTransaction struct { SourceHash *common.Hash `json:"sourceHash,omitempty"` Mint *hexutil.Big `json:"mint,omitempty"` IsSystemTx *bool `json:"isSystemTx,omitempty"` + // deposit-tx post-Canyon only + DepositReceiptVersion *hexutil.Uint64 `json:"depositReceiptVersion,omitempty"` } // newRPCTransaction returns a transaction that will serialize to the RPC @@ -1607,6 +1609,10 @@ func newRPCTransaction(tx *types.Transaction, blockHash common.Hash, blockNumber result.Mint = (*hexutil.Big)(tx.Mint()) if receipt != nil && receipt.DepositNonce != nil { result.Nonce = hexutil.Uint64(*receipt.DepositNonce) + if receipt.DepositReceiptVersion != nil { + result.DepositReceiptVersion = new(hexutil.Uint64) + *result.DepositReceiptVersion = hexutil.Uint64(*receipt.DepositReceiptVersion) + } } case types.LegacyTxType: if v.Sign() == 0 && r.Sign() == 0 && s.Sign() == 0 { // pre-bedrock relayed tx does not have a signature @@ -2026,6 +2032,9 @@ func marshalReceipt(receipt *types.Receipt, blockHash common.Hash, blockNumber u } if chainConfig.Optimism != nil && tx.IsDepositTx() && receipt.DepositNonce != nil { fields["depositNonce"] = hexutil.Uint64(*receipt.DepositNonce) + if receipt.DepositReceiptVersion != nil { + fields["depositReceiptVersion"] = hexutil.Uint64(*receipt.DepositReceiptVersion) + } } // Assign receipt status or post state.