-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Don't format if 'withdrawals' coming back from get_block is null #2941
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see... so this should no longer be an issue past geth v1.11.1
? I'm good with adding this in. No need to update any tests then either since we test for the empty list return value already.
web3/_utils/method_formatters.py
Outdated
@@ -297,7 +297,9 @@ def apply_list_to_array_formatter(formatter: Any) -> Callable[..., Any]: | |||
) | |||
), | |||
"transactionsRoot": apply_formatter_if(is_not_null, to_hexbytes(32)), | |||
"withdrawals": apply_formatter_to_array(withdrawal_result_formatter), | |||
"withdrawals": apply_formatter_if( | |||
is_not_null, apply_formatter_to_array(withdrawal_result_formatter) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I actually noticed this will return an empty tuple if it is empty... I think we need a bugfix to make this formatter use apply_list_to_array_formatter()
:
"withdrawals": apply_formatter_if(
is_not_null, apply_list_to_array_formatter(withdrawal_result_formatter),
),
This is what the transaction and uncle formatters use. This might be a relevant PR to sneak this into.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I actually noticed this will return an empty tuple if it is empty
Ah, interesting. I was seeing an empty list. Out of curiousity, what provider were you using?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh I do see that with geth, hmm. I was messing around with a rough draft of what eth-tester
would look like plugged into the latest py-evm
. I think I'd still rather have the list-like formatter for consistency (which I see was already added) but I think it's good to know it likely won't affect the user base too much.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, gotcha. Yeah, I think it makes sense to have it in there in case it comes back as a tuple.
2109dc6
to
36ac9cb
Compare
What was wrong?
Before 1.11.1, Geth was sending back the
withdrawal
field as nil (See: ethereum/go-ethereum#26707)Closes #2932
How was it fixed?
Added a is_not_null check to the
withdrawals
field on the block formatter results.Todo:
Cute Animal Picture