feat (explorer): add wrapper designation to order details#7404
Conversation
Renders a "Wrappers" row in the verbose order details table for orders whose appData references known wrapper contracts. Wrapper-specific UI is code-split and loaded on demand; unknown addresses fall back to fetching the contract name() on-chain. Supported wrappers: - OrderFlowFactory (bridge & swap) - Euler EVC – Open Position, Close Position, Collateral Swap Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughDecodes order appData to resolve wrappers via a registry, adds Euler EVC wrapper views and hooks (with cached Viem clients), provides shared UI/styles, inserts a "Wrappers" DetailRow into order details (with tooltip), updates AGENTS.md and reorders app dependencies, and adds FiniteMap + tests. ChangesWrapper Smart Contract Display in Explorer
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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 |
|
All contributors have signed the CLA ✍️ ✅ |
shouldn't be in this pr
|
I have read the CLA Document and I hereby sign the CLA |
Deploying swap-dev with
|
| Latest commit: |
787c8bc
|
| Status: | 🚫 Build failed. |
|
Found a few regressions / polish items to address before merge:
|
* docs(explorer): add UI and code conventions to AGENTS.md Standards identified through review feedback on #7404: - Link display: ↗ suffix rules, parenthesis placement, flex gap pitfall - Token display: TokenImg icon placement, orange link colour via Color.explorer_textActive - Text wrapping: word-break override inside RenderedData - State caching: use jotai/jotai-family instead of module-level Map - Address utilities: AddressKey type, shortenAddress, getAddressKey for lookups - React imports: named imports over React.X prefix - Contract reads: batch with multicall instead of separate readContract calls - Hook organisation: one hook per file under a domain directory - Component shape: *View sub-component pattern for lint compliance - Pre-commit checklist: lint and dead-code removal Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * try to fix pnpm lock --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/explorer/src/components/orders/DetailsTable/VerboseDetails.tsx (1)
16-16: ⚡ Quick winUse a same-directory relative import instead of the round-trip path.
The current file lives in
.../DetailsTable/;../../../components/orders/DetailsTable/climbs three levels tosrc/and navigates right back to the same folder. This should be a simple relative import.♻️ Proposed fix
-import { DetailsTableTooltips } from '../../../components/orders/DetailsTable/detailsTableTooltips' +import { DetailsTableTooltips } from './detailsTableTooltips'🤖 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 `@apps/explorer/src/components/orders/DetailsTable/VerboseDetails.tsx` at line 16, The import in VerboseDetails.tsx uses a round-trip path to the same directory; replace the top-level import "import { DetailsTableTooltips } from '../../../components/orders/DetailsTable/detailsTableTooltips'" with a same-directory relative import (e.g., "import { DetailsTableTooltips } from './detailsTableTooltips'") to avoid unnecessary path traversal and keep the module reference local to the DetailsTable folder.
🤖 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.
Nitpick comments:
In `@apps/explorer/src/components/orders/DetailsTable/VerboseDetails.tsx`:
- Line 16: The import in VerboseDetails.tsx uses a round-trip path to the same
directory; replace the top-level import "import { DetailsTableTooltips } from
'../../../components/orders/DetailsTable/detailsTableTooltips'" with a
same-directory relative import (e.g., "import { DetailsTableTooltips } from
'./detailsTableTooltips'") to avoid unnecessary path traversal and keep the
module reference local to the DetailsTable folder.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 3169152e-f5a9-4a74-841c-1c7fe19dd09f
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (18)
apps/explorer/AGENTS.mdapps/explorer/package.jsonapps/explorer/src/components/orders/DetailsTable/VerboseDetails.tsxapps/explorer/src/components/orders/DetailsTable/detailsTableTooltips.tsxapps/explorer/src/components/orders/OrderWrapperDetails/index.tsxapps/explorer/src/hooks/euler/abis.tsapps/explorer/src/hooks/euler/client.tsapps/explorer/src/hooks/euler/index.tsapps/explorer/src/hooks/euler/useContractName.tsapps/explorer/src/hooks/euler/useConvertToAssets.tsapps/explorer/src/hooks/euler/useVaultAsset.tsapps/explorer/src/utils/getOrderWrappers.tsapps/explorer/src/utils/wrapperRegistry.tsapps/explorer/src/utils/wrappers/eulerEvc/closePosition.tsxapps/explorer/src/utils/wrappers/eulerEvc/collateralSwap.tsxapps/explorer/src/utils/wrappers/eulerEvc/common.tsxapps/explorer/src/utils/wrappers/eulerEvc/index.tsapps/explorer/src/utils/wrappers/eulerEvc/openPosition.tsx
✅ Files skipped from review due to trivial changes (8)
- apps/explorer/src/components/orders/DetailsTable/detailsTableTooltips.tsx
- apps/explorer/src/utils/wrappers/eulerEvc/index.ts
- apps/explorer/src/hooks/euler/index.ts
- apps/explorer/src/utils/wrapperRegistry.ts
- apps/explorer/package.json
- apps/explorer/src/utils/getOrderWrappers.ts
- apps/explorer/src/utils/wrappers/eulerEvc/common.tsx
- apps/explorer/src/hooks/euler/abis.ts
🚧 Files skipped from review as they are similar to previous changes (8)
- apps/explorer/src/hooks/euler/useConvertToAssets.ts
- apps/explorer/src/hooks/euler/useContractName.ts
- apps/explorer/src/utils/wrappers/eulerEvc/openPosition.tsx
- apps/explorer/src/utils/wrappers/eulerEvc/collateralSwap.tsx
- apps/explorer/src/hooks/euler/useVaultAsset.ts
- apps/explorer/src/components/orders/OrderWrapperDetails/index.tsx
- apps/explorer/src/utils/wrappers/eulerEvc/closePosition.tsx
- apps/explorer/src/hooks/euler/client.ts
There was a problem hiding this comment.
Regarding the changes here, I don't think this is an issue. We have a PR ready to pin the versions, so this will soon be more stable: #7450
There was a problem hiding this comment.
OrderWrapperDetails.tsx might be a better name.
See https://www.notion.so/cownation/CoW-Swap-React-Dapp-Architecture-1858da5f04ca806b9796cfd63c982275.
There was a problem hiding this comment.
OrderWrapperDetails.styled.tsx
| ) | ||
| } | ||
|
|
||
| export function ClosePositionComponent({ data }: { data: string }): ReactElement | null { |
There was a problem hiding this comment.
Ideally, create 2 different files and export 1 component from each of them, with a matching name. Also, avoid the "Component" suffix. I suggest:
- ClosePositionComponent => ClosePosition in ClosePosition.tsx
- ClosePositionView in ClosePositionView.tsx
There was a problem hiding this comment.
This one should probably be broken down into individual files.
| 'bytes signature', | ||
| ]) | ||
|
|
||
| type OpenPositionParams = { |
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (2)
apps/explorer/src/components/orders/OrderWrapperDetails/OrderWrapperDetails.styled.tsx (1)
10-10: 💤 Low value
flex-flow: column wrap— thewraphas no effect without a fixed height
wrapin a column flex container only triggers when the container has a constrainedheight. Since noheightis set here, items will always stack in a single column andwrapis inert. Consider simplifying toflex-direction: columnto make the intent explicit.♻️ Proposed simplification
- flex-flow: column wrap; + flex-direction: column;🤖 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 `@apps/explorer/src/components/orders/OrderWrapperDetails/OrderWrapperDetails.styled.tsx` at line 10, The rule "flex-flow: column wrap" in the styled component (OrderWrapperDetails) is misleading because `wrap` is no-op without a constrained height; update the styling to either set an explicit constrained height if wrapping is intended or, more simply, replace `flex-flow: column wrap` with `flex-direction: column` in the OrderWrapperDetails styled component so the intent is explicit and wrapping is not implied.apps/explorer/src/components/orders/OrderWrapperDetails/item/OrderWrapperDetailsItem.component.tsx (1)
18-29: ⚡ Quick winExtract a
*Viewsub-component to satisfy the lint complexity guideline.This component has five hooks inline (
useState,useEffect,useNetworkId,useContractName,usePopperDefault). The explorer coding guideline specifically targets this pattern:Wrapper components that render decoded data in the explorer app SHOULD extract a
*Viewsub-component with all props passed explicitly and no hooks, to satisfy lint complexity limits.Derive all values from hooks in
OrderWrapperDetailsItem, then pass them as plain props to anOrderWrapperDetailsItemViewthat contains only JSX. This also unblocks the lint failures mentioned in the PR thread.As per coding guidelines: "Wrapper components that render decoded data in the explorer app SHOULD extract a
*Viewsub-component with all props passed explicitly and no hooks, to satisfy lint complexity limits."🤖 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 `@apps/explorer/src/components/orders/OrderWrapperDetails/item/OrderWrapperDetailsItem.component.tsx` around lines 18 - 29, OrderWrapperDetailsItem currently runs hooks (useState, useEffect, useNetworkId, useContractName, usePopperDefault) and renders JSX; extract a pure presentation component OrderWrapperDetailsItemView that accepts all derived values as plain props (e.g. Component, info, address, data, isOmittable, chainId, isUnknown, contractName, tooltipProps, targetProps) and contains only JSX/no hooks. In OrderWrapperDetailsItem keep all hook logic: compute Component via wrapper.loadComponent(), call useNetworkId(), useContractName(), usePopperDefault(), derive isUnknown and other values, then render <OrderWrapperDetailsItemView {...props} /> passing explicit props only. Ensure wrapper.loadComponent usage and setComponent logic remain in OrderWrapperDetailsItem and that OrderWrapperDetailsItemView does not call any hooks.
🤖 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
`@apps/explorer/src/components/orders/OrderWrapperDetails/item/OrderWrapperDetailsItem.component.tsx`:
- Around line 26-29: The useEffect is re-running because it depends on the whole
wrapper object; replace that with stable primitive identities: change the dep
array to depend on data's stable identifier and the load function itself (e.g.,
[data?.id, wrapper.loadComponent]) instead of [wrapper, data]; inside the effect
guard on data and wrapper.loadComponent (e.g., if (!data ||
!wrapper.loadComponent) return) and call wrapper.loadComponent().then((Comp) =>
setComponent(() => Comp)); if there is no stable primitive on data, ask the
parent to provide a stable key (or memoize wrapper/loadComponent in
getOrderWrappers) so the effect only fires when the actual component or data
identity changes.
In `@apps/explorer/src/utils/wrappers/eulerEvc/EulerWrappers.styles.tsx`:
- Around line 139-176: Change the address types to AddressKey and normalize
decoded viem Addresses: update TokenLink.tokenAddress to AddressKey | undefined,
OwnerLink.address to AddressKey, and subaccountNumber(owner: AddressKey,
account: AddressKey); update any downstream .pure.tsx param interfaces and
.component.tsx decoders to accept AddressKey and, where viem decoders return
Address (0x${string}), call getAddressKey() to convert before passing/storing
the value so all exported functions (TokenLink, OwnerLink, subaccountNumber) use
the AddressKey type consistently.
In
`@apps/explorer/src/utils/wrappers/eulerEvc/positions/close/EulerClosePosition.pure.tsx`:
- Around line 21-28: The interface EulerClosePositionParams currently types
address fields as string; update the types of owner, account, borrowVault, and
collateralVault to AddressKey (from `@cowprotocol/cow-sdk`) and in the companion
.component.tsx where you map the decoded viem tuple into an
EulerClosePositionParams instance, wrap the decoded address strings with
getAddressKey() before assignment so the params object uses AddressKey-typed
addresses.
In
`@apps/explorer/src/utils/wrappers/eulerEvc/positions/collateral/EulerCollateralSwap.pure.tsx`:
- Around line 25-33: The EulerCollateralSwapParams interface currently types
owner, account, fromVault, and toVault as string; change these four fields to
AddressKey (imported from `@cowprotocol/cow-sdk`) and update any code constructing
EulerCollateralSwapParams (notably in the corresponding .component.tsx) to call
getAddressKey() on decoded viem Address values before passing them in so the
params object uses AddressKey types.
In
`@apps/explorer/src/utils/wrappers/eulerEvc/positions/open/EulerOpenPosition.pure.tsx`:
- Around line 20-28: Update the EulerOpenPositionParams type so the Ethereum
address fields owner, account, collateralVault, and borrowVault use AddressKey
(imported from `@cowprotocol/cow-sdk`) instead of string; then in the companion
.component.tsx convert the decoded viem tuple values to AddressKey by calling
getAddressKey(...) when assigning those fields (use getAddressKey from
`@cowprotocol/cow-sdk`) so the params and assignment are consistently typed.
---
Nitpick comments:
In
`@apps/explorer/src/components/orders/OrderWrapperDetails/item/OrderWrapperDetailsItem.component.tsx`:
- Around line 18-29: OrderWrapperDetailsItem currently runs hooks (useState,
useEffect, useNetworkId, useContractName, usePopperDefault) and renders JSX;
extract a pure presentation component OrderWrapperDetailsItemView that accepts
all derived values as plain props (e.g. Component, info, address, data,
isOmittable, chainId, isUnknown, contractName, tooltipProps, targetProps) and
contains only JSX/no hooks. In OrderWrapperDetailsItem keep all hook logic:
compute Component via wrapper.loadComponent(), call useNetworkId(),
useContractName(), usePopperDefault(), derive isUnknown and other values, then
render <OrderWrapperDetailsItemView {...props} /> passing explicit props only.
Ensure wrapper.loadComponent usage and setComponent logic remain in
OrderWrapperDetailsItem and that OrderWrapperDetailsItemView does not call any
hooks.
In
`@apps/explorer/src/components/orders/OrderWrapperDetails/OrderWrapperDetails.styled.tsx`:
- Line 10: The rule "flex-flow: column wrap" in the styled component
(OrderWrapperDetails) is misleading because `wrap` is no-op without a
constrained height; update the styling to either set an explicit constrained
height if wrapping is intended or, more simply, replace `flex-flow: column wrap`
with `flex-direction: column` in the OrderWrapperDetails styled component so the
intent is explicit and wrapping is not implied.
🪄 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: 7635ef30-8673-4a69-b00e-4ec5adde876d
📒 Files selected for processing (18)
apps/explorer/src/components/orders/DetailsTable/VerboseDetails.tsxapps/explorer/src/components/orders/OrderWrapperDetails/OrderWrapperDetails.component.tsxapps/explorer/src/components/orders/OrderWrapperDetails/OrderWrapperDetails.provider.tsxapps/explorer/src/components/orders/OrderWrapperDetails/OrderWrapperDetails.styled.tsxapps/explorer/src/components/orders/OrderWrapperDetails/item/OrderWrapperDetailsItem.component.tsxapps/explorer/src/components/orders/OrderWrapperDetails/item/OrderWrapperDetailsItem.styled.tsxapps/explorer/src/utils/getOrderWrappers.tsapps/explorer/src/utils/wrappers/eulerEvc/EulerWrappers.styles.tsxapps/explorer/src/utils/wrappers/eulerEvc/index.tsapps/explorer/src/utils/wrappers/eulerEvc/positions/close/EulerClosePosition.component.tsxapps/explorer/src/utils/wrappers/eulerEvc/positions/close/EulerClosePosition.pure.tsxapps/explorer/src/utils/wrappers/eulerEvc/positions/collateral/EulerCollateralSwap.component.tsxapps/explorer/src/utils/wrappers/eulerEvc/positions/collateral/EulerCollateralSwap.pure.tsxapps/explorer/src/utils/wrappers/eulerEvc/positions/collateral/EulerCollateralSwap.styled.tsapps/explorer/src/utils/wrappers/eulerEvc/positions/open/EulerOpenPosition.component.tsxapps/explorer/src/utils/wrappers/eulerEvc/positions/open/EulerOpenPosition.pure.tsxapps/explorer/src/utils/wrappers/wrapperRegistry.constants.tsapps/explorer/src/utils/wrappers/wrapperRegistry.types.ts
✅ Files skipped from review due to trivial changes (5)
- apps/explorer/src/components/orders/OrderWrapperDetails/OrderWrapperDetails.provider.tsx
- apps/explorer/src/utils/wrappers/wrapperRegistry.constants.ts
- apps/explorer/src/components/orders/OrderWrapperDetails/OrderWrapperDetails.component.tsx
- apps/explorer/src/utils/wrappers/eulerEvc/positions/collateral/EulerCollateralSwap.styled.ts
- apps/explorer/src/utils/wrappers/wrapperRegistry.types.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/explorer/src/utils/getOrderWrappers.ts
- apps/explorer/src/components/orders/DetailsTable/VerboseDetails.tsx
| export function TokenLink({ | ||
| symbol, | ||
| tokenAddress, | ||
| chainId, | ||
| }: { | ||
| symbol: string | ||
| tokenAddress: string | undefined | ||
| chainId: SupportedChainId | null | ||
| }): ReactElement { | ||
| if (!tokenAddress || !chainId) return <>{symbol}</> | ||
| const url = getBlockExplorerUrl(chainId, 'token', tokenAddress) | ||
| return ( | ||
| <> | ||
| <TokenIcon address={tokenAddress} network={chainId} symbol={symbol} /> | ||
| <a href={url} target="_blank" rel="noopener noreferrer" title={tokenAddress}> | ||
| {symbol} | ||
| </a> | ||
| </> | ||
| ) | ||
| } | ||
|
|
||
| export function OwnerLink({ address }: { address: string }): ReactElement { | ||
| return ( | ||
| <Link to={`/address/${address}`} title={address}> | ||
| {shortenAddress(address)}↗ | ||
| </Link> | ||
| ) | ||
| } | ||
|
|
||
| export function formatAmount(raw: bigint | undefined, decimals: number | undefined): string { | ||
| if (raw === undefined) return '…' | ||
| if (decimals === undefined) return raw.toString() | ||
| return (Number(raw) / 10 ** decimals).toLocaleString(undefined, { maximumFractionDigits: 6 }) | ||
| } | ||
|
|
||
| export function subaccountNumber(owner: string, account: string): number { | ||
| return parseInt(owner.slice(-2), 16) ^ parseInt(account.slice(-2), 16) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Address parameters must be typed as AddressKey, not string
Three exported symbols use plain string for addresses where the guideline mandates AddressKey:
TokenLink.tokenAddress: string | undefined(line 144) →AddressKey | undefinedOwnerLink.address: string(line 160) →AddressKeysubaccountNumber(owner: string, account: string)(line 174) → both paramsAddressKey
The downstream .pure.tsx param interfaces and .component.tsx decoders will also need updating: decoded viem Address values (0x${string}) should be converted with getAddressKey() before being stored.
♻️ Proposed fix
-import { getBlockExplorerUrl, shortenAddress } from '@cowprotocol/common-utils'
-import { SupportedChainId } from '@cowprotocol/cow-sdk'
+import { getBlockExplorerUrl, shortenAddress } from '@cowprotocol/common-utils'
+import { AddressKey, SupportedChainId } from '@cowprotocol/cow-sdk'
export function TokenLink({
symbol,
tokenAddress,
chainId,
}: {
symbol: string
- tokenAddress: string | undefined
+ tokenAddress: AddressKey | undefined
chainId: SupportedChainId | null
}): ReactElement {
export function OwnerLink({ address }: { address: AddressKey }): ReactElement {
-export function OwnerLink({ address }: { address: string }): ReactElement {
-export function subaccountNumber(owner: string, account: string): number {
+export function subaccountNumber(owner: AddressKey, account: AddressKey): number {As per coding guidelines: "Type address parameters as AddressKey from @cowprotocol/cow-sdk, not plain string, in the explorer app."
🤖 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 `@apps/explorer/src/utils/wrappers/eulerEvc/EulerWrappers.styles.tsx` around
lines 139 - 176, Change the address types to AddressKey and normalize decoded
viem Addresses: update TokenLink.tokenAddress to AddressKey | undefined,
OwnerLink.address to AddressKey, and subaccountNumber(owner: AddressKey,
account: AddressKey); update any downstream .pure.tsx param interfaces and
.component.tsx decoders to accept AddressKey and, where viem decoders return
Address (0x${string}), call getAddressKey() to convert before passing/storing
the value so all exported functions (TokenLink, OwnerLink, subaccountNumber) use
the AddressKey type consistently.
| export interface EulerClosePositionParams { | ||
| owner: string | ||
| account: string | ||
| deadline: bigint | ||
| borrowVault: string | ||
| collateralVault: string | ||
| collateralAmount: bigint | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
EulerClosePositionParams address fields must be AddressKey, not string
owner, account, borrowVault, and collateralVault are Ethereum addresses and must be typed as AddressKey. Apply getAddressKey() in the companion .component.tsx when assigning from the decoded viem tuple.
♻️ Proposed fix
+import { AddressKey } from '@cowprotocol/cow-sdk'
export interface EulerClosePositionParams {
- owner: string
- account: string
+ owner: AddressKey
+ account: AddressKey
deadline: bigint
- borrowVault: string
- collateralVault: string
+ borrowVault: AddressKey
+ collateralVault: AddressKey
collateralAmount: bigint
}As per coding guidelines: "Type address parameters as AddressKey from @cowprotocol/cow-sdk, not plain string, in the explorer app."
🤖 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
`@apps/explorer/src/utils/wrappers/eulerEvc/positions/close/EulerClosePosition.pure.tsx`
around lines 21 - 28, The interface EulerClosePositionParams currently types
address fields as string; update the types of owner, account, borrowVault, and
collateralVault to AddressKey (from `@cowprotocol/cow-sdk`) and in the companion
.component.tsx where you map the decoded viem tuple into an
EulerClosePositionParams instance, wrap the decoded address strings with
getAddressKey() before assignment so the params object uses AddressKey-typed
addresses.
| export interface EulerCollateralSwapParams { | ||
| owner: string | ||
| account: string | ||
| deadline: bigint | ||
| fromVault: string | ||
| toVault: string | ||
| fromAmount: bigint | ||
| disableSourceCollateral: boolean | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
EulerCollateralSwapParams address fields must be AddressKey, not string
owner, account, fromVault, and toVault are Ethereum addresses and must be typed as AddressKey. The corresponding .component.tsx should apply getAddressKey() to the decoded viem Address values before constructing this params object.
♻️ Proposed fix
+import { AddressKey } from '@cowprotocol/cow-sdk'
export interface EulerCollateralSwapParams {
- owner: string
- account: string
+ owner: AddressKey
+ account: AddressKey
deadline: bigint
- fromVault: string
- toVault: string
+ fromVault: AddressKey
+ toVault: AddressKey
fromAmount: bigint
disableSourceCollateral: boolean
}As per coding guidelines: "Type address parameters as AddressKey from @cowprotocol/cow-sdk, not plain string, in the explorer app."
🤖 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
`@apps/explorer/src/utils/wrappers/eulerEvc/positions/collateral/EulerCollateralSwap.pure.tsx`
around lines 25 - 33, The EulerCollateralSwapParams interface currently types
owner, account, fromVault, and toVault as string; change these four fields to
AddressKey (imported from `@cowprotocol/cow-sdk`) and update any code constructing
EulerCollateralSwapParams (notably in the corresponding .component.tsx) to call
getAddressKey() on decoded viem Address values before passing them in so the
params object uses AddressKey types.
There was a problem hiding this comment.
@limitofzero I'm not sure if this is right. Are we supposed to use AddressKey everywhere and call getAddressKey() for each address? The name and TSDoc there might be a bit missleading, so if an address is supposed to be just that, a plain address, and not used as a key, does this guideline still apply?
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
| export interface EulerOpenPositionParams { | ||
| owner: string | ||
| account: string | ||
| deadline: bigint | ||
| collateralVault: string | ||
| borrowVault: string | ||
| collateralAmount: bigint | ||
| borrowAmount: bigint | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
EulerOpenPositionParams address fields must be AddressKey, not string
owner, account, collateralVault, and borrowVault are Ethereum addresses and must be typed as AddressKey. Apply getAddressKey() in the companion .component.tsx when assigning from the decoded viem tuple.
♻️ Proposed fix
+import { AddressKey } from '@cowprotocol/cow-sdk'
export interface EulerOpenPositionParams {
- owner: string
- account: string
+ owner: AddressKey
+ account: AddressKey
deadline: bigint
- collateralVault: string
- borrowVault: string
+ collateralVault: AddressKey
+ borrowVault: AddressKey
collateralAmount: bigint
borrowAmount: bigint
}As per coding guidelines: "Type address parameters as AddressKey from @cowprotocol/cow-sdk, not plain string, in the explorer app."
📝 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.
| export interface EulerOpenPositionParams { | |
| owner: string | |
| account: string | |
| deadline: bigint | |
| collateralVault: string | |
| borrowVault: string | |
| collateralAmount: bigint | |
| borrowAmount: bigint | |
| } | |
| import { AddressKey } from '@cowprotocol/cow-sdk' | |
| export interface EulerOpenPositionParams { | |
| owner: AddressKey | |
| account: AddressKey | |
| deadline: bigint | |
| collateralVault: AddressKey | |
| borrowVault: AddressKey | |
| collateralAmount: bigint | |
| borrowAmount: bigint | |
| } |
🤖 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
`@apps/explorer/src/utils/wrappers/eulerEvc/positions/open/EulerOpenPosition.pure.tsx`
around lines 20 - 28, Update the EulerOpenPositionParams type so the Ethereum
address fields owner, account, collateralVault, and borrowVault use AddressKey
(imported from `@cowprotocol/cow-sdk`) instead of string; then in the companion
.component.tsx convert the decoded viem tuple values to AddressKey by calling
getAddressKey(...) when assigning those fields (use getAddressKey from
`@cowprotocol/cow-sdk`) so the params and assignment are consistently typed.
@kaze-cow I removed In any case, |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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 `@apps/explorer/src/utils/wrappers/eulerEvc/components/TokenLink.pure.tsx`:
- Around line 22-27: Wrap the fragment returned by TokenLink in a local wrapper
element styled with display: flex and align-items: center (e.g., TokenLink
container) instead of returning a raw fragment, and change the TokenIcon sizing
from a fixed 2rem to an em-relative size so the icon scales with surrounding
text (use 1em or similar). Update both returned fragments (the block using
TokenIcon with <a> at lines shown and the other occurrence at 31-34) to use this
new flex container and em-based icon sizing, referencing the TokenIcon component
and the TokenLink return points so the layout and icon scale match the font
context.
- Around line 16-17: Update the TokenLink component prop type for tokenAddress
to use AddressKey from `@cowprotocol/cow-sdk` (i.e., change tokenAddress: string |
undefined to tokenAddress: AddressKey | undefined), add/import AddressKey in the
TokenLink.pure.tsx imports, and adjust any internal usages or type casts of
tokenAddress within TokenLink (or consumers) to satisfy the new AddressKey type.
In `@libs/common-utils/src/finiteMap.ts`:
- Around line 14-23: The get method treats a stored undefined value as a missing
key, so it skips LRU refresh; change get(K) in the FiniteMap class to first
check this.has(key) to determine presence, then read the value (via
super.get(key)), perform the LRU refresh by calling super.delete(key) and
super.set(key, value) only when the key exists, and return the retrieved value
(which may be undefined); update the override get, not other methods, and keep
using super.get, super.delete, and super.set for the refresh logic.
- Around line 33-36: The eviction logic in FiniteMap uses value comparison
(oldestKey !== undefined) to detect iterator exhaustion, which fails when K can
be undefined; change it to inspect the iterator result's done flag by capturing
the iterator result (e.g., const iter = this.keys().next()) and if (!iter.done)
call super.delete(iter.value) so valid undefined keys are correctly evicted;
update the block around this.size/maxSize and references to oldestKey
accordingly in the FiniteMap implementation.
🪄 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: 660c4b21-76a0-470d-9328-7bb5bb056c7e
📒 Files selected for processing (13)
apps/explorer/package.jsonapps/explorer/src/components/orders/OrderWrapperDetails/item/OrderWrapperDetailsItem.component.tsxapps/explorer/src/hooks/euler/client.tsapps/explorer/src/utils/wrappers/eulerEvc/components/OwnerLink.pure.tsxapps/explorer/src/utils/wrappers/eulerEvc/components/TokenLink.pure.tsxapps/explorer/src/utils/wrappers/eulerEvc/euler.utils.tsapps/explorer/src/utils/wrappers/eulerEvc/positions/EulerPosition.styles.tsxapps/explorer/src/utils/wrappers/eulerEvc/positions/close/EulerClosePosition.pure.tsxapps/explorer/src/utils/wrappers/eulerEvc/positions/collateral/EulerCollateralSwap.pure.tsxapps/explorer/src/utils/wrappers/eulerEvc/positions/open/EulerOpenPosition.pure.tsxlibs/common-utils/src/finiteMap.test.tslibs/common-utils/src/finiteMap.tslibs/common-utils/src/index.ts
✅ Files skipped from review due to trivial changes (5)
- libs/common-utils/src/finiteMap.test.ts
- apps/explorer/src/utils/wrappers/eulerEvc/components/OwnerLink.pure.tsx
- apps/explorer/src/utils/wrappers/eulerEvc/positions/EulerPosition.styles.tsx
- apps/explorer/src/utils/wrappers/eulerEvc/euler.utils.ts
- apps/explorer/package.json
🚧 Files skipped from review as they are similar to previous changes (4)
- apps/explorer/src/components/orders/OrderWrapperDetails/item/OrderWrapperDetailsItem.component.tsx
- apps/explorer/src/utils/wrappers/eulerEvc/positions/open/EulerOpenPosition.pure.tsx
- apps/explorer/src/utils/wrappers/eulerEvc/positions/close/EulerClosePosition.pure.tsx
- apps/explorer/src/utils/wrappers/eulerEvc/positions/collateral/EulerCollateralSwap.pure.tsx
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
|
Warning Review the following alerts detected in dependencies. According to your organization's Security Policy, it is recommended to resolve "Warn" alerts. Learn more about Socket for GitHub.
|
6 participants
Summary
Renders a "Wrappers" row in the verbose order details table for orders whose
appDatareferences known wrapper contracts. Wrapper-specific UI is code-split and loaded on demand; unknown addresses fall back to fetching the contractname()on-chain (supported by allCowWrappercontracts).Supported wrappers for starting off: Euler EVC – Open Position, Close Position, Collateral Swap'
Open Position (https://explorer-dev-git-explorer-wrapper-designation-cowswap-dev.vercel.app/orders/0xea2c93014d0a41820e3b49f7273790041bfd0829e0a26841ab8c10cddfb0d55d9fa3c00a92ec5f96b1ad2527ab41b3932efeda5869eb4e11)
Collateral Swap (https://explorer-dev-git-explorer-wrapper-designation-cowswap-dev.vercel.app/orders/0xef7e3cca05ecddad3515ea524c9be16995fa05824b23e624bd0993fb3ad9842c9fa3c00a92ec5f96b1ad2527ab41b3932efeda5869eb62fe)
Close Position (https://explorer-dev-git-explorer-wrapper-designation-cowswap-dev.vercel.app/orders/0x85f93c3fe29fc35e966c36145a997e2a254d7098fe5ebff928e5c09c917fc45dd6cfba01838223dbb47956a2f40d701ce70c382869ef6355)
Unknown wrapper (https://explorer-dev-git-explorer-wrapper-designation-cowswap-dev.vercel.app/orders/0x5594c109c44dd585bc7c494998e1c06dce1b42dfae3dc0fd38094fa4b04130b1e227398f066f11e7fa49893bb97cac6caa7790f969d3acc3)
Mobile version:
To Test
Obtain a Order ID for a trade that uses Euler. This address has a ton of Euler trades:
Start the explorer with
pnpm start:explorerLoad the page on the cow explorer. Observe.
Background
As wrappers are expected to proliferate in the future, we will want to display useful information on the explorer to aid users (and ourselves as devs) in understanding an order.
Please let me know if the overall strategy for displaying wrapper information seems reasonable. The idea is to make it easy to plug in new wrapper informations when the time comes while causing minimal impact on the rest of the repo
fixes https://linear.app/cowswap/issue/SC-63/add-to-explorer
Summary by CodeRabbit
New Features
Documentation
Chores
Tests