-
Notifications
You must be signed in to change notification settings - Fork 48
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
introduces a gas-optimized periphery contract for same-chain swaps (GenericSwapper v1.0.0) #710
base: main
Are you sure you want to change the base?
Changes from all commits
8cbbde6
f679471
6617874
6280e03
7eeb62e
c15eed0
a98e5c5
d9a98fd
7099249
f684505
1099596
4764d00
e1cbcdd
6337197
3ba9e54
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -26,5 +26,6 @@ | |
"ThorSwapFacet": "0xa6aAe470E7B8E8916e692882A5db25bB40C398A7", | ||
"AmarokFacetPacked": "0x7ac3EB2D191EBAb9E925CAbFD4F8155be066b3aa", | ||
"MayanBridgeFacet": "0x5Ba4FeD1DAd2fD057A9f687B399B8e4cF2368214", | ||
"MayanFacet": "0xd596C903d78870786c5DB0E448ce7F87A65A0daD" | ||
"MayanFacet": "0xd596C903d78870786c5DB0E448ce7F87A65A0daD", | ||
"GenericSwapFacetV3": "0xE871874D8AC30E8aCD0eC67529b4a5dDD73Bf0d6" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Codebase verification Version mismatch detected in deployed contract The deployed GenericSwapFacetV3 contract has version 1.0.1 (as shown in the source code comment 🔗 Analysis chainVerify contract deployment and version Please verify that the deployed GenericSwapFacetV3 contract at 🏁 Scripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Description: Verify contract deployment and version
# Check if contract is deployed and verify its bytecode
echo "Checking contract deployment..."
cast code 0xE871874D8AC30E8aCD0eC67529b4a5dDD73Bf0d6 --rpc-url https://bsc-testnet.public.blastapi.io
# Search for version string in contract source
echo "Checking contract version..."
rg -l "1\.0\.0" src/
Length of output: 1485 Script: #!/bin/bash
# Let's check the GenericSwapFacetV3 contract source and its version
echo "Searching for GenericSwapFacetV3 contract..."
fd "GenericSwapFacetV3"
echo "Checking version in GenericSwapFacetV3..."
rg -A 5 "contract GenericSwapFacetV3"
# Check if there are any version-related constants or variables
echo "Checking for version constants..."
rg "VERSION|version" --type solidity
Length of output: 2015 Script: #!/bin/bash
# Let's check the contract source and deployment scripts for version information
echo "Checking GenericSwapFacetV3 source and related files..."
rg -p "version|VERSION" src/Facets/GenericSwapFacetV3.sol script/deploy/facets/DeployGenericSwapFacetV3.s.sol script/deploy/facets/UpdateGenericSwapFacetV3.s.sol
# Let's also check the deployment script content to understand deployment configuration
echo "Checking deployment script content..."
cat script/deploy/facets/DeployGenericSwapFacetV3.s.sol
Length of output: 1732 |
||
} |
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,97 @@ | ||||||||||||||||||||||||||||||||||||||
# GenericSwapper | ||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||
## Description | ||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||
Gas-optimized periphery contract used for executing same-chain swaps (incl. an optional fee collection step) | ||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||
## How To Use | ||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||
This contract was designed to provide a heavily gas-optimized way to execute same-chain swaps. It will not emit any events and it can only use the LI.FI DEX Aggregator to execute these swaps. Other DEXs are not supported. | ||||||||||||||||||||||||||||||||||||||
It will also not check if token approvals from the GenericSwapper to the LI.FI DEX Aggregator and to the FeeCollector exist. If such approvals are missing, they would have to be set first (see function below). | ||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||
In order to still be able to trace transactions a `transactionId` will be appended to the calldata, separated from the actual calldata with a delimiter ('`0xdeadbeef`'). | ||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||
The contract has a number of specialized methods for various use cases: | ||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||
This method is used to execute a single swap from ERC20 to ERC20 | ||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||
```solidity | ||||||||||||||||||||||||||||||||||||||
/// @notice Performs a single swap from an ERC20 token to another ERC20 token | ||||||||||||||||||||||||||||||||||||||
/// @param _receiver the address to receive the swapped tokens into (also excess tokens) | ||||||||||||||||||||||||||||||||||||||
/// @param _minAmountOut the minimum amount of the final asset to receive | ||||||||||||||||||||||||||||||||||||||
/// @param _swapData an object containing swap related data to perform swaps before bridging | ||||||||||||||||||||||||||||||||||||||
function swapTokensSingleV3ERC20ToERC20( | ||||||||||||||||||||||||||||||||||||||
address _receiver, | ||||||||||||||||||||||||||||||||||||||
uint256 _minAmountOut, | ||||||||||||||||||||||||||||||||||||||
LibSwap.SwapData calldata _swapData | ||||||||||||||||||||||||||||||||||||||
) | ||||||||||||||||||||||||||||||||||||||
``` | ||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||
This method is used to execute a single swap from ERC20 to native | ||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||
```solidity | ||||||||||||||||||||||||||||||||||||||
/// @notice Performs a single swap from an ERC20 token to the network's native token | ||||||||||||||||||||||||||||||||||||||
/// @param _receiver the address to receive the swapped tokens into (also excess tokens) | ||||||||||||||||||||||||||||||||||||||
/// @param _minAmountOut the minimum amount of the final asset to receive | ||||||||||||||||||||||||||||||||||||||
/// @param _swapData an object containing swap related data to perform swaps before bridging | ||||||||||||||||||||||||||||||||||||||
function swapTokensSingleV3ERC20ToNative( | ||||||||||||||||||||||||||||||||||||||
address payable _receiver, | ||||||||||||||||||||||||||||||||||||||
uint256 _minAmountOut, | ||||||||||||||||||||||||||||||||||||||
LibSwap.SwapData calldata _swapData | ||||||||||||||||||||||||||||||||||||||
) | ||||||||||||||||||||||||||||||||||||||
``` | ||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||
This method is used to execute a single swap from native to ERC20 | ||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||
```solidity | ||||||||||||||||||||||||||||||||||||||
/// @notice Performs a single swap from the network's native token to ERC20 token | ||||||||||||||||||||||||||||||||||||||
/// @param _receiver the address to receive the swapped tokens into (also excess tokens) | ||||||||||||||||||||||||||||||||||||||
/// @param _minAmountOut the minimum amount of the final asset to receive | ||||||||||||||||||||||||||||||||||||||
/// @param _swapData an object containing swap related data to perform swaps before bridging | ||||||||||||||||||||||||||||||||||||||
function swapTokensSingleV3NativeToERC20( | ||||||||||||||||||||||||||||||||||||||
address payable _receiver, | ||||||||||||||||||||||||||||||||||||||
uint256 _minAmountOut, | ||||||||||||||||||||||||||||||||||||||
LibSwap.SwapData calldata _swapData | ||||||||||||||||||||||||||||||||||||||
) | ||||||||||||||||||||||||||||||||||||||
``` | ||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||
This method is used to execute a swap from ERC20 any other token (ERC20 or native) incl. a previous fee collection step | ||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||
```solidity | ||||||||||||||||||||||||||||||||||||||
/// @notice Performs multiple swaps in one transaction, starting with ERC20 and ending with native | ||||||||||||||||||||||||||||||||||||||
/// @param _receiver the address to receive the swapped tokens into (also excess tokens) | ||||||||||||||||||||||||||||||||||||||
/// @param _minAmountOut the minimum amount of the final asset to receive | ||||||||||||||||||||||||||||||||||||||
/// @param _swapData an object containing swap related data to perform swaps before bridging | ||||||||||||||||||||||||||||||||||||||
function swapTokensMultipleV3ERC20ToAny( | ||||||||||||||||||||||||||||||||||||||
address payable _receiver, | ||||||||||||||||||||||||||||||||||||||
uint256 _minAmountOut, | ||||||||||||||||||||||||||||||||||||||
LibSwap.SwapData[] calldata _swapData | ||||||||||||||||||||||||||||||||||||||
) | ||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||
Comment on lines
+61
to
+70
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Incorrect documentation for swapTokensMultipleV3ERC20ToAny The NatSpec comment states "ending with native" but the function name and description suggest it can end with any token type (ERC20 or native). - /// @notice Performs multiple swaps in one transaction, starting with ERC20 and ending with native
+ /// @notice Performs multiple swaps in one transaction, starting with ERC20 and ending with any token (ERC20 or native) 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||||
``` | ||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||
This method is used to execute a swap from native to ERC20 incl. a previous fee collection step | ||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||
```solidity | ||||||||||||||||||||||||||||||||||||||
/// @notice Performs multiple swaps in one transaction, starting with native and ending with ERC20 | ||||||||||||||||||||||||||||||||||||||
/// @param _receiver the address to receive the swapped tokens into (also excess tokens) | ||||||||||||||||||||||||||||||||||||||
/// @param _minAmountOut the minimum amount of the final asset to receive | ||||||||||||||||||||||||||||||||||||||
/// @param _swapData an object containing swap related data to perform swaps before bridging | ||||||||||||||||||||||||||||||||||||||
function swapTokensMultipleV3NativeToERC20( | ||||||||||||||||||||||||||||||||||||||
address payable _receiver, | ||||||||||||||||||||||||||||||||||||||
uint256 _minAmountOut, | ||||||||||||||||||||||||||||||||||||||
LibSwap.SwapData[] calldata _swapData | ||||||||||||||||||||||||||||||||||||||
) | ||||||||||||||||||||||||||||||||||||||
``` | ||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||
This method is used to set max approvals between the GenericSwapper and the DEXAggregator as well as the FeeCollector | ||||||||||||||||||||||||||||||||||||||
(it can only be called by the registered admin address) | ||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||
```solidity | ||||||||||||||||||||||||||||||||||||||
/// @notice (Re-)Sets max approvals from this contract to DEX Aggregator and FeeCollector | ||||||||||||||||||||||||||||||||||||||
/// @param _approvals The information which approvals to set for which token | ||||||||||||||||||||||||||||||||||||||
function setApprovalForTokens( | ||||||||||||||||||||||||||||||||||||||
TokenApproval[] calldata _approvals | ||||||||||||||||||||||||||||||||||||||
) | ||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||
``` |
Original file line number | Diff line number | Diff line change | ||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -3,13 +3,39 @@ pragma solidity ^0.8.17; | |||||||||||||||||||
|
||||||||||||||||||||
import { DeployScriptBase } from "./utils/DeployScriptBase.sol"; | ||||||||||||||||||||
import { GenericSwapFacetV3 } from "lifi/Facets/GenericSwapFacetV3.sol"; | ||||||||||||||||||||
import { stdJson } from "forge-std/Script.sol"; | ||||||||||||||||||||
|
||||||||||||||||||||
contract DeployScript is DeployScriptBase { | ||||||||||||||||||||
using stdJson for string; | ||||||||||||||||||||
|
||||||||||||||||||||
constructor() DeployScriptBase("GenericSwapFacetV3") {} | ||||||||||||||||||||
|
||||||||||||||||||||
function run() public returns (GenericSwapFacetV3 deployed) { | ||||||||||||||||||||
function run() | ||||||||||||||||||||
public | ||||||||||||||||||||
returns (GenericSwapFacetV3 deployed, bytes memory constructorArgs) | ||||||||||||||||||||
{ | ||||||||||||||||||||
constructorArgs = getConstructorArgs(); | ||||||||||||||||||||
|
||||||||||||||||||||
deployed = GenericSwapFacetV3( | ||||||||||||||||||||
deploy(type(GenericSwapFacetV3).creationCode) | ||||||||||||||||||||
); | ||||||||||||||||||||
} | ||||||||||||||||||||
|
||||||||||||||||||||
function getConstructorArgs() internal override returns (bytes memory) { | ||||||||||||||||||||
// get path of global config file | ||||||||||||||||||||
string memory globalConfigPath = string.concat( | ||||||||||||||||||||
root, | ||||||||||||||||||||
"/config/global.json" | ||||||||||||||||||||
); | ||||||||||||||||||||
|
||||||||||||||||||||
// read file into json variable | ||||||||||||||||||||
string memory globalConfigJson = vm.readFile(globalConfigPath); | ||||||||||||||||||||
|
||||||||||||||||||||
// extract network's native address | ||||||||||||||||||||
address nativeAddress = globalConfigJson.readAddress( | ||||||||||||||||||||
string.concat(".nativeAddress.", network) | ||||||||||||||||||||
); | ||||||||||||||||||||
Comment on lines
+34
to
+37
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add validation for native address The native address is critical for gas-optimized swaps and should be validated before deployment. Apply this diff: address nativeAddress = globalConfigJson.readAddress(
string.concat(".nativeAddress.", network)
);
+ require(nativeAddress != address(0), "DeployGenericSwapFacetV3: invalid native address"); 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||
|
||||||||||||||||||||
return abi.encode(nativeAddress); | ||||||||||||||||||||
} | ||||||||||||||||||||
} |
Original file line number | Diff line number | Diff line change | ||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,37 @@ | ||||||||||||||||
// SPDX-License-Identifier: UNLICENSED | ||||||||||||||||
pragma solidity ^0.8.17; | ||||||||||||||||
|
||||||||||||||||
import { DeployScriptBase } from "./utils/DeployScriptBase.sol"; | ||||||||||||||||
import { stdJson } from "forge-std/Script.sol"; | ||||||||||||||||
import { GenericSwapper } from "lifi/Periphery/GenericSwapper.sol"; | ||||||||||||||||
|
||||||||||||||||
contract DeployScript is DeployScriptBase { | ||||||||||||||||
using stdJson for string; | ||||||||||||||||
|
||||||||||||||||
constructor() DeployScriptBase("GenericSwapper") {} | ||||||||||||||||
|
||||||||||||||||
function run() | ||||||||||||||||
public | ||||||||||||||||
returns (GenericSwapper deployed, bytes memory constructorArgs) | ||||||||||||||||
{ | ||||||||||||||||
constructorArgs = getConstructorArgs(); | ||||||||||||||||
|
||||||||||||||||
deployed = GenericSwapper(deploy(type(GenericSwapper).creationCode)); | ||||||||||||||||
} | ||||||||||||||||
|
||||||||||||||||
function getConstructorArgs() internal override returns (bytes memory) { | ||||||||||||||||
string memory path = string.concat( | ||||||||||||||||
root, | ||||||||||||||||
"/deployments/", | ||||||||||||||||
network, | ||||||||||||||||
".", | ||||||||||||||||
fileSuffix, | ||||||||||||||||
"json" | ||||||||||||||||
); | ||||||||||||||||
string memory json = vm.readFile(path); | ||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Add error handling for JSON file reading The Apply this diff: - string memory json = vm.readFile(path);
+ string memory json;
+ try vm.readFile(path) returns (string memory result) {
+ json = result;
+ } catch {
+ revert("DeployGenericSwapper: failed to read deployment config");
+ } 📝 Committable suggestion
Suggested change
|
||||||||||||||||
address dexAggregatorAddress = json.readAddress(".LiFiDEXAggregator"); | ||||||||||||||||
address feeCollectorAddress = json.readAddress(".FeeCollector"); | ||||||||||||||||
|
||||||||||||||||
Comment on lines
+32
to
+34
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add address validation checks The constructor arguments should be validated to ensure they are non-zero addresses before deployment. Apply this diff to add validation: string memory json = vm.readFile(path);
address dexAggregatorAddress = json.readAddress(".LiFiDEXAggregator");
address feeCollectorAddress = json.readAddress(".FeeCollector");
+ require(dexAggregatorAddress != address(0), "DeployGenericSwapper: zero dexAggregator address");
+ require(feeCollectorAddress != address(0), "DeployGenericSwapper: zero feeCollector address"); 📝 Committable suggestion
Suggested change
|
||||||||||||||||
return abi.encode(dexAggregatorAddress, feeCollectorAddress); | ||||||||||||||||
} | ||||||||||||||||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix typo in Celo native address
The Celo native address has an erroneous "and" suffix which would make it an invalid address.
📝 Committable suggestion