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

SHARD-2035: fix leading 0 in gasUsed #142

Merged
merged 1 commit into from
Mar 24, 2025
Merged

Conversation

urnotsam
Copy link
Contributor

@urnotsam urnotsam commented Mar 21, 2025

PR Type

Enhancement, Tests


Description

  • Introduced calculateBlockGasUsed function for gas calculation.

  • Replaced manual gas calculation with utility function.

  • Added unit tests for calculateBlockGasUsed.

  • Improved handling of gas values in transactions.


Changes walkthrough 📝

Relevant files
Enhancement
Collector.ts
Use utility function for gas calculation                                 

src/external/Collector.ts

  • Replaced manual gas calculation with calculateBlockGasUsed.
  • Improved gas value handling in transactions.
  • +3/-15   
    gasCalculator.ts
    Introduce utility for block gas calculation                           

    src/utils/gasCalculator.ts

  • Added calculateBlockGasUsed function.
  • Handles gas value parsing and summation.
  • +30/-0   
    Tests
    gasCalculator.test.ts
    Add unit tests for gas calculation utility                             

    test/unit/utils/gasCalculator.test.ts

  • Added tests for calculateBlockGasUsed.
  • Covered various gas value scenarios.
  • +157/-0 

    Need help?
  • Type /help how to ... in the comments thread for any questions about PR-Agent usage.
  • Check out the documentation for more information.
  • Copy link

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    ⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
    🏅 Score: 95
    🧪 PR contains tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Possible Issue

    The calculateBlockGasUsed function uses replace(/0x0+/, '0x') to remove leading zeros. This might incorrectly transform values like 0x0001 to 0x1 instead of 0x01. Ensure this behavior is intended.

    // Convert to hex and remove leading zeros
    const hexString = blockGasUsed.toHexString()
    return hexString.replace(/0x0+/, '0x')


    // Convert to hex and remove leading zeros
    const hexString = blockGasUsed.toHexString()
    return hexString.replace(/0x0+/, '0x')

    Choose a reason for hiding this comment

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

    Suggestion: Ensure that the regular expression used for removing leading zeros does not inadvertently remove the '0' from '0x0' when the gas used is zero. [possible issue, importance: 7]

    Suggested change
    return hexString.replace(/0x0+/, '0x')
    return hexString.replace(/^0x0+(?!$)/, '0x')

    @aniketdivekar aniketdivekar merged commit 2d612b5 into mainnet-launch Mar 24, 2025
    5 checks passed
    @aniketdivekar aniketdivekar deleted the SHARD-2035 branch March 24, 2025 09:18
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Projects
    None yet
    Development

    Successfully merging this pull request may close these issues.

    None yet

    3 participants