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

ethclient: fix HeaderByNumber for pending block #22517

Closed

Conversation

meowsbits
Copy link
Contributor

Given 'pending' value of -1, this method (Client.HeaderByNumber) used
to return an error because the result could not
be marshaled into a Header type.

This solution implements a special case for pending
headers -- which are only partially filled, missing
fields mixHash and nonce, notably -- by reading
the result first into a dynamic map and then
constructing a header field-by-field for expected/present values.

The test is refactored to allow more flexible
tests that can accommodate these expectations.

Given 'pending' value of -1, this method used
to return an error because the result could not
be marshaled into a Header type.

This solution implements a special case for pending
headers -- which are only partially filled, missing
fields mixHash and nonce ,notably -- by reading
the result first into a dynamic map and then
constructing a header field-by-field for expected
/present values.

The test is refactored to allow more flexible
tests that can accomodate these expectations.

Date: 2021-03-17 10:12:02-05:00
Signed-off-by: meows <[email protected]>
@fjl
Copy link
Contributor

fjl commented Mar 19, 2021

We have discussed this in the team and decided that it's not a good change.
HeaderByNumber is not supposed to work for the pending block. There is no
value in accessing the pending block header because it changes all the time
and does not reflect an actual block. It may be useful to access pending transactions
and logs, and this can be done with other methods, but the pending header is
really not useful.

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