Refactor Test Utils - Githubissues

delvtech / hyperdrive

An automated market maker for fixed and variable yield with on-demand terms.

Apache License 2.0

26 stars 3 forks source link

Description

While it may not be apparent when implementing the typical differential testing test, the current test utilities are quite cumbersome to work with. In retrospect, I think the abstractions came together too early, and it has been a pain to implement things like migration scripts and to consider doing more with the tools. The goal of this PR was to decouple the utilities of TestChain and DevChain to simplify the DevEx of interacting with the Rust repo. The guiding principle was to preserve as much "orthogonality" as possible.

While it may not look like it when reviewing the hyperdrive-math tests, for example, the tools are now much easier to work with. If you want to connect to a remote chain and deploy something, no problem. You don't have to use a TestChain, and you don't have to bundle signers into it. If you want to get the addresses from a DevChain, no problem. Just call the function.

To keep ergonomics relatively constant (I actually think they have improved), I re-implemented TestChain using the new utilities and made some test accounts like ALICE, BOB, and CELINE first-class citizens. It's now easier to set up a test since account management isn't a concern anymore. This significantly reduces the number of imports required. Perhaps more importantly, the tools are more orthogonal, which sets us up for future improvements.

There is more that could and should be improved with these tools, but I didn't want the PR to be too large (it's already fairly large as-is). I'm going to need this orthogonality to implement the testnet migration script.

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 and/or Rust, the corresponding checklist can be removed.

[[Reviewer Name]]

Rust

[ ] 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?
[ ] If matching Solidity behavior, are there differential fuzz tests that ensure that Rust matches Solidity?

Hyperdrive Gas Benchmark

Benchmark suite	Current: 627b98c44523b10406db953ee02d9fb3dc7e0e5a	Previous: f4929a74927634cae72e240249b48564c4dde59e	Deviation	Status
`addLiquidity: min`	`33783` gas	`33915` gas	`-0.3892%`	✅
`addLiquidity: avg`	`145089` gas	`143443` gas	`1.1475%`	🚨
`addLiquidity: max`	`428119` gas	`404385` gas	`5.8692%`	🚨
`checkpoint: min`	`40220` gas	`29232` gas	`37.5889%`	🚨
`checkpoint: avg`	`104092` gas	`103630` gas	`0.4458%`	🚨
`checkpoint: max`	`212154` gas	`211837` gas	`0.1496%`	🚨
`closeLong: min`	`31495` gas	`31428` gas	`0.2132%`	🚨
`closeLong: avg`	`138025` gas	`137783` gas	`0.1756%`	🚨
`closeLong: max`	`2640391` gas	`2640321` gas	`0.0027%`	🚨
`closeShort: min`	`31416` gas	`31349` gas	`0.2137%`	🚨
`closeShort: avg`	`132886` gas	`132566` gas	`0.2414%`	🚨
`closeShort: max`	`227543` gas	`227388` gas	`0.0682%`	🚨
`initialize: min`	`31327` gas	`31283` gas	`0.1407%`	🚨
`initialize: avg`	`253593` gas	`253505` gas	`0.0347%`	🚨
`initialize: max`	`322650` gas	`322697` gas	`-0.0146%`	✅
`openLong: min`	`33547` gas	`33503` gas	`0.1313%`	🚨
`openLong: avg`	`167333` gas	`166952` gas	`0.2282%`	🚨
`openLong: max`	`253206` gas	`253061` gas	`0.0573%`	🚨
`openShort: min`	`33937` gas	`33848` gas	`0.2629%`	🚨
`openShort: avg`	`170318` gas	`169836` gas	`0.2838%`	🚨
`openShort: max`	`385540` gas	`363400` gas	`6.0925%`	🚨
`redeemWithdrawalShares: min`	`31227` gas	`31227` gas	`0%`	🟰
`redeemWithdrawalShares: avg`	`62061` gas	`61624` gas	`0.7091%`	🚨
`redeemWithdrawalShares: max`	`167484` gas	`167368` gas	`0.0693%`	🚨
`removeLiquidity: min`	`31169` gas	`31323` gas	`-0.4917%`	✅
`removeLiquidity: avg`	`223099` gas	`222313` gas	`0.3536%`	🚨
`removeLiquidity: max`	`398777` gas	`375299` gas	`6.2558%`	🚨

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