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

Move error data from error.data to error.message for failing rpc-compat calls #6332

Closed
Rjected opened this issue Feb 1, 2024 · 3 comments
Closed
Labels
A-rpc Related to the RPC implementation C-bug An unexpected or incorrect behavior C-test A change that impacts how or what we test M-prevent-stale Prevents old inactive issues/PRs from being closed due to inactivity

Comments

@Rjected
Copy link
Member

Rjected commented Feb 1, 2024

Describe the feature

ref ethereum/hive#975 (comment)

We should, where possible, prevent using the error.data field if the error message can be contained in the error.message field.

This includes, but is not limited to debug_getRawReceipts.

Analyzing the rpc-compat suite failures is the best way to understand every call that needs a change in error message:
https://github.com/paradigmxyz/reth/actions/runs/7734052427/job/21088043313

Additional context

No response

@Rjected Rjected added C-bug An unexpected or incorrect behavior A-rpc Related to the RPC implementation C-test A change that impacts how or what we test labels Feb 1, 2024
Copy link
Contributor

This issue is stale because it has been open for 21 days with no activity.

@github-actions github-actions bot added the S-stale This issue/PR is stale and will close with no further activity label Feb 23, 2024
@Rjected Rjected added M-prevent-stale Prevents old inactive issues/PRs from being closed due to inactivity and removed S-stale This issue/PR is stale and will close with no further activity labels Feb 23, 2024
@emhane
Copy link
Member

emhane commented Jun 25, 2024

so this originates from here you mean @Rjected ?

impl From<RpcInvalidTransactionError> for ErrorObject<'static> {
fn from(err: RpcInvalidTransactionError) -> Self {
match err {
RpcInvalidTransactionError::Revert(revert) => {
// include out data if some
rpc_err(
revert.error_code(),
revert.to_string(),
revert.output.as_ref().map(|out| out.as_ref()),
)
}
err => rpc_err(err.error_code(), err.to_string(), None),
}
}
}

and the solution is: control that the output field on RevertError is only used by spec error codes, not custom errors?

/// Represents a reverted transaction and its output data.
///
/// Displays "execution reverted(: reason)?" if the reason is a string.
#[derive(Debug, Clone)]
pub struct RevertError {
/// The transaction output data
///
/// Note: this is `None` if output was empty
output: Option<Bytes>,
}

links to where in specs the errors can be found?

@Rjected
Copy link
Member Author

Rjected commented Jun 26, 2024

links to where in specs the errors can be found?

there are no specs for rpc errors, the hive tests are considered the spec more or less

We basically cannot use the data field here:

pub(crate) fn rpc_err(
code: i32,
msg: impl Into<String>,
data: Option<&[u8]>,

@linear linear bot closed this as not planned Won't fix, can't repro, duplicate, stale Aug 23, 2024
@github-project-automation github-project-automation bot moved this from Todo to Done in Reth Tracker Aug 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-rpc Related to the RPC implementation C-bug An unexpected or incorrect behavior C-test A change that impacts how or what we test M-prevent-stale Prevents old inactive issues/PRs from being closed due to inactivity
Projects
Archived in project
Development

No branches or pull requests

2 participants