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

Remove mandatory viaIR #872

Merged
merged 13 commits into from
Sep 5, 2024
Merged

Remove mandatory viaIR #872

merged 13 commits into from
Sep 5, 2024

Conversation

saucepoint
Copy link
Collaborator

Related Issue

Mandatory viaIR sucks 💯

Description of changes

  • Killed myself in Pool.sol

Copy link

github-actions bot commented Sep 5, 2024

Forge code coverage:

File % Lines % Statements % Branches % Funcs
src/ERC6909.sol 82.61% (19/23) 78.57% (22/28) 100.00% (2/2) 85.71% (6/7)
src/ERC6909Claims.sol 100.00% (6/6) 100.00% (8/8) 100.00% (2/2) 100.00% (1/1)
src/Extsload.sol 0.00% (0/28) 0.00% (0/30) 0.00% (0/2) 100.00% (3/3)
src/Exttload.sol 0.00% (0/15) 0.00% (0/16) 0.00% (0/1) 50.00% (1/2)
src/NoDelegateCall.sol 66.67% (2/3) 75.00% (3/4) 100.00% (1/1) 100.00% (3/3)
src/PoolManager.sol 98.98% (97/98) 97.78% (132/135) 90.48% (19/21) 100.00% (20/20)
src/ProtocolFees.sol 87.50% (21/24) 87.88% (29/33) 83.33% (5/6) 100.00% (6/6)
src/libraries/BipsLibrary.sol 100.00% (2/2) 100.00% (4/4) 100.00% (1/1) 100.00% (1/1)
src/libraries/BitMath.sol 18.18% (2/11) 18.18% (2/11) 0.00% (0/4) 100.00% (2/2)
src/libraries/CurrencyDelta.sol 33.33% (3/9) 45.45% (5/11) 100.00% (0/0) 100.00% (3/3)
src/libraries/CurrencyReserves.sol 0.00% (0/5) 0.00% (0/5) 100.00% (0/0) 100.00% (4/4)
src/libraries/CustomRevert.sol 0.00% (0/35) 0.00% (0/35) 100.00% (0/0) 100.00% (8/8)
src/libraries/FullMath.sol 68.97% (20/29) 72.73% (24/33) 33.33% (2/6) 100.00% (2/2)
src/libraries/Hooks.sol 94.12% (80/85) 95.00% (133/140) 92.00% (23/25) 100.00% (14/14)
src/libraries/LPFeeLibrary.sol 90.00% (9/10) 93.75% (15/16) 100.00% (1/1) 100.00% (7/7)
src/libraries/LiquidityMath.sol 0.00% (0/4) 0.00% (0/4) 0.00% (0/1) 100.00% (1/1)
src/libraries/Lock.sol 0.00% (0/3) 0.00% (0/3) 100.00% (0/0) 100.00% (3/3)
src/libraries/NonzeroDeltaCount.sol 0.00% (0/7) 0.00% (0/7) 100.00% (0/0) 100.00% (3/3)
src/libraries/ParseBytes.sol 0.00% (0/3) 0.00% (0/3) 100.00% (0/0) 100.00% (3/3)
src/libraries/Pool.sol 94.16% (145/154) 93.45% (157/168) 98.08% (51/52) 100.00% (13/13)
src/libraries/Position.sol 52.63% (10/19) 57.14% (12/21) 100.00% (3/3) 100.00% (3/3)
src/libraries/ProtocolFeeLibrary.sol 20.00% (2/10) 20.00% (2/10) 100.00% (0/0) 100.00% (4/4)
src/libraries/SafeCast.sol 100.00% (12/12) 100.00% (19/19) 100.00% (6/6) 100.00% (6/6)
src/libraries/SqrtPriceMath.sol 60.42% (29/48) 69.84% (44/63) 58.33% (7/12) 100.00% (9/9)
src/libraries/StateLibrary.sol 71.43% (45/63) 79.31% (69/87) 100.00% (4/4) 100.00% (14/14)
src/libraries/SwapMath.sol 80.77% (21/26) 81.48% (22/27) 100.00% (6/6) 100.00% (2/2)
src/libraries/TickBitmap.sol 40.00% (12/30) 50.00% (18/36) 66.67% (2/3) 100.00% (4/4)
src/libraries/TickMath.sol 37.11% (36/97) 56.94% (82/144) 95.83% (23/24) 100.00% (4/4)
src/libraries/TransientStateLibrary.sol 70.00% (7/10) 76.92% (10/13) 100.00% (0/0) 100.00% (5/5)
src/libraries/UnsafeMath.sol 0.00% (0/2) 0.00% (0/2) 100.00% (0/0) 100.00% (2/2)
src/types/BalanceDelta.sol 0.00% (0/2) 0.00% (0/2) 100.00% (0/0) 100.00% (2/2)
src/types/BeforeSwapDelta.sol 0.00% (0/2) 0.00% (0/2) 100.00% (0/0) 100.00% (2/2)
src/types/Currency.sol 59.09% (13/22) 70.00% (21/30) 75.00% (6/8) 100.00% (6/6)
src/types/PoolId.sol 0.00% (0/1) 0.00% (0/1) 100.00% (0/0) 100.00% (1/1)
src/types/Slot0.sol 0.00% (0/8) 0.00% (0/8) 100.00% (0/0) 100.00% (8/8)
Total 68.69% (1349/1964) 71.43% (1813/2538) 42.35% (274/647) 82.98% (395/476)

@@ -265,13 +265,20 @@ library Pool {
}

struct SwapParams {
int256 amountSpecified;
Copy link
Member

Choose a reason for hiding this comment

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

does this ordering change matter?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

we removed the cached exactInput = params.amountSpecified < 0, and are now using params.amountSpecified < 0 in place of the cached variable

making it the first element in the struct avoids the need of offset calculations

snreynolds
snreynolds previously approved these changes Sep 5, 2024
Copy link
Member

@snreynolds snreynolds left a comment

Choose a reason for hiding this comment

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

i also do not like via-ir

// a swap fee totaling MAX_SWAP_FEE (100%) makes exact output swaps impossible since the input is entirely consumed by the fee
if (swapFee >= SwapMath.MAX_SWAP_FEE) {
if (!exactInput) {
// if not exactInput
if (!(params.amountSpecified < 0)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if (!(params.amountSpecified < 0)) {
if (params.amountSpecified >= 0) {

might be better?

@saucepoint saucepoint merged commit 18b223c into main Sep 5, 2024
6 checks passed
@saucepoint saucepoint deleted the i-hate-ir branch September 5, 2024 22:19
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