Closed
Conversation
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughAdds Etherlink network support throughout the application, including network configuration, RPC integration, token lists, and contract addresses. Also enhances the smart rebalance engine with constraint violation diagnostics and adds conditional Morpho app link rendering based on network support. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/utils/networks.ts (1)
26-37:⚠️ Potential issue | 🟠 MajorDon't store an empty RPC URL for Etherlink.
This new path is the only one with no Alchemy fallback, so an unset
NEXT_PUBLIC_ETHERLINK_RPCnow resolves to''. That leaves Etherlink configured with a non-endpoint instead of failing fast or using a real fallback.Also applies to: 181-181
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/utils/networks.ts` around lines 26 - 37, getRpcUrl currently can return an empty string when neither a specific RPC nor Alchemy URL is available, which causes Etherlink to be configured with a non-endpoint; update the function (getRpcUrl) to never return '' by using undefined as the final fallback (replace the literal '' in both return branches with undefined) and ensure specificRpcUrl/NEXT_PUBLIC_ETHERLINK_RPC are still sanitized to undefined (the existing targetRpc logic is fine); leave caller behavior unchanged but confirm callers handle undefined instead of an empty string.
🧹 Nitpick comments (1)
src/utils/external.ts (1)
17-19: Preferundefinedover a homepage fallback for unsupported Morpho links.
supportsMorphoAppLinks()already models the unsupported case. Returning the Morpho homepage here makes a missed guard look like a valid deep link, which is easy to ship by accident. Let these helpers returnundefinedinstead, so callers have to handle unsupported chains explicitly.Also applies to: 21-30
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/utils/external.ts` around lines 17 - 19, Change helpers to return undefined for unsupported Morpho chains instead of defaulting to the Morpho homepage: update supportsMorphoAppLinks to return getMorphoNetworkSlug(chainId) !== undefined (already OK) and modify the other helper(s) in this file that currently return the Morpho homepage fallback (referenced in lines 21-30) to return undefined when getMorphoNetworkSlug(chainId) is undefined; ensure callers must explicitly handle the undefined case rather than receiving a homepage URL.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/features/autovault/components/vault-identity.tsx`:
- Around line 96-109: The tooltip action still uses actionHref regardless of
chain gating; update the tooltip rendering to apply the same showLink &&
canLinkToMorpho guard used for interactiveContent: when rendering the tooltip
action that currently uses actionHref, only render the external Link (or anchor
with href=vaultHref/actionHref) if showLink && canLinkToMorpho, otherwise render
the non-clickable/baseContent (or a disabled/plain element). Locate
interactiveContent, baseContent, showLink, canLinkToMorpho and actionHref in the
VaultIdentity component and make the tooltip’s action follow the same
conditional pattern (preserve target, rel and stopPropagation behavior when
emitting the Link).
In `@src/features/market-detail/components/market-header.tsx`:
- Around line 633-636: The onClick handler in market-header's DropdownMenuItem
uses window.open(getMarketURL(resolvedMarketId, network), '_blank') without
security flags; update the window.open call in the MarketHeader component to
include the noopener,noreferrer feature string (e.g., third argument
'noopener,noreferrer') to prevent window.opener access and reverse-tabnabbing,
and scan other components mentioned (Navbar, AccountDropdown,
useTransactionWithToast) for similar window.open uses and apply the same change
or replace with an <a> element that includes rel="noopener noreferrer" where
appropriate.
In `@src/features/positions/components/rebalance/rebalance-modal.tsx`:
- Around line 154-156: getSmartConstraintWarning currently returns null when
diagnostics.constraintViolations is empty, which misses cases where
diagnostics.selectedCapacityShortfall or diagnostics.unallocatedAmount > 0;
update getSmartConstraintWarning to also detect positive
selectedCapacityShortfall or unallocatedAmount and return the existing warning
object (and emit the console.warn) even if constraintViolations is empty so the
banner is shown; also update any other gating logic that only checks
diagnostics.constraintViolations (the other instances referenced in the review)
to include checks for selectedCapacityShortfall and unallocatedAmount so
shortfalls trigger the same warning/console.warn behavior.
In `@src/features/positions/smart-rebalance/engine.ts`:
- Around line 269-277: The helper hasExplicitMaxAllocationConstraint currently
treats any cap <100% as "explicit" even when it's non-binding; update
hasExplicitMaxAllocationConstraint to only return true when a clamped
maxAllocationBps is defined, < 10_000, AND it actually binds the current
portfolio (i.e., entry.currentAllocationBps > maxAllocationBps). Locate
hasExplicitMaxAllocationConstraint and the analogous logic at the other spot
(lines ~431-433) and change the condition to check the entry's current
allocation (e.g., entry.currentAllocationBps) against the clamped
maxAllocationBps before marking the cap as explicit.
In `@src/utils/tokens.ts`:
- Around line 368-373: There are duplicate token records for symbol 'xU3O8' on
the same chain (etherlink) and identical address
(0x79052Ab3C166D4899a1e0DD033aC3b379AF0B1fD) which causes findToken() to always
match the first and getUniqueTokens() to potentially return both; remove or
merge the duplicate entry so each chain-scoped token (chainId + address) appears
only once, ensuring token identity follows the canonical chain-scoped rule used
by findToken() and getUniqueTokens().
---
Outside diff comments:
In `@src/utils/networks.ts`:
- Around line 26-37: getRpcUrl currently can return an empty string when neither
a specific RPC nor Alchemy URL is available, which causes Etherlink to be
configured with a non-endpoint; update the function (getRpcUrl) to never return
'' by using undefined as the final fallback (replace the literal '' in both
return branches with undefined) and ensure
specificRpcUrl/NEXT_PUBLIC_ETHERLINK_RPC are still sanitized to undefined (the
existing targetRpc logic is fine); leave caller behavior unchanged but confirm
callers handle undefined instead of an empty string.
---
Nitpick comments:
In `@src/utils/external.ts`:
- Around line 17-19: Change helpers to return undefined for unsupported Morpho
chains instead of defaulting to the Morpho homepage: update
supportsMorphoAppLinks to return getMorphoNetworkSlug(chainId) !== undefined
(already OK) and modify the other helper(s) in this file that currently return
the Morpho homepage fallback (referenced in lines 21-30) to return undefined
when getMorphoNetworkSlug(chainId) is undefined; ensure callers must explicitly
handle the undefined case rather than receiving a homepage URL.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 1730c7a2-9481-41da-8834-024569aae833
⛔ Files ignored due to path filters (5)
src/imgs/chains/etherlink.svgis excluded by!**/*.svgsrc/imgs/tokens/mbasis.pngis excluded by!**/*.pngsrc/imgs/tokens/mmev.svgis excluded by!**/*.svgsrc/imgs/tokens/mtbill.pngis excluded by!**/*.pngsrc/imgs/tokens/xu3o8.pngis excluded by!**/*.png
📒 Files selected for processing (16)
.env.local.examplesrc/config/appkit.tssrc/constants/public-allocator.tssrc/features/admin-v2/components/chain-volume-chart.tsxsrc/features/autovault/components/vault-identity.tsxsrc/features/market-detail/components/market-header.tsxsrc/features/positions/components/rebalance/rebalance-modal.tsxsrc/features/positions/smart-rebalance/engine.tssrc/features/positions/smart-rebalance/types.tssrc/store/createWagmiConfig.tssrc/types/token.tssrc/utils/external.tssrc/utils/morpho.tssrc/utils/networks.tssrc/utils/rpc.tssrc/utils/tokens.ts
Comment on lines
+96
to
+109
| const interactiveContent = | ||
| showLink && canLinkToMorpho ? ( | ||
| <Link | ||
| href={vaultHref} | ||
| target="_blank" | ||
| rel="noopener noreferrer" | ||
| className="no-underline" | ||
| onClick={(e) => e.stopPropagation()} | ||
| > | ||
| {baseContent} | ||
| </Link> | ||
| ) : ( | ||
| baseContent | ||
| ); |
There was a problem hiding this comment.
Apply Morpho-link gating to tooltip action too
The new gate only affects interactiveContent. On unsupported chains, the tooltip still renders actionHref (Lines 151-153), so users can still open Morpho from the tooltip.
Proposed fix
- actionIcon={<ExternalLinkIcon className="h-4 w-4" />}
- actionHref={vaultHref}
+ actionIcon={canLinkToMorpho ? <ExternalLinkIcon className="h-4 w-4" /> : undefined}
+ actionHref={canLinkToMorpho ? vaultHref : undefined}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/features/autovault/components/vault-identity.tsx` around lines 96 - 109,
The tooltip action still uses actionHref regardless of chain gating; update the
tooltip rendering to apply the same showLink && canLinkToMorpho guard used for
interactiveContent: when rendering the tooltip action that currently uses
actionHref, only render the external Link (or anchor with
href=vaultHref/actionHref) if showLink && canLinkToMorpho, otherwise render the
non-clickable/baseContent (or a disabled/plain element). Locate
interactiveContent, baseContent, showLink, canLinkToMorpho and actionHref in the
VaultIdentity component and make the tooltip’s action follow the same
conditional pattern (preserve target, rel and stopPropagation behavior when
emitting the Link).
Comment on lines
+633
to
+636
| {supportsMorphoAppLinks(network) && ( | ||
| <DropdownMenuItem | ||
| onClick={() => window.open(getMarketURL(resolvedMarketId, network), '_blank')} | ||
| startContent={<FiExternalLink className="h-4 w-4" />} |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find _blank window.open calls that appear to omit the 3rd-arg feature string
rg -nP --type=ts --type=tsx "window\.open\(\s*[^,]+,\s*['\"]_blank['\"]\s*\)" -C2Repository: antoncoding/monarch
Length of output: 90
🏁 Script executed:
#!/bin/bash
# Read the specific file to see lines 633-640
cat -n src/features/market-detail/components/market-header.tsx | sed -n '630,645p'Repository: antoncoding/monarch
Length of output: 784
🏁 Script executed:
#!/bin/bash
# Fix the file type syntax and search for window.open with _blank calls
rg -nP --type=ts "window\.open\([^,]+,\s*['\"]_blank['\"]\s*\)" -C2Repository: antoncoding/monarch
Length of output: 1727
🏁 Script executed:
#!/bin/bash
# Also search without type restriction to catch tsx files
fd -e ts -e tsx -x grep -n "window\.open" {} + | grep "_blank" -C2Repository: antoncoding/monarch
Length of output: 23504
Add security flags to window.open call
Line 635 opens a new tab without noopener,noreferrer flags, leaving the opened page access to the window.opener property. Add the flags to block reverse-tabnabbing attacks.
Proposed fix
- onClick={() => window.open(getMarketURL(resolvedMarketId, network), '_blank')}
+ onClick={() => window.open(getMarketURL(resolvedMarketId, network), '_blank', 'noopener,noreferrer')}Note: Several other files in the codebase also lack these flags (e.g., Navbar.tsx, AccountDropdown.tsx, useTransactionWithToast.tsx), so consider a broader audit and fix.
📝 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
| {supportsMorphoAppLinks(network) && ( | |
| <DropdownMenuItem | |
| onClick={() => window.open(getMarketURL(resolvedMarketId, network), '_blank')} | |
| startContent={<FiExternalLink className="h-4 w-4" />} | |
| {supportsMorphoAppLinks(network) && ( | |
| <DropdownMenuItem | |
| onClick={() => window.open(getMarketURL(resolvedMarketId, network), '_blank', 'noopener,noreferrer')} | |
| startContent={<FiExternalLink className="h-4 w-4" />} |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/features/market-detail/components/market-header.tsx` around lines 633 -
636, The onClick handler in market-header's DropdownMenuItem uses
window.open(getMarketURL(resolvedMarketId, network), '_blank') without security
flags; update the window.open call in the MarketHeader component to include the
noopener,noreferrer feature string (e.g., third argument 'noopener,noreferrer')
to prevent window.opener access and reverse-tabnabbing, and scan other
components mentioned (Navbar, AccountDropdown, useTransactionWithToast) for
similar window.open uses and apply the same change or replace with an <a>
element that includes rel="noopener noreferrer" where appropriate.
Comment on lines
+154
to
+156
| function getSmartConstraintWarning(plan: SmartRebalancePlan | null): { title: string; detail: string } | null { | ||
| const violations = plan?.diagnostics.constraintViolations ?? []; | ||
| if (violations.length === 0) return null; |
There was a problem hiding this comment.
Warn on global capacity shortfalls even without per-market violations.
selectedCapacityShortfall / unallocatedAmount can be positive when every chosen market lands exactly at its cap, which leaves constraintViolations empty. With the current gates, that suppresses both the banner and the console.warn, so users get no signal that some funds will stay in the wallet.
Also applies to: 335-359, 1185-1197
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/features/positions/components/rebalance/rebalance-modal.tsx` around lines
154 - 156, getSmartConstraintWarning currently returns null when
diagnostics.constraintViolations is empty, which misses cases where
diagnostics.selectedCapacityShortfall or diagnostics.unallocatedAmount > 0;
update getSmartConstraintWarning to also detect positive
selectedCapacityShortfall or unallocatedAmount and return the existing warning
object (and emit the console.warn) even if constraintViolations is empty so the
banner is shown; also update any other gating logic that only checks
diagnostics.constraintViolations (the other instances referenced in the review)
to include checks for selectedCapacityShortfall and unallocatedAmount so
shortfalls trigger the same warning/console.warn behavior.
Comment on lines
+269
to
+277
| function hasExplicitMaxAllocationConstraint( | ||
| entries: SmartRebalanceEngineInput['entries'], | ||
| constraints: SmartRebalanceConstraintMap | undefined, | ||
| ): boolean { | ||
| return entries.some((entry) => { | ||
| const maxAllocationBps = clampBps(constraints?.[entry.uniqueKey]?.maxAllocationBps); | ||
| return maxAllocationBps !== undefined && maxAllocationBps < 10_000; | ||
| }); | ||
| } |
There was a problem hiding this comment.
Don't drop the reliability guard for non-binding caps.
Any cap below 100% makes hasExplicitConstraints true, even when the current allocation already satisfies every cap. In that case the current portfolio is still a valid constrained plan, so skipping the guard lets the greedy allocator return a lower weighted APY for no reason.
Possible fix
- const hasExplicitConstraints = hasExplicitMaxAllocationConstraint(entries, constraints);
-
// 1. Simulate extra liquidity and start state:
// - attempt best-effort withdrawal from each selected market
// - keep any non-withdrawable remainder locked in-place
const cleaned = cleanStates(entries, constraints, totalPool);
+ const currentAllocationViolatesConstraints = entries.some((entry) => {
+ const maxAllocation = cleaned.maxAllocationMap.get(entry.uniqueKey);
+ return maxAllocation !== undefined && entry.currentSupply > maxAllocation;
+ });
@@
- if (!hasExplicitConstraints && projectedObjective < currentObjective) {
+ if (!currentAllocationViolatesConstraints && projectedObjective < currentObjective) {Also applies to: 431-433
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/features/positions/smart-rebalance/engine.ts` around lines 269 - 277, The
helper hasExplicitMaxAllocationConstraint currently treats any cap <100% as
"explicit" even when it's non-binding; update hasExplicitMaxAllocationConstraint
to only return true when a clamped maxAllocationBps is defined, < 10_000, AND it
actually binds the current portfolio (i.e., entry.currentAllocationBps >
maxAllocationBps). Locate hasExplicitMaxAllocationConstraint and the analogous
logic at the other spot (lines ~431-433) and change the condition to check the
entry's current allocation (e.g., entry.currentAllocationBps) against the
clamped maxAllocationBps before marking the cap as explicit.
Comment on lines
+368
to
+373
| { | ||
| symbol: 'xU3O8', | ||
| img: undefined, | ||
| decimals: 18, | ||
| networks: [{ chain: etherlink, address: '0x79052Ab3C166D4899a1e0DD033aC3b379AF0B1fD' }], | ||
| }, |
There was a problem hiding this comment.
Deduplicate the Etherlink xU3O8 entry.
These two records resolve to the same chainId + address. findToken() will always hit the first one, and getUniqueTokens() can return both, so the later Midas/image metadata is effectively dead.
♻️ One simple fix
- {
- symbol: 'xU3O8',
- img: undefined,
- decimals: 18,
- networks: [{ chain: etherlink, address: '0x79052Ab3C166D4899a1e0DD033aC3b379AF0B1fD' }],
- },As per coding guidelines, "All market/token/route identity checks must be chain-scoped using canonical identifiers (chainId + market.uniqueKey or chainId + address)."
Also applies to: 421-429
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/utils/tokens.ts` around lines 368 - 373, There are duplicate token
records for symbol 'xU3O8' on the same chain (etherlink) and identical address
(0x79052Ab3C166D4899a1e0DD033aC3b379AF0B1fD) which causes findToken() to always
match the first and getUniqueTokens() to potentially return both; remove or
merge the duplicate entry so each chain-scoped token (chainId + address) appears
only once, ensuring token identity follows the canonical chain-scoped rule used
by findToken() and getUniqueTokens().
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.
Summary by CodeRabbit
Release Notes