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

Consolidate Instances on ERC4626 #454

Merged
merged 55 commits into from
Jul 18, 2023

Conversation

ControlCplusControlV
Copy link
Contributor

@ControlCplusControlV ControlCplusControlV commented Jul 11, 2023

Instead of using instance specific code, this PR consolidates all of our code around the ERC4626 instance. We wrap many different yield sources using Yield Daddy to make the compliant with the ERC4626 standard. This reduces any potential error between different implementations such as DsrHyperdrive vs StethHyperdrive, and allows us to more thoroughly test one instance.

I've also worked on adding in a single test suite to verify an ERC4626 is compliant with hyperdrive, and will function as expected

@ControlCplusControlV ControlCplusControlV added the contracts Requires smart contract engineering label Jul 11, 2023
@ControlCplusControlV ControlCplusControlV self-assigned this Jul 11, 2023
@coveralls
Copy link
Collaborator

coveralls commented Jul 11, 2023

Coverage Status

coverage: 98.189% (+1.6%) from 96.571% when pulling 557f08c on controlc/erc4626_consolidation into bfc1230 on main.

@github-actions
Copy link

github-actions bot commented Jul 12, 2023

Hyperdrive Gas Benchmark

Benchmark suite Current: 557f08c Previous: 5007450 Deviation Status
addLiquidity: min 785 gas 709 gas 10.7193% 🚨
addLiquidity: avg 43700 gas 45498 gas -3.9518%
addLiquidity: max 80524 gas 77728 gas 3.5972% 🚨
checkpoint: min 514 gas 514 gas 0% 🟰
checkpoint: avg 23044 gas 23019 gas 0.1086% 🚨
checkpoint: max 33438 gas 33407 gas 0.0928% 🚨
closeLong: min 852 gas 659 gas 29.2868% 🚨
closeLong: avg 45870 gas 47497 gas -3.4255%
closeLong: max 86074 gas 83240 gas 3.4046% 🚨
closeShort: min 809 gas 616 gas 31.3312% 🚨
closeShort: avg 40932 gas 42330 gas -3.3026%
closeShort: max 87306 gas 84477 gas 3.3488% 🚨
initialize: min 714 gas 532 gas 34.2105% 🚨
initialize: avg 159493 gas 160281 gas -0.4916%
initialize: max 233625 gas 161781 gas 44.4082% 🚨
openLong: min 740 gas 661 gas 11.9516% 🚨
openLong: avg 113249 gas 114770 gas -1.3253%
openLong: max 180018 gas 175911 gas 2.3347% 🚨
openShort: min 782 gas 709 gas 10.2962% 🚨
openShort: avg 151265 gas 159830 gas -5.3588%
openShort: max 219306 gas 218810 gas 0.2267% 🚨
removeLiquidity: min 762 gas 569 gas 33.9192% 🚨
removeLiquidity: avg 57251 gas 62682 gas -8.6644%
removeLiquidity: max 119378 gas 116558 gas 2.4194% 🚨

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

@ControlCplusControlV ControlCplusControlV marked this pull request as ready for review July 14, 2023 23:53
yarn.lock Outdated Show resolved Hide resolved
Copy link
Contributor

@jalextowle jalextowle left a comment

Choose a reason for hiding this comment

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

Excellent work on this PR! I'm approving, so once you've addressed my comments and Jonny's comments, feel free to merge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contracts Requires smart contract engineering
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants