Harbor#2698
Open
fabiaz84 wants to merge 6 commits into
Open
Conversation
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR converts Harbor to multi-chain (ethereum, megaeth) config, adds chain-aware Chainlink pricing and composite collateral feed handling, updates reward APR calculation, and rewrites pool discovery to aggregate and emit chain-scoped pools with labeled metadata. ChangesHarbor Multi-Chain Pricing Refactor
Sequence Diagram(s)sequenceDiagram
participant Pool as Pool Discovery
participant Config as Multi-Chain Config
participant Pricing as Pricing Helpers
participant Rewards as Reward Data
Pool->>Config: lookup activeMarkets by chain
Config-->>Pool: markets with chain, label, collateralPriceFeed
Pool->>Pricing: getChainlinkFeedPriceUsd(peggedFeed, chain)
Pricing-->>Pool: pegged token USD price + decimals
Pool->>Pricing: getCollateralUsdPrice(collateralFeed, chain)
Pricing-->>Pool: collateral USD price (composite if needed)
Pool->>Rewards: query rewardData (minter)
Rewards-->>Pool: reward tokens, rates, finishAt
Pool->>Pricing: calculateAPRFromRewards(rewards)
Pricing-->>Pool: aggregated APR contributions
Pool-->>Pool: group TVL/APR by chain:pegged token and emit pools
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
The harbor adapter exports pools: Test Suites: 1 passed, 1 total |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/adaptors/harbor/index.js (1)
272-314: 💤 Low valueConsider making minter and price feed selection explicit for the same market.
The code selects
peggedTokenPriceInUnderlyingfrom the first minter that succeeds (lines 272-291) andpriceFeedKeyfrom the first market withcollateralPriceFeed(lines 300-301). For the USD pricing to be correct, both must reference the same collateral type.Currently this works because the MARKETS array happens to be ordered such that the first haUSD market (USD/PAXG) provides both values. However, if the array order changes or new markets are inserted, the minter's exchange rate could be multiplied by a mismatched collateral price.
💡 Suggestion: couple minter and price feed explicitly
- for (const market of tokenMarkets) { - if (market.minterAddress && peggedTokenPriceInUnderlying === 0) { + let selectedMarket = null; + for (const market of tokenMarkets) { + if (market.minterAddress && peggedTokenPriceInUnderlying === 0) { try { const minterPriceResult = await sdk.api.abi.call({ target: market.minterAddress, abi: MINTER_ABI.find((m) => m.name === 'peggedTokenPrice'), chain, }); if (minterPriceResult?.output) { peggedTokenPriceInUnderlying = Number(minterPriceResult.output) / 1e18; + selectedMarket = market; // ... logging ... break; } } catch (error) { continue; } } } // ... - const priceFeedKey = - tokenMarkets.find((m) => m.collateralPriceFeed)?.collateralPriceFeed || defaultFeedKey; + const priceFeedKey = + selectedMarket?.collateralPriceFeed || defaultFeedKey;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/adaptors/harbor/index.js` around lines 272 - 314, The pegged token USD price can mismatch because peggedTokenPriceInUnderlying is taken from the first successful minter while priceFeedKey is taken from the first market with collateralPriceFeed; fix by selecting a single market that provides both values (e.g., find a market where m.minterAddress && m.collateralPriceFeed) and use that market to call the minter (MINTER_ABI 'peggedTokenPrice') and to set priceFeedKey, falling back to the existing separate-search logic only if no such paired market exists; ensure you still handle errors from the SDK call and the getCollateralUsdPrice(chain, priceFeedKey) call and preserve the existing fallbacks for peggedTokenPriceInUnderlying and underlyingAssetPriceUSD.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/adaptors/harbor/config.js`:
- Around line 215-227: UNDERLYING_ASSETS currently maps haUSD to Chainlink feed
contracts instead of the ERC20 token contracts; update the haUSD values in the
UNDERLYING_ASSETS object so ethereum.haUSD is
0x2536A8636A99466173229AB15fdb37Fcaa05BA1A and megaeth.haUSD is
0xbEd2c24Cf10d7aC58350364aF8d3AbC0ce0D626f (these are the actual haUSD token
contract addresses) so that code paths using UNDERLYING_ASSETS (e.g.,
underlyingTokens) receive real token addresses rather than price feed addresses;
keep all other entries unchanged.
---
Nitpick comments:
In `@src/adaptors/harbor/index.js`:
- Around line 272-314: The pegged token USD price can mismatch because
peggedTokenPriceInUnderlying is taken from the first successful minter while
priceFeedKey is taken from the first market with collateralPriceFeed; fix by
selecting a single market that provides both values (e.g., find a market where
m.minterAddress && m.collateralPriceFeed) and use that market to call the minter
(MINTER_ABI 'peggedTokenPrice') and to set priceFeedKey, falling back to the
existing separate-search logic only if no such paired market exists; ensure you
still handle errors from the SDK call and the getCollateralUsdPrice(chain,
priceFeedKey) call and preserve the existing fallbacks for
peggedTokenPriceInUnderlying and underlyingAssetPriceUSD.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 10c67dd8-eced-4d2c-801c-75e8f61e55d6
📒 Files selected for processing (2)
src/adaptors/harbor/config.jssrc/adaptors/harbor/index.js
…price feed. Co-authored-by: Cursor <cursoragent@cursor.com>
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/adaptors/harbor/index.js (1)
272-327:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPotential pricing mismatch in fallback path for haUSD markets.
The preferred market selection (lines 272-274) correctly requires both
minterAddressandcollateralPriceFeed. However, the fallback loop (lines 295-315) setspricingMarketto any market that returns a price, regardless of whether it hascollateralPriceFeed.If the fallback market lacks
collateralPriceFeed, lines 325-327 will use a different market'scollateralPriceFeed, causing a unit mismatch:
peggedTokenPriceInUnderlyingfrom USD/stETH minter = "1 haUSD = X stETH"- Using
PAXG_USDorTBTC_USDfeed would multiply X (in stETH terms) by the wrong USD price🛠️ Suggested fix: ensure fallback also couples minter with its collateralPriceFeed
if (peggedTokenPriceInUnderlying === 0) { for (const market of tokenMarkets) { - if (!market.minterAddress) continue; + if (!market.minterAddress || !market.collateralPriceFeed) continue; try { const minterPriceResult = await sdk.api.abi.call({ target: market.minterAddress, abi: MINTER_ABI.find((m) => m.name === 'peggedTokenPrice'), chain, }); if (minterPriceResult?.output) { peggedTokenPriceInUnderlying = Number(minterPriceResult.output) / 1e18; pricingMarket = market; console.log( ` [${chain}] peggedTokenPrice from ${market.marketLabel || 'market'} minter: ${peggedTokenPriceInUnderlying.toFixed(6)} ${UNDERLYING_ASSET_DISPLAY[peggedTokenSymbol] || 'collateral units'}` ); break; } } catch (error) { continue; } } } + + // Final fallback for tokens without collateralPriceFeed (e.g., haETH, haBTC) + if (peggedTokenPriceInUnderlying === 0) { + for (const market of tokenMarkets) { + if (!market.minterAddress) continue; + try { + const minterPriceResult = await sdk.api.abi.call({ + target: market.minterAddress, + abi: MINTER_ABI.find((m) => m.name === 'peggedTokenPrice'), + chain, + }); + if (minterPriceResult?.output) { + peggedTokenPriceInUnderlying = Number(minterPriceResult.output) / 1e18; + pricingMarket = market; + break; + } + } catch (error) { + continue; + } + } + }Alternatively, a simpler approach is to only enter the fallback loop if the preferred selection failed and none of the markets have
collateralPriceFeed:if (peggedTokenPriceInUnderlying === 0) { + const hasAnyFeedMarket = tokenMarkets.some((m) => m.collateralPriceFeed); for (const market of tokenMarkets) { - if (!market.minterAddress) continue; + // For tokens with collateral-specific feeds, require the feed; otherwise allow any minter + if (!market.minterAddress || (hasAnyFeedMarket && !market.collateralPriceFeed)) continue;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/adaptors/harbor/index.js` around lines 272 - 327, The fallback loop can choose a market with a minterAddress but no collateralPriceFeed, causing peggedTokenPriceInUnderlying (from that minter) to be paired with an unrelated collateralPriceFeed later when computing priceFeedKey; modify the fallback logic in the block that iterates tokenMarkets (the loop that sets peggedTokenPriceInUnderlying and pricingMarket) to only accept and assign a market if it has both market.minterAddress and market.collateralPriceFeed (or, alternatively, when setting pricingMarket also attach/validate collateralPriceFeed exists), so that the subsequent priceFeedKey resolution (which reads pricingMarket.collateralPriceFeed) remains consistent with the minter used; ensure references to pricingMarket, tokenMarkets, peggedTokenPriceInUnderlying, market.minterAddress and market.collateralPriceFeed are updated accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/adaptors/harbor/index.js`:
- Around line 272-327: The fallback loop can choose a market with a
minterAddress but no collateralPriceFeed, causing peggedTokenPriceInUnderlying
(from that minter) to be paired with an unrelated collateralPriceFeed later when
computing priceFeedKey; modify the fallback logic in the block that iterates
tokenMarkets (the loop that sets peggedTokenPriceInUnderlying and pricingMarket)
to only accept and assign a market if it has both market.minterAddress and
market.collateralPriceFeed (or, alternatively, when setting pricingMarket also
attach/validate collateralPriceFeed exists), so that the subsequent priceFeedKey
resolution (which reads pricingMarket.collateralPriceFeed) remains consistent
with the minter used; ensure references to pricingMarket, tokenMarkets,
peggedTokenPriceInUnderlying, market.minterAddress and
market.collateralPriceFeed are updated accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: cc12a235-3ee1-402f-bdf8-5405c62e1e9a
📒 Files selected for processing (2)
src/adaptors/harbor/config.jssrc/adaptors/harbor/index.js
|
The harbor adapter exports pools: Test Suites: 1 passed, 1 total |
Only use fallback minters on markets that also have collateralPriceFeed when any market in the group has a feed, so peg ratio and Chainlink feed stay paired. Co-authored-by: Cursor <cursoragent@cursor.com>
Resolve pegged token USD inside the market loop with caching so multi-collateral haUSD pools use matching minter and Chainlink feed pairs. Co-authored-by: Cursor <cursoragent@cursor.com>
|
The harbor adapter exports pools: Test Suites: 1 passed, 1 total |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Added new markets and additional chain support MegaETH
Summary by CodeRabbit
New Features
Refactor