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.

Sign up for GitHub

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

Jump to bottom

gas optimization: leave one wei feature #303

Merged
merged 5 commits into from
Jan 16, 2024
Merged

gas optimization: leave one wei feature #303

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
75449d1
leave 1 wei in SOR
alex0207s Nov 29, 2023
c7cdde6
leave 1 wei in GS
alex0207s Nov 30, 2023
fb61d5f
remove the deal that send 1 wei to GS and SOR
alex0207s Dec 6, 2023
d4a2b55
fix typo and refine by comment
alex0207s Dec 7, 2023
d1e2f50
add a multi users test for leaving one wei feature and update some co…
alex0207s Jan 15, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 6 additions & 3 deletions contracts/GenericSwap.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,16 +67,19 @@ contract GenericSwap is IGenericSwap, TokenCollector, EIP712 {

if (_inputToken.isETH()) {
if (msg.value != _swapData.takerTokenAmount) revert InvalidMsgValue();
}

if (!_inputToken.isETH()) {
} else {
if (msg.value != 0) revert InvalidMsgValue();
_collect(_inputToken, _authorizedUser, _swapData.maker, _swapData.takerTokenAmount, _takerTokenPermit);
}

IStrategy(_swapData.maker).executeStrategy{ value: msg.value }(_inputToken, _outputToken, _swapData.takerTokenAmount, _swapData.strategyData);

returnAmount = _outputToken.getBalance(address(this));
if (returnAmount > 1) {
unchecked {
--returnAmount;
}
}
if (returnAmount < _swapData.minMakerTokenAmount) revert InsufficientOutput();

_outputToken.transferTo(_swapData.recipient, returnAmount);
Expand Down
11 changes: 11 additions & 0 deletions contracts/SmartOrderStrategy.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,11 @@ contract SmartOrderStrategy is ISmartOrderStrategy, AdminManagement {
}
}
uint256 selfBalance = Asset.getBalance(outputToken, address(this));
if (selfBalance > 1) {
unchecked {
--selfBalance;
}
}
Asset.transferTo(outputToken, payable(genericSwap), selfBalance);
}

Expand All @@ -66,6 +71,12 @@ contract SmartOrderStrategy is ISmartOrderStrategy, AdminManagement {
// replace amount if ratio != 0
if (_inputRatio != 0) {
uint256 inputTokenBalance = IERC20(_inputToken).balanceOf(address(this));
// leaving one wei for gas optimization
if (inputTokenBalance > 1) {
unchecked {
--inputTokenBalance;
}
}

// calculate input amount if ratio should be applied
if (_inputRatio != Constant.BPS_MAX) {
Expand Down
1 change: 1 addition & 0 deletions contracts/libraries/GenericSwapData.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ struct GenericSwapData {
}

// solhint-disable-next-line func-visibility
// free functions cannot have function visibility
function getGSDataHash(GenericSwapData memory gsData) pure returns (bytes32) {
return
keccak256(
Expand Down
95 changes: 88 additions & 7 deletions test/forkMainnet/GenericSwap.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,17 +38,21 @@ contract GenericSwapTest is Test, Tokens, BalanceUtil, Permit2Helper, SigHelper
address strategyAdmin = makeAddr("strategyAdmin");
address allowanceTargetOwner = makeAddr("allowanceTargetOwner");
uint256 takerPrivateKey = uint256(1);
uint256 alicePrivateKey = uint256(2);
address taker = vm.addr(takerPrivateKey);
address alice = vm.addr(alicePrivateKey);
uint256 defaultExpiry = block.timestamp + 1;
address defaultInputToken = USDT_ADDRESS;
uint256 defaultInputAmount = 10 * 1e6;
address defaultOutputToken = DAI_ADDRESS;
address[] defaultPath = [defaultInputToken, defaultOutputToken];
uint24[] defaultV3Fees = [3000];
bytes defaultTakerPermit;
bytes alicePermit;
SmartOrderStrategy smartStrategy;
GenericSwap genericSwap;
GenericSwapData defaultGSData;
GenericSwapData aliceGSData;
MockStrategy mockStrategy;
AllowanceTarget allowanceTarget;

Expand All @@ -62,6 +66,7 @@ contract GenericSwapTest is Test, Tokens, BalanceUtil, Permit2Helper, SigHelper

genericSwap = new GenericSwap(UNISWAP_PERMIT2_ADDRESS, address(allowanceTarget));
smartStrategy = new SmartOrderStrategy(strategyAdmin, address(genericSwap), WETH_ADDRESS);

mockStrategy = new MockStrategy();
vm.prank(strategyAdmin);
address[] memory tokenList = new address[](1);
Expand All @@ -72,7 +77,7 @@ contract GenericSwapTest is Test, Tokens, BalanceUtil, Permit2Helper, SigHelper

IUniswapV3Quoter v3Quoter = IUniswapV3Quoter(UNISWAP_V3_QUOTER_ADDRESS);
bytes memory encodedPath = UniswapV3.encodePath(defaultPath, defaultV3Fees);
uint256 expectedOut = v3Quoter.quoteExactInput(encodedPath, defaultInputAmount);
uint256 expectedOut = v3Quoter.quoteExactInput(encodedPath, defaultInputAmount) - 2; // leaving 1 wei in GS and SOS separately
uint256 minOutputAmount = (expectedOut * 95) / 100; // default 5% slippage tolerance
bytes memory routerPayload = abi.encodeCall(
IUniswapSwapRouter02.exactInputSingle,
Expand Down Expand Up @@ -101,6 +106,8 @@ contract GenericSwapTest is Test, Tokens, BalanceUtil, Permit2Helper, SigHelper

deal(taker, 100 ether);
setTokenBalanceAndApprove(taker, UNISWAP_PERMIT2_ADDRESS, tokens, 100000);
deal(alice, 100 ether);
setTokenBalanceAndApprove(alice, UNISWAP_PERMIT2_ADDRESS, tokens, 100000);
deal(address(mockStrategy), 100 ether);
setTokenBalanceAndApprove(address(mockStrategy), UNISWAP_PERMIT2_ADDRESS, tokens, 100000);

Expand Down Expand Up @@ -156,16 +163,17 @@ contract GenericSwapTest is Test, Tokens, BalanceUtil, Permit2Helper, SigHelper
Snapshot memory makerMakerToken = BalanceSnapshot.take({ owner: address(mockStrategy), token: gsData.makerToken });

uint256 actualOutput = 900;
uint256 realChangedInGS = actualOutput - 1; // leaving 1 wei in GS

// 800 < 900 < 1000
mockStrategy.setOutputAmountAndRecipient(actualOutput, payable(address(genericSwap)));
vm.expectEmit(true, true, true, true);
emit Swap(getGSDataHash(gsData), gsData.maker, taker, taker, gsData.takerToken, gsData.takerTokenAmount, gsData.makerToken, actualOutput);
emit Swap(getGSDataHash(gsData), gsData.maker, taker, taker, gsData.takerToken, gsData.takerTokenAmount, gsData.makerToken, realChangedInGS);
vm.prank(taker);
genericSwap.executeSwap(gsData, defaultTakerPermit);

takerTakerToken.assertChange(-int256(gsData.takerTokenAmount));
takerMakerToken.assertChange(int256(actualOutput));
takerMakerToken.assertChange(int256(realChangedInGS));
makerTakerToken.assertChange(int256(gsData.takerTokenAmount));
makerMakerToken.assertChange(-int256(actualOutput));
}
Expand All @@ -176,19 +184,21 @@ contract GenericSwapTest is Test, Tokens, BalanceUtil, Permit2Helper, SigHelper
gsData.takerToken = Constant.ETH_ADDRESS;
gsData.takerTokenAmount = 1 ether;

uint256 realChangedInGS = gsData.makerTokenAmount - 1; // leaving 1 wei in GS

Snapshot memory takerTakerToken = BalanceSnapshot.take({ owner: taker, token: gsData.takerToken });
Snapshot memory takerMakerToken = BalanceSnapshot.take({ owner: taker, token: gsData.makerToken });
Snapshot memory makerTakerToken = BalanceSnapshot.take({ owner: address(mockStrategy), token: gsData.takerToken });
Snapshot memory makerMakerToken = BalanceSnapshot.take({ owner: address(mockStrategy), token: gsData.makerToken });

mockStrategy.setOutputAmountAndRecipient(gsData.makerTokenAmount, payable(address(genericSwap)));
vm.expectEmit(true, true, true, true);
emit Swap(getGSDataHash(gsData), gsData.maker, taker, taker, gsData.takerToken, gsData.takerTokenAmount, gsData.makerToken, gsData.makerTokenAmount);
emit Swap(getGSDataHash(gsData), gsData.maker, taker, taker, gsData.takerToken, gsData.takerTokenAmount, gsData.makerToken, realChangedInGS);
vm.prank(taker);
genericSwap.executeSwap{ value: gsData.takerTokenAmount }(gsData, defaultTakerPermit);

takerTakerToken.assertChange(-int256(gsData.takerTokenAmount));
takerMakerToken.assertChange(int256(gsData.makerTokenAmount));
takerMakerToken.assertChange(int256(realChangedInGS));
makerTakerToken.assertChange(int256(gsData.takerTokenAmount));
makerMakerToken.assertChange(-int256(gsData.makerTokenAmount));
}
Expand All @@ -200,19 +210,21 @@ contract GenericSwapTest is Test, Tokens, BalanceUtil, Permit2Helper, SigHelper
gsData.makerTokenAmount = 1 ether;
gsData.minMakerTokenAmount = 1 ether - 1000;

uint256 realChangedInGS = gsData.makerTokenAmount - 1; // leaving 1 wei in GS

Snapshot memory takerTakerToken = BalanceSnapshot.take({ owner: taker, token: gsData.takerToken });
Snapshot memory takerMakerToken = BalanceSnapshot.take({ owner: taker, token: gsData.makerToken });
Snapshot memory makerTakerToken = BalanceSnapshot.take({ owner: address(mockStrategy), token: gsData.takerToken });
Snapshot memory makerMakerToken = BalanceSnapshot.take({ owner: address(mockStrategy), token: gsData.makerToken });

mockStrategy.setOutputAmountAndRecipient(gsData.makerTokenAmount, payable(address(genericSwap)));
vm.expectEmit(true, true, true, true);
emit Swap(getGSDataHash(gsData), gsData.maker, taker, taker, gsData.takerToken, gsData.takerTokenAmount, gsData.makerToken, gsData.makerTokenAmount);
emit Swap(getGSDataHash(gsData), gsData.maker, taker, taker, gsData.takerToken, gsData.takerTokenAmount, gsData.makerToken, realChangedInGS);
vm.prank(taker);
genericSwap.executeSwap(gsData, defaultTakerPermit);

takerTakerToken.assertChange(-int256(gsData.takerTokenAmount));
takerMakerToken.assertChange(int256(gsData.makerTokenAmount));
takerMakerToken.assertChange(int256(realChangedInGS));
makerTakerToken.assertChange(int256(gsData.takerTokenAmount));
makerMakerToken.assertChange(-int256(gsData.makerTokenAmount));
}
Expand Down Expand Up @@ -306,4 +318,73 @@ contract GenericSwapTest is Test, Tokens, BalanceUtil, Permit2Helper, SigHelper
vm.expectRevert(IGenericSwap.AlreadyFilled.selector);
genericSwap.executeSwapWithSig(defaultGSData, defaultTakerPermit, taker, takerSig);
}

function testLeaveOneWeiWithMultipleUsers() public {
Snapshot memory takerTakerToken = BalanceSnapshot.take({ owner: taker, token: defaultGSData.takerToken });
Snapshot memory takerMakerToken = BalanceSnapshot.take({ owner: taker, token: defaultGSData.makerToken });
Snapshot memory gsTakerToken = BalanceSnapshot.take({ owner: address(genericSwap), token: defaultGSData.takerToken });
Snapshot memory gsMakerToken = BalanceSnapshot.take({ owner: address(genericSwap), token: defaultGSData.makerToken });
Snapshot memory makerTakerToken = BalanceSnapshot.take({ owner: defaultGSData.maker, token: defaultGSData.takerToken });
Snapshot memory makerMakerToken = BalanceSnapshot.take({ owner: defaultGSData.maker, token: defaultGSData.makerToken });

// the first user: taker
// his makerTokenAmount has already been reduced by 2 in the setup function
// leaving 1 wei in GS and SOS separately
vm.expectEmit(true, true, true, true);
emit Swap(
getGSDataHash(defaultGSData),
defaultGSData.maker,
taker,
taker,
defaultGSData.takerToken,
defaultGSData.takerTokenAmount,
defaultGSData.makerToken,
defaultGSData.makerTokenAmount
);

vm.prank(taker);
genericSwap.executeSwap(defaultGSData, defaultTakerPermit);

// the second user: Alice
// his makerTokenAmount is recalculate by `quoteExactInput() function base on the current state`
// but there is no need to reduce it by 2 this time
aliceGSData = defaultGSData;

IUniswapV3Quoter v3Quoter = IUniswapV3Quoter(UNISWAP_V3_QUOTER_ADDRESS);
bytes memory encodedPath = UniswapV3.encodePath(defaultPath, defaultV3Fees);
uint256 aliceExpectedOut = v3Quoter.quoteExactInput(encodedPath, defaultInputAmount);

aliceGSData.recipient = payable(alice);
aliceGSData.makerTokenAmount = aliceExpectedOut;
alicePermit = getTokenlonPermit2Data(alice, alicePrivateKey, aliceGSData.takerToken, address(genericSwap));

Snapshot memory aliceTakerToken = BalanceSnapshot.take({ owner: alice, token: aliceGSData.takerToken });
Snapshot memory aliceMakerToken = BalanceSnapshot.take({ owner: alice, token: aliceGSData.makerToken });

vm.expectEmit(true, true, true, true);

emit Swap(
getGSDataHash(aliceGSData),
aliceGSData.maker,
alice,
alice,
aliceGSData.takerToken,
aliceGSData.takerTokenAmount,
aliceGSData.makerToken,
aliceGSData.makerTokenAmount
);

vm.startPrank(alice);
genericSwap.executeSwap(aliceGSData, alicePermit);
vm.stopPrank();

takerTakerToken.assertChange(-int256(defaultGSData.takerTokenAmount));
takerMakerToken.assertChange(int256(defaultGSData.makerTokenAmount));
aliceTakerToken.assertChange(-int256(aliceGSData.takerTokenAmount));
aliceMakerToken.assertChange(int256(aliceGSData.makerTokenAmount));
gsTakerToken.assertChange(0);
gsMakerToken.assertChange(1);
makerTakerToken.assertChange(0);
makerMakerToken.assertChange(1);
}
}
23 changes: 12 additions & 11 deletions test/forkMainnet/SmartOrderStrategy/AMMs.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ contract AMMsTest is SmartOrderStrategyTest {

// get the exact quote from uniswap
uint256 expectedOut = v3Quoter.quoteExactInput(encodedUniv3Path, defaultInputAmount);
uint256 realChangedInGS = expectedOut - 1;

vm.startPrank(genericSwap, genericSwap);
IERC20(defaultInputToken).safeTransfer(address(smartOrderStrategy), defaultInputAmount);
Expand All @@ -55,7 +56,7 @@ contract AMMsTest is SmartOrderStrategyTest {
vm.stopPrank();

sosInputToken.assertChange(-int256(defaultInputAmount));
gsOutputToken.assertChange(int256(expectedOut));
gsOutputToken.assertChange(int256(realChangedInGS));
}

function testUniswapV3WithAmountReplace() public {
Expand Down Expand Up @@ -85,18 +86,16 @@ contract AMMsTest is SmartOrderStrategyTest {
bytes memory data = abi.encode(operations);

// get the exact quote from uniswap
uint256 inputAmountAfterRatio = (defaultInputAmount * defaultInputRatio) / Constant.BPS_MAX;
uint256 inputAmountAfterRatio = ((defaultInputAmount - 1) * defaultInputRatio) / Constant.BPS_MAX;
uint256 expectedOut = v3Quoter.quoteExactInput(encodedUniv3Path, inputAmountAfterRatio);

vm.startPrank(genericSwap, genericSwap);
IERC20(defaultInputToken).safeTransfer(address(smartOrderStrategy), defaultInputAmount);
Snapshot memory sosInputToken = BalanceSnapshot.take(address(smartOrderStrategy), defaultInputToken);
Snapshot memory gsOutputToken = BalanceSnapshot.take(genericSwap, defaultOutputToken);
smartOrderStrategy.executeStrategy(defaultInputToken, defaultOutputToken, defaultInputAmount, data);
vm.stopPrank();

sosInputToken.assertChange(-int256(inputAmountAfterRatio));
gsOutputToken.assertChange(int256(expectedOut));
gsOutputToken.assertChange(int256(expectedOut - 1));
}

function testUniswapV3WithMaxAmountReplace() public {
Expand Down Expand Up @@ -129,7 +128,7 @@ contract AMMsTest is SmartOrderStrategyTest {
uint256 actualInputAmount = 5678;

// get the exact quote from uniswap
uint256 expectedOut = v3Quoter.quoteExactInput(encodedUniv3Path, actualInputAmount);
uint256 expectedOut = v3Quoter.quoteExactInput(encodedUniv3Path, actualInputAmount - 1);

vm.startPrank(genericSwap, genericSwap);
IERC20(defaultInputToken).safeTransfer(address(smartOrderStrategy), actualInputAmount);
Expand All @@ -139,8 +138,8 @@ contract AMMsTest is SmartOrderStrategyTest {
vm.stopPrank();

// the amount change will be the actual balance at the moment
sosInputToken.assertChange(-int256(actualInputAmount));
gsOutputToken.assertChange(int256(expectedOut));
sosInputToken.assertChange(-int256(actualInputAmount - 1)); // leaving 1 wei in SOS
gsOutputToken.assertChange(int256(expectedOut - 1)); // leaving 1 wei in GS
}

function testUniswapV2WithWETHUnwrap() public {
Expand Down Expand Up @@ -171,6 +170,7 @@ contract AMMsTest is SmartOrderStrategyTest {

// get the exact quote from uniswap
uint256 expectedOut = v3Quoter.quoteExactInput(encodedUniv3Path, defaultInputAmount);
uint256 realChangedInGS = expectedOut - 1;

// set output token as ETH
address outputToken = Constant.ETH_ADDRESS;
Expand All @@ -182,7 +182,7 @@ contract AMMsTest is SmartOrderStrategyTest {
vm.stopPrank();

sosInputToken.assertChange(-int256(defaultInputAmount));
gsOutputToken.assertChange(int256(expectedOut));
gsOutputToken.assertChange(int256(realChangedInGS));
}

function testMultipleAMMs() public {
Expand All @@ -208,10 +208,11 @@ contract AMMsTest is SmartOrderStrategyTest {
)
);

// exhange function selector : 0x5b41b908
// exchange function selector : 0x5b41b908
bytes memory curveData = abi.encodeWithSelector(0x5b41b908, 2, 0, uniOut, 0);
ICurveFiV2 curvePool = ICurveFiV2(CURVE_TRICRYPTO2_POOL_ADDRESS);
uint256 curveOut = curvePool.get_dy(2, 0, uniOut);
uint256 realChangedInGS = curveOut - 1;

ISmartOrderStrategy.Operation[] memory operations = new ISmartOrderStrategy.Operation[](2);
operations[0] = ISmartOrderStrategy.Operation({
Expand Down Expand Up @@ -240,6 +241,6 @@ contract AMMsTest is SmartOrderStrategyTest {
vm.stopPrank();

sosInputToken.assertChange(-int256(defaultInputAmount));
gsOutputToken.assertChange(int256(curveOut));
gsOutputToken.assertChange(int256(realChangedInGS));
}
}
10 changes: 8 additions & 2 deletions test/forkMainnet/SmartOrderStrategy/IntegrationV6.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,9 @@ contract IntegrationV6Test is SmartOrderStrategyTest, SigHelper {
expiry: defaultExpiry,
salt: defaultSalt
});

uint256 realChangedInGS = rfqOffer.makerTokenAmount - 1; // leaving 1 wei in GS

RFQTx memory rfqTx = RFQTx({ rfqOffer: rfqOffer, takerRequestAmount: rfqOffer.takerTokenAmount, recipient: payable(address(smartOrderStrategy)) });
bytes memory makerSig = signRFQOffer(makerPrivateKey, rfqOffer, address(rfq));
bytes memory rfqData = abi.encodeWithSelector(RFQ_FILL_SELECTOR, rfqTx, makerSig, defaultPermit, defaultPermit);
Expand All @@ -87,7 +90,7 @@ contract IntegrationV6Test is SmartOrderStrategyTest, SigHelper {
vm.stopPrank();

sosInputToken.assertChange(-int256(rfqOffer.takerTokenAmount));
gsOutputToken.assertChange(int256(rfqOffer.makerTokenAmount));
gsOutputToken.assertChange(int256(realChangedInGS));
}

function testV6LOIntegration() public {
Expand All @@ -103,6 +106,9 @@ contract IntegrationV6Test is SmartOrderStrategyTest, SigHelper {
expiry: defaultExpiry,
salt: defaultSalt
});

uint256 realChangedInGS = order.makerTokenAmount - 1; // leaving 1 wei in GS

bytes memory makerSig = signLimitOrder(makerPrivateKey, order, address(limitOrderSwap));
ILimitOrderSwap.TakerParams memory takerParams = ILimitOrderSwap.TakerParams({
takerTokenAmount: order.takerTokenAmount,
Expand Down Expand Up @@ -132,6 +138,6 @@ contract IntegrationV6Test is SmartOrderStrategyTest, SigHelper {
vm.stopPrank();

sosInputToken.assertChange(-int256(order.takerTokenAmount));
gsOutputToken.assertChange(int256(order.makerTokenAmount));
gsOutputToken.assertChange(int256(realChangedInGS));
}
}
Loading