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

Fix #752: Reuse _computeSlot result in _accountDelta #753

Merged
merged 8 commits into from
Jul 15, 2024

Conversation

wjmelements
Copy link
Contributor

Reviewers @snreynolds @marktoda @hensha256
Fixes #752

Changes

  • Reuse _computeSlot result in _accountDelta by combining getDelta and setDelta via applyDelta

@wjmelements
Copy link
Contributor Author

This saves a few hundred gas now but it will be a few thousand gas after https://eips.ethereum.org/EIPS/eip-7667

@wjmelements wjmelements changed the title Fix #752 Fix #752: Reuse _computeSlot result in _accountDelta Jun 26, 2024
@hensha256
Copy link
Contributor

Sorry for the delay we've been having audits so the code was frozen. We are starting another set of audits this week so lets try to merge this... today if possible?
Please could you fix linting and add a little natspec?

@hensha256
Copy link
Contributor

Nice thanks for doing that. I've asked @snreynolds to take a look too which hopefully she will do today.

@wjmelements
Copy link
Contributor Author

The coverage error is 403 unauthorized.

Error: Unhandled error: HttpError: Resource not accessible by integration

It appears that you need to authorize comment-forge-coverage to comment on Github.

Here is what it was trying to post:

Forge code coverage:

File % Lines % Statements % Branches % Funcs
src/ERC6909.sol 91.30% (21/23) 85.71% (24/28) 100.00% (4/4) 85.71% (6/7)
src/ERC6909Claims.sol 100.00% (6/6) 100.00% (8/8) 100.00% (4/4) 100.00% (1/1)
src/Extsload.sol 100.00% (0/0) 100.00% (0/0) 100.00% (0/0) 100.00% (3/3)
src/Exttload.sol 100.00% (0/0) 100.00% (0/0) 100.00% (0/0) 50.00% (1/2)
src/NoDelegateCall.sol 100.00% (3/3) 100.00% (5/5) 100.00% (2/2) 100.00% (3/3)
src/PoolManager.sol 100.00% (81/81) 98.20% (109/111) 100.00% (34/34) 100.00% (17/17)
src/ProtocolFees.sol 100.00% (20/20) 97.06% (33/34) 100.00% (12/12) 100.00% (6/6)
src/libraries/BitMath.sol 100.00% (47/47) 100.00% (57/57) 100.00% (36/36) 100.00% (2/2)
src/libraries/CurrencyDelta.sol 100.00% (4/4) 100.00% (5/5) 100.00% (0/0) 100.00% (2/2)
src/libraries/CustomRevert.sol 100.00% (0/0) 100.00% (0/0) 100.00% (0/0) 100.00% (6/6)
src/libraries/FullMath.sol 100.00% (28/28) 100.00% (32/32) 100.00% (8/8) 100.00% (2/2)
src/libraries/Hooks.sol 100.00% (77/77) 100.00% (139/139) 100.00% (56/56) 100.00% (14/14)
src/libraries/LPFeeLibrary.sol 100.00% (10/10) 100.00% (17/17) 100.00% (4/4) 100.00% (7/7)
src/libraries/LiquidityMath.sol 100.00% (2/2) 100.00% (1/1) 100.00% (1/1) 100.00% (1/1)
src/libraries/Lock.sol 100.00% (1/1) 100.00% (1/1) 100.00% (0/0) 100.00% (3/3)
src/libraries/NonZeroDeltaCount.sol 100.00% (3/3) 100.00% (3/3) 100.00% (0/0) 100.00% (3/3)
src/libraries/ParseBytes.sol 100.00% (3/3) 100.00% (3/3) 100.00% (0/0) 100.00% (3/3)
src/libraries/Pool.sol 99.35% (153/154) 99.42% (170/171) 100.00% (86/86) 100.00% (13/13)
src/libraries/Position.sol 100.00% (11/11) 100.00% (12/12) 100.00% (4/4) 100.00% (2/2)
src/libraries/ProtocolFeeLibrary.sol 100.00% (4/4) 100.00% (6/6) 100.00% (0/0) 100.00% (4/4)
src/libraries/Reserves.sol 100.00% (7/7) 100.00% (12/12) 100.00% (6/6) 100.00% (3/3)
src/libraries/SafeCast.sol 100.00% (10/10) 100.00% (26/26) 100.00% (10/10) 100.00% (6/6)
src/libraries/SqrtPriceMath.sol 100.00% (37/37) 100.00% (53/53) 88.24% (15/17) 100.00% (9/9)
src/libraries/StateLibrary.sol 100.00% (65/65) 100.00% (94/94) 100.00% (4/4) 100.00% (14/14)
src/libraries/SwapMath.sol 100.00% (22/22) 100.00% (24/24) 100.00% (6/6) 100.00% (2/2)
src/libraries/TickBitmap.sol 100.00% (17/17) 100.00% (23/23) 100.00% (3/3) 100.00% (4/4)
src/libraries/TickMath.sol 100.00% (95/95) 100.00% (140/140) 100.00% (43/43) 100.00% (4/4)
src/libraries/TransientStateLibrary.sol 87.50% (7/8) 84.62% (11/13) 100.00% (0/0) 75.00% (3/4)
src/libraries/UnsafeMath.sol 100.00% (1/1) 100.00% (1/1) 100.00% (0/0) 100.00% (1/1)
src/types/BalanceDelta.sol 100.00% (2/2) 100.00% (2/2) 100.00% (0/0) 100.00% (2/2)
src/types/BeforeSwapDelta.sol 100.00% (2/2) 100.00% (2/2) 100.00% (0/0) 100.00% (2/2)
src/types/Currency.sol 100.00% (12/12) 94.12% (16/17) 100.00% (8/8) 100.00% (6/6)
src/types/PoolId.sol 100.00% (1/1) 100.00% (1/1) 100.00% (0/0) 100.00% (1/1)
src/types/Slot0.sol 100.00% (8/8) 100.00% (8/8) 100.00% (0/0) 100.00% (8/8)
Total 84.08% (1479/1759) 83.80% (1991/2376) 81.08% (587/724) 77.73% (356/458)

@hensha256
Copy link
Contributor

Yep no worries! Its just because youre working from a fork branch and they cannot access our github secrets :)

src/libraries/CurrencyDelta.sol Outdated Show resolved Hide resolved
src/PoolManager.sol Outdated Show resolved Hide resolved
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.

lgtm

@hensha256 hensha256 merged commit 1f4e8b6 into Uniswap:main Jul 15, 2024
5 of 6 checks passed
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.

_accountDelta duplicates _computeSlot
3 participants