Skip to content

Added an extra data field to the Hyperdrive interface#608

Merged
jalextowle merged 16 commits intomainfrom
jalextowle/interface/extra-data
Oct 19, 2023
Merged

Added an extra data field to the Hyperdrive interface#608
jalextowle merged 16 commits intomainfrom
jalextowle/interface/extra-data

Conversation

@jalextowle
Copy link
Copy Markdown
Contributor

Some yield sources may need additional data outside of the core Hyperdrive interface to function. Since we can't anticipate all of these requirements and don't want to clutter the interface with parameters that will only be used by specific yield sources, it makes sense to add a catch-all parameter that is just a string of bytes.

This ended up being a pretty straightforward change. It will have a large impact on the frontend and sims teams, but from a system perspective, it wasn't an invasive change. The one place that was slightly less trivial was the bond wrapper which makes repeated calls to Hyperdrive. sweepAndRedeem takes an array of bytes to sidestep any issues caused by trying to re-use the same extra data over and over.

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Oct 9, 2023

Hyperdrive Gas Benchmark

Benchmark suite Current: ba1f069 Previous: 6ba277c Deviation Status
addLiquidity: min 733 gas 807 gas -9.1698%
addLiquidity: avg 55943 gas 55700 gas 0.4363% 🚨
addLiquidity: max 97572 gas 97346 gas 0.2322% 🚨
checkpoint: min 536 gas 514 gas 4.2802% 🚨
checkpoint: avg 30346 gas 30342 gas 0.0132% 🚨
checkpoint: max 79983 gas 79961 gas 0.0275% 🚨
closeLong: min 733 gas 852 gas -13.9671%
closeLong: avg 23247 gas 23016 gas 1.0036% 🚨
closeLong: max 106163 gas 106073 gas 0.0848% 🚨
closeShort: min 802 gas 854 gas -6.0890%
closeShort: avg 24820 gas 24540 gas 1.1410% 🚨
closeShort: max 105590 gas 105372 gas 0.2069% 🚨
initialize: min 662 gas 803 gas -17.5592%
initialize: avg 160854 gas 160648 gas 0.1282% 🚨
initialize: max 234375 gas 233705 gas 0.2867% 🚨
openLong: min 735 gas 768 gas -4.2969%
openLong: avg 55814 gas 55557 gas 0.4626% 🚨
openLong: max 225577 gas 224842 gas 0.3269% 🚨
openShort: min 690 gas 744 gas -7.2581%
openShort: avg 56443 gas 56173 gas 0.4807% 🚨
openShort: max 220946 gas 220664 gas 0.1278% 🚨
removeLiquidity: min 755 gas 784 gas -3.6990%
removeLiquidity: avg 79523 gas 79100 gas 0.5348% 🚨
removeLiquidity: max 184551 gas 184059 gas 0.2673% 🚨

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

@coveralls
Copy link
Copy Markdown
Collaborator

coveralls commented Oct 9, 2023

Coverage Status

coverage: 97.018% (-0.09%) from 97.106% when pulling ba1f069 on jalextowle/interface/extra-data into 6ba277c on main.

Copy link
Copy Markdown
Contributor

@jrhea jrhea left a comment

Choose a reason for hiding this comment

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

lgtm. you have a few FIXMEs to cleanup comments. is the plan to address them in this PR?

This commit moved the `destination`, `asUnderlying`, and `extraData`
fields into a single struct called `IHyperdrive.Options`. Along with
this change, `Hyperdrive` is now `IHyperdriveWrite` and
`HyperdriveDataProvider` is now `IHyperdriveRead`, which ensures that
future interface changes will result in compiler errors rather than
issues at run-time.
@jalextowle jalextowle force-pushed the jalextowle/interface/extra-data branch from d7aff24 to 37a162c Compare October 19, 2023 05:08
Copy link
Copy Markdown
Contributor

@jrhea jrhea left a comment

Choose a reason for hiding this comment

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

looks good. just one question

Comment thread contracts/src/instances/ERC4626Hyperdrive.sol Outdated
Comment thread test/combinatorial/BondWrapper.close.t.sol
@jalextowle jalextowle enabled auto-merge (squash) October 19, 2023 19:24
@jalextowle jalextowle merged commit 5dae7f5 into main Oct 19, 2023
@jalextowle jalextowle deleted the jalextowle/interface/extra-data branch October 19, 2023 20:23
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.

3 participants