fix: retry transaction sends across every RPC provider - #683
Merged
MoonBoi9001 merged 49 commits intoJul 30, 2026
Conversation
Broadcasting a transaction built its own one-shot connection to whichever endpoint the pool happened to point at, so the one call that spends money got none of the retry and failover every read call gets. One provider answering an error abandoned the send with a healthy one unused.
Some providers answer with HTTP 200 and put the overload in the JSON-RPC error object, each with its own code, so reading the HTTP status alone still treated those as permanent. Defer to alloy, which knows the provider codes and reports an execution error as not worth retrying.
The note explaining why the receipt poll waits 15 seconds cited a 300 second agreement deadline, while the configured default is 600, so anyone checking whether the retry budget fits inside the deadline started from a figure twice too small.
Callers read that text to recognise a stale nonce and resync from the chain, and it is the only record of why a submission was refused, but the pool replaced it with a count of providers tried. A rejection therefore stopped being recognisable once the send went through the pool.
One comment claimed the final error message was the only record of why every endpoint refused, which stopped being true once the rotation warning started carrying each one. The pool's other doc comments restated the function signatures, so they lose the padding.
Working out which endpoint a ring position lands on was written out three times, once in the rotation counter and twice inside the retry loop, so a change to one could silently disagree with the others. They now share a single small helper.
A submission rejected for a stale nonce is recognised by reading the error text, but that text only carried the last endpoint's reason, so a sick second endpoint hid the first one's rejection and the recovery never ran. Every reason is now in the message.
One variable carried the last failure across every endpoint, so reading it back always meant handling an "absent" case that could not happen and left the log line unsure whose failure it was printing.
The existing test used a single endpoint, which is the one arrangement where the rejection reason could not be lost. With two endpoints the reason has to survive the second one failing a different way, which is the case that stranded a submission in practice.
Reading the HTTP status and the error code was written so that either one answering "no" ended the question, which quietly dropped faults only the wording identifies, such as a throttling notice sent under a 403 or a timeout described in a 200 response.
Providers and the proxies in front of them sometimes report throttling with a status that otherwise means the request was malformed. Only the wording distinguishes the two, so this pins that a retry still follows.
The list of error codes that mean "throttled" is well known and already handled, but a gateway reporting a dropped connection inside an otherwise normal response has no such code. The wording is the only signal, so this pins that a retry still follows.
This test rebuilt the wording comparison inline and compared strings against the list, so it passed no matter what the real decision did. It now calls that decision, and the cases naming bare status numbers are gone since those are judged by status elsewhere.
The mock replied to every request with a transaction hash, so a future change that made the client look up the chain id or a gas price first would have been handed a hash where a number belongs and failed somewhere confusing. It now says what went wrong.
Offering identical bytes to a second endpoint is answered with "I already have this", which was read as a stale nonce and prompted a second transaction at a fresh nonce, paying twice for one agreement offer. Signing now happens once up front, so that answer carries a hash.
Rotating between endpoints is only safe while each is offered identical bytes, otherwise two transactions could both be mined and both be paid for. That property was argued in a comment and never checked, so this compares what each endpoint actually received.
A submission holds a lock that every other submission queues behind, and with the shipped settings retrying each of 2 endpoints 4 times could hold it for roughly 500 seconds against a 600 second acceptance window. Each endpoint now gets one attempt, bounding this near 120.
One test checked both how a struggling endpoint is treated and how a chain rejection is treated, so a failure did not say which of the two had broken and neither could be run on its own.
The test threw away what the submission returned and only counted requests, so it would have passed just as happily if the submission had failed outright rather than moving on to the spare endpoint as intended.
Each attempt built its own HTTP client, so every retry opened a new connection and repeated the TLS handshake, on exactly the path that is already short of time. The connection is now built once per endpoint and shared by that endpoint's attempts.
Building it per call meant every call carried an error path for "could not make an HTTP client", which has nothing to do with any endpoint being reachable, and gave each call its own connection pool so nothing was ever reused between them.
Routing submissions through the shared endpoint pool made them come back labelled as a generic RPC problem, which reads the same as a failed read and loses the fact that money was about to move. Anything the pool can name precisely keeps its own label.
Each call keeps its own place in the rotation rather than reading the shared one, so that concurrent calls cannot send it back to an endpoint it has already tried and leave it giving up before it reaches a working one. That reasoning had nothing checking it.
Every read the service makes shares the retry and failover machinery that submissions use, and it was reworked here without anything checking a read still behaves. This asks for a block number across one sick endpoint and one working one.
Transactions from one wallet are numbered and must be mined in order. Recovery from a rejected number aimed one past the next free slot, leaving a hole, and a transaction behind a hole waits indefinitely, so the wallet could sit stuck until something else filled it.
A broadcast reported whatever hash the endpoint echoed back, though the hash follows from the bytes we signed. A wrong hash sends the receipt poll after a transaction that never mines, so the offer is reported dropped and resubmitted while the first one is still live.
Hosted RPC endpoints carry their API key in the URL path or query, and the whole URL was written into the failure text and the log fields. Endpoints are now named by host and port, which still says which one refused without carrying the key along with it.
Signing a transaction no longer fills in missing fields, and a request that names no chain is signed for Ethereum mainnet rather than refused. Every submission passes through one place, so that place now checks the chain matches before a signature exists.
A send that no endpoint accepts is reported as a failed submission rather than as the pool's generic RPC fault, since that is what tells a caller the transaction went nowhere. The test only checked that something went wrong, so the distinction could have been lost unnoticed.
When no endpoint accepts a call, the failure gathers what each one said in the order they were tried, because a caller reads that text to tell a rejection from the chain apart from a broken connection. Nothing checked that reasons from earlier endpoints survived the later ones.
The provider that stranded an accepted agreement on 2026-07-29 answered `code 19 Temporary internal error. Please retry`. That was only recognised because it arrived with an HTTP 500; sent under a 200, the same complaint bought no retry, because nothing read the wording.
The note claimed later calls start from an endpoint that has not just failed. Only calls that begin afterwards get that, a call already walking the ring keeps the start it took, and a sweep that fails on every endpoint leaves the shared start back where it began.
The one-second backoff this test waits out looks like something tokio's clock control could skip. It cannot: with time paused, tokio advances the clock while the HTTP response is still arriving, the request timeout fires, and every endpoint then looks dead.
Half the tests in these two files were named after the function under test and half after the behaviour they pin, so a failure told you where to look but not what broke. They now all read as a sentence about the behaviour.
An endpoint is already named by host alone so its API key stays out of the logs, but the reason a call failed was passed through as the HTTP client wrote it, and that names the whole URL. Take the URL out of the reason too, leaving the host in its place.
The endpoint the existing check points at answers every call, and an endpoint that answers describes its own refusal without ever naming a URL. Point a second check at a port nothing listens on, which is what makes the HTTP client write the URL into the failure.
Only the last endpoint's error was inspected for a structured rejection, so a contract refusing the call on one endpoint was downgraded to a generic RPC fault whenever the next endpoint was merely unreachable. Callers then retried a refusal that will never clear.
One endpoint reports the contract refusing the call while the next cannot be reached at all, which is the shape that used to hide the refusal behind a generic fault.
A node reporting that it already holds the transaction is listed as a nonce problem, which reads as though refreshing the nonce and resending were the answer. The send path claims that wording first and reports the successful broadcast it is, so say so in both places.
No field is filled in for the caller on the send path, so a request missing one stops before the signature exists. Calling that a signing failure sends a reader after the wrong thing, when the wording underneath already names the field that was left out.
A submission got a single attempt per endpoint, on the reasoning that a different endpoint was likelier to help than pressing a struggling one. Against an endpoint that fails quickly that gave up in milliseconds, and it left anyone running one endpoint with no retry at all.
Submissions run one at a time, and how long one takes grew with the number of endpoints configured and the retries each is given, so a slow spell could park everything behind it. Cap a submission at 60 seconds, which frees the lock and lets the job run again later.
Whether a call starts on an endpoint that just failed depends on how many other calls failed alongside it, because the shared starting point counts failures. Say so where it happens, since the next reader will otherwise wonder whether it is a bug.
A transaction naming the wrong chain was refused as a configuration fault, and the path that cancels a stale agreement reads a configuration fault as the chain client being switched off: it would skip the cancel and mark the agreement abandoned while it was still live on-chain.
Every call the chain client makes is a small one, so 30 seconds was patience it never needed, and it multiplies: an endpoint that stops answering is asked 4 times before the next is tried. Waiting 10 thirds how long a stalled endpoint delays a submission or a receipt check.
Several comment blocks said in 4 to 8 lines what fits in 3, so trim them without losing the reasoning they carry. No code changes.
A fixed 60s cut-off could cancel a submission mid-failover once endpoint count, retries and timeouts multiplied past it. Derive the deadline from the configured schedule instead, capped to leave a worker job room for the receipt poll that follows the broadcast.
A submission that failed on every endpoint advanced the nonce counter anyway, leaving a slot the chain kept waiting on while every later transaction queued behind it until a restart. Advance the counter only after an endpoint accepts the transaction.
MoonBoi9001
marked this pull request as ready for review
July 30, 2026 14:15
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR makes dipper retry an on-chain submission against its other configured RPC providers instead of giving up after one endpoint fails. Dipper wraps every read call in retry, backoff and failover through a pool of provider URLs, but broadcasting a transaction built its own single-use connection to whichever endpoint the pool currently pointed at. So the one call that spends money and carries a deadline had neither, and the rotation counter never advanced, pinning it to the first provider for the life of the process.
On 2026-07-29 that stranded a real agreement. An indexer accepted a proposal off-chain, the primary provider answered
HTTP 500 code 19 Temporary internal errorto 3 consecutive submissions, the second provider was never contacted, and the agreement expired unfunded.Sending now goes through the pool. Rebroadcasting is safe because the nonce, gas limit and both fees are fixed beforehand, so every endpoint receives identical bytes under one hash. Retryability is read from the HTTP status where the transport reports one, and otherwise from the JSON-RPC error code, rather than by searching error text for digits.
Worth a reviewer's attention: a failing send now spends longer holding
submit_lock, since the retries happen inside it, which serialises concurrent submissions for longer.