[submitter] fix metrics when out of gas #2817 #2818
Conversation
WalkthroughThe changes in Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant chainPendingQueue
participant calculatePendingTxes
User ->> chainPendingQueue: Call function
chainPendingQueue ->> calculatePendingTxes: Calculate pending txes
calculatePendingTxes -->> chainPendingQueue: Number of pending txes
chainPendingQueue ->> User: Updated metrics
Assessment against linked issues
Poem
Tip Early access features
Note:
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 as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR Summary
- Added
calculatePendingTxesfunction to accurately count pending transactions - Fixed metrics recording for pending transactions when out of gas
- Ensured metrics are updated even if transactions are not bumpable due to gas issues
- Adjusted logic to handle non-bumpable transactions correctly
1 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #2818 +/- ##
===================================================
- Coverage 25.67420% 25.55943% -0.11478%
===================================================
Files 708 765 +57
Lines 52025 54743 +2718
Branches 80 80
===================================================
+ Hits 13357 13992 +635
- Misses 37299 39315 +2016
- Partials 1369 1436 +67
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Deploying sanguine-fe with
|
| Latest commit: |
0d8b5ce
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://406407a6.sanguine-fe.pages.dev |
| Branch Preview URL: | https://fix-2817.sanguine-fe.pages.dev |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (1)
- ethergo/submitter/chain_queue.go (3 hunks)
Additional comments not posted (1)
ethergo/submitter/chain_queue.go (1)
67-70: LGTM!However, ensure that the new function
calculatePendingTxesis correctly implemented and used.Verification successful
The function
calculatePendingTxesis correctly implemented and used.
- The function iterates through the transactions and counts those with a nonce greater than or equal to the provided nonce.
- It returns the count of such transactions, which is used to set the number of pending transactions.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the correct implementation and usage of the function `calculatePendingTxes`. # Test: Search for the function usage. Expect: Only occurances of the new function signature. rg --type go $'calculatePendingTxes'Length of output: 359
Script:
#!/bin/bash # Extract the implementation of the function `calculatePendingTxes` from the file `ethergo/submitter/chain_queue.go`. ast-grep --lang go --pattern $'func calculatePendingTxes(txes []db.TX, nonce uint64) int { $$$ }' ethergo/submitter/chain_queue.goLength of output: 939
[goreleaser]
Description
Fixes #2817.
In #2817, what happened was we concluded here:
sanguine/ethergo/submitter/chain_queue.go
Lines 99 to 103 in 4448759
That the txes were not bumpable and as such we never recorded
len(cq.reprocessQueue)since the metric was an observable, we didn't get stale data either.I know why we use observables here, but it's possible we'll want to consider some future work for when metrics get full
Summary by CodeRabbit