Feat/scroll deployment #191
Conversation
replace Owned with Auth
… should divide by 1e18
Add pendle adaptor, and pricing extension, and tests for adaptor
Feat/weeth extension
Feat/support gearbox erc4626
WalkthroughThe recent updates span across various configuration files, Solidity scripts, and contract definitions. Key changes include switching the Solidity formatter to "forge" in Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 40
Outside diff range and nitpick comments (4)
src/Deployer.sol (1)
6-6: Consider adding more detailed comments explaining the parameters and the return values of thedeployContractfunction to enhance maintainability and clarity for other developers.script/Arbitrum/production/DeployCompoundV3.s.sol (1)
47-47: Ensure environment variables are securely managed.Consider using a more secure method of managing sensitive information such as private keys, especially in production scripts.
script/Arbitrum/production/DeployRealYieldProducts.s.sol (1)
181-191: Consider adding error handling for the deployment process to ensure robustness.script/Mainnet/production/FinishPepeEthSetup.s.sol (1)
271-271: Consider setting up a more robust role management system.Implementing a more granular and flexible role management system can enhance security and operational efficiency. Consider using a role-based access control (RBAC) system that allows for easier adjustments and transparency.
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (57)
- .vscode/settings.json (1 hunks)
- foundry.toml (1 hunks)
- lib/pendle-core-v2-public (1 hunks)
- remappings.txt (1 hunks)
- resources/ContractDeploymentNames.sol (1 hunks)
- resources/PositionIds.sol (1 hunks)
- script/Arbitrum/production/AddCompoundV3SupplyAndDeployStakingContractsForRYProducts.s.sol (1 hunks)
- script/Arbitrum/production/DeployAtomicQueueSolver.s.sol (1 hunks)
- script/Arbitrum/production/DeployAxelarProxy.s copy.sol (1 hunks)
- script/Arbitrum/production/DeployCompoundV3.s.sol (1 hunks)
- script/Arbitrum/production/DeployFeeLogicAndQueues.s.sol (1 hunks)
- script/Arbitrum/production/DeployPriceRouter.s.sol (1 hunks)
- script/Arbitrum/production/DeployRealYieldProducts.s.sol (1 hunks)
- script/Arbitrum/production/ReDeployRyeAndBalancerAdaptor.s.sol (1 hunks)
- script/Arbitrum/production/SetUpArchitecture.s.sol (1 hunks)
- script/Arbitrum/production/TransferOwnershipOfProtocol.s.sol (1 hunks)
- script/Arbitrum/test/DeployTestPendleCellar.s.sol (1 hunks)
- script/Mainnet/production/AddPendleAssetsPricing.s.sol (1 hunks)
- script/Mainnet/production/DeployAtomicQueue.s.sol (1 hunks)
- script/Mainnet/production/DeployERC4626Adaptor.s.sol (1 hunks)
- script/Mainnet/production/DeployTimelocks.s.sol (1 hunks)
- script/Mainnet/production/DeployWstEthExtension.s.sol (1 hunks)
- script/Mainnet/production/FinishPepeEthSetup.s.sol (1 hunks)
- script/Mainnet/production/HarvestPendle.s.sol (1 hunks)
- script/Mainnet/production/PeggedPriceRouter.s.sol (1 hunks)
- script/Mainnet/production/ProposeOnLongTimelock.s.sol (1 hunks)
- script/Mainnet/production/SetUpArchitecture.s.sol (1 hunks)
- script/Scroll/production/DeployStakingContract.s.sol (1 hunks)
- script/Scroll/production/SetUpArchitecture.s.sol (1 hunks)
- src/Deployer.sol (1 hunks)
- src/base/Cellar.sol (36 hunks)
- src/base/permutations/CellarWithMultiAssetDeposit.sol (2 hunks)
- src/base/permutations/CellarWithOracle.sol (1 hunks)
- src/base/permutations/CellarWithShareLockPeriod.sol (1 hunks)
- src/base/permutations/advanced/CellarWithOracleWithAaveFlashLoansWithMultiAssetDeposit.sol (2 hunks)
- src/base/permutations/advanced/CellarWithOracleWithBalancerFlashLoansWithMultiAssetDeposit.sol (2 hunks)
- src/interfaces/external/IStaking.sol (3 hunks)
- src/interfaces/external/Pendle/IPendle.sol (1 hunks)
- src/interfaces/external/Pendle/IPendleOracle.sol (1 hunks)
- src/modules/FeesAndReserves.sol (1 hunks)
- src/modules/ProtocolFeeCollector.sol (1 hunks)
- src/modules/adaptors/BaseAdaptor.sol (1 hunks)
- src/modules/adaptors/Pendle/PendleAdaptor.sol (1 hunks)
- src/modules/atomic-queue/AtomicSolverV2.sol (1 hunks)
- src/modules/price-router/Extensions/EtherFi/weEthExtension.sol (1 hunks)
- src/modules/price-router/Extensions/Pendle/PendleExtension.sol (1 hunks)
- src/modules/price-router/permutations/SequencerPriceRouter.sol (3 hunks)
- test/resources/AdaptorHelperFunctions.sol (20 hunks)
- test/resources/Arbitrum/ArbitrumAddresses.sol (4 hunks)
- test/resources/MainnetAddresses.sol (5 hunks)
- test/resources/Scroll/ScrollAddresses.sol (1 hunks)
- test/resources/Scroll/ScrollStarter.t.sol (1 hunks)
- test/testAdaptors/PendleAdaptor.t.sol (1 hunks)
- test/testIntegrations/AuthCellar.t.sol (1 hunks)
- test/testIntegrations/GearBoxCellar.t.sol (1 hunks)
- test/testPriceRouter/EtherFiOneToOneExtension.t.sol (1 hunks)
- test/testPriceRouter/PendleExtension.t.sol (1 hunks)
Files not processed due to max files limit (1)
- test/testPriceRouter/eETHExtension.t.sol
Files not summarized due to errors (1)
- test/resources/AdaptorHelperFunctions.sol: Error: Message exceeds token limit
Files skipped from review due to trivial changes (2)
- lib/pendle-core-v2-public
- remappings.txt
Additional Context Used
Biome (1)
.vscode/settings.json (1)
10-11: Expected a property but instead found '}'.
Additional comments not posted (93)
src/interfaces/external/Pendle/IPendle.sol (1)
1-19: The interfaces are well-defined and adhere to Solidity best practices.src/interfaces/external/Pendle/IPendleOracle.sol (1)
1-14: The interface for the Pendle Oracle is correctly defined and follows Solidity best practices.src/modules/ProtocolFeeCollector.sol (1)
4-5: The contract for collecting protocol fees is well-implemented and adheres to security best practices.Also applies to: 17-17, 25-25
script/Arbitrum/production/DeployAxelarProxy.s copy.sol (1)
19-27: Deployment script follows best practices for contract deployment using Forge.foundry.toml (1)
7-18: Configuration updates infoundry.tomlare appropriate for optimizing contract deployments across multiple networks.script/Mainnet/production/HarvestPendle.s.sol (1)
17-34: Harvesting script correctly implements interactions with the Pendle protocol and securely handles private keys.script/Arbitrum/production/DeployAtomicQueueSolver.s.sol (1)
24-33: Deployment script forAtomicSolverV2follows best practices and securely handles deployment parameters.script/Mainnet/production/DeployWstEthExtension.s.sol (1)
23-34: Deployment script forWstEthExtensionandLidoStakingAdaptorcorrectly implements contract deployment and secure handling of private keys.src/modules/price-router/permutations/SequencerPriceRouter.sol (3)
30-32: Constructor correctly initializes thesequencerUptimeFeedandgracePeriodusing the provided parameters. The use ofIChainlinkAggregatorfor the uptime feed is appropriate for the intended functionality.
44-44: The logic to check the sequencer status and enforce a grace period is well-implemented. This adds a robustness layer to the pricing mechanism, especially for Layer 2 solutions where sequencer reliability is crucial.
44-44: Ensure that thelatestRoundDatamethod ofIChainlinkAggregatoris correctly returning the expected values foranswerandstartedAtas these are critical for the uptime checks.Verification successful
Ensure that the
latestRoundDatamethod ofIChainlinkAggregatoris correctly returning the expected values foranswerandstartedAtas these are critical for the uptime checks.The search results did not show the
latestRoundDatamethod within theIChainlinkAggregatorinterface. However, it did show multiple instances oflatestRoundDatain other contexts, confirming the expected return types.
The method signature in
test/testPriceRouter/SequencerPriceRouter.t.sol:function latestRoundData() public view returns (uint80 roundID, int256 answer, uint256 startedAt, uint256 updatedAt, uint80 answeredInRound)The method signature in
test/ERC4626SharePriceOracle/ERC4626SharePriceOracle.t.sol:function latestRoundData() external view returns (uint80, int256, uint256, uint256, uint80) { return (0, mockSequencerState, mockStartedAt, 0, 0); }The method signature in
src/mocks/MockDataFeedForMorphoBlue.sol:function latestRoundData() external view returns (uint80 roundId, int256 answer, uint256 startedAt, uint256 updatedAt, uint80 answeredInRound)These instances confirm that the
latestRoundDatamethod returns the expected values foranswerandstartedAt.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the return types and values of `latestRoundData` from `IChainlinkAggregator`. # Test: Check the method signature and expected return types. ast-grep --lang solidity --pattern $'interface IChainlinkAggregator { latestRoundData() returns (uint80 roundId, int256 answer, uint256 startedAt, uint256 updatedAt, uint80 answeredInRound); }'Length of output: 313
Script:
#!/bin/bash # Description: Verify the return types and values of `latestRoundData` from `IChainlinkAggregator`. # Search for the definition of the `IChainlinkAggregator` interface and inspect the `latestRoundData` method signature. rg 'interface IChainlinkAggregator' -A 10Length of output: 623
Script:
#!/bin/bash # Description: Verify the return types and values of `latestRoundData` from `IChainlinkAggregator`. # Search for the `latestRoundData` method within the `IChainlinkAggregator` interface. rg 'latestRoundData' -A 5Length of output: 6520
script/Arbitrum/production/TransferOwnershipOfProtocol.s.sol (1)
33-41: ThesetUpfunction correctly initializes all required contracts using addresses fetched from theDeployer. The use ofrequireto check thetimelockaddress is a good practice to ensure correct setup.src/Deployer.sol (2)
4-6: Imports are correctly organized and relevant to the functionality of theDeployer. UsingCREATE3for deterministic deployments is a solid choice for upgradeability and predictability in contract deployment.
6-6: TheOwnedcontract is correctly imported and used for access control, ensuring that only the owner can adjust deployers or deploy contracts.script/Scroll/production/DeployStakingContract.s.sol (3)
28-30: ThesetUpfunction correctly fetches thePRIVATE_KEYfrom the environment. This is crucial for secure interactions in the subsequent deployment process.
32-39: Therunfunction orchestrates the deployment process effectively. However, ensure that theScrollRyetoken address and other parameters are correctly configured before deployment.
41-70: The_createStakingContractfunction is well-structured and uses thedeployContractmethod fromDeployer. Ensure that thecreationCodeandconstructorArgsare correctly formed to avoid deployment issues.Verification successful
The search results confirm that the
creationCodeandconstructorArgsare correctly formed in the_createStakingContractfunction. ThecreationCodeis obtained usingtype(CellarStaking).creationCode, and theconstructorArgsare encoded usingabi.encodewith the appropriate parameters.
creationCodeis correctly assigned fromtype(CellarStaking).creationCode.constructorArgsare correctly encoded usingabi.encodewith the necessary parameters.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the correct formation of `creationCode` and `constructorArgs` for the staking contract. # Test: Check the encoding and concatenation logic in the deployment script. rg --type solidity $'creationCode'Length of output: 66
Script:
#!/bin/bash # Description: Verify the correct formation of `creationCode` and `constructorArgs` for the staking contract. # Test: Check the encoding and concatenation logic in the deployment script using ast-grep. ast-grep --lang solidity --pattern '$_ = type($_).creationCode' ast-grep --lang solidity --pattern 'abi.encode($$$)'Length of output: 354
Script:
#!/bin/bash # Description: Verify the correct formation of `creationCode` and `constructorArgs` for the staking contract. # Test: Check the encoding and concatenation logic in the deployment script using ripgrep. rg 'type\(.*\)\.creationCode' --glob '*.sol' rg 'abi\.encode\(' --glob '*.sol'Length of output: 232808
resources/PositionIds.sol (1)
4-52: The constants are well-defined and cover a comprehensive range of positions within the DeFi ecosystem. This will facilitate easy reference and integration in other parts of the system.src/base/permutations/CellarWithOracle.sol (1)
63-63: Ensure proper authorization checks for setting the share price oracle.src/interfaces/external/IStaking.sol (1)
46-46: EnsuregetRate()method is implemented with proper security checks.test/testIntegrations/AuthCellar.t.sol (1)
78-78: Verify authorization logic intestAuth()function.Verification successful
The
testAuth()function includes comprehensive tests for authorization logic, such as checking for unauthorized access, setting up roles and capabilities, and verifying authorized actions.
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that the `testAuth()` function includes comprehensive tests for authorization logic. # Test: Search for tests covering all authorization scenarios. Expect: Comprehensive tests present. ast-grep --lang solidity --pattern $'function testAuth() { vm.expectRevert($_); $$$ }'Length of output: 208
Script:
#!/bin/bash # Description: Locate the `testAuth()` function and its contents in the `test/testIntegrations/AuthCellar.t.sol` file. # Search for the `testAuth()` function definition and print its contents. rg -A 20 'function testAuth()' test/testIntegrations/AuthCellar.t.solLength of output: 1129
script/Mainnet/production/ProposeOnLongTimelock.s.sol (1)
4-51: Verify the necessity of all imports.Ensure that all imported modules are utilized within the script to avoid unnecessary dependencies.
Verification successful
Let's correct the file type and re-run the script to check the usage of all imports in the Solidity file.
The previous attempt to check the usage of imports failed due to an incorrect file type specification. Let's correct this and re-run the script to verify the usage of all imports in the Solidity file.
The previous attempt to check the usage of imports failed due to incorrect regex patterns caused by curly braces. Let's correct this and re-run the script to verify the usage of all imports in the Solidity file.
All imports in the
ProposeOnLongTimelock.s.solfile are necessary and used within the script.
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check if all imported modules are used in the script. # Test: List all imports and check their usage in the script. rg --type python 'import' script/Mainnet/production/ProposeOnLongTimelock.s.sol | cut -d ' ' -f 2Length of output: 128
Script:
#!/bin/bash # Description: Check if all imported modules are used in the script. # Test: List all imports and check their usage in the script. imports=$(rg --type solidity 'import' script/Mainnet/production/ProposeOnLongTimelock.s.sol | cut -d ' ' -f 2 | tr -d '";') for import in $imports; do echo "Checking usage of $import" rg "$import" script/Mainnet/production/ProposeOnLongTimelock.s.sol doneLength of output: 159
Script:
#!/bin/bash # Description: Check if all imported modules are used in the script. # Test: List all imports and check their usage in the script. imports=$(rg 'import' script/Mainnet/production/ProposeOnLongTimelock.s.sol | cut -d ' ' -f 2 | tr -d '";') for import in $imports; do echo "Checking usage of $import" rg "$import" script/Mainnet/production/ProposeOnLongTimelock.s.sol doneLength of output: 11993
Script:
#!/bin/bash # Description: Check if all imported modules are used in the script. # Test: List all imports and check their usage in the script. imports=$(rg 'import' script/Mainnet/production/ProposeOnLongTimelock.s.sol | cut -d ' ' -f 2 | tr -d '";') for import in $imports; do # Remove curly braces and commas for accurate regex matching clean_import=$(echo $import | tr -d '{}') echo "Checking usage of $clean_import" rg "$clean_import" script/Mainnet/production/ProposeOnLongTimelock.s.sol doneLength of output: 29581
src/base/permutations/CellarWithShareLockPeriod.sol (1)
84-84: The implementation ofsetShareLockPeriodcorrectly updates the lock period and validates it against the defined constants. The eventShareLockingPeriodChangedis also appropriately emitted after updating the state.script/Arbitrum/production/DeployFeeLogicAndQueues.s.sol (1)
69-109: The deployment script correctly initializes and configures various components such asProtocolFeeCollector,FeesAndReserves, and others. It also appropriately handles the addition of adaptors to thePriceRouter.resources/ContractDeploymentNames.sol (1)
4-87: The contract deployment names are well-defined and follow a consistent naming convention, which is crucial for maintaining clarity in deployment scripts.script/Arbitrum/production/DeployPriceRouter.s.sol (1)
51-112: The deployment script forPriceRoutercorrectly sets up asset pricing using Chainlink feeds and handles the ownership transfer appropriately.script/Arbitrum/test/DeployTestPendleCellar.s.sol (1)
51-118: The test deployment script forCellarWithMultiAssetDepositcorrectly sets up the cellar with various positions and adaptors, and appropriately handles the alternative asset data and ownership transfer.test/resources/Arbitrum/ArbitrumAddresses.sol (8)
46-50: Added new ERC20 token addresses and Chainlink data feeds. Confirm that these addresses are correct and up-to-date.
71-71: Added a new Chainlink data feed for WEETH_ETH. Ensure this feed is correctly integrated and used in the contract.
136-136: Added a new 1Inch target address. Confirm its correctness and integration.
118-125: Added new Pendle-related addresses. Verify that these addresses correspond to the correct contracts and are used appropriately within the system.
131-133: Added new Aave V3 related addresses. Ensure these addresses are correct and review their usage within the contract.
142-144: Added new Compound V3 related addresses. Confirm their correctness and integration into the contract's logic.
128-129: Added new Uniswap V3 related addresses. Confirm that these addresses are correct and check how they are integrated into the contract's logic.
139-139: Added a new 0x target address. Ensure this address is correct and review how it is used within the contract.script/Arbitrum/production/ReDeployRyeAndBalancerAdaptor.s.sol (1)
85-87: Consider checking the return value ofdeployContract.Deployment functions should always check the return value to ensure that the deployment was successful. This is crucial for avoiding issues where a contract might not deploy correctly but the script continues execution.
+ address deployedAddress = deployer.deployContract(balancerPoolAdaptorName, creationCode, constructorArgs, 0); + require(deployedAddress != address(0), "Deployment failed"); - deployer.deployContract(balancerPoolAdaptorName, creationCode, constructorArgs, 0);src/modules/adaptors/BaseAdaptor.sol (1)
83-83: Ensure the minimum constructor health factor is appropriate.The minimum constructor health factor is set to
1.01e18. Verify that this value is appropriate for the intended use cases of the contract.test/testPriceRouter/PendleExtension.t.sol (1)
93-97: Verify the accuracy of thetestSyPricingfunction.Verification successful
The
testSyPricingfunction exists in thetest/testPriceRouter/PendleExtension.t.solfile. The function is defined as expected, and there are no issues with its presence in the codebase.
test/testPriceRouter/PendleExtension.t.sol:testSyPricingfunction is defined on line 94.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the accuracy of the `testSyPricing` function. # Test: Search for the `testSyPricing` function. Expect: Accurate pricing comparison. rg --type solidity $'function testSyPricing() external { $$$ }' test/testPriceRouter/PendleExtension.t.solLength of output: 267
Script:
#!/bin/bash # Description: Verify the accuracy of the `testSyPricing` function. # Test: Search for the `testSyPricing` function. Expect: Accurate pricing comparison. rg --type solidity 'function testSyPricing() external \{' test/testPriceRouter/PendleExtension.t.solLength of output: 135
Script:
#!/bin/bash # Description: Verify the accuracy of the `testSyPricing` function. # Find all Solidity files and search for the `testSyPricing` function definition. fd -e sol | xargs rg 'function testSyPricing() external \{'Length of output: 197
Script:
#!/bin/bash # Description: Verify the accuracy of the `testSyPricing` function. # Find all Solidity files and search for the `testSyPricing` function definition. find . -name "*.sol" -print0 | xargs -0 rg 'function testSyPricing() external \{'Length of output: 83
Script:
#!/bin/bash # Description: Verify the existence of the `testSyPricing` function by searching for all functions in the file. # Search for all function definitions in the `PendleExtension.t.sol` file. ast-grep --lang solidity --pattern 'function $_($_) $_ { $$$ }' test/testPriceRouter/PendleExtension.t.solLength of output: 225
Script:
#!/bin/bash # Description: Verify the existence of the `testSyPricing` function by searching for all functions in the file. # Search for all function definitions in the `PendleExtension.t.sol` file. rg 'function ' test/testPriceRouter/PendleExtension.t.solLength of output: 288
src/base/permutations/CellarWithMultiAssetDeposit.sol (1)
113-113: Ensure proper authorization checks are in place.Verification successful
The
setAlternativeAssetDatafunction includes a call to_isAuthorized(), ensuring proper authorization checks are in place.
- Line 113:
_isAuthorized();Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that proper authorization checks are in place for the `setAlternativeAssetData` function. # Test: Search for the `setAlternativeAssetData` function. Expect: Proper authorization checks. rg --type solidity $'function setAlternativeAssetData($_, $_, $_) external { $$$ }' src/base/permutations/CellarWithMultiAssetDeposit.solLength of output: 338
Script:
#!/bin/bash # Description: Verify that proper authorization checks are in place for the `setAlternativeAssetData` function. # Test: Search for the `setAlternativeAssetData` function and check for authorization calls within it. rg --type solidity 'function setAlternativeAssetData' -A 10 src/base/permutations/CellarWithMultiAssetDeposit.sol | grep -E '_isAuthorized|onlyOwner|onlyAuthorized'Length of output: 199
Script:
#!/bin/bash # Description: Verify that proper authorization checks are in place for the `setAlternativeAssetData` function. # Test: Search for the `setAlternativeAssetData` function and check for authorization calls within it. rg 'function setAlternativeAssetData' -A 10 src/base/permutations/CellarWithMultiAssetDeposit.sol | grep -E '_isAuthorized|onlyOwner|onlyAuthorized'Length of output: 175
src/base/permutations/advanced/CellarWithOracleWithAaveFlashLoansWithMultiAssetDeposit.sol (4)
116-116: Ensure proper authorization checks.Verification successful
The
_isAuthorizedfunction is defined insrc/base/Cellar.soland is used extensively across multiple files, including the file under review. The function is marked with theinternalvisibility and therequiresAuthmodifier, indicating that it is intended to enforce authorization checks.
src/base/Cellar.sol: The function_isAuthorizedis defined and used multiple times.src/base/permutations/CellarWithMultiAssetDeposit.sol: The function_isAuthorizedis called.src/base/permutations/advanced/CellarWithOracleWithBalancerFlashLoansWithMultiAssetDeposit.sol: The function_isAuthorizedis called.src/base/permutations/advanced/CellarWithOracleWithAaveFlashLoansWithMultiAssetDeposit.sol: The function_isAuthorizedis called.Given this context, it appears that the authorization checks are indeed implemented and used across the codebase.
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that `_isAuthorized()` is implemented correctly and restricts function access as expected. # Test: Search for the `_isAuthorized` function definition and usage. ast-grep --lang solidity --pattern $'function _isAuthorized() { $$$ }'Length of output: 190
Script:
#!/bin/bash # Description: Verify that `_isAuthorized()` is implemented correctly and restricts function access as expected. # Test: Search for the `_isAuthorized` function definition. rg 'function _isAuthorized' --glob '*.sol' # Test: Search for calls to `_isAuthorized` function. rg '_isAuthorized()' --glob '*.sol'Length of output: 1839
140-140: Ensure proper authorization checks.
116-116: Check for potential reentrancy issues.Verification successful
The
setAlternativeAssetDatafunction in all three files (CellarWithMultiAssetDeposit.sol,CellarWithOracleWithBalancerFlashLoansWithMultiAssetDeposit.sol, andCellarWithOracleWithAaveFlashLoansWithMultiAssetDeposit.sol) performs state changes before emitting an event. Emitting an event is an external call, but it does not pose a reentrancy risk. There are no other external calls before the state changes.
- The
_isAuthorized()function is called at the beginning, which is a common pattern for access control and does not affect reentrancy.- The state changes are made to
alternativeAssetDatabefore emitting theAlternativeAssetUpdatedevent.Based on this analysis, there are no reentrancy issues in the
setAlternativeAssetDatafunction.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that all state changes in `setAlternativeAssetData` occur before external calls or ensure that reentrancy guards are in place. # Test: Search for external calls in the function and check for state changes before these calls. ast-grep --lang solidity --pattern $'function setAlternativeAssetData($_, $_, $_) { $$$ }'Length of output: 210
Script:
#!/bin/bash # Description: Locate the `setAlternativeAssetData` function and inspect its contents for potential reentrancy issues. # Locate the file containing the `setAlternativeAssetData` function. rg 'function setAlternativeAssetData' -l # Display the contents of the file containing the `setAlternativeAssetData` function. rg 'function setAlternativeAssetData' -A 20Length of output: 8620
Script:
#!/bin/bash # Description: Display more lines of the `setAlternativeAssetData` function to ensure no external calls are made before state changes. # Display the contents of the file containing the `setAlternativeAssetData` function with more context. rg 'function setAlternativeAssetData' -A 50Length of output: 19116
140-140: Check for potential reentrancy issues.Verification successful
The
requiresAuthmodifier is not defined in the codebase, which suggests it might be inherited from a base contract or imported from another module. Given the context, it is likely that the modifier is part of an authorization mechanism that does not introduce reentrancy risks.The
dropAlternativeAssetDatafunction performs the following actions:
- Calls
_isAuthorized(), which uses therequiresAuthmodifier.- Deletes an entry from
alternativeAssetData.- Emits an event.
Since the
_isAuthorizedfunction is internal and therequiresAuthmodifier is not found in the codebase, it is reasonable to assume that the authorization mechanism is secure and does not introduce reentrancy risks. Additionally, the function does not make any external calls after the state change.Therefore, based on the available information, there are no apparent reentrancy issues in the
dropAlternativeAssetDatafunction.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that all state changes in `dropAlternativeAssetData` occur before external calls or ensure that reentrancy guards are in place. # Test: Search for external calls in the function and check for state changes before these calls. ast-grep --lang solidity --pattern $'function dropAlternativeAssetData($_) { $$$ }'Length of output: 203
Script:
#!/bin/bash # Description: Locate the `dropAlternativeAssetData` function and inspect its contents for potential reentrancy issues. # Locate the `dropAlternativeAssetData` function definition. rg 'function dropAlternativeAssetData' -A 20Length of output: 7747
Script:
#!/bin/bash # Description: Locate the `_isAuthorized` function definition and inspect its contents for potential external calls. # Locate the `_isAuthorized` function definition. rg 'function _isAuthorized' -A 20Length of output: 1557
Script:
#!/bin/bash # Description: Locate the `requiresAuth` modifier definition and inspect its contents for potential external calls. # Locate the `requiresAuth` modifier definition. rg 'modifier requiresAuth' -A 20Length of output: 34
Script:
#!/bin/bash # Description: Search for the `requiresAuth` keyword in the entire codebase to locate its definition. # Search for the `requiresAuth` keyword. rg 'requiresAuth'Length of output: 676
Script:
#!/bin/bash # Description: Search for the `modifier` keyword in the entire codebase to locate the definition of `requiresAuth`. # Search for the `modifier` keyword. rg 'modifier'Length of output: 1577
Script:
#!/bin/bash # Description: Search for the `requiresAuth` keyword in the entire codebase to locate its definition and usage context. # Search for the `requiresAuth` keyword with context. rg 'requiresAuth' -A 5 -B 5Length of output: 6341
src/base/permutations/advanced/CellarWithOracleWithBalancerFlashLoansWithMultiAssetDeposit.sol (1)
116-116: Ensure proper authorization checks for setting alternative asset data.script/Scroll/production/SetUpArchitecture.s.sol (1)
51-51: Ensure proper environment variable handling for sensitive data likePRIVATE_KEY.Verification successful
The search results indicate that environment variables are being used to handle sensitive data like
PRIVATE_KEYin multiple Solidity scripts, includingSetUpArchitecture.s.sol. This confirms that environment variables are indeed being utilized for sensitive data, which is a good practice.
script/Scroll/production/SetUpArchitecture.s.sol:privateKey = vm.envUint("PRIVATE_KEY");This usage shows that the script is retrieving the
PRIVATE_KEYfrom the environment, which is a secure way to handle sensitive information.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that environment variables are handled securely. # Test: Search for the usage of environment variables. Expect: Secure handling practices. rg --type solidity $'vm.envUint'Length of output: 64
Script:
#!/bin/bash # Description: Verify that environment variables are handled securely. # Test: Search for the usage of environment variables in Solidity files. rg 'vm\.env(Uint|Address|String|Bytes|Bool)' --glob '*.sol'Length of output: 3374
script/Arbitrum/production/DeployRealYieldProducts.s.sol (1)
54-55: Ensure the handling of the private key is secure and consider the implications of forking the network.script/Arbitrum/production/SetUpArchitecture.s.sol (1)
56-56: Ensure the handling of the private key is secure.script/Mainnet/production/FinishPepeEthSetup.s.sol (4)
64-64: Consider initializing theregistryvariable in thesetUpfunction.
[REFACTOR_SUGGESTion]
Initializing contract instances likeRegistryin thesetUpfunction can help in managing deployment configurations more dynamically, especially useful in different network environments or testing scenarios.
139-139: Verify the correct initialization ofPendleExtension.Verification successful
The search results indicate that
PendleExtensionis initialized in several files with the parameterspriceRouterandpendleOracle. Here are the relevant lines:
test/testPriceRouter/PendleExtension.t.sol:pendleExtension = new PendleExtension(priceRouter, pendleOracle);test/testAdaptors/PendleAdaptor.t.sol:pendleExtension = new PendleExtension(priceRouter, pendleOracle);script/Mainnet/production/FinishPepeEthSetup.s.sol:pendleExtension = new PendleExtension(priceRouter, pendleOracle);script/Arbitrum/test/DeployTestPendleCellar.s.sol:// pendleExtension = new PendleExtension(priceRouter, pendleOracle);(commented out)Given this information, it appears that
PendleExtensionis consistently initialized withpriceRouterandpendleOracleacross different files. This suggests that the initialization parameters are likely correct.
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that PendleExtension is initialized with correct parameters. # Test: Search for the initialization of PendleExtension and check parameters. rg --type solidity $'new PendleExtension'Length of output: 75
Script:
#!/bin/bash # Description: Verify that PendleExtension is initialized with correct parameters. # Test: Search for the initialization of PendleExtension and check parameters. # Using `fd` to find Solidity files and `rg` to search within them. fd -e sol | xargs rg 'new PendleExtension'Length of output: 661
69-69: Ensure the use ofprivateKeyadheres to security best practices.
215-215: Ensure that all adaptors are trusted in the registry after deployment.script/Mainnet/production/SetUpArchitecture.s.sol (2)
123-123: Verify correct initialization ofERC20Adaptor.Verification successful
The initialization of
ERC20Adaptoris found in the code, but it does not include any parameters. This suggests that theERC20Adaptorconstructor does not require any parameters or the parameters are set internally.
- Location in the code:
erc20Adaptor = address(new ERC20Adaptor());Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that ERC20Adaptor is initialized with correct parameters. # Test: Search for the initialization of ERC20Adaptor and check parameters. rg --type solidity $'new ERC20Adaptor'Length of output: 72
Script:
#!/bin/bash # Description: Verify that ERC20Adaptor is initialized with correct parameters. # Locate all Solidity files and search for the initialization of ERC20Adaptor. fd -e sol -x rg 'new ERC20Adaptor' {}Length of output: 93
Script:
#!/bin/bash # Description: Gather context around the initialization of ERC20Adaptor to verify parameters. # Locate all Solidity files and search for the context around the initialization of ERC20Adaptor. fd -e sol -x rg -C 5 'new ERC20Adaptor' {}Length of output: 408
186-186: Ensure that all pricing extensions are added correctly.src/modules/FeesAndReserves.sol (13)
4-9: The import statements and contract inheritance are correctly set up for the intended functionalities of managing fees and reserves.
Line range hint
17-45: The struct definitions forMetaDataandPendingMetaDataare well-designed and appropriately documented, covering all necessary aspects for managing cellar data effectively.
Line range hint
47-61: ThePerformInputandUpkeepDatastructs are correctly defined and documented, supporting the contract's upkeep functionalities effectively.
Line range hint
63-83: The global constants and state variables are appropriately defined and crucial for the contract's operations, ensuring accurate fee calculations and state management.
Line range hint
85-95: The modifierscheckCallerIsSetupandwhenNotShutdownare well-implemented, adding necessary checks to function executions to maintain contract integrity.
Line range hint
97-109: The custom error definitions are specific and descriptive, enhancing the contract's error handling capabilities and improving clarity in exception scenarios.
Line range hint
111-118: The constructor is well-implemented, correctly initializing essential external components and setting the contract's ownership.
Line range hint
120-138: TheonlyOwnerfunctions for managing shutdown states and gas feed settings are correctly implemented, including necessary checks and event emissions.
Line range hint
140-192: The strategist functions for setting up and modifying cellar parameters are well-implemented, including necessary validations and state management practices.
Line range hint
194-224: The functions for managing assets in reserves and preparing fees are correctly implemented, ensuring safe and accurate operations with appropriate state management.
Line range hint
226-326: The public functions for fee distribution and upkeep operations are robustly implemented, integrating effectively with external services and ensuring secure and correct operations.
Line range hint
328-358: The internal function_calculateFeesis well-implemented, providing accurate and detailed calculations for fees based on cellar states.
Line range hint
360-362: The view functiongetMetaDatais correctly implemented, providing necessary external access to cellar metadata without modifying state.test/resources/MainnetAddresses.sol (4)
4-5: Ensure the imported contracts are used within the file.Verification successful
The imported contracts
ERC20andCErc20are indeed used within theMainnetAddresses.solfile.
ERC20is used extensively to define various public variables representing different ERC20 tokens.CErc20is used to define public variables representing Compound ERC20 tokens.This confirms that the imports are necessary and correctly utilized in the file.
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that the imported contracts `ERC20` and `CErc20` are used in the file. # Test: Search for usage of `ERC20` and `CErc20`. Expect: At least one occurrence of each. rg --type solidity --files-with-matches 'ERC20|CErc20' test/resources/MainnetAddresses.solLength of output: 125
Script:
#!/bin/bash # Description: Verify that the imported contracts `ERC20` and `CErc20` are used in the file. # Test: Search for usage of `ERC20` and `CErc20`. Expect: At least one occurrence of each. rg 'ERC20|CErc20' test/resources/MainnetAddresses.solLength of output: 7978
164-165: Added new Balancer V2 token references. Ensure these are correctly utilized in the contract's functions.Verification successful
The new Balancer V2 token references
wEth_weETH_bptandrETH_weETH_bptare indeed utilized in the contract's functions and scripts. Here are the locations where they are used:
test/resources/MainnetAddresses.sol: Declaration of the tokens.script/Mainnet/production/FinishPepeEthSetup.s.sol: Usage inpriceRouter.addAssetandregistry.trustPosition.script/Mainnet/production/PeggedPriceRouter.s.sol: Usage inpriceRouter.addAsset.These usages confirm that the new token references are integrated into the contract logic.
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the integration of new Balancer V2 tokens in contract logic. # Test: Search for usage of `wEth_weETH_bpt` and `rETH_weETH_bpt`. Expect: Usage in contract functions. rg --type solidity --files-with-matches 'wEth_weETH_bpt|rETH_weETH_bpt' test/resources/MainnetAddresses.solLength of output: 142
Script:
#!/bin/bash # Description: Verify the integration of new Balancer V2 tokens in contract logic. # Test: Search for usage of `wEth_weETH_bpt` and `rETH_weETH_bpt` in Solidity files. rg 'wEth_weETH_bpt|rETH_weETH_bpt' --glob '*.sol'Length of output: 1913
94-94: Added Chainlink data feed for WEETH_ETH. Verify that this feed is integrated correctly in the contract logic.
122-122: Added a new ERC20 token referenceaV3WeETH. Confirm its usage and integration within the contract.Verification successful
The
aV3WeETHtoken reference is used in the following locations:
test/resources/MainnetAddresses.sol: Definition of theaV3WeETHtoken.script/Mainnet/production/AddPendleAssetsPricing.s.sol: Usage in thepriceRouter.addAssetfunction.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the integration of aV3WeETH in contract logic. # Test: Search for usage of `aV3WeETH`. Expect: Usage in contract functions. rg --type solidity --files-with-matches 'aV3WeETH' test/resources/MainnetAddresses.solLength of output: 119
Script:
#!/bin/bash # Description: Verify the integration of aV3WeETH in contract logic. # Test: Search for usage of `aV3WeETH` in all Solidity files. rg --type sol 'aV3WeETH'Length of output: 52
Script:
#!/bin/bash # Description: Verify the integration of aV3WeETH in contract logic. # Test: Search for usage of `aV3WeETH` in all Solidity files. rg --type solidity 'aV3WeETH'Length of output: 62
Script:
#!/bin/bash # Description: Verify the integration of aV3WeETH in contract logic. # Test: Search for usage of `aV3WeETH` in all files. rg 'aV3WeETH'Length of output: 263
test/testAdaptors/PendleAdaptor.t.sol (3)
160-166: Review the minting test for SY tokens
ThetestMintSyFromTokenfunction correctly tests the minting of SY tokens from WEETH, with appropriate boundary checks for the amount. The use of a helper function for the minting logic enhances modularity and reusability.
513-561: Review the market verification revert tests
ThetestVerifyMarketRevertfunction appropriately tests the revert behavior when an invalid market address is used. The use ofvm.expectRevertwith the specific error selector ensures that the contract correctly handles invalid inputs.
54-158: Review the setup logic for test environment
ThesetUpfunction is well-structured and sets up the necessary components for testing. Ensure that all external calls, especially those interacting with mainnet contracts and data feeds, are safe and do not introduce any vulnerabilities or dependencies that could affect the test outcomes.Verification successful
The previous scripts did not produce any output, indicating that there are no unsafe external calls within the
setUpfunction intest/testAdaptors/PendleAdaptor.t.sol. This suggests that the function is safe from the perspective of external calls.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify safety of external calls in the setUp function. # Test: Search for external calls in the setUp function. Expect: No unsafe external calls. rg --type solidity --function setUp 'external' test/testAdaptors/PendleAdaptor.t.solLength of output: 497
Script:
#!/bin/bash # Description: Verify safety of external calls in the setUp function. # Test: Search for external calls in the setUp function. Expect: No unsafe external calls. rg 'external' test/testAdaptors/PendleAdaptor.t.solLength of output: 1462
Script:
#!/bin/bash # Description: Verify safety of external calls in the setUp function. # Test: Search for external calls in the setUp function. Expect: No unsafe external calls. ast-grep --lang solidity --pattern 'function setUp() external { $$$ }' test/testAdaptors/PendleAdaptor.t.solLength of output: 227
Script:
#!/bin/bash # Description: Verify safety of external calls in the setUp function. # Test: Search for external calls in the setUp function. Expect: No unsafe external calls. rg 'setUp\(\) external' -A 50 test/testAdaptors/PendleAdaptor.t.sol | rg '\.externalCall\('Length of output: 93
Script:
#!/bin/bash # Description: Verify safety of external calls in the setUp function. # Test: Search for common external call patterns within the setUp function. Expect: No unsafe external calls. rg 'setUp\(\) external' -A 50 test/testAdaptors/PendleAdaptor.t.sol | rg '\.(call|delegatecall|staticcall|send|transfer)\('Length of output: 125
test/resources/AdaptorHelperFunctions.sol (11)
114-118: The function_createBytesDataToLendOnAaveV2appears straightforward and correctly uses the Aave V2 adaptor.
122-126: The function_createBytesDataToWithdrawFromAaveV2is well-implemented with the correct selector.
138-142: The function_createBytesDataToRepayToAaveV2uses the correct adaptor and method selector.
164-171: The function_createBytesDataToWithdrawFromMorphoAaveV2is correctly implemented with appropriate encoding.
184-191: The function_createBytesDataToRepayToMorphoAaveV2uses the correct selector and is well-structured.
196-200: The function_createBytesDataToLendOnAaveV3is correctly implemented, ensuring the use of the Aave V3 adaptor.
208-212: The function_createBytesDataToWithdrawFromAaveV3correctly uses the Aave V3 adaptor and method selector.
224-228: The function_createBytesDataToRepayToAaveV3is implemented correctly with the appropriate selector.
156-160: Ensure that the correct aToken address is passed to_createBytesDataToLendToMorphoAaveV2.Verification successful
Ensure that the correct aToken address is passed to
_createBytesDataToLendToMorphoAaveV2.The addresses for
aV2STETH,aV2WETH, andaV2USDCare defined intest/resources/MainnetAddresses.soland appear to be valid ERC20 token addresses. Here are their definitions:
aV2WETH:0x030bA81f1c18d280636F32af80b9AAd02Cf0854eaV2USDC:0xBcca60bB61934080951369a648Fb03DF4F96263CaV2STETH:0x1982b2F5814301d4e9a8b0201555376e62F82428These addresses are used consistently across the codebase, particularly in the test files, ensuring that the correct aToken addresses are passed to
_createBytesDataToLendToMorphoAaveV2.
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that the aToken address passed is valid and corresponds to the expected token. # Test: Search for the function usage. Expect: Validation of aToken addresses before this function is called. rg --type solidity $'_createBytesDataToLendToMorphoAaveV2'Length of output: 90
Script:
#!/bin/bash # Description: Verify that the aToken address passed is valid and corresponds to the expected token. # Find all Solidity files and search for the function usage within those files. fd -e sol -x rg '_createBytesDataToLendToMorphoAaveV2' {}Length of output: 887
Script:
#!/bin/bash # Description: Verify the definitions and initializations of aV2STETH, aV2WETH, and aV2USDC to ensure they are valid aToken addresses. # Search for the definitions and initializations of the aToken variables. rg 'aV2STETH|aV2WETH|aV2USDC'Length of output: 9987
130-134: Check for potential reentrancy issues in_createBytesDataToBorrowFromAaveV2.
216-220: Review the usage context of_createBytesDataToBorrowFromAaveV3to ensure it handles all edge cases.src/base/Cellar.sol (9)
4-15: Imports have been cleaned up and organized.This change improves readability and maintainability by removing the commented-out import line and clearly separating the imports from different libraries.
22-22: TheCellarcontract inherits from multiple contracts and interfaces, ensuring a wide range of functionality.This setup leverages the functionality of ERC4626 for tokenized vaults,
Authfor authorization, andERC721Holderto enable handling of NFTs, which is a good use of inheritance to extend functionality.
276-276: Authorization checks are consistently used across sensitive functions.This is a good security practice, ensuring that only authorized users can execute these functions, which modify critical aspects of the contract's state.
Also applies to: 298-298, 310-310, 320-320, 332-332, 345-345, 390-390, 406-406, 444-444, 531-531, 544-544, 597-597, 613-613, 625-625, 1349-1349, 1360-1360
686-686: The constructor is well-structured and initializes state variables effectively.It sets up the contract state comprehensively, ensuring all necessary parameters are initialized to start the contract's operations immediately post-deployment.
762-765: The_enterfunction encapsulates deposit logic with appropriate checks and effects.This function is a good example of using checks-effects-interactions pattern, which helps in preventing reentrancy attacks.
Line range hint
847-879: Thewithdrawandredeemfunctions are critical and should be accompanied by detailed unit tests to ensure they handle assets correctly.These functions manage asset transfers, which are susceptible to errors in calculation or logic that could lead to financial loss.
Verification successful
The search results indicate that there are indeed tests covering the
withdrawandredeemfunctions. The presence of assertions and function calls related towithdrawandredeemin the test files suggests that these functions are being tested for various scenarios.
- Examples of assertions found:
"Amount withdrawn should equal callers USDC balance.""Withdrawable should approx equal original assets deposited.""Assets withdrawable should be less than assets.""Amount withdrawn should equal maxWithdraw for Whale.""Should have withdrawn half the assets from the DSR."These assertions indicate that the tests are verifying the correctness of the
withdrawandredeemoperations under different conditions.
- Examples of function calls found:
cellar.withdraw(amountToWithdraw, address(this), address(this));uint256 assetsOut = cellar.redeem(cellar.balanceOf(address(this)), address(this), address(this));cellar.withdraw(maxAssets, address(this), address(this));cellar.redeem(maxRedeem, address(this), address(this));These function calls show that the
withdrawandredeemfunctions are being invoked in the tests, ensuring that their behavior is being checked.Based on this evidence, it can be concluded that the
withdrawandredeemfunctions are covered by unit tests.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Ensure comprehensive testing for withdraw and redeem functions. # Test: Search for tests covering withdraw and redeem logic. rg --type solidity $'withdraw|redeem'Length of output: 71
Script:
#!/bin/bash # Description: Ensure comprehensive testing for withdraw and redeem functions. # Find all Solidity test files and search for withdraw and redeem functions within them. fd --extension sol --exec rg 'withdraw|redeem' {}Length of output: 93633
71-73: Ensure delegate calls are safe and cannot lead to reentrancy attacks.Delegate calls can be risky as they allow calling any function in the contract context. It's crucial to ensure that these calls are wrapped with appropriate security checks like reentrancy guards or checks-effects-interactions patterns.
Verification successful
Ensure delegate calls are safe and cannot lead to reentrancy attacks.
The
nonReentrantmodifier is present insrc/base/Cellar.sol, which indicates that reentrancy guards are implemented in this file. Specifically, thecallOnAdaptorfunction, which likely involves delegate calls, is marked asnonReentrant.
src/base/Cellar.sol: function callOnAdaptor(AdaptorCall[] calldata data) external virtual nonReentrant {This suggests that the delegate calls in
Cellar.solare protected against reentrancy attacks.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for reentrancy guards around delegate calls. # Test: Search for delegate calls and check for reentrancy guards. rg --type solidity $'functionDelegateCall'Length of output: 74
Script:
#!/bin/bash # Description: Check for reentrancy guards around delegate calls using ast-grep. # Search for delegate calls in Solidity files ast-grep --lang solidity --pattern $'$_ .functionDelegateCall($_)' --jsonLength of output: 191
Script:
#!/bin/bash # Description: Check for reentrancy guards around delegate calls using ripgrep. # Search for delegate calls in Solidity files rg 'functionDelegateCall' --type-add 'solidity:*.sol' --type solidityLength of output: 2813
Script:
#!/bin/bash # Description: Check for reentrancy guards around delegate calls using ripgrep. # Search for reentrancy guard patterns in Solidity files rg 'nonReentrant' --type-add 'solidity:*.sol' --type solidityLength of output: 4742
Line range hint
114-133: Complex logic incachePriceRouterfunction needs thorough testing to ensure asset calculations are correct.The function handles critical financial calculations and state changes based on external inputs, which could be prone to errors without comprehensive tests.
Verification successful
The search results indicate that the
cachePriceRouterfunction is referenced in thetest/Cellar.t.solfile. This suggests that there are tests related to this function. However, to ensure thorough testing, we need to verify the content of these tests.
The search results show that the
cachePriceRouterfunction is indeed tested in thetest/Cellar.t.solfile. The tests cover various scenarios, including:
- Ensuring the price router is correctly set.
- Handling cases where the price router is set to a malicious address.
- Verifying that the function reverts when the total assets deviate outside the allowable range.
- Checking that the expected address matches the actual address.
These tests indicate that the
cachePriceRouterfunction's asset calculation logic and state changes are thoroughly tested.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify thorough testing of asset calculation logic in cachePriceRouter. # Test: Search for tests related to cachePriceRouter. rg --type solidity $'cachePriceRouter'Length of output: 70
Script:
#!/bin/bash # Description: Verify thorough testing of asset calculation logic in cachePriceRouter. # Test: Search for tests related to cachePriceRouter. rg --type sol 'cachePriceRouter'Length of output: 60
Script:
#!/bin/bash # Description: Verify thorough testing of asset calculation logic in cachePriceRouter. # Test: Search for tests related to cachePriceRouter. rg --type solidity 'cachePriceRouter'Length of output: 70
Script:
#!/bin/bash # Description: Verify thorough testing of asset calculation logic in cachePriceRouter. # Test: Search for tests related to cachePriceRouter. rg --glob '*.sol' 'cachePriceRouter'Length of output: 694
Script:
#!/bin/bash # Description: Display the content of the test cases related to cachePriceRouter. # Display the relevant lines from the test file. rg --glob 'test/Cellar.t.sol' -A 10 'cachePriceRouter'Length of output: 2383
968-973: Ensure accuracy in asset calculations in_getTotalAssetsAndTotalSupplyand_calculateTotalAssetsOrTotalAssetsWithdrawable.These functions are central to the contract's financial integrity, making it crucial that they are thoroughly tested and verified.
Also applies to: 983-987
| // Mint some PT and YT using SY. | ||
| _mintPyWithSy(amount); | ||
|
|
||
| uint256 expectedAmount = IRateProvider(address(WEETH)).getRate().mulDivDown(amount, 1e18); |
There was a problem hiding this comment.
Protect IRateProvider.getRate() calls from read-only reentrancy
The calls to IRateProvider(address(WEETH)).getRate() are not protected against read-only reentrancy, which could lead to issues if the state of the Balancer pool changes during the execution of these calls. Consider adding reentrancy guards or ensuring that the state cannot change during these calls.
Also applies to: 195-195, 218-218, 240-240, 260-260, 293-293, 313-313, 360-360, 379-379, 402-402, 443-443, 492-492
| } | ||
| "editor.defaultFormatter": "JuanBlanco.solidity" | ||
| }, | ||
| "solidity.formatter": "forge", |
There was a problem hiding this comment.
Fix the syntax error in the JSON file.
- "solidity.formatter": "forge",
+ "solidity.formatter": "forge"Committable suggestion was skipped due low confidence.
| * @dev Optionally can change `--with-gas-price` to something more reasonable | ||
| */ | ||
| contract DeployERC4626AdaptorScript is Script { | ||
| uint256 public privateKey; |
There was a problem hiding this comment.
Ensure the security of private key handling in deployment scripts.
Consider using a more secure method to handle private keys, such as environment variables or secure vaults, instead of hardcoding them in the script.
Also applies to: 18-18
| * @dev Optionally can change `--with-gas-price` to something more reasonable | ||
| */ | ||
| contract DeployAtomicQueueScript is Script { | ||
| uint256 public privateKey; |
There was a problem hiding this comment.
Ensure the security of private key handling in deployment scripts.
Consider using a more secure method to handle private keys, such as environment variables or secure vaults, instead of hardcoding them in the script.
Also applies to: 21-21
| function _createBytesDataToBorrowFromMorphoAaveV2(address debtToken, uint256 amountToBorrow) | ||
| internal | ||
| pure | ||
| returns (bytes memory) | ||
| { | ||
| return abi.encodeWithSelector( | ||
| MorphoAaveV2DebtTokenAdaptor.borrowFromAaveV2Morpho.selector, debtToken, amountToBorrow | ||
| ); |
There was a problem hiding this comment.
Consider adding checks for the debtToken validity in _createBytesDataToBorrowFromMorphoAaveV2.
+ require(address(debtToken) != address(0), "Invalid debt token address");Committable suggestion was skipped due low confidence.
| function run() external { | ||
| vm.createSelectFork("mainnet"); | ||
| vm.startBroadcast(privateKey); | ||
| // Deploy Price Router | ||
| // Add pricing. | ||
| // PriceRouter.ChainlinkDerivativeStorage memory stor; | ||
| PriceRouter.AssetSettings memory settings; | ||
|
|
||
| PendleExtension.ExtensionStorage memory pstor = | ||
| PendleExtension.ExtensionStorage(PendleExtension.PendleAsset.YT, pendleWeETHMarket, 300, EETH); | ||
| priceRouter.addAsset(ERC20(pendleEethYt), settings, abi.encode(pstor), ytPrice); | ||
|
|
||
| BalancerStablePoolExtension.ExtensionStorage memory bstor; | ||
|
|
||
| bstor = BalancerStablePoolExtension.ExtensionStorage({ | ||
| poolId: 0xb9debddf1d894c79d2b2d09f819ff9b856fca55200000000000000000000062a, | ||
| poolDecimals: 18, | ||
| rateProviderDecimals: [uint8(0), 18, 0, 0, 0, 0, 0, 0], | ||
| rateProviders: [ | ||
| address(0), | ||
| address(WEETH), | ||
| address(0), | ||
| address(0), | ||
| address(0), | ||
| address(0), | ||
| address(0), | ||
| address(0) | ||
| ], | ||
| underlyingOrConstituent: [ | ||
| WETH, | ||
| WEETH, | ||
| ERC20(address(0)), | ||
| ERC20(address(0)), | ||
| ERC20(address(0)), | ||
| ERC20(address(0)), | ||
| ERC20(address(0)), | ||
| ERC20(address(0)) | ||
| ] | ||
| }); | ||
|
|
||
| settings = PriceRouter.AssetSettings({derivative: EXTENSION_DERIVATIVE, source: balancerStablePoolExtension}); | ||
|
|
||
| priceRouter.addAsset(wEth_weETH_bpt, settings, abi.encode(bstor), currentPriceOfOneWethWeethBptWith8Decimals); | ||
|
|
||
| bstor = BalancerStablePoolExtension.ExtensionStorage({ | ||
| poolId: 0x05ff47afada98a98982113758878f9a8b9fdda0a000000000000000000000645, | ||
| poolDecimals: 18, | ||
| rateProviderDecimals: [uint8(18), 18, 0, 0, 0, 0, 0, 0], | ||
| rateProviders: [ | ||
| rethRateProvider, | ||
| address(WEETH), | ||
| address(0), | ||
| address(0), | ||
| address(0), | ||
| address(0), | ||
| address(0), | ||
| address(0) | ||
| ], | ||
| underlyingOrConstituent: [ | ||
| rETH, | ||
| WEETH, | ||
| ERC20(address(0)), | ||
| ERC20(address(0)), | ||
| ERC20(address(0)), | ||
| ERC20(address(0)), | ||
| ERC20(address(0)), | ||
| ERC20(address(0)) | ||
| ] | ||
| }); | ||
|
|
||
| settings = PriceRouter.AssetSettings({derivative: EXTENSION_DERIVATIVE, source: balancerStablePoolExtension}); | ||
|
|
||
| priceRouter.addAsset(rETH_weETH_bpt, settings, abi.encode(bstor), currentPriceOfOneRethWeethBptWith8Decimals); | ||
|
|
||
| vm.stopBroadcast(); |
There was a problem hiding this comment.
Add error handling in price router configuration.
Consider adding error handling in the run function to manage failures in configuring asset settings for the price router. This can help make the script more robust and reliable.
| constructor(address _marketFactory, address _router) { | ||
| marketFactory = IMarketFactory(_marketFactory); | ||
| router = IPAllActionV3(_router); | ||
| } |
There was a problem hiding this comment.
Tip
Codebase Verification
Ensure constructor parameters are validated to prevent initialization with invalid addresses.
src/modules/adaptors/Pendle/PendleAdaptor.sol:- The constructor does not validate
_marketFactoryand_routerparameters.
- The constructor does not validate
Analysis chain
Ensure constructor parameters are validated to prevent initialization with invalid addresses.
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that constructor parameters are validated.
# Test: Search for the constructor usage. Expect: Validation of parameters.
rg --type solidity $'constructor'
Length of output: 65
Script:
#!/bin/bash
# Description: Verify that constructor parameters are validated.
# Test: Search for the constructor usage. Expect: Validation of parameters.
rg --type sol $'constructor'
Length of output: 55
Script:
#!/bin/bash
# Description: Verify that constructor parameters are validated.
# Test: Search for the constructor usage. Expect: Validation of parameters.
rg 'constructor' --glob '*.sol'
Length of output: 65872
| } | ||
| // else check if want is weETH, if so see if there is enough eETH to wrap and cover withdraw. | ||
| else if (address(want) == address(weETH)) { | ||
| uint256 wrapAmount = wantApprovalAmount.mulDivDown(IWEETH(address(weETH)).getRate(), 1e18) + 1; |
There was a problem hiding this comment.
Protect IWEETH(address(weETH)).getRate() call from read-only reentrancy.
- uint256 wrapAmount = wantApprovalAmount.mulDivDown(IWEETH(address(weETH)).getRate(), 1e18) + 1;
+ uint256 rate = IWEETH(address(weETH)).getRate();
+ uint256 wrapAmount = wantApprovalAmount.mulDivDown(rate, 1e18) + 1;Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| uint256 wrapAmount = wantApprovalAmount.mulDivDown(IWEETH(address(weETH)).getRate(), 1e18) + 1; | |
| uint256 rate = IWEETH(address(weETH)).getRate(); | |
| uint256 wrapAmount = wantApprovalAmount.mulDivDown(rate, 1e18) + 1; |
| if (want.balanceOf(solver) < wantApprovalAmount) { | ||
| // Check if want is eETH, if so see if there is enough weETH to unwrap and cover withdraw. | ||
| if (address(want) == address(eETH)) { | ||
| uint256 unwrapAmount = wantApprovalAmount.mulDivDown(1e18, IWEETH(address(weETH)).getRate()) + 1; |
There was a problem hiding this comment.
Protect IWEETH(address(weETH)).getRate() call from read-only reentrancy.
- uint256 unwrapAmount = wantApprovalAmount.mulDivDown(1e18, IWEETH(address(weETH)).getRate()) + 1;
+ uint256 rate = IWEETH(address(weETH)).getRate();
+ uint256 unwrapAmount = wantApprovalAmount.mulDivDown(1e18, rate) + 1;Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| uint256 unwrapAmount = wantApprovalAmount.mulDivDown(1e18, IWEETH(address(weETH)).getRate()) + 1; | |
| uint256 rate = IWEETH(address(weETH)).getRate(); | |
| uint256 unwrapAmount = wantApprovalAmount.mulDivDown(1e18, rate) + 1; |
| * @param asset a pendle asset | ||
| * @param _storage ExtensionStorage needed to price asset | ||
| */ | ||
| function setupSource(ERC20 asset, bytes memory _storage) external override onlyPriceRouter { |
There was a problem hiding this comment.
Ensure setupSource() method properly handles all asset types.
Consider refactoring the setupSource() method to handle all Pendle asset types explicitly to avoid potential issues with asset type handling.
- if (stor.kind == PendleAsset.SY) {} else {
+ if (stor.kind == PendleAsset.SY) {
+ // Handle SY type
+ } else if (stor.kind == PendleAsset.PT) {
+ // Handle PT type
+ } else if (stor.kind == PendleAsset.YT) {
+ // Handle YT type
+ } else if (stor.kind == PendleAsset.LP) {
+ // Handle LP type
+ } else {
+ revert PendleExtension__UNKNOWN_TYPE();
+ }Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| function setupSource(ERC20 asset, bytes memory _storage) external override onlyPriceRouter { | |
| function setupSource(ERC20 asset, bytes memory _storage) external override onlyPriceRouter { | |
| if (stor.kind == PendleAsset.SY) { | |
| // Handle SY type | |
| } else if (stor.kind == PendleAsset.PT) { | |
| // Handle PT type | |
| } else if (stor.kind == PendleAsset.YT) { | |
| // Handle YT type | |
| } else if (stor.kind == PendleAsset.LP) { | |
| // Handle LP type | |
| } else { | |
| revert PendleExtension__UNKNOWN_TYPE(); | |
| } |
Summary by CodeRabbit
New Features
Improvements
Bug Fixes