Skip to content

Change to always underestimate in YieldSpaceMath.sol#636

Merged
jalextowle merged 10 commits intomainfrom
jalextowle/math/yield-space-overestimation
Oct 27, 2023
Merged

Change to always underestimate in YieldSpaceMath.sol#636
jalextowle merged 10 commits intomainfrom
jalextowle/math/yield-space-overestimation

Conversation

@jalextowle
Copy link
Copy Markdown
Contributor

@jalextowle jalextowle commented Oct 25, 2023

Prior to this PR, several of the YieldSpaceMath functions would overestimate some of the intermediate values and the final result. This PR switches these cases to underestimate instead of overestimate.

jrhea
jrhea reviewed Oct 25, 2023
View reviewed changes
Comment thread contracts/src/libraries/YieldSpaceMath.sol Outdated
Comment thread contracts/src/libraries/YieldSpaceMath.sol
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Oct 25, 2023 •
edited
Loading

Hyperdrive Gas Benchmark

Benchmark suite Current: d92575d Previous: c937804 Deviation Status
addLiquidity: min 733 gas 733 gas 0% 🟰
addLiquidity: avg 54041 gas 56856 gas -4.9511% ✅
addLiquidity: max 96720 gas 97520 gas -0.8203% ✅
checkpoint: min 558 gas 558 gas 0% 🟰
checkpoint: avg 47634 gas 30349 gas 56.9541% 🚨
checkpoint: max 99410 gas 79983 gas 24.2889% 🚨
closeLong: min 755 gas 755 gas 0% 🟰
closeLong: avg 23526 gas 23271 gas 1.0958% 🚨
closeLong: max 114183 gas 106145 gas 7.5727% 🚨
closeShort: min 802 gas 802 gas 0% 🟰
closeShort: avg 24919 gas 24825 gas 0.3787% 🚨
closeShort: max 111483 gas 105555 gas 5.6160% 🚨
initialize: min 662 gas 662 gas 0% 🟰
initialize: avg 177322 gas 161026 gas 10.1201% 🚨
initialize: max 251633 gas 234375 gas 7.3634% 🚨
openLong: min 757 gas 757 gas 0% 🟰
openLong: avg 55290 gas 56003 gas -1.2731% ✅
openLong: max 216368 gas 225599 gas -4.0918% ✅
openShort: min 712 gas 712 gas 0% 🟰
openShort: avg 56183 gas 56568 gas -0.6806% ✅
openShort: max 217803 gas 220880 gas -1.3931% ✅
removeLiquidity: min 777 gas 777 gas 0% 🟰
removeLiquidity: avg 75980 gas 76709 gas -0.9503% ✅
removeLiquidity: max 203187 gas 184507 gas 10.1243% 🚨

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

@coveralls
Copy link
Copy Markdown
Collaborator

coveralls commented Oct 25, 2023 •
edited
Loading

Coverage Status

coverage: 97.29% (+4.7%) from 92.632% when pulling d92575d on jalextowle/math/yield-space-overestimation into e937090 on main.

jrhea
jrhea reviewed Oct 25, 2023
View reviewed changes
Comment thread contracts/src/libraries/YieldSpaceMath.sol Outdated
jrhea
jrhea previously approved these changes Oct 25, 2023
View reviewed changes
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

@jrhea jrhea dismissed their stale review October 26, 2023 14:43

PR is still in work

jrhea
jrhea approved these changes Oct 27, 2023
View reviewed changes
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.

well done

Comment thread crates/hyperdrive-math/src/yield_space.rs Outdated
@jalextowle jalextowle force-pushed the jalextowle/math/yield-space-overestimation branch from 5cd8d1a to d92575d Compare October 27, 2023 16:23
@jalextowle jalextowle enabled auto-merge (squash) October 27, 2023 16:33
@jalextowle jalextowle merged commit e999f56 into main Oct 27, 2023
@jalextowle jalextowle deleted the jalextowle/math/yield-space-overestimation branch October 27, 2023 17:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jrhea jrhea jrhea approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@jalextowle @coveralls @jrhea