Skip to content
51 changes: 46 additions & 5 deletions contracts/Lens/PoolLens.sol
Original file line number Diff line number Diff line change
Expand Up @@ -136,12 +136,22 @@ contract PoolLens is ExponentialNoError, TimeManagerV8 {
BadDebt[] badDebts;
}

/// @notice Address of the core pool comptroller (all markets are returned for this pool, including paused ones)
address public immutable corePoolComptroller;

/**
* @param timeBased_ A boolean indicating whether the contract is based on time or block.
* @param blocksPerYear_ The number of blocks per year
* @param corePoolComptroller_ The address of the core pool comptroller
* @custom:oz-upgrades-unsafe-allow constructor
*/
constructor(bool timeBased_, uint256 blocksPerYear_) TimeManagerV8(timeBased_, blocksPerYear_) {}
constructor(
bool timeBased_,
uint256 blocksPerYear_,
address corePoolComptroller_
) TimeManagerV8(timeBased_, blocksPerYear_) {
corePoolComptroller = corePoolComptroller_;
}

/**
* @notice Queries the user's supply/borrow balances in vTokens
Expand Down Expand Up @@ -250,7 +260,7 @@ contract PoolLens is ExponentialNoError, TimeManagerV8 {
address account,
address comptrollerAddress
) external view returns (RewardSummary[] memory) {
VToken[] memory markets = ComptrollerInterface(comptrollerAddress).getAllMarkets();
VToken[] memory markets = _getMarkets(ComptrollerInterface(comptrollerAddress));
RewardsDistributor[] memory rewardsDistributors = ComptrollerViewInterface(comptrollerAddress)
.getRewardDistributors();
RewardSummary[] memory rewardSummary = new RewardSummary[](rewardsDistributors.length);
Expand All @@ -276,9 +286,9 @@ contract PoolLens is ExponentialNoError, TimeManagerV8 {
function getPoolBadDebt(address comptrollerAddress) external view returns (BadDebtSummary memory) {
uint256 totalBadDebtUsd;

// Get every market in the pool
// Get every listed market in the pool
ComptrollerViewInterface comptroller = ComptrollerViewInterface(comptrollerAddress);
VToken[] memory markets = comptroller.getAllMarkets();
VToken[] memory markets = _getMarkets(ComptrollerInterface(comptrollerAddress));
Comment on lines +289 to +291

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Keep bad-debt accounting on getAllMarkets().

A market can still carry badDebt() after it is unlisted or fully paused. Filtering with _getMarkets(...) here will understate both badDebts and totalBadDebtUsd for isolated pools—the exact markets most likely to have bad debt are the ones being dropped.

Suggested fix
-        VToken[] memory markets = _getMarkets(ComptrollerInterface(comptrollerAddress));
+        VToken[] memory markets = comptroller.getAllMarkets();
📝 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. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// Get every listed market in the pool
ComptrollerViewInterface comptroller = ComptrollerViewInterface(comptrollerAddress);
VToken[] memory markets = comptroller.getAllMarkets();
VToken[] memory markets = _getMarkets(ComptrollerInterface(comptrollerAddress));
// Get every listed market in the pool
ComptrollerViewInterface comptroller = ComptrollerViewInterface(comptrollerAddress);
VToken[] memory markets = comptroller.getAllMarkets();
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@contracts/Lens/PoolLens.sol` around lines 289 - 291, The code is filtering
out unlisted/paused markets by calling _getMarkets(...), which understates
badDebt and totalBadDebtUsd; replace the filtered call with the comptroller's
full market list (call getAllMarkets() on
ComptrollerViewInterface(comptrollerAddress)) instead of
_getMarkets(ComptrollerInterface(comptrollerAddress)) so you iterate every
VToken returned (including unlisted/paused) and include each token's badDebt()
and its USD contribution when computing badDebts and totalBadDebtUsd
(referencing ComptrollerViewInterface, getAllMarkets, _getMarkets, VToken[],
comptrollerAddress, and badDebt()).

ResilientOracleInterface priceOracle = comptroller.oracle();

BadDebt[] memory badDebts = new BadDebt[](markets.length);
Expand Down Expand Up @@ -344,7 +354,7 @@ contract PoolLens is ExponentialNoError, TimeManagerV8 {
// Get tokens in the Pool
ComptrollerInterface comptrollerInstance = ComptrollerInterface(venusPool.comptroller);

VToken[] memory vTokens = comptrollerInstance.getAllMarkets();
VToken[] memory vTokens = _getMarkets(comptrollerInstance);

VTokenMetadata[] memory vTokenMetadataItems = vTokenMetadataAll(vTokens);

Expand Down Expand Up @@ -453,6 +463,37 @@ contract PoolLens is ExponentialNoError, TimeManagerV8 {
});
}

/**
* @notice Returns markets from a comptroller. For the core pool, all markets are returned.
* For other pools, markets with zero internalCash are filtered.
* @param comptroller The comptroller to query
* @return An array of VToken addresses
*/
function _getMarkets(ComptrollerInterface comptroller) internal view returns (VToken[] memory) {
VToken[] memory allMarkets = comptroller.getAllMarkets();

if (address(comptroller) == corePoolComptroller) {
return allMarkets;
}

// Single-loop filter: pack active markets to the front of allMarkets
uint256 count;
uint256 len = allMarkets.length;
for (uint256 i; i < len; ++i) {
if (allMarkets[i].internalCash() > 0) {
allMarkets[count] = allMarkets[i];
++count;
}
}

// Trim the array to the active count
assembly {
mstore(allMarkets, count)
}

return allMarkets;
}

function _calculateNotDistributedAwards(
address account,
VToken[] memory markets,
Expand Down
26 changes: 24 additions & 2 deletions deploy/007-deploy-pool-lens.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,36 @@ const func: DeployFunction = async function (hre: HardhatRuntimeEnvironment) {

const { isTimeBased, blocksPerYear } = getBlockOrTimestampBasedDeploymentInfo(hre.getNetworkName());

await deploy("PoolLens", {
const corePoolComptroller = await deployments.get("Comptroller_Core");
const constructorArgs = [isTimeBased, blocksPerYear, corePoolComptroller.address];
Comment on lines +13 to +14

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

# First, explore the deploy directory structure and find related files
fd . deploy/ --type f --name "*.ts" | head -20

Repository: VenusProtocol/isolated-pools

Length of output: 322


🏁 Script executed:

# Read the specific file mentioned in the review
wc -l deploy/007-deploy-pool-lens.ts

Repository: VenusProtocol/isolated-pools

Length of output: 105


🏁 Script executed:

# Read the full context of deploy/007-deploy-pool-lens.ts
cat -n deploy/007-deploy-pool-lens.ts

Repository: VenusProtocol/isolated-pools

Length of output: 1857


🏁 Script executed:

# Search for how Comptroller_Core is used in other deploy scripts
rg "Comptroller_Core" deploy/ -A 3 -B 3

Repository: VenusProtocol/isolated-pools

Length of output: 598


🏁 Script executed:

# Check hardhat.config.ts to see network definitions and which are marked as live
cat -n hardhat.config.ts | head -100

Repository: VenusProtocol/isolated-pools

Length of output: 5299


🏁 Script executed:

# Look for skipIfAlreadyDeployed pattern in other deploy scripts
rg "skipIfAlreadyDeployed" deploy/ -B 5 -A 2

Repository: VenusProtocol/isolated-pools

Length of output: 13685


🏁 Script executed:

# Look for where Comptroller_Core is deployed
rg "Comptroller_Core" . --type ts -B 5 -A 5

Repository: VenusProtocol/isolated-pools

Length of output: 9286


🏁 Script executed:

# Check deploy/008 which deploys comptrollers
cat -n deploy/008-deploy-comptrollers.ts

Repository: VenusProtocol/isolated-pools

Length of output: 2554


🏁 Script executed:

# Search for getOrNull pattern in the codebase
rg "getOrNull|deployments\.get" deploy/ -A 2 -B 2

Repository: VenusProtocol/isolated-pools

Length of output: 5685


🏁 Script executed:

# Check the deploymentConfig to see what pools are configured
rg "Core" helpers/deploymentConfig -A 3 -B 3

Repository: VenusProtocol/isolated-pools

Length of output: 136


🏁 Script executed:

# Look for where pools are defined and if "Core" is a pool id
fd deploymentConfig.ts

Repository: VenusProtocol/isolated-pools

Length of output: 99


🏁 Script executed:

# Check if there are references to which pools should be deployed
cat -n helpers/deploymentConfig.ts | head -150

Repository: VenusProtocol/isolated-pools

Length of output: 7298


🏁 Script executed:

# Search for all pool definitions to understand pool naming
rg "pool.id|poolId" helpers/deploymentConfig.ts -B 2 -A 2

Repository: VenusProtocol/isolated-pools

Length of output: 54


🏁 Script executed:

# Check what getUnregisteredPools does - it determines which comptrollers get deployed
rg "getUnregisteredPools" helpers/ -A 10

Repository: VenusProtocol/isolated-pools

Length of output: 987


🏁 Script executed:

# Check deployment configurations for BSC networks to see if "Core" pool is defined
rg -A 500 "bscmainnet:" helpers/deploymentConfig.ts | head -150

Repository: VenusProtocol/isolated-pools

Length of output: 4733


🏁 Script executed:

# Check bsctestnet config
rg -A 500 "bsctestnet:" helpers/deploymentConfig.ts | head -150

Repository: VenusProtocol/isolated-pools

Length of output: 4680


🏁 Script executed:

# Look for where poolConfig gets the "Core" pool definition
rg "\"Core\"" helpers/deploymentConfig.ts

Repository: VenusProtocol/isolated-pools

Length of output: 659


🏁 Script executed:

# Check all deployment configuration files
fd deploymentConfig -t f

Repository: VenusProtocol/isolated-pools

Length of output: 99


🏁 Script executed:

# Look for where pool configs are actually defined with the Core pool
rg "id.*Core|Core.*poolConfig" . --type ts | head -20

Repository: VenusProtocol/isolated-pools

Length of output: 934


🏁 Script executed:

# Check the full deploymentConfig to understand when Core pool is configured
wc -l helpers/deploymentConfig.ts

Repository: VenusProtocol/isolated-pools

Length of output: 104


🏁 Script executed:

# Check the section with actual pool config definitions to see if Core is always configured
sed -n '150,500p' helpers/deploymentConfig.ts

Repository: VenusProtocol/isolated-pools

Length of output: 13887


🏁 Script executed:

# Look at deploy scripts to understand execution order - hardhat-deploy executes in numeric order
ls -la deploy/ | grep "^-" | head -30

Repository: VenusProtocol/isolated-pools

Length of output: 1866


🏁 Script executed:

# Check if deploy/007 depends on deploy/008's output
rg "dependsOn|tags.*il|func.tags" deploy/007-deploy-pool-lens.ts

Repository: VenusProtocol/isolated-pools

Length of output: 103


🏁 Script executed:

# Check deploy/008 tags
rg "dependsOn|tags.*il|func.tags" deploy/008-deploy-comptrollers.ts

Repository: VenusProtocol/isolated-pools

Length of output: 107


🏁 Script executed:

# Check if Comptroller_Core might be from external deployments in venus-protocol
rg "Comptroller_Core" node_modules/@venusprotocol/venus-protocol/deployments/ 2>/dev/null | head -20

Repository: VenusProtocol/isolated-pools

Length of output: 54


🏁 Script executed:

# Verify hardhat-deploy execution order - does it execute by numeric filename?
# Check if there are any explicit dependencies between 007 and 008
rg "dependsOn" deploy/007-deploy-pool-lens.ts deploy/008-deploy-comptrollers.ts

Repository: VenusProtocol/isolated-pools

Length of output: 54


🏁 Script executed:

# Look at how deploy/004-swap-router handles getting Comptroller_DeFi which is deployed in 008
cat -n deploy/004-swap-router.ts | head -50

Repository: VenusProtocol/isolated-pools

Length of output: 2406


🏁 Script executed:

# Check if Comptroller_DeFi actually exists or if it's also from external deployments
rg "Comptroller_DeFi" helpers/deploymentConfig.ts -B 3 -A 3

Repository: VenusProtocol/isolated-pools

Length of output: 54


🏁 Script executed:

# Search for how hardhat-deploy handles the execution when getting deployments that don't exist yet
# Check if there's any code that ensures deploy/008 runs before 007
rg "run\|tags\|il" deploy/ --type ts -B 2 -A 2 | grep -A 5 -B 5 "tags.*il"

Repository: VenusProtocol/isolated-pools

Length of output: 54


🏁 Script executed:

# Look for any hardhat.config overrides for deployment order
rg "deploymentDependencies|namedAccounts|deploy.*order" hardhat.config.ts

Repository: VenusProtocol/isolated-pools

Length of output: 90


🏁 Script executed:

# Check what happens when running deploy scripts - does deploy 008 need to run first?
# Look for any documentation or test that shows execution order
fd README -o DEPLOYMENT_ORDER -o deployment.md

Repository: VenusProtocol/isolated-pools

Length of output: 223


🏁 Script executed:

# Verify which pool names map to which comptroller deployments in deploy/008
grep -n "pool.id\|pool.name\|Comptroller_" deploy/008-deploy-comptrollers.ts

Repository: VenusProtocol/isolated-pools

Length of output: 119


🏁 Script executed:

# Check if there's a specific way the scripts are meant to be run
cat -n package.json | grep -A 5 -B 5 "deploy"

Repository: VenusProtocol/isolated-pools

Length of output: 2196


🏁 Script executed:

# Check if Comptroller_Core is provided by external venus-protocol deployments or only deployed locally
rg "Comptroller_Core" node_modules/@venusprotocol/ 2>/dev/null | head -5

Repository: VenusProtocol/isolated-pools

Length of output: 54


🏁 Script executed:

# Look at the actual pool configurations to confirm Core pool is in every network's config
grep -A 50 "export const globalConfig" helpers/deploymentConfig.ts | grep -A 20 "bscmainnet:"

Repository: VenusProtocol/isolated-pools

Length of output: 54


🏁 Script executed:

# Check if there's a deployment execution order documented or if tags affect order
cat -n deploy/006-deploy-pool-registry.ts | tail -10

Repository: VenusProtocol/isolated-pools

Length of output: 284


🏁 Script executed:

# Key insight: Check if deploy scripts that have "il" tag all need to run in sequence
# See if any deployment script references both "il" tag and a previous numbered script's deployment
rg "func.tags.*il" deploy/ -B 2 -A 5

Repository: VenusProtocol/isolated-pools

Length of output: 4164


🏁 Script executed:

# Check actual behavior - are there any snapshots or artifacts showing deployment order?
fd "deployments" --type d | head -5

Repository: VenusProtocol/isolated-pools

Length of output: 84


🏁 Script executed:

# Final confirmation: Check if hardhat-deploy truly executes numerically
# Look at the actual behavior documentation in hardhat-deploy to confirm execution order
cat -n node_modules/hardhat-deploy/package.json | grep -A 2 -B 2 "version"

Repository: VenusProtocol/isolated-pools

Length of output: 281


Use getOrNull() to handle missing Comptroller_Core on first deployment.

deployments.get("Comptroller_Core") will fail when deploying to live networks for the first time. Hardhat-deploy executes scripts numerically, so script 007 runs before script 008, which creates the Comptroller_Core deployment. On initial rollout, this artifact does not yet exist.

Possible fix
-  const corePoolComptroller = await deployments.get("Comptroller_Core");
-  const constructorArgs = [isTimeBased, blocksPerYear, corePoolComptroller.address];
+  const corePoolComptroller = await deployments.getOrNull("Comptroller_Core");
+  if (!corePoolComptroller) {
+    throw new Error("Comptroller_Core deployment not found. Run deploy/008-deploy-comptrollers.ts first.");
+  }
+  const constructorArgs = [isTimeBased, blocksPerYear, corePoolComptroller.address];
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@deploy/007-deploy-pool-lens.ts` around lines 13 - 14, Replace
deployments.get("Comptroller_Core") with await
deployments.getOrNull("Comptroller_Core") and handle the null case: assign
corePoolComptrollerAddress = corePoolComptroller?.address ??
ethers.constants.AddressZero (or otherwise skip/queue this deploy) so
constructorArgs = [isTimeBased, blocksPerYear, corePoolComptrollerAddress];
ensure the code checks for a null corePoolComptroller returned by getOrNull
before accessing .address to avoid failures on first rollout.


const poolLens = await deploy("PoolLens", {
from: deployer,
args: [isTimeBased, blocksPerYear],
args: constructorArgs,
log: true,
autoMine: true,
skipIfAlreadyDeployed: true,
Comment on lines +16 to +21

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

# First, check the deploy script file
cat -n deploy/007-deploy-pool-lens.ts

Repository: VenusProtocol/isolated-pools

Length of output: 1857


🏁 Script executed:

# Check if there are constructor args changes in this PR
git diff HEAD~1 deploy/007-deploy-pool-lens.ts 2>/dev/null || echo "Git diff not available, will check file directly"

Repository: VenusProtocol/isolated-pools

Length of output: 120


🏁 Script executed:

# Search for any references to "core-pool exemption" in the codebase
rg "core-pool" --type ts --type js -i

Repository: VenusProtocol/isolated-pools

Length of output: 54


🏁 Script executed:

# Check what PoolLens contract does and if constructor args relate to exemptions
rg "PoolLens" --type ts --type js -A 3 -B 3 | head -100

Repository: VenusProtocol/isolated-pools

Length of output: 6575


🏁 Script executed:

# Search for the PoolLens contract definition to understand the constructor
fd "PoolLens.sol" -type f

Repository: VenusProtocol/isolated-pools

Length of output: 242


🏁 Script executed:

# Search for "exemption" in the codebase to find what this refers to
rg "exemption" -i --type ts --type js --type sol -B 2 -A 2

Repository: VenusProtocol/isolated-pools

Length of output: 99


🏁 Script executed:

# Check other deploy scripts for comparison with PoolLens deployment pattern
rg "skipIfAlreadyDeployed" deploy/ --type ts -B 5 -A 5 | head -80

Repository: VenusProtocol/isolated-pools

Length of output: 5215


🏁 Script executed:

# Look for recent changes or PR context in version control
git log --oneline deploy/007-deploy-pool-lens.ts 2>/dev/null | head -5

Repository: VenusProtocol/isolated-pools

Length of output: 139


🏁 Script executed:

# Find the PoolLens contract definition
find . -name "PoolLens.sol" -o -name "PoolLens.ts" | grep -v node_modules | grep -v test

Repository: VenusProtocol/isolated-pools

Length of output: 101


🏁 Script executed:

# Search for corePoolComptroller to understand its usage
rg "corePoolComptroller" -B 3 -A 3

Repository: VenusProtocol/isolated-pools

Length of output: 50387


🏁 Script executed:

# Check what the comment says in the git commit
git show 3ba76ad --stat 2>/dev/null | head -20

Repository: VenusProtocol/isolated-pools

Length of output: 1065


🏁 Script executed:

# Look at the PoolLens contract to understand the constructor and what the core comptroller parameter does
rg "contract PoolLens" -A 30 --type-list

Repository: VenusProtocol/isolated-pools

Length of output: 461


🏁 Script executed:

# Read the PoolLens.sol contract to understand the constructor and parameter
cat contracts/Lens/PoolLens.sol | head -150

Repository: VenusProtocol/isolated-pools

Length of output: 5183


🏁 Script executed:

# Search specifically for corePoolComptroller or coreComptroller usage in PoolLens
rg "corePoolComptroller\|coreComptroller" contracts/Lens/PoolLens.sol -A 5 -B 2

Repository: VenusProtocol/isolated-pools

Length of output: 54


🏁 Script executed:

# Look for the constructor in PoolLens
ast-grep --pattern 'contract PoolLens { $$$
  constructor($_) { $$$
  }
}'

Repository: VenusProtocol/isolated-pools

Length of output: 54


🏁 Script executed:

# Search for any previous versions or references to PoolLens constructor without corePoolComptroller
rg "PoolLens\(" tests/ -B 2 -A 2 | head -40

Repository: VenusProtocol/isolated-pools

Length of output: 54


🏁 Script executed:

# Check if there are any comments or git history about when corePoolComptroller was added
git log --all --oneline --grep="PoolLens" 2>/dev/null | head -10

Repository: VenusProtocol/isolated-pools

Length of output: 659


🏁 Script executed:

# Search for the actual usage of corePoolComptroller in PoolLens to understand the behavior
rg "corePoolComptroller" contracts/Lens/PoolLens.sol -A 10 | head -60

Repository: VenusProtocol/isolated-pools

Length of output: 1540


🏁 Script executed:

# Check if PoolLens was previously deployed and what the constructor looked like before this commit
git show 3ba76ad^:contracts/Lens/PoolLens.sol 2>/dev/null | grep -A 15 "constructor" | head -20

Repository: VenusProtocol/isolated-pools

Length of output: 54


🏁 Script executed:

# Check deployments directory to see if PoolLens is already deployed on existing networks
ls -la deployments/ | grep -E "^d" | head -20

Repository: VenusProtocol/isolated-pools

Length of output: 1227


🏁 Script executed:

# Check if PoolLens was deployed in prior deployment scripts
fd "deploy.*pool.*lens" deploy/ -i

Repository: VenusProtocol/isolated-pools

Length of output: 102


🏁 Script executed:

# Verify if there's an existing deployment artifact for PoolLens
find deployments -name "PoolLens.json" 2>/dev/null | head -5

Repository: VenusProtocol/isolated-pools

Length of output: 264


🏁 Script executed:

# Check what the previous version of PoolLens constructor looked like
git show 3ba76ad~1:contracts/Lens/PoolLens.sol 2>/dev/null | grep -A 20 "constructor" | head -25

Repository: VenusProtocol/isolated-pools

Length of output: 54


🏁 Script executed:

# Check one of the existing deployment artifacts to see if it has the new constructor args
cat deployments/bscmainnet/PoolLens.json | jq '.args // .constructorArgs' 2>/dev/null

Repository: VenusProtocol/isolated-pools

Length of output: 95


🏁 Script executed:

# Get a clearer picture of what changed - check the diff for the PoolLens contract
git diff 3ba76ad~1 3ba76ad -- contracts/Lens/PoolLens.sol 2>/dev/null | head -80

Repository: VenusProtocol/isolated-pools

Length of output: 54


🏁 Script executed:

# Let's check the full git commit to see what all changed
git show 3ba76ad --stat 2>/dev/null | head -30

Repository: VenusProtocol/isolated-pools

Length of output: 1685


🏁 Script executed:

# Check if there's a deployment helper or if PoolLens gets deployed elsewhere or upgraded
rg "PoolLens" deploy/ --type ts -l

Repository: VenusProtocol/isolated-pools

Length of output: 102


🏁 Script executed:

# Verify that skipIfAlreadyDeployed really does skip when contract exists
rg "skipIfAlreadyDeployed" -B 5 -A 5 --type ts | grep -A 10 "deploy\(" | head -40

Repository: VenusProtocol/isolated-pools

Length of output: 95


🏁 Script executed:

# Check if there's an upgrade mechanism for PoolLens
rg "upgrade\|Upgrade" deploy/007-deploy-pool-lens.ts -i

Repository: VenusProtocol/isolated-pools

Length of output: 54


Constructor signature change blocks deployment on networks with existing PoolLens.

The PR adds corePoolComptroller as a required constructor parameter (changing signature from 2 to 3 args), but skipIfAlreadyDeployed: true prevents redeployment on existing networks. PoolLens is already deployed on mainnet chains (BSC, Ethereum, Arbitrum, etc.) with the old 2-parameter constructor. When this deploy script runs on those networks, it will skip entirely, leaving the old deployment in place without the corePoolComptroller parameter. This means the core-pool market filtering logic won't activate on those networks.

Either remove skipIfAlreadyDeployed: true to allow re-deployment, or implement an upgrade path using a proxy if storage changes are a concern.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@deploy/007-deploy-pool-lens.ts` around lines 16 - 21, The deployment
currently calls deploy("PoolLens", ...) with skipIfAlreadyDeployed: true which
prevents updating existing PoolLens instances after adding the new required
constructor arg corePoolComptroller; update the deploy logic so networks with an
existing PoolLens get the new constructor value: either remove
skipIfAlreadyDeployed from the deploy(...) call to force redeployment with the
new constructorArgs (ensuring deployer/args are correct), or implement an
explicit upgrade path (use a proxy/upgrade mechanism to replace PoolLens
implementation while preserving storage) and wire corePoolComptroller into the
new implementation; locate references to PoolLens, constructorArgs,
corePoolComptroller, and skipIfAlreadyDeployed to make the change.

});

if (poolLens.newlyDeployed) {
console.log("Verifying PoolLens...");
try {
await hre.run("verify:verify", {
address: poolLens.address,
constructorArguments: constructorArgs,
});
console.log("PoolLens verified successfully");
} catch (error: any) {
if (error.message.includes("Already Verified")) {
console.log("PoolLens already verified");
} else {
console.error("Verification failed:", error.message);
}
}
}
};

func.tags = ["PoolLens", "il"];
func.skip = async (hre: HardhatRuntimeEnvironment) => !hre.network.live;

export default func;
54 changes: 38 additions & 16 deletions deployments/arbitrumone/PoolLens.json

Large diffs are not rendered by default.

Large diffs are not rendered by default.

56 changes: 39 additions & 17 deletions deployments/basemainnet/PoolLens.json

Large diffs are not rendered by default.

Large diffs are not rendered by default.

56 changes: 39 additions & 17 deletions deployments/ethereum/PoolLens.json

Large diffs are not rendered by default.

139 changes: 139 additions & 0 deletions deployments/ethereum/solcInputs/09f8b2889a382752688c03578bc27f8d.json

Large diffs are not rendered by default.

56 changes: 39 additions & 17 deletions deployments/opbnbmainnet/PoolLens.json

Large diffs are not rendered by default.

Large diffs are not rendered by default.

54 changes: 38 additions & 16 deletions deployments/opmainnet/PoolLens.json

Large diffs are not rendered by default.

139 changes: 139 additions & 0 deletions deployments/opmainnet/solcInputs/09f8b2889a382752688c03578bc27f8d.json

Large diffs are not rendered by default.

56 changes: 39 additions & 17 deletions deployments/unichainmainnet/PoolLens.json

Large diffs are not rendered by default.

Large diffs are not rendered by default.

142 changes: 82 additions & 60 deletions deployments/zksyncmainnet/PoolLens.json

Large diffs are not rendered by default.

Large diffs are not rendered by default.

164 changes: 164 additions & 0 deletions tests/hardhat/Fork/PoolLensForkTest.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,164 @@
import chai from "chai";
import { ethers } from "hardhat";

import {
Comptroller,
Comptroller__factory,
PoolLens,
PoolLens__factory,
PoolRegistry__factory,
} from "../../../typechain";
import { getContractAddresses, setForkBlock } from "./utils";

const { expect } = chai;

const FORK = process.env.FORK === "true";
const FORKED_NETWORK = process.env.FORKED_NETWORK || "bscmainnet";

const { POOL_REGISTRY, COMPTROLLER, CORE_COMPTROLLER, ACC1, BLOCK_NUMBER } = getContractAddresses(
FORKED_NETWORK as string,
);

// BSC uses block-based, Arbitrum/Optimism/Base/zkSync use time-based
const TIME_BASED_NETWORKS = [
"arbitrumone",
"arbitrumsepolia",
"zksyncmainnet",
"zksyncsepolia",
"opmainnet",
"opsepolia",
"basemainnet",
"basesepolia",
"unichainmainnet",
"unichainsepolia",
];

const isTimeBased = TIME_BASED_NETWORKS.includes(FORKED_NETWORK);
const BSC_BLOCKS_PER_YEAR = 70_080_000;
const ETH_BLOCKS_PER_YEAR = 2_628_000;
const OPBNB_BLOCKS_PER_YEAR = 126_144_000;

function getBlocksPerYear(network: string): number {
if (TIME_BASED_NETWORKS.includes(network)) return 0;
if (network.includes("opbnb")) return OPBNB_BLOCKS_PER_YEAR;
if (network === "ethereum" || network === "sepolia") return ETH_BLOCKS_PER_YEAR;
return BSC_BLOCKS_PER_YEAR;
}

if (FORK) {
describe(`PoolLens Fork Test (${FORKED_NETWORK})`, function () {
this.timeout(120_000); // fork tests need more time due to RPC calls
let poolLens: PoolLens;
let comptroller: Comptroller;

before(async () => {
await setForkBlock(BLOCK_NUMBER);

const poolLensFactory = (await ethers.getContractFactory("PoolLens")) as PoolLens__factory;
poolLens = await poolLensFactory.deploy(
isTimeBased,
getBlocksPerYear(FORKED_NETWORK),
CORE_COMPTROLLER || ethers.constants.AddressZero,
);
await poolLens.deployed();

comptroller = Comptroller__factory.connect(CORE_COMPTROLLER, ethers.provider);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Comptroller connection will fail for networks with zero-address CORE_COMPTROLLER.

For bsctestnet and bscmainnet, CORE_COMPTROLLER is 0x0000..., so connecting to it creates an invalid contract instance. Tests like vTokenMetadataAll (line 111) that call comptroller.getAllMarkets() will fail.

Proposed fix
-      comptroller = Comptroller__factory.connect(CORE_COMPTROLLER, ethers.provider);
+      if (CORE_COMPTROLLER && CORE_COMPTROLLER !== ethers.constants.AddressZero) {
+        comptroller = Comptroller__factory.connect(CORE_COMPTROLLER, ethers.provider);
+      }

Then guard tests that use comptroller with a check for its existence.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/hardhat/Fork/PoolLensForkTest.ts` at line 65, The test unconditionally
calls Comptroller__factory.connect(CORE_COMPTROLLER, ethers.provider) which
creates an invalid instance when CORE_COMPTROLLER is the zero address; change
the setup to only call Comptroller__factory.connect when CORE_COMPTROLLER is not
the zero address (compare against ethers.constants.AddressZero) and set
comptroller to undefined/null otherwise, and then guard any tests that call
comptroller.getAllMarkets() (e.g. vTokenMetadataAll) by checking comptroller
exists and skipping/returning the test early when it does not.

});

describe("getAllPools", () => {
it("should not revert", async () => {
const pools = await poolLens.getAllPools(POOL_REGISTRY);
expect(pools.length).to.be.greaterThan(0);

for (const pool of pools) {
expect(pool.comptroller).to.not.equal(ethers.constants.AddressZero);
}
});
});

describe("getPoolByComptroller", () => {
it("should not revert and return all markets for core pool", async () => {
const pool = await poolLens.getPoolByComptroller(POOL_REGISTRY, CORE_COMPTROLLER);
expect(pool.comptroller).to.equal(CORE_COMPTROLLER);
expect(pool.vTokens.length).to.be.greaterThan(0);
for (const vToken of pool.vTokens) {
expect(vToken.isListed).to.equal(true);
}
});
});

describe("getPendingRewards", () => {
it("should not revert", async () => {
const account = ACC1 || ethers.constants.AddressZero;
const rewards = await poolLens.getPendingRewards(account, CORE_COMPTROLLER);
expect(rewards).to.be.an("array");
});
});

describe("getPoolBadDebt", () => {
it("should not revert", async () => {
const badDebtSummary = await poolLens.getPoolBadDebt(CORE_COMPTROLLER);
expect(badDebtSummary.comptroller).to.equal(CORE_COMPTROLLER);
expect(badDebtSummary.badDebts).to.be.an("array");
for (const badDebt of badDebtSummary.badDebts) {
expect(badDebt.vTokenAddress).to.not.equal(ethers.constants.AddressZero);
}
});
});

describe("vTokenMetadataAll", () => {
it("should not revert for markets in pool", async () => {
const allMarkets = await comptroller.getAllMarkets();

if (allMarkets.length > 0) {
const metadata = await poolLens.vTokenMetadataAll(allMarkets);
expect(metadata.length).to.equal(allMarkets.length);
}
});
});

describe("getPoolDataFromVenusPool", () => {
it("should not revert", async () => {
const poolRegistry = PoolRegistry__factory.connect(POOL_REGISTRY, ethers.provider);
const venusPool = await poolRegistry.getPoolByComptroller(CORE_COMPTROLLER);
const poolData = await poolLens.getPoolDataFromVenusPool(POOL_REGISTRY, venusPool);
expect(poolData.comptroller).to.equal(CORE_COMPTROLLER);
});
});

describe("non-core pool filters out vTokens with internalCash == 0", () => {
it("should not include vTokens with zero internalCash for non-core pools", async () => {
if (COMPTROLLER === CORE_COMPTROLLER) {
return;
}

const pools = await poolLens.getAllPools(POOL_REGISTRY);
for (const pool of pools) {
if (pool.comptroller === CORE_COMPTROLLER) continue;

for (const vToken of pool.vTokens) {
expect(vToken.totalCash).to.be.gt(0, `vToken ${vToken.vToken} in pool ${pool.comptroller} has zero cash`);
}
}
});
});

describe("core pool includes all markets", () => {
it("should include all markets for core pool", async () => {
if (!CORE_COMPTROLLER || CORE_COMPTROLLER === ethers.constants.AddressZero) {
return;
}

const coreComptroller = Comptroller__factory.connect(CORE_COMPTROLLER, ethers.provider);
const allMarkets = await coreComptroller.getAllMarkets();
const pool = await poolLens.getPoolByComptroller(POOL_REGISTRY, CORE_COMPTROLLER);
const returnedAddresses = pool.vTokens.map(v => v.vToken.toLowerCase());

expect(returnedAddresses.length).to.equal(allMarkets.length);
for (const market of allMarkets) {
expect(returnedAddresses).to.include(market.toLowerCase());
}
});
});
});
}
Loading
Loading