op-service/txmgr: multiple fixes / improvements #11614
Conversation
roberto-bayardo
changed the title
roberto-bayardo
changed the title
roberto-bayardo
force-pushed
the
resend-after-fee-bump
branch
19 times, most recently
from
a1eee31
to
95293ed
Compare
There was a problem hiding this comment.
Does this fix the behavior we observed, where the cancellation tx mode was active in the batcher and it tried to cancel a transaction for a nonce that had actually already been confirmed on L1? In such a situation, the cancellation should immediately be aborted.
roberto-bayardo
force-pushed
the
resend-after-fee-bump
branch
from
95293ed
to
1b9d9b3
Compare
Yes it fixes both the cases related to this. Note that in send_state I added a check that if nonce-too-low is returned by no tx has been successfully published, it will trigger an immediate abort. In the problems we saw though, the issue was that we did successfully publish a transaction with that nonce at first. However since another one with the same nonce was confirmed in parallel, it was later pushed from the mempool. In this case, we also need the re-trying, which will ultimately start returning "nonce too low" and once it hits the abort-count (3 errors) it will abort. |
roberto-bayardo
changed the title
roberto-bayardo
changed the title
roberto-bayardo
force-pushed
the
resend-after-fee-bump
branch
from
e296b1b
to
552166f
Compare
roberto-bayardo
force-pushed
the
resend-after-fee-bump
branch
4 times, most recently
from
6e8f288
to
ae50124
Compare
roberto-bayardo
force-pushed
the
resend-after-fee-bump
branch
from
ae50124
to
ae3b6fe
Compare
roberto-bayardo
force-pushed
the
resend-after-fee-bump
branch
from
ae3b6fe
to
ca25af4
Compare
|
Ready for review! |
roberto-bayardo
force-pushed
the
resend-after-fee-bump
branch
2 times, most recently
from
d5e4bd6
to
940d4af
Compare
… succeed otherwise) - make txmgr resubmit a transaction when fee bumping fails in case it has been dropped from the mempool - only bump fees when they really should be bumped - set txmgr overall default send timeout of 10 minutes. It was infinite, which led to permanently stuck transaction in combination with the other bugs fixed in this PR.
roberto-bayardo
force-pushed
the
resend-after-fee-bump
branch
from
940d4af
to
8f84353
Compare
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
9452aa6
Description
have transactions get resubmitted even if we fail to bump fees to prevent indefinite hanging when a tx gets dropped from the mempool (as experienced by both base-sepolia and op-sepolia on 8/27/2024).
do not bump fees in response to errors that do not require fee bumping like nonce-too-low.
force a critical error if we get nonce-too-low before successfully publishing a transaction so the nonce state will reset before trying to publish another transaction.
set default send timeout to 10 minutes instead of infinite
some control flow refactoring to (hopefully) make the code a bit more understandable.
Tests
fixed existing unit tests that were relying on fee bumping when fees shouldn't have been bumped, and added new test for the immediate-fail-on-nonce-too-low case.
add test to ensure tx gets resubmitted even if fees can't be bumped.
add tests to check that both the network and send timeouts are appropriately respected when sending a transaction