Conversation
|
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:
📝 WalkthroughWalkthroughAdds a Marketplace feature (pages, hooks, modals, state, and UI), integrates marketplace wiring into global state and dispatch flows, removes Jest/Playwright test infra and many tests, updates CI/workflows and dependencies, and introduces URL/network validation utilities and several UI primitives. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI as Marketplace Page / Modal
participant Query as React Query
participant Contract as ArNS Marketplace Contract
participant ANT as ANT Process
participant Wallet as Wallet/Signer
User->>UI: Open Marketplace / Click "List Domain"
UI->>Query: fetch user assets & ARIO price
Query-->>UI: return balances & assets
User->>UI: Configure listing & Confirm
UI->>Wallet: ensure signer/address
UI->>Contract: check deposit requirement
alt Insufficient balance
UI->>Contract: deposit ARIO (write)
UI->>Query: poll user assets until balance updated
Query-->>UI: balance updated
end
UI->>ANT: transfer ANT to marketplace (via signer)
ANT-->>UI: transfer tx confirmed
UI->>Contract: create listing intent (write)
Contract-->>UI: intent created
UI->>Query: invalidate marketplace queries (refresh)
UI-->>User: show success (Arweave link)
sequenceDiagram
participant User
participant UI as ViewListing Page
participant Query as React Query
participant Contract as ArNS Marketplace Contract
participant Wallet as Wallet/Signer
participant ANT as ANT Process
User->>UI: Open listing page
UI->>Query: fetch order, domain info, ARIO price
Query-->>UI: order & pricing
User->>UI: Click "Buy"
UI->>Wallet: ensure signer/address
UI->>Query: check buyer balance
alt Low balance
UI->>Contract: deposit ARIO
UI->>Query: poll assets until balance sufficient
end
UI->>Contract: execute buyFixedPriceANT (write)
Contract->>ANT: transfer ANT to buyer
ANT-->>Contract: transfer confirmed
Contract-->>UI: purchase success
UI->>Query: invalidate/update assets & listings
UI-->>User: show success (Arweave link)
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
|
Visit the preview URL for this PR (updated for commit 1788b5e): https://arns-portal--pr903-pe-8734-marketplace-x3nib7bq.web.app (expires Tue, 30 Dec 2025 15:47:59 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 Sign: 1f59769442cb89cf2a92042a342e51fe94047b94 |
There was a problem hiding this comment.
Actionable comments posted: 16
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/state/contexts/ArNSState.tsx (1)
71-82: AddmarketplaceProcessIdto the useEffect dependency array.The
marketplaceProcessIdis passed todispatchArNSUpdateon line 77 but is missing from the dependency array on line 82. This violates the React exhaustive-deps rule and will prevent the effect from re-running whenmarketplaceProcessIdchanges, potentially causing stale or incorrect ArNS state.Apply this diff to fix the missing dependency:
- }, [walletAddress, aoNetwork, arioProcessId, hyperbeamUrl]); + }, [walletAddress, aoNetwork, arioProcessId, hyperbeamUrl, marketplaceProcessId, antRegistryProcessId]);Note: I've also added
antRegistryProcessIdwhich appears to have the same issue (used on line 78 but not in dependencies).src/components/data-display/tables/DomainsTable.tsx (1)
518-524: Missing dependencies in useEffect.The effect uses
marketplaceProcessId,latestAntVersion, andwalletAddressinside the callback (lines 479, 472, 462), but these aren't in the dependency array. This could lead to stale table data when these values change.}, [ domainData, loading, loadingArnsState, primaryNameData, dispatchArNSState, + marketplaceProcessId, + latestAntVersion?.moduleId, + walletAddress, ]);
🟡 Minor comments (13)
src/components/pages/Settings/devtools/ANTTools.tsx-225-229 (1)
225-229: Fix text color for visibility.The
text-blackclass will render the message invisible against the darkbg-metallic-greybackground. Use a visible color liketext-greyortext-whitefor consistency with other status messages.{!walletAddress && ( - <span className="text-black text-xs"> + <span className="text-grey text-xs"> Connect wallet to spawn ANT </span> )}src/components/inputs/DateTimePicker/DateTimePicker.tsx-21-23 (1)
21-23: Consider initializing time picker to 1 hour from now instead of noon.The default time value of '12:00' might be in the past if the component is opened later in the day, which could cause confusion when the enforcement logic adjusts it.
Consider initializing to 1 hour from now:
+ const getDefaultTime = () => { + const oneHourFromNow = new Date(Date.now() + 60 * 60 * 1000); + return format(oneHourFromNow, 'HH:mm'); + }; + const [timeValue, setTimeValue] = useState<string>( - value ? format(value, 'HH:mm') : '12:00', + value ? format(value, 'HH:mm') : getDefaultTime(), );src/components/cards/MarketplaceOrderInfoCard/MarketplaceOrderInfoCard.tsx-81-81 (1)
81-81: Remove debug console.log before merge.Debug logging should not be present in production code.
- console.log(config);README.md-11-12 (1)
11-12: README uses incorrect package manager commands.The README (lines 11-12, 18, 24) contains
pnpmcommands, but the project usesyarnas evidenced byyarn.lockbeing committed andpackage.jsonspecifying"packageManager": "yarn@1.22.22". The CI workflows (.github/workflows/pr.yml, .github/workflows/staging_deploy.yml) correctly useyarn install --frozen-lockfile. Update the README to use yarn commands consistently with the project setup.src/state/actions/dispatchArIOInteraction.ts-23-27 (1)
23-27: Remove unused import NETWORK_DEFAULTS.The import of
NETWORK_DEFAULTSis not referenced anywhere in this file.Apply this diff:
import { APP_NAME, - NETWORK_DEFAULTS, WRITE_OPTIONS, } from '@src/utils/constants';src/components/modals/ManageMarketplaceARIOModal/panels/WithdrawPanel.tsx-8-8 (1)
8-8: Unused import.
connectis imported from@permaweb/aoconnectbut never used in this file.-import { connect } from '@permaweb/aoconnect';src/hooks/useMarketplaceUserAssets.ts-19-25 (1)
19-25: Query key contains an object which may cause cache misses.
aoNetwork.ARIOis included in the query key, but it's an object. React Query uses referential equality for objects in query keys. IfaoNetwork.ARIOis recreated on each render (even with the same values), it will cause unnecessary cache invalidations and refetches.Consider using a stable identifier instead:
queryKey: [ 'marketplace-user-assets', address, marketplaceProcessId, - aoNetwork.ARIO, + aoNetwork.ARIO.CU_URL, // or another stable primitive identifier ],src/components/modals/ContinueWorkflowModal/ContinueWorkflowModal.tsx-349-352 (1)
349-352: Success message text is incorrect for PUSH_INTENT workflow type.The success message always states "The ANT has been transferred to the marketplace with the correct intent ID", but for
PUSH_INTENT, the action is pushing an intent resolution, not transferring the ANT. The correct message is computed at line 227-230 but not used in this success state display.Consider using a dynamic message or differentiating the displayed text based on the workflow type.
src/components/data-display/tables/MarketplaceListingsTable.tsx-338-340 (1)
338-340: Missing dependencyantToNameMapincolumnsuseMemo.The
columnsdefinition usesantToNameMapat line 166, but it's not included in the dependency array. This could lead to stale data being displayed when ArNS records update.], - [arioTicker], + [arioTicker, antToNameMap], );src/components/pages/ViewListing/ViewListing.tsx-61-128 (1)
61-128: Missing validation: user should not be able to buy their own listing.The
handleBuyfunction validates wallet and order data but doesn't check if the buyer is the seller. This could lead to a confusing UX or unexpected behavior if a user attempts to purchase their own listing.Consider adding a check:
if (!name) { throw new Error('Domain name not found'); } + if (orderData.creator === walletAddress.toString()) { + throw new Error('Cannot purchase your own listing'); + } const marketplaceContract = new ArNSMarketplaceWrite({src/components/modals/ListNameForSaleModal/ListNameForSaleModal.tsx-52-54 (1)
52-54: Misleading comment.The comment says "Default to 1 hour from now (current day)" but the expiration date panel (FixedPricePanel line 247) enforces a minimum of 1 day from now. This default will fail validation when the user clicks Next without changing the date.
const [expirationDate, setExpirationDate] = useState<Date | undefined>( - new Date(Date.now() + 60 * 60 * 1000), // Default to 1 hour from now (current day) + new Date(Date.now() + 24 * 60 * 60 * 1000), // Default to 1 day from now (matches minDate) );src/components/data-display/tables/DomainsTable.tsx-206-252 (1)
206-252: Missing loading state handling in MarketplaceActionIcon.The component doesn't handle the loading state from
useMarketplaceOrder. Whenorderis undefined but still loading, the logic falls through to the error state tooltip (line 221-223), which could show a false error flash while data is being fetched.function MarketplaceActionIcon({ domainName, processId, }: { domainName: string; processId: string }) { const { hasIntent } = useANTIntent(processId); - const { data: order, error: orderError } = useMarketplaceOrder({ + const { data: order, error: orderError, isLoading: orderLoading } = useMarketplaceOrder({ antId: processId, }); + // Show loading state while fetching order + if (orderLoading) { + return <Activity className="w-[18px] h-[18px] text-grey animate-pulse" />; + } + // If there's an intent but no order (or order fetch failed), show Activity icon const hasOrder = order && !orderError;src/components/modals/ListNameForSaleModal/panels/ConfirmListingPanel.tsx-117-134 (1)
117-134: Potential null safety issue with walletAddress.If
walletAddressis undefined, the 'Current Owner' field will display"undefined...undefined". Add a fallback or guard.const orderSummary: Record<string, ReactNode> = { 'Listing Type': getListingTypeLabel(), 'Domain Name': domainName, 'Current Owner': - walletAddress?.toString().slice(0, 8) + - '...' + - walletAddress?.toString().slice(-8), + walletAddress + ? `${walletAddress.toString().slice(0, 8)}...${walletAddress.toString().slice(-8)}` + : 'Unknown', Expires: expirationDate
🧹 Nitpick comments (37)
src/components/pages/Settings/devtools/ANTTools.tsx (3)
31-36: Consider removingantStatefrom dependencies to allow manual clearing.Including
antStatein the dependency array means if a user clears the textarea, it will immediately reset to the default state. If intentional for devtools convenience, consider adding a "Reset to Default" button instead for clearer UX.React.useEffect(() => { - if (walletAddress && !antState) { + if (walletAddress) { const defaultState = createAntStateForOwner(walletAddress.toString()); setAntState(JSON.stringify(defaultState, null, 2)); } - }, [walletAddress, antState]); + }, [walletAddress]);
43-47: Remove unreachable code.The check at lines 45-47 is unreachable because
canSpawn(line 44) already includeswallet?.contractSignerin its condition. IfcanSpawnis false, the function returns early; if true,wallet.contractSigneris guaranteed to exist.async function handleSpawnANT() { if (!canSpawn) return; - if (!wallet?.contractSigner) { - throw new Error('No Wander Signer found'); - } setIsSpawning(true);
177-183: Consider adding real-time JSON validation.Unlike the Module ID input which shows immediate validation feedback, the JSON textarea only validates on spawn attempt. For devtools UX, consider adding a visual indicator (border color change or error message) when JSON is malformed.
src/components/inputs/DateTimePicker/DateTimePicker.tsx (6)
6-12: Export theDateTimePickerPropsinterface for reusability.The interface is not exported, which limits its reusability in other parts of the codebase that might need to reference these prop types.
Apply this diff:
-interface DateTimePickerProps { +export interface DateTimePickerProps { value?: Date; onChange: (date: Date | undefined) => void; placeholder?: string; minDate?: Date; className?: string; }
77-89: Consider the implications of returningfalsewhenselectedDateis undefined.When no date is selected,
isTimeDisabledreturnsfalsefor all times, meaning all time options appear enabled. This is somewhat inconsistent with the component's behavior, though in practice the time picker is only used after a date is selected.Consider adding a comment to clarify this design decision:
const isTimeDisabled = ( hours: number, minutes: number, selectedDate?: Date, ) => { + // When no date is selected, allow all times (will be validated on date selection) if (!selectedDate) return false; const testDateTime = new Date(selectedDate); testDateTime.setHours(hours, minutes, 0, 0); const oneHourFromNow = new Date(Date.now() + 60 * 60 * 1000); return testDateTime < oneHourFromNow; };
92-197: Move Calendar component outside to prevent recreation on every render.The
Calendarcomponent is defined insideDateTimePicker, causing it to be recreated on every render. This is inefficient and can impact performance.Move the Calendar component outside the DateTimePicker function:
+// Calendar component for date selection +const Calendar = ({ + selected, + onSelect, + isDateDisabled, +}: { + selected?: Date; + onSelect: (date: Date | undefined) => void; + isDateDisabled: (date: Date) => boolean; +}) => { + const [currentMonth, setCurrentMonth] = useState(selected || new Date()); + // ... rest of Calendar implementation +}; + function DateTimePicker({ value, onChange, placeholder = 'Select date and time', className = '', }: DateTimePickerProps) { // ... component implementation - - // Simple calendar component - const Calendar = ({ - selected, - onSelect, - }: { selected?: Date; onSelect: (date: Date | undefined) => void }) => { - // ... Calendar implementation - };Note: You'll need to pass
isDateDisabledas a prop to the Calendar component.
152-156: Consider constraining calendar navigation to valid date range.The calendar allows navigating to any month, but dates are only valid for today to 30 days from now. Users might navigate to months where all dates are disabled, which could be confusing.
Add constraints to the navigation:
const navigateMonth = (direction: 'prev' | 'next') => { const newDate = new Date(currentMonth); newDate.setMonth(newDate.getMonth() + (direction === 'next' ? 1 : -1)); - setCurrentMonth(newDate); + + // Prevent navigating to months outside the valid range + const today = new Date(); + const maxDate = new Date(); + maxDate.setDate(maxDate.getDate() + 30); + + if (newDate >= today && newDate <= maxDate) { + setCurrentMonth(newDate); + } };
162-178: Improve accessibility with ARIA attributes and labels.The calendar navigation buttons use arrow symbols without ARIA labels, which may not be clear for screen reader users.
Add ARIA attributes to improve accessibility:
<button type="button" className="p-1 hover:bg-grey rounded text-white" onClick={() => navigateMonth('prev')} + aria-label="Previous month" > ← </button> <h3 className="text-white font-medium"> {format(currentMonth, 'MMMM yyyy')} </h3> <button type="button" className="p-1 hover:bg-grey rounded text-white" onClick={() => navigateMonth('next')} + aria-label="Next month" > → </button>Similarly, consider adding
aria-labelto the hour and minute selects at Lines 239 and 268.
227-227: Reconsider the extremely high z-index value.The z-index of 99999 is very high and might cause stacking context issues or conflicts with other UI elements (e.g., modals, tooltips).
Consider using a more reasonable z-index value that fits within your application's z-index scale:
<Popover.Content className="w-80 rounded-lg border border-dark-grey bg-metallic-grey shadow-lg data-[state=open]:animate-in data-[state=closed]:animate-out data-[state=closed]:fade-out-0 data-[state=open]:fade-in-0 data-[state=closed]:zoom-out-95 data-[state=open]:zoom-in-95 data-[side=bottom]:slide-in-from-top-2 data-[side=left]:slide-in-from-right-2 data-[side=right]:slide-in-from-left-2 data-[side=top]:slide-in-from-bottom-2" - style={{ zIndex: 99999 }} + style={{ zIndex: 1000 }} sideOffset={8} align="start" >If this high value is necessary due to other elements in your application, consider defining z-index values in a centralized constants file.
src/components/modals/ant-management/UpgradeDomainForMarketplaceModal/UpgradeDomainForMarketplaceModal.tsx (1)
65-138: Consider improving error handling in upgradeDomain.The
.catch()on line 99-101 swallows the error and only pushes tofailedUpgrades. This means theawaitresolves successfully even when the upgrade fails, which could be confusing. Also,failedUpgradesis an array but only ever contains at most one element.Consider either:
- Using a boolean flag instead of an array
- Re-throwing after recording the failure, or using try/catch instead
- const failedUpgrades = []; + let upgradeFailed = false; await dispatchANTInteraction({ // ... payload }).catch(() => { - failedUpgrades.push(processId); + upgradeFailed = true; }); - if (failedUpgrades.length) { + if (upgradeFailed) { eventEmitter.emit('error', {src/components/indicators/InterruptedWorkflowIndicator/InterruptedWorkflowIndicator.css (1)
11-14: Move transition property to base class for smooth hover-out animation.The
transitionproperty is currently only applied during hover, which means the element will snap back instantly when hover ends. Moving it to the base.interrupted-workflow-indicatorclass ensures smooth transitions in both directions.Apply this diff:
.interrupted-workflow-indicator { position: relative; display: inline-flex; align-items: center; justify-content: center; cursor: pointer; + transition: transform 0.2s ease-in-out; } .interrupted-workflow-indicator:hover { transform: scale(1.1); - transition: transform 0.2s ease-in-out; }src/components/modals/ManageMarketplaceARIOModal/ManageMarketplaceARIOModal.tsx (2)
29-31: Add accessibility label to the close button.The close button should have an accessible label for screen readers.
- <button onClick={onClose}> + <button onClick={onClose} aria-label="Close modal"> <XIcon className="size-5" /> </button>
37-59: Consider using Tabs.Trigger for better accessibility.The custom tab buttons work, but using Radix UI's
Tabs.Triggerwould provide built-in keyboard navigation and ARIA attributes. The current implementation duplicates tab state management.If keeping custom buttons, add ARIA attributes:
<button + role="tab" + aria-selected={activeTab === 'deposit'} className={`flex-1 flex items-center justify-center gap-2 px-4 py-2 rounded-md text-sm font-medium transition-all ${ activeTab === 'deposit' ? 'bg-white text-black' : 'text-grey hover:text-white' }`} onClick={() => setActiveTab('deposit')} >src/components/indicators/InterruptedWorkflowIndicator/InterruptedWorkflowIndicator.tsx (1)
24-30: Consider extracting tooltip style colors to avoid duplication.The tooltip style colors here (
#fef2f2,#fecaca,#991b1b) duplicate values fromRedPingAnimation.css(lines 67-69). Consider using CSS classes instead of inline styles for consistency and easier maintenance.<Tooltip - tooltipOverrides={{ - overlayInnerStyle: { - backgroundColor: '#fef2f2', - border: '1px solid #fecaca', - color: '#991b1b', - }, - }} + tooltipOverrides={{ + overlayClassName: 'interrupted-workflow-tooltip', + }} message={src/components/pages/Settings/devtools/MarketplaceTools.tsx (2)
33-39: Redundant state update in handleReset.Line 38 sets
newVersionmanually, but theuseEffecton lines 19-21 will sync it automatically whenminimumANTVersionForMarketplacechanges. This redundancy is harmless but unnecessary.const handleReset = () => { dispatchGlobalState({ type: 'setMinimumANTVersionForMarketplace', payload: DEFAULT_MIN_MARKETPLACE_ANT_VERSION, }); - setNewVersion(DEFAULT_MIN_MARKETPLACE_ANT_VERSION.toString()); };
23-31: Input validation accepts decimal values that get truncated.
parseIntwill truncate decimal inputs (e.g., "25.7" becomes 25). Consider using a stricter validation or informing users that only integers are accepted.const handleVersionUpdate = () => { const version = parseInt(newVersion, 10); - if (!isNaN(version) && version > 0) { + if (!isNaN(version) && version > 0 && Number.isInteger(Number(newVersion))) { dispatchGlobalState({ type: 'setMinimumANTVersionForMarketplace', payload: version, }); + } else { + // Optionally show validation error to user } };src/components/modals/ManageMarketplaceARIOModal/panels/WithdrawPanel.tsx (3)
273-285: Silent input clamping may confuse users.When the user types a value exceeding the balance, line 278 silently clamps it to
currentBalance. This could be unexpected - users might not realize their input was modified. Consider showing a visual indication or allowing the invalid input with an error message.onChange={(e) => { const val = e.target.value; if (isValidCustomFormat(val)) { const numVal = Number(val); - const clampedVal = Math.min(numVal, currentBalance); - setCustomValue(clampedVal.toString()); + setCustomValue(val); setPercentageSelected(undefined); setCustomValueError( - isValidCustomAmount(clampedVal.toString()) || '', + isValidCustomAmount(val) || '', ); setError(''); } }}
145-147: Consider simplifying the amount conversion.
amountInMARIO.valueOf().toString()could potentially be simplified if the SDK accepts the mARIO type directly, though this depends on the SDK's API.#!/bin/bash # Check the withdrawArIO method signature in the SDK rg -n "withdrawArIO" --type=ts -C 3 node_modules/@ar.io/sdk || echo "SDK not in node_modules, check docs"@ar.io/sdk ArNSMarketplaceWrite withdrawArIO method signature
149-157: Hardcoded sleep duration for blockchain confirmation.The 3-second sleep is a magic number and may not be sufficient for all network conditions. Consider making this configurable or adding a comment explaining the rationale.
- // Wait a bit for cranking to complete - await sleep(3000); + // Wait for the AO network to process the withdrawal (cranking) + // 3 seconds is typically sufficient for normal network conditions + const WITHDRAWAL_CONFIRMATION_DELAY_MS = 3000; + await sleep(WITHDRAWAL_CONFIRMATION_DELAY_MS);src/components/pages/ViewListing/ViewListing.tsx (3)
259-269: Potential runtime error ifdomainInfo?.processIdis undefined.
ArweaveTransactionIDconstructor throws an error if passed an invalid or undefined value. WhiledomainInfoshould exist at this point due to the earlier guard (line 131), accessingprocessIddirectly without a fallback could cause issues if the component re-renders in an unexpected state.Consider a defensive check or ensure the guard covers this case:
- <ArweaveID - id={new ArweaveTransactionID(domainInfo?.processId)} + <ArweaveID + id={new ArweaveTransactionID(domainInfo?.processId ?? '')}Alternatively, wrap this section with
{domainInfo?.processId && (...)}.
274-282: Type assertionas anybypasses type safety.The cast
(domainInfo.arnsRecord as any)?.endTimestampsuggests a type mismatch. This could hide issues if the record structure changes.Consider defining a proper type guard or extending the type definition to include
endTimestampfor lease records:// Define a type guard or use conditional typing const leaseEndTimestamp = domainInfo?.arnsRecord?.type === 'lease' ? (domainInfo.arnsRecord as { endTimestamp?: number }).endTimestamp : undefined;
386-406: Buy button is always enabled when not processing, even without a connected wallet.The button doesn't visually indicate that a wallet connection is required. While
handleBuywill throw an error, a better UX would disable the button or show a connect wallet prompt.<button className={`w-full font-semibold py-3 px-6 rounded transition-colors flex items-center justify-center gap-2 mt-8 ${ - isBuying - ? 'bg-grey text-white cursor-not-allowed' - : 'bg-primary hover:bg-warning text-black' + isBuying || !wallet?.contractSigner + ? 'bg-grey text-white cursor-not-allowed' + : 'bg-primary hover:bg-warning text-black' }`} onClick={handleBuy} - disabled={isBuying} + disabled={isBuying || !wallet?.contractSigner} > {isBuying ? ( <> <Loader size={20} /> Processing... </> + ) : !wallet?.contractSigner ? ( + <>Connect Wallet to Buy</> ) : ( <> <ShoppingCart className="w-5 h-5" /> Buy Now </> )} </button>src/components/modals/ContinueWorkflowModal/ContinueWorkflowModal.tsx (2)
8-8: Unused import:connectfrom@permaweb/aoconnect.The
connectfunction is imported but never used in this file.-import { connect } from '@permaweb/aoconnect';
187-188: Type assertionas anybypasses type safety forarioContract.This cast appears in multiple places in the PR. Consider creating a properly typed interface or fixing the underlying type mismatch.
src/components/data-display/tables/MarketplaceListingsTable.tsx (1)
101-105: Early return may cause unnecessary re-renders or flash of empty state.Returning an empty array when
arnsRecordsis falsy happens afterordersDatais checked. IfarnsRecordstakes longer to load, users might see an empty table briefly even when orders exist. Consider combining this check with the loading state or showing a more informative message.src/hooks/useANTIntent.ts (2)
3-3: Unused import:useDomainInfo.This import is declared but never used in the hook.
-import useDomainInfo from './useDomainInfo';
48-63:isInterruptedWorkflowandhasIntentare semantically identical.Both values are derived from
!!antIntent, making them redundant. The comment at lines 48-50 suggests that any existing intent indicates an interrupted workflow, but this naming may be misleading since not all intents represent interrupted states (some might be in-progress but not interrupted).If the intent is that "interrupted" means the intent exists but wasn't processed, consider adding more specific logic based on
antIntent.statusto differentiate active vs interrupted intents.// Check if this is an interrupted workflow - // An interrupted workflow occurs when: - // There's an intent for the ANT - const isInterruptedWorkflow = !!antIntent; + // An interrupted workflow occurs when the intent exists but is in a pending/failed state + const isInterruptedWorkflow = antIntent && antIntent.status !== 'completed';If the current behavior is intentional, consider removing one of the redundant fields or documenting why both are needed.
src/components/layout/Navbar/NotificationMenu/NotificationMenu.tsx (1)
159-163: Optional: trivial memoization.Memoizing
interruptedWorkflows.lengthprovides minimal benefit since array length access is O(1). You could simply useinterruptedWorkflows.lengthdirectly in thenotificationsuseMemo.src/components/modals/ListNameForSaleModal/panels/FixedPricePanel.tsx (1)
259-264: Unexpected navigation:window.history.back()in modal.Using
window.history.back()for the Cancel button may cause unexpected navigation if the user opened this modal from a different route or via deep linking. Consider adding anonCancelprop callback to allow the parent to control this behavior.src/components/data-display/tables/DomainsTable.tsx (2)
288-371: Optional: extract shared ContinueWorkflowModal logic.Both
RoleDisplayandInterruptedWorkflowActioncomponents manage their ownshowContinueWorkflowModalstate and renderContinueWorkflowModal. Consider extracting a shared hook or wrapper component to reduce duplication.
869-938: Consider extracting IIFE to a named component.The inline IIFE for marketplace action logic (lines 869-938) is quite complex. Extracting this to a named component like
<MarketplaceListingAction domainName={...} processId={...} ... />would improve readability and testability.src/hooks/useMarketplaceOrders.ts (2)
59-64:totalCountmay be misleading.
totalCount: allOrders.lengthreturns the number of fetched items, not the actual total count of orders in the marketplace. If the API provides a total count, it should be surfaced; otherwise, consider renaming tofetchedCountto avoid confusion.return { items: allOrders, hasMore, nextCursor: cursor, - totalCount: allOrders.length, + fetchedCount: allOrders.length, + // If the API provides totalCount, add: totalCount: result.totalCount, };
36-44: Pagination logic could be clearer.The limit calculation
Math.min(limit, (filters?.limit || limit) - allOrders.length)is complex. Since the while condition already guards against exceedingfilters.limit, simplifying to justlimitfor the per-page size would be clearer, with the outer loop handling the total constraint.- while (hasMore && (!filters?.limit || allOrders.length < filters.limit)) { - const result: any = await marketplaceContract.getPaginatedOrders({ - cursor, - limit: Math.min(limit, (filters?.limit || limit) - allOrders.length), + const targetLimit = filters?.limit ?? Infinity; + while (hasMore && allOrders.length < targetLimit) { + const remaining = targetLimit - allOrders.length; + const result: any = await marketplaceContract.getPaginatedOrders({ + cursor, + limit: Math.min(limit, remaining),src/components/modals/ListNameForSaleModal/ListNameForSaleModal.tsx (1)
122-136: Query reset predicates may be overly broad.The predicate
key.startsWith('marketplace')could reset unrelated queries if other features use similar prefixes. Consider using exact query key matching or a more specific pattern.src/state/actions/dispatchArNSUpdate.ts (1)
124-124: Remove debug logging before merge.This console.log appears to be debug output that should be removed or converted to conditional debug logging before merging to develop.
- console.log('marketplaceUserDomains', userDomains);src/components/modals/ManageMarketplaceARIOModal/panels/DepositPanel.tsx (1)
298-311: Input clamping may silently truncate user input.When the user enters a value greater than
MAX_ARIO_DEPOSIT, the input is silently clamped to the max. Consider showing immediate feedback or preventing the input rather than auto-clamping, which could confuse users.onChange={(e) => { const val = e.target.value; if (isValidCustomFormat(val)) { const numVal = Number(val); - const clampedVal = Math.min(numVal, MAX_ARIO_DEPOSIT); - setCustomValue(clampedVal.toString()); + setCustomValue(val); setButtonSelected(undefined); setCustomValueError( - isValidCustomAmount(clampedVal.toString()) || '', + isValidCustomAmount(val) || '', ); setError(''); } }}src/hooks/useInterruptedWorkflows.ts (1)
15-22: Consider stronger typing for domains parameter.The
domains: Record<string, any>parameter could be more strictly typed to match the actual domain record structure (e.g.,Record<string, AoArNSNameData>), improving type safety and IDE support.+import { AoArNSNameData } from '@ar.io/sdk'; + export function useInterruptedWorkflows( ants: Record<string, ANTProcessData>, - domains: Record<string, any>, + domains: Record<string, AoArNSNameData>, ): {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
src/components/inputs/text/ValidationInput/__snapshots__/ValidationInput.test.tsx.snapis excluded by!**/*.snapyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (76)
.github/workflows/build_and_test.yml(2 hunks).github/workflows/pr.yml(1 hunks).github/workflows/production.yml(2 hunks).github/workflows/staging_deploy.yml(1 hunks)README.md(1 hunks)__mocks__/react-router-dom.ts(0 hunks)babel.config.cjs(0 hunks)biome.json(2 hunks)jest-setup.ts(0 hunks)jest.config.json(0 hunks)package.json(3 hunks)playwright.config.ts(0 hunks)src/App.tsx(3 hunks)src/components/animations/RedPingAnimation.css(1 hunks)src/components/cards/MarketplaceOrderInfoCard/MarketplaceOrderInfoCard.tsx(1 hunks)src/components/cards/MarketplaceOrderInfoCard/index.ts(1 hunks)src/components/cards/NavMenuCard/NavMenuCard.tsx(7 hunks)src/components/data-display/tables/DomainsTable.tsx(9 hunks)src/components/data-display/tables/MarketplaceListingsTable.tsx(1 hunks)src/components/indicators/InterruptedWorkflowIndicator/InterruptedWorkflowIndicator.css(1 hunks)src/components/indicators/InterruptedWorkflowIndicator/InterruptedWorkflowIndicator.tsx(1 hunks)src/components/inputs/DateTimePicker/DateTimePicker.tsx(1 hunks)src/components/inputs/buttons/ConnectButton/ConnectButton.test.tsx(0 hunks)src/components/inputs/text/ValidationInput/ValidationInput.test.tsx(0 hunks)src/components/layout/Navbar/NavBarLink/NavBarLink.test.tsx(0 hunks)src/components/layout/Navbar/NavGroup/NavGroup.test.tsx(0 hunks)src/components/layout/Navbar/NotificationMenu/NotificationMenu.tsx(2 hunks)src/components/layout/Notifications/Notifications.tsx(3 hunks)src/components/modals/ContinueWorkflowModal/ContinueWorkflowModal.tsx(1 hunks)src/components/modals/ListNameForSaleModal/ListNameForSaleModal.tsx(1 hunks)src/components/modals/ListNameForSaleModal/panels/ConfirmListingPanel.tsx(1 hunks)src/components/modals/ListNameForSaleModal/panels/FixedPricePanel.tsx(1 hunks)src/components/modals/LogoUploadModal/__tests__/LogoUploadModal.test.tsx(0 hunks)src/components/modals/ManageMarketplaceARIOModal/ManageMarketplaceARIOModal.tsx(1 hunks)src/components/modals/ManageMarketplaceARIOModal/index.ts(1 hunks)src/components/modals/ManageMarketplaceARIOModal/panels/DepositPanel.tsx(1 hunks)src/components/modals/ManageMarketplaceARIOModal/panels/WithdrawPanel.tsx(1 hunks)src/components/modals/ant-management/UpgradeDomainForMarketplaceModal/UpgradeDomainForMarketplaceModal.tsx(1 hunks)src/components/modals/ant-management/UpgradeDomainForMarketplaceModal/index.ts(1 hunks)src/components/modals/ant-management/UpgradeDomainForMarketplaceModal/styles.css(1 hunks)src/components/modals/index.ts(1 hunks)src/components/pages/Manage/Manage.tsx(2 hunks)src/components/pages/Marketplace/Marketplace.tsx(1 hunks)src/components/pages/Register/Checkout.tsx(2 hunks)src/components/pages/Settings/NetworkSettings.tsx(7 hunks)src/components/pages/Settings/devtools/ANTTools.tsx(2 hunks)src/components/pages/Settings/devtools/DevTools.tsx(2 hunks)src/components/pages/Settings/devtools/MarketplaceTools.tsx(1 hunks)src/components/pages/Settings/settingsConfig.ts(2 hunks)src/components/pages/ViewListing/ViewListing.tsx(1 hunks)src/components/pages/index.ts(2 hunks)src/hooks/index.ts(1 hunks)src/hooks/useANTIntent.ts(1 hunks)src/hooks/useInterruptedWorkflows.ts(1 hunks)src/hooks/useMarketplaceInfo.ts(1 hunks)src/hooks/useMarketplaceOrder.tsx(1 hunks)src/hooks/useMarketplaceOrders.ts(1 hunks)src/hooks/useMarketplaceUserAssets.ts(1 hunks)src/state/actions/dispatchArIOInteraction.ts(2 hunks)src/state/actions/dispatchArNSUpdate.ts(6 hunks)src/state/contexts/ArNSState.tsx(2 hunks)src/state/contexts/GlobalState.tsx(8 hunks)src/state/reducers/GlobalReducer.ts(3 hunks)src/utils/constants.ts(2 hunks)src/utils/imageUtils.test.ts(0 hunks)src/utils/routes.tsx(3 hunks)src/utils/searchUtils/searchUtils.test.ts(0 hunks)src/utils/searchUtils/searchUtils.ts(1 hunks)src/utils/sentry.ts(1 hunks)tests/common/fixtures/TestRecords.ts(0 hunks)tests/common/mocks/ArweaveCompositeDataProviderMock.ts(0 hunks)tests/common/mocks/svg.js(0 hunks)tests/playwright/mobile.spec.ts(0 hunks)tests/playwright/setup.ts(0 hunks)tests/unit/HomeSearch.test.tsx(0 hunks)vite.config.ts(0 hunks)
💤 Files with no reviewable changes (19)
- tests/common/mocks/svg.js
- vite.config.ts
- tests/common/fixtures/TestRecords.ts
- playwright.config.ts
- tests/playwright/mobile.spec.ts
- src/components/modals/LogoUploadModal/tests/LogoUploadModal.test.tsx
- mocks/react-router-dom.ts
- babel.config.cjs
- src/components/inputs/text/ValidationInput/ValidationInput.test.tsx
- src/components/layout/Navbar/NavGroup/NavGroup.test.tsx
- src/utils/searchUtils/searchUtils.test.ts
- src/utils/imageUtils.test.ts
- tests/unit/HomeSearch.test.tsx
- jest.config.json
- src/components/layout/Navbar/NavBarLink/NavBarLink.test.tsx
- tests/common/mocks/ArweaveCompositeDataProviderMock.ts
- src/components/inputs/buttons/ConnectButton/ConnectButton.test.tsx
- tests/playwright/setup.ts
- jest-setup.ts
🧰 Additional context used
🧠 Learnings (5)
📚 Learning: 2025-12-11T05:20:18.825Z
Learnt from: vilenarios
Repo: ar-io/arns-react PR: 902
File: src/components/modals/turbo/panels/crypto/CryptoConfirmation.tsx:536-540
Timestamp: 2025-12-11T05:20:18.825Z
Learning: Guideline: For tokens on Base network that use 6 decimals (e.g., base-ario ERC-20), convert amount to smallest units using a 1e6 multiplier. Use Math.round(amount * 1e6) when computing the on-chain value. Do not assume 18 decimals. Apply this conversion consistently in code paths that perform payments in CryptoConfirmation.tsx or similar components, and document that the base-ario/USDC-like tokens use 6 decimals to avoid mispricing or transfer errors.
Applied to files:
src/components/pages/Settings/devtools/MarketplaceTools.tsxsrc/components/pages/Manage/Manage.tsxsrc/components/pages/Register/Checkout.tsxsrc/components/modals/ContinueWorkflowModal/ContinueWorkflowModal.tsxsrc/components/modals/ant-management/UpgradeDomainForMarketplaceModal/UpgradeDomainForMarketplaceModal.tsxsrc/components/layout/Notifications/Notifications.tsxsrc/components/pages/ViewListing/ViewListing.tsxsrc/components/modals/ManageMarketplaceARIOModal/panels/WithdrawPanel.tsxsrc/components/data-display/tables/DomainsTable.tsxsrc/components/pages/Settings/devtools/DevTools.tsxsrc/components/pages/Settings/devtools/ANTTools.tsxsrc/components/data-display/tables/MarketplaceListingsTable.tsxsrc/components/modals/ListNameForSaleModal/panels/FixedPricePanel.tsxsrc/components/inputs/DateTimePicker/DateTimePicker.tsxsrc/components/modals/ManageMarketplaceARIOModal/ManageMarketplaceARIOModal.tsxsrc/components/cards/NavMenuCard/NavMenuCard.tsxsrc/components/pages/Marketplace/Marketplace.tsxsrc/components/modals/ManageMarketplaceARIOModal/panels/DepositPanel.tsxsrc/components/cards/MarketplaceOrderInfoCard/MarketplaceOrderInfoCard.tsxsrc/components/indicators/InterruptedWorkflowIndicator/InterruptedWorkflowIndicator.tsxsrc/components/modals/ListNameForSaleModal/ListNameForSaleModal.tsxsrc/components/modals/ListNameForSaleModal/panels/ConfirmListingPanel.tsxsrc/components/pages/Settings/NetworkSettings.tsxsrc/components/layout/Navbar/NotificationMenu/NotificationMenu.tsx
📚 Learning: 2025-09-10T16:35:55.727Z
Learnt from: dtfiedler
Repo: ar-io/arns-react PR: 873
File: src/components/modals/ant-management/UpgradeDomainsModal/UpgradeDomainsModal.tsx:0-0
Timestamp: 2025-09-10T16:35:55.727Z
Learning: In arns-react, when calling buildDomainInfoQuery (src/hooks/useDomainInfo.tsx), you must pass arioProcessId along with arioContract, domain, aoNetwork, hyperbeamUrl, and antRegistryProcessId; omitting arioProcessId can lead to "No ANT id or record found" and break upgrades.
Applied to files:
src/components/pages/Manage/Manage.tsxsrc/components/pages/Register/Checkout.tsxsrc/components/modals/ant-management/UpgradeDomainForMarketplaceModal/UpgradeDomainForMarketplaceModal.tsxsrc/components/data-display/tables/DomainsTable.tsxsrc/state/contexts/ArNSState.tsxsrc/state/actions/dispatchArNSUpdate.tssrc/hooks/useMarketplaceOrder.tsxsrc/components/pages/Settings/settingsConfig.tssrc/state/contexts/GlobalState.tsx
📚 Learning: 2025-09-10T16:35:55.727Z
Learnt from: dtfiedler
Repo: ar-io/arns-react PR: 873
File: src/components/modals/ant-management/UpgradeDomainsModal/UpgradeDomainsModal.tsx:0-0
Timestamp: 2025-09-10T16:35:55.727Z
Learning: In arns-react, when calling buildDomainInfoQuery (src/hooks/useDomainInfo.tsx), you must pass arioProcessId along with arioContract, domain, aoNetwork, hyperbeamUrl, and antRegistryProcessId; omitting arioProcessId can lead to “No ANT id or record found” and break upgrades.
Applied to files:
src/components/pages/Manage/Manage.tsxsrc/components/pages/Register/Checkout.tsxsrc/components/modals/ant-management/UpgradeDomainForMarketplaceModal/UpgradeDomainForMarketplaceModal.tsxsrc/components/data-display/tables/DomainsTable.tsxsrc/state/contexts/ArNSState.tsxsrc/state/actions/dispatchArNSUpdate.tssrc/hooks/useMarketplaceOrder.tsxsrc/components/pages/Settings/settingsConfig.tssrc/state/contexts/GlobalState.tsx
📚 Learning: 2025-12-02T17:38:52.889Z
Learnt from: vilenarios
Repo: ar-io/arns-react PR: 899
File: src/services/turbo/BaseTokenPurchaseService.ts:0-0
Timestamp: 2025-12-02T17:38:52.889Z
Learning: In src/services/turbo/BaseTokenPurchaseService.ts, the function executeBaseTokenPurchase intentionally only handles Base token top-up to Turbo credits, not the actual ArNS purchase. The ArNS purchase is handled separately by dispatchArIOInteraction in Checkout.tsx because it manages multiple complex purchase workflows (lease, permabuy, extend, increase undernames, etc.) with their own state management. This separation of concerns is by design.
Applied to files:
src/components/pages/Register/Checkout.tsxsrc/state/contexts/ArNSState.tsxsrc/state/actions/dispatchArNSUpdate.tssrc/state/reducers/GlobalReducer.tssrc/state/contexts/GlobalState.tsx
📚 Learning: 2025-10-22T16:07:22.654Z
Learnt from: atticusofsparta
Repo: ar-io/arns-react PR: 887
File: src/state/actions/dispatchANTInteraction.ts:0-0
Timestamp: 2025-10-22T16:07:22.654Z
Learning: In the ar.io/sdk, ANT.init accepts ContractSigner directly (any signer type) and internally resolves it, while ANT.spawn requires an AoSigner and should be passed createAoSigner(signer).
Applied to files:
src/components/pages/Settings/devtools/ANTTools.tsx
🧬 Code graph analysis (28)
src/utils/searchUtils/searchUtils.ts (1)
src/utils/constants.ts (1)
URL_REGEX(90-99)
src/components/pages/Settings/devtools/MarketplaceTools.tsx (3)
src/hooks/useMarketplaceInfo.ts (1)
useMarketplaceInfo(4-25)src/state/contexts/GlobalState.tsx (1)
useGlobalState(175-176)src/utils/constants.ts (1)
DEFAULT_MIN_MARKETPLACE_ANT_VERSION(29-29)
src/components/pages/Manage/Manage.tsx (1)
src/state/contexts/GlobalState.tsx (1)
useGlobalState(175-176)
src/hooks/useMarketplaceInfo.ts (2)
src/state/contexts/GlobalState.tsx (1)
useGlobalState(175-176)src/state/contexts/WalletState.tsx (1)
useWalletState(49-50)
src/components/layout/Notifications/Notifications.tsx (1)
src/utils/errors.ts (1)
NotificationOnlyError(4-9)
src/components/modals/ManageMarketplaceARIOModal/panels/WithdrawPanel.tsx (8)
src/state/contexts/GlobalState.tsx (1)
useGlobalState(175-176)src/state/contexts/WalletState.tsx (1)
useWalletState(49-50)src/hooks/useArIOPrice.ts (1)
useArIoPrice(9-32)src/hooks/useMarketplaceUserAssets.ts (1)
useMarketplaceUserAssets(44-61)src/utils/common/common.tsx (1)
formatARIOWithCommas(215-219)src/utils/searchUtils/searchUtils.ts (1)
sleep(128-130)src/utils/network.ts (1)
queryClient(39-45)src/components/layout/index.ts (1)
Loader(12-12)
src/components/data-display/tables/MarketplaceListingsTable.tsx (11)
src/state/contexts/GlobalState.tsx (1)
useGlobalState(175-176)src/hooks/useArIOPrice.ts (1)
useArIoPrice(9-32)src/hooks/useArNSRecords.tsx (1)
useArNSRecords(5-40)src/hooks/useMarketplaceOrders.ts (1)
useMarketplaceOrders(72-84)src/utils/searchUtils/searchUtils.ts (1)
decodeDomainToASCII(48-52)src/utils/common/common.tsx (3)
formatForMaxCharCount(134-148)formatARIOWithCommas(215-219)formatVerboseDate(21-36)src/components/data-display/index.tsx (1)
Tooltip(5-5)src/services/arweave/ArweaveTransactionID.ts (1)
ArweaveTransactionID(7-31)src/types.ts (1)
AoAddress(562-562)src/components/icons/index.ts (1)
RefreshIcon(127-127)src/components/layout/index.ts (1)
Loader(12-12)
src/components/modals/ListNameForSaleModal/panels/FixedPricePanel.tsx (4)
src/hooks/useArNSDomainPriceList.tsx (1)
useArNSDomainPriceList(9-83)src/utils/common/common.tsx (1)
formatARIOWithCommas(215-219)src/components/data-display/index.tsx (1)
Tooltip(5-5)src/components/icons/index.ts (1)
InfoIcon(115-115)
src/utils/routes.tsx (1)
src/components/pages/Marketplace/Marketplace.tsx (1)
Marketplace(6-41)
src/components/cards/NavMenuCard/NavMenuCard.tsx (3)
src/state/contexts/GlobalState.tsx (1)
useGlobalState(175-176)src/hooks/useMarketplaceUserAssets.ts (1)
buildMarketplaceUserAssetsQuery(6-42)src/components/modals/index.ts (1)
ManageMarketplaceARIOModal(14-14)
src/state/contexts/ArNSState.tsx (1)
src/state/contexts/GlobalState.tsx (1)
useGlobalState(175-176)
src/state/actions/dispatchArNSUpdate.ts (4)
src/state/reducers/ArNSReducer.ts (1)
ArNSAction(10-30)src/types.ts (1)
AoAddress(562-562)src/utils/network.ts (1)
queryClient(39-45)src/hooks/useMarketplaceUserAssets.ts (1)
buildMarketplaceUserAssetsQuery(6-42)
src/hooks/useMarketplaceOrder.tsx (2)
src/utils/constants.ts (1)
NETWORK_DEFAULTS(144-178)src/state/contexts/GlobalState.tsx (1)
useGlobalState(175-176)
src/hooks/useInterruptedWorkflows.ts (3)
src/state/contexts/ArNSState.tsx (1)
ANTProcessData(16-21)src/hooks/useMarketplaceUserAssets.ts (1)
useMarketplaceUserAssets(44-61)src/state/contexts/WalletState.tsx (1)
useWalletState(49-50)
src/hooks/useMarketplaceUserAssets.ts (3)
src/utils/constants.ts (1)
NETWORK_DEFAULTS(144-178)src/state/contexts/GlobalState.tsx (1)
useGlobalState(175-176)src/state/contexts/WalletState.tsx (1)
useWalletState(49-50)
src/components/pages/Marketplace/Marketplace.tsx (2)
src/state/contexts/GlobalState.tsx (1)
useGlobalState(175-176)src/components/modals/index.ts (1)
ManageMarketplaceARIOModal(14-14)
src/hooks/useMarketplaceOrders.ts (2)
src/utils/constants.ts (1)
NETWORK_DEFAULTS(144-178)src/state/contexts/GlobalState.tsx (1)
useGlobalState(175-176)
src/components/modals/ManageMarketplaceARIOModal/panels/DepositPanel.tsx (7)
src/state/contexts/GlobalState.tsx (1)
useGlobalState(175-176)src/state/contexts/WalletState.tsx (1)
useWalletState(49-50)src/hooks/useArIOPrice.ts (1)
useArIoPrice(9-32)src/hooks/useMarketplaceUserAssets.ts (1)
useMarketplaceUserAssets(44-61)src/utils/searchUtils/searchUtils.ts (1)
sleep(128-130)src/utils/network.ts (1)
queryClient(39-45)src/components/layout/index.ts (1)
Loader(12-12)
src/components/cards/MarketplaceOrderInfoCard/MarketplaceOrderInfoCard.tsx (8)
src/hooks/useDomainInfo.tsx (1)
useDomainInfo(206-255)src/utils/common/common.tsx (2)
formatARIOWithCommas(215-219)formatVerboseDate(21-36)src/components/icons/index.ts (1)
ArNSLogo(72-72)src/components/data-display/AntLogoIcon.tsx (1)
AntLogoIcon(7-57)src/utils/transactionUtils/transactionUtils.tsx (1)
isArweaveTransactionID(40-48)src/utils/constants.ts (1)
DEFAULT_ANT_LOGO(117-117)src/components/data-display/index.tsx (1)
Tooltip(5-5)src/utils/searchUtils/searchUtils.ts (1)
decodeDomainToASCII(48-52)
src/components/indicators/InterruptedWorkflowIndicator/InterruptedWorkflowIndicator.tsx (1)
src/components/data-display/index.tsx (1)
Tooltip(5-5)
src/components/modals/ListNameForSaleModal/ListNameForSaleModal.tsx (3)
src/hooks/useArIOPrice.ts (1)
useArIoPrice(9-32)src/hooks/useANTIntent.ts (1)
useANTIntent(17-75)src/services/arweave/ArweaveTransactionID.ts (1)
ArweaveTransactionID(7-31)
src/components/modals/ListNameForSaleModal/panels/ConfirmListingPanel.tsx (10)
src/state/contexts/WalletState.tsx (1)
useWalletState(49-50)src/hooks/useDomainInfo.tsx (1)
useDomainInfo(206-255)src/utils/transactionUtils/transactionUtils.tsx (1)
isArweaveTransactionID(40-48)src/hooks/useMarketplaceInfo.ts (1)
useMarketplaceInfo(4-25)src/utils/common/common.tsx (1)
formatARIOWithCommas(215-219)src/components/data-display/index.tsx (1)
Tooltip(5-5)src/components/icons/index.ts (2)
InfoIcon(115-115)ArNSLogo(72-72)src/utils/constants.ts (1)
DEFAULT_ANT_LOGO(117-117)src/utils/searchUtils/searchUtils.ts (1)
decodeDomainToASCII(48-52)src/components/inputs/Checkbox.tsx (1)
Checkbox(46-46)
src/App.tsx (1)
src/utils/routes.tsx (1)
Route(27-35)
src/components/pages/Settings/settingsConfig.ts (1)
src/utils/constants.ts (1)
DEFAULT_MIN_MARKETPLACE_ANT_VERSION(29-29)
src/components/pages/Settings/NetworkSettings.tsx (2)
src/utils/searchUtils/searchUtils.ts (1)
isValidNetworkURL(204-241)src/utils/constants.ts (1)
NETWORK_DEFAULTS(144-178)
src/hooks/useANTIntent.ts (2)
src/hooks/useMarketplaceUserAssets.ts (1)
useMarketplaceUserAssets(44-61)src/state/contexts/WalletState.tsx (1)
useWalletState(49-50)
src/components/layout/Navbar/NotificationMenu/NotificationMenu.tsx (4)
src/hooks/useInterruptedWorkflows.ts (1)
useInterruptedWorkflows(15-67)src/utils/searchUtils/searchUtils.ts (1)
getAntsRequiringUpdate(136-162)src/components/data-display/index.tsx (1)
Tooltip(5-5)src/components/icons/index.ts (1)
BellIcon(87-87)
src/state/contexts/GlobalState.tsx (1)
src/utils/constants.ts (2)
MARKETPLACE_PROCESS_ID(103-105)DEFAULT_MIN_MARKETPLACE_ANT_VERSION(29-29)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build_for_deploy
|
|
||
| // set the log level of ar-io-sdk | ||
| Logger.default.setLogLevel('none'); | ||
| Logger.default.setLogLevel('debug'); |
There was a problem hiding this comment.
Reconsider enabling debug logging in all environments.
Setting the log level to 'debug' for the ar-io-sdk will apply to all environments, including production. This could negatively impact performance, expose sensitive information in browser consoles, and increase noise in production logs.
Consider making the log level environment-dependent:
-Logger.default.setLogLevel('debug');
+Logger.default.setLogLevel(
+ import.meta.env.MODE === 'production' ? 'none' : 'debug'
+);📝 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.
| Logger.default.setLogLevel('debug'); | |
| Logger.default.setLogLevel( | |
| import.meta.env.MODE === 'production' ? 'none' : 'debug' | |
| ); |
🤖 Prompt for AI Agents
In src/App.tsx around line 26, the code unconditionally sets
Logger.default.setLogLevel('debug'), which forces debug logging in all
environments; change this to be environment-dependent by reading NODE_ENV (or
the app's env var like REACT_APP_ENV), and only set 'debug' when NODE_ENV ===
'development' (or the explicit dev env), otherwise set a less verbose default
such as 'info' or 'warn'; ensure a safe fallback if the env var is missing so
production never defaults to 'debug'.
| import { Tooltip } from '@src/components/data-display'; | ||
| import { AntLogoIcon } from '@src/components/data-display/AntLogoIcon'; | ||
| import { ArNSLogo } from '@src/components/icons'; | ||
| import { Checkbox } from '@src/components/inputs/Checkbox'; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Remove unused import.
Checkbox is imported but not used anywhere in this component.
-import { Checkbox } from '@src/components/inputs/Checkbox';📝 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.
| import { Checkbox } from '@src/components/inputs/Checkbox'; |
🤖 Prompt for AI Agents
In src/components/cards/MarketplaceOrderInfoCard/MarketplaceOrderInfoCard.tsx
around line 5, there is an unused import "Checkbox" from
'@src/components/inputs/Checkbox'; remove that import line to eliminate the
unused dependency and resolve the linter warning, and run the project/linter to
ensure no further unused-imports remain.
| <Link | ||
| to={`/marketplace/listing/${name}`} | ||
| className="text-link hover:text-primary-light transition-colors font-medium" | ||
| > | ||
| {formatForMaxCharCount(decodedName, 30)} | ||
| </Link> |
There was a problem hiding this comment.
Inconsistent URL path for listing links.
Line 157 uses /marketplace/listing/${name} while line 125 uses /marketplace/names/${name}. These should be consistent to avoid routing issues.
Verify which path is correct and update for consistency:
<Link
- to={`/marketplace/listing/${name}`}
+ to={`/marketplace/names/${name}`}
className="text-link hover:text-primary-light transition-colors font-medium"
>📝 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.
| <Link | |
| to={`/marketplace/listing/${name}`} | |
| className="text-link hover:text-primary-light transition-colors font-medium" | |
| > | |
| {formatForMaxCharCount(decodedName, 30)} | |
| </Link> | |
| <Link | |
| to={`/marketplace/names/${name}`} | |
| className="text-link hover:text-primary-light transition-colors font-medium" | |
| > | |
| {formatForMaxCharCount(decodedName, 30)} | |
| </Link> |
🤖 Prompt for AI Agents
In src/components/data-display/tables/MarketplaceListingsTable.tsx around lines
156 to 161, the link path uses `/marketplace/listing/${name}` which is
inconsistent with `/marketplace/names/${name}` used elsewhere; confirm the
correct route (likely `/marketplace/names/${name}`) and update this Link to use
the same path, and then run/adjust any affected tests or usages to ensure
routing remains consistent.
| value?: Date; | ||
| onChange: (date: Date | undefined) => void; | ||
| placeholder?: string; | ||
| minDate?: Date; |
There was a problem hiding this comment.
Remove unused minDate prop or implement its logic.
The minDate prop is defined in the interface but never used in the component logic. This is misleading to consumers of the component who might expect it to constrain date selection.
Apply this diff to remove the unused prop:
interface DateTimePickerProps {
value?: Date;
onChange: (date: Date | undefined) => void;
placeholder?: string;
- minDate?: Date;
className?: string;
}Alternatively, if minDate is intended to be used, implement it in the isDateDisabled function at Line 66.
📝 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.
| minDate?: Date; | |
| interface DateTimePickerProps { | |
| value?: Date; | |
| onChange: (date: Date | undefined) => void; | |
| placeholder?: string; | |
| className?: string; | |
| } |
🤖 Prompt for AI Agents
In src/components/inputs/DateTimePicker/DateTimePicker.tsx around line 10 (prop
definition) and line 66 (isDateDisabled), the minDate prop is declared but
unused; either remove it from the component props and related types/usages, or
implement it in isDateDisabled. To implement: update isDateDisabled to
early-return true when a candidate date is before the provided minDate (handle
undefined minDate), comparing dates normalized to the same time basis (e.g.,
strip time or use getTime()) so selection is correctly constrained; to remove:
delete minDate from the props interface and any stories/tests/usages that pass
it. Ensure TypeScript types and exported prop docs are updated accordingly.
| const marketPriceDisplay = domainPrices?.buy | ||
| ? `Market Value: ${formatARIOWithCommas(Math.ceil(domainPrices.buy / 1000000))} ${arioTicker}` | ||
| : ''; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Inconsistent mARIO to ARIO conversion.
Lines 51-53 correctly use new mARIOToken(domainPrices.buy).toARIO().valueOf(), but lines 103, 113, and 117 use manual division by 1000000. Use the SDK's conversion method consistently to avoid errors if the decimal precision changes.
- const marketPriceDisplay = domainPrices?.buy
- ? `Market Value: ${formatARIOWithCommas(Math.ceil(domainPrices.buy / 1000000))} ${arioTicker}`
- : '';
+ const marketValueARIO = domainPrices?.buy
+ ? Math.ceil(new mARIOToken(domainPrices.buy).toARIO().valueOf())
+ : 0;
+ const marketPriceDisplay = marketValueARIO > 0
+ ? `Market Value: ${formatARIOWithCommas(marketValueARIO)} ${arioTicker}`
+ : '';Then use marketValueARIO in the JSX instead of recalculating.
Also applies to: 113-117
🤖 Prompt for AI Agents
In src/components/modals/ListNameForSaleModal/panels/FixedPricePanel.tsx around
lines 102-117, the code manually divides domainPrices.buy by 1,000,000 to
convert mARIO->ARIO; replace those manual divisions by using the SDK conversion
used earlier (new mARIOToken(domainPrices.buy).toARIO().valueOf()), assign the
result to a marketValueARIO variable (e.g., const marketValueARIO =
domainPrices?.buy ? new mARIOToken(domainPrices.buy).toARIO().valueOf() : 0) and
then use marketValueARIO when building the market price display and in the JSX
instead of recalculating with /1000000; ensure proper null/undefined handling to
match existing behavior.
| {PRICE_BUTTON_VALUES.map((value, index) => ( | ||
| <button | ||
| key={index} | ||
| className={`rounded p-2.5 transition-colors ${ | ||
| buttonSelected === index | ||
| ? 'bg-white text-black' | ||
| : 'bg-dark-grey text-white hover:bg-gray-600' | ||
| }`} | ||
| onClick={() => { | ||
| setCustomValue(value.toString()); | ||
| setListingPrice(value); | ||
| setButtonSelected(index); |
There was a problem hiding this comment.
Bug: button selection comparison uses index instead of value.
buttonSelected === index compares the selected value with the loop index, but setButtonSelected(index) stores the index. However, lines 64-67 try to find a matching button by value: PRICE_BUTTON_VALUES.find((value) => value === listingPrice). This creates an inconsistency where returning from the confirm screen sets buttonSelected to the value, but the rendering compares against the index.
Either consistently use values:
{PRICE_BUTTON_VALUES.map((value, index) => (
<button
key={index}
className={`rounded p-2.5 transition-colors ${
- buttonSelected === index
+ buttonSelected === value
? 'bg-white text-black'
: 'bg-dark-grey text-white hover:bg-gray-600'
}`}
onClick={() => {
setCustomValue(value.toString());
setListingPrice(value);
- setButtonSelected(index);
+ setButtonSelected(value);
setCustomValueError('');
}}📝 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.
| {PRICE_BUTTON_VALUES.map((value, index) => ( | |
| <button | |
| key={index} | |
| className={`rounded p-2.5 transition-colors ${ | |
| buttonSelected === index | |
| ? 'bg-white text-black' | |
| : 'bg-dark-grey text-white hover:bg-gray-600' | |
| }`} | |
| onClick={() => { | |
| setCustomValue(value.toString()); | |
| setListingPrice(value); | |
| setButtonSelected(index); | |
| {PRICE_BUTTON_VALUES.map((value, index) => ( | |
| <button | |
| key={index} | |
| className={`rounded p-2.5 transition-colors ${ | |
| buttonSelected === value | |
| ? 'bg-white text-black' | |
| : 'bg-dark-grey text-white hover:bg-gray-600' | |
| }`} | |
| onClick={() => { | |
| setCustomValue(value.toString()); | |
| setListingPrice(value); | |
| setButtonSelected(value); |
🤖 Prompt for AI Agents
In src/components/modals/ListNameForSaleModal/panels/FixedPricePanel.tsx around
lines 133 to 144, the rendered button selection compares buttonSelected ===
index but elsewhere buttonSelected is being set from a value (listingPrice) when
returning from the confirm screen, causing mismatch; make the stored and
compared identifier consistent — either store and compare the actual price value
or consistently store and compare the index. Fix by choosing one approach and
updating all usages: if you prefer values, call setButtonSelected(value) in the
onClick and change the render comparison to buttonSelected === value (and ensure
initial state and any find/lookup use the value); if you prefer indices, ensure
when restoring from listingPrice you compute the matching index (e.g., findIndex
on PRICE_BUTTON_VALUES) and call setButtonSelected(index) so rendering remains
index-based.
| Sentry.browserTracingIntegration(), | ||
| Sentry.reactRouterV6BrowserTracingIntegration({ | ||
| useEffect, | ||
| useLocation, | ||
| useNavigationType, | ||
| createRoutesFromChildren, | ||
| matchRoutes, | ||
| }), |
There was a problem hiding this comment.
Remove the redundant browserTracingIntegration() integration.
For React Router v6, use only reactRouterV6BrowserTracingIntegration(). The Sentry documentation states to "update your Sentry.browserTracingIntegration to Sentry.reactRouterV6BrowserTracingIntegration", indicating a replacement. Using both integrations together will create duplicate tracing spans and unnecessary overhead. Remove Sentry.browserTracingIntegration() from line 24.
🤖 Prompt for AI Agents
In src/utils/sentry.ts around lines 24 to 31, the
Sentry.browserTracingIntegration() call is redundant when using
Sentry.reactRouterV6BrowserTracingIntegration; remove the
Sentry.browserTracingIntegration() entry from the integrations list so only
Sentry.reactRouterV6BrowserTracingIntegration({...}) remains, ensuring you leave
the reactRouterV6 integration and its passed hooks intact and update any imports
if necessary.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/pages/Settings/NetworkSettings.tsx (1)
492-492: Add validation guard to SU Address onPressEnter handler.The SU Address
onPressEnterhandler lacks validation, which is inconsistent with the pattern established for other inputs in this file. Transaction ID inputs (AR.IO Contract at lines 330-337 and ANT Registry at lines 369-376) validate withisArweaveTransactionIDbefore dispatching changes.Apply this diff to add validation:
- onPressEnter={(value) => updateAoNetwork({ SCHEDULER: value })} + onPressEnter={(value) => { + if (isArweaveTransactionID(value)) { + updateAoNetwork({ SCHEDULER: value }); + } + }}
🧹 Nitpick comments (3)
package.json (2)
61-61:permaweb-deployshould be a devDependency.This is a deployment CLI tool, not a runtime dependency. Moving it to
devDependenciesreduces the production bundle footprint."dependencies": { ... - "permaweb-deploy": "^2.5.1", ... }, "devDependencies": { ... + "permaweb-deploy": "^2.5.1", ... }
52-53: Redundant date libraries:date-fnsanddayjs.Both
date-fnsanddayjsare date manipulation libraries. Consider standardizing on one to reduce bundle size, unless there's a specific reason for both (e.g., existing codebase uses one, new features use another for a specific capability).src/services/wallets/EthWalletConnector.ts (1)
79-81: Remove @ts-ignore and document the type compatibility workaround or correct the signature mismatch.The aoSigner function has a
@ts-ignoredirective and defaults empty data to a space character, both suggesting a known incompatibility with theAoSignerinterface from@ar.io/sdk. This approach masks the type mismatch rather than addressing it.Consider one of:
- If data can legitimately be absent: Update the type signature to explicitly mark
dataas optional and document when this occurs and why defaulting to a space is acceptable for signing operations.- If data should always be present: Remove the default guard and fix the underlying type compatibility issue by ensuring the implementation matches the interface contract.
The current pattern with
@ts-ignoreand silent defaulting makes the code difficult to maintain and could hide integration issues with the SDK in the future.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
package.json(3 hunks)src/components/pages/Settings/NetworkSettings.tsx(7 hunks)src/services/wallets/EthWalletConnector.ts(1 hunks)src/services/wallets/WanderWalletConnector.ts(3 hunks)src/state/contexts/GlobalState.tsx(8 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/services/wallets/WanderWalletConnector.ts
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
Learnt from: dtfiedler
Repo: ar-io/arns-react PR: 873
File: src/components/modals/ant-management/UpgradeDomainsModal/UpgradeDomainsModal.tsx:0-0
Timestamp: 2025-09-10T16:35:55.727Z
Learning: In arns-react, when calling buildDomainInfoQuery (src/hooks/useDomainInfo.tsx), you must pass arioProcessId along with arioContract, domain, aoNetwork, hyperbeamUrl, and antRegistryProcessId; omitting arioProcessId can lead to “No ANT id or record found” and break upgrades.
Learnt from: dtfiedler
Repo: ar-io/arns-react PR: 873
File: src/components/modals/ant-management/UpgradeDomainsModal/UpgradeDomainsModal.tsx:0-0
Timestamp: 2025-09-10T16:35:55.727Z
Learning: In arns-react, when calling buildDomainInfoQuery (src/hooks/useDomainInfo.tsx), you must pass arioProcessId along with arioContract, domain, aoNetwork, hyperbeamUrl, and antRegistryProcessId; omitting arioProcessId can lead to "No ANT id or record found" and break upgrades.
Learnt from: vilenarios
Repo: ar-io/arns-react PR: 899
File: src/services/turbo/BaseTokenPurchaseService.ts:0-0
Timestamp: 2025-12-02T17:38:52.889Z
Learning: In src/services/turbo/BaseTokenPurchaseService.ts, the function executeBaseTokenPurchase intentionally only handles Base token top-up to Turbo credits, not the actual ArNS purchase. The ArNS purchase is handled separately by dispatchArIOInteraction in Checkout.tsx because it manages multiple complex purchase workflows (lease, permabuy, extend, increase undernames, etc.) with their own state management. This separation of concerns is by design.
📚 Learning: 2025-12-11T05:20:18.825Z
Learnt from: vilenarios
Repo: ar-io/arns-react PR: 902
File: src/components/modals/turbo/panels/crypto/CryptoConfirmation.tsx:536-540
Timestamp: 2025-12-11T05:20:18.825Z
Learning: Guideline: For tokens on Base network that use 6 decimals (e.g., base-ario ERC-20), convert amount to smallest units using a 1e6 multiplier. Use Math.round(amount * 1e6) when computing the on-chain value. Do not assume 18 decimals. Apply this conversion consistently in code paths that perform payments in CryptoConfirmation.tsx or similar components, and document that the base-ario/USDC-like tokens use 6 decimals to avoid mispricing or transfer errors.
Applied to files:
src/components/pages/Settings/NetworkSettings.tsx
📚 Learning: 2025-12-16T16:47:36.348Z
Learnt from: dtfiedler
Repo: ar-io/arns-react PR: 904
File: .github/workflows/pr-preview.yaml:52-58
Timestamp: 2025-12-16T16:47:36.348Z
Learning: In the ar-io/arns-react repository, ArNS undername cleanup for PR previews is handled by the permaweb-deploy GitHub action, not by a local script in this repository. The cleanup step should not be included in the PR preview workflow.
Applied to files:
package.json
📚 Learning: 2025-12-02T17:38:52.889Z
Learnt from: vilenarios
Repo: ar-io/arns-react PR: 899
File: src/services/turbo/BaseTokenPurchaseService.ts:0-0
Timestamp: 2025-12-02T17:38:52.889Z
Learning: In src/services/turbo/BaseTokenPurchaseService.ts, the function executeBaseTokenPurchase intentionally only handles Base token top-up to Turbo credits, not the actual ArNS purchase. The ArNS purchase is handled separately by dispatchArIOInteraction in Checkout.tsx because it manages multiple complex purchase workflows (lease, permabuy, extend, increase undernames, etc.) with their own state management. This separation of concerns is by design.
Applied to files:
src/state/contexts/GlobalState.tsx
📚 Learning: 2025-09-10T16:35:55.727Z
Learnt from: dtfiedler
Repo: ar-io/arns-react PR: 873
File: src/components/modals/ant-management/UpgradeDomainsModal/UpgradeDomainsModal.tsx:0-0
Timestamp: 2025-09-10T16:35:55.727Z
Learning: In arns-react, when calling buildDomainInfoQuery (src/hooks/useDomainInfo.tsx), you must pass arioProcessId along with arioContract, domain, aoNetwork, hyperbeamUrl, and antRegistryProcessId; omitting arioProcessId can lead to "No ANT id or record found" and break upgrades.
Applied to files:
src/state/contexts/GlobalState.tsx
📚 Learning: 2025-09-10T16:35:55.727Z
Learnt from: dtfiedler
Repo: ar-io/arns-react PR: 873
File: src/components/modals/ant-management/UpgradeDomainsModal/UpgradeDomainsModal.tsx:0-0
Timestamp: 2025-09-10T16:35:55.727Z
Learning: In arns-react, when calling buildDomainInfoQuery (src/hooks/useDomainInfo.tsx), you must pass arioProcessId along with arioContract, domain, aoNetwork, hyperbeamUrl, and antRegistryProcessId; omitting arioProcessId can lead to “No ANT id or record found” and break upgrades.
Applied to files:
src/state/contexts/GlobalState.tsx
🧬 Code graph analysis (1)
src/state/contexts/GlobalState.tsx (1)
src/utils/constants.ts (2)
MARKETPLACE_PROCESS_ID(103-105)DEFAULT_MIN_MARKETPLACE_ANT_VERSION(29-29)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: lint_and_build
🔇 Additional comments (14)
package.json (1)
23-29: Confirm intentional use of alpha dependencies in production.Using alpha versions (
@ar.io/sdk 3.23.0-alpha.1,@ardrive/turbo-sdk ^1.35.0-alpha.2) in production carries stability risks. Ensure the team is prepared to handle potential breaking changes.src/components/pages/Settings/NetworkSettings.tsx (5)
21-26: LGTM: Clean import of new validator.The addition of
isValidNetworkURLsupports the validation improvements throughout this file.
452-452: Consistent validation pattern for MU_URL.The changes mirror the CU_URL validation improvements and maintain consistency across network configuration inputs.
Also applies to: 460-464
526-526: Consistent validation for Hyperbeam URL with optional field handling.The validation changes are consistent with other network URLs. Note that Hyperbeam URL is optional (can be undefined/disabled), and the
onPressEntervalidation guard means users cannot clear the field by entering an empty string and pressing Enter. However, the Set button (lines 528-533) properly handles clearing via thetrim() || undefinedlogic, so the UX remains functional.Also applies to: 545-552
565-565: Consistent validation pattern for Turbo Payment URL.The changes align with the validation improvements for other network URLs and maintain consistency across the file.
Also applies to: 584-590
427-427: Switch toisValidNetworkURLfor network endpoint validation is sound.The implementation properly validates AO network endpoint formats (CU_URL, MU_URL) by checking for HTTPS/HTTP protocols and validating domain structure, including subdomains like
cu.ao-testnet.xyz. The addition of validation guards inonPressEnterprovides good defensive programming.src/state/contexts/GlobalState.tsx (8)
34-35: LGTM!The marketplace constants are properly imported and align with their definitions in
src/utils/constants.ts.
41-82: LGTM!The
loadSettingsFromStoragefunction is correctly extended to load marketplace settings with proper fallbacks to constants. The implementation follows the established pattern for other settings.
95-99: LGTM!Initial marketplace variables are properly initialized with the correct fallback chain from saved settings to constants.
125-142: LGTM!The
GlobalStatetype is correctly extended with marketplace fields. The union typeArNSMarketplaceRead | ArNSMarketplaceWriteappropriately mirrors the pattern used forarioContract, allowing the contract to be upgraded when a wallet connects.
144-172: LGTM!The
initialStateis correctly extended with marketplace fields and follows consistent patterns:
- Marketplace fields are properly initialized with the constants and defaults
- The network configuration changes to
aoClientandantAoClientmirror thedefaultArIOpatternmarketplaceContractis properly initialized withdefaultMarketplaceContract
8-9: Upgrade to latest stable SDK version when marketplace support is available.The code uses
@ar.io/sdkversion3.23.0-alpha.1(alpha) for marketplace types, but the latest stable version is 3.18.3. Plan to upgrade to a stable release once marketplace support is added to it, as alpha versions carry stability risks and may introduce breaking changes.
115-123: Verify marketplace contract configuration, especially MU_URL requirement.The
defaultMarketplaceContractis configured withoutMU_URL, whiledefaultArIOincludes bothCU_URLandMU_URLfrom the sameinitialAoNetwork.ARIOsource. AlthoughArNSMarketplaceReadis read-only, @ar.io/sdk documentation consistently shows both parameters in all AOProcess configuration examples. Confirm whether:
MU_URLis truly optional for read-onlyArNSMarketplaceReadcontracts, or if it should be included likedefaultArIO- Whether the marketplace process should use its own dedicated endpoint configuration instead of sharing
initialAoNetwork.ARIO.CU_URL
102-113: No issue found. The code correctly initializesdefaultArIOwith explicitCU_URL,MU_URL, andMODE: 'legacy'properties. These are the only properties required by theconnect()function for ARIO initialization, which aligns with the official ARIO SDK documentation. Other network properties likeSCHEDULERandHYPERBEAM_URLare used separately in transaction creation and ARIO initialization respectively, not in the connect() call. The pattern is consistent across allconnect()usages in the file (aoClient, antAoClient, defaultMarketplaceContract).
Permaweb Preview DeployedYour preview is available at: https://arns-react-pr-903_arns.ar.io
|
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
package.json (1)
6-20: Remove test infrastructure or document testing strategy.The complete test infrastructure has been removed: no test configuration files (jest, playwright, vitest), test files, or test dependencies remain in the repository. This PR introduces substantial marketplace functionality without any automated tests, significantly increasing regression risk.
Either:
- Restore the test suite with coverage for the new marketplace features, or
- Document the testing strategy and quality assurance approach used to validate this code
♻️ Duplicate comments (1)
package.json (1)
36-37: Sentry version mismatch remains unresolved.This issue was previously flagged:
@sentry/browser(^10.29.0) and@sentry/react(^7.45.0) use incompatible major versions. Please align them to the same major version to prevent runtime conflicts.
🧹 Nitpick comments (2)
src/components/data-display/tables/MarketplaceListingsTable.tsx (2)
83-95: Consider server-side pagination for larger datasets.The hard-coded
limit: 100may be insufficient as the marketplace grows. While the API supports pagination (hasMore,nextCursor), the component doesn't expose pagination controls to users or implement fetching beyond the first page.Consider either:
- Increasing the limit significantly (e.g., 1000) if the total listing count is expected to remain modest, or
- Implementing proper pagination controls that allow users to navigate through all listings, fetching additional pages as needed.
410-434: Unconventional but functional loading state management.The component passes
isLoading={false}toTableViewwhile managing loading state throughnoDataFoundTextand an empty data array. This works correctly but diverges from the typical pattern whereTableView'sisLoadingprop would control the loading indicator.If you prefer conventional table loading patterns, consider:
Alternative approach using TableView's isLoading prop
<TableView columns={columns as ColumnDef<MarketplaceListing>[]} - data={loadingTableData ? [] : filteredData} - isLoading={false} + data={filteredData} + isLoading={loadingTableData} defaultSortingState={{ id: 'listedAt', desc: true }} noDataFoundText={ - loadingTableData ? ( - <div className="flex flex-col items-center justify-center p-8 text-center"> - <ARIOLoadingSpinner /> - </div> - ) : ( <div className="flex flex-col items-center justify-center p-8 text-center"> <div className="text-grey mb-4"> No marketplace listings found </div> <button onClick={() => refetch()} className="flex items-center gap-2 px-4 py-2 border-white text-white rounded transition-colors" > <RefreshIcon className="w-4 h-4 fill-white" /> Retry </button> </div> - ) } />However, your current implementation is perfectly functional and may provide better UX if TableView's default loading indicator doesn't fit your design requirements.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (5)
package.json(3 hunks)src/components/data-display/tables/MarketplaceListingsTable.tsx(1 hunks)src/components/data-display/tables/TableView.tsx(1 hunks)src/components/indicators/ARIOLoadingSpinner.tsx(1 hunks)src/components/pages/Marketplace/Marketplace.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/components/pages/Marketplace/Marketplace.tsx
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: vilenarios
Repo: ar-io/arns-react PR: 899
File: src/services/turbo/BaseTokenPurchaseService.ts:0-0
Timestamp: 2025-12-02T17:38:52.889Z
Learning: In src/services/turbo/BaseTokenPurchaseService.ts, the function executeBaseTokenPurchase intentionally only handles Base token top-up to Turbo credits, not the actual ArNS purchase. The ArNS purchase is handled separately by dispatchArIOInteraction in Checkout.tsx because it manages multiple complex purchase workflows (lease, permabuy, extend, increase undernames, etc.) with their own state management. This separation of concerns is by design.
📚 Learning: 2025-12-11T05:20:18.825Z
Learnt from: vilenarios
Repo: ar-io/arns-react PR: 902
File: src/components/modals/turbo/panels/crypto/CryptoConfirmation.tsx:536-540
Timestamp: 2025-12-11T05:20:18.825Z
Learning: Guideline: For tokens on Base network that use 6 decimals (e.g., base-ario ERC-20), convert amount to smallest units using a 1e6 multiplier. Use Math.round(amount * 1e6) when computing the on-chain value. Do not assume 18 decimals. Apply this conversion consistently in code paths that perform payments in CryptoConfirmation.tsx or similar components, and document that the base-ario/USDC-like tokens use 6 decimals to avoid mispricing or transfer errors.
Applied to files:
src/components/indicators/ARIOLoadingSpinner.tsxsrc/components/data-display/tables/TableView.tsxsrc/components/data-display/tables/MarketplaceListingsTable.tsx
📚 Learning: 2025-08-08T16:53:05.995Z
Learnt from: dtfiedler
Repo: ar-io/arns-react PR: 841
File: src/components/data-display/tables/UndernamesTable.tsx:59-59
Timestamp: 2025-08-08T16:53:05.995Z
Learning: In src/components/data-display/tables/UndernamesTable.tsx, the isLoading prop should be optional and default to false via destructuring.
Applied to files:
src/components/data-display/tables/TableView.tsx
🧬 Code graph analysis (1)
src/components/data-display/tables/MarketplaceListingsTable.tsx (9)
src/hooks/useArIOPrice.ts (1)
useArIoPrice(9-32)src/hooks/useArNSRecords.tsx (1)
useArNSRecords(5-40)src/hooks/useMarketplaceOrders.ts (1)
useMarketplaceOrders(72-84)src/components/data-display/index.tsx (1)
Tooltip(5-5)src/utils/searchUtils/searchUtils.ts (2)
decodeDomainToASCII(48-52)lowerCaseDomain(132-134)src/utils/common/common.tsx (3)
formatForMaxCharCount(134-148)formatARIOWithCommas(215-219)formatVerboseDate(21-36)src/services/arweave/ArweaveTransactionID.ts (1)
ArweaveTransactionID(7-31)src/types.ts (1)
AoAddress(562-562)src/components/icons/index.ts (1)
RefreshIcon(127-127)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: lint_and_build
🔇 Additional comments (4)
package.json (1)
23-23: Use stable SDK versions instead of pre-release builds.@ar.io/sdk has a stable version 3.18.3 available, and @ardrive/turbo-sdk has a stable version 1.30.0 available. The current alpha versions in package.json introduce unnecessary risk. Migrate to the latest stable releases unless the alpha features are specifically required for marketplace functionality.
src/components/data-display/tables/TableView.tsx (1)
220-220: LGTM - UI styling improvement.The change from
border-btoborder-twith addedroundedclass improves the visual presentation of the no-data indicator, aligning it with the table structure above.src/components/indicators/ARIOLoadingSpinner.tsx (1)
1-17: LGTM - Clean and simple loading indicator.The component is well-structured with sensible defaults and straightforward Lottie integration. The parameterized
sizeprop and optionalclassNameprovide good flexibility for different use cases across the marketplace UI.src/components/data-display/tables/MarketplaceListingsTable.tsx (1)
1-440: Comprehensive marketplace table implementation with good UX.The component demonstrates solid engineering with:
- Proper data fetching and transformation using multiple hooks
- Resolved past issues (null name filtering, consistent URL routing)
- Comprehensive column definitions with appropriate sorting and formatting
- Good error handling and user feedback mechanisms
- Effective use of React patterns (memoization, effects, etc.)
The pagination limitation and loading state pattern have been noted separately for consideration.
There was a problem hiding this comment.
Actionable comments posted: 6
♻️ Duplicate comments (3)
package.json (1)
36-37: Sentry version mismatch between@sentry/browserand@sentry/react.This issue was previously flagged:
@sentry/browserat^10.29.0and@sentry/reactat^7.45.0are incompatible major versions. Please align both packages to the same major version to avoid runtime conflicts.src/components/modals/ListNameForSaleModal/ListNameForSaleModal.tsx (1)
364-395: Infinite loop risk if balance check succeeds but loop continues.The while loop on line 367 checks
newARIOBalance < feeDetails.listingFee, but inside the loop,newARIOBalanceis only updated within the try block. If an exception occurs repeatedly without updatingnewARIOBalance, the loop will continue untilmaxTriesis exhausted, which is expected. However, if the balance check succeeds (line 380-382 doesn't throw), the loop should break but currently continues.Also, the loop will exit on success when the while condition becomes false, but the success path on lines 380-385 incorrectly throws when
newARIOBalance <= feeDetails.listingFee.This is related to the logic error above. The combined fix should ensure the loop exits correctly on success.
src/components/modals/ContinueWorkflowModal/ContinueWorkflowModal.tsx (1)
94-118: Inconsistent price unit handling in orderConfig.At line 96,
priceis assigned asNumber(orderParams.price)without division, but at lines 108-112,startingPriceandreservePricedivide by1000000to convert from mARIO. This inconsistency could lead to displaying incorrect values.🔎 Proposed fix
const listingType = orderParams.orderType || 'fixed'; - const price = orderParams.price ? Number(orderParams.price) : 0; + const price = orderParams.price ? Number(orderParams.price) / 1000000 : 0; const expirationDate = orderParams.expirationTimeAlternatively, use
mARIOTokenfrom the SDK for consistent conversion:const price = orderParams.price ? new mARIOToken(Number(orderParams.price)).toARIO().valueOf() : 0;
🧹 Nitpick comments (8)
package.json (1)
61-61: Movepermaweb-deploytodevDependencies.
permaweb-deployis a deployment CLI tool used only in build/deploy scripts. It doesn't need to be bundled with production dependencies."dependencies": { ... - "permaweb-deploy": "^2.5.1", ... }, "devDependencies": { + "permaweb-deploy": "^2.5.1", ... }src/components/modals/ListNameForSaleModal/ListNameForSaleModal.tsx (2)
107-109: Consider a longer default expiration period.The default expiration of 1 hour from now is quite short. Users may not complete the listing in time, or potential buyers may have limited time to discover and purchase. Consider a default of 24 hours or longer.
const [expirationDate, setExpirationDate] = useState<Date | undefined>( - new Date(Date.now() + 60 * 60 * 1000), // Default to 1 hour from now (current day) + new Date(Date.now() + 24 * 60 * 60 * 1000), // Default to 24 hours from now );
489-503: Query reset predicates may match unintended queries.The string-based query key matching with
startsWith('marketplace')andincludes(domainName)could inadvertently reset unrelated queries. Consider using more specific query keys or exact matching.🔎 Suggested improvement
queryClient.resetQueries({ predicate: (query) => query.queryKey.some( (key: unknown) => - typeof key === 'string' && key.startsWith('marketplace'), + key === 'marketplace-orders' || + key === 'marketplace-intents' || + key === 'marketplace-user-assets', ), });src/components/data-display/VerticalTimelineStepper.tsx (1)
15-39: Consider caching the entries array to avoid repeated computation.
Object.entries(steps)is called twice per render (once in the map and once for length check). Caching it improves efficiency.🔎 Suggested improvement
export default function VerticalTimelineStepper({ steps, }: { steps: Record< string, { title: ReactNode; description: ReactNode; icon: ReactNode; } >; }) { + const stepEntries = Object.entries(steps); return ( <ol className="relative gap-2"> - {Object.entries(steps).map(([, { title, description, icon }], i) => ( + {stepEntries.map(([key, { title, description, icon }], i) => ( - <div key={i}> + <div key={key}> <li className="ms-7"> <span className="absolute flex items-center justify-center w-8 h-8 rounded-full -start-4 ring-1 ring-dark-grey"> {icon} </span> <div className="flex flex-col max-h-8 overflow-visible"> {' '} <h3 className="text-md leading-tight">{title}</h3> <p className="text-xs text-grey">{description}</p> </div> </li> <div className={ - i === Object.entries(steps).length - 1 + i === stepEntries.length - 1 ? '' : 'border-l border-dark-grey h-8' } - key={`${i}-divider`} /> </div> ))} </ol> ); }src/components/modals/ContinueWorkflowModal/ContinueWorkflowModal.tsx (1)
188-191: Avoidanytype casts for better type safety.Using
as anyon line 188 andanyforresulton line 191 loses TypeScript's type checking benefits. Consider using proper types or creating a union type if the exact type is complex.- ario: arioContract as any, // Cast to avoid type issues + ario: arioContract as AoARIOWrite,For line 191, consider typing
resultbased on the possible return types from the switch cases.src/components/data-display/tables/DomainsTable.tsx (3)
57-65: Consolidate duplicate lucide-react imports.The imports from
lucide-reactare split across two import statements (lines 57 and 58-65). Consolidate them for cleaner code.🔎 Suggested fix
-import { Activity, StoreIcon } from 'lucide-react'; -import { - AlertTriangle, - CircleCheck, - Copy, - DollarSign, - ExternalLink, - Star, -} from 'lucide-react'; +import { + Activity, + AlertTriangle, + CircleCheck, + Copy, + DollarSign, + ExternalLink, + Star, + StoreIcon, +} from 'lucide-react';
288-371: Consider extracting modal state management to reduce duplication.The
RoleDisplaycomponent andInterruptedWorkflowActionboth manageshowContinueWorkflowModalstate with similar patterns. Consider creating a custom hook or using a render prop pattern to reduce duplication.This is a minor improvement for maintainability when the pattern is used more broadly.
869-938: IIFE pattern works but could be cleaner.The immediately-invoked function expression (IIFE) pattern on lines 869-938 works for the conditional rendering logic, but extracting this to a named helper component (similar to
MarketplaceActionIcon) would improve readability and testability.This is optional since the current implementation is functional and the component is already handling multiple concerns.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (8)
package.jsonsrc/components/data-display/VerticalTimelineStepper.tsxsrc/components/data-display/tables/DomainsTable.tsxsrc/components/modals/ContinueWorkflowModal/ContinueWorkflowModal.tsxsrc/components/modals/ListNameForSaleModal/ListNameForSaleModal.tsxsrc/components/modals/ListNameForSaleModal/panels/ConfirmListingPanel.tsxsrc/components/modals/ListNameForSaleModal/panels/ProcessTransactionPanel.tsxsrc/components/pages/Manage/Manage.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- src/components/modals/ListNameForSaleModal/panels/ConfirmListingPanel.tsx
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2025-12-02T17:38:52.889Z
Learnt from: vilenarios
Repo: ar-io/arns-react PR: 899
File: src/services/turbo/BaseTokenPurchaseService.ts:0-0
Timestamp: 2025-12-02T17:38:52.889Z
Learning: In src/services/turbo/BaseTokenPurchaseService.ts, the function executeBaseTokenPurchase intentionally only handles Base token top-up to Turbo credits, not the actual ArNS purchase. The ArNS purchase is handled separately by dispatchArIOInteraction in Checkout.tsx because it manages multiple complex purchase workflows (lease, permabuy, extend, increase undernames, etc.) with their own state management. This separation of concerns is by design.
Applied to files:
src/components/pages/Manage/Manage.tsxsrc/components/modals/ContinueWorkflowModal/ContinueWorkflowModal.tsx
📚 Learning: 2025-09-10T16:35:55.727Z
Learnt from: dtfiedler
Repo: ar-io/arns-react PR: 873
File: src/components/modals/ant-management/UpgradeDomainsModal/UpgradeDomainsModal.tsx:0-0
Timestamp: 2025-09-10T16:35:55.727Z
Learning: In arns-react, when calling buildDomainInfoQuery (src/hooks/useDomainInfo.tsx), you must pass arioProcessId along with arioContract, domain, aoNetwork, hyperbeamUrl, and antRegistryProcessId; omitting arioProcessId can lead to "No ANT id or record found" and break upgrades.
Applied to files:
src/components/pages/Manage/Manage.tsxsrc/components/data-display/tables/DomainsTable.tsxsrc/components/modals/ListNameForSaleModal/ListNameForSaleModal.tsx
📚 Learning: 2025-09-10T16:35:55.727Z
Learnt from: dtfiedler
Repo: ar-io/arns-react PR: 873
File: src/components/modals/ant-management/UpgradeDomainsModal/UpgradeDomainsModal.tsx:0-0
Timestamp: 2025-09-10T16:35:55.727Z
Learning: In arns-react, when calling buildDomainInfoQuery (src/hooks/useDomainInfo.tsx), you must pass arioProcessId along with arioContract, domain, aoNetwork, hyperbeamUrl, and antRegistryProcessId; omitting arioProcessId can lead to “No ANT id or record found” and break upgrades.
Applied to files:
src/components/pages/Manage/Manage.tsxsrc/components/data-display/tables/DomainsTable.tsxsrc/components/modals/ListNameForSaleModal/ListNameForSaleModal.tsx
📚 Learning: 2025-12-11T05:20:18.825Z
Learnt from: vilenarios
Repo: ar-io/arns-react PR: 902
File: src/components/modals/turbo/panels/crypto/CryptoConfirmation.tsx:536-540
Timestamp: 2025-12-11T05:20:18.825Z
Learning: Guideline: For tokens on Base network that use 6 decimals (e.g., base-ario ERC-20), convert amount to smallest units using a 1e6 multiplier. Use Math.round(amount * 1e6) when computing the on-chain value. Do not assume 18 decimals. Apply this conversion consistently in code paths that perform payments in CryptoConfirmation.tsx or similar components, and document that the base-ario/USDC-like tokens use 6 decimals to avoid mispricing or transfer errors.
Applied to files:
src/components/pages/Manage/Manage.tsxsrc/components/data-display/VerticalTimelineStepper.tsxsrc/components/modals/ListNameForSaleModal/panels/ProcessTransactionPanel.tsxsrc/components/data-display/tables/DomainsTable.tsxsrc/components/modals/ContinueWorkflowModal/ContinueWorkflowModal.tsxsrc/components/modals/ListNameForSaleModal/ListNameForSaleModal.tsx
🧬 Code graph analysis (3)
src/components/pages/Manage/Manage.tsx (1)
src/state/contexts/GlobalState.tsx (1)
useGlobalState(178-179)
src/components/modals/ListNameForSaleModal/panels/ProcessTransactionPanel.tsx (1)
src/components/data-display/VerticalTimelineStepper.tsx (1)
VerticalTimelineStepper(3-41)
src/components/modals/ListNameForSaleModal/ListNameForSaleModal.tsx (10)
src/state/contexts/GlobalState.tsx (1)
useGlobalState(178-179)src/state/contexts/WalletState.tsx (1)
useWalletState(49-50)src/hooks/useArIOPrice.ts (1)
useArIoPrice(9-32)src/hooks/useMarketplaceInfo.ts (1)
useMarketplaceInfo(4-25)src/hooks/useMarketplaceUserAssets.ts (1)
useMarketplaceUserAssets(44-61)src/utils/network.ts (1)
queryClient(39-45)src/hooks/useANTIntent.ts (1)
useANTIntent(17-75)src/components/layout/index.ts (1)
Loader(12-12)src/utils/searchUtils/searchUtils.ts (1)
sleep(128-130)src/services/arweave/ArweaveTransactionID.ts (1)
ArweaveTransactionID(7-31)
🔇 Additional comments (12)
src/components/pages/Manage/Manage.tsx (3)
103-122: Marketplace integration in refresh flow looks good.The addition of
marketplaceProcessIdto thedispatchArNSUpdatecall is consistent with the marketplace functionality being added. The parameter follows the same pattern as other process IDs already in use.
59-60: Minor styling refactor - no functional changes.The change from styled wrapper to class-based styling simplifies the markup without affecting behavior.
21-29: Verify ifdispatchArNSUpdatehandles optional or undefinedmarketplaceProcessId.The newly added
marketplaceProcessIdis destructured from global state but its optionality in thedispatchArNSUpdatefunction signature requires confirmation. Verify the function signature to ensure it either marksmarketplaceProcessIdas optional (with?) or provides a default value, and confirm thatundefinedvalues are handled gracefully rather than causing runtime errors.src/components/modals/ListNameForSaleModal/ListNameForSaleModal.tsx (3)
1-48: LGTM on imports and type definitions.The imports are well-organized and the type definitions for
ListingTypeandPanelStatesare clear and appropriately exported for use by other components.
114-150: Fee calculation logic looks correct.The fee calculation using SDK functions
calculateListingFeeandcalculateSaleTaxwith proper mARIO/ARIO conversions is well implemented. The separation of listing fee (paid upfront) from sale tax (deducted from proceeds) is correctly handled.
179-280: Well-structured workflow step management.The
updateWorkflowStepscallback provides a clean abstraction for updating step states with appropriate icons. The switch statements are comprehensive and handle all defined states.src/components/modals/ListNameForSaleModal/panels/ProcessTransactionPanel.tsx (1)
1-70: LGTM!Clean presentational component with clear state-based rendering. The exported
ProcessTransactionPanelPropstype enables proper typing for consumers, and the conditional UI logic for complete/error states is straightforward.src/components/modals/ContinueWorkflowModal/ContinueWorkflowModal.tsx (1)
51-68: Well-structured state management from global context.Good use of destructuring to extract only the needed values from global and wallet state hooks.
src/components/data-display/tables/DomainsTable.tsx (4)
126-204: LGTM on ErrorStateTooltip component.Well-structured error handling component with clipboard copy functionality and Discord reporting. The error config object captures relevant debugging information for support.
438-452: Helper functions are well-designed.
isMarketplaceCompatibleandgetInterruptedWorkflowForDomainare clean helper functions that improve readability of the table rendering logic.
1081-1111: LGTM on modal integrations.The modal rendering for
ListNameForSaleModalandUpgradeDomainForMarketplaceModalis well-implemented with proper state management. TheonUpgradeCompletecallback pattern for chaining the upgrade-to-sale flow is a nice UX touch.
476-489: Role detection with marketplace ownership is well-implemented.The
getRoleWithMarketplacefunction correctly prioritizes marketplace ownership check before falling back to the standard ownership status logic.
| const retries = 0; | ||
| const maxRetries = 10; | ||
| while (retries < maxRetries) { | ||
| try { | ||
| const _res = await marketplaceContract.getUserAssets({ | ||
| address: walletAddress.toString(), | ||
| arioProcessId, | ||
| }); | ||
| } catch { | ||
| console.error('Issue retrieving marketplace assets'); | ||
| } | ||
| } |
There was a problem hiding this comment.
Critical: Infinite loop will freeze the application.
retries is declared as const and never incremented, causing the while loop to run forever. This will freeze the UI when the PUSH_INTENT workflow is executed.
🔎 Proposed fix
- const retries = 0;
+ let retries = 0;
const maxRetries = 10;
while (retries < maxRetries) {
try {
const _res = await marketplaceContract.getUserAssets({
address: walletAddress.toString(),
arioProcessId,
});
+ // Add success condition to break out of loop
+ break;
} catch {
console.error('Issue retrieving marketplace assets');
+ retries++;
+ await new Promise(resolve => setTimeout(resolve, 2000)); // Add delay between retries
}
}📝 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.
| const retries = 0; | |
| const maxRetries = 10; | |
| while (retries < maxRetries) { | |
| try { | |
| const _res = await marketplaceContract.getUserAssets({ | |
| address: walletAddress.toString(), | |
| arioProcessId, | |
| }); | |
| } catch { | |
| console.error('Issue retrieving marketplace assets'); | |
| } | |
| } | |
| let retries = 0; | |
| const maxRetries = 10; | |
| while (retries < maxRetries) { | |
| try { | |
| const _res = await marketplaceContract.getUserAssets({ | |
| address: walletAddress.toString(), | |
| arioProcessId, | |
| }); | |
| // Add success condition to break out of loop | |
| break; | |
| } catch { | |
| console.error('Issue retrieving marketplace assets'); | |
| retries++; | |
| await new Promise(resolve => setTimeout(resolve, 2000)); // Add delay between retries | |
| } | |
| } |
🤖 Prompt for AI Agents
In src/components/modals/ContinueWorkflowModal/ContinueWorkflowModal.tsx around
lines 221 to 232, the retry loop is broken because retries is declared as const
and never incremented, causing an infinite loop; change retries to a mutable
variable (let), increment it on each iteration, exit the loop when the call
succeeds (break) or when retries >= maxRetries, and implement a small
delay/backoff between attempts (e.g., setTimeout/await sleep) and surface or
handle the final failure after max retries to avoid freezing the UI.
| if ( | ||
| tries >= maxTries || | ||
| newARIOBalance <= feeDetails.listingFee | ||
| ) { | ||
| throw new Error('Failed to deposit enough ARIO'); | ||
| } |
There was a problem hiding this comment.
Logic error in deposit verification loop.
The condition newARIOBalance <= feeDetails.listingFee on line 382 will throw an error even when the balance equals the listing fee, which should be a valid state. Additionally, the error is thrown when tries >= maxTries regardless of whether the balance check passed.
🔎 Proposed fix
if (
- tries >= maxTries ||
- newARIOBalance <= feeDetails.listingFee
+ newARIOBalance < feeDetails.listingFee &&
+ tries >= maxTries
) {
throw new Error('Failed to deposit enough ARIO');
}
+ if (newARIOBalance >= feeDetails.listingFee) {
+ break; // Exit the loop on success
+ }🤖 Prompt for AI Agents
In src/components/modals/ListNameForSaleModal/ListNameForSaleModal.tsx around
lines 380 to 385, the current condition throws even when balance equals the
listing fee and also throws on max tries regardless of balance; change the logic
so the error is thrown only when both maxTries reached AND the balance is
strictly less than the required listingFee (use <, not <=), i.e. check tries >=
maxTries && newARIOBalance < feeDetails.listingFee, otherwise allow success when
balance >= listingFee or continue retrying until maxTries.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
src/components/modals/ListNameForSaleModal/ListNameForSaleModal.tsx (1)
385-390: Logic error in deposit verification loop.The condition throws an error even when the balance equals the listing fee (using
<=instead of<). Additionally, the error is thrown whentries >= maxTriesregardless of whether the balance check passed. The loop never explicitly breaks on success.🔎 Proposed fix (from previous review)
if ( - tries >= maxTries || - newARIOBalance <= feeDetails.listingFee + newARIOBalance < feeDetails.listingFee && + tries >= maxTries ) { throw new Error('Failed to deposit enough ARIO'); } + if (newARIOBalance >= feeDetails.listingFee) { + break; // Exit the loop on success + }
🧹 Nitpick comments (4)
src/components/modals/ListNameForSaleModal/ListNameForSaleModal.tsx (4)
316-318: Document or make configurable the 30-day expiration limit.The 30-day maximum expiration period is hardcoded without explanation. Consider documenting why this limit exists (marketplace policy, technical constraint, etc.) or making it configurable via marketplace settings.
💡 Suggested documentation
if (expirationDate.getTime() > Date.now() + 1000 * 60 * 60 * 24 * 30) { - throw new Error('Expiration date must be within next 30 days'); + throw new Error('Expiration date must be within next 30 days'); // Marketplace policy limit }Alternatively, retrieve this from marketplace info if available:
const maxExpirationDays = marketplaceInfo?.maxListingDays ?? 30;
494-508: Consider more precise query invalidation.The current predicates reset all queries containing 'marketplace', domainName, or antId. This might invalidate more queries than necessary. Consider using specific query keys instead.
💡 Suggested improvement
// we need to reset the query states for orders and ant state. - queryClient.resetQueries({ - predicate: (query) => - query.queryKey.some( - (key: unknown) => - typeof key === 'string' && key.startsWith('marketplace'), - ), - }); - queryClient.resetQueries({ - predicate: (query) => - query.queryKey.some( - (key: unknown) => - (typeof key === 'string' && key.includes(domainName)) || - (typeof key === 'string' && key.includes(antId ?? '')), - ), - }); + // Invalidate specific marketplace queries + queryClient.invalidateQueries({ + queryKey: ['marketplace-intents', marketplaceProcessId] + }); + queryClient.invalidateQueries({ + queryKey: ['marketplace-orders'] + }); + + // Invalidate domain and ANT info + if (antId) { + queryClient.invalidateQueries({ queryKey: ['ant', antId] }); + queryClient.invalidateQueries({ queryKey: ['domain', domainName] }); + }Using
invalidateQueriesinstead ofresetQueriesis also preferred, as it refetches rather than clearing the cache.
552-552: Add optional chaining for consistency.While the code is safe due to the enclosing conditional on line 525, using optional chaining here aligns with the null-safety pattern and makes the code more robust to future refactoring.
🔎 Proposed change
<ArweaveID - id={new ArweaveTransactionID(result.antTransferResult.id)} + id={new ArweaveTransactionID(result.antTransferResult?.id ?? '')} type={ArweaveIdTypes.INTERACTION} shouldLink characterCount={8} />Note: This assumes
ArweaveTransactionIDconstructor handles empty strings gracefully. Based on the relevant code snippet, it throws an error for invalid IDs, so you may need to conditionally render theArweaveIDcomponent instead:- <span> + {result.antTransferResult?.id && ( + <span> View transfer on aolink{' '} <ArweaveID - id={new ArweaveTransactionID(result.antTransferResult.id)} + id={new ArweaveTransactionID(result.antTransferResult.id)} type={ArweaveIdTypes.INTERACTION} shouldLink characterCount={8} /> - </span> + </span> + )}
577-577: Remove redundant query invalidation.This query invalidation on line 577 is redundant with the specific query resets performed on lines 494-508 within the success path. Consider removing this or consolidating the invalidation logic.
💡 Suggested change
- queryClient.invalidateQueries({ queryKey: ['marketplace'] }); } catch (error) { setWorkflowComplete(true); setWorkflowError(true); eventEmitter.emit('error', error); } finally { setIsLoading(false); }If you need to ensure queries are invalidated even on error, move this to the
finallyblock and remove the earlier invalidations:} catch (error) { setWorkflowComplete(true); setWorkflowError(true); eventEmitter.emit('error', error); } finally { setIsLoading(false); + // Invalidate queries whether success or error + queryClient.invalidateQueries({ queryKey: ['marketplace'] }); }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/components/modals/ListNameForSaleModal/ListNameForSaleModal.tsx
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-09-10T16:35:55.727Z
Learnt from: dtfiedler
Repo: ar-io/arns-react PR: 873
File: src/components/modals/ant-management/UpgradeDomainsModal/UpgradeDomainsModal.tsx:0-0
Timestamp: 2025-09-10T16:35:55.727Z
Learning: In arns-react, when calling buildDomainInfoQuery (src/hooks/useDomainInfo.tsx), you must pass arioProcessId along with arioContract, domain, aoNetwork, hyperbeamUrl, and antRegistryProcessId; omitting arioProcessId can lead to “No ANT id or record found” and break upgrades.
Applied to files:
src/components/modals/ListNameForSaleModal/ListNameForSaleModal.tsx
📚 Learning: 2025-09-10T16:35:55.727Z
Learnt from: dtfiedler
Repo: ar-io/arns-react PR: 873
File: src/components/modals/ant-management/UpgradeDomainsModal/UpgradeDomainsModal.tsx:0-0
Timestamp: 2025-09-10T16:35:55.727Z
Learning: In arns-react, when calling buildDomainInfoQuery (src/hooks/useDomainInfo.tsx), you must pass arioProcessId along with arioContract, domain, aoNetwork, hyperbeamUrl, and antRegistryProcessId; omitting arioProcessId can lead to "No ANT id or record found" and break upgrades.
Applied to files:
src/components/modals/ListNameForSaleModal/ListNameForSaleModal.tsx
📚 Learning: 2025-12-11T05:20:18.825Z
Learnt from: vilenarios
Repo: ar-io/arns-react PR: 902
File: src/components/modals/turbo/panels/crypto/CryptoConfirmation.tsx:536-540
Timestamp: 2025-12-11T05:20:18.825Z
Learning: Guideline: For tokens on Base network that use 6 decimals (e.g., base-ario ERC-20), convert amount to smallest units using a 1e6 multiplier. Use Math.round(amount * 1e6) when computing the on-chain value. Do not assume 18 decimals. Apply this conversion consistently in code paths that perform payments in CryptoConfirmation.tsx or similar components, and document that the base-ario/USDC-like tokens use 6 decimals to avoid mispricing or transfer errors.
Applied to files:
src/components/modals/ListNameForSaleModal/ListNameForSaleModal.tsx
🧬 Code graph analysis (1)
src/components/modals/ListNameForSaleModal/ListNameForSaleModal.tsx (6)
src/hooks/useArIOPrice.ts (1)
useArIoPrice(9-32)src/hooks/useMarketplaceInfo.ts (1)
useMarketplaceInfo(4-25)src/hooks/useMarketplaceUserAssets.ts (1)
useMarketplaceUserAssets(44-61)src/hooks/useANTIntent.ts (1)
useANTIntent(17-75)src/utils/searchUtils/searchUtils.ts (1)
sleep(128-130)src/services/arweave/ArweaveTransactionID.ts (1)
ArweaveTransactionID(7-31)
| deposit: { | ||
| title: 'Deposit Listing Fee', | ||
| description: 'Deposit the listing fee to the marketplace', | ||
| icon: <DollarSign className="w-4 h-4" />, | ||
| }, | ||
| createIntent: { | ||
| title: 'Create Intent', | ||
| description: 'Create an intent to list the name for sale', | ||
| icon: <DollarSign className="w-4 h-4" />, | ||
| }, | ||
| transferANT: { | ||
| title: 'Transfer ANT', | ||
| description: 'Transfer the ANT to the marketplace', | ||
| icon: <DollarSign className="w-4 h-4" />, | ||
| }, |
There was a problem hiding this comment.
Icon inconsistency in default workflow steps.
All workflow steps initially use DollarSign icon, but createIntent (line 76) and transferANT (line 81) should use their respective icons (Handshake and Send) from the start. While updateWorkflowSteps corrects these icons when the workflow runs, any initial rendering before updates will display incorrect icons.
🔎 Proposed fix
createIntent: {
title: 'Create Intent',
description: 'Create an intent to list the name for sale',
- icon: <DollarSign className="w-4 h-4" />,
+ icon: <Handshake className="w-4 h-4" />,
},
transferANT: {
title: 'Transfer ANT',
description: 'Transfer the ANT to the marketplace',
- icon: <DollarSign className="w-4 h-4" />,
+ icon: <Send className="w-4 h-4" />,
},🤖 Prompt for AI Agents
In src/components/modals/ListNameForSaleModal/ListNameForSaleModal.tsx around
lines 68 to 82, the default workflow steps set all icons to DollarSign which
causes createIntent and transferANT to render incorrect icons initially; update
the createIntent icon to use the Handshake component and the transferANT icon to
use the Send component in the initial steps object, and ensure Handshake and
Send are imported at the top of the file so the correct icons display before
updateWorkflowSteps runs.
| const difference = Math.max( | ||
| 1, | ||
| Math.round(feeDetails.listingFee - existingBalance), | ||
| ).toString(); |
There was a problem hiding this comment.
Use Math.ceil instead of Math.round for deposit amount.
Math.round() can round down, potentially depositing less than required. For example, if the difference is 1.4 ARIO, Math.round(1.4) returns 1, which is insufficient. Use Math.ceil() to always round up.
🔎 Proposed fix
const difference = Math.max(
1,
- Math.round(feeDetails.listingFee - existingBalance),
+ Math.ceil(feeDetails.listingFee - existingBalance),
).toString();📝 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.
| const difference = Math.max( | |
| 1, | |
| Math.round(feeDetails.listingFee - existingBalance), | |
| ).toString(); | |
| const difference = Math.max( | |
| 1, | |
| Math.ceil(feeDetails.listingFee - existingBalance), | |
| ).toString(); |
🤖 Prompt for AI Agents
In src/components/modals/ListNameForSaleModal/ListNameForSaleModal.tsx around
lines 351 to 354, the deposit difference uses Math.round which can under-deposit
(e.g., 1.4 → 1); change the calculation to use Math.ceil so the difference is
always rounded up, and keep the Math.max(1, ...) guard and .toString()
conversion unchanged.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/data-display/tables/DomainsTable.tsx (1)
519-525: Missing dependencies inuseEffectmay cause stale table data.The effect uses
walletAddress(lines 463, 472, 487),latestAntVersion(line 473), andmarketplaceProcessId(line 480), but these are not included in the dependency array. This can cause:
- Table not updating when user connects/disconnects wallet
- Stale role calculations when marketplace process ID changes
- Incorrect compatibility status with stale ANT version
🔎 Proposed fix
}, [ domainData, loading, loadingArnsState, primaryNameData, dispatchArNSState, + walletAddress, + latestAntVersion, + marketplaceProcessId, ]);
🧹 Nitpick comments (1)
src/components/data-display/tables/DomainsTable.tsx (1)
289-372: Consider extracting duplicated modal logic.The
ContinueWorkflowModalrendering pattern is duplicated across multiple conditional branches (lines 321-329, 357-365). This could be extracted into a shared pattern.🔎 Suggested refactor
function RoleDisplay({ role, domainName, processId, }: { role: string; domainName: string; processId: string; }) { const { hasIntent, intent } = useANTIntent(processId); const [showContinueWorkflowModal, setShowContinueWorkflowModal] = useState(false); + const renderModal = () => + showContinueWorkflowModal && intent && ( + <ContinueWorkflowModal + show={showContinueWorkflowModal} + onClose={() => setShowContinueWorkflowModal(false)} + domainName={domainName} + antId={processId} + intentId={intent.intentId} + /> + ); + if (role === 'marketplace') { if (hasIntent && intent) { return ( <> <div className="flex items-center gap-2"> <button onClick={() => setShowContinueWorkflowModal(true)} className="relative flex items-center gap-2 hover:opacity-80 transition-opacity" > {/* ... */} </button> </div> - {showContinueWorkflowModal && ( - <ContinueWorkflowModal - show={showContinueWorkflowModal} - onClose={() => setShowContinueWorkflowModal(false)} - domainName={domainName} - antId={processId} - intentId={intent.intentId} - /> - )} + {renderModal()} </> ); } // ... apply similarly to other branches }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (4)
package.jsonsrc/components/data-display/tables/DomainsTable.tsxsrc/components/pages/Register/Checkout.tsxsrc/utils/constants.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- package.json
- src/components/pages/Register/Checkout.tsx
- src/utils/constants.ts
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-09-10T16:35:55.727Z
Learnt from: dtfiedler
Repo: ar-io/arns-react PR: 873
File: src/components/modals/ant-management/UpgradeDomainsModal/UpgradeDomainsModal.tsx:0-0
Timestamp: 2025-09-10T16:35:55.727Z
Learning: In arns-react, when calling buildDomainInfoQuery (src/hooks/useDomainInfo.tsx), you must pass arioProcessId along with arioContract, domain, aoNetwork, hyperbeamUrl, and antRegistryProcessId; omitting arioProcessId can lead to "No ANT id or record found" and break upgrades.
Applied to files:
src/components/data-display/tables/DomainsTable.tsx
📚 Learning: 2025-09-10T16:35:55.727Z
Learnt from: dtfiedler
Repo: ar-io/arns-react PR: 873
File: src/components/modals/ant-management/UpgradeDomainsModal/UpgradeDomainsModal.tsx:0-0
Timestamp: 2025-09-10T16:35:55.727Z
Learning: In arns-react, when calling buildDomainInfoQuery (src/hooks/useDomainInfo.tsx), you must pass arioProcessId along with arioContract, domain, aoNetwork, hyperbeamUrl, and antRegistryProcessId; omitting arioProcessId can lead to “No ANT id or record found” and break upgrades.
Applied to files:
src/components/data-display/tables/DomainsTable.tsx
📚 Learning: 2025-12-11T05:20:18.825Z
Learnt from: vilenarios
Repo: ar-io/arns-react PR: 902
File: src/components/modals/turbo/panels/crypto/CryptoConfirmation.tsx:536-540
Timestamp: 2025-12-11T05:20:18.825Z
Learning: Guideline: For tokens on Base network that use 6 decimals (e.g., base-ario ERC-20), convert amount to smallest units using a 1e6 multiplier. Use Math.round(amount * 1e6) when computing the on-chain value. Do not assume 18 decimals. Apply this conversion consistently in code paths that perform payments in CryptoConfirmation.tsx or similar components, and document that the base-ario/USDC-like tokens use 6 decimals to avoid mispricing or transfer errors.
Applied to files:
src/components/data-display/tables/DomainsTable.tsx
🧬 Code graph analysis (1)
src/components/data-display/tables/DomainsTable.tsx (6)
src/utils/constants.ts (1)
ARIO_DISCORD_LINK(251-251)src/hooks/useANTIntent.ts (1)
useANTIntent(17-75)src/hooks/useMarketplaceOrder.tsx (1)
useMarketplaceOrder(44-56)src/hooks/useInterruptedWorkflows.ts (2)
InterruptedWorkflow(6-10)useInterruptedWorkflows(15-67)src/utils/common/common.tsx (1)
formatForMaxCharCount(134-148)src/components/modals/index.ts (2)
ListNameForSaleModal(15-15)UpgradeDomainForMarketplaceModal(16-16)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: deploy-preview
- GitHub Check: lint_and_build
🔇 Additional comments (6)
src/components/data-display/tables/DomainsTable.tsx (6)
7-66: LGTM!The new imports are well-organized and support the marketplace integration. The separation between component imports, hooks, and icons is clear.
127-205: LGTM!Error state tooltip is well-implemented with proper event handling, clipboard API error handling, and secure external link opening.
406-453: LGTM!The marketplace-related state management and helper functions follow existing patterns and are cleanly implemented.
607-632: LGTM!The enhanced name column rendering with tooltip and external link is well-structured.
870-939: LGTM!The action column logic correctly handles the different marketplace states:
- Marketplace-owned domains without interrupted workflows → marketplace icon
- Domains with interrupted workflows → continue workflow action
- Other domains → list for sale/upgrade flow
The conditional flow is clear and the upgrade-to-marketplace path correctly chains to the listing modal.
1082-1113: LGTM!The modal integrations correctly handle the upgrade-to-marketplace flow, chaining the sale modal after successful upgrade.
There was a problem hiding this comment.
Actionable comments posted: 9
🤖 Fix all issues with AI agents
In @src/components/modals/ListNameForSaleModal/ListNameForSaleModal.tsx:
- Around line 107-109: The comment next to the useState initializing
expirationDate is inaccurate: update the comment by replacing "Default to 1 hour
from now (current day)" with a correct description such as "Default to 1 hour
from now" (or similar) to reflect that the initial value is one hour ahead of
the current time; locate the useState call for expirationDate /
setExpirationDate inside the ListNameForSaleModal component and change only the
comment text.
- Line 17: The import for Tabs is referencing a non-existent package; update the
import statement that currently imports Tabs (the Tabs symbol) from "radix-ui"
to "@radix-ui/react-tabs" and add "@radix-ui/react-tabs" to package.json
dependencies (then run your package manager to install) so the Tabs component is
resolved at runtime.
- Around line 364-401: The loop currently throws when newARIOBalance <=
feeDetails.listingFee which is inverted; change the logic so you only throw if
you've exhausted retries and the balance is still insufficient: inside the while
loop, after computing newARIOBalance from
writeMarketplaceContract.getMarketplaceBalance and mARIOToken, if newARIOBalance
>= feeDetails.listingFee then break out of the loop (balance satisfied);
otherwise if tries >= maxTries && newARIOBalance < feeDetails.listingFee then
throw new Error('Failed to deposit enough ARIO'); keep the existing retry/sleep
behavior using tries, maxTries and sleep.
In @src/components/pages/ViewListing/ViewListing.tsx:
- Around line 839-844: The conditional rendering for the status message
currently only uses workflowComplete, workflowError and name and always shows
"There was an error completing your purchase." — update that JSX to make the
error text contextual to the workflow type by branching on the workflowType (or
workflowAction) value: when workflowError is true, render a message like "There
was an error cancelling your listing." if workflowType === 'cancel', otherwise
keep "There was an error completing your purchase."; keep the success message
(`${name} is now yours!`) for purchase completion and keep the waiting message
unchanged; update the conditional expression near
workflowComplete/workflowError/name accordingly.
- Around line 280-299: The polling loop that checks deposit confirmation
(variables newBalance, tries, maxTries, sleep() and the call
marketplaceContract.getMarketplaceBalance comparing to orderPrice) uses
hardcoded 5s intervals and 10 attempts; make these polling parameters
configurable by extracting interval and maxAttempts into configurable
constants/props/state (e.g., depositPollIntervalMs, depositMaxAttempts) or read
from a config, and replace the fixed sleep(5000) and maxTries usage with those
variables; alternatively implement exponential backoff by increasing the delay
each iteration (e.g., delay = baseInterval * 2**tries capped at a maxDelay) and
ensure tries logic still stops after depositMaxAttempts, so the polling becomes
resilient on slow networks.
- Around line 435-438: Replace the blind await sleep(10_000) after calling
marketplaceContract.cancelOrder(orderData.id) with a polling loop (reuse the
same approach as handleBuy) that repeatedly queries the marketplace for the
order status (e.g., call the same method used elsewhere to fetch order details
or marketplaceContract.getOrder/orderById) until the order shows as
cancelled/removed or a timeout/attempt limit is reached; then proceed to refresh
UI state. Ensure you surface errors or timeout as before and stop polling once
confirmation is received to avoid unnecessary waits.
- Around line 676-680: The code uses an unsafe type assertion
"(domainInfo.arnsRecord as any)?.endTimestamp" when setting endTimestamp;
replace this with a type-safe check: create a type guard function like
hasEndTimestamp(record: unknown): record is { endTimestamp: number } that
returns true when record?.endTimestamp is a number, then change the endTimestamp
expression to use domainInfo?.arnsRecord?.type === 'lease' &&
hasEndTimestamp(domainInfo.arnsRecord) ? domainInfo.arnsRecord.endTimestamp :
undefined so you avoid "as any" and keep proper typing for endTimestamp,
domainInfo and arnsRecord.
In @src/utils/common/common.tsx:
- Around line 171-208: Update the catch block that currently swallows errors in
the expiry formatting block (the try around
formatVerboseDate/formatDate/Tooltip) to capture the error (e.g., catch (err))
and log it with context before returning the fallback string; include the error
object and relevant variables like endTimestamp in the log (use the project
logger if available or console.error) so issues in formatVerboseDate/formatDate
can be diagnosed.
🧹 Nitpick comments (10)
src/components/pages/ViewListing/ViewListing.tsx (2)
456-476: Standardize query invalidation pattern.The query invalidation approach differs from
handleBuy(line 366). Here you useresetQuerieswith a predicate checking for string inclusion of 'marketplace', whilehandleBuyusesinvalidateQueries({ queryKey: ['marketplace'] }). This inconsistency could lead to different cache behaviors.♻️ Align with handleBuy pattern
queryClient.resetQueries({ predicate: (query) => query.queryKey.some( (key: unknown) => typeof key === 'string' && key.includes(name), ), }); - queryClient.resetQueries({ - predicate: (query) => - query.queryKey.some( - (key: unknown) => - typeof key === 'string' && key.includes('marketplace'), - ), - }); + queryClient.invalidateQueries({ queryKey: ['marketplace'] });
714-723: Extract hardcoded URL to configuration.The aolink base URL is hardcoded. This should be extracted to a constant or configuration to enable environment-specific URLs and easier maintenance.
♻️ Suggested refactor
At the top of the file or in a config file:
const AOLINK_BASE_URL = 'https://aolink.ar-io.dev';Then update the URL:
<a - href={`https://aolink.ar-io.dev/#/entity/${orderData.creator}`} + href={`${AOLINK_BASE_URL}/#/entity/${orderData.creator}`} target="_blank" rel="noopener noreferrer" className="text-blue-400 hover:text-blue-300 transition-colors flex items-center gap-1 justify-end" >src/components/modals/ListNameForSaleModal/ListNameForSaleModal.tsx (8)
60-88: Inconsistent icons in default workflow steps.All workflow steps use
DollarSignicon in the default configuration, but they represent different actions. WhileupdateWorkflowStepscorrects this dynamically, the initial state should be semantically consistent.♻️ Proposed fix to use appropriate icons per step
+const IntentIcon = Handshake; +const TransferIcon = Send; + const defaultWorkflowSteps: Record< string, { title: ReactNode; description: ReactNode; icon: ReactNode; } > = { deposit: { title: 'Deposit Listing Fee', description: 'Deposit the listing fee to the marketplace', icon: <DollarSign className="w-4 h-4" />, }, createIntent: { title: 'Create Intent', description: 'Create an intent to list the name for sale', - icon: <DollarSign className="w-4 h-4" />, + icon: <IntentIcon className="w-4 h-4" />, }, transferANT: { title: 'Transfer ANT', description: 'Transfer the ANT to the marketplace', - icon: <DollarSign className="w-4 h-4" />, + icon: <TransferIcon className="w-4 h-4" />, }, complete: { title: 'Complete', description: 'Name has been listed for sale', icon: <CheckIcon className="w-4 h-4" />, }, };
494-508: Overly broad query invalidation predicates.The query invalidation logic includes
antId ?? ''which will match queries containing an empty string. This could unintentionally invalidate unrelated queries.♻️ Proposed fix for more precise query matching
// we need to reset the query states for orders and ant state. queryClient.resetQueries({ predicate: (query) => query.queryKey.some( (key: unknown) => typeof key === 'string' && key.startsWith('marketplace'), ), }); queryClient.resetQueries({ predicate: (query) => query.queryKey.some( (key: unknown) => - (typeof key === 'string' && key.includes(domainName)) || - (typeof key === 'string' && key.includes(antId ?? '')), + typeof key === 'string' && + (key.includes(domainName) || (antId && key.includes(antId))), ), });
345-354: Potential precision loss in deposit calculation.Using
Math.round()on the deposit difference could result in depositing slightly less than needed. While the retry logic may catch this, usingMath.ceil()would be safer to ensure sufficient funds are deposited.♻️ Proposed fix
// if we should deposit the listing fee is greater than the existing balance, we need to deposit the difference. 1 ARIO is the minimum deposit const difference = Math.max( 1, - Math.round(feeDetails.listingFee - existingBalance), + Math.ceil(feeDetails.listingFee - existingBalance), ).toString();
379-384: Remove debug logging from production code.Console logging should be removed or guarded with environment checks to avoid polluting production logs.
🧹 Proposed fix
- console.log({ - newARIOBalance, - listingFee: feeDetails.listingFee, - tries, - maxTries, - }); + if (process.env.NODE_ENV === 'development') { + console.log({ + newARIOBalance, + listingFee: feeDetails.listingFee, + tries, + maxTries, + }); + }
447-448: Remove debug logging from production code.onProgress: (event: ListNameForSaleProgressEvent) => { - console.log('event', event); switch (event.step) {
36-39: Remove unused import.
buildMarketplaceUserAssetsQueryis imported but never used in this file.import { - buildMarketplaceUserAssetsQuery, useMarketplaceUserAssets, } from '@src/hooks/useMarketplaceUserAssets';
316-318: Extract the 30-day listing duration limit to a named constant.The 30-day expiration limit is hardcoded at line 316. Extract this to a constant for consistency with
MAX_LISTING_PRICEalready defined inFixedPricePanel.tsxand the existingMAX_LEASE_DURATIONpattern in the codebase.♻️ Suggested extraction
+const MAX_LISTING_DURATION_DAYS = 30; +const MAX_LISTING_DURATION_MS = MAX_LISTING_DURATION_DAYS * 24 * 60 * 60 * 1000; + async function handleConfirmListing() { try { // ... validation code ... - if (expirationDate.getTime() > Date.now() + 1000 * 60 * 60 * 24 * 30) { + if (expirationDate.getTime() > Date.now() + MAX_LISTING_DURATION_MS) { throw new Error('Expiration date must be within next 30 days'); }
581-585: Use structured error object for consistency and better user experience.Line 584 emits a raw error object, but 13 lines earlier (571–574) the same catch handler uses a structured object with user-friendly
nameandmessagefields. Raw error objects show generic titles like "Error" instead of descriptive names. Restructure to match the pattern:eventEmitter.emit('error', { name: 'List Name Error', // or appropriate error type message: error instanceof Error ? error.message : 'Failed to list name', });
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/components/modals/ListNameForSaleModal/ListNameForSaleModal.tsxsrc/components/pages/ViewListing/ViewListing.tsxsrc/utils/common/common.tsx
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2025-09-10T16:35:55.727Z
Learnt from: dtfiedler
Repo: ar-io/arns-react PR: 873
File: src/components/modals/ant-management/UpgradeDomainsModal/UpgradeDomainsModal.tsx:0-0
Timestamp: 2025-09-10T16:35:55.727Z
Learning: In arns-react, when calling buildDomainInfoQuery (src/hooks/useDomainInfo.tsx), you must pass arioProcessId along with arioContract, domain, aoNetwork, hyperbeamUrl, and antRegistryProcessId; omitting arioProcessId can lead to “No ANT id or record found” and break upgrades.
Applied to files:
src/components/modals/ListNameForSaleModal/ListNameForSaleModal.tsxsrc/components/pages/ViewListing/ViewListing.tsx
📚 Learning: 2025-09-10T16:35:55.727Z
Learnt from: dtfiedler
Repo: ar-io/arns-react PR: 873
File: src/components/modals/ant-management/UpgradeDomainsModal/UpgradeDomainsModal.tsx:0-0
Timestamp: 2025-09-10T16:35:55.727Z
Learning: In arns-react, when calling buildDomainInfoQuery (src/hooks/useDomainInfo.tsx), you must pass arioProcessId along with arioContract, domain, aoNetwork, hyperbeamUrl, and antRegistryProcessId; omitting arioProcessId can lead to "No ANT id or record found" and break upgrades.
Applied to files:
src/components/modals/ListNameForSaleModal/ListNameForSaleModal.tsxsrc/components/pages/ViewListing/ViewListing.tsx
📚 Learning: 2025-12-11T05:20:18.825Z
Learnt from: vilenarios
Repo: ar-io/arns-react PR: 902
File: src/components/modals/turbo/panels/crypto/CryptoConfirmation.tsx:536-540
Timestamp: 2025-12-11T05:20:18.825Z
Learning: Guideline: For tokens on Base network that use 6 decimals (e.g., base-ario ERC-20), convert amount to smallest units using a 1e6 multiplier. Use Math.round(amount * 1e6) when computing the on-chain value. Do not assume 18 decimals. Apply this conversion consistently in code paths that perform payments in CryptoConfirmation.tsx or similar components, and document that the base-ario/USDC-like tokens use 6 decimals to avoid mispricing or transfer errors.
Applied to files:
src/components/modals/ListNameForSaleModal/ListNameForSaleModal.tsxsrc/components/pages/ViewListing/ViewListing.tsx
📚 Learning: 2025-12-02T17:38:52.889Z
Learnt from: vilenarios
Repo: ar-io/arns-react PR: 899
File: src/services/turbo/BaseTokenPurchaseService.ts:0-0
Timestamp: 2025-12-02T17:38:52.889Z
Learning: In src/services/turbo/BaseTokenPurchaseService.ts, the function executeBaseTokenPurchase intentionally only handles Base token top-up to Turbo credits, not the actual ArNS purchase. The ArNS purchase is handled separately by dispatchArIOInteraction in Checkout.tsx because it manages multiple complex purchase workflows (lease, permabuy, extend, increase undernames, etc.) with their own state management. This separation of concerns is by design.
Applied to files:
src/components/pages/ViewListing/ViewListing.tsx
🧬 Code graph analysis (1)
src/utils/common/common.tsx (2)
src/utils/constants.ts (2)
PERMANENT_DOMAIN_MESSAGE(252-252)MILLISECONDS_IN_GRACE_PERIOD(209-209)src/components/data-display/index.tsx (1)
Tooltip(5-5)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: lint_and_build
🔇 Additional comments (6)
src/components/pages/ViewListing/ViewListing.tsx (6)
1-86: LGTM!Imports are well-organized and workflow step configurations are clearly structured for the buy and cancel flows.
88-150: LGTM!Component state initialization and data hooks are properly set up. The computed values for
isExpired,hasSufficientBalance, andisUserSellercorrectly handle edge cases with appropriate fallbacks.
152-213: LGTM!The
updateWorkflowStepscallback correctly manages workflow step state updates with appropriate visual feedback for different statuses.
215-393: Purchase flow is well-structured.The
handleBuyfunction properly validates all required data, handles the deposit-if-needed flow, executes the purchase, and manages state updates with appropriate error handling.
511-571: LGTM!Error and loading states are properly handled with clear user messaging and appropriate navigation options.
580-887: Main rendering logic is comprehensive.The UI properly handles all states (loading, expired, seller vs buyer views) and provides clear user feedback throughout the purchase/cancel workflows.
| TrendingDown, | ||
| XIcon, | ||
| } from 'lucide-react'; | ||
| import { Tabs } from 'radix-ui'; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Check package.json for radix dependencies
cat package.json | grep -A 20 '"dependencies"' | grep -i radix
# Also check devDependencies
cat package.json | grep -A 20 '"devDependencies"' | grep -i radixRepository: ar-io/arns-react
Length of output: 268
🏁 Script executed:
# Check other Radix imports in codebase to see the pattern
rg "from\s+['\"]@?radix" --type tsx --type ts -A 1 | head -50Repository: ar-io/arns-react
Length of output: 87
🏁 Script executed:
# Check if file exists and view the exact import
cat src/components/modals/ListNameForSaleModal/ListNameForSaleModal.tsx | head -25Repository: ar-io/arns-react
Length of output: 689
🏁 Script executed:
# Search for uses of Tabs in the codebase
rg "Tabs" src/components/modals/ListNameForSaleModal/ -A 3 -B 3Repository: ar-io/arns-react
Length of output: 7912
🏁 Script executed:
# Check if there's a local 'radix-ui' module/file
fd "radix-ui" --type f
fd "radix-ui" --type dRepository: ar-io/arns-react
Length of output: 42
🏁 Script executed:
# Check if there's any alias or re-export for 'radix-ui'
rg "radix-ui" --type tsx --type ts | head -20Repository: ar-io/arns-react
Length of output: 87
🏁 Script executed:
# Check tsconfig or any config that might have aliases
cat tsconfig.json | grep -A 10 "paths"Repository: ar-io/arns-react
Length of output: 187
Fix Radix UI Tabs import path and add missing dependency.
The import from 'radix-ui' references a non-existent package. This will cause a runtime error. Change to from '@radix-ui/react-tabs' and add @radix-ui/react-tabs to dependencies (not currently present, despite other @radix-ui packages being installed).
🤖 Prompt for AI Agents
In @src/components/modals/ListNameForSaleModal/ListNameForSaleModal.tsx at line
17, The import for Tabs is referencing a non-existent package; update the import
statement that currently imports Tabs (the Tabs symbol) from "radix-ui" to
"@radix-ui/react-tabs" and add "@radix-ui/react-tabs" to package.json
dependencies (then run your package manager to install) so the Tabs component is
resolved at runtime.
| const [expirationDate, setExpirationDate] = useState<Date | undefined>( | ||
| new Date(Date.now() + 60 * 60 * 1000), // Default to 1 hour from now (current day) | ||
| ); |
There was a problem hiding this comment.
Misleading comment.
The comment states "current day" but the code sets the default to 1 hour from now.
📝 Proposed fix
const [expirationDate, setExpirationDate] = useState<Date | undefined>(
- new Date(Date.now() + 60 * 60 * 1000), // Default to 1 hour from now (current day)
+ new Date(Date.now() + 60 * 60 * 1000), // Default to 1 hour from now
);📝 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.
| const [expirationDate, setExpirationDate] = useState<Date | undefined>( | |
| new Date(Date.now() + 60 * 60 * 1000), // Default to 1 hour from now (current day) | |
| ); | |
| const [expirationDate, setExpirationDate] = useState<Date | undefined>( | |
| new Date(Date.now() + 60 * 60 * 1000), // Default to 1 hour from now | |
| ); |
🤖 Prompt for AI Agents
In @src/components/modals/ListNameForSaleModal/ListNameForSaleModal.tsx around
lines 107 - 109, The comment next to the useState initializing expirationDate is
inaccurate: update the comment by replacing "Default to 1 hour from now (current
day)" with a correct description such as "Default to 1 hour from now" (or
similar) to reflect that the initial value is one hour ahead of the current
time; locate the useState call for expirationDate / setExpirationDate inside the
ListNameForSaleModal component and change only the comment text.
| let newARIOBalance = 0; | ||
| let tries = 0; | ||
| const maxTries = 10; | ||
| while (newARIOBalance < feeDetails.listingFee) { | ||
| try { | ||
| const userBalanceResult = | ||
| await writeMarketplaceContract.getMarketplaceBalance({ | ||
| address: walletAddress.toString(), | ||
| }); | ||
|
|
||
| newARIOBalance = new mARIOToken( | ||
| Number(userBalanceResult?.balance ?? 0), | ||
| ) | ||
| .toARIO() | ||
| .valueOf(); | ||
| console.log({ | ||
| newARIOBalance, | ||
| listingFee: feeDetails.listingFee, | ||
| tries, | ||
| maxTries, | ||
| }); | ||
| if ( | ||
| tries >= maxTries || | ||
| newARIOBalance <= feeDetails.listingFee | ||
| ) { | ||
| throw new Error('Failed to deposit enough ARIO'); | ||
| } | ||
| } catch (error) { | ||
| tries++; | ||
| console.error(error); | ||
| if (tries >= maxTries) { | ||
| throw error; | ||
| } | ||
| // allow for cranking time for message passing | ||
| await sleep(7000); | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
Critical logic error in deposit retry validation.
The condition on lines 385-390 throws an error when newARIOBalance <= feeDetails.listingFee, which is the opposite of what should happen. This will cause the function to fail even when the balance is insufficient and retries should continue.
🐛 Proposed fix
console.log({
newARIOBalance,
listingFee: feeDetails.listingFee,
tries,
maxTries,
});
- if (
- tries >= maxTries ||
- newARIOBalance <= feeDetails.listingFee
- ) {
+ if (tries >= maxTries) {
throw new Error('Failed to deposit enough ARIO');
}
+
+ // Break out of loop if we have sufficient balance
+ if (newARIOBalance >= feeDetails.listingFee) {
+ break;
+ }
} catch (error) {
tries++;
console.error(error);
if (tries >= maxTries) {
throw error;
}
// allow for cranking time for message passing
await sleep(7000);
}
}🤖 Prompt for AI Agents
In @src/components/modals/ListNameForSaleModal/ListNameForSaleModal.tsx around
lines 364 - 401, The loop currently throws when newARIOBalance <=
feeDetails.listingFee which is inverted; change the logic so you only throw if
you've exhausted retries and the balance is still insufficient: inside the while
loop, after computing newARIOBalance from
writeMarketplaceContract.getMarketplaceBalance and mARIOToken, if newARIOBalance
>= feeDetails.listingFee then break out of the loop (balance satisfied);
otherwise if tries >= maxTries && newARIOBalance < feeDetails.listingFee then
throw new Error('Failed to deposit enough ARIO'); keep the existing retry/sleep
behavior using tries, maxTries and sleep.
| let newBalance = 0; | ||
| let tries = 0; | ||
| const maxTries = 10; | ||
| while (newBalance < orderPrice && tries < maxTries) { | ||
| try { | ||
| const balanceResult = | ||
| await marketplaceContract.getMarketplaceBalance({ | ||
| address: walletAddress.toString(), | ||
| }); | ||
| newBalance = new mARIOToken(Number(balanceResult?.balance ?? 0)) | ||
| .toARIO() | ||
| .valueOf(); | ||
|
|
||
| if (newBalance >= orderPrice) break; | ||
| } catch (error) { | ||
| console.error('Error checking balance:', error); | ||
| } | ||
| tries++; | ||
| await sleep(5000); | ||
| } |
There was a problem hiding this comment.
Consider making polling parameters configurable.
The deposit confirmation polling uses hardcoded values (5-second intervals, 10 max tries = 50 seconds total). On slower networks or during high load, this might not be sufficient, leading to false "Failed to deposit enough ARIO" errors even when the deposit eventually succeeds.
Consider either:
- Increasing the max tries or timeout duration
- Making these values configurable
- Implementing exponential backoff for better resilience
♻️ Suggested improvement with longer timeout
// Wait for deposit to be confirmed
let newBalance = 0;
let tries = 0;
- const maxTries = 10;
+ const maxTries = 20; // Increased to 100 seconds total
while (newBalance < orderPrice && tries < maxTries) {🤖 Prompt for AI Agents
In @src/components/pages/ViewListing/ViewListing.tsx around lines 280 - 299, The
polling loop that checks deposit confirmation (variables newBalance, tries,
maxTries, sleep() and the call marketplaceContract.getMarketplaceBalance
comparing to orderPrice) uses hardcoded 5s intervals and 10 attempts; make these
polling parameters configurable by extracting interval and maxAttempts into
configurable constants/props/state (e.g., depositPollIntervalMs,
depositMaxAttempts) or read from a config, and replace the fixed sleep(5000) and
maxTries usage with those variables; alternatively implement exponential backoff
by increasing the delay each iteration (e.g., delay = baseInterval * 2**tries
capped at a maxDelay) and ensure tries logic still stops after
depositMaxAttempts, so the polling becomes resilient on slow networks.
| const result = await marketplaceContract.cancelOrder(orderData.id); | ||
|
|
||
| // Wait for the cancellation to be processed/cranked | ||
| await sleep(10_000); |
There was a problem hiding this comment.
Replace blind sleep with polling verification.
Unlike handleBuy which polls to confirm the deposit (lines 280-299), this function uses a fixed 10-second sleep without verifying that the cancellation was actually processed. If the cancellation takes longer, subsequent query refreshes might not reflect the change, leading to stale UI state.
♻️ Suggested fix with polling verification
const result = await marketplaceContract.cancelOrder(orderData.id);
- // Wait for the cancellation to be processed/cranked
- await sleep(10_000);
+ // Poll to verify the cancellation was processed
+ let cancelled = false;
+ let tries = 0;
+ const maxTries = 20;
+ while (!cancelled && tries < maxTries) {
+ try {
+ const order = await marketplaceContract.getOrder({
+ orderId: orderData.id,
+ });
+ if (!order) {
+ cancelled = true;
+ break;
+ }
+ } catch (error) {
+ // Order not found likely means it was cancelled
+ cancelled = true;
+ break;
+ }
+ tries++;
+ await sleep(5000);
+ }
+
+ if (!cancelled) {
+ throw new Error('Failed to verify cancellation');
+ }
updateWorkflowSteps({Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In @src/components/pages/ViewListing/ViewListing.tsx around lines 435 - 438,
Replace the blind await sleep(10_000) after calling
marketplaceContract.cancelOrder(orderData.id) with a polling loop (reuse the
same approach as handleBuy) that repeatedly queries the marketplace for the
order status (e.g., call the same method used elsewhere to fetch order details
or marketplaceContract.getOrder/orderById) until the order shows as
cancelled/removed or a timeout/attempt limit is reached; then proceed to refresh
UI state. Ensure you surface errors or timeout as before and stop polling once
confirmation is received to avoid unnecessary waits.
| endTimestamp={ | ||
| domainInfo?.arnsRecord?.type === 'lease' | ||
| ? (domainInfo.arnsRecord as any)?.endTimestamp | ||
| : undefined // Permabuy domains or no record - let component show "Permanent" | ||
| } |
There was a problem hiding this comment.
Avoid as any type assertion.
Using as any bypasses TypeScript's type checking and makes the code fragile to changes in the data structure. If the lease type structure changes, this could silently break.
♻️ Suggested type-safe approach
<LeaseDurationFromEndTimestamp
endTimestamp={
domainInfo?.arnsRecord?.type === 'lease'
- ? (domainInfo.arnsRecord as any)?.endTimestamp
+ ? 'endTimestamp' in domainInfo.arnsRecord
+ ? domainInfo.arnsRecord.endTimestamp
+ : undefined
: undefined // Permabuy domains or no record - let component show "Permanent"
}
/>Or better yet, if the SDK types are incorrect, add a proper type guard:
function hasEndTimestamp(record: any): record is { endTimestamp: number } {
return record && typeof record.endTimestamp === 'number';
}
// Then use:
endTimestamp={
domainInfo?.arnsRecord?.type === 'lease' && hasEndTimestamp(domainInfo.arnsRecord)
? domainInfo.arnsRecord.endTimestamp
: undefined
}📝 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.
| endTimestamp={ | |
| domainInfo?.arnsRecord?.type === 'lease' | |
| ? (domainInfo.arnsRecord as any)?.endTimestamp | |
| : undefined // Permabuy domains or no record - let component show "Permanent" | |
| } | |
| endTimestamp={ | |
| domainInfo?.arnsRecord?.type === 'lease' | |
| ? 'endTimestamp' in domainInfo.arnsRecord | |
| ? domainInfo.arnsRecord.endTimestamp | |
| : undefined | |
| : undefined // Permabuy domains or no record - let component show "Permanent" | |
| } |
🤖 Prompt for AI Agents
In @src/components/pages/ViewListing/ViewListing.tsx around lines 676 - 680, The
code uses an unsafe type assertion "(domainInfo.arnsRecord as
any)?.endTimestamp" when setting endTimestamp; replace this with a type-safe
check: create a type guard function like hasEndTimestamp(record: unknown):
record is { endTimestamp: number } that returns true when record?.endTimestamp
is a number, then change the endTimestamp expression to use
domainInfo?.arnsRecord?.type === 'lease' &&
hasEndTimestamp(domainInfo.arnsRecord) ? domainInfo.arnsRecord.endTimestamp :
undefined so you avoid "as any" and keep proper typing for endTimestamp,
domainInfo and arnsRecord.
| <p className="text-sm text-grey"> | ||
| {workflowComplete | ||
| ? workflowError | ||
| ? 'There was an error completing your purchase.' | ||
| : `${name} is now yours!` | ||
| : 'Please wait while we process your purchase. Do not close this page.'} |
There was a problem hiding this comment.
Make error message contextual.
The error message always says "There was an error completing your purchase" even when the user is cancelling a listing. This should be contextual based on the workflow type.
🔧 Proposed fix
<p className="text-sm text-grey">
{workflowComplete
? workflowError
- ? 'There was an error completing your purchase.'
- : `${name} is now yours!`
+ ? isUserSeller
+ ? 'There was an error cancelling the listing.'
+ : 'There was an error completing your purchase.'
+ : isUserSeller
+ ? 'The listing has been successfully cancelled.'
+ : `${name} is now yours!`
: 'Please wait while we process your purchase. Do not close this page.'}
</p>Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In @src/components/pages/ViewListing/ViewListing.tsx around lines 839 - 844, The
conditional rendering for the status message currently only uses
workflowComplete, workflowError and name and always shows "There was an error
completing your purchase." — update that JSX to make the error text contextual
to the workflow type by branching on the workflowType (or workflowAction) value:
when workflowError is true, render a message like "There was an error cancelling
your listing." if workflowType === 'cancel', otherwise keep "There was an error
completing your purchase."; keep the success message (`${name} is now yours!`)
for purchase completion and keep the waiting message unchanged; update the
conditional expression near workflowComplete/workflowError/name accordingly.
| try { | ||
| if (!endTimestamp) { | ||
| return PERMANENT_DOMAIN_MESSAGE; | ||
| } | ||
| const isGracePeriod = | ||
| Date.now() > endTimestamp && | ||
| Date.now() < endTimestamp + MILLISECONDS_IN_GRACE_PERIOD; | ||
| const isExpired = endTimestamp < Date.now(); | ||
|
|
||
| return ( | ||
| <Tooltip | ||
| message={ | ||
| isGracePeriod | ||
| ? 'Name is in Grace Period' | ||
| : isExpired | ||
| ? 'Name is Expired' | ||
| : 'Enters grace period on approximately ' + | ||
| formatVerboseDate(endTimestamp) | ||
| } | ||
| icon={ | ||
| <span | ||
| style={{ | ||
| color: | ||
| endTimestamp > Date.now() | ||
| ? 'var(--success-green)' | ||
| : isGracePeriod | ||
| ? 'var(--accent)' | ||
| : 'var(--error-red)', | ||
| }} | ||
| > | ||
| {formatDate(endTimestamp)} | ||
| </span> | ||
| } | ||
| /> | ||
| ); | ||
| } catch { | ||
| return 'Error formatting expiry date'; | ||
| } |
There was a problem hiding this comment.
Capture and log the error for better debugging.
The try/catch adds resilience, but silently swallowing errors makes debugging difficult. Consider capturing the error parameter and logging it to help diagnose issues in marketplace flows where data might be incomplete or malformed.
🔍 Recommended fix to capture and log errors
try {
if (!endTimestamp) {
return PERMANENT_DOMAIN_MESSAGE;
}
const isGracePeriod =
Date.now() > endTimestamp &&
Date.now() < endTimestamp + MILLISECONDS_IN_GRACE_PERIOD;
const isExpired = endTimestamp < Date.now();
return (
<Tooltip
message={
isGracePeriod
? 'Name is in Grace Period'
: isExpired
? 'Name is Expired'
: 'Enters grace period on approximately ' +
formatVerboseDate(endTimestamp)
}
icon={
<span
style={{
color:
endTimestamp > Date.now()
? 'var(--success-green)'
: isGracePeriod
? 'var(--accent)'
: 'var(--error-red)',
}}
>
{formatDate(endTimestamp)}
</span>
}
/>
);
- } catch {
+ } catch (error) {
+ console.error('Error formatting expiry date:', error, { endTimestamp });
return 'Error formatting expiry date';
}📝 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.
| try { | |
| if (!endTimestamp) { | |
| return PERMANENT_DOMAIN_MESSAGE; | |
| } | |
| const isGracePeriod = | |
| Date.now() > endTimestamp && | |
| Date.now() < endTimestamp + MILLISECONDS_IN_GRACE_PERIOD; | |
| const isExpired = endTimestamp < Date.now(); | |
| return ( | |
| <Tooltip | |
| message={ | |
| isGracePeriod | |
| ? 'Name is in Grace Period' | |
| : isExpired | |
| ? 'Name is Expired' | |
| : 'Enters grace period on approximately ' + | |
| formatVerboseDate(endTimestamp) | |
| } | |
| icon={ | |
| <span | |
| style={{ | |
| color: | |
| endTimestamp > Date.now() | |
| ? 'var(--success-green)' | |
| : isGracePeriod | |
| ? 'var(--accent)' | |
| : 'var(--error-red)', | |
| }} | |
| > | |
| {formatDate(endTimestamp)} | |
| </span> | |
| } | |
| /> | |
| ); | |
| } catch { | |
| return 'Error formatting expiry date'; | |
| } | |
| try { | |
| if (!endTimestamp) { | |
| return PERMANENT_DOMAIN_MESSAGE; | |
| } | |
| const isGracePeriod = | |
| Date.now() > endTimestamp && | |
| Date.now() < endTimestamp + MILLISECONDS_IN_GRACE_PERIOD; | |
| const isExpired = endTimestamp < Date.now(); | |
| return ( | |
| <Tooltip | |
| message={ | |
| isGracePeriod | |
| ? 'Name is in Grace Period' | |
| : isExpired | |
| ? 'Name is Expired' | |
| : 'Enters grace period on approximately ' + | |
| formatVerboseDate(endTimestamp) | |
| } | |
| icon={ | |
| <span | |
| style={{ | |
| color: | |
| endTimestamp > Date.now() | |
| ? 'var(--success-green)' | |
| : isGracePeriod | |
| ? 'var(--accent)' | |
| : 'var(--error-red)', | |
| }} | |
| > | |
| {formatDate(endTimestamp)} | |
| </span> | |
| } | |
| /> | |
| ); | |
| } catch (error) { | |
| console.error('Error formatting expiry date:', error, { endTimestamp }); | |
| return 'Error formatting expiry date'; | |
| } |
🤖 Prompt for AI Agents
In @src/utils/common/common.tsx around lines 171 - 208, Update the catch block
that currently swallows errors in the expiry formatting block (the try around
formatVerboseDate/formatDate/Tooltip) to capture the error (e.g., catch (err))
and log it with context before returning the fallback string; include the error
object and relevant variables like endTimestamp in the log (use the project
logger if available or console.error) so issues in formatVerboseDate/formatDate
can be diagnosed.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/data-display/tables/DomainsTable.tsx (1)
520-590: Add missingmarketplaceProcessIdto useEffect dependencies.The
useEffectat line 520 computes table data and usesmarketplaceProcessId(line 545) to determine marketplace ownership, butmarketplaceProcessIdis not included in the dependencies array (lines 584-590). This means changes tomarketplaceProcessIdwon't trigger a re-computation of table data, potentially displaying stale or incorrect role information.🐛 Proposed fix
useEffect(() => { // ... table data computation logic ... }, [ domainData, loading, loadingArnsState, primaryNameData, dispatchArNSState, + marketplaceProcessId, + walletAddress, + latestAntVersion, ]);
🤖 Fix all issues with AI agents
In @src/hooks/usePendingWorkflows.ts:
- Around line 30-42: The early-return in usePendingWorkflows incorrectly reports
isLoading as assetsLoading even when userAssets is null after the query; update
the return to reflect that missing data means still loading (or errored) by
setting isLoading to assetsLoading || userAssets == null (or use the hook's
isError if available), e.g. change the isLoading value in the early return to
assetsLoading || !userAssets so callers see a loading state when data is
unavailable; keep the rest of the returned shape (pendingWorkflows: [],
hasPendingWorkflows: false, getPendingWorkflowForDomain: () => undefined)
unchanged.
🧹 Nitpick comments (3)
src/hooks/usePendingWorkflows.ts (1)
44-91: Consider memoizing computed values to prevent unnecessary re-renders.The
pendingWorkflowsarray andgetPendingWorkflowForDomainfunction are recreated on every render. This can trigger unnecessary re-renders in components that consume this hook and depend on these values (e.g., viauseEffectdependencies or props comparison).Wrap the computation in
useMemoand the helper function inuseCallback:♻️ Suggested refactor
+import { useMemo, useCallback } from 'react'; + export function usePendingWorkflows( ants: Record<string, ANTProcessData>, domains: Record<string, any>, ): { pendingWorkflows: PendingWorkflow[]; isLoading: boolean; hasPendingWorkflows: boolean; getPendingWorkflowForDomain: ( domainName: string, antId: string, ) => PendingWorkflow | undefined; } { const { data: userAssets, isLoading: assetsLoading } = useMarketplaceUserAssets(); const [{ walletAddress }] = useWalletState(); const [{ marketplaceProcessId }] = useGlobalState(); if (!userAssets || !walletAddress || assetsLoading) { return { pendingWorkflows: [], isLoading: assetsLoading || !userAssets, hasPendingWorkflows: false, getPendingWorkflowForDomain: () => undefined, }; } - const pendingWorkflows: PendingWorkflow[] = []; - - // Check intents from user assets - const intents = userAssets.intents || []; - - // Check each domain for pending workflows - Object.entries(domains).forEach(([domainName, record]) => { - const antId = record.processId; - const ant = ants[antId]; - - if (!ant?.state?.Owner) return; - - // Skip if ANT is owned by marketplace (those are handled by interrupted workflows) - if (ant.state.Owner === marketplaceProcessId) return; - - // Check if there's a pending intent for this ANT with 'pending' status - const antIntent = intents.find( - (intent: MarketplaceIntent) => - intent.antProcessId === antId && intent.status === 'pending', - ); - - // If there's a pending intent, add to pending workflows - if (antIntent) { - pendingWorkflows.push({ - domainName, - antId, - intent: antIntent, - }); - } - }); - - const getPendingWorkflowForDomain = ( - domainName: string, - antId: string, - ): PendingWorkflow | undefined => { - return pendingWorkflows.find( - (workflow) => - workflow.domainName === domainName && workflow.antId === antId, - ); - }; - - return { - pendingWorkflows, - isLoading: false, - hasPendingWorkflows: pendingWorkflows.length > 0, - getPendingWorkflowForDomain, - }; + const pendingWorkflows = useMemo(() => { + const workflows: PendingWorkflow[] = []; + const intents = userAssets.intents || []; + + Object.entries(domains).forEach(([domainName, record]) => { + const antId = record.processId; + const ant = ants[antId]; + + if (!ant?.state?.Owner) return; + + // Skip if ANT is owned by marketplace (those are handled by interrupted workflows) + if (ant.state.Owner === marketplaceProcessId) return; + + // Check if there's a pending intent for this ANT with 'pending' status + const antIntent = intents.find( + (intent: MarketplaceIntent) => + intent.antProcessId === antId && intent.status === 'pending', + ); + + // If there's a pending intent, add to pending workflows + if (antIntent) { + workflows.push({ + domainName, + antId, + intent: antIntent, + }); + } + }); + + return workflows; + }, [ants, domains, userAssets, marketplaceProcessId]); + + const getPendingWorkflowForDomain = useCallback( + (domainName: string, antId: string): PendingWorkflow | undefined => { + return pendingWorkflows.find( + (workflow) => + workflow.domainName === domainName && workflow.antId === antId, + ); + }, + [pendingWorkflows], + ); + + return { + pendingWorkflows, + isLoading: false, + hasPendingWorkflows: pendingWorkflows.length > 0, + getPendingWorkflowForDomain, + }; }src/components/data-display/tables/DomainsTable.tsx (2)
504-518: Memoize helper functions to prevent unnecessary re-renders.The
isMarketplaceCompatibleandgetInterruptedWorkflowForDomainhelper functions are recreated on every render. While this doesn't cause functional issues, memoizing them withuseCallbackwould improve performance and prevent unnecessary re-renders in child components that might receive these as props.♻️ Suggested refactor
+import { useCallback } from 'react'; + // ... other code ... - // Helper function to check if ANT is marketplace compatible - const isMarketplaceCompatible = (antVersion: number): boolean => { - return antVersion >= minimumANTVersionForMarketplace; - }; - - // Helper function to check if a domain has an interrupted workflow - const getInterruptedWorkflowForDomain = ( - domainName: string, - antId: string, - ) => { - return interruptedWorkflows.find( - (workflow) => - workflow.domainName === domainName && workflow.antId === antId, - ); - }; + // Helper function to check if ANT is marketplace compatible + const isMarketplaceCompatible = useCallback( + (antVersion: number): boolean => { + return antVersion >= minimumANTVersionForMarketplace; + }, + [minimumANTVersionForMarketplace], + ); + + // Helper function to check if a domain has an interrupted workflow + const getInterruptedWorkflowForDomain = useCallback( + (domainName: string, antId: string) => { + return interruptedWorkflows.find( + (workflow) => + workflow.domainName === domainName && workflow.antId === antId, + ); + }, + [interruptedWorkflows], + );
935-1017: Consider extracting action column logic to a separate component.The action column rendering logic (lines 935-1017) handles multiple workflow states with nested conditions. While the logic is correct, extracting it to a dedicated
DomainActionCellcomponent would improve readability and make the code easier to test and maintain.♻️ Optional refactor suggestion
Create a new component:
function DomainActionCell({ domainName, processId, role, version, interruptedWorkflow, pendingWorkflow, isMarketplaceCompatible, minimumANTVersionForMarketplace, onListForSale, onUpgradeForMarketplace, }: DomainActionCellProps) { // If domain is owned by marketplace, show marketplace link or activity icon if (role === 'marketplace' && !interruptedWorkflow) { return ( <MarketplaceActionIcon domainName={domainName} processId={processId} /> ); } if (interruptedWorkflow) { return <InterruptedWorkflowAction interruptedWorkflow={interruptedWorkflow} />; } if (pendingWorkflow) { return <PendingWorkflowAction pendingWorkflow={pendingWorkflow} />; } // Show marketplace listing icon return ( <Tooltip message={ isMarketplaceCompatible ? 'List for Sale' : `Upgrade to version ${minimumANTVersionForMarketplace}+ to list for sale` } icon={ <button onClick={isMarketplaceCompatible ? onListForSale : onUpgradeForMarketplace}> <DollarSign className={`w-[18px] transition-colors ${ isMarketplaceCompatible ? 'text-grey hover:text-white' : 'text-warning hover:text-warning-light' }`} /> </button> } /> ); }Then use it in the table:
{(() => { const domainName = row.getValue('name') as string; const processId = row.getValue('processId') as string; const role = row.getValue('role') as string; const interruptedWorkflow = getInterruptedWorkflowForDomain(domainName, processId); const pendingWorkflow = getPendingWorkflowForDomain(domainName, processId); return ( <DomainActionCell domainName={domainName} processId={processId} role={role} version={row.original.version} interruptedWorkflow={interruptedWorkflow} pendingWorkflow={pendingWorkflow} isMarketplaceCompatible={isMarketplaceCompatible(row.original.version)} minimumANTVersionForMarketplace={minimumANTVersionForMarketplace} onListForSale={() => { setSelectedDomainForSale({ name: domainName, antId: processId }); setShowListForSaleModal(true); }} onUpgradeForMarketplace={() => { setDomainToUpgradeForMarketplace({ domain: lowerCaseDomain(domainName), processId: processId, }); setShowUpgradeForMarketplaceModal(true); }} /> ); })()}
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/components/data-display/tables/DomainsTable.tsxsrc/components/modals/ListNameForSaleModal/ListNameForSaleModal.tsxsrc/hooks/usePendingWorkflows.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/components/modals/ListNameForSaleModal/ListNameForSaleModal.tsx
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-09-10T16:35:55.727Z
Learnt from: dtfiedler
Repo: ar-io/arns-react PR: 873
File: src/components/modals/ant-management/UpgradeDomainsModal/UpgradeDomainsModal.tsx:0-0
Timestamp: 2025-09-10T16:35:55.727Z
Learning: In arns-react, when calling buildDomainInfoQuery (src/hooks/useDomainInfo.tsx), you must pass arioProcessId along with arioContract, domain, aoNetwork, hyperbeamUrl, and antRegistryProcessId; omitting arioProcessId can lead to "No ANT id or record found" and break upgrades.
Applied to files:
src/components/data-display/tables/DomainsTable.tsx
📚 Learning: 2025-09-10T16:35:55.727Z
Learnt from: dtfiedler
Repo: ar-io/arns-react PR: 873
File: src/components/modals/ant-management/UpgradeDomainsModal/UpgradeDomainsModal.tsx:0-0
Timestamp: 2025-09-10T16:35:55.727Z
Learning: In arns-react, when calling buildDomainInfoQuery (src/hooks/useDomainInfo.tsx), you must pass arioProcessId along with arioContract, domain, aoNetwork, hyperbeamUrl, and antRegistryProcessId; omitting arioProcessId can lead to “No ANT id or record found” and break upgrades.
Applied to files:
src/components/data-display/tables/DomainsTable.tsx
📚 Learning: 2025-12-11T05:20:18.825Z
Learnt from: vilenarios
Repo: ar-io/arns-react PR: 902
File: src/components/modals/turbo/panels/crypto/CryptoConfirmation.tsx:536-540
Timestamp: 2025-12-11T05:20:18.825Z
Learning: Guideline: For tokens on Base network that use 6 decimals (e.g., base-ario ERC-20), convert amount to smallest units using a 1e6 multiplier. Use Math.round(amount * 1e6) when computing the on-chain value. Do not assume 18 decimals. Apply this conversion consistently in code paths that perform payments in CryptoConfirmation.tsx or similar components, and document that the base-ario/USDC-like tokens use 6 decimals to avoid mispricing or transfer errors.
Applied to files:
src/components/data-display/tables/DomainsTable.tsx
🧬 Code graph analysis (2)
src/hooks/usePendingWorkflows.ts (4)
src/state/contexts/ArNSState.tsx (1)
ANTProcessData(16-21)src/hooks/useMarketplaceUserAssets.ts (1)
useMarketplaceUserAssets(44-61)src/state/contexts/WalletState.tsx (1)
useWalletState(49-50)src/state/contexts/GlobalState.tsx (1)
useGlobalState(178-179)
src/components/data-display/tables/DomainsTable.tsx (10)
src/utils/constants.ts (1)
ARIO_DISCORD_LINK(251-251)src/components/data-display/index.tsx (1)
Tooltip(5-5)src/hooks/useANTIntent.ts (1)
useANTIntent(17-75)src/hooks/useMarketplaceOrder.tsx (1)
useMarketplaceOrder(44-56)src/hooks/useInterruptedWorkflows.ts (2)
InterruptedWorkflow(6-10)useInterruptedWorkflows(15-67)src/hooks/usePendingWorkflows.ts (2)
PendingWorkflow(6-10)usePendingWorkflows(18-91)src/hooks/useANTVersions.tsx (1)
useLatestANTVersion(35-51)src/utils/common/common.tsx (1)
formatForMaxCharCount(134-148)src/components/icons/index.ts (1)
ExternalLinkIcon(110-110)src/components/modals/index.ts (2)
ListNameForSaleModal(15-15)UpgradeDomainForMarketplaceModal(16-16)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: lint_and_build
🔇 Additional comments (3)
src/components/data-display/tables/DomainsTable.tsx (3)
131-209: LGTM! Well-structured error handling component.The
ErrorStateTooltipcomponent provides a clear user experience for reporting error states when the marketplace owns an ANT but no order exists. The copy-to-clipboard functionality and Discord link integration are well-implemented with proper error handling and security attributes.
211-257: LGTM! Clear state handling for marketplace actions.The
MarketplaceActionIconcomponent correctly handles the three states (error, pending activity, and marketplace listing) with appropriate visual indicators and tooltips.
1160-1191: LGTM! Clean modal integration with good user flow.The modal integration creates a seamless workflow where users are automatically prompted to list their domain for sale after upgrading for marketplace compatibility. The state management and callback structure are well-implemented.
| const { data: userAssets, isLoading: assetsLoading } = | ||
| useMarketplaceUserAssets(); | ||
| const [{ walletAddress }] = useWalletState(); | ||
| const [{ marketplaceProcessId }] = useGlobalState(); | ||
|
|
||
| if (!userAssets || !walletAddress || assetsLoading) { | ||
| return { | ||
| pendingWorkflows: [], | ||
| isLoading: assetsLoading, | ||
| hasPendingWorkflows: false, | ||
| getPendingWorkflowForDomain: () => undefined, | ||
| }; | ||
| } |
There was a problem hiding this comment.
Clarify loading state when data is unavailable.
The early return sets isLoading: assetsLoading, but if the useMarketplaceUserAssets query completes with null data (e.g., due to an error or empty response), assetsLoading will be false while userAssets is still null. This results in isLoading: false even though no valid data is available, potentially causing the UI to display "no pending workflows" instead of a loading or error state.
Consider handling the case where data fetch completes but returns no valid data:
💡 Suggested improvement
if (!userAssets || !walletAddress || assetsLoading) {
return {
pendingWorkflows: [],
- isLoading: assetsLoading,
+ isLoading: assetsLoading || !userAssets,
hasPendingWorkflows: false,
getPendingWorkflowForDomain: () => undefined,
};
}🤖 Prompt for AI Agents
In @src/hooks/usePendingWorkflows.ts around lines 30 - 42, The early-return in
usePendingWorkflows incorrectly reports isLoading as assetsLoading even when
userAssets is null after the query; update the return to reflect that missing
data means still loading (or errored) by setting isLoading to assetsLoading ||
userAssets == null (or use the hook's isError if available), e.g. change the
isLoading value in the early return to assetsLoading || !userAssets so callers
see a loading state when data is unavailable; keep the rest of the returned
shape (pendingWorkflows: [], hasPendingWorkflows: false,
getPendingWorkflowForDomain: () => undefined) unchanged.
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/state/contexts/GlobalState.tsx (1)
118-126: Add MU_URL to the defaultMarketplaceContract AO connection configuration.The
defaultMarketplaceContractinitialization is missingMU_URLin theconnect()call. According to AR.IO SDK documentation,ArNSMarketplaceReadrequires bothCU_URLandMU_URLto properly route read requests to the AO message unit. The configuration should mirror thedefaultArIOpattern:export const defaultMarketplaceContract = new ArNSMarketplaceRead({ process: new AOProcess({ processId: initialMarketplaceProcessId, ao: connect({ CU_URL: initialAoNetwork.ARIO.CU_URL, MU_URL: initialAoNetwork.ARIO.MU_URL, MODE: 'legacy', }), }), });
🤖 Fix all issues with AI agents
In @package.json:
- Line 94: The prepare script in package.json uses the deprecated "husky
install" command; update the "prepare" npm script value to "husky" so it uses
Husky v9 syntax (edit the "prepare" script entry in package.json to replace
"husky install" with "husky"), leaving existing .husky/ hook files unchanged.
- Around line 27-29: The Sentry packages are mismatched—make @sentry/browser and
@sentry/react the same major version (either both v7.x or both v10.x) by
updating the "@sentry/browser" and "@sentry/react" entries so they align; for
ethers, do not blindly upgrade to 6.x—either pin "ethers" to a compatible 5.x
release or follow the official ethers v6 migration guide and update all usages
of BigNumber, Contract API imports, and renamed utilities before bumping the
version; "date-fns" can be upgraded to 4.x with minimal work (mostly TypeScript
fixes), and leave the "@permaweb/aoconnect" and "@permaweb/ao-scheduler-utils"
0.0.x pins as-is since they are intentionally versioned.
In @src/utils/constants.ts:
- Around line 28-29: The constant DEFAULT_MIN_MARKETPLACE_ANT_VERSION is set to
a non-existent ANT version (25); update its value to the released version 23 (or
the correct released version if project needs differ). Locate and edit the
exported constant DEFAULT_MIN_MARKETPLACE_ANT_VERSION in src/utils/constants.ts
and change its numeric value from 25 to 23, then run tests or lint to ensure no
other code expects 25.
🧹 Nitpick comments (1)
package.json (1)
60-60: Consider using semantic versioning range for lucide-react.The
lucide-reactdependency is pinned to an exact version0.556.0without a^prefix, unlike most other dependencies in this file. This prevents automatic patch and minor updates, which could include important security fixes.Unless there's a specific reason for the exact pin (e.g., known breaking changes in patch versions), consider using
^0.556.0to allow safe minor updates while preventing breaking changes.♻️ Proposed fix
- "lucide-react": "0.556.0", + "lucide-react": "^0.556.0",
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (3)
package.jsonsrc/state/contexts/GlobalState.tsxsrc/utils/constants.ts
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2025-12-02T17:38:52.889Z
Learnt from: vilenarios
Repo: ar-io/arns-react PR: 899
File: src/services/turbo/BaseTokenPurchaseService.ts:0-0
Timestamp: 2025-12-02T17:38:52.889Z
Learning: In src/services/turbo/BaseTokenPurchaseService.ts, the function executeBaseTokenPurchase intentionally only handles Base token top-up to Turbo credits, not the actual ArNS purchase. The ArNS purchase is handled separately by dispatchArIOInteraction in Checkout.tsx because it manages multiple complex purchase workflows (lease, permabuy, extend, increase undernames, etc.) with their own state management. This separation of concerns is by design.
Applied to files:
src/state/contexts/GlobalState.tsx
📚 Learning: 2025-09-10T16:35:55.727Z
Learnt from: dtfiedler
Repo: ar-io/arns-react PR: 873
File: src/components/modals/ant-management/UpgradeDomainsModal/UpgradeDomainsModal.tsx:0-0
Timestamp: 2025-09-10T16:35:55.727Z
Learning: In arns-react, when calling buildDomainInfoQuery (src/hooks/useDomainInfo.tsx), you must pass arioProcessId along with arioContract, domain, aoNetwork, hyperbeamUrl, and antRegistryProcessId; omitting arioProcessId can lead to "No ANT id or record found" and break upgrades.
Applied to files:
src/state/contexts/GlobalState.tsx
📚 Learning: 2025-09-10T16:35:55.727Z
Learnt from: dtfiedler
Repo: ar-io/arns-react PR: 873
File: src/components/modals/ant-management/UpgradeDomainsModal/UpgradeDomainsModal.tsx:0-0
Timestamp: 2025-09-10T16:35:55.727Z
Learning: In arns-react, when calling buildDomainInfoQuery (src/hooks/useDomainInfo.tsx), you must pass arioProcessId along with arioContract, domain, aoNetwork, hyperbeamUrl, and antRegistryProcessId; omitting arioProcessId can lead to “No ANT id or record found” and break upgrades.
Applied to files:
src/state/contexts/GlobalState.tsx
📚 Learning: 2025-12-16T16:47:36.348Z
Learnt from: dtfiedler
Repo: ar-io/arns-react PR: 904
File: .github/workflows/pr-preview.yaml:52-58
Timestamp: 2025-12-16T16:47:36.348Z
Learning: In the ar-io/arns-react repository, ArNS undername cleanup for PR previews is handled by the permaweb-deploy GitHub action, not by a local script in this repository. The cleanup step should not be included in the PR preview workflow.
Applied to files:
package.json
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: deploy-preview
- GitHub Check: deploy-preview
- GitHub Check: lint_and_build
🔇 Additional comments (8)
package.json (1)
6-20: Test infrastructure removal is confirmed as intentional.The removal of all test-related scripts (test, test:updateSnapshot, test:coverage, test:playwright) and associated tooling is complete and intentional. Analysis confirms:
- No test files exist in the repository
- No test frameworks remain in devDependencies
- CI workflows contain no references to test script execution
- No test configuration files present
This represents a deliberate architectural decision to remove automated testing infrastructure entirely. While this is a significant change affecting quality assurance processes, it is already aligned with the project's CI/CD configuration, which expects no test execution.
src/utils/constants.ts (1)
90-99: URL validation broadened appropriately.The regex changes correctly allow:
- Uppercase letters in hostnames (DNS is case-insensitive)
localhostfor development environmentsThese changes improve URL validation without introducing security concerns.
src/state/contexts/GlobalState.tsx (6)
8-9: LGTM: Marketplace SDK imports added.The ArNSMarketplaceRead and ArNSMarketplaceWrite types are correctly imported for marketplace contract integration.
44-85: LGTM: Settings persistence extended for marketplace.The function correctly loads and persists marketplace-related settings with appropriate fallbacks to the constants defined in constants.ts.
98-102: LGTM: Marketplace initialization values.The initialization follows the established pattern for other process IDs with proper fallback handling.
105-116: Pattern change: Explicit AO configuration instead of spreading.The initialization now explicitly sets
CU_URL,MU_URL, andMODE: 'legacy'instead of spreading the network configuration. This is more rigid but provides clearer control over the connection parameters.
128-145: LGTM: GlobalState type extended appropriately.The marketplace-related fields follow the established patterns for ARIO state management. The union type for
marketplaceContractcorrectly allows both read and write implementations.
147-175: LGTM: Initial state properly configured.All marketplace-related fields are correctly initialized with appropriate defaults. The explicit AO client configurations are consistent with the pattern used in
defaultArIO.
| "@dha-team/arbundles": "^1.0.4", | ||
| "@permaweb/ao-scheduler-utils": "^0.0.29", | ||
| "@permaweb/aoconnect": "0.0.68", |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check Sentry package information and compatibility
echo "=== Checking @sentry/browser@10.29.0 details ==="
npm view @sentry/browser@10.29.0 --json 2>/dev/null | jq '{version: .version, peerDependencies: .peerDependencies}' || echo "Package lookup failed"
echo -e "\n=== Checking @sentry/react@7.45.0 details ==="
npm view @sentry/react@7.45.0 --json 2>/dev/null | jq '{version: .version, peerDependencies: .peerDependencies}' || echo "Package lookup failed"
echo -e "\n=== Checking @sentry/browser major versions ==="
npm view @sentry/browser --json 2>/dev/null | jq '.versions[-5:]' || echo "Cannot fetch version history"
echo -e "\n=== Checking pre-release package info ==="
npm view "@permaweb/aoconnect@0.0.68" --json 2>/dev/null | jq '{version: .version, description: .description, deprecated: .deprecated}' || echo "Cannot fetch package info"
npm view "@permaweb/ao-scheduler-utils@0.0.29" --json 2>/dev/null | jq '{version: .version, description: .description, deprecated: .deprecated}' || echo "Cannot fetch package info"Repository: ar-io/arns-react
Length of output: 888
🌐 Web query:
@sentry/browser 10 @sentry/react 7 compatibility
💡 Result:
Short answer: No — they’re not compatible. @sentry/react v7 is built to pair with the 7.x Sentry packages (it depends on @sentry/browser 7.x), so using @sentry/browser@10 will cause a version mismatch. Upgrade both packages to the same major (e.g., all 7.x or all 10.x), or use a @sentry/react release that matches @sentry/browser@10. [1][2]
Sources:
- @sentry/react package.json (shows dependency on @sentry/browser 7.x). [1]
- Sentry migration note: “make sure to upgrade all of them to version 7.” [2]
🌐 Web query:
date-fns v4 breaking changes migration
💡 Result:
Short summary — upgrading to date-fns v4 (from v3.x)
-
Breaking changes: there are very few user-facing breaks. Most changes are TypeScript/type-level only and affect code that relies on internal date-fns types. The package is now ESM-first (CommonJS still supported but may break in some environments). v4 adds first-class time‑zone support (TZDate / tz helper) and adds a context "in" option to many functions. Run the type checker after upgrade and address any type errors. [1][2]
-
Practical migration steps
- Upgrade package (npm/yarn) to date-fns@^4.x. [2]
- Run your TypeScript build/type checker; fix any type errors (they’re typically about internal types). If you hit unfixable problems, open an issue on the repo. [2]
- If you use time zones: install and/or upgrade the companion TZ package (/@date-fns/tz or TZDate) to the recommended version and adapt code that should use TZDate or the new context "in" option. Also update any formatting calls that rely on timezone behavior — v4 added tz support for format and related formatters. [1][4]
- If you consume date-fns in a CommonJS-only environment and see runtime issues, try ESM-compatible imports or a dual import approach; report environment-specific problems to the project if needed. [1][2]
- Links / references
- v4 announcement / time zones blog post. [1]
- v4.0.0 GitHub release notes (lists breaks and recommendations). [2]
If you want, tell me whether you’re using plain JS or TypeScript, CommonJS or ESM, and whether you use time-zone features — I’ll give specific code changes to apply.
🌐 Web query:
ethers v6 breaking changes from v5
💡 Result:
Key breaking changes moving from ethers v5 → v6 (summary):
- Big numbers: BigNumber removed — use JS BigInt (and FixedNumber for fixed-point). [1]
- Contract API: Contract is an ES6 Proxy; overloaded/ambiguous methods and helper operations were reorganized (e.g., contract.foo.send, .staticCall, .estimateGas, .populateTransaction). [1]
- Imports: most subpackages moved to the root package (imports changed). [1]
- Providers: Web3Provider renamed BrowserProvider (EIP‑1193); StaticJsonRpcProvider merged into JsonRpcProvider; sendTransaction → broadcastTransaction; fee APIs consolidated into getFeeData. [1]
- Signatures / Transactions: signature helpers are now a Signature class; transactions use Transaction class (parsing/serializing changes). [1]
- Utilities / naming: many utils renamed/moved (e.g., ethers.utils.formatBytes32String → ethers.encodeBytes32String; ethers.constants.AddressZero → ethers.ZeroAddress; hex/byte helpers renamed). [1]
- Removed/changed helpers: Logger removed (replaced by error utilities); some small utils removed or replaced (see list in migration guide). [1]
For full details and code examples for each change, see the official migration guide. [1]
Sources:
[1] Ethers.js — Migrating from v5 (official docs).
Fix @sentry/browser and @sentry/react version mismatch.
@sentry/browser v10.29.0 (line 36) is incompatible with @sentry/react v7.45.0 (line 37). @sentry/react v7 requires @sentry/browser v7.x; mixing major versions breaks integration. Upgrade both packages to the same major version (either both v7.x or both v10.x).
Additionally, ethers v6.16.0 introduces major breaking changes from v5: BigNumber is removed (use JS BigInt), Contract API is reorganized, imports have changed, and many utility functions are renamed. These require code migration beyond a simple version bump—review the migration guide and test thoroughly.
date-fns v4.1.0 has few user-facing breaking changes; most are TypeScript type-level only. Migration is straightforward.
The @permaweb/aoconnect (0.0.68) and @permaweb/ao-scheduler-utils (0.0.29) packages are active (not deprecated) and use intentional 0.0.x versioning; they are suitable for production use.
🤖 Prompt for AI Agents
In @package.json around lines 27 - 29, The Sentry packages are mismatched—make
@sentry/browser and @sentry/react the same major version (either both v7.x or
both v10.x) by updating the "@sentry/browser" and "@sentry/react" entries so
they align; for ethers, do not blindly upgrade to 6.x—either pin "ethers" to a
compatible 5.x release or follow the official ethers v6 migration guide and
update all usages of BigNumber, Contract API imports, and renamed utilities
before bumping the version; "date-fns" can be upgraded to 4.x with minimal work
(mostly TypeScript fixes), and leave the "@permaweb/aoconnect" and
"@permaweb/ao-scheduler-utils" 0.0.x pins as-is since they are intentionally
versioned.
| "jest-environment-jsdom": "^29.7.0", | ||
| "jest-location-mock": "^2.0.0", | ||
| "jest-transform-stub": "^2.0.0", | ||
| "husky": "^9.1.7", |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check Husky hook configurations for v9 compatibility
echo "=== Checking Husky hook files ==="
if [ -d ".husky" ]; then
find .husky -type f | head -20 | while read file; do
echo "--- $file ---"
cat "$file"
done
else
echo "No .husky directory found"
fi
echo -e "\n=== Checking package.json for Husky and prepare script ==="
cat package.json | jq '.scripts.prepare, .devDependencies.husky' 2>/dev/null || echo "Could not parse package.json"
echo -e "\n=== Checking for legacy Husky patterns ==="
if grep -r "husky.*hooks\|husky.*config\|\.huskyrc" . --include="*.json" --include="*.js" 2>/dev/null; then
echo "Found legacy patterns"
else
echo "No obvious legacy Husky patterns found"
fiRepository: ar-io/arns-react
Length of output: 460
🌐 Web query:
Husky v9 breaking changes migration from v8
💡 Result:
Summary of breaking changes (v8 → v9) and quick migration steps
Key breaking changes
- Husky no longer uses a JSON "husky" config in package.json — hooks are file-based under .husky/. [1][2]
- Change prepare script: "husky install" → "husky". The old install command is deprecated. [1][2]
- Hook scripts now run as shell files in .husky (use $1, $2… for git params; HUSKY_GIT_PARAMS removed). [1][3]
- Env var changes: HUSKY replaces HUSKY_SKIP_HOOKS/HUSKY_SKIP_INSTALL; debug uses HUSKY=2. [1][6]
- Module API moved to ESM (import), and Node 14/16 support dropped. [1][2]
- Behavior tweaks: .git missing now warns instead of failing; Windows-executable permission handling improved; XDG_CONFIG_HOME support for ~/.huskyrc moved to ~/.config/husky/init.sh. [1][2]
Quick migration steps
- Install v9: npm install --save-dev husky@^9. [1]
- Update package.json prepare script: "prepare": "husky". Remove any "husky" object with hooks. [1][2]
- Create file-based hooks in .husky/, e.g.:
- echo "npm run lint-staged" > .husky/pre-commit
- echo "commitlint --edit $1" > .husky/commit-msg
- chmod +x .husky/*
Use positional params ($1) instead of HUSKY_GIT_PARAMS. [3][1][6]
- If using Husky as a module, switch to ESM import usage. [2]
- Test commits/pushes and fix any script/path differences (use package manager to run local binaries if needed). [6]
References
[1] Husky v9 release/notes.
[2] Multiple migration examples / merge requests (v8→v9).
[3] Official Husky migration docs (env var and params changes).
Update the prepare script to use the Husky v9 syntax.
The prepare script in package.json must be changed from "husky install" to "husky" for compatibility with Husky v9. The old install command is deprecated. The existing hook configurations in .husky/ are properly structured for v9.
Update package.json line 17:
- "prepare": "husky install",
+ "prepare": "husky",Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In @package.json at line 94, The prepare script in package.json uses the
deprecated "husky install" command; update the "prepare" npm script value to
"husky" so it uses Husky v9 syntax (edit the "prepare" script entry in
package.json to replace "husky install" with "husky"), leaving existing .husky/
hook files unchanged.
| // Default minimum ANT version required for marketplace functionality | ||
| export const DEFAULT_MIN_MARKETPLACE_ANT_VERSION = 25; |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
AR.IO ANT version 25 features release marketplace
💡 Result:
I couldn’t find a clear match for “AR.IO ANT version 25” or a related “features release marketplace.” Do you mean one of these?
- AR.io (or ar.io) platform release v25
- Apache Ant (build tool) version 2.5 or v25 (unlikely)
- ANT product from a vendor (e.g., AutoRABIT ARM/ANT releases) or an app named “ANT” on a marketplace
- A specific marketplace release (Atlassian Marketplace, Mendix Marketplace, etc.) referencing “ANT v25”
Tell me which one you mean (or paste a link/screenshot) and I’ll fetch the exact v25 release notes and features.
🌐 Web query:
Arweave Name Token ANT versions releases
💡 Result:
Brief summary — Arweave Name Token (ANT) versions & releases
- What ANT is: a SmartWeave/ AO process implementing the Arweave Name System (ArNS) token standard used to own/control ArNS names. [1][2]
- Pilot / early releases: ANT source/test versions were published for the ArNS pilot (examples reference ANT Source v0.1.5 used in the pilot). [3]
- Current implementation(s): the official AO-compatible implementation lives in the ar-io GitHub (ar-io/ar-io-ant-process) and AR.IO docs/SDK reference ANT APIs; AR.IO migrated ArNS/ANT functionality to AO (migration notes posted). [2][1][4]
Links / sources
- ArNS / ANT docs (AR.IO / arweave docs). [1]
- ar-io GitHub — ar-io-ant-process (AO implementation). [2]
- ArNS pilot / ANT source v0.1.5 mention. [3]
- AR.IO migration to AO (migration notes). [4]
If you want, I can:
- list exact ANT contract source TX IDs used in each release,
- fetch the GitHub tags/commit history for ar-io-ant-process, or
- retrieve the ANT v0.1.5 source file — which would I should fetch now?
🌐 Web query:
AR.IO ANT token latest version GitHub
💡 Result:
Latest ANT (Arweave Name Token) repo: ar-io/ar-io-ant-process on GitHub. [1]
Latest ANT module/version (per AR.IO docs): version "23", moduleId "FKtQtOOtlcWCW2pXrwWFiCSlnuewMZOHCzhulVkyqBE". [2]
Sources:
- ar-io/ar-io-ant-process (GitHub). [1]
- AR.IO docs — ANT versions / getLatestANTVersion. [2]
🌐 Web query:
site:github.com ar-io/ar-io-ant-process releases version 25
💡 Result:
I couldn't find a GitHub release "version 25" for ar-io/ar-io-ant-process. The repository page and tags/releases view show no v25 release. [1][2]
Sources:
- ar-io/ar-io-ant-process repository (GitHub). [1]
- ar-io GitHub organization listing (shows ar-io-ant-process repo). [2]
🌐 Web query:
site:github.com ar-io/ar-io-ant-process "v25" OR "version 25"
💡 Result:
I searched the ar-io/ar-io-ant-process repository for "v25" / "version 25" and found the repository and ANT docs, but no occurrences of "v25" or "version 25" in that repo or its public docs. Sources: GitHub repo and AR.IO SDK docs. [1][2]
Sources
- ar-io/ar-io-ant-process (GitHub). [1]
- AR.IO SDK / Versions documentation (shows example getVersion output "23"). [2]
Update DEFAULT_MIN_MARKETPLACE_ANT_VERSION to a released version.
ANT version 25 does not exist. The latest released ANT version is 23 (per AR.IO documentation). Update this constant to use version 23 or verify if a different released version is required for marketplace functionality.
🤖 Prompt for AI Agents
In @src/utils/constants.ts around lines 28 - 29, The constant
DEFAULT_MIN_MARKETPLACE_ANT_VERSION is set to a non-existent ANT version (25);
update its value to the released version 23 (or the correct released version if
project needs differ). Locate and edit the exported constant
DEFAULT_MIN_MARKETPLACE_ANT_VERSION in src/utils/constants.ts and change its
numeric value from 25 to 23, then run tests or lint to ensure no other code
expects 25.
| export const MARKETPLACE_PROCESS_ID = | ||
| import.meta.env.VITE_MARKETPLACE_PROCESS_ID || | ||
| 'ijCygKzV48PHx_TUdFN7NNJgbBUatQDL_h8FfqVzXfE'; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, verify the file exists and get context around the constant
cat -n src/utils/constants.ts | head -120 | tail -30Repository: ar-io/arns-react
Length of output: 1276
🏁 Script executed:
# Search for usages of MARKETPLACE_PROCESS_ID
rg "MARKETPLACE_PROCESS_ID" --type ts --type jsRepository: ar-io/arns-react
Length of output: 461
🏁 Script executed:
# Look for ARIO_PROCESS_ID mentioned in the review for pattern comparison
rg "ARIO_PROCESS_ID" --type ts --type js -A 2 -B 2Repository: ar-io/arns-react
Length of output: 4985
🏁 Script executed:
# Check for any documentation files mentioning marketplace or process IDs
fd -e md -e txt | xargs rg "MARKETPLACE_PROCESS_ID|marketplace process" 2>/dev/null || trueRepository: ar-io/arns-react
Length of output: 42
🏁 Script executed:
# Check if there's a MARKETPLACE_TESTNET_PROCESS_ID constant
rg "MARKETPLACE_TESTNET|marketplace.*testnet" --type ts --type js -iRepository: ar-io/arns-react
Length of output: 42
🏁 Script executed:
# Look at vite-env.d.ts to see all environment variable declarations
cat src/vite-env.d.tsRepository: ar-io/arns-react
Length of output: 695
🏁 Script executed:
# Check for ARIO_TESTNET_PROCESS_ID to understand the pattern better
rg "ARIO_TESTNET_PROCESS_ID|ARIO_MAINNET_PROCESS_ID" --type ts --type js -B 2 -A 2Repository: ar-io/arns-react
Length of output: 6402
Add VITE_MARKETPLACE_PROCESS_ID to environment variable type definitions and align the fallback pattern with ARIO_PROCESS_ID.
The VITE_MARKETPLACE_PROCESS_ID environment variable is referenced in the code but not declared in src/vite-env.d.ts, which breaks TypeScript intellisense. Additionally, the hardcoded string fallback differs from how ARIO_PROCESS_ID handles it—ARIO_PROCESS_ID falls back to an imported SDK constant (ARIO_TESTNET_PROCESS_ID), while MARKETPLACE_PROCESS_ID uses a hardcoded string. Consider either importing a marketplace process ID constant from the SDK or adding explicit testnet/mainnet variants (similar to ARIO_TESTNET_PROCESS_ID/ARIO_MAINNET_PROCESS_ID) to maintain consistency.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @package.json:
- Line 23: The package dependency entry "@ar.io/sdk": "^3.23.0-alpha.3"
references a non-existent npm version; update that dependency in package.json to
a valid published version (for example "3.19.0") or the correct released tag,
then run npm install to verify; ensure the change is made to the dependency
string for "@ar.io/sdk" so CI and local installs succeed.
- Around line 36-37: package.json lists incompatible Sentry versions:
"@sentry/react" at ^7.45.0 and "@sentry/browser" at ^10.29.0; pick a matching
pair and update package.json accordingly — either bump "@sentry/react" to
^10.32.0 to match "@sentry/browser" 10.x, or downgrade "@sentry/browser" to
^7.120.0 so both are on 7.x; after changing the version string for
"@sentry/react" or "@sentry/browser", run your package manager install and
verify the lockfile updates and the app builds/tests pass.
🧹 Nitpick comments (2)
package.json (2)
29-29: Consider using caret range for version flexibility.The
@permaweb/aoconnectdependency is pinned to exact version0.0.68without a caret (^), which prevents automatic patch updates. Since this is a0.0.xversion indicating an unstable API, consider whether strict pinning is necessary or if^0.0.68would be more appropriate to receive bug fixes.Verify if the exact version pinning is intentional due to breaking changes between patches. If not required, use semantic versioning with a caret to receive non-breaking updates.
61-61: Consider moving deployment tool to devDependencies.The
permaweb-deploypackage is used only in build scripts (lines 8-9) and is typically a development/deployment tool rather than a runtime dependency. Unless the built application requires this package at runtime, it should be moved todevDependenciesto reduce production bundle size.📦 Suggested placement in devDependencies
Move
permaweb-deployfromdependenciestodevDependenciesunless it's required at runtime.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (1)
package.json
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-12-16T16:47:36.348Z
Learnt from: dtfiedler
Repo: ar-io/arns-react PR: 904
File: .github/workflows/pr-preview.yaml:52-58
Timestamp: 2025-12-16T16:47:36.348Z
Learning: In the ar-io/arns-react repository, ArNS undername cleanup for PR previews is handled by the permaweb-deploy GitHub action, not by a local script in this repository. The cleanup step should not be included in the PR preview workflow.
Applied to files:
package.json
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: deploy-preview
- GitHub Check: lint_and_build
🔇 Additional comments (2)
package.json (2)
6-20: This comment is incorrect. There is no evidence of test infrastructure or test-related scripts being removed in this change. The codebase contains no test files, test configuration files, or test dependencies. The existing workflowbuild_and_test.yml(which was recently added) only executes linting and building—it does not run any tests. The review comment's assertion that test scripts have been "removed" cannot be verified against the actual codebase state.Likely an incorrect or invalid review comment.
27-28: No known security vulnerabilities detected in the specified dependencies. All packages are free from publicly reported security advisories, and none are marked as deprecated.Some packages have newer versions available (@sentry/browser@10.32.1, lucide-react@0.562.0, permaweb-deploy@2.5.2), but the current versions pose no security risk. The use of @ar.io/sdk@3.23.0-alpha.3 is intentional for access to alpha features.
| "dependencies": { | ||
| "@ant-design/icons": "5.4.0", | ||
| "@ar.io/sdk": "^3.22.1", | ||
| "@ar.io/sdk": "^3.23.0-alpha.3", |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check available versions and info for @ar.io/sdk
npm view @ar.io/sdk versions --json | tail -20
echo "---"
# Get specific info about the alpha version
npm view @ar.io/sdk@3.23.0-alpha.3 --json 2>/dev/null | jq '{version, deprecated, description}' || echo "Alpha version info not found"
echo "---"
# Check latest version
npm view @ar.io/sdk@latest --json 2>/dev/null | jq '{version, deprecated}' || echo "Latest version info not found"Repository: ar-io/arns-react
Length of output: 176
🌐 Web query:
@ar.io/sdk npm package versions 3.23.0-alpha
💡 Result:
I couldn't find a published @ar.io/sdk version 3.23.0-alpha on npm or the project's GitHub — the npm package shows latest published v3.19.0 (as of now). [1][2]
Sources:
- npm package page for @ar.io/sdk (shows current versions). [1]
- ar-io/ar-io-sdk GitHub repository (project home). [2]
Want me to search the GitHub tags/releases for an unpublished tag or check a specific registry/commit?
Remove or replace non-existent package version.
The @ar.io/sdk dependency specifies version ^3.23.0-alpha.3, which does not exist in the npm registry. The latest published version is 3.19.0. This configuration will cause npm install to fail.
Update to an available version (either the latest stable 3.19.0 or confirm the correct version if a newer release is intended).
🤖 Prompt for AI Agents
In @package.json at line 23, The package dependency entry "@ar.io/sdk":
"^3.23.0-alpha.3" references a non-existent npm version; update that dependency
in package.json to a valid published version (for example "3.19.0") or the
correct released tag, then run npm install to verify; ensure the change is made
to the dependency string for "@ar.io/sdk" so CI and local installs succeed.
| "@sentry/browser": "^10.29.0", | ||
| "@sentry/react": "^7.45.0", |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
@sentry/react 7.45.0 @sentry/browser compatibility peer dependencies
💡 Result:
@sentry/react@7.45.0 is tied to the same 7.45.x Sentry JS stack — @sentry/react bundles/depends on the matching @sentry/browser 7.45.0 (not as a peerDependency). See the package pattern for the 7.x releases (e.g. @sentry/react lists @sentry/browser@"") and the @sentry/browser 7.45.0 release. [1][2]
Sources:
- @sentry/react package.json (example of 7.x releases showing "@sentry/browser": ""). [1]
- @sentry/browser release 7.45.0. [2]
🏁 Script executed:
#!/bin/bash
# Get peer dependencies for @sentry/react 7.45.0
npm view @sentry/react@7.45.0 peerDependencies
# Check @sentry/browser v10.29.0 peer dependencies
npm view @sentry/browser@10.29.0 peerDependencies
# Get latest versions of both packages
npm view @sentry/browser dist-tags
npm view @sentry/react dist-tagsRepository: ar-io/arns-react
Length of output: 301
Fix incompatible Sentry package versions.
@sentry/react@^7.45.0 requires @sentry/browser to be in the matching 7.x version range, but the package.json specifies @sentry/browser@^10.29.0. This will cause dependency resolution failures or runtime errors.
Either upgrade @sentry/react to ^10.32.0 or downgrade @sentry/browser to ^7.120.0 to match.
🤖 Prompt for AI Agents
In @package.json around lines 36 - 37, package.json lists incompatible Sentry
versions: "@sentry/react" at ^7.45.0 and "@sentry/browser" at ^10.29.0; pick a
matching pair and update package.json accordingly — either bump "@sentry/react"
to ^10.32.0 to match "@sentry/browser" 10.x, or downgrade "@sentry/browser" to
^7.120.0 so both are on 7.x; after changing the version string for
"@sentry/react" or "@sentry/browser", run your package manager install and
verify the lockfile updates and the app builds/tests pass.
…sage and display of price
There was a problem hiding this comment.
Actionable comments posted: 15
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/data-display/tables/DomainsTable.tsx (1)
520-591: Fix staletableDataby correcting theuseEffectdependency list.This effect derives
ioCompatible(depends onwalletAddress,latestAntVersion?.moduleId) androle(depends onwalletAddress,marketplaceProcessId), but those aren’t in the deps array, so rows can render with stale role/compatibility.Proposed fix
useEffect(() => { if (domainData) { const newTableData: TableData[] = []; @@ setTableData(newTableData); } }, [ domainData, loading, loadingArnsState, primaryNameData, dispatchArNSState, + walletAddress, + latestAntVersion?.moduleId, + marketplaceProcessId, ]);
🤖 Fix all issues with AI agents
In @.github/workflows/pr-preview.yaml:
- Around line 3-10: The workflow currently declares a manual trigger
(workflow_dispatch) but the deploy-preview job is gated by a PR-only condition
(if: github.event.pull_request...), which makes manual runs skip/fail; either
remove workflow_dispatch from the top-level triggers, or update the
deploy-preview job's if expression (the one on the deploy-preview job) to
explicitly require a PR event (e.g., include github.event_name == 'pull_request'
in the if) or add alternate logic/inputs for manual dispatch so that
deploy-preview can run on workflow_dispatch; adjust the job-level if on
deploy-preview accordingly.
- Around line 5-6: The workflow currently lists pull_request types as [opened,
synchronize, reopened, closed], which causes the same build/deploy job to run on
PR close; either remove "closed" from the pull_request types array to stop
redeploys on PR close, or add a separate job that targets the
pull_request.closed event to perform explicit cleanup (e.g., remove the ArNS
undername pointer) instead of reusing the deploy job; update the pull_request
trigger (the types array) accordingly and, if adding cleanup, create a new job
conditioned on the closed event to run only cleanup steps.
In @src/components/data-display/tables/DomainsTable.tsx:
- Around line 212-257: The route construction in MarketplaceActionIcon (the Link
to prop) uses raw domainName which can break on Unicode/special chars; update
all marketplace route usages (e.g., the Link in MarketplaceActionIcon and the
other similar Link/route constructions noted) to canonicalize and URL-encode the
name by calling your domain ASCII conversion helper (e.g.,
encodeDomainToASCII(domainName)) and then encodeURIComponent(...) before
interpolating into the path so the route is safe and unambiguous; ensure you
apply the same change to every other place that builds
`/marketplace/names/${...}` using domainName.
- Around line 671-698: The Tooltip's icon currently uses the router Link for an
external URL
(to={`https://${encodeDomainToASCII(row.getValue('name'))}.${gateway}`}) with
target="_blank" which is unsafe; replace the Link with a plain anchor element
(<a>) for external navigation or ensure the component supports an explicit rel
prop, and add rel="noopener noreferrer" alongside target="_blank", preserving
the existing classes, content
(formatForMaxCharCount(decodeDomainToASCII(rowValue), 20) and <ExternalLinkIcon
className="size-3 fill-grey" />), so the external link in DomainsTable (inside
the Tooltip icon) opens in a new tab securely.
In @src/components/data-display/tables/MarketplaceListingsTable.tsx:
- Around line 119-123: The tableData useMemo currently returns [] when
arnsRecords is falsy, causing the listings table to stay empty while names load;
update the logic in the useMemo (tableData) to still map ordersData.items into
rows when ordersData exists and arnsRecords is not yet available, and for name
fields insert a loading placeholder like "Loading name..." or null so the table
renders partial results; once arnsRecords becomes available, reconcile by
replacing placeholders with resolved names (use the same mapping function that
looks up arnsRecords by address to fill names).
- Around line 295-306: The cell renderer in MarketplaceListingsTable currently
constructs new ArweaveTransactionID(antId) which can throw for invalid ANT IDs;
wrap the creation in a try-catch (or pre-validate antId format) inside the cell
function so a thrown error won’t crash rendering, and on failure render a safe
fallback (e.g., ArweaveID with null/placeholder props or plain text like
"Invalid ID"). Ensure you reference the same symbols (the cell renderer, antId,
ArweaveTransactionID and ArweaveID) and return the fallback component from the
cell when validation or construction fails.
- Around line 275-286: The ArweaveTransactionID constructor can throw when given
an invalid seller ID; update the cell renderer in MarketplaceListingsTable.tsx
(the cell function that references seller, ArweaveTransactionID, ArweaveID,
AoAddress and ArweaveIdTypes.ADDRESS) to guard construction: either validate
seller first or wrap new ArweaveTransactionID(seller) in a try-catch, and on
error render a safe fallback (e.g., render the raw seller string, a placeholder,
or null) instead of letting the exception propagate and crash the table.
- Around line 90-94: The MarketplaceListingsTable currently passes a hard-coded
limit: 100 to useMarketplaceOrders which prevents the hook's cursor-based
pagination from returning more than 100 accumulated items; either remove the
explicit limit argument so the hook can default to full pagination, or keep the
limit but document the UX/performance rationale near the
MarketplaceListingsTable (e.g., a comment above the useMarketplaceOrders call)
and consider exposing the limit as a configurable prop (e.g., marketplaceLimit)
so it’s clear and adjustable.
In @src/components/modals/ListNameForSaleModal/ListNameForSaleModal.tsx:
- Around line 634-645: In the ListNameForSaleModal JSX block, the external docs
link is using react-router's Link; replace that Link usage with a plain anchor
element: use <a> with href="https://docs.ar.io/build/guides/arns-marketplace",
keep the same className and target="_blank", and add rel="noopener noreferrer"
so external navigation is safe; ensure you remove/stop using the Link's to prop
in this instance and do not import or use react-router Link for this external
URL.
- Around line 395-415: The temporary console.log of { newARIOBalance,
listingFee: feeDetails.listingFee, tries, maxTries } inside ListNameForSaleModal
should be removed or wrapped behind a development/debug guard to avoid leaking
operational details; replace it with a gated debug call (e.g., only log when
process.env.NODE_ENV !== 'production' or when a local debug flag is true) or use
the app's structured logger at debug level, and make the same change for the
other console logging at the later location referenced (lines 464-465) so
production builds do not emit sensitive balance/event data.
- Around line 615-618: In the ListNameForSaleModal component, don’t silently
return null when hasPendingIntent is true; instead either call onClose() and
emit a toast/event or render a small fallback UI that guides the user to
continue the existing flow (e.g., show a short message and a CTA that opens
ContinueWorkflowModal). Replace the early-return block that checks
hasPendingIntent with logic that: (a) invokes the existing onClose() prop and
dispatches a toast/event if you want to auto-close, or (b) renders a compact
message + button that sets a local state (e.g., showContinueWorkflow) or calls
the handler to open ContinueWorkflowModal so the user can resume; ensure you
reuse the onClose, hasPendingIntent, and ContinueWorkflowModal symbols so
behavior integrates with parent state.
- Around line 356-439: The deposit-verification loop misthrows because it checks
newARIOBalance <= feeDetails.listingFee (which is true until the balance is
strictly greater); fix by replacing that logic so the loop only throws on
exceeded tries and otherwise treats newARIOBalance >= feeDetails.listingFee as
success. Concretely, inside the while loop that reads newARIOBalance (variables:
newARIOBalance, feeDetails.listingFee, tries, maxTries) remove the
`newARIOBalance <= feeDetails.listingFee` throw branch and instead break/return
when `newARIOBalance >= feeDetails.listingFee`, keeping the existing `tries >=
maxTries` check to throw after retries; ensure getMarketplaceBalance handling
remains the same.
- Around line 141-179: The fee calculations convert bigint mARIO results to
Number causing precision loss and use Math.round which can under-deposit; update
calculateListingFee and calculateSaleTax usage to keep results as bigint (avoid
Number(...) and .valueOf() where they yield JS Numbers), perform mARIO
arithmetic using bigint and the mARIOToken/ARIOToken helpers that accept bigint
or string, only convert to string when calling the SDK, and replace any
Math.round(...) checks (e.g., where computing required top-up using
feeDetails.listingFee - existingBalance) with Math.ceil to ensure sufficient
deposit; adjust the useMemo that computes feeDetails and the
hasSufficientListingBalance logic to compare bigint mARIO balances directly (or
convert both sides to the same bigint representation) rather than using Number
conversions.
In @src/components/pages/ViewListing/ViewListing.tsx:
- Around line 842-848: The success/error messages in the JSX using
workflowComplete, workflowError and name are buyer-focused and need to handle
the seller cancellation flow; update the conditional rendering to branch on the
seller/owner flag (e.g., isSeller or isListingOwner) and when the current user
is the seller show seller-specific texts (error: "There was an error completing
your cancellation." and success: "Your listing has been cancelled."), otherwise
keep the existing buyer messages (error: "There was an error completing your
purchase." and success: "{name} is now yours!"). Ensure you reference
workflowComplete, workflowError, name and the existing seller flag in the same
JSX block so the correct message displays for both roles.
🧹 Nitpick comments (11)
src/components/pages/ViewListing/ViewListing.tsx (6)
133-142: Optional: Remove redundant null coalescing operator.Line 135 uses
?? 0after already checking!userAssets?.balances?.balanceon line 134. The null coalescing is redundant since we know the value exists at that point.♻️ Proposed simplification
const hasSufficientBalance = useMemo(() => { if (!orderData?.price || !userAssets?.balances?.balance) return false; - const userBalance = new mARIOToken(Number(userAssets.balances.balance ?? 0)) + const userBalance = new mARIOToken(Number(userAssets.balances.balance)) .toARIO() .valueOf(); const orderPrice = new mARIOToken(Number(orderData.price)) .toARIO() .valueOf(); return userBalance >= orderPrice; }, [orderData?.price, userAssets?.balances?.balance]);
153-214: Optional: Simplify icon selection logic.The switch statement (lines 172-188) for icon selection could be replaced with a more concise object lookup pattern, reducing cognitive complexity.
♻️ Proposed simplification
const updateWorkflowSteps = useCallback( ({ step, status, description, }: { step: 'deposit' | 'buy' | 'cancel' | 'complete'; status: 'pending' | 'processing' | 'success' | 'error'; description?: string; }) => { - const DepositIcon = DollarSign; - const BuyIcon = ShoppingCart; - const CancelIcon = XIcon; - const CompleteIcon = CheckIcon; - const ErrorIcon = XIcon; - - let CurrentIcon: React.ForwardRefExoticComponent< - Omit<LucideProps, 'ref'> & React.RefAttributes<SVGSVGElement> - >; - switch (step) { - case 'deposit': - CurrentIcon = DepositIcon; - break; - case 'buy': - CurrentIcon = BuyIcon; - break; - case 'cancel': - CurrentIcon = CancelIcon; - break; - case 'complete': - CurrentIcon = CompleteIcon; - break; - default: - CurrentIcon = ErrorIcon; - break; - } + const iconMap = { + deposit: DollarSign, + buy: ShoppingCart, + cancel: XIcon, + complete: CheckIcon, + }; + const CurrentIcon = iconMap[step] ?? XIcon; const iconClass = { pending: 'w-4 h-4 text-grey', processing: 'w-4 h-4 animate-spin text-white', success: 'w-4 h-4 text-success', error: 'w-4 h-4 text-error', }[status]; const IconComponent = status === 'processing' ? ( <div className="w-4 h-4 border-2 border-white border-t-transparent rounded-full animate-spin" /> ) : ( <CurrentIcon className={iconClass} /> ); setWorkflowSteps((prev) => ({ ...prev, [step]: { ...prev[step], icon: IconComponent, description: description ?? prev[step].description, }, })); }, [], );
282-302: Consider exponential backoff for deposit confirmation polling.The polling loop uses fixed 5-second intervals (line 301). For better UX and reduced server load, consider using exponential backoff (e.g., 1s, 2s, 4s, 8s...) with a reasonable maximum interval.
♻️ Proposed improvement with exponential backoff
// Wait for deposit to be confirmed let newBalance = 0; let tries = 0; const maxTries = 10; + let backoffMs = 1000; // Start with 1 second while (newBalance < orderPrice && tries < maxTries) { try { const balanceResult = await marketplaceContract.getMarketplaceBalance({ address: walletAddress.toString(), }); newBalance = new mARIOToken(Number(balanceResult?.balance ?? 0)) .toARIO() .valueOf(); if (newBalance >= orderPrice) break; } catch (error) { console.error('Error checking balance:', error); } tries++; - await sleep(5000); + await sleep(backoffMs); + backoffMs = Math.min(backoffMs * 2, 10000); // Cap at 10 seconds }
576-581: Consider memoizing price calculations to avoid repeated conversions.Lines 577-581 convert prices on every render. Since these depend on
orderData?.priceandarioPrice, they should be memoized to avoid unnecessary recalculations.♻️ Proposed memoization
+ const priceInArio = useMemo(() => { + return orderData?.price + ? new mARIOToken(Number(orderData.price)).toARIO().valueOf() + : 0; + }, [orderData?.price]); + + const priceInUsd = useMemo(() => { + return arioPrice && typeof arioPrice === 'number' + ? priceInArio * arioPrice + : null; + }, [priceInArio, arioPrice]); - // Convert price from mARIO to ARIO - const priceInArio = orderData?.price - ? new mARIOToken(Number(orderData.price)).toARIO().valueOf() - : 0; - const priceInUsd = - arioPrice && typeof arioPrice === 'number' ? priceInArio * arioPrice : null;
675-686: Remove type assertion by improving type definitions.Line 681 uses
(domainInfo.arnsRecord as any)?.endTimestampwhich bypasses type safety. This suggests the type definition forarnsRecorddoesn't properly discriminate between lease and permabuy types.Consider defining a proper discriminated union for the arnsRecord type:
type LeaseRecord = { type: 'lease'; endTimestamp: number; // ... other lease fields }; type PermabuyRecord = { type: 'permabuy'; // ... other permabuy fields }; type ArnsRecord = LeaseRecord | PermabuyRecord;Then use a type guard:
<LeaseDurationFromEndTimestamp endTimestamp={ domainInfo?.arnsRecord?.type === 'lease' - ? (domainInfo.arnsRecord as any)?.endTimestamp + ? domainInfo.arnsRecord.endTimestamp : undefined // Permabuy domains or no record - let component show "Permanent" } />
793-825: Optional: Simplify nested ternaries in button className.Lines 794-800 use deeply nested ternaries that reduce readability. Consider extracting the className logic into a computed variable or helper function.
♻️ Proposed simplification
+ const getButtonClassName = () => { + if (isBuying || isExpired) { + return 'bg-grey text-white cursor-not-allowed'; + } + if (isUserSeller) { + return 'bg-error-thin hover:bg-error text-white'; + } + return 'bg-primary hover:bg-warning text-black'; + }; + <button - className={`w-full font-semibold py-3 px-6 rounded transition-colors flex items-center justify-center gap-2 mt-8 ${ - isBuying || isExpired - ? 'bg-grey text-white cursor-not-allowed' - : isUserSeller - ? 'bg-error-thin hover:bg-error text-white' - : 'bg-primary hover:bg-warning text-black' - }`} + className={`w-full font-semibold py-3 px-6 rounded transition-colors flex items-center justify-center gap-2 mt-8 ${getButtonClassName()}`} onClick={isUserSeller ? handleCancel : handleBuy} disabled={isBuying || isExpired} >src/components/data-display/tables/MarketplaceListingsTable.tsx (3)
133-135: Silent filtering of listings without names may hide data issues.Listings without a corresponding ArNS name are silently filtered out (line 134). While the comment indicates these are not "valid" in the system, this could mask:
- Data sync issues between the marketplace and ArNS registry
- New listings not yet indexed
- Deleted or expired names still listed
Consider logging these cases or showing a warning to help detect data inconsistencies.
218-229: Defaulting unknown order types to 'Fixed Price' may be misleading.The switch statement defaults to
'Fixed Price'for unknown or null order types (line 227). If a new order type is introduced or if the type is genuinely unknown, showing 'Fixed Price' could confuse users.Consider returning
'Unknown'or displaying the raw type value for transparency.♻️ Proposed refinement
const formatType = (type: string) => { switch (type?.toLowerCase()) { case 'fixed': return 'Fixed Price'; case 'dutch': return 'Dutch Auction'; case 'english': return 'English Auction'; default: - return type || 'Fixed Price'; + return type || 'Unknown'; } };
407-437: Consider aligningisLoadingprop with actual loading state.The component passes
isLoading={false}toTableView(line 412) even when data is loading, relying instead on custom loading UI innoDataFoundText. While this works, it's potentially confusing for maintainers who might expectisLoadingto reflect the actual loading state.If
TableViewsupports a loading state, consider passingloadingTableDatafor consistency. Otherwise, document whyisLoading={false}is used.src/components/data-display/tables/DomainsTable.tsx (1)
131-209: Watch out for per-row hook/query amplification (intent/order) in large tables.
useANTIntentanduseMarketplaceOrderare used in row-level components; React Query will dedupe, but you can still end up with a lot of subscriptions/renders for users with many names. Consider computing intents/orders once at table scope and passing derived flags down.Also applies to: 212-257, 351-433
.github/workflows/pr-preview.yaml (1)
12-21: Add explicitpermissions:with least privilege forGITHUB_TOKEN.
The workflow passesgithub-tokento thepermaweb/permaweb-deployaction without declaring explicit permissions. At minimum, addcontents: readfor checkout. If the action posts PR status updates or comments, also includepull-requests: writeand/orissues: write.
This prevents failures if repository-level token permissions are restricted, and follows security best practices.Example configuration
jobs: deploy-preview: runs-on: ubuntu-latest permissions: contents: read pull-requests: write # if permaweb-deploy posts PR updates if: github.event.pull_request.head.repo.full_name == github.repository && github.event.action != 'closed'Also applies to: 28-34
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
.github/workflows/pr-preview.yamlsrc/components/data-display/tables/DomainsTable.tsxsrc/components/data-display/tables/MarketplaceListingsTable.tsxsrc/components/modals/ListNameForSaleModal/ListNameForSaleModal.tsxsrc/components/pages/ViewListing/ViewListing.tsx
🧰 Additional context used
🧠 Learnings (5)
📚 Learning: 2025-12-11T05:20:18.825Z
Learnt from: vilenarios
Repo: ar-io/arns-react PR: 902
File: src/components/modals/turbo/panels/crypto/CryptoConfirmation.tsx:536-540
Timestamp: 2025-12-11T05:20:18.825Z
Learning: Guideline: For tokens on Base network that use 6 decimals (e.g., base-ario ERC-20), convert amount to smallest units using a 1e6 multiplier. Use Math.round(amount * 1e6) when computing the on-chain value. Do not assume 18 decimals. Apply this conversion consistently in code paths that perform payments in CryptoConfirmation.tsx or similar components, and document that the base-ario/USDC-like tokens use 6 decimals to avoid mispricing or transfer errors.
Applied to files:
src/components/data-display/tables/MarketplaceListingsTable.tsxsrc/components/pages/ViewListing/ViewListing.tsxsrc/components/data-display/tables/DomainsTable.tsxsrc/components/modals/ListNameForSaleModal/ListNameForSaleModal.tsx
📚 Learning: 2025-12-16T16:47:36.348Z
Learnt from: dtfiedler
Repo: ar-io/arns-react PR: 904
File: .github/workflows/pr-preview.yaml:52-58
Timestamp: 2025-12-16T16:47:36.348Z
Learning: In the ar-io/arns-react repository, ArNS undername cleanup for PR previews is handled by the permaweb-deploy GitHub action, not by a local script in this repository. The cleanup step should not be included in the PR preview workflow.
Applied to files:
.github/workflows/pr-preview.yaml
📚 Learning: 2025-12-02T17:38:52.889Z
Learnt from: vilenarios
Repo: ar-io/arns-react PR: 899
File: src/services/turbo/BaseTokenPurchaseService.ts:0-0
Timestamp: 2025-12-02T17:38:52.889Z
Learning: In src/services/turbo/BaseTokenPurchaseService.ts, the function executeBaseTokenPurchase intentionally only handles Base token top-up to Turbo credits, not the actual ArNS purchase. The ArNS purchase is handled separately by dispatchArIOInteraction in Checkout.tsx because it manages multiple complex purchase workflows (lease, permabuy, extend, increase undernames, etc.) with their own state management. This separation of concerns is by design.
Applied to files:
src/components/pages/ViewListing/ViewListing.tsx
📚 Learning: 2025-09-10T16:35:55.727Z
Learnt from: dtfiedler
Repo: ar-io/arns-react PR: 873
File: src/components/modals/ant-management/UpgradeDomainsModal/UpgradeDomainsModal.tsx:0-0
Timestamp: 2025-09-10T16:35:55.727Z
Learning: In arns-react, when calling buildDomainInfoQuery (src/hooks/useDomainInfo.tsx), you must pass arioProcessId along with arioContract, domain, aoNetwork, hyperbeamUrl, and antRegistryProcessId; omitting arioProcessId can lead to “No ANT id or record found” and break upgrades.
Applied to files:
src/components/pages/ViewListing/ViewListing.tsxsrc/components/data-display/tables/DomainsTable.tsxsrc/components/modals/ListNameForSaleModal/ListNameForSaleModal.tsx
📚 Learning: 2025-09-10T16:35:55.727Z
Learnt from: dtfiedler
Repo: ar-io/arns-react PR: 873
File: src/components/modals/ant-management/UpgradeDomainsModal/UpgradeDomainsModal.tsx:0-0
Timestamp: 2025-09-10T16:35:55.727Z
Learning: In arns-react, when calling buildDomainInfoQuery (src/hooks/useDomainInfo.tsx), you must pass arioProcessId along with arioContract, domain, aoNetwork, hyperbeamUrl, and antRegistryProcessId; omitting arioProcessId can lead to "No ANT id or record found" and break upgrades.
Applied to files:
src/components/pages/ViewListing/ViewListing.tsxsrc/components/data-display/tables/DomainsTable.tsxsrc/components/modals/ListNameForSaleModal/ListNameForSaleModal.tsx
🧬 Code graph analysis (3)
src/components/data-display/tables/MarketplaceListingsTable.tsx (8)
src/hooks/useArIOPrice.ts (1)
useArIoPrice(9-32)src/hooks/useArNSRecords.tsx (1)
useArNSRecords(5-40)src/hooks/useMarketplaceOrders.ts (1)
useMarketplaceOrders(72-84)src/utils/searchUtils/searchUtils.ts (2)
decodeDomainToASCII(48-52)lowerCaseDomain(132-134)src/utils/common/common.tsx (2)
formatForMaxCharCount(134-148)formatARIOWithCommas(219-223)src/services/arweave/ArweaveTransactionID.ts (1)
ArweaveTransactionID(7-31)src/types.ts (1)
AoAddress(562-562)src/components/indicators/ARIOLoadingSpinner.tsx (1)
ARIOLoadingSpinner(4-17)
src/components/pages/ViewListing/ViewListing.tsx (9)
src/hooks/useDomainInfo.tsx (1)
useDomainInfo(206-255)src/hooks/useMarketplaceOrder.tsx (1)
useMarketplaceOrder(44-56)src/hooks/useMarketplaceUserAssets.ts (1)
useMarketplaceUserAssets(44-61)src/utils/searchUtils/searchUtils.ts (2)
sleep(128-130)decodeDomainToASCII(48-52)src/utils/network.ts (1)
queryClient(39-45)src/services/arweave/ArweaveTransactionID.ts (1)
ArweaveTransactionID(7-31)src/components/data-display/AntLogoIcon.tsx (1)
AntLogoIcon(7-57)src/utils/common/common.tsx (1)
formatARIOWithCommas(219-223)src/components/data-display/VerticalTimelineStepper.tsx (1)
VerticalTimelineStepper(3-41)
src/components/data-display/tables/DomainsTable.tsx (11)
src/utils/constants.ts (1)
ARIO_DISCORD_LINK(251-251)src/components/data-display/index.tsx (1)
Tooltip(5-5)src/hooks/useANTIntent.ts (1)
useANTIntent(17-75)src/hooks/useMarketplaceOrder.tsx (1)
useMarketplaceOrder(44-56)src/hooks/useInterruptedWorkflows.ts (2)
InterruptedWorkflow(6-10)useInterruptedWorkflows(15-67)src/hooks/usePendingWorkflows.ts (2)
PendingWorkflow(6-10)usePendingWorkflows(18-91)src/hooks/useANTVersions.tsx (1)
useLatestANTVersion(35-51)src/hooks/usePrimaryName.tsx (1)
usePrimaryName(4-30)src/utils/common/common.tsx (1)
formatForMaxCharCount(134-148)src/components/icons/index.ts (1)
ExternalLinkIcon(110-110)src/components/modals/index.ts (2)
ListNameForSaleModal(15-15)UpgradeDomainForMarketplaceModal(16-16)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: deploy-preview
- GitHub Check: lint_and_build
🔇 Additional comments (11)
src/components/pages/ViewListing/ViewListing.tsx (1)
109-115: No action needed. TheuseMarketplaceOrderhook already handles undefinedantIdgracefully through theenabledflag inbuildMarketplaceOrderQuery, which includes!!antId. WhendomainInfo?.processIdis undefined during initial render, the query is disabled and no API calls are triggered. This follows React Query best practices for conditional queries.src/components/data-display/tables/MarketplaceListingsTable.tsx (6)
1-41: LGTM! Imports and type definitions are appropriate.The imports are well-organized, and the
MarketplaceListingtype definition appropriately captures the table row structure. The use ofRecord<string, any>provides flexibility for additional table properties.
45-64: LGTM! Filter logic is robust.The function correctly filters across multiple fields with proper null-safety using optional chaining.
390-405: LGTM! Error handling provides clear feedback and recovery.The error state displays a clear message and offers a retry button, giving users a way to recover from transient failures.
183-206: LGTM! Name column properly handles domain encoding and security.The column correctly decodes domains for display, normalizes them for links, and uses secure external link attributes (
noopener noreferrer).
238-267: LGTM! Price display with dual currency is well-formatted.The column appropriately shows both ARIO and USD prices with proper formatting and locale-aware number display.
126-128: Remove the redundantNumber()conversion —order.priceis already a JavaScript number.The
pricefield from the @ar.io/sdk Order type is a plain JavaScript number (numeric mARIO value). TheNumber()conversion is unnecessary. Simplify to:Suggested change
const priceInARIO = new mARIOToken(order.price ?? 0) .toARIO() .valueOf(); // Convert from mARIO to ARIO⛔ Skipped due to learnings
Learnt from: vilenarios Repo: ar-io/arns-react PR: 902 File: src/components/modals/turbo/panels/crypto/CryptoConfirmation.tsx:536-540 Timestamp: 2025-12-11T05:20:30.374Z Learning: The Base ARIO (base-ario) ERC-20 token contract on Base network uses 6 decimals, not 18 decimals. When converting base-ario amounts to smallest units for payment transactions, use 1e6 multiplier (Math.round(amount * 1e6)), consistent with USDC handling.src/components/data-display/tables/DomainsTable.tsx (1)
734-792: No issue found. ThedispatchANTUpdatecall in DomainsTable.tsx safely passesprocessIdasantId, which is sufficient to fetch ANT state without requiringarioProcessId. The missingarioProcessIdis only needed when mapping domain names to ANTs (as in UpgradeDomainsModal), not when the ANT processId is already known.src/components/modals/ListNameForSaleModal/ListNameForSaleModal.tsx (1)
22-23: Remove — the Radix Tabs import is correct.The
radix-uipackage (^1.1.3in package.json) is an official umbrella package from Radix that re-exports individual primitives including Tabs. Importing{ Tabs } from 'radix-ui'and using<Tabs.Root>and<Tabs.Content>is a valid, documented pattern. The code is already in use across multiple modals in the codebase without build issues.Likely an incorrect or invalid review comment.
.github/workflows/pr-preview.yaml (2)
12-34: Good: no local “undername cleanup” step included.
This aligns with the repo’s intended PR preview cleanup handling via the permaweb-deploy action. Based on learnings, this is the right direction.
28-34: This workflow uses a non-existent or misconfigured GitHub Action.
permaweb/permaweb-deployis not a published GitHub Action—it is an npm CLI package (permaweb-deployv2.5.1 in your package.json). Theuses: permaweb/permaweb-deploy@v0.0.2syntax will fail at runtime. The official documentation shows deploying by running the npm CLI directly in a workflow step (e.g.,npm run deploy), not as a reusable action.If
permaweb/permaweb-deployis a custom private action in your repository, it needs to be located in.github/actions/permaweb-deploy/with anaction.ymlfile defining the inputs. Otherwise, replace this step with the standard approach: run the CLI via npm or yarn instead.⛔ Skipped due to learnings
Learnt from: dtfiedler Repo: ar-io/arns-react PR: 904 File: .github/workflows/pr-preview.yaml:52-58 Timestamp: 2025-12-16T16:47:36.348Z Learning: In the ar-io/arns-react repository, ArNS undername cleanup for PR previews is handled by the permaweb-deploy GitHub action, not by a local script in this repository. The cleanup step should not be included in the PR preview workflow.
| function MarketplaceActionIcon({ | ||
| domainName, | ||
| processId, | ||
| }: { domainName: string; processId: string }) { | ||
| const { hasIntent } = useANTIntent(processId); | ||
|
|
||
| const { data: order, error: orderError } = useMarketplaceOrder({ | ||
| antId: processId, | ||
| }); | ||
|
|
||
| // If there's an intent but no order (or order fetch failed), show Activity icon | ||
| const hasOrder = order && !orderError; | ||
|
|
||
| // Error state: no intent and no order (marketplace owns ANT but nothing exists) | ||
| if (!hasIntent && !hasOrder) { | ||
| return <ErrorStateTooltip domainName={domainName} antId={processId} />; | ||
| } | ||
|
|
||
| if (hasIntent && !hasOrder) { | ||
| return ( | ||
| <Tooltip | ||
| message="Pending marketplace activity" | ||
| icon={ | ||
| <div className="flex items-center justify-center w-[18px] h-[18px] text-orange-400 hover:text-orange-300 transition-colors"> | ||
| <Activity className="w-[18px] h-[18px]" /> | ||
| </div> | ||
| } | ||
| /> | ||
| ); | ||
| } | ||
|
|
||
| // Default to marketplace store icon | ||
| return ( | ||
| <Tooltip | ||
| message="View in Marketplace" | ||
| icon={ | ||
| <Link | ||
| to={`/marketplace/names/${domainName}`} | ||
| className="flex items-center justify-center w-[18px] h-[18px] text-blue-400 hover:text-blue-300 transition-colors" | ||
| > | ||
| <StoreIcon className="w-[18px] h-[18px]" /> | ||
| </Link> | ||
| } | ||
| /> | ||
| ); | ||
| } |
There was a problem hiding this comment.
Encode/canonicalize domainName in marketplace routes.
to={/marketplace/names/${domainName}} will break on unicode/special chars and can create route ambiguity. Consider encodeURIComponent(encodeDomainToASCII(domainName)) (or whatever the marketplace route expects).
Example adjustment
- to={`/marketplace/names/${domainName}`}
+ to={`/marketplace/names/${encodeURIComponent(
+ encodeDomainToASCII(domainName),
+ )}`}Also applies to: 303-323, 248-254, 314-320
🤖 Prompt for AI Agents
In @src/components/data-display/tables/DomainsTable.tsx around lines 212 - 257,
The route construction in MarketplaceActionIcon (the Link to prop) uses raw
domainName which can break on Unicode/special chars; update all marketplace
route usages (e.g., the Link in MarketplaceActionIcon and the other similar
Link/route constructions noted) to canonicalize and URL-encode the name by
calling your domain ASCII conversion helper (e.g.,
encodeDomainToASCII(domainName)) and then encodeURIComponent(...) before
interpolating into the path so the route is safe and unambiguous; ensure you
apply the same change to every other place that builds
`/marketplace/names/${...}` using domainName.
| return ( | ||
| <Tooltip | ||
| tooltipOverrides={{ | ||
| overlayClassName: 'w-fit', | ||
| overlayInnerStyle: { width: 'fit-content' }, | ||
| }} | ||
| message={ | ||
| <span className="w-fit whitespace-nowrap text-white"> | ||
| {rowValue} | ||
| </span> | ||
| } | ||
| icon={ | ||
| <Link | ||
| className="link gap-2 w-fit whitespace-nowrap items-center" | ||
| to={`https://${encodeDomainToASCII( | ||
| row.getValue('name'), | ||
| )}.${gateway}`} | ||
| target="_blank" | ||
| > | ||
| {formatForMaxCharCount(decodeDomainToASCII(rowValue), 20)}{' '} | ||
| <ExternalLinkIcon className="size-3 fill-grey" /> | ||
| </Link> | ||
| } | ||
| <div className="flex items-center gap-2"> | ||
| <Tooltip | ||
| tooltipOverrides={{ | ||
| overlayClassName: 'w-fit', | ||
| overlayInnerStyle: { width: 'fit-content' }, | ||
| }} | ||
| message={ | ||
| <span className="w-fit whitespace-nowrap text-white"> | ||
| {rowValue} | ||
| </span> | ||
| } | ||
| icon={ | ||
| <Link | ||
| className="link gap-2 w-fit whitespace-nowrap items-center" | ||
| to={`https://${encodeDomainToASCII( | ||
| row.getValue('name'), | ||
| )}.${gateway}`} | ||
| target="_blank" | ||
| > | ||
| {formatForMaxCharCount(decodeDomainToASCII(rowValue), 20)}{' '} | ||
| <ExternalLinkIcon className="size-3 fill-grey" /> | ||
| </Link> | ||
| } | ||
| /> | ||
| </div> | ||
| ); | ||
| } |
There was a problem hiding this comment.
Avoid <Link> for external navigation; add rel="noopener noreferrer" for _blank.
This is an external URL (https://...) and uses target="_blank" without rel, which is a security risk.
Proposed fix
- icon={
- <Link
- className="link gap-2 w-fit whitespace-nowrap items-center"
- to={`https://${encodeDomainToASCII(
- row.getValue('name'),
- )}.${gateway}`}
- target="_blank"
- >
- {formatForMaxCharCount(decodeDomainToASCII(rowValue), 20)}{' '}
- <ExternalLinkIcon className="size-3 fill-grey" />
- </Link>
- }
+ icon={
+ <a
+ className="link gap-2 w-fit whitespace-nowrap items-center"
+ href={`https://${encodeDomainToASCII(
+ row.getValue('name'),
+ )}.${gateway}`}
+ target="_blank"
+ rel="noopener noreferrer"
+ >
+ {formatForMaxCharCount(decodeDomainToASCII(rowValue), 20)}{' '}
+ <ExternalLinkIcon className="size-3 fill-grey" />
+ </a>
+ }📝 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.
| return ( | |
| <Tooltip | |
| tooltipOverrides={{ | |
| overlayClassName: 'w-fit', | |
| overlayInnerStyle: { width: 'fit-content' }, | |
| }} | |
| message={ | |
| <span className="w-fit whitespace-nowrap text-white"> | |
| {rowValue} | |
| </span> | |
| } | |
| icon={ | |
| <Link | |
| className="link gap-2 w-fit whitespace-nowrap items-center" | |
| to={`https://${encodeDomainToASCII( | |
| row.getValue('name'), | |
| )}.${gateway}`} | |
| target="_blank" | |
| > | |
| {formatForMaxCharCount(decodeDomainToASCII(rowValue), 20)}{' '} | |
| <ExternalLinkIcon className="size-3 fill-grey" /> | |
| </Link> | |
| } | |
| <div className="flex items-center gap-2"> | |
| <Tooltip | |
| tooltipOverrides={{ | |
| overlayClassName: 'w-fit', | |
| overlayInnerStyle: { width: 'fit-content' }, | |
| }} | |
| message={ | |
| <span className="w-fit whitespace-nowrap text-white"> | |
| {rowValue} | |
| </span> | |
| } | |
| icon={ | |
| <Link | |
| className="link gap-2 w-fit whitespace-nowrap items-center" | |
| to={`https://${encodeDomainToASCII( | |
| row.getValue('name'), | |
| )}.${gateway}`} | |
| target="_blank" | |
| > | |
| {formatForMaxCharCount(decodeDomainToASCII(rowValue), 20)}{' '} | |
| <ExternalLinkIcon className="size-3 fill-grey" /> | |
| </Link> | |
| } | |
| /> | |
| </div> | |
| ); | |
| } | |
| return ( | |
| <div className="flex items-center gap-2"> | |
| <Tooltip | |
| tooltipOverrides={{ | |
| overlayClassName: 'w-fit', | |
| overlayInnerStyle: { width: 'fit-content' }, | |
| }} | |
| message={ | |
| <span className="w-fit whitespace-nowrap text-white"> | |
| {rowValue} | |
| </span> | |
| } | |
| icon={ | |
| <a | |
| className="link gap-2 w-fit whitespace-nowrap items-center" | |
| href={`https://${encodeDomainToASCII( | |
| row.getValue('name'), | |
| )}.${gateway}`} | |
| target="_blank" | |
| rel="noopener noreferrer" | |
| > | |
| {formatForMaxCharCount(decodeDomainToASCII(rowValue), 20)}{' '} | |
| <ExternalLinkIcon className="size-3 fill-grey" /> | |
| </a> | |
| } | |
| /> | |
| </div> | |
| ); |
🤖 Prompt for AI Agents
In @src/components/data-display/tables/DomainsTable.tsx around lines 671 - 698,
The Tooltip's icon currently uses the router Link for an external URL
(to={`https://${encodeDomainToASCII(row.getValue('name'))}.${gateway}`}) with
target="_blank" which is unsafe; replace the Link with a plain anchor element
(<a>) for external navigation or ensure the component supports an explicit rel
prop, and add rel="noopener noreferrer" alongside target="_blank", preserving
the existing classes, content
(formatForMaxCharCount(decodeDomainToASCII(rowValue), 20) and <ExternalLinkIcon
className="size-3 fill-grey" />), so the external link in DomainsTable (inside
the Tooltip icon) opens in a new tab securely.
| } = useMarketplaceOrders({ | ||
| limit: 100, | ||
| sortBy: 'createdAt', | ||
| sortOrder: 'desc', | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find and examine the useMarketplaceOrders hook implementation
fd -t f 'useMarketplaceOrders' src/ --exec cat -n {} \;Repository: ar-io/arns-react
Length of output: 2987
🏁 Script executed:
# Also search for the file more broadly
find src -name "*arketplace*" -o -name "*useMarketplace*" 2>/dev/null | head -20Repository: ar-io/arns-react
Length of output: 830
🏁 Script executed:
# Check for pagination patterns in the codebase related to marketplace orders
rg -n -A 5 -B 2 'useMarketplaceOrders' src/ | head -60Repository: ar-io/arns-react
Length of output: 2174
Clarify the intent of the 100-item limit.
The useMarketplaceOrders hook already implements pagination with cursor-based fetching and can accumulate results across multiple pages. However, the component explicitly limits results to 100 items. If this limit is for performance or UX reasons, document it; otherwise, consider removing it to display all marketplace listings.
🤖 Prompt for AI Agents
In @src/components/data-display/tables/MarketplaceListingsTable.tsx around lines
90 - 94, The MarketplaceListingsTable currently passes a hard-coded limit: 100
to useMarketplaceOrders which prevents the hook's cursor-based pagination from
returning more than 100 accumulated items; either remove the explicit limit
argument so the hook can default to full pagination, or keep the limit but
document the UX/performance rationale near the MarketplaceListingsTable (e.g., a
comment above the useMarketplaceOrders call) and consider exposing the limit as
a configurable prop (e.g., marketplaceLimit) so it’s clear and adjustable.
| // Don't show the modal if there's a pending intent | ||
| if (hasPendingIntent) { | ||
| return null; // The intent should be handled through the domains table workflow | ||
| } |
There was a problem hiding this comment.
Don’t silently return null on pending intent; close the modal or show a “continue workflow” path.
Right now the user can click “List for Sale”, nothing appears, and the parent can remain in “show modal” state. Suggest: render a small message with a CTA to open ContinueWorkflowModal, or call onClose() and emit a toast/event.
🤖 Prompt for AI Agents
In @src/components/modals/ListNameForSaleModal/ListNameForSaleModal.tsx around
lines 615 - 618, In the ListNameForSaleModal component, don’t silently return
null when hasPendingIntent is true; instead either call onClose() and emit a
toast/event or render a small fallback UI that guides the user to continue the
existing flow (e.g., show a short message and a CTA that opens
ContinueWorkflowModal). Replace the early-return block that checks
hasPendingIntent with logic that: (a) invokes the existing onClose() prop and
dispatches a toast/event if you want to auto-close, or (b) renders a compact
message + button that sets a local state (e.g., showContinueWorkflow) or calls
the handler to open ContinueWorkflowModal so the user can resume; ensure you
reuse the onClose, hasPendingIntent, and ContinueWorkflowModal symbols so
behavior integrates with parent state.
| <div className="text-sm text-light-grey flex flex-col gap-2 p-2"> | ||
| List your ArNS name on the marketplace for other users to | ||
| purchase. You will retain ownership until the sale is | ||
| completed. | ||
| <Link | ||
| className="underline w-full flex text-link justify-end" | ||
| to="https://docs.ar.io/build/guides/arns-marketplace" | ||
| target="_blank" | ||
| > | ||
| Learn more | ||
| </Link> | ||
| </div> |
There was a problem hiding this comment.
Use <a> (not react-router <Link>) for external docs URLs; include rel.
This is an external https://docs... link and should be a normal anchor with target="_blank" rel="noopener noreferrer".
🤖 Prompt for AI Agents
In @src/components/modals/ListNameForSaleModal/ListNameForSaleModal.tsx around
lines 634 - 645, In the ListNameForSaleModal JSX block, the external docs link
is using react-router's Link; replace that Link usage with a plain anchor
element: use <a> with href="https://docs.ar.io/build/guides/arns-marketplace",
keep the same className and target="_blank", and add rel="noopener noreferrer"
so external navigation is safe; ensure you remove/stop using the Link's to prop
in this instance and do not import or use react-router Link for this external
URL.
| const result = await marketplaceContract.cancelOrder(orderData.id); | ||
|
|
||
| // Wait for the cancellation to be processed/cranked | ||
| await sleep(10_000); | ||
|
|
||
| updateWorkflowSteps({ | ||
| step: 'cancel', | ||
| status: 'success', | ||
| description: 'Listing cancelled', |
There was a problem hiding this comment.
Replace hard-coded wait with polling to verify cancellation.
Line 441 uses a fixed 10-second wait (sleep(10_000)) without verifying that the cancellation actually completed. If the cancellation takes longer than 10 seconds, the success message could display prematurely, and subsequent query invalidation might not reflect the updated state.
🔄 Recommended fix with polling verification
Similar to the deposit flow in handleBuy, add polling to verify the order is actually cancelled:
const result = await marketplaceContract.cancelOrder(orderData.id);
- // Wait for the cancellation to be processed/cranked
- await sleep(10_000);
+ // Poll to verify the order is cancelled
+ let orderStillExists = true;
+ let tries = 0;
+ const maxTries = 10;
+ let backoffMs = 1000;
+ while (orderStillExists && tries < maxTries) {
+ try {
+ const checkOrder = await marketplaceContract.getOrder({
+ antId: domainInfo.processId,
+ });
+ if (!checkOrder) {
+ orderStillExists = false;
+ break;
+ }
+ } catch (error) {
+ // Order not found is expected after cancellation
+ orderStillExists = false;
+ break;
+ }
+ tries++;
+ await sleep(backoffMs);
+ backoffMs = Math.min(backoffMs * 2, 10000);
+ }
+
+ if (orderStillExists) {
+ throw new Error('Failed to verify order cancellation');
+ }
updateWorkflowSteps({
step: 'cancel',
status: 'success',
description: 'Listing cancelled',
});…ddresses and safe ario formatting
fix(marketplace): bug fixes
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/components/pages/ViewListing/ViewListing.tsx`:
- Around line 131-135: The isExpired memo (computed from
orderData?.expirationTime) won't flip when time passes; add a currentTime state
(useState) and a useEffect that, when orderData?.expirationTime exists, sets a
timeout for timeUntilExpiry = orderData.expirationTime - Date.now() to update
currentTime (and clear the timer on cleanup) so the component re-renders exactly
when the listing expires; then include currentTime in the dependency array for
useMemo that computes isExpired so isExpired updates immediately when the timer
fires.
♻️ Duplicate comments (16)
src/components/pages/ViewListing/ViewListing.tsx (8)
248-255: Type assertionarioContract as AoARIOWritebypasses type safety.The assertion at Line 254 could mask type mismatches. If
arioContractdoesn't fully satisfyAoARIOWrite, this could cause runtime errors. The same pattern is repeated inhandleCancelat Line 433.Consider adding runtime validation before the assertion:
+ if (!arioContract || typeof (arioContract as AoARIOWrite).depositArIO !== 'function') { + throw new Error('Invalid ARIO contract instance'); + } const marketplaceContract = new ArNSMarketplaceWrite({ process: new AOProcess({ processId: marketplaceProcessId, ao: aoClient, }), signer: createAoSigner(wallet.contractSigner), ario: arioContract as AoARIOWrite, });
287-307: Balance polling loop lacks cleanup and has hardcoded parameters.Two issues with this polling loop:
Memory leak risk: No abort mechanism if the component unmounts during polling. The loop will continue and may attempt state updates on an unmounted component.
Hardcoded timeout: 10 tries × 5 seconds = 50 seconds max, which may be insufficient during network congestion.
🔧 Suggested improvement with AbortController
Consider extracting polling to a hook or adding an abort signal:
async function handleBuy() { + const abortController = new AbortController(); // ... validation ... while (newBalance < orderPrice && tries < maxTries) { + if (abortController.signal.aborted) { + throw new Error('Purchase cancelled'); + } // ... polling logic ... tries++; - await sleep(5000); + await sleep(5000); // Consider exponential backoff }Also consider increasing
maxTriesto 20 for better resilience on slow networks.
383-390:ArweaveTransactionIDconstructor can throw in success message.If
result.idis malformed,new ArweaveTransactionID(result.id)will throw, preventing the success message from displaying. This degrades user experience after a successful purchase.🔧 Proposed defensive rendering
<span>View transaction on aolink </span> - <ArweaveID - id={new ArweaveTransactionID(result.id)} - type={ArweaveIdTypes.INTERACTION} - characterCount={8} - shouldLink={true} - /> + {(() => { + try { + return ( + <ArweaveID + id={new ArweaveTransactionID(result.id)} + type={ArweaveIdTypes.INTERACTION} + characterCount={8} + shouldLink={true} + /> + ); + } catch { + return <span className="font-mono">{result.id}</span>; + } + })()}
443-446: Replace blind sleep with polling verification.Unlike
handleBuywhich polls to confirm the deposit,handleCanceluses a fixed 10-second sleep without verifying that the cancellation was actually processed. If cancellation takes longer, the success message displays prematurely.🔧 Suggested polling verification
const result = await marketplaceContract.cancelOrder(orderData.id); - // Wait for the cancellation to be processed/cranked - await sleep(10_000); + // Poll to verify the order is cancelled + let orderStillExists = true; + let tries = 0; + const maxTries = 12; + while (orderStillExists && tries < maxTries) { + try { + const checkOrder = await marketplaceContract.getOrder?.({ + antId: domainInfo.processId, + }); + if (!checkOrder) { + orderStillExists = false; + break; + } + } catch { + // Order not found is expected after cancellation + orderStillExists = false; + break; + } + tries++; + await sleep(5000); + } updateWorkflowSteps({
649-660:ArweaveTransactionIDconstructor can still throw for invalid addresses.The conditional checks
isEthAddressbut iforderData.creatoris neither a valid ETH address nor a valid Arweave transaction ID, theArweaveTransactionIDconstructor will throw and crash the component.🔧 Add validation before construction
<ArweaveID id={ isEthAddress(orderData.creator) ? orderData.creator - : new ArweaveTransactionID(orderData.creator) + : isArweaveTransactionID(orderData.creator) + ? new ArweaveTransactionID(orderData.creator) + : orderData.creator } type={ArweaveIdTypes.ADDRESS} - shouldLink={isArweaveTransactionID( - orderData.creator, - )} + shouldLink={isArweaveTransactionID(orderData.creator)} characterCount={12} />
694-701: Avoidas anytype assertion for lease record.Using
(domainInfo.arnsRecord as any)?.endTimestampbypasses TypeScript's type checking. This could silently break if the lease type structure changes.🔧 Use proper type guard
<LeaseDurationFromEndTimestamp endTimestamp={ domainInfo?.arnsRecord?.type === 'lease' - ? (domainInfo.arnsRecord as any)?.endTimestamp + ? 'endTimestamp' in domainInfo.arnsRecord + ? (domainInfo.arnsRecord as { endTimestamp: number }).endTimestamp + : undefined : undefined } />
858-864: Status messages don't account for seller's cancellation workflow.The messages are buyer-centric but this panel is also shown to sellers cancelling their listing:
- Line 861: "There was an error completing your purchase" should differ for sellers
- Line 862:
${name} is now yours!doesn't apply when a seller cancels- Line 863: "process your purchase" should say "process your cancellation" for sellers
🔧 Make messages context-aware
<p className="text-sm text-grey"> {workflowComplete ? workflowError - ? 'There was an error completing your purchase.' - : `${name} is now yours!` - : 'Please wait while we process your purchase. Do not close this page.'} + ? isUserSeller + ? 'There was an error cancelling your listing.' + : 'There was an error completing your purchase.' + : isUserSeller + ? 'Your listing has been removed from the marketplace.' + : `${name} is now yours!` + : isUserSeller + ? 'Please wait while we process your cancellation. Do not close this page.' + : 'Please wait while we process your purchase. Do not close this page.'} </p>
787-800: Order ID rendering can throw iforderData.idis malformed.Similar to other
ArweaveTransactionIDusages, this can crash rendering if the ID is invalid.🔧 Add defensive check
{orderData?.id && ( <div className="flex justify-between"> <span className="text-grey whitespace-nowrap"> Order ID: </span> <div className="flex justify-end"> + {isArweaveTransactionID(orderData.id) ? ( <ArweaveID id={new ArweaveTransactionID(orderData.id)} type={ArweaveIdTypes.INTERACTION} characterCount={12} shouldLink={true} /> + ) : ( + <span className="font-mono text-white">{orderData.id}</span> + )} </div> </div> )}src/components/modals/ContinueWorkflowModal/ContinueWorkflowModal.tsx (1)
183-194: Fix infinite retry loop in PUSH_INTENT path.
retriesnever increments and the loop never exits, which can freeze the UI.🔧 Proposed fix
- const retries = 0; + let retries = 0; const maxRetries = 10; while (retries < maxRetries) { try { const _res = await marketplaceContract.getUserAssets({ address: walletAddress.toString(), arioProcessId, }); + break; } catch { console.error('Issue retrieving marketplace assets'); + retries++; + await new Promise((resolve) => setTimeout(resolve, 2000)); } } + if (retries >= maxRetries) { + throw new Error('Failed to refresh marketplace assets after push.'); + }src/components/data-display/tables/DomainsTable.tsx (3)
217-228: Avoid error-state icon while intent/order queries are loading.
useANTIntent/useMarketplaceOrdercan return undefined during load, causing the error tooltip to flash prematurely.✅ Suggested guard
- const { hasIntent } = useANTIntent(processId); + const { hasIntent, isLoading: intentLoading } = useANTIntent(processId); - const { data: order, error: orderError } = useMarketplaceOrder({ + const { data: order, error: orderError, isLoading: orderLoading } = + useMarketplaceOrder({ antId: processId, }); + if (intentLoading || orderLoading) { + return null; + }
689-698: Use anchor +relfor external gateway link.
Linkis for internal routing; for external URLs withtarget="_blank", use<a>and addrel="noopener noreferrer".🔒 Proposed fix
- icon={ - <Link - className="link gap-2 w-fit whitespace-nowrap items-center" - to={`https://${encodeDomainToASCII( - row.getValue('name'), - )}.${gateway}`} - target="_blank" - > + icon={ + <a + className="link gap-2 w-fit whitespace-nowrap items-center" + href={`https://${encodeDomainToASCII( + row.getValue('name'), + )}.${gateway}`} + target="_blank" + rel="noopener noreferrer" + > {formatForMaxCharCount(decodeDomainToASCII(rowValue), 20)}{' '} <ExternalLinkIcon className="size-3 fill-grey" /> - </Link> + </a> }
249-252: Encode domain names in marketplace routes.Raw
domainNamecan include unicode/special chars and break routing. Use ASCII conversion +encodeURIComponentfor all/marketplace/names/...routes.🧭 Example fix
- to={`/marketplace/names/${domainName}`} + to={`/marketplace/names/${encodeURIComponent( + encodeDomainToASCII(domainName), + )}`}Also applies to: 316-318
src/components/layout/Navbar/NotificationMenu/NotificationMenu.tsx (2)
182-193: Use per-domain undername limits (not hardcoded 10,000).This diverges from
createNamesExceedingUndernameLimitNotificationand can miscount.🔧 Suggested fix
- if ( - ant?.state?.Records && - Object.keys(ant.state.Records).length > 10000 - ) { + const undernameCount = Object.keys( + ant?.state?.Records ?? {}, + ).filter((key) => key !== '@').length; + if (undernameCount > record.undernameLimit) { acc++; }
320-323: Don’t string‑matchReactNodeto detect “Interrupted”.
toString()on React elements yields[object Object], so this check won’t work reliably. Add a discriminant (e.g.,kind) toNotification.✅ Example approach
export type Notification = { type: 'success' | 'warning' | 'error'; message: ReactNode; link: string; + kind?: 'interrupted' | 'expiring' | 'undername-limit' | 'update-required'; }; // when creating: - notificationList.push({ + notificationList.push({ type: 'error', + kind: 'interrupted', message: ( ... ), link: '/manage/names', }); // when rendering: - ) : notification.type === 'error' && - notification.message - ?.toString() - .includes('Interrupted') ? ( + ) : notification.type === 'error' && + notification.kind === 'interrupted' ? (Also applies to: 339-357
src/components/data-display/tables/MarketplaceListingsTable.tsx (2)
121-124: Table remains empty until ArNS records load.The early return on line 124 means no listings appear until
arnsRecordsfinishes loading, even if marketplace orders are available. Since the ArNS records query has a 4-hour stale time and fetches all records with pagination, initial loads could leave users staring at an empty table.Consider either:
- Showing a dedicated loading state for the name resolution phase
- Displaying listings with a placeholder (e.g., "Resolving name...") until names are available
278-293: Seller column can still throw on invalid IDs.When
selleris neither a valid Ethereum address nor a valid Arweave transaction ID,new ArweaveTransactionID(seller)will throw and crash the table. Unlike the ANT column (line 305), there's no validation before construction.🛡️ Proposed fix with validation
cell: ({ row }) => { const seller = row.original.seller; if (!seller) return 'N/A'; + if (isEthAddress(seller)) { + return ( + <ArweaveID + id={seller} + type={ArweaveIdTypes.ADDRESS} + shouldLink={false} + characterCount={8} + /> + ); + } + + if (!isArweaveTransactionID(seller)) { + return <span className="text-grey text-xs">{seller.slice(0, 8)}...</span>; + } + return ( <ArweaveID - id={ - isEthAddress(seller) ? seller : new ArweaveTransactionID(seller) - } + id={new ArweaveTransactionID(seller)} type={ArweaveIdTypes.ADDRESS} - shouldLink={isArweaveTransactionID(seller)} + shouldLink characterCount={8} /> ); },
🧹 Nitpick comments (2)
src/components/data-display/tables/MarketplaceListingsTable.tsx (2)
79-80: Remove unused state variable.
internalFilteris declared but never updated (the setter is not destructured), and the ternary on line 80 is always false sinceexternalFilterdefaults to''. This is dead code.♻️ Suggested simplification
function MarketplaceListingsTable({ filter: externalFilter = '', refreshTrigger, }: MarketplaceListingsTableProps = {}) { const [{ arioTicker, gateway }] = useGlobalState(); const { data: arIoPrice } = useArIoPrice(); - const [internalFilter] = useState(''); - const filter = externalFilter !== undefined ? externalFilter : internalFilter; + const filter = externalFilter; const { data: arnsRecords } = useArNSRecords();
416-445: Consider adding a comment explaining the loading pattern.The
isLoading={false}with loading state handled vianoDataFoundTextworks but is non-obvious. A brief comment would help future maintainers understand the intentional design choice.return ( <div> + {/* Loading state is handled via noDataFoundText rather than TableView's isLoading prop */} <TableView columns={columns as ColumnDef<MarketplaceListing>[]} data={loadingTableData ? [] : filteredData} isLoading={false}
| // Check if the order is expired | ||
| const isExpired = useMemo(() => { | ||
| if (!orderData?.expirationTime) return false; | ||
| return Date.now() >= orderData.expirationTime; | ||
| }, [orderData?.expirationTime]); |
There was a problem hiding this comment.
isExpired won't update if listing expires while user is viewing.
The isExpired memo only recalculates when orderData?.expirationTime changes. If a user stays on the page and the listing expires, the UI won't reflect this until a re-render is triggered. The user could attempt to purchase an expired listing.
🔧 Consider adding a timer to check expiration
const [currentTime, setCurrentTime] = useState(Date.now());
useEffect(() => {
if (!orderData?.expirationTime) return;
const timeUntilExpiry = orderData.expirationTime - Date.now();
if (timeUntilExpiry <= 0) return;
const timer = setTimeout(() => {
setCurrentTime(Date.now());
}, timeUntilExpiry);
return () => clearTimeout(timer);
}, [orderData?.expirationTime]);
const isExpired = useMemo(() => {
if (!orderData?.expirationTime) return false;
return currentTime >= orderData.expirationTime;
}, [orderData?.expirationTime, currentTime]);🤖 Prompt for AI Agents
In `@src/components/pages/ViewListing/ViewListing.tsx` around lines 131 - 135, The
isExpired memo (computed from orderData?.expirationTime) won't flip when time
passes; add a currentTime state (useState) and a useEffect that, when
orderData?.expirationTime exists, sets a timeout for timeUntilExpiry =
orderData.expirationTime - Date.now() to update currentTime (and clear the timer
on cleanup) so the component re-renders exactly when the listing expires; then
include currentTime in the dependency array for useMemo that computes isExpired
so isExpired updates immediately when the timer fires.
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@src/components/modals/ContinueWorkflowModal/ContinueWorkflowModal.tsx`:
- Around line 154-155: The code uses a loosely-typed variable result (from the
switch on workflowType) and calls setTransactionId(result.id) and constructs an
ArweaveTransactionID without ensuring result.id exists; update the handling in
ContinueWorkflowModal so after each API call that assigns to result you validate
that result is an object with a non-empty id (e.g., typeof result === 'object'
&& result !== null && typeof result.id === 'string' && result.id.trim() !== ''),
and if the check fails log the error, show a user-facing
error/throw/early-return instead of calling setTransactionId or constructing
ArweaveTransactionID; also narrow the type of result (avoid any) and apply the
same validation before the other usage sites mentioned (the second occurrence
around setTransactionId and ArweaveTransactionID).
In `@src/components/modals/ListNameForSaleModal/panels/ConfirmListingPanel.tsx`:
- Around line 292-308: The icon-only Popover.Trigger button (wrapped by
Popover.Trigger and rendering InfoIcon) lacks an accessible label; update the
button element used with Popover.Trigger to include an appropriate aria-label
(e.g., aria-label="More info" or context-specific text) and optionally a title
attribute, keeping existing handlers (onMouseEnter/onMouseLeave) and state hooks
(isPopoverOpen, setIsPopoverOpen) intact so screen readers can announce the
control; ensure the label describes the popover’s purpose and matches the
InfoIcon’s intent.
- Around line 74-81: The "Current Owner" entry in orderSummary can render
"undefined...undefined" when walletAddress is missing; update the logic in
ConfirmListingPanel where orderSummary is built so "Current Owner" prefers
domainInfo?.owner if present, otherwise formats walletAddress safely (only slice
when walletAddress exists) and falls back to a sensible placeholder like
"Unknown owner"; reference the orderSummary object, walletAddress,
domainInfo?.owner, getListingTypeLabel, domainName, and expirationDate to locate
and adjust the value construction.
- Around line 46-49: The useDomainInfo call currently only passes antId (or
undefined) which disables the query when antId is invalid; update the call to
pass domainName as a fallback so the hook can perform lookup by domain when
antId is not a valid Arweave transaction ID—i.e., call useDomainInfo with an
object containing antId: isArweaveTransactionID(antId) ? antId : undefined and
domain: domainName (or similar property accepted by useDomainInfo) so the hook
receives domainName as fallback; adjust references to antId, domainName,
useDomainInfo, and isArweaveTransactionID accordingly.
🧹 Nitpick comments (4)
src/components/modals/ContinueWorkflowModal/ContinueWorkflowModal.tsx (2)
109-113: Incomplete dutch auction configuration.
decrementAmountis hardcoded toundefinedwith a comment indicating it needs calculation. For dutch auctions, this value is typically essential for displaying how much the price decreases per interval. Consider calculating it or verifying thatMarketplaceOrderInfoCardhandles a missingdecrementAmountgracefully.🛠️ Possible calculation approach
If you have
startingPrice,reservePrice, andexpirationTime, you could calculate:decrementAmount: orderParams.price && orderParams.minimumPrice && orderParams.decreaseInterval ? ((new mARIOToken(Number(orderParams.price)).toARIO().valueOf() - new mARIOToken(Number(orderParams.minimumPrice)).toARIO().valueOf()) / (/* number of intervals until expiration */)) : undefined,
151-151: Avoidas anycast.The
as anycast bypasses type checking. If there's a genuine type mismatch betweenarioContractand whatArNSMarketplaceWriteexpects, consider using a more specific type assertion or updating the types to align properly.src/components/modals/ListNameForSaleModal/panels/ConfirmListingPanel.tsx (1)
248-255: Use stable keys for list items.Index keys can cause stale UI when items reorder or change. Use the label for
orderSummaryand add a stableidforallListingEntries.Proposed fix
- {Object.entries(orderSummary).map(([key, value], i) => ( - <div className="flex flex-row w-full text-sm" key={i}> + {Object.entries(orderSummary).map(([key, value]) => ( + <div className="flex flex-row w-full text-sm" key={key}> <span className="flex w-1/3 whitespace-nowrap text-grey"> {key} </span> <span className="flex w-2/3 whitespace-nowrap">{value}</span> </div> ))}- const allListingEntries = [ + const allListingEntries = [ { + id: 'listing-price', key: 'Listing Price', value: ( <div className="flex flex-col items-end"> <span> {formatARIOWithCommas(listingPrice)} {arioTicker} </span> {arIoPrice && ( <span className="text-xs text-grey"> {formatUsdValue(listingPrice)} </span> )} </div> ), }, { + id: 'listing-fee', key: ( <div className="flex items-center gap-1"> <span>Listing Fee</span> <Tooltip message={`1 ${arioTicker} per hour the listing is live (paid upfront)`} icon={ <InfoIcon style={{ fontSize: '14px', fill: 'var(--text-grey)', width: '14px', cursor: 'pointer', }} /> } tooltipOverrides={{ overlayInnerStyle: { padding: '12px', }, }} /> </div> ), value: ( <div className="flex flex-col items-end"> <span> {formatARIOWithCommas(listingFee)} {arioTicker} </span> {arIoPrice && ( <span className="text-xs text-grey"> {formatUsdValue(listingFee)} </span> )} </div> ), }, { + id: 'marketplace-fee', key: ( <div className="flex items-center gap-1"> <span>Marketplace Fee</span> <Tooltip message="5% fee on the sale price" icon={ <InfoIcon style={{ fontSize: '14px', fill: 'var(--text-grey)', width: '14px', cursor: 'pointer', }} /> } tooltipOverrides={{ overlayInnerStyle: { padding: '12px', }, }} /> </div> ), value: ( <div className="flex flex-col items-end"> <span> {formatARIOWithCommas(saleTax)} {arioTicker} </span> {arIoPrice && ( <span className="text-xs text-grey">{formatUsdValue(saleTax)}</span> )} </div> ), }, { + id: 'you-will-receive', key: 'You will receive (after sale)', value: ( <div className="flex flex-col items-end"> <span className="text-white font-medium"> {formatARIOWithCommas(youWillReceive)} {arioTicker} </span> {arIoPrice && ( <span className="text-xs text-grey"> {formatUsdValue(youWillReceive)} </span> )} </div> ), }, ];- {allListingEntries.map((entry, i) => ( + {allListingEntries.map((entry) => ( <div className="flex flex-row w-full text-sm justify-between" - key={i} + key={entry.id} >Also applies to: 261-273
src/components/inputs/DateTimePicker/DateTimePicker.tsx (1)
123-129: Calendar month can drift ifselectedchanges externally while the popover is open.When a user navigates the calendar months and the parent component updates
selected, thecurrentMonthstate won't automatically follow. Consider syncing them with auseEffectso the calendar month aligns with the currently selected date.♻️ Optional sync for
currentMonthconst Calendar = ({ selected, onSelect, }: { selected?: Date; onSelect: (date: Date | undefined) => void }) => { const [currentMonth, setCurrentMonth] = useState(selected || new Date()); + + useEffect(() => { + if (selected) setCurrentMonth(selected); + }, [selected]);Note: Add
useEffectto the imports at the top of the file.
| let result: any; | ||
| switch (workflowType) { |
There was a problem hiding this comment.
Add validation for result.id before use.
result is typed as any and used without validation. If the API response is malformed or missing the id property, setTransactionId(result.id) and the subsequent ArweaveTransactionID construction at line 226 will fail or behave unexpectedly.
🛠️ Suggested validation
}
+ if (!result?.id) {
+ throw new Error('Transaction result missing ID');
+ }
+
setTransactionId(result.id);
setIsComplete(true);Also applies to: 207-208
🤖 Prompt for AI Agents
In `@src/components/modals/ContinueWorkflowModal/ContinueWorkflowModal.tsx` around
lines 154 - 155, The code uses a loosely-typed variable result (from the switch
on workflowType) and calls setTransactionId(result.id) and constructs an
ArweaveTransactionID without ensuring result.id exists; update the handling in
ContinueWorkflowModal so after each API call that assigns to result you validate
that result is an object with a non-empty id (e.g., typeof result === 'object'
&& result !== null && typeof result.id === 'string' && result.id.trim() !== ''),
and if the check fails log the error, show a user-facing
error/throw/early-return instead of calling setTransactionId or constructing
ArweaveTransactionID; also narrow the type of result (avoid any) and apply the
same validation before the other usage sites mentioned (the second occurrence
around setTransactionId and ArweaveTransactionID).
| const [{ walletAddress }] = useWalletState(); | ||
| const { data: domainInfo } = useDomainInfo({ | ||
| antId: isArweaveTransactionID(antId) ? antId : undefined, | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's find the useDomainInfo hook definition
fd -t tsx -t ts -H "useDomainInfo" | head -20Repository: ar-io/arns-react
Length of output: 230
🏁 Script executed:
# Check the useDomainInfo hook implementation
find . -name "*useDomainInfo*" -type fRepository: ar-io/arns-react
Length of output: 89
🏁 Script executed:
# Search for useDomainInfo calls to understand its signature and typical usage
rg -n "useDomainInfo\(" --type tsx --type ts -B2 -A3Repository: ar-io/arns-react
Length of output: 87
🏁 Script executed:
# Read the useDomainInfo hook definition
cat -n ./src/hooks/useDomainInfo.tsxRepository: ar-io/arns-react
Length of output: 8624
🏁 Script executed:
# Search for useDomainInfo calls in the codebase
rg "useDomainInfo" -B2 -A3 | head -100Repository: ar-io/arns-react
Length of output: 7483
🏁 Script executed:
# Find where ConfirmListingPanel is used
rg "ConfirmListingPanel" -B2 -A2Repository: ar-io/arns-react
Length of output: 3595
🏁 Script executed:
# Check how antId is provided to ConfirmListingPanel
cat -n ./src/components/modals/ListNameForSaleModal/ListNameForSaleModal.tsx | head -100Repository: ar-io/arns-react
Length of output: 3349
🏁 Script executed:
# Get more context around the ConfirmListingPanel call
rg -n "ConfirmListingPanel" -B5 -A5 ./src/components/modals/ListNameForSaleModal/ListNameForSaleModal.tsxRepository: ar-io/arns-react
Length of output: 1153
Pass domainName to useDomainInfo as a fallback.
antId is optional but used as the sole query parameter. If antId is not a valid Arweave transaction ID, useDomainInfo receives { antId: undefined } and the query is disabled despite domainName being available. This pattern is used elsewhere in the codebase (e.g., MarketplaceOrderInfoCard, ViewListing, ManageDomain) which pass domain to enable fallback lookup.
Proposed fix
const { data: domainInfo } = useDomainInfo({
+ domain: domainName,
antId: isArweaveTransactionID(antId) ? antId : undefined,
});🤖 Prompt for AI Agents
In `@src/components/modals/ListNameForSaleModal/panels/ConfirmListingPanel.tsx`
around lines 46 - 49, The useDomainInfo call currently only passes antId (or
undefined) which disables the query when antId is invalid; update the call to
pass domainName as a fallback so the hook can perform lookup by domain when
antId is not a valid Arweave transaction ID—i.e., call useDomainInfo with an
object containing antId: isArweaveTransactionID(antId) ? antId : undefined and
domain: domainName (or similar property accepted by useDomainInfo) so the hook
receives domainName as fallback; adjust references to antId, domainName,
useDomainInfo, and isArweaveTransactionID accordingly.
| const orderSummary: Record<string, ReactNode> = { | ||
| 'Listing Type': getListingTypeLabel(), | ||
| 'Domain Name': domainName, | ||
| 'Current Owner': | ||
| walletAddress?.toString().slice(0, 8) + | ||
| '...' + | ||
| walletAddress?.toString().slice(-8), | ||
| Expires: expirationDate |
There was a problem hiding this comment.
Avoid “undefined...undefined” in Current Owner; prefer actual owner.
If walletAddress is missing, the current concatenation renders "undefined...undefined". Also, domainInfo?.owner is a more accurate source when available.
Proposed fix
- const orderSummary: Record<string, ReactNode> = {
+ const ownerAddress = domainInfo?.owner ?? walletAddress?.toString();
+ const orderSummary: Record<string, ReactNode> = {
'Listing Type': getListingTypeLabel(),
'Domain Name': domainName,
- 'Current Owner':
- walletAddress?.toString().slice(0, 8) +
- '...' +
- walletAddress?.toString().slice(-8),
+ 'Current Owner': ownerAddress
+ ? `${ownerAddress.slice(0, 8)}...${ownerAddress.slice(-8)}`
+ : 'Not connected',🤖 Prompt for AI Agents
In `@src/components/modals/ListNameForSaleModal/panels/ConfirmListingPanel.tsx`
around lines 74 - 81, The "Current Owner" entry in orderSummary can render
"undefined...undefined" when walletAddress is missing; update the logic in
ConfirmListingPanel where orderSummary is built so "Current Owner" prefers
domainInfo?.owner if present, otherwise formats walletAddress safely (only slice
when walletAddress exists) and falls back to a sensible placeholder like
"Unknown owner"; reference the orderSummary object, walletAddress,
domainInfo?.owner, getListingTypeLabel, domainName, and expirationDate to locate
and adjust the value construction.
| <Popover.Root open={isPopoverOpen} onOpenChange={setIsPopoverOpen}> | ||
| <Popover.Trigger asChild> | ||
| <button | ||
| type="button" | ||
| className="inline-flex items-center justify-center" | ||
| onMouseEnter={() => setIsPopoverOpen(true)} | ||
| onMouseLeave={() => setIsPopoverOpen(false)} | ||
| > | ||
| <InfoIcon | ||
| style={{ | ||
| fontSize: '16px', | ||
| fill: 'var(--text-grey)', | ||
| width: '16px', | ||
| cursor: 'pointer', | ||
| }} | ||
| /> | ||
| </button> |
There was a problem hiding this comment.
Add an accessible label to the icon-only popover trigger.
The button only contains an icon; screen readers won’t have a label. Add aria-label (and optionally title).
Proposed fix
- <button
+ <button
type="button"
className="inline-flex items-center justify-center"
+ aria-label="Fee policy details"
+ title="Fee policy details"
onMouseEnter={() => setIsPopoverOpen(true)}
onMouseLeave={() => setIsPopoverOpen(false)}
>📝 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.
| <Popover.Root open={isPopoverOpen} onOpenChange={setIsPopoverOpen}> | |
| <Popover.Trigger asChild> | |
| <button | |
| type="button" | |
| className="inline-flex items-center justify-center" | |
| onMouseEnter={() => setIsPopoverOpen(true)} | |
| onMouseLeave={() => setIsPopoverOpen(false)} | |
| > | |
| <InfoIcon | |
| style={{ | |
| fontSize: '16px', | |
| fill: 'var(--text-grey)', | |
| width: '16px', | |
| cursor: 'pointer', | |
| }} | |
| /> | |
| </button> | |
| <Popover.Root open={isPopoverOpen} onOpenChange={setIsPopoverOpen}> | |
| <Popover.Trigger asChild> | |
| <button | |
| type="button" | |
| className="inline-flex items-center justify-center" | |
| aria-label="Fee policy details" | |
| title="Fee policy details" | |
| onMouseEnter={() => setIsPopoverOpen(true)} | |
| onMouseLeave={() => setIsPopoverOpen(false)} | |
| > | |
| <InfoIcon | |
| style={{ | |
| fontSize: '16px', | |
| fill: 'var(--text-grey)', | |
| width: '16px', | |
| cursor: 'pointer', | |
| }} | |
| /> | |
| </button> |
🤖 Prompt for AI Agents
In `@src/components/modals/ListNameForSaleModal/panels/ConfirmListingPanel.tsx`
around lines 292 - 308, The icon-only Popover.Trigger button (wrapped by
Popover.Trigger and rendering InfoIcon) lacks an accessible label; update the
button element used with Popover.Trigger to include an appropriate aria-label
(e.g., aria-label="More info" or context-specific text) and optionally a title
attribute, keeping existing handlers (onMouseEnter/onMouseLeave) and state hooks
(isPopoverOpen, setIsPopoverOpen) intact so screen readers can announce the
control; ensure the label describes the popover’s purpose and matches the
InfoIcon’s intent.
fix(PE-8931): marketplace feedback 2
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@src/components/modals/ListNameForSaleModal/panels/FixedPricePanel.tsx`:
- Around line 239-244: The Cancel button currently calls window.history.back()
which can navigate away from the app for deep links; modify the FixedPricePanel
component to accept an onCancel prop (mirror the existing onNext prop pattern)
and call props.onCancel() from the button handler, falling back to a safe
navigation (e.g., router.push to a known route or history.replace('/') ) only if
onCancel is not provided; update the Cancel button's onClick to use the new
onCancel behavior and adjust the component's prop signature (FixedPricePanel)
accordingly so callers can control cancellation.
- Around line 200-212: The handler for onChange allows customValue (string) to
exceed MAX_LISTING_PRICE while listingPrice is clamped, causing a UI/value
mismatch; update the onChange in FixedPricePanel so that after computing
clampedVal (using MAX_LISTING_PRICE and Number(val)) you set both
setListingPrice(clampedVal) and setCustomValue to the clamped string when the
input exceeds the max (otherwise keep the raw val), and continue to call
setButtonSelected(undefined) and setCustomValueError(isValidCustomAmount(val));
reference isValidCustomFormat, setCustomValue, setListingPrice,
setButtonSelected, setCustomValueError, MAX_LISTING_PRICE and
isValidCustomAmount to locate and apply this change.
In `@src/components/pages/ViewListing/ViewListing.tsx`:
- Around line 361-373: The predicate used in the two queryClient.resetQueries
calls is too permissive because it uses
key.includes(name)/includes(domainInfo.processId); change these to precise
checks: inspect query.queryKey (handle both string and array forms) and match
elements exactly (===) or require a dedicated prefix (e.g., check string ===
expectedKey or Array.isArray(query.queryKey) && query.queryKey.some(k => k ===
name)). Update the predicates in the calls to queryClient.resetQueries to use
exact equality against domainInfo.processId and name (or a well-known prefix)
instead of key.includes to avoid accidental matches.
🧹 Nitpick comments (5)
src/components/data-display/tables/MarketplaceListingsTable.tsx (1)
79-80:internalFilteris dead code.
externalFilterdefaults to''(line 74), soexternalFilter !== undefinedis alwaystrue, makinginternalFilterunreachable. Either removeinternalFilterentirely or expose a setter if local filtering is intended.♻️ Suggested cleanup
- const [internalFilter] = useState(''); - const filter = externalFilter !== undefined ? externalFilter : internalFilter; + const filter = externalFilter;src/components/data-display/tables/DomainsTable.tsx (3)
480-492:limit: 500may silently miss orders;anytype loses safety.The
useMarketplaceOrderscall hardcodeslimit: 500. If the marketplace has more than 500 orders, the lookup map will be incomplete and some domains will incorrectly show "no order." Additionally,o: anyon line 487 discards type information.Suggested improvement
const marketplaceOrderByAntId = useMemo(() => { - const map: Record<string, { dateCreated: number }> = {}; - marketplaceOrdersData?.items?.forEach((o: any) => { - if (o?.dominantToken) - map[o.dominantToken] = { dateCreated: o.dateCreated ?? 0 }; + const map: Record<string, { dateCreated: number }> = {}; + marketplaceOrdersData?.items?.forEach((o: Order) => { + if (o?.dominantToken) + map[o.dominantToken] = { dateCreated: o.dateCreated ?? 0 }; }); return map; }, [marketplaceOrdersData?.items]);Consider paginating or increasing the limit with a TODO to handle overflow.
64-73: Duplicate import block fromlucide-react.Lines 64 and 65–73 both import from
lucide-react. Merge them into a single import statement.Suggested fix
-import { Activity, StoreIcon } from 'lucide-react'; -import { - AlertTriangle, - CircleCheck, - Copy, - DollarSign, - ExternalLink, - Lock, - Star, -} from 'lucide-react'; +import { + Activity, + AlertTriangle, + CircleCheck, + Copy, + DollarSign, + ExternalLink, + Lock, + Star, + StoreIcon, +} from 'lucide-react';
975-1111: Complex IIFE in the action column cell — consider extracting to a named component.The immediately-invoked function expression spans ~135 lines with multiple branches and hook-dependent data. Extracting it into a dedicated
<DomainActionCell>component would improve readability, allow each branch to be tested independently, and avoid re-creating closures on every render.package.json (1)
68-68: Bothradix-ui(unified) and scoped@radix-ui/*packages are listed.The
radix-uipackage (line 68) is the unified entry point officially released by Radix Primitives (Jan 2025), designed as a single tree-shakeable package to avoid version conflicts and dependency duplication. Having bothradix-uiand individual@radix-ui/react-*packages (lines 34–38) creates potential version skew and increases bundle size through duplicate code. Migrate fully to the unifiedradix-uipackage and remove the scoped packages, or continue using only scoped packages.
| onChange={(e) => { | ||
| const val = e.target.value; | ||
| if (isValidCustomFormat(val)) { | ||
| const numVal = Number(val); | ||
| const clampedVal = Math.min( | ||
| Number.isNaN(numVal) ? 0 : numVal, | ||
| MAX_LISTING_PRICE, | ||
| ); | ||
| setCustomValue(val); | ||
| setListingPrice(clampedVal); | ||
| setButtonSelected(undefined); | ||
| setCustomValueError(isValidCustomAmount(val)); | ||
| } |
There was a problem hiding this comment.
Clamped listingPrice diverges from displayed customValue on over-max input.
When the user types a value exceeding MAX_LISTING_PRICE, customValue retains the raw string (line 208) while listingPrice is silently clamped (lines 204–209). The error message displays correctly, but the Next button operates on the clamped value, which could confuse users—they see "2000000" but the listing would proceed at 1,000,000. Consider either preventing input beyond the max or replacing customValue with the clamped value when over the limit.
🤖 Prompt for AI Agents
In `@src/components/modals/ListNameForSaleModal/panels/FixedPricePanel.tsx` around
lines 200 - 212, The handler for onChange allows customValue (string) to exceed
MAX_LISTING_PRICE while listingPrice is clamped, causing a UI/value mismatch;
update the onChange in FixedPricePanel so that after computing clampedVal (using
MAX_LISTING_PRICE and Number(val)) you set both setListingPrice(clampedVal) and
setCustomValue to the clamped string when the input exceeds the max (otherwise
keep the raw val), and continue to call setButtonSelected(undefined) and
setCustomValueError(isValidCustomAmount(val)); reference isValidCustomFormat,
setCustomValue, setListingPrice, setButtonSelected, setCustomValueError,
MAX_LISTING_PRICE and isValidCustomAmount to locate and apply this change.
| <button | ||
| className="flex-1 bg-transparent border border-dark-grey text-white px-6 py-3 rounded hover:bg-grey hover:bg-opacity-20 transition-colors" | ||
| onClick={() => window.history.back()} | ||
| > | ||
| Cancel | ||
| </button> |
There was a problem hiding this comment.
window.history.back() on Cancel may navigate away from the app.
If the user arrived at this page directly (e.g. via deep link), window.history.back() could navigate to an external site or a blank tab. Consider accepting an onCancel callback prop (consistent with how onNext is handled) or navigating to a known route.
🤖 Prompt for AI Agents
In `@src/components/modals/ListNameForSaleModal/panels/FixedPricePanel.tsx` around
lines 239 - 244, The Cancel button currently calls window.history.back() which
can navigate away from the app for deep links; modify the FixedPricePanel
component to accept an onCancel prop (mirror the existing onNext prop pattern)
and call props.onCancel() from the button handler, falling back to a safe
navigation (e.g., router.push to a known route or history.replace('/') ) only if
onCancel is not provided; update the Cancel button's onClick to use the new
onCancel behavior and adjust the component's prop signature (FixedPricePanel)
accordingly so callers can control cancellation.
| queryClient.resetQueries({ | ||
| predicate: (query) => | ||
| query.queryKey.some( | ||
| (key: unknown) => | ||
| typeof key === 'string' && key.includes(domainInfo.processId), | ||
| ), | ||
| }); | ||
| queryClient.resetQueries({ | ||
| predicate: (query) => | ||
| query.queryKey.some( | ||
| (key: unknown) => typeof key === 'string' && key.includes(name), | ||
| ), | ||
| }); |
There was a problem hiding this comment.
Query invalidation using key.includes(name) is fragile — short domain names can match unrelated queries.
If name is something short like "io" or "ar", key.includes(name) could inadvertently match and reset unrelated query keys (e.g., any key containing "ario"). Consider using a more precise predicate, such as exact key matching or a dedicated query key prefix.
🤖 Prompt for AI Agents
In `@src/components/pages/ViewListing/ViewListing.tsx` around lines 361 - 373, The
predicate used in the two queryClient.resetQueries calls is too permissive
because it uses key.includes(name)/includes(domainInfo.processId); change these
to precise checks: inspect query.queryKey (handle both string and array forms)
and match elements exactly (===) or require a dedicated prefix (e.g., check
string === expectedKey or Array.isArray(query.queryKey) && query.queryKey.some(k
=> k === name)). Update the predicates in the calls to queryClient.resetQueries
to use exact equality against domainInfo.processId and name (or a well-known
prefix) instead of key.includes to avoid accidental matches.
Summary by CodeRabbit
New Features
Removed Features
Chores