Skip to content
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 gas estimation #38

Open
wants to merge 20 commits into
base: master
Choose a base branch
from

Conversation

darrenvechain
Copy link
Owner

@darrenvechain darrenvechain commented Jun 21, 2024

Description

Please include a summary of the changes and the related issue. Please also include relevant motivation and context. List
any dependencies that are required for this change.

Fixes # (issue)

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also
list any relevant details for your test configuration

  • Test A
  • Test B

Test Configuration:

  • Go Version:
  • Hardware:
  • Docker Version:

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • New and existing E2E tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules
  • I have not added any vulnerable dependencies to my code

PR-Codex overview

This PR updates the VeChain Thor codebase to version 2.1.3.

Detailed summary

  • Updated version to 2.1.3 in cmd/thor/VERSION and api/doc/thor.yaml
  • Revised utils.ParseRevision in api/blocks/blocks.go and api/debug/debug.go
  • Added new test functions and revised existing ones in multiple test files

The following files were skipped due to too many changes: api/utils/revisions_test.go, api/utils/revisions.go, api/accounts/accounts.go

✨ Ask PR-Codex anything about this PR by commenting with /codex {your question}

Summary by CodeRabbit

  • New Features

    • Introduced support for specifying block revisions in API endpoints.
    • Added a new test function to enhance trace call testing.
  • Bug Fixes

    • Improved parameter handling in several API methods to ensure accurate state and header retrieval.
  • Documentation

    • Updated API documentation with new parameter references and version number.
  • Chores

    • Updated GitHub Actions workflows to use the latest versions for setup and linting tools.
    • Bumped the project version to 2.1.3.

Copy link

coderabbitai bot commented Jun 21, 2024

Walkthrough

The updates span modifications to handle arguments and method signatures, especially focusing on revisions and state handling in APIs for accounts, blocks, and debugging. Enhancements include adding new parameters, changing method signatures, updating GitHub Actions, and incrementing version numbers. Additionally, test functions have been refactored and new tests added to ensure robustness.

Changes

Files Change Summary
api/accounts/accounts.go, api/blocks/blocks.go, api/debug/debug.go Updated method signatures to include new parameters, modified function calls to ParseRevision, and adjusted parameter handling in various functions.
api/debug/debug_test.go Added testTraceCallNextBlock to test tracing the next block.
api/doc/thor.yaml Updated version number to 2.1.3, changed parameter references, and added new parameter CallCodeRevisionInQuery.
api/utils/revisions.go, api/utils/revisions_test.go Modified Revision type, added new constants and functions, updated ParseRevision function, and introduced new test TestAllowNext.
cmd/thor/VERSION Updated version number from 2.1.2 to 2.1.3.
.github/workflows/go-mod-check.yaml, .github/workflows/lint-go.yaml, .github/workflows/on-release.yaml, .github/workflows/test.yaml, .github/workflows/test-e2e.yaml Updated GitHub Actions to use newer versions of setup actions and dependencies.
consensus/consensus_test.go Added VIP191 setting in forkConfig, transaction handling logic, and a new test case for validating proposer with ErrInvalidFeatures.
api/blocks/blocks_test.go Refactored TestBlock function to streamline test execution using t.Run.

Poem

In the code of Thor, new paths we find,
Revisions and states, all redesigned.
With headers and nodes, we now align,
Debugging enhanced, tests well-defined.
Version bumped, workflows refined,
A rabbit's joy, in progress entwined.
Code evolves, with time combined.


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?

Share
Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai generate interesting stats about this repository and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between c23c19a and 872f27e.

Files selected for processing (9)
  • api/accounts/accounts.go (7 hunks)
  • api/accounts/accounts_test.go (2 hunks)
  • api/blocks/blocks.go (1 hunks)
  • api/debug/debug.go (5 hunks)
  • api/debug/debug_test.go (2 hunks)
  • api/doc/thor.yaml (9 hunks)
  • api/utils/revisions.go (2 hunks)
  • api/utils/revisions_test.go (5 hunks)
  • cmd/thor/VERSION (1 hunks)
Files skipped from review due to trivial changes (1)
  • cmd/thor/VERSION
Additional context used
yamllint
api/doc/thor.yaml

[error] 5-5: trailing spaces (trailing-spaces)


[error] 7-7: trailing spaces (trailing-spaces)


[error] 9-9: trailing spaces (trailing-spaces)


[error] 10-10: trailing spaces (trailing-spaces)


[error] 79-79: trailing spaces (trailing-spaces)


[error] 84-84: trailing spaces (trailing-spaces)


[error] 145-145: trailing spaces (trailing-spaces)


[error] 174-174: trailing spaces (trailing-spaces)


[error] 224-224: trailing spaces (trailing-spaces)


[error] 225-225: trailing spaces (trailing-spaces)


[error] 226-226: trailing spaces (trailing-spaces)


[error] 228-228: trailing spaces (trailing-spaces)


[error] 255-255: trailing spaces (trailing-spaces)


[error] 297-297: trailing spaces (trailing-spaces)


[error] 299-299: trailing spaces (trailing-spaces)


[error] 325-325: trailing spaces (trailing-spaces)


[error] 356-356: trailing spaces (trailing-spaces)


[error] 400-400: trailing spaces (trailing-spaces)


[error] 401-401: trailing spaces (trailing-spaces)


[error] 403-403: trailing spaces (trailing-spaces)


[error] 404-404: trailing spaces (trailing-spaces)


[error] 406-406: trailing spaces (trailing-spaces)


[error] 407-407: trailing spaces (trailing-spaces)


[error] 409-409: trailing spaces (trailing-spaces)


[error] 445-445: trailing spaces (trailing-spaces)


[error] 447-447: trailing spaces (trailing-spaces)


[error] 449-449: trailing spaces (trailing-spaces)


[error] 452-452: trailing spaces (trailing-spaces)


[error] 493-493: trailing spaces (trailing-spaces)


[error] 495-495: trailing spaces (trailing-spaces)


[error] 498-498: trailing spaces (trailing-spaces)


[error] 537-537: trailing spaces (trailing-spaces)


[error] 539-539: trailing spaces (trailing-spaces)


[error] 542-542: trailing spaces (trailing-spaces)


[error] 578-578: trailing spaces (trailing-spaces)


[error] 580-580: trailing spaces (trailing-spaces)


[error] 583-583: trailing spaces (trailing-spaces)


[error] 618-618: trailing spaces (trailing-spaces)


[error] 620-620: trailing spaces (trailing-spaces)


[error] 623-623: trailing spaces (trailing-spaces)


[error] 659-659: trailing spaces (trailing-spaces)


[error] 661-661: trailing spaces (trailing-spaces)


[error] 692-692: trailing spaces (trailing-spaces)


[error] 693-693: trailing spaces (trailing-spaces)


[error] 726-726: trailing spaces (trailing-spaces)


[error] 728-728: trailing spaces (trailing-spaces)


[error] 948-948: trailing spaces (trailing-spaces)


[error] 968-968: trailing spaces (trailing-spaces)


[error] 1002-1002: trailing spaces (trailing-spaces)


[error] 1843-1843: trailing spaces (trailing-spaces)


[error] 1852-1852: trailing spaces (trailing-spaces)


[error] 1855-1855: trailing spaces (trailing-spaces)


[error] 1856-1856: trailing spaces (trailing-spaces)


[error] 1858-1858: trailing spaces (trailing-spaces)


[error] 1883-1883: trailing spaces (trailing-spaces)


[error] 1901-1901: trailing spaces (trailing-spaces)


[error] 1933-1933: trailing spaces (trailing-spaces)


[error] 1941-1941: trailing spaces (trailing-spaces)


[error] 1942-1942: trailing spaces (trailing-spaces)


[error] 1944-1944: trailing spaces (trailing-spaces)


[error] 1952-1952: trailing spaces (trailing-spaces)


[error] 1953-1953: trailing spaces (trailing-spaces)


[error] 1955-1955: trailing spaces (trailing-spaces)


[error] 1963-1963: trailing spaces (trailing-spaces)


[error] 1964-1964: trailing spaces (trailing-spaces)


[error] 1966-1966: trailing spaces (trailing-spaces)


[error] 1974-1974: trailing spaces (trailing-spaces)


[error] 1975-1975: trailing spaces (trailing-spaces)


[error] 1977-1977: trailing spaces (trailing-spaces)


[error] 1980-1980: trailing spaces (trailing-spaces)


[error] 1981-1981: trailing spaces (trailing-spaces)


[error] 1982-1982: trailing spaces (trailing-spaces)


[error] 1990-1990: trailing spaces (trailing-spaces)


[error] 2099-2099: trailing spaces (trailing-spaces)


[error] 2100-2100: trailing spaces (trailing-spaces)


[error] 2170-2170: trailing spaces (trailing-spaces)


[error] 2261-2261: trailing spaces (trailing-spaces)


[error] 2331-2331: trailing spaces (trailing-spaces)


[error] 2333-2333: trailing spaces (trailing-spaces)


[error] 2344-2344: trailing spaces (trailing-spaces)


[error] 2377-2377: trailing spaces (trailing-spaces)


[error] 2388-2388: trailing spaces (trailing-spaces)


[error] 2401-2401: trailing spaces (trailing-spaces)


[error] 2415-2415: trailing spaces (trailing-spaces)


[error] 2429-2429: trailing spaces (trailing-spaces)

Additional comments not posted (22)
api/blocks/blocks.go (1)

33-33: Approved: Updated ParseRevision usage to include allowNext flag.

The change correctly implements the handling of revisions by explicitly disallowing the "next" revision in this context, which aligns with the new function signature and intended usage.

api/utils/revisions.go (4)

21-23: Approved: Addition of new revision constants.

The introduction of constants for revBest, revFinalized, and revNext is clear and supports the enhanced revision handling mechanism described in the PR.


Line range hint 35-66: Approved: Enhanced ParseRevision function to handle allowNext parameter.

The refactoring of ParseRevision to include the allowNext parameter and support various revision types enhances the function's flexibility and security, aligning with the objectives of handling revisions more robustly.


68-96: Approved: Refactored GetSummary function to use new Revision type.

The update to GetSummary to utilize the new Revision type and handle different revision values through a switch statement improves the function's clarity and maintainability.


98-139: Approved: Implementation of GetHeaderAndState for handling 'next' revision.

The GetHeaderAndState function's ability to construct a hypothetical next block's header and state is well-implemented, providing valuable functionality for simulations or predictions based on the next block.

api/utils/revisions_test.go (3)

Line range hint 27-77: Approved: Updated test cases in TestParseRevision to reflect new Revision type and allowNext handling.

The updates to the test cases in TestParseRevision correctly reflect the changes in the ParseRevision function, ensuring that the new functionality is thoroughly tested.


88-96: Approved: Addition of TestAllowNext to validate allowNext behavior in revisions.

The TestAllowNext function is a crucial addition that tests the behavior of the ParseRevision function when the allowNext parameter is toggled, ensuring the feature works as intended.


Line range hint 111-179: Approved: Comprehensive testing of GetSummary and GetHeaderAndState functions.

The updates to TestGetSummary and TestGetHeaderAndState provide comprehensive testing for the revised functions, covering various scenarios and ensuring robustness and correctness.

api/accounts/accounts.go (3)

69-69: Approved: Updated ParseRevision calls in handler functions.

The updates to the ParseRevision calls in handleGetCode, handleGetAccount, and handleGetStorage correctly align with the new function signature and enhance the robustness of revision handling.

Also applies to: 125-125, 152-152


175-175: Approved: Enabled 'next' revision handling in contract call functions.

The changes in handleCallContract and handleCallBatchCode to allow the "next" revision enable simulations of contract calls for future states, aligning with the PR's goals for enhanced flexibility and testing capabilities.

Also applies to: 218-218


236-243: Approved: Updated batchCall function to handle new header and state parameters.

The batchCall function has been updated to accept header and st parameters, which are necessary for executing batch calls with the revised state and header information. This change enhances the function's flexibility and capability.

api/debug/debug.go (3)

23-23: Approved new import for block handling.

The inclusion of github.com/vechain/thor/v2/block is justified as it's used in the updated function signatures for handling revisions and state.


181-181: Approved updates in handleTraceCall function.

The updates to use ParseRevision with a new parameter and GetHeaderAndState are aligned with the PR's goals to improve revision handling. Consider adding a comment explaining the purpose of the true parameter in ParseRevision for clarity.

Also applies to: 185-203


Line range hint 218-237: Approved changes in traceCall function.

The function now appropriately uses the updated block header and state structures. These changes are crucial for the integration with the new handling mechanisms. Monitor the performance implications of these changes, especially in high-load scenarios.

api/debug/debug_test.go (1)

254-257: Approved new test case for 'next' revision handling.

The test case testTraceCallNextBlock effectively checks the new revision handling feature by using the updated query parameter. This is a good practice to ensure that the new functionality works as expected.

api/accounts/accounts_test.go (1)

474-476: Confirm behavior for the 'next' revision

The tests for the 'next' revision seem to be properly implemented. However, given the significant changes in revision handling mentioned in the PR summary, it would be prudent to ensure that the behavior of the 'next' revision is thoroughly tested and documented, especially since it affects the core functionality of the system.

api/doc/thor.yaml (6)

15-15: Update API version number to reflect new changes

The version number has been updated from 2.1.2 to 2.1.3 to reflect the new changes and enhancements made to the API. This is a standard practice to help users identify which version of the API they are interacting with, ensuring compatibility and awareness of new features or fixes.


73-73: Added new parameter for gas estimation accuracy

A new parameter, CallCodeRevisionInQuery, has been introduced to the /accounts/* endpoint to enhance the accuracy of gas estimation. This change is crucial for users who need precise gas estimations for their transactions, especially in a production environment.


93-93: Refinement in schema referencing for clarity

The reference to ExecuteCodesRequest schema under the requestBody section of the /accounts/* POST method has been updated. This ensures that the correct schema is used for executing codes, which is essential for maintaining the integrity and functionality of the API.


696-696: Ensure consistency in parameter usage across endpoints

The CallCodeRevisionInQuery parameter has been added to the /debug/tracers/call endpoint. It's important to ensure that this parameter is consistently used across all relevant endpoints to maintain uniformity and prevent confusion among API users.


1187-1187: Clarification in PostDebugTracerRequest schema

The PostDebugTracerRequest schema has been updated to include a configuration object, which allows users to specify additional options for tracing. This enhancement provides flexibility and customization for developers looking to debug transactions more effectively.


2261-2267: Introduction of 'next' block handling in API queries

The revision parameter description has been expanded to include handling of the 'next' block. This is a significant update as it allows developers to test against the state of the blockchain as it will appear in the next block, which is crucial for applications that need to prepare for changes that have not yet been finalized.

Tools
yamllint

[error] 2261-2261: trailing spaces (trailing-spaces)

Comment on lines 378 to 384

// next revisoun should be valid
_, statusCode = httpPost(t, ts.URL+"/accounts/"+contractAddr.String()+"?revision=next", reqBody)
assert.Equal(t, http.StatusOK, statusCode, "next revision should be okay")
_, statusCode = httpPost(t, ts.URL+"/accounts?revisioun=next", reqBody)
assert.Equal(t, http.StatusOK, statusCode, "next revision should be okay")

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typographical issue and potential misunderstanding in parameter name

The parameter revisioun should be corrected to revision in the HTTP request URLs. This typographical error could lead to potential bugs or misunderstandings when interfacing with the API.

- httpPost(t, ts.URL+"/accounts?revisioun=next", reqBody)
+ httpPost(t, ts.URL+"/accounts?revision=next", reqBody)
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// next revisoun should be valid
_, statusCode = httpPost(t, ts.URL+"/accounts/"+contractAddr.String()+"?revision=next", reqBody)
assert.Equal(t, http.StatusOK, statusCode, "next revision should be okay")
_, statusCode = httpPost(t, ts.URL+"/accounts?revisioun=next", reqBody)
assert.Equal(t, http.StatusOK, statusCode, "next revision should be okay")
// next revisoun should be valid
_, statusCode = httpPost(t, ts.URL+"/accounts/"+contractAddr.String()+"?revision=next", reqBody)
assert.Equal(t, http.StatusOK, statusCode, "next revision should be okay")
_, statusCode = httpPost(t, ts.URL+"/accounts?revision=next", reqBody)
assert.Equal(t, http.StatusOK, statusCode, "next revision should be okay")

Copy link

@coderabbitai coderabbitai bot left a 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

Commits

Files that changed from the base of the PR and between 872f27e and 9535b6d.

Files selected for processing (1)
  • api/accounts/accounts_test.go (2 hunks)
Files skipped from review as they are similar to previous changes (1)
  • api/accounts/accounts_test.go

Copy link

@coderabbitai coderabbitai bot left a 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

Commits

Files that changed from the base of the PR and between 9535b6d and ee58f62.

Files selected for processing (6)
  • .github/workflows/go-mod-check.yaml (1 hunks)
  • .github/workflows/lint-go.yaml (1 hunks)
  • .github/workflows/on-release.yaml (1 hunks)
  • .github/workflows/test-e2e.yaml (1 hunks)
  • .github/workflows/test.yaml (2 hunks)
  • consensus/consensus_test.go (5 hunks)
Files skipped from review due to trivial changes (5)
  • .github/workflows/go-mod-check.yaml
  • .github/workflows/lint-go.yaml
  • .github/workflows/on-release.yaml
  • .github/workflows/test-e2e.yaml
  • .github/workflows/test.yaml
Additional comments not posted (4)
consensus/consensus_test.go (4)

87-89: Approved: Configuration settings added.

The VIP191, BLOCKLIST, and VIP214 settings are correctly initialized in the forkConfig.


99-107: Approved: Transaction handling logic added.

The transaction is correctly built, signed, and adopted into the flow.


217-218: Approved: Transaction features set in the builder.

The transaction features are correctly set in the builder using tc.original.Header().TxsFeatures().


682-702: Approved: Test case for ErrInvalidFeatures added.

The test case correctly validates the block's transaction features and expects the appropriate consensus error.

Copy link

@coderabbitai coderabbitai bot left a 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

Outside diff range, codebase verification and nitpick comments (1)
api/debug/debug_test.go (1)

254-257: LGTM! Consider adding assertions to validate the response content.

The function testTraceCallNextBlock is a good addition. However, it would be beneficial to add assertions to verify the response content, not just the status code.

  func testTraceCallNextBlock(t *testing.T) {
  	traceCallOption := &TraceCallOption{}
  	res := httpPostAndCheckResponseStatus(t, ts.URL+"/debug/tracers/call?revision=next", traceCallOption, 200)
+  	var parsedExecutionRes *logger.ExecutionResult
+  	if err := json.Unmarshal([]byte(res), &parsedExecutionRes); err != nil {
+  		t.Fatal(err)
+  	}
+  	expectedExecutionResult := &logger.ExecutionResult{
+  		Gas:         0,
+  		Failed:      false,
+  		ReturnValue: "",
+  		StructLogs:  make([]logger.StructLogRes, 0),
+  	}
+  	assert.Equal(t, expectedExecutionResult, parsedExecutionRes)
  }
Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between ee58f62 and 448ab50.

Files selected for processing (1)
  • api/debug/debug_test.go (2 hunks)

* getsummary and getsummaryAndState decoupling

* restart build
@codecov-commenter
Copy link

Welcome to Codecov 🎉

Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests.

Thanks for integrating Codecov - We've got you covered ☂️

Copy link

@coderabbitai coderabbitai bot left a 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

Commits

Files that changed from the base of the PR and between 448ab50 and f1b3c67.

Files selected for processing (6)
  • api/accounts/accounts.go (10 hunks)
  • api/blocks/blocks_test.go (1 hunks)
  • api/debug/debug.go (5 hunks)
  • api/debug/debug_test.go (3 hunks)
  • api/utils/revisions.go (2 hunks)
  • api/utils/revisions_test.go (5 hunks)
Files skipped from review due to trivial changes (1)
  • api/blocks/blocks_test.go
Files skipped from review as they are similar to previous changes (1)
  • api/debug/debug_test.go
Additional comments not posted (18)
api/utils/revisions_test.go (4)

27-27: Good use of pointers for Revision.

The changes to use pointers for Revision in the test cases are appropriate and align with the updated function signature.

Also applies to: 32-32, 37-37, 42-42, 47-51, 56-56


88-96: New test function TestAllowNext is well-implemented.

The test function correctly verifies the behavior of the ParseRevision function with the allowNext parameter.


111-112: Good use of pointers for Revision.

The changes to use pointers for Revision in the test cases are appropriate and align with the updated function signature.

Also applies to: 117-117, 122-122, 127-127, 132-132, 137-138


155-179: Test function TestGetSummaryAndState is well-implemented.

The test function correctly verifies the behavior of the GetSummaryAndState function with various revisions, including the next revision.

api/utils/revisions.go (4)

26-28: Changes to Revision type and addition of IsNext method are appropriate.

The changes align with the new functionality and ensure that the Revision type can handle the new requirements.

Also applies to: 30-31


Line range hint 35-66:
Changes to ParseRevision function are well-implemented.

The function correctly handles the new Revision type and the allowNext parameter, with appropriate error handling.


68-96: Changes to GetSummary function are well-implemented.

The function correctly retrieves the block summary for the given revision, with appropriate error handling.


98-146: New GetSummaryAndState function is well-implemented.

The function correctly retrieves the block summary and state for the given revision, with appropriate handling for the next revision.

api/accounts/accounts.go (7)

53-56: Changes to getCode function are well-implemented.

The function correctly retrieves the code for the given address and state, with appropriate error handling.


67-83: Changes to handleGetCode function are well-implemented.

The function correctly handles the request, parses the revision, and retrieves the code for the given address and state, with appropriate error handling.


Line range hint 87-98:
Changes to getAccount function are well-implemented.

The function correctly retrieves the account information for the given address, header, and state, with appropriate error handling.


121-134: Changes to handleGetAccount function are well-implemented.

The function correctly handles the request, parses the revision, and retrieves the account information for the given address, header, and state, with appropriate error handling.


108-111: Changes to getStorage function are well-implemented.

The function correctly retrieves the storage value for the given address, key, and state, with appropriate error handling.


150-163: Changes to handleGetStorage function are well-implemented.

The function correctly handles the request, parses the revision, and retrieves the storage value for the given address, key, and state, with appropriate error handling.


175-179: Changes to handleCallContract and handleCallBatchCode functions are well-implemented.

The functions correctly handle the requests, parse the revisions, and retrieve the results for the given batch call data, header, and state, with appropriate error handling.

Also applies to: 218-222

api/debug/debug.go (3)

181-185: Changes to handleTraceCall function are well-implemented.

The function correctly handles the request, parses the revision, and retrieves the results for the given trace call options, header, and state, with appropriate error handling.

Also applies to: 203-203


Line range hint 218-237:
Changes to traceCall function are well-implemented.

The function correctly traces the call using the given tracer, header, state, transaction context, gas, and clause, with appropriate error handling.


Line range hint 276-279:
Changes to debugStorage function are well-implemented.

The function correctly retrieves the storage debug information for the given contract address, block ID, transaction index, clause index, key start, and max result, with appropriate error handling.

Copy link

@coderabbitai coderabbitai bot left a 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

Commits

Files that changed from the base of the PR and between f1b3c67 and 85d11bc.

Files selected for processing (1)
  • api/blocks/blocks_test.go (1 hunks)
Files skipped from review as they are similar to previous changes (1)
  • api/blocks/blocks_test.go

Copy link

@coderabbitai coderabbitai bot left a 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

Outside diff range, codebase verification and nitpick comments (1)
api/doc/thor.yaml (1)

86-86: Fix trailing spaces.

Trailing spaces have been flagged by static analysis tools.

-        
+        

Also applies to: 2264-2264

Tools
yamllint

[error] 86-86: trailing spaces

(trailing-spaces)

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 85d11bc and a4bc240.

Files selected for processing (2)
  • .github/workflows/test-e2e.yaml (1 hunks)
  • api/doc/thor.yaml (9 hunks)
Files skipped from review due to trivial changes (1)
  • .github/workflows/test-e2e.yaml
Additional context used
yamllint
api/doc/thor.yaml

[error] 86-86: trailing spaces

(trailing-spaces)


[error] 2264-2264: trailing spaces

(trailing-spaces)

Additional comments not posted (4)
api/doc/thor.yaml (4)

15-15: Version update to 2.1.3.

The version update is straightforward and in line with the changes mentioned.


Line range hint 73-87: Added CallCodeRevisionInQuery parameter and recommendation to set revision to next for gas estimation.

The changes are consistent with the PR-Codex overview and enhance the API's functionality.

Tools
yamllint

[error] 84-84: trailing spaces

(trailing-spaces)


[error] 86-86: trailing spaces

(trailing-spaces)


697-699: Added CallCodeRevisionInQuery parameter.

The change is consistent with the PR-Codex overview and enhances the debugging capabilities.


2259-2272: Added CallCodeRevisionInQuery parameter definition.

The definition provides clear guidelines on the usage of the revision parameter, enhancing the API documentation.

Tools
yamllint

[error] 2264-2264: trailing spaces

(trailing-spaces)

@libotony libotony force-pushed the fix-gas-estimation branch from a4bc240 to 8b559a3 Compare July 11, 2024 10:47
Copy link

@coderabbitai coderabbitai bot left a 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

Outside diff range, codebase verification and nitpick comments (1)
api/doc/thor.yaml (1)

86-86: Remove trailing spaces.

Trailing spaces were found on these lines. Removing trailing spaces helps maintain code cleanliness.

-        
+

Also applies to: 2264-2264

Tools
yamllint

[error] 86-86: trailing spaces

(trailing-spaces)

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between a4bc240 and 8b559a3.

Files selected for processing (3)
  • .github/workflows/test-e2e.yaml (1 hunks)
  • api/blocks/blocks_test.go (1 hunks)
  • api/doc/thor.yaml (9 hunks)
Files skipped from review due to trivial changes (1)
  • api/blocks/blocks_test.go
Files skipped from review as they are similar to previous changes (1)
  • .github/workflows/test-e2e.yaml
Additional context used
yamllint
api/doc/thor.yaml

[error] 86-86: trailing spaces

(trailing-spaces)


[error] 2264-2264: trailing spaces

(trailing-spaces)

Additional comments not posted (4)
api/doc/thor.yaml (4)

15-15: Version update approved.

The version number has been correctly updated to 2.1.3.


73-73: New parameter reference approved.

The new reference to CallCodeRevisionInQuery has been correctly added.

However, ensure that the new parameter reference is consistently used across the documentation.


87-87: New gas estimation recommendation approved.

The new recommendation to set the revision query parameter to next when estimating gas has been clearly added.

However, ensure the accuracy of this recommendation and that it is aligned with other related documentation.

Verification successful

New gas estimation recommendation approved.

The new recommendation to set the revision query parameter to next when estimating gas has been added clearly. It aligns with the existing documentation, which includes detailed descriptions of the revision parameter and its possible values, including next.

  • Ensure the accuracy of this recommendation and that it is aligned with other related documentation.
  • Verify that the description for the next value of the revision parameter is consistent and clear across all references in the api/doc/thor.yaml file.
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the accuracy of the new gas estimation recommendation.

# Test: Search for related documentation. Expect: Consistent guidance on using `revision` query parameter for gas estimation.
rg --type yaml -A 5 $'revision'

Length of output: 4263


699-699: New parameter reference approved.

The new reference to CallCodeRevisionInQuery has been correctly added.

However, ensure that the new parameter reference is consistently used across the documentation.

Verification successful

New parameter reference consistently used across the documentation.

The CallCodeRevisionInQuery parameter reference is correctly defined and consistently used in the relevant endpoints.

  • The first occurrence is in the /accounts/* endpoint.
  • The second occurrence is in the tracer creation endpoint.
  • The parameter is properly defined in the components section.
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the usage of the new parameter reference `CallCodeRevisionInQuery`.

# Test: Search for the parameter reference. Expect: Consistent usage across the documentation.
rg --type yaml $'CallCodeRevisionInQuery'

Length of output: 255


Script:

#!/bin/bash
# Extracting context around the occurrences of `CallCodeRevisionInQuery` in `api/doc/thor.yaml`.
rg --type yaml -A 5 -B 5 'CallCodeRevisionInQuery' api/doc/thor.yaml

Length of output: 1151

@libotony libotony deleted the fix-gas-estimation branch July 23, 2024 09:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants