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

new min lpShare check plus tests #653

Merged
merged 3 commits into from
Nov 7, 2023
Merged

new min lpShare check plus tests #653

merged 3 commits into from
Nov 7, 2023

Conversation

jrhea
Copy link
Contributor

@jrhea jrhea commented Nov 6, 2023

resolves #630

The fix here is:

  • in test_nonstandard_decimals_lp(): enforce min/max apr guards in the failing test and conditionally vm.ExpectRevert()
  • add a check to addLiquidity that reverts if the number of lpShares to be minted is < minTransaction amount
  • test for new minTransaction revert in addLiquidity

Copy link

github-actions bot commented Nov 6, 2023

Hyperdrive Gas Benchmark

Benchmark suite Current: fa16299 Previous: ecb3124 Deviation Status
addLiquidity: min 755 gas 755 gas 0% 🟰
addLiquidity: avg 51441 gas 54735 gas -6.0181%
addLiquidity: max 97600 gas 97582 gas 0.0184% 🚨
checkpoint: min 558 gas 558 gas 0% 🟰
checkpoint: avg 47626 gas 47634 gas -0.0168%
checkpoint: max 99410 gas 99410 gas 0% 🟰
closeLong: min 755 gas 755 gas 0% 🟰
closeLong: avg 24284 gas 24242 gas 0.1733% 🚨
closeLong: max 114894 gas 114894 gas 0% 🟰
closeShort: min 713 gas 713 gas 0% 🟰
closeShort: avg 27117 gas 27210 gas -0.3418%
closeShort: max 113274 gas 113274 gas 0% 🟰
initialize: min 706 gas 706 gas 0% 🟰
initialize: avg 178066 gas 178049 gas 0.0095% 🚨
initialize: max 252327 gas 252327 gas 0% 🟰
openLong: min 757 gas 757 gas 0% 🟰
openLong: avg 57591 gas 57583 gas 0.0139% 🚨
openLong: max 219128 gas 219128 gas 0% 🟰
openShort: min 712 gas 712 gas 0% 🟰
openShort: avg 57124 gas 57045 gas 0.1385% 🚨
openShort: max 218660 gas 218660 gas 0% 🟰
removeLiquidity: min 777 gas 777 gas 0% 🟰
removeLiquidity: avg 78855 gas 77245 gas 2.0843% 🚨
removeLiquidity: max 204374 gas 204374 gas 0% 🟰

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

@coveralls
Copy link
Collaborator

coveralls commented Nov 6, 2023

Coverage Status

coverage: 97.589% (+0.008%) from 97.581%
when pulling fa16299 on minlpshares
into ecb3124 on main.

@jrhea jrhea requested a review from jalextowle November 6, 2023 23:49
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.

LGTM :shipit:

@jrhea jrhea changed the title add failing edge case test and add min lpShare check new min lpShare check plus tests Nov 7, 2023
@jrhea jrhea enabled auto-merge (squash) November 7, 2023 14:40
@jrhea jrhea merged commit 6f067f9 into main Nov 7, 2023
@jrhea jrhea deleted the minlpshares branch November 7, 2023 15:57
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.

test_nonstandard_decimals_lp failure
3 participants