Skip to content

4626 Cleanup#649

Merged
jrhea merged 11 commits intomainfrom
4626cleanup
Nov 6, 2023
Merged

4626 Cleanup#649
jrhea merged 11 commits intomainfrom
4626cleanup

Conversation

@jrhea
Copy link
Copy Markdown
Contributor

@jrhea jrhea commented Nov 2, 2023

  • update event fields for consistency
  • ensure that options.asBase is respected
    • open/close long/short expect input to match options.asBase and output will always return the values that respect options.asBase
    • Hyperdrive4626 _deposit expects the input amount to respect options.asBase, but ALWAYS returns vaultShares
    • Hyperdrive4626 _withdraw ALWAYS expects input amount in shares, but returns values that respect options.asBase

#621 - partially resolves

@jrhea jrhea changed the title update event fields for consistency 4626 Cleanup Nov 2, 2023
@coveralls
Copy link
Copy Markdown
Collaborator

coveralls commented Nov 2, 2023

Coverage Status

coverage: 97.581% (+0.04%) from 97.54%
when pulling 03d8e6a on 4626cleanup
into 7e0766e on main.

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Nov 2, 2023

Hyperdrive Gas Benchmark

Benchmark suite Current: 03d8e6a Previous: e3658bb Deviation Status
addLiquidity: min 755 gas 733 gas 3.0014% 🚨
addLiquidity: avg 54735 gas 54041 gas 1.2842% 🚨
addLiquidity: max 97582 gas 96720 gas 0.8912% 🚨
checkpoint: min 558 gas 558 gas 0% 🟰
checkpoint: avg 47622 gas 47628 gas -0.0126%
checkpoint: max 99410 gas 99410 gas 0% 🟰
closeLong: min 755 gas 755 gas 0% 🟰
closeLong: avg 24124 gas 23498 gas 2.6641% 🚨
closeLong: max 114894 gas 114183 gas 0.6227% 🚨
closeShort: min 713 gas 802 gas -11.0973%
closeShort: avg 27315 gas 26634 gas 2.5569% 🚨
closeShort: max 113274 gas 112872 gas 0.3562% 🚨
initialize: min 706 gas 662 gas 6.6465% 🚨
initialize: avg 178039 gas 177327 gas 0.4015% 🚨
initialize: max 252327 gas 251633 gas 0.2758% 🚨
openLong: min 757 gas 757 gas 0% 🟰
openLong: avg 57492 gas 56962 gas 0.9304% 🚨
openLong: max 219128 gas 218431 gas 0.3191% 🚨
openShort: min 712 gas 712 gas 0% 🟰
openShort: avg 57096 gas 56198 gas 1.5979% 🚨
openShort: max 218660 gas 217803 gas 0.3935% 🚨
removeLiquidity: min 777 gas 777 gas 0% 🟰
removeLiquidity: avg 76929 gas 75980 gas 1.2490% 🚨
removeLiquidity: max 204374 gas 203187 gas 0.5842% 🚨

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

@jrhea jrhea marked this pull request as ready for review November 6, 2023 00:33
Comment thread contracts/src/HyperdriveLP.sol
Comment thread contracts/src/HyperdriveLP.sol Outdated
Comment thread contracts/src/HyperdriveLP.sol Outdated
Comment thread contracts/src/HyperdriveLP.sol Outdated
Comment thread contracts/src/HyperdriveLP.sol Outdated
Comment thread contracts/src/HyperdriveLP.sol Outdated
Comment thread contracts/src/HyperdriveLong.sol Outdated
Comment thread contracts/src/HyperdriveShort.sol
Comment thread contracts/src/HyperdriveShort.sol Outdated
Comment thread contracts/src/HyperdriveLong.sol
Copy link
Copy Markdown
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.

LGTM. I'm happy to take another look when you implement the conversion functions, but it's up to you.

@jrhea jrhea enabled auto-merge (squash) November 6, 2023 03:58
@jrhea jrhea merged commit 191bd10 into main Nov 6, 2023
@jrhea jrhea deleted the 4626cleanup branch November 6, 2023 04:58
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