feat(l1-sender): In-flight tx detection & minor enhancements#1172
feat(l1-sender): In-flight tx detection & minor enhancements#1172Artemka374 wants to merge 14 commits intomainfrom
Conversation
Test results291 tests 291 ✅ 22m 14s ⏱️ Results for commit 886491c. ♻️ This comment has been updated with latest results. |
| // Probe whether the provider supports `eth_getTransactionBySenderAndNonce` before | ||
| // iterating over all pending nonces. | ||
| if let Err(TransportError::ErrorResp(ref e)) = provider | ||
| .raw_request::<_, Option<TxResponse>>( |
There was a problem hiding this comment.
There is existing get_transaction_by_sender_nonce that you can use instead
| // For each pending nonce, fetch the in-flight tx then peek at the next queued command. | ||
| // If the command's calldata matches what is on-chain, consume and pair it. On the first | ||
| // mismatch, stop — the unmatched command stays in `inbound` and will be re-sent by the | ||
| // normal send path (replacing the in-flight tx at that nonce). |
There was a problem hiding this comment.
Hmm, there seems to be a race condition between batcher that relies on last committed batch as discovered during startup and this component that might be doing recovery at a slightly different point in time.
We need a way to align them somehow. Either make this logic resistant to receiving older calldata that is no longer in-flight. Or moving this logic somewhere earlier (but even then I am not sure if we'd be able to avoid race condition).
| for nonce in latest_nonce..pending_nonce { | ||
| let tx = match provider | ||
| .raw_request::<_, Option<TxResponse>>( | ||
| "eth_getTransactionBySenderAndNonce".into(), |
There was a problem hiding this comment.
Same thing about get_transaction_by_sender_nonce here
| // recovered on startup and prepend them. Their nonces are lower than the ones we | ||
| // are about to assign, so they must be first in the ordering. On all subsequent | ||
| // iterations `recovered` is empty and this produces nothing. | ||
| let mut pending_txs: Vec<(TransactionReceiptFuture, Input, Instant)> = recovered |
There was a problem hiding this comment.
I think it's reasonable to wait for all in-flight transactions first before entering this loop. Current state of code is becoming very convoluted... We have pending_txs, new_txs, recovered, commands etc. And all of them are mutable and hard to reason about.
2 participants
Summary
In-flight transaction recovery on startup. Previously, if the L1 sender was restarted while transactions were still pending in the mempool, it would crash when they eventually got mined. The sender now detects and resumes tracking any such transactions, falling back to the normal send
path if recovery isn't possible (e.g. unsupported RPC method, dropped mempool transaction).
Gas limit tightened. The hardcoded gas limit was reduced from 15M (a ZKsync Era default) to 2M, which is a more realistic ceiling for the actual transactions sent by the L1 sender.
IMPORTANT NOTE
The recovery only works with Reth nodes v1.0.6, as other providers doesn't support
eth_getTransactionBySenderAndNoncewhich is able to check the mempoolFollow-ups