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

Save gas and clean the upper bits of computed pool address properly #291

Open
wants to merge 10 commits into
base: main
Choose a base branch
from
41 changes: 41 additions & 0 deletions .gas-snapshot
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
UniversalRouterTest:testCallModule() (gas: 6639)
UniversalRouterTest:testSupportsInterface() (gas: 9045)
UniversalRouterTest:testSweepERC1155() (gas: 70732)
UniversalRouterTest:testSweepERC1155InsufficientOutput() (gas: 60170)
UniversalRouterTest:testSweepERC1155NotFullAmount() (gas: 70713)
UniversalRouterTest:testSweepETH() (gas: 58089)
UniversalRouterTest:testSweepETHInsufficientOutput() (gas: 76030)
UniversalRouterTest:testSweepToken() (gas: 82345)
UniversalRouterTest:testSweepTokenInsufficientOutput() (gas: 82056)
V2DaiWeth:testExactInput0For1() (gas: 107336)
V2DaiWeth:testExactInput0For1FromRouter() (gas: 249776)
V2DaiWeth:testExactInput1For0() (gas: 107408)
V2DaiWeth:testExactInput1For0FromRouter() (gas: 249939)
V2DaiWeth:testExactOutput0For1() (gas: 108235)
V2DaiWeth:testExactOutput0For1FromRouter() (gas: 250864)
V2DaiWeth:testExactOutput1For0() (gas: 108659)
V2DaiWeth:testExactOutput1For0FromRouter() (gas: 251105)
V2MockMock:testExactInput0For1() (gas: 106677)
V2MockMock:testExactInput0For1FromRouter() (gas: 249531)
V2MockMock:testExactInput1For0() (gas: 106636)
V2MockMock:testExactInput1For0FromRouter() (gas: 249639)
V2MockMock:testExactOutput0For1() (gas: 106852)
V2MockMock:testExactOutput0For1FromRouter() (gas: 250050)
V2MockMock:testExactOutput1For0() (gas: 107027)
V2MockMock:testExactOutput1For0FromRouter() (gas: 250137)
V2MockWeth:testExactInput0For1() (gas: 101507)
V2MockWeth:testExactInput0For1FromRouter() (gas: 245121)
V2MockWeth:testExactInput1For0() (gas: 101553)
V2MockWeth:testExactInput1For0FromRouter() (gas: 245575)
V2MockWeth:testExactOutput0For1() (gas: 101685)
V2MockWeth:testExactOutput0For1FromRouter() (gas: 245640)
V2MockWeth:testExactOutput1For0() (gas: 102817)
V2MockWeth:testExactOutput1For0FromRouter() (gas: 246750)
V2WethApe:testExactInput0For1() (gas: 112115)
V2WethApe:testExactInput0For1FromRouter() (gas: 249479)
V2WethApe:testExactInput1For0() (gas: 106933)
V2WethApe:testExactInput1For0FromRouter() (gas: 249595)
V2WethApe:testExactOutput0For1() (gas: 113146)
V2WethApe:testExactOutput0For1FromRouter() (gas: 250672)
V2WethApe:testExactOutput1For0() (gas: 108132)
V2WethApe:testExactOutput1For0FromRouter() (gas: 250728)
25 changes: 16 additions & 9 deletions contracts/modules/uniswap/v2/UniswapV2Library.sol
Original file line number Diff line number Diff line change
Expand Up @@ -52,15 +52,22 @@ library UniswapV2Library {
pure
returns (address pair)
{
pair = address(
uint160(
uint256(
keccak256(
abi.encodePacked(hex'ff', factory, keccak256(abi.encodePacked(token0, token1)), initCodeHash)
)
)
)
);
assembly ("memory-safe") {
Copy link
Member

@ewilz ewilz May 8, 2023

Choose a reason for hiding this comment

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

I think I like the gas savings here. Wonder if it's worth a comment:

// accomplishes the following:
// address(keccak256(abi.encodePacked(hex'ff', factory, keccak256(abi.encodePacked(token0, token1)), initCodeHash)))

to get a feel for what this is doing at a glance

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added.

Copy link
Member

Choose a reason for hiding this comment

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

thanks, I think that's pretty helpful for readability, one for the V3Lib would probably make sense too. (sorry, missed that)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added.

// Get the free memory pointer.
let fmp := mload(0x40)
// keccak256(abi.encodePacked(token0, token1))
mstore(add(fmp, 0x14), token1)
mstore(fmp, token0)
let pairHash := keccak256(add(fmp, 0x0c), 0x28)
// abi.encodePacked(hex'ff', factory, pairHash, initCodeHash)
mstore(fmp, factory)
fmp := add(fmp, 0x0b)
mstore8(fmp, 0xff)
mstore(add(fmp, 0x15), pairHash)
mstore(add(fmp, 0x35), initCodeHash)
// Compute the CREATE2 pair address and clean the upper bits.
pair := and(keccak256(fmp, 0x55), 0xffffffffffffffffffffffffffffffffffffffff)
}
}

/// @notice Calculates the v2 address for a pair and fetches the reserves for each token
Expand Down
23 changes: 13 additions & 10 deletions contracts/modules/uniswap/v2/V2SwapRouter.sol
Original file line number Diff line number Diff line change
Expand Up @@ -23,19 +23,22 @@ abstract contract V2SwapRouter is RouterImmutables, Permit2Payments {
(address token0,) = UniswapV2Library.sortTokens(path[0], path[1]);
uint256 finalPairIndex = path.length - 1;
uint256 penultimatePairIndex = finalPairIndex - 1;
for (uint256 i; i < finalPairIndex; i++) {
(address input, address output) = (path[i], path[i + 1]);
(uint256 reserve0, uint256 reserve1,) = IUniswapV2Pair(pair).getReserves();
(uint256 reserveInput, uint256 reserveOutput) =
input == token0 ? (reserve0, reserve1) : (reserve1, reserve0);
uint256 amountInput = ERC20(input).balanceOf(pair) - reserveInput;
uint256 amountOutput = UniswapV2Library.getAmountOut(amountInput, reserveInput, reserveOutput);
(uint256 amount0Out, uint256 amount1Out) =
input == token0 ? (uint256(0), amountOutput) : (amountOutput, uint256(0));
for (uint256 i; i < finalPairIndex; ++i) {
address input = path[i];
uint256 amount0Out;
uint256 amount1Out;
{
(uint256 reserve0, uint256 reserve1,) = IUniswapV2Pair(pair).getReserves();
(uint256 reserveInput, uint256 reserveOutput) =
input == token0 ? (reserve0, reserve1) : (reserve1, reserve0);
uint256 amountInput = ERC20(input).balanceOf(pair) - reserveInput;
uint256 amountOutput = UniswapV2Library.getAmountOut(amountInput, reserveInput, reserveOutput);
(amount0Out, amount1Out) = input == token0 ? (uint256(0), amountOutput) : (amountOutput, uint256(0));
}
address nextPair;
(nextPair, token0) = i < penultimatePairIndex
? UniswapV2Library.pairAndToken0For(
UNISWAP_V2_FACTORY, UNISWAP_V2_PAIR_INIT_CODE_HASH, output, path[i + 2]
UNISWAP_V2_FACTORY, UNISWAP_V2_PAIR_INIT_CODE_HASH, path[i + 1], path[i + 2]
)
: (recipient, address(0));
IUniswapV2Pair(pair).swap(amount0Out, amount1Out, nextPair, new bytes(0));
Expand Down
3 changes: 1 addition & 2 deletions contracts/modules/uniswap/v3/BytesLib.sol
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
// SPDX-License-Identifier: GPL-3.0-or-later

/// @title Library for Bytes Manipulation
pragma solidity ^0.8.0;

import {Constants} from '../../../libraries/Constants.sol';

/// @title Library for Bytes Manipulation
library BytesLib {
error SliceOutOfBounds();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,6 @@
exports[`Cryptopunks purchases 1 cryptopunk gas 1`] = `
Object {
"calldataByteLength": 356,
"gasUsed": 109587,
"gasUsed": 109576,
}
`;
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,6 @@
exports[`NFTX Gas Tests gas: buyAndRedeem w/ specific selection 1`] = `
Object {
"calldataByteLength": 964,
"gasUsed": 574870,
"gasUsed": 574873,
}
`;
Original file line number Diff line number Diff line change
Expand Up @@ -10,42 +10,42 @@ Object {
exports[`Payments Gas Tests Individual Command Tests gas: SWEEP with ERC20 1`] = `
Object {
"calldataByteLength": 356,
"gasUsed": 39235,
"gasUsed": 39247,
}
`;

exports[`Payments Gas Tests Individual Command Tests gas: SWEEP_WITH_FEE 1`] = `
Object {
"calldataByteLength": 516,
"gasUsed": 67925,
"gasUsed": 67949,
}
`;

exports[`Payments Gas Tests Individual Command Tests gas: TRANSFER with ERC20 1`] = `
Object {
"calldataByteLength": 356,
"gasUsed": 38248,
"gasUsed": 38260,
}
`;

exports[`Payments Gas Tests Individual Command Tests gas: TRANSFER with ETH 1`] = `
Object {
"calldataByteLength": 356,
"gasUsed": 33821,
"gasUsed": 33833,
}
`;

exports[`Payments Gas Tests Individual Command Tests gas: UNWRAP_WETH 1`] = `
Object {
"calldataByteLength": 324,
"gasUsed": 46765,
"gasUsed": 46768,
}
`;

exports[`Payments Gas Tests Individual Command Tests gas: UNWRAP_WETH_WITH_FEE 1`] = `
Object {
"calldataByteLength": 644,
"gasUsed": 53127,
"gasUsed": 53154,
}
`;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
exports[`Seaport v1.4 Gas Tests ERC20 -> NFT gas: fulfillAdvancedOrder 1`] = `
Object {
"calldataByteLength": 2532,
"gasUsed": 237135,
"gasUsed": 237147,
}
`;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
exports[`Seaport v1.5 Gas Tests ETH -> NFT gas: fulfillAdvancedOrder 1`] = `
Object {
"calldataByteLength": 2020,
"gasUsed": 152097,
"gasUsed": 152100,
}
`;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,14 @@
exports[`Sudoswap Gas Tests ERC20 -> NFT gas: purchases NFTs with FRAX ERC20 token already approved 1`] = `
Object {
"calldataByteLength": 1380,
"gasUsed": 289920,
"gasUsed": 289932,
}
`;

exports[`Sudoswap Gas Tests ERC20 -> NFT gas: purchases tokens 2402, 2509 of Based Ghoul with FRAX ERC20 token 1`] = `
Object {
"calldataByteLength": 1508,
"gasUsed": 311549,
"gasUsed": 311561,
}
`;

Expand Down
Loading