-
Notifications
You must be signed in to change notification settings - Fork 604
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
fix(rollup_relayer): retry CommitFailed batches #911
Conversation
Codecov Report
@@ Coverage Diff @@
## develop #911 +/- ##
========================================
Coverage 43.74% 43.74%
========================================
Files 72 72
Lines 6991 6991
========================================
Hits 3058 3058
Misses 3682 3682
Partials 251 251
Flags with carried forward coverage won't be shown. Click here to find out more.
|
@@ -332,7 +332,7 @@ func (r *Layer2Relayer) ProcessGasPriceOracle() { | |||
// ProcessPendingBatches processes the pending batches by sending commitBatch transactions to layer 1. | |||
func (r *Layer2Relayer) ProcessPendingBatches() { | |||
// get pending batches from database in ascending order by their index. | |||
pendingBatches, err := r.batchOrm.GetPendingBatches(r.ctx, 5) | |||
pendingBatches, err := r.batchOrm.GetPendingAndCommitFailedBatches(r.ctx, 5) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should not keep retrying forever. If it's a gas limit issue, then it should succeed the 2nd time. If it's a bug (e.g. Golang batch hash does not match Solidity batch hash), then retrying again and again will just burn ETH, in this case we need human intervention to fix the bug
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see.
Purpose or design rationale of this PR
In today's incident, caused by #897, our sender keeps sending
commitBatch
txs and didn't realize they failed (reverted when being executed, e.g., https://sepolia.etherscan.io/tx/0x4846ed1477974716d9b8c57b6aee4e3fc710c8250499fd0631bf360844f0e5ea & https://sepolia.etherscan.io/tx/0xcb75cd2f2b086e40b7aaa03a31a63a239ab7776246ea9b1ae69946bd41c15e2b)And the rollup_status in DB were updated to
RollupCommitFailed
. logs:However, our sender should not keep sending txs if the previous ones fail.
This PR adds the retry logic, and we will realize the commitment gets stuck earlier.
PR title
Your PR title must follow conventional commits (as we are doing squash merge for each PR), so it must start with one of the following types:
Deployment tag versioning
Has
tag
incommon/version.go
been updated or have you addedbump-version
tag to this PR?Breaking change label
Does this PR have the
breaking-change
label?