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.

Sign up for GitHub

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

Jump to bottom

Improved the debugging tools #1057

Merged
merged 5 commits into from
Jun 14, 2024
Merged

Improved the debugging tools #1057

merged 5 commits into from
Jun 14, 2024

Conversation

jalextowle
Copy link
Contributor

Description

This PR adds a new mode to the debugging tools for simulating a transaction that has already been mined. This addresses a limitation of the tool by allowing users to simulate a transaction at a specified block height and transaction height within the block. This will make the tool more useful on mainnet and testnet environments where we can't assume that each transaction will be executed in separate blocks.

In addition to adding the new debugging mode, this PR also spruces up the tool and makes it more usable by relying more heavily on debugging-specific environment variables.

Review Checklists

Please check each item before approving the pull request. While going
through the checklist, it is recommended to leave comments on items that are
referenced in the checklist to make sure that they are reviewed. If there are
multiple reviewers, copy the checklists into sections titled ## [Reviewer Name].
If the PR doesn't touch Solidity, the corresponding checklist can
be removed.

[[Reviewer Name]]

  • Tokens
    • Do all approve calls use forceApprove?
    • Do all transfer calls use safeTransfer?
    • Do all transferFrom calls use msg.sender as the from address?
      • If not, is the function access restricted to prevent unauthorized
        token spend?
  • Low-level calls (call, delegatecall, staticcall, transfer, send)
    • Is the returned success boolean checked to handle failed calls?
    • If using delegatecall, are there strict access controls on the
      addresses that can be called? It shouldn't be possible to delegatecall
      arbitrary addresses, so the list of possible targets should either be
      immutable or tightly controlled by an admin.
  • Reentrancy
    • Are functions that make external calls or transfer ether marked as nonReentrant?
      • If not, is there documentation that explains why reentrancy is
        not a concern or how it's mitigated?
  • Gas Optimizations
    • Is the logic as simple as possible?
    • Are the storage values that are used repeatedly cached in stack or
      memory variables?
    • If loops are used, are there guards in place to avoid out-of-gas
      issues?
  • Visibility
    • Are all payable functions restricted to avoid stuck ether?
  • Math
    • Is all of the arithmetic checked or guarded by if-statements that will
      catch underflows?
    • If Safe functions are altered, are potential underflows and
      overflows caught so that a failure flag can be thrown?
    • Are all of the rounding directions clearly documented?
  • Testing
    • Are there new or updated unit or integration tests?
    • Do the tests cover the happy paths?
    • Do the tests cover the unhappy paths?
    • Are there an adequate number of fuzz tests to ensure that we are
      covering the full input space?

@jalextowle jalextowle force-pushed the jalextowle/debug/improved-etching-tools branch from 2ef6aa8 to d7ec3e0 Compare June 13, 2024 19:04
@coveralls
Copy link
Collaborator

coveralls commented Jun 13, 2024 •
edited
Loading

Pull Request Test Coverage Report for Build 9505286878

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 92.888%
Totals Coverage Status
Change from base Build 9504821912: 0.0%
Covered Lines: 1907
Relevant Lines: 2053
💛 - Coveralls

@coveralls
Copy link
Collaborator

coveralls commented Jun 13, 2024 •
edited
Loading

Pull Request Test Coverage Report for Build 9505329870

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage decreased (-0.05%) to 92.84%
Files with Coverage Reduction New Missed Lines %
contracts/src/libraries/LPMath.sol 1 83.14%
Totals Coverage Status
Change from base Build 9504821912: -0.05%
Covered Lines: 1906
Relevant Lines: 2053
💛 - Coveralls

@github-actions GitHub Actions
Copy link

github-actions bot commented Jun 13, 2024 •
edited
Loading

Hyperdrive Gas Benchmark

Benchmark suite Current: 7c5cdf6 Previous: 4f02324 Deviation Status
addLiquidity: min 33937 gas 33827 gas 0.3252% 🚨
addLiquidity: avg 156052 gas 155252 gas 0.5153% 🚨
addLiquidity: max 429437 gas 429092 gas 0.0804% 🚨
checkpoint: min 40316 gas 40292 gas 0.0596% 🚨
checkpoint: avg 144560 gas 142311 gas 1.5803% 🚨
checkpoint: max 255830 gas 253424 gas 0.9494% 🚨
closeLong: min 31361 gas 31361 gas 0% 🟰
closeLong: avg 136339 gas 136004 gas 0.2463% 🚨
closeLong: max 2621435 gas 2621386 gas 0.0019% 🚨
closeShort: min 31327 gas 31349 gas -0.0702% ✅
closeShort: avg 132301 gas 132022 gas 0.2113% 🚨
closeShort: max 272530 gas 263302 gas 3.5047% 🚨
initialize: min 31349 gas 31371 gas -0.0701% ✅
initialize: avg 333305 gas 330980 gas 0.7025% 🚨
initialize: max 399922 gas 397010 gas 0.7335% 🚨
openLong: min 33370 gas 33370 gas 0% 🟰
openLong: avg 174369 gas 174132 gas 0.1361% 🚨
openLong: max 335241 gas 306958 gas 9.2140% 🚨
openShort: min 33936 gas 33936 gas 0% 🟰
openShort: avg 168770 gas 168517 gas 0.1501% 🚨
openShort: max 415854 gas 415737 gas 0.0281% 🚨
redeemWithdrawalShares: min 31251 gas 31251 gas 0% 🟰
redeemWithdrawalShares: avg 75185 gas 75086 gas 0.1318% 🚨
redeemWithdrawalShares: max 305633 gas 210204 gas 45.3983% 🚨
removeLiquidity: min 31301 gas 31301 gas 0% 🟰
removeLiquidity: avg 215498 gas 214869 gas 0.2927% 🚨
removeLiquidity: max 404194 gas 403971 gas 0.0552% 🚨

This comment was automatically generated by workflow using github-action-benchmark.

@jalextowle jalextowle force-pushed the jalextowle/debug/improved-etching-tools branch from fc72617 to 2284972 Compare June 13, 2024 19:42
@coveralls
Copy link
Collaborator

coveralls commented Jun 13, 2024 •
edited
Loading

Pull Request Test Coverage Report for Build 9505740981

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 92.888%
Totals Coverage Status
Change from base Build 9504821912: 0.0%
Covered Lines: 1907
Relevant Lines: 2053
💛 - Coveralls

@coveralls
Copy link
Collaborator

coveralls commented Jun 13, 2024 •
edited
Loading

Pull Request Test Coverage Report for Build 9505783860

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 92.888%
Totals Coverage Status
Change from base Build 9504821912: 0.0%
Covered Lines: 1907
Relevant Lines: 2053
💛 - Coveralls

@coveralls
Copy link
Collaborator

coveralls commented Jun 13, 2024 •
edited
Loading

Pull Request Test Coverage Report for Build 9505908141

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage decreased (-0.05%) to 92.84%
Files with Coverage Reduction New Missed Lines %
contracts/src/libraries/LPMath.sol 1 83.14%
Totals Coverage Status
Change from base Build 9504821912: -0.05%
Covered Lines: 1906
Relevant Lines: 2053
💛 - Coveralls

@jalextowle jalextowle force-pushed the jalextowle/debug/improved-etching-tools branch from 3212191 to ddfeb66 Compare June 14, 2024 20:40
@jalextowle jalextowle force-pushed the jalextowle/debug/improved-etching-tools branch from ddfeb66 to 67a57bc Compare June 14, 2024 20:51
@coveralls
Copy link
Collaborator

coveralls commented Jun 14, 2024 •
edited
Loading

Pull Request Test Coverage Report for Build 9522112274

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 92.124%
Totals Coverage Status
Change from base Build 9510431696: 0.0%
Covered Lines: 1965
Relevant Lines: 2133
💛 - Coveralls

@coveralls
Copy link
Collaborator

coveralls commented Jun 14, 2024 •
edited
Loading

Pull Request Test Coverage Report for Build 9522219255

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 92.124%
Totals Coverage Status
Change from base Build 9510431696: 0.0%
Covered Lines: 1965
Relevant Lines: 2133
💛 - Coveralls

mcclurejt
mcclurejt approved these changes Jun 14, 2024
View reviewed changes
Copy link
Contributor

@mcclurejt mcclurejt left a comment

Choose a reason for hiding this comment

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

Awesome job! looking forward to using these

test/debug/Debug.t.sol Outdated Show resolved Hide resolved
foundry.toml Show resolved Hide resolved
@jalextowle jalextowle disabled auto-merge June 14, 2024 21:21
@jalextowle jalextowle force-pushed the jalextowle/debug/improved-etching-tools branch from 5fb3485 to 7c5cdf6 Compare June 14, 2024 21:23
@coveralls
Copy link
Collaborator

coveralls commented Jun 14, 2024 •
edited
Loading

Pull Request Test Coverage Report for Build 9522519186

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 92.124%
Totals Coverage Status
Change from base Build 9510431696: 0.0%
Covered Lines: 1965
Relevant Lines: 2133
💛 - Coveralls

@coveralls
Copy link
Collaborator

coveralls commented Jun 14, 2024 •
edited
Loading

Pull Request Test Coverage Report for Build 9522537306

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 92.124%
Totals Coverage Status
Change from base Build 9510431696: 0.0%
Covered Lines: 1965
Relevant Lines: 2133
💛 - Coveralls

@jalextowle jalextowle added this pull request to the merge queue Jun 14, 2024
Merged via the queue into main with commit e5525e9 Jun 14, 2024
32 checks passed
@jalextowle jalextowle deleted the jalextowle/debug/improved-etching-tools branch June 14, 2024 22:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mcclurejt mcclurejt mcclurejt approved these changes

@jrhea jrhea Awaiting requested review from jrhea jrhea is a code owner

@cashd cashd Awaiting requested review from cashd cashd is a code owner

@sentilesdal sentilesdal Awaiting requested review from sentilesdal sentilesdal is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@jalextowle @coveralls @mcclurejt