🥅 server: resolve orphaned proposals on nonce-too-low revert#765
🥅 server: resolve orphaned proposals on nonce-too-low revert#765cruzdanilo merged 1 commit intomainfrom
Conversation
🦋 Changeset detectedLatest commit: 0728413 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. WalkthroughAdds NonceTooLow handling in the block hook: keeper exaSend is called with an ignore for NonceTooLow, and when NonceTooLow occurs during proposal execution the proposal is removed from the Redis "proposals" sorted set to prevent orphaned retries. Changes
Sequence Diagram(s)sequenceDiagram
participant BlockHook as BlockHook (server/hooks/block.ts)
participant Keeper as Keeper/exaSend
participant Redis as Redis (proposals zset)
participant Sentry as Sentry/error capture
BlockHook->>Keeper: exaSend(tx, { ignore: ["NonceTooLow()"] })
alt Keeper returns NonceTooLow revert
Keeper-->>BlockHook: NonceTooLow error
BlockHook->>Redis: ZREM proposals <proposalId>
BlockHook->>Sentry: capture (non-fatal / optional)
else Keeper succeeds
Keeper-->>BlockHook: success
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
Summary of ChangesHello @cruzdanilo, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses a bug in the server where proposals would remain pending after a 'NonceTooLow' error, leading to orphaned proposals. The changes include adding specific error handling for 'NonceTooLow' errors and updating tests to ensure the correct behavior. This ensures that proposals are properly removed from the processing queue when such errors occur. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request aims to resolve orphaned proposals that occur due to NonceTooLow transaction reverts. The approach is to catch these specific errors and remove the corresponding proposal from the processing queue, which is a sound strategy. The implementation in server/hooks/block.ts correctly handles this for the main proposal execution flow. The identified comment regarding redundant error handling for nonce skipping is valid and has been retained, as it points out unreachable code that complicates error handling. The accompanying test updates in server/test/hooks/block.test.ts are well-written and correctly validate the intended new behavior.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #765 +/- ##
==========================================
- Coverage 67.71% 67.66% -0.06%
==========================================
Files 207 207
Lines 6945 7026 +81
Branches 2165 2207 +42
==========================================
+ Hits 4703 4754 +51
- Misses 2053 2071 +18
- Partials 189 201 +12
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
cruzdanilo
force-pushed
the
proposals
branch
from
e09750d to
bb473a6
Compare
![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
🤖 Fix all issues with AI agents
In `@server/hooks/block.ts`:
- Around line 359-367: The span is being marked as error unconditionally in the
catch block via parent.setStatus({ code: SPAN_STATUS_ERROR, ... }) before
handling the intentional NonceTooLow cleanup; move or adjust the status handling
so NonceTooLow is treated as a non-error: either relocate the parent.setStatus
call to after the NonceTooLow guard or, inside the NonceTooLow branch (where you
call redis.zrem("proposals", message)), set the span to a success/aborted status
(e.g., SPAN_STATUS_OK or similar with a message like "nonce_too_low"/"aborted")
instead of SPAN_STATUS_ERROR; update only the catch block around the error
instanceof BaseError && error.cause instanceof ContractFunctionRevertedError &&
error.cause.data?.errorName === "NonceTooLow" check so other errors still call
parent.setStatus({ code: SPAN_STATUS_ERROR, message: "proposal_failed" }).
bb473a6 to
b8d6f50
Compare
cruzdanilo
force-pushed
the
proposals
branch
from
b8d6f50 to
0728413
Compare
|
Sentry Issue: SERVER-HZ |
|
Sentry Issue: SERVER-HG |
1 participant
Summary by CodeRabbit