Skip to content
This repository has been archived by the owner on Oct 19, 2024. It is now read-only.

Fix handling of nonce too low error #643

Merged
merged 2 commits into from
Dec 3, 2021

Conversation

kekonen
Copy link
Contributor

@kekonen kekonen commented Dec 3, 2021

Motivation

There is a problem currently in EscalatingPending Future implementation, which is responsible for backing the send_escalating method in Middleware.
The problem is in the part of the Future which tests for the nonce too low issues. That test consists of checking if the error's Display contains string nonce too low. The point here is that the error's display might not have nonce too low. For instance, a custom error's Display might not provide the desired nonce too low, while Debug would. Here is an example from POC which shows Display (error_display) and Debug(error_debug) of an error

 Poll::Ready(Err(e)) => {
     // kludge. Prevents erroring on "nonce too low" which indicates
     // a previous escalation confirmed during this broadcast attempt
     tracing::info!(
         error_display = %e,
         error_debug = ?e,
         "Want to see the diffrernce for error"
     );
     if format!("{}", e).contains("nonce too low") {

Debug line produces:

{"timestamp":"Jan 01 01:01:01.1337","level":"INFO","fields":{"message":"Want to see the diffrernce for error","error_display":"Max requests reached","error_debug":"JsonRpcClientError(MaxRequests([JsonRpcError(JsonRpcError { code: -32000, message: \"nonce too low\", data: None }), JsonRpcError(JsonRpcError { code: -32000, message: \"nonce too low\", data....

Solution

Obviously, Debug includes wrapped parts, which expose the desired line, whereas Display shows only Max requests reached. In order to fix the test I propose to change the format from format!("{}", e) to format!("{:?}", e), which would expose the debug.

Copy link
Owner

@gakonst gakonst left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch!

@kekonen
Copy link
Contributor Author

kekonen commented Dec 3, 2021

@gakonst lints at WASM are failing, how shall we proceed?

@mattsse
Copy link
Collaborator

mattsse commented Dec 3, 2021

@gakonst lints at WASM are failing, how shall we proceed?

wasm has been fixed with #642, but there is a fmt issue here

Diff in /home/runner/work/ethers-rs/ethers-rs/ethers-providers/src/pending_escalator.rs at line 187:
                         let fut = this.provider.send_raw_transaction(next_to_broadcast);
                         *this.state = BroadcastingNew(fut);
                         cx.waker().wake_by_ref();
-                        return Poll::Pending;
+                        return Poll::Pending
                     }
                 }
                 check_all_receipts!(cx, this);

please use nightly formatting

@gakonst gakonst merged commit 9fc75ef into gakonst:master Dec 3, 2021
@kekonen kekonen deleted the daniil/fix_pending_escalator branch December 3, 2021 18:55
meetmangukiya pushed a commit to meetmangukiya/ethers-rs that referenced this pull request Mar 21, 2022
* feat: more elegant ether value parsing

* Update cli/src/opts/cast.rs

Co-authored-by: Georgios Konstantopoulos <[email protected]>

* cargo clippy

Co-authored-by: Georgios Konstantopoulos <[email protected]>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants