fix(tracing): convert empty revertReason to none #353
Conversation
|
Copied the revert/error handling from geth's, PTAL. BTW, also changed the panic message in alloy-rs/core#1015 |
| } | ||
| } | ||
| } | ||
| GenericRevertReason::RawString(err) => err, |
There was a problem hiding this comment.
i think we only need to add the empty check here?
There was a problem hiding this comment.
I think raw string should also be omitted, changed into a simplify way
There was a problem hiding this comment.
why should this be omitted here?
can we only add the empty check, I dont see a reason to change this
There was a problem hiding this comment.
@mattsse, actually it was for another case, for this tx https://etherscan.io/tx/0x801eaaf69510d9a5fe9b8e8f2c9000f120d82b47ded984ee0fc875839684bc5e
reth will return as the raw string(not panic, or error), so abi decode will decode it as raw string, but geth only outputs for the panic or error:
// If that fails, try to decode as a regular string.
if let Ok(decoded_string) = core::str::from_utf8(out) {
return Some(decoded_string.to_string().into());
}output:
sorry for the mixin issue
There was a problem hiding this comment.
but why would we not include the raw string error?
| } | ||
| } | ||
| } | ||
| GenericRevertReason::RawString(err) => err, |
There was a problem hiding this comment.
why should this be omitted here?
can we only add the empty check, I dont see a reason to change this
|
hold on, waiting for alloy/core to be merged first, then we can use the as_geth_str |
| } | ||
| } | ||
| } | ||
| GenericRevertReason::RawString(err) => err, |
There was a problem hiding this comment.
but why would we not include the raw string error?
| Some(reason) | ||
| } | ||
| } | ||
| GenericRevertReason::RawString(err) => err, |
There was a problem hiding this comment.
actually, can we do this separately?
this doesnt make much sense to me to not do this
Geth will convert the empty bytes to null, and omit it in the response, I think we should align with it, the empty nulls seems meanless.
Below is the differ of the tx: https://etherscan.io/tx/0x337749b7c19859b2d773868b60e82a06ae008db9a80d41db76d7020d976a72e7