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.

Sign up for GitHub

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

Add clarifiication on uncle headers #158

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Add clarifiication on uncle headers #158

Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
f57c3d3
Revert changes
acolytec3 Jul 25, 2022
d5092ac
Add clarifying note
acolytec3 Jul 25, 2022
1bd0e83
Revise reference and note
acolytec3 Jul 26, 2022
65219fc
Update history-network.md
acolytec3 Aug 11, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion history-network.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,7 @@ encoded_uncles = rlp.encode(list_of_uncle_headers)
Note 1: The type-specific encoding might be different in future transaction types, but this encoding
works for all current transaction types.

Note 2: The `list_of_uncle_headers` refers to the array of uncle headers [defined in the devp2p spec](https://github.com/ethereum/devp2p/blob/master/caps/eth.md#block-encoding-and-validity).
Note 2: The `list_of_uncle_headers` refers to the list of uncle headers [defined in the Ethereum yellow paper](https://github.com/ethereum/yellowpaper/blob/30782852fef61f61a247dafbdad814ae7e00fb39/Paper.tex#L399) and is represented as an array of byte arrays where each byte array is the raw bytes representing an uncle header.
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think you rather need to say here that it is an RLP encoded list of block headers (from the uncle blocks) that results in a byte string/array.

E.g.:

Suggested change
Note 2: The `list_of_uncle_headers` refers to the list of uncle headers [defined in the Ethereum yellow paper](https://github.com/ethereum/yellowpaper/blob/30782852fef61f61a247dafbdad814ae7e00fb39/Paper.tex#L399) and is represented as an array of byte arrays where each byte array is the raw bytes representing an uncle header.
Note 2: The `list_of_uncle_headers` refers to the list of uncle headers [defined in the Ethereum yellow paper](https://github.com/ethereum/yellowpaper/blob/30782852fef61f61a247dafbdad814ae7e00fb39/Paper.tex#L399) and is represented RLP encoded list of block headers, where each block header is the header from an uncle block.

Also, I think a bit more relevant yellow paper link is this line: https://github.com/ethereum/yellowpaper/blob/30782852fef61f61a247dafbdad814ae7e00fb39/Paper.tex#L414 ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'd be hesitant to say it's an RLP encoded list since that's kind of what the larger content: rlp(list_of_uncle_headers) already says and I'm talking specifically about what's inside the parentheses. I wouldn't want implementers to think we mean our actual code should be rlp.encode(rlp.encode(list_of_uncle_headers)). Otherwise, I'm good with the above.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Right, forgot about that. Then I'd suggest dropping everything from the and onwards. I mostly wanted to point out that the byte array wording is not really correct.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I went with @carver's suggestion. Any last comments before we merge this?

acolytec3 marked this conversation as resolved.
Show resolved Hide resolved

#### Receipts

Expand Down