-
Notifications
You must be signed in to change notification settings - Fork 614
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(sender): nonce update #1570
Conversation
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 golangci-lint (1.62.2)level=warning msg="[runner] Can't run linter goanalysis_metalinter: buildir: failed to load package : could not load export data: no export data for "scroll-tech/rollup/mock_bridge"" WalkthroughThe changes in this pull request involve updating the version tag in the Changes
Possibly related PRs
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🔇 Additional comments (4)rollup/internal/controller/sender/sender.go (4)
The change from pointer-based nonce to direct value passing makes the nonce handling more explicit and thread-safe.
The nonce is now correctly updated only after successful transaction send, using the transaction's nonce value directly. This is a robust approach that prevents nonce gaps and ensures consistency.
The method signature change to accept a direct nonce value instead of a pointer aligns well with the improved nonce handling strategy. Line range hint The resubmission logic correctly adapts to the new createTx signature. Let's verify the entire resubmission flow to ensure nonce consistency. ✅ Verification successfulLet me gather more information about the nonce handling in the createTx function and its usage. Based on the comprehensive results, I can now provide the final response: Resubmission flow correctly handles nonces with proper safeguards The resubmission flow maintains nonce consistency through several mechanisms:
🏁 Scripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Description: Verify the resubmission flow handles nonces correctly
# Search for all places where transactions are resubmitted
rg -A 5 "resubmit transaction" --type go
# Search for nonce handling in resubmission logic
rg -A 10 "currentNonce.*originalTx.Nonce" --type go
Length of output: 3522 Script: #!/bin/bash
# Search for createTx function implementation and its nonce handling
ast-grep --pattern 'func (s *Sender) createTx($$$) ($$$) {
$$$
}'
# Search for nonce-related error handling in the sender
rg -B 2 -A 5 "nonce.*err" --type go
# Search for transaction status updates related to nonce
rg -A 5 "UpdateTxStatus.*nonce" --type go
Length of output: 22379 Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #1570 +/- ##
===========================================
- Coverage 52.06% 52.04% -0.03%
===========================================
Files 157 157
Lines 12472 12462 -10
===========================================
- Hits 6494 6486 -8
+ Misses 5418 5416 -2
Partials 560 560
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Purpose or design rationale of this PR
Only updating nonce when sending transaction success.
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
label to this PR?Breaking change label
Does this PR have the
breaking-change
label?Summary by CodeRabbit
New Features
Bug Fixes
Tests