Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Canyon Hard Fork] Receipt Root Deposit Nonce Hashing Bug Fix #99

Merged
merged 10 commits into from
Nov 7, 2023

Conversation

pcw109550
Copy link
Member

@pcw109550 pcw109550 commented Nov 3, 2023

All codes referenced from

Description

This PR fixes two issues.

Issue 1: Receipt Root Deposit Nonce Hashing Bug

When receipt is hashed and alters receipt root, method EncodeIndex is invoked.

func (rs Receipts) EncodeIndex(i int, w *bytes.Buffer) {

When receipt is generated from deposit tx, following case will be executed.

case DepositTxType:
w.WriteByte(DepositTxType)
if err := rlp.Encode(w, data); err != nil {
panic(err)
}

receiptRLP struct does not contain DepositNonce field, resulting in not including DepositNonce to receipt root. We must use depositReceiptRLP struct to correctly encode deposit receipts into receipt root. For backwards compatibility, we add DepositReceiptVersion field to receipts.

  • Canyon: DepositReceiptVersion is not nil. Encode using depositReceiptRLP
  • Pre-Canyon: DepositReceiptVersion is nil. Encode using receiptRLP.

Ran

codecgen -o receipt_codecgen_gen.go -r "^Receipts$|^Receipt$|^Logs$|^Log$" -st "codec" -j=false -nx=true -ta=true -oe=false -d 2 receipt.go log.go

to update cbor encoding/decoding code.

Issue 2: Receipt JSON Marshaling/Unmarshaling Bug

When receipt is JSON marshalled, it uses receiptMarshaling struct. Based on this, gencodec generates MarshalJSON() method. This custom method will be called when json.Marshal is called to receipt.

receiptMarshaling struct did not contain optimism related fields. Added them for correct json marshaling for optimism.

Ran

gencodec -type Receipt -field-override receiptMarshaling -out gen_receipt_json.go

to update autogenerated codes.

Testing

Unit tests added.

Misc

No erigon-lib update.

@pcw109550 pcw109550 changed the base branch from pcw109550/canyon/eip1559-denominator-adjust to tip/canyon November 3, 2023 19:25
@pcw109550 pcw109550 force-pushed the pcw109550/canyon/deposit-tx-receipt-hashing-fix branch from e0232e0 to 91362e8 Compare November 3, 2023 19:26
@pcw109550 pcw109550 mentioned this pull request Nov 3, 2023
6 tasks
@pcw109550 pcw109550 force-pushed the pcw109550/canyon/deposit-tx-receipt-hashing-fix branch from 4180904 to 699ca44 Compare November 6, 2023 06:57
@pcw109550 pcw109550 requested a review from ImTei November 6, 2023 07:43
Copy link
Member

@ImTei ImTei left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

left some comments about code style - not mandatory.

cmd/rpcdaemon/commands/eth_api.go Show resolved Hide resolved
core/state_processor.go Show resolved Hide resolved
core/types/receipt.go Show resolved Hide resolved
@pcw109550 pcw109550 merged commit a1b6e0b into tip/canyon Nov 7, 2023
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants