-
Notifications
You must be signed in to change notification settings - Fork 987
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
ethereum: fix GETH_ETH_CALL_ERRORS_ENV #2784
Conversation
858b84c
to
c62b751
Compare
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.
Wow, I didn't know about this.
c62b751
to
29900cd
Compare
Previously if `GETH_ETH_CALL_ERRORS_ENV` is not set, then any error returned from eth_call would be considered deterministic. This is because calling `split` on an empty string returns `[""]`, not `[]`, and all strings contain `""`. This affects release 0.24 and can cause subgraph failures on subgraphs using contract calls, and most severely incorrect data on subgraphs using `try_` calls.
29900cd
to
ed28b2f
Compare
Wouldn't let foo: Vec<_> = String::new()
.split_terminator(';')
.map(ToOwned::to_owned)
.collect();
// Prints `[]`
println!("{:?}", foo);
let abc: Vec<_> = "A;B;C"
.split_terminator(';')
.map(ToOwned::to_owned)
.collect();
// Prints `["A", "B", "C"]`
println!("{:?}", abc); |
@vrmiguel that would work better, good to know. This is all very confusing, so I've settled for just explicitly filtering out empty strings. |
.split(';') | ||
.map(ToOwned::to_owned) | ||
.collect() | ||
.map(|s| s.split(';').filter(|s| s.len() > 0).map(ToOwned::to_owned).collect()) |
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.
Actually s.is_empty()
is a better and clearer option then s.len() > 0
.
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 always have a hard time reasoning over negated conditionals, so I prefer s.len() > 0
over !s.is_empty()
.
Previously if
GETH_ETH_CALL_ERRORS_ENV
is not set, then any error returned from eth_call would be considered deterministic. This is because callingsplit
on an empty string returns[""]
, not[]
(playground), and all strings contain""
. This affects release 0.24 and can cause subgraph failures on subgraphs using contract calls, and most severely incorrect data on subgraphs usingtry_
calls, which would return areverted
result when it fact it was a spurious error.This did not affect the hosted service because it has
GETH_ETH_CALL_ERRORS_ENV
set to"out of gas"
.