-
Notifications
You must be signed in to change notification settings - Fork 12
Handle ErrHeightOutOfRange errors in EstimateGas
#498
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
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughThe changes involve enhancements to the logging functionality in the Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant EVM
participant Logger
Client->>EVM: Call(data)
EVM->>Logger: Log hex-encoded data
Logger-->>EVM: Log entry created
Client->>EVM: EstimateGas(data)
EVM->>Logger: Check error type
alt Error not ErrHeightOutOfRange
EVM->>Logger: Log error details with hex data and heights
end
EVM->>Logger: Log success with evm-height and cadence-height
Logger-->>EVM: Log entry created
Poem
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 (
|
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.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- services/requester/requester.go (3 hunks)
Additional comments not posted (2)
services/requester/requester.go (2)
481-481: Approved: Enhanced logging inCallmethod.The change to log the
dataparameter in hexadecimal format enhances readability and is consistent with the PR's objectives.
Line range hint
531-554: Approved with suggestions: Enhanced error handling inEstimateGas.The introduction of specific error handling for
ErrHeightOutOfRangealigns with the PR's objectives. Ensure that this new logic correctly filters out the specified error while allowing other errors to be logged appropriately.Verification successful
Verified: Error handling for
ErrHeightOutOfRangeis correctly implemented and tested.The error handling logic for
ErrHeightOutOfRangeis consistently applied across the codebase, and corresponding tests verify its behavior. This confirms that the changes align with the intended objectives of the PR.
- The error handling logic is implemented in
services/requester/requester.go.- Tests in
services/requester/cross-spork_client_test.goverify the behavior.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the error handling logic in the `EstimateGas` method. # Test: Search for the error handling implementation. Expect: Only occurrences of the new logic. rg --type go -A 5 $'errs.ErrHeightOutOfRange'Length of output: 3930
Description
Just like we do for
Call/GetNonce/GetCode/GetBalance, we should handleErrHeightOutOfRangeinEstimateGasas well.For contributor use:
masterbranchFiles changedin the Github PR explorerSummary by CodeRabbit