Deploy: SwapQuoterV2 on Arbitrum/Avalanche
#266
Conversation
Pull Request Test Coverage Report for Build 6865200911
💛 - Coveralls |
|
The existing sections are already well-structured and provide a clear overview of the changes. I will proceed with crafting a whimsical poem to celebrate the changes:
The poem captures the spirit of change and progression, aligning with the evolving nature of the codebase. TipsChat with CodeRabbit Bot (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 13
Files ignored due to filter (13)
- deployments/arbitrum/SwapQuoterV2.json
- deployments/avalanche/DefaultPoolCalc.json
- deployments/avalanche/SwapQuoterV2.json
- deployments/mainnet/DefaultPoolCalc.json
- deployments/mainnet/SwapQuoterV2.json
- deployments/optimism/DefaultPoolCalc.json
- deployments/optimism/SwapQuoterV2.json
- script/configs/SwapQuoterV2.ignored.json
- script/configs/arbitrum/SwapQuoterV2.dc.json
- script/configs/avalanche/SwapQuoterV2.dc.json
- script/configs/mainnet/SwapQuoterV2.dc.json
- script/configs/optimism/SwapQuoterV2.dc.json
- script/networks.json
Files selected for processing (11)
- script/interfaces/IBridgeConfigV3.sol (1 hunks)
- script/interfaces/IMulticall3.sol (1 hunks)
- script/interfaces/IOwnable.sol (1 hunks)
- script/interfaces/ISynapseRouter.sol (1 hunks)
- script/router/ActivateRouterV1sQuoter.s.sol (1 hunks)
- script/router/BasicRouter.s.sol (1 hunks)
- script/router/TransferOwnership.s.sol (1 hunks)
- script/router/quoter/ConfigureQuoterV2.s.sol (1 hunks)
- script/router/quoter/DeployQuoterV2.s.sol (1 hunks)
- script/router/quoter/SaveConfigQuoterV2.s.sol (1 hunks)
- script/router/quoter/helpers/BridgeConfigLens.sol (1 hunks)
Files skipped from review due to trivial changes (2)
- script/interfaces/IOwnable.sol
- script/router/quoter/helpers/BridgeConfigLens.sol
Additional comments (Suppressed): 18
script/interfaces/ISynapseRouter.sol (1)
- 1-8: The
ISynapseRouterinterface is well defined with functions to set and get theswapQuoteraddress. Ensure that thesetSwapQuoterfunction is properly secured to prevent unauthorized access. This could be done by adding a modifier in the implementation to allow only the contract owner or authorized addresses to call this function.script/interfaces/IMulticall3.sol (1)
- 1-17: The
IMulticall3interface is well defined with clear function signatures and data structures. Theaggregate3function takes an array ofCall3structs and returns an array ofResultstructs. This function is marked aspayable, which means it can receive Ether. Ensure that this is the intended behavior and that the function properly handles any Ether sent to it. Also, verify that theallowFailurefield in theCall3struct is used correctly in the implementation to allow or disallow failures as intended.script/interfaces/IBridgeConfigV3.sol (2)
1-35: The new interface
IBridgeConfigV3introduces two new structsTokenandPoolto represent token and pool configurations respectively. It also introduces three new functionsgetAllTokenIDs,getTokenByID, andgetPoolConfigto retrieve these configurations. Ensure that these new data structures and functions are correctly used in the rest of the codebase.25-35: The functions
getAllTokenIDs,getTokenByID, andgetPoolConfigare declared to return memory arrays or structs. Ensure that these are handled correctly in the calling code to avoid potential memory-related issues.script/router/TransferOwnership.s.sol (1)
- 1-4: The
IOwnableinterface is imported but the newtransferOwnershipfunction is not reflected in the interface. Ensure that theIOwnableinterface has been updated to include this function.script/router/ActivateRouterV1sQuoter.s.sol (1)
- 9-26: The function
rundoes not have any access control. This means that any address can call this function and potentially change theswapQuoterof theSynapseRouter. This could be a serious security issue if not intended. Please verify if this is the desired behavior. If not, consider adding appropriate access control, such asonlyOwneroronlyAdmin.script/router/quoter/DeployQuoterV2.s.sol (3)
1-35: The contract
DeployQuoterV2is well-structured and follows the best practices of Solidity. It deploys theSwapQuoterV2contract and saves its address for future use. The contract uses environment variables to get the owner's address, which is a good practice for security reasons. However, ensure that the environment variables are properly set and secured. Also, make sure that theOWNER_ADDRis a trusted address, as it will have ownership of the deployed contract.14-20: The
runfunction sets up the script, starts broadcasting, deploys theSwapQuoterV2contract, and then stops broadcasting. This is a good practice as it encapsulates the entire process in a single function. However, ensure that thesetUpfunction and thevm.startBroadcastandvm.stopBroadcastfunctions are working as expected and handle any potential errors.26-33: The
deployQuoterV2function deploys theSwapQuoterV2contract with the necessary parameters. It retrieves the latest router deployment, the default pool calculator, the wrapped Ether (or equivalent on other chains), and the owner's address. It then deploys theSwapQuoterV2contract and returns its address and the constructor arguments. This function is well-structured and follows best practices. However, ensure that thegetLatestRouterDeployment,getDeploymentAddress, andvm.envAddressfunctions are working as expected and handle any potential errors.script/router/quoter/ConfigureQuoterV2.s.sol (9)
34-44: The
runfunction is the main entry point for this contract. It sets up theBasicSynapseScript, checks if the sender is the owner, reads the configuration, starts broadcasting, sets theSynapseRouter, sets the pools, and then stops broadcasting. This function is well-structured and follows a logical sequence of operations. However, it's important to ensure that thesetUp,readConfig,setSynapseRouter, andsetPoolsfunctions are implemented correctly and handle errors appropriately.46-71: The
readConfigfunction reads the configuration for theSwapQuoterV2contract from the deployment configuration. It then parses this configuration and stores it in theconfigBridgePools,bridgeTokenToSymbol, andsymbolToBridgeTokendata structures. This function also checks for duplicate symbols and bridge tokens, which is a good practice to prevent configuration errors. However, it's important to ensure that the configuration is formatted correctly and contains all the necessary information.73-79: The
setSynapseRouterfunction sets thesynapseRouterof theSwapQuoterV2contract to the latest router deployment. This function checks if the currentsynapseRouteris different from the latest router deployment before setting it, which is a good practice to prevent unnecessary operations. However, it's important to ensure that thegetLatestRouterDeploymentfunction returns the correct address.81-87: The
setPoolsfunction updates the pools of theSwapQuoterV2contract based on the configuration. It first removes all bridge pools that are not in the configuration, and then adds all pools from the configuration that are not in the bridge pools. This function is well-structured and follows a logical sequence of operations. However, it's important to ensure that theremoveMissingPoolsandaddMissingPoolsfunctions are implemented correctly and handle errors appropriately.89-115: The
getAllPoolsfunction retrieves all pools from theSwapQuoterV2contract and combines them into a single array. This function is well-structured and follows a logical sequence of operations. However, it's important to ensure that thegetOriginDefaultPools,getOriginLinkedPools, andgetBridgePoolsfunctions return the correct addresses.117-134: The
removeMissingPoolsfunction removes all pools from theSwapQuoterV2contract that are not in the configuration. This function uses thegetMissingPoolsfunction to find the pools to remove, and then calls theremovePoolsfunction of theSwapQuoterV2contract to remove them. This function is well-structured and follows a logical sequence of operations. However, it's important to ensure that thegetMissingPoolsandremovePoolsfunctions are implemented correctly and handle errors appropriately.136-159: The
addMissingPoolsfunction adds all pools from the configuration to theSwapQuoterV2contract that are not already in the bridge pools. This function uses thegetMissingPoolsfunction to find the pools to add, and then calls theaddPoolsfunction of theSwapQuoterV2contract to add them. This function is well-structured and follows a logical sequence of operations. However, it's important to ensure that thegetMissingPoolsandaddPoolsfunctions are implemented correctly and handle errors appropriately.163-178: The
equalBridgeTokenandequalFullyfunctions are used to compare two pools. TheequalBridgeTokenfunction checks if thebridgeTokenfield in the two pools is equal, and theequalFullyfunction checks if all fields in the two pools are equal. These functions are well-implemented and follow good practices for comparing data structures.182-213: The
getMissingPoolsfunction returns a list of pools that are inpoolsToFilterbut not inexistingPools. This function uses acomparePoolsfunction to compare the pools, which allows for flexible comparison logic. This function is well-structured and follows a logical sequence of operations. However, it's important to ensure that thecomparePoolsfunction is implemented correctly and handles errors appropriately.
…ly and cctp pools
There was a problem hiding this comment.
Review Status
Actionable comments generated: 11
Configuration used: CodeRabbit UI
Files ignored due to filter (9)
- deployments/base/DefaultPoolCalc.json
- deployments/base/SwapQuoterV2.json
- script/configs/SwapQuoterV2.originOnly.json
- script/configs/arbitrum/SwapQuoterV2.dc.json
- script/configs/avalanche/SwapQuoterV2.dc.json
- script/configs/base/SwapQuoterV2.dc.json
- script/configs/mainnet/SwapQuoterV2.dc.json
- script/configs/optimism/SwapQuoterV2.dc.json
- script/networks.json
Files selected for processing (3)
- script/router/quoter/ConfigureQuoterV2.s.sol (1 hunks)
- script/router/quoter/SaveConfigQuoterV2.s.sol (1 hunks)
- script/templates/BasicSynapse.s.sol (1 hunks)
Additional comments: 5
script/templates/BasicSynapse.s.sol (1)
- 189-199: The
loadDeploymentSaltfunction has been updated to check if a key exists in theallSaltsJSON string using thekeyExistsfunction. If the key exists, it decodes the corresponding salt entry and verifies theinitCodeHash. If the key doesn't exist, it returns 0. This change seems to be logically correct and should improve the function's robustness by handling the case where the key doesn't exist in theallSaltsJSON string.script/router/quoter/SaveConfigQuoterV2.s.sol (2)
31-49: The
runfunction is the main entry point for this contract. It checks if a configuration file already exists for theSwapQuoterV2contract. If it does, it loads the configuration from the file and returns. If not, it loads the ignored pools, saves the origin-only pools, saves the SynapseCCTP pools, saves the BridgeConfigV3 pools, and then serializes and writes the configuration to a file. This function is well-structured and follows a clear sequence of steps.164-166: The
getPoolTypefunction uses theisLinkedPoolfunction to determine the type of a pool. It returnsSwapQuoterV2.PoolType.Linkedif the pool is aLinkedPoolandSwapQuoterV2.PoolType.Defaultotherwise. This function is straightforward and correctly uses theisLinkedPoolfunction.script/router/quoter/ConfigureQuoterV2.s.sol (2)
1-4: Ensure that the correct version of Solidity is being used and that the imported contracts are compatible with this version.
34-45: The
runfunction is the main entry point for this contract. It sets up the contract, checks if the sender is the owner, reads the configuration, and then sets the Synapse router and pools. It's important to ensure that the owner check is robust and secure.
|
LGTM |
Description
SwapQuoterV2configuration from Ethereum's BridgeConfigV3 contract.SwapQuoterV2deployment.SynapseRoutercontract.TODO:
SynapseCCTPcontract and save them to the config.Checklist
Summary by CodeRabbit
IBridgeConfigV3andIMulticall3interfaces to enhance token and pool configurations and streamline multicall processes.transferOwnershipfunction toIOwnableinterface, enabling ownership transfers.ISynapseRouterinterface with functions to manage swap quoters.ActivateRouterV1sQuoter,BasicRouterScript,TransferOwnership,ConfigureQuoterV2,DeployQuoterV2, andSaveConfigQuoterV2to improve router and quoter management.BridgeConfigLenscontract to provide easy access to token and pool configurations.