-
Notifications
You must be signed in to change notification settings - Fork 4
feat(PE-8734): marketplace #903
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Conversation
📝 WalkthroughWalkthroughAdds a Marketplace feature (pages, hooks, modals, state, and UI) and related marketplace wiring; removes Jest/Playwright test infra and many tests; updates CI/workflows, dependencies, and network/URL validation utilities. 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 / Token
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, domainInfo, 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. 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
src/App.tsx
Outdated
|
|
||
| // set the log level of ar-io-sdk | ||
| Logger.default.setLogLevel('none'); | ||
| Logger.default.setLogLevel('debug'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ 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.
src/components/data-display/tables/MarketplaceListingsTable.tsx
Outdated
Show resolved
Hide resolved
| <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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
| id={ | ||
| result.antTransferResult?.id | ||
| ? new ArweaveTransactionID(result.antTransferResult?.id) | ||
| : '0xanErrorOccured' | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Error state will throw: invalid ArweaveTransactionID.
If result.antTransferResult?.id is undefined, creating new ArweaveTransactionID('0xanErrorOccured') will throw an error because the constructor validates that the ID must be a 43-character alphanumeric string (per ArweaveTransactionID.ts lines 6-12). This could crash the success panel.
- id={
- result.antTransferResult?.id
- ? new ArweaveTransactionID(result.antTransferResult?.id)
- : '0xanErrorOccured'
- }
+ id={
+ result.antTransferResult?.id
+ ? new ArweaveTransactionID(result.antTransferResult.id)
+ : undefined
+ }Then handle the undefined case in the ArweaveID component or conditionally render this section only when result.antTransferResult?.id exists.
📝 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.
| id={ | |
| result.antTransferResult?.id | |
| ? new ArweaveTransactionID(result.antTransferResult?.id) | |
| : '0xanErrorOccured' | |
| } | |
| id={ | |
| result.antTransferResult?.id | |
| ? new ArweaveTransactionID(result.antTransferResult.id) | |
| : undefined | |
| } |
🤖 Prompt for AI Agents
In src/components/modals/ListNameForSaleModal/ListNameForSaleModal.tsx around
lines 160-164, the code constructs new ArweaveTransactionID('0xanErrorOccured')
when result.antTransferResult?.id is missing, which triggers the
ArweaveTransactionID constructor validation and will throw; change the logic to
only instantiate ArweaveTransactionID when result.antTransferResult?.id is a
valid non-empty string (e.g., conditionally render this component/section only
if result.antTransferResult?.id exists) or modify the prop passed to the
ArweaveID component to be undefined/null when id is absent and let ArweaveID
handle the empty state gracefully (do not pass an invalid placeholder string).
| const marketPriceDisplay = domainPrices?.buy | ||
| ? `Market Value: ${formatARIOWithCommas(Math.ceil(domainPrices.buy / 1000000))} ${arioTicker}` | ||
| : ''; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
src/components/modals/ManageMarketplaceARIOModal/panels/WithdrawPanel.tsx
Show resolved
Hide resolved
| Sentry.browserTracingIntegration(), | ||
| Sentry.reactRouterV6BrowserTracingIntegration({ | ||
| useEffect, | ||
| useLocation, | ||
| useNavigationType, | ||
| createRoutesFromChildren, | ||
| matchRoutes, | ||
| }), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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://pr-903_arns.arweave.net
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
src/components/modals/ContinueWorkflowModal/ContinueWorkflowModal.tsx
Outdated
Show resolved
Hide resolved
| 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🧹 Nitpick comments (5)
src/components/pages/ViewListing/ViewListing.tsx (5)
239-266: Enhance deposit confirmation polling with better reliability.The balance polling logic has several areas for improvement:
- Fixed 5-second intervals: No exponential backoff could lead to unnecessary load during temporary network issues.
- Silent error handling: Line 262 logs balance check errors but doesn't inform the user of transient issues.
- No progress feedback: Users wait up to 50 seconds without updates during the polling loop.
🔎 Proposed improvements
// Wait for deposit to be confirmed let newBalance = 0; let tries = 0; const maxTries = 10; + const baseDelay = 3000; // Start with 3s 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; + + // Update user on progress + updateWorkflowSteps({ + step: 'deposit', + status: 'processing', + description: `Confirming deposit... (${tries + 1}/${maxTries})`, + }); } catch (error) { console.error('Error checking balance:', error); } tries++; - await sleep(5000); + // Exponential backoff: 3s, 6s, 12s, capped at 15s + const delay = Math.min(baseDelay * Math.pow(2, tries - 1), 15000); + await sleep(delay); }
320-333: Query invalidation strategy may be overly broad.Lines 320-332 use
resetQuerieswith predicate functions that search forprocessIdornamein query keys. This approach:
- Could inadvertently reset unrelated queries if keys happen to contain these strings
- May not be comprehensive if some related queries use different key structures
Consider using more specific query key patterns if the query keys follow a consistent structure:
// Instead of predicate-based matching queryClient.resetQueries({ queryKey: ['domainInfo', name] }); queryClient.resetQueries({ queryKey: ['ant', domainInfo.processId] }); queryClient.resetQueries({ queryKey: ['ant-info', domainInfo.processId] }); queryClient.invalidateQueries({ queryKey: ['marketplace'] }); queryClient.invalidateQueries({ queryKey: ['marketplace-order', domainInfo.processId] }); queryClient.invalidateQueries({ queryKey: ['marketplace-user-assets', walletAddress] });This is more explicit and reduces the risk of unintended cache invalidations.
556-564: Manual address truncation instead of using ArweaveID component.Lines 556-564 manually truncate the seller address with string slicing, while other places use the
ArweaveIDcomponent. This creates inconsistency in how addresses are displayed.For consistency with the rest of the component, consider using the
ArweaveIDcomponent here as well:🔎 Proposed change
<a href={`https://aolink.ar-io.dev/#/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" > - {orderData.creator.slice(0, 8)}... - {orderData.creator.slice(-8)} + <ArweaveID + id={new ArweaveTransactionID(orderData.creator)} + type={ArweaveIdTypes.ADDRESS} + shouldLink={false} + characterCount={8} + /> <ExternalLink className="w-3 h-3" /> </a>Note: You'll need to adjust this to avoid double-linking, or remove the outer
<a>tag ifArweaveIDhandles the link.
393-401: Loading state could use Loader component.The loading state displays plain text instead of using the imported
Loadercomponent, which would provide a more polished and consistent loading experience.🔎 Proposed improvement
if (isLoading) { return ( <div className="page text-white"> <div className="flex justify-center items-center min-h-[400px]"> - <div className="text-white text-xl">Loading listing details...</div> + <div className="flex flex-col items-center gap-4"> + <Loader size={40} /> + <div className="text-white text-xl">Loading listing details...</div> + </div> </div> </div> ); }
113-121: Use consistent mARIO-to-ARIO conversion method for balance comparison.The balance check uses two different conversion APIs from different packages:
- Line 116–118:
new mARIOToken(...).toARIO().valueOf()(from @ar.io/sdk) for user balance- Line 119:
mARIOToTokenAmount(...)(from @ardrive/turbo-sdk) for order priceWhile both convert mARIO to ARIO, standardizing on a single method throughout the component would improve clarity and reduce maintenance risk. Consider using
mARIOTokenconsistently since it's already imported and used elsewhere in the codebase.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/components/pages/ViewListing/ViewListing.tsx
🧰 Additional context used
🧠 Learnings (1)
📚 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/ViewListing/ViewListing.tsx
🧬 Code graph analysis (1)
src/components/pages/ViewListing/ViewListing.tsx (15)
src/components/icons/index.ts (2)
CheckIcon(89-89)ArNSLogo(72-72)src/components/pages/index.ts (1)
ViewListing(18-18)src/state/contexts/GlobalState.tsx (1)
useGlobalState(178-179)src/state/contexts/WalletState.tsx (1)
useWalletState(49-50)src/hooks/useDomainInfo.tsx (1)
useDomainInfo(206-255)src/hooks/useMarketplaceOrder.tsx (1)
useMarketplaceOrder(44-56)src/hooks/useArIOPrice.ts (1)
useArIoPrice(9-32)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(215-219)src/components/layout/index.ts (1)
Loader(12-12)src/components/data-display/VerticalTimelineStepper.tsx (1)
VerticalTimelineStepper(3-41)
🔇 Additional comments (9)
src/components/pages/ViewListing/ViewListing.tsx (9)
1-41: LGTM!The imports are well-organized and all appear necessary for the marketplace listing functionality.
43-66: LGTM!The workflow step configuration is clear and provides good user feedback structure for the multi-step purchase process.
68-112: LGTM!The component initialization and state management are well-structured. The
isExpiredcheck correctly compares the current time against the order expiration.
123-180: LGTM!The
updateWorkflowStepsfunction provides clear visual feedback with appropriate icons and styling for each workflow state.
233-236: Deposit amount calculation looks correct.The calculation
Math.max(1, Math.ceil(orderPrice - existingBalance))appropriately:
- Ensures at least 1 ARIO is deposited (prevents zero deposits)
- Rounds up fractional amounts to cover the full order price
- Accounts for existing balance to avoid over-depositing
628-654: LGTM! Buy button state handling is comprehensive.The buy button correctly handles all states:
- Disabled when expired or already processing
- Shows appropriate icons and text for each state
- Uses proper styling for visual feedback
656-698: LGTM! Workflow UI provides excellent user feedback.The processing workflow UI effectively:
- Shows real-time progress through the VerticalTimelineStepper
- Provides clear messaging for success and failure states
- Offers appropriate actions (retry on error, view name on success)
- Warns users not to close the page during processing
239-244: Add timeout handling for the purchase transaction.The
buyFixedPriceANTcall at line 300 lacks any timeout mechanism. If the transaction hangs indefinitely, users remain stuck in the processing state with no recourse except refreshing the page.The deposit call (line 239) has implicit timeout protection via its polling loop (10 retries × 5-second intervals), but the purchase call has none.
Wrap the purchase transaction with explicit timeout handling:
async function withTimeout<T>( promise: Promise<T>, timeoutMs: number, errorMessage: string ): Promise<T> { const timeout = new Promise<never>((_, reject) => setTimeout(() => reject(new Error(errorMessage)), timeoutMs) ); return Promise.race([promise, timeout]); } // Usage at line 300: const result = await withTimeout( marketplaceContract.buyFixedPriceANT({ antId: domainInfo.processId, }), 60000, // 60 second timeout 'Purchase transaction timed out' );⛔ 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.
427-428: Decimal precision for ARIO conversions is accurate.Mainnet ARIO has a denomination of 6, and each ARIO is divisible into 1,000,000 µARIO, confirming 6 decimal precision. The code correctly uses
mARIOToTokenAmountfrom turbo-sdk to convert from mARIO (10^-6 ARIO) to ARIO, andformatARIOWithCommasappropriately displays the result with up to 3 decimal places. The base-ario ERC-20 token also uses 6 decimals, consistent with mainnet ARIO. No precision issues detected.
| ao: aoClient, | ||
| }), | ||
| signer: createAoSigner(wallet.contractSigner), | ||
| ario: arioContract as AoARIOWrite, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Type assertion bypasses type safety.
The type assertion arioContract as AoARIOWrite on Line 215 could hide potential type mismatches. If arioContract doesn't fully satisfy the AoARIOWrite interface, this could lead to runtime errors.
🔎 Verify type compatibility or add validation
Option 1: Add runtime validation:
+ if (!arioContract || typeof arioContract.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,
});Option 2: Verify that arioContract from global state is properly typed as AoARIOWrite throughout the application to eliminate the need for this assertion.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/components/pages/ViewListing/ViewListing.tsx around line 215, the inline
type assertion "arioContract as AoARIOWrite" bypasses compile-time safety;
instead ensure arioContract is correctly typed at its source (e.g., update the
global state/type definitions or the selector that provides arioContract to
return AoARIOWrite) or add a small runtime type guard/validation that checks
required methods/properties before using it and only then narrow the type;
remove the unsafe "as" cast and either rely on the corrected source typing or
perform the guard and narrow the type with a safe conditional.
| id={new ArweaveTransactionID(orderData.creator)} | ||
| type={ArweaveIdTypes.ADDRESS} | ||
| shouldLink={true} | ||
| characterCount={12} | ||
| /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ArweaveTransactionID constructor can throw on invalid input.
The ArweaveTransactionID constructor throws an error if the provided ID doesn't match the expected 43-character format. If orderData.creator, domainInfo.processId, or orderData.id contain invalid values, this will cause unhandled exceptions in the render phase, crashing the component.
Add validation before constructing ArweaveTransactionID instances:
🔎 Proposed defensive rendering
{orderData?.creator && (
<div className="flex items-center justify-between gap-2 text-grey text-sm">
<span>Owner:</span>
<div className="flex items-end justify-end">
+ {(() => {
+ try {
+ return (
<ArweaveID
id={new ArweaveTransactionID(orderData.creator)}
type={ArweaveIdTypes.ADDRESS}
shouldLink={true}
characterCount={12}
/>
+ );
+ } catch {
+ return <span className="text-white font-mono">{orderData.creator}</span>;
+ }
+ })()}
</div>
</div>
)}Apply similar try-catch wrappers for Lines 505-509 (ANT Process) and Lines 617-621 (Order ID).
Alternatively, create a safe wrapper component:
function SafeArweaveID({ id, ...props }: { id: string } & Omit<ComponentProps<typeof ArweaveID>, 'id'>) {
try {
return <ArweaveID id={new ArweaveTransactionID(id)} {...props} />;
} catch {
return <span className="text-white font-mono">{id}</span>;
}
}Also applies to: 505-509, 617-621
🤖 Prompt for AI Agents
In src/components/pages/ViewListing/ViewListing.tsx around lines 483-487,
constructing new ArweaveTransactionID(orderData.creator) can throw for invalid
IDs; wrap that construction with a validation or try/catch and fall back to
rendering the raw string (or a safe placeholder) so render errors are not
thrown; apply the same defensive pattern to the other instances mentioned (lines
505-509 for domainInfo.processId and 617-621 for orderData.id), or replace
usages with a small SafeArweaveID wrapper component that catches construction
errors and returns a safe fallback.
| <LeaseDurationFromEndTimestamp | ||
| endTimestamp={ | ||
| domainInfo?.arnsRecord?.type === 'lease' | ||
| ? (domainInfo.arnsRecord as any)?.endTimestamp |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Type assertion bypasses type safety for lease record.
Line 519 uses (domainInfo.arnsRecord as any)?.endTimestamp to access the endTimestamp property. This bypasses TypeScript's type checking and could lead to runtime errors if the structure changes.
🔎 Use proper type guard instead
<span className="text-white">
<LeaseDurationFromEndTimestamp
endTimestamp={
- domainInfo?.arnsRecord?.type === 'lease'
- ? (domainInfo.arnsRecord as any)?.endTimestamp
- : undefined // Permabuy domains or no record - let component show "Permanent"
+ domainInfo?.arnsRecord?.type === 'lease' &&
+ 'endTimestamp' in domainInfo.arnsRecord
+ ? domainInfo.arnsRecord.endTimestamp
+ : undefined
}
/>
</span>Or better, update the type definition to properly discriminate the union type based on the type field.
📝 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.
| ? (domainInfo.arnsRecord as any)?.endTimestamp | |
| <span className="text-white"> | |
| <LeaseDurationFromEndTimestamp | |
| endTimestamp={ | |
| domainInfo?.arnsRecord?.type === 'lease' && | |
| 'endTimestamp' in domainInfo.arnsRecord | |
| ? domainInfo.arnsRecord.endTimestamp | |
| : undefined | |
| } | |
| /> | |
| </span> |
🤖 Prompt for AI Agents
In src/components/pages/ViewListing/ViewListing.tsx around line 519, the code
uses `(domainInfo.arnsRecord as any)?.endTimestamp`, which bypasses TypeScript
safety; replace this with a proper type-narrowing approach: add a type guard
(e.g., function isLeaseRecord(r): r is LeaseRecord { return r?.type === 'lease'
&& 'endTimestamp' in r }) or refine the domainInfo/arnsRecord union type so you
can safely access endTimestamp only when arnsRecord is the lease variant, then
use that guard before reading endTimestamp and handle the non-lease case
explicitly (fallback/undefined) instead of casting to any.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/components/pages/ViewListing/ViewListing.tsx (1)
320-333: Prefer explicit query key patterns over string matching.The query invalidation logic uses string matching on query keys, which is fragile. If query key structures change (e.g., from strings to structured objects), this will silently fail.
🔎 Proposed refactor using explicit query keys
Consider defining query key factories in your hooks and using them consistently:
// In your hooks file or a shared queryKeys file export const queryKeys = { domain: (name: string) => ['domain', name] as const, marketplaceOrder: (antId: string) => ['marketplace', 'order', antId] as const, marketplaceUserAssets: () => ['marketplace', 'user-assets'] as const, };Then update invalidation:
- 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), - ), - }); - queryClient.invalidateQueries({ queryKey: ['marketplace'] }); + // Invalidate domain-specific queries + queryClient.invalidateQueries({ + queryKey: queryKeys.domain(name) + }); + // Invalidate order queries + queryClient.invalidateQueries({ + queryKey: queryKeys.marketplaceOrder(domainInfo.processId) + }); + // Invalidate user assets + queryClient.invalidateQueries({ + queryKey: queryKeys.marketplaceUserAssets() + }); + // Invalidate all marketplace listings + queryClient.invalidateQueries({ + queryKey: ['marketplace'] + });This approach is type-safe, explicit, and won't break silently if query key structures evolve.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/components/pages/ViewListing/ViewListing.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/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.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.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/ViewListing/ViewListing.tsx
🔇 Additional comments (1)
src/components/pages/ViewListing/ViewListing.tsx (1)
81-87: No issue. TheuseDomainInfohook's public signature only acceptsdomainandantId; callers cannot supplyarioProcessId. The call at line 85 with{ domain: name }is correct as implemented.
| // Wait for deposit to be confirmed | ||
| 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add cleanup for balance polling to prevent memory leaks.
The balance polling loop doesn't check if the component is still mounted. If the user navigates away during the purchase flow, the loop continues executing and may attempt state updates on an unmounted component.
🔎 Recommended fix using AbortController pattern
async function handleBuy() {
+ const abortController = new AbortController();
setIsBuying(true);
setShowProcessing(true);
setWorkflowComplete(false);
setWorkflowError(false);
setWorkflowSteps(defaultBuyWorkflowSteps);
try {
// ... existing validation ...
if (!hasSufficientBalance) {
updateWorkflowSteps({ step: 'deposit', status: 'processing' });
// ... deposit logic ...
// Wait for deposit to be confirmed
let newBalance = 0;
let tries = 0;
const maxTries = 10;
- while (newBalance < orderPrice && tries < maxTries) {
+ while (newBalance < orderPrice && tries < maxTries && !abortController.signal.aborted) {
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);
}
+ if (abortController.signal.aborted) {
+ throw new Error('Purchase cancelled');
+ }
+
if (newBalance < orderPrice) {
throw new Error('Failed to deposit enough ARIO');
}
// ... rest of logic ...
}
} catch (error) {
console.error(error);
setWorkflowComplete(true);
setWorkflowError(true);
eventEmitter.emit('error', error);
} finally {
setIsBuying(false);
}
}Then add cleanup in a useEffect:
useEffect(() => {
const controller = new AbortController();
return () => controller.abort();
}, []);Or consider moving the entire purchase flow to a custom hook that properly manages lifecycle.
📝 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.
| // Wait for deposit to be confirmed | |
| 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); | |
| } | |
| const abortControllerRef = useRef<AbortController | null>(null); | |
| async function handleBuy() { | |
| abortControllerRef.current = new AbortController(); | |
| setIsBuying(true); | |
| setShowProcessing(true); | |
| setWorkflowComplete(false); | |
| setWorkflowError(false); | |
| setWorkflowSteps(defaultBuyWorkflowSteps); | |
| try { | |
| // ... existing validation ... | |
| if (!hasSufficientBalance) { | |
| updateWorkflowSteps({ step: 'deposit', status: 'processing' }); | |
| // ... deposit logic ... | |
| // Wait for deposit to be confirmed | |
| let newBalance = 0; | |
| let tries = 0; | |
| const maxTries = 10; | |
| while (newBalance < orderPrice && tries < maxTries && !abortControllerRef.current?.signal.aborted) { | |
| 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); | |
| } | |
| if (abortControllerRef.current?.signal.aborted) { | |
| throw new Error('Purchase cancelled'); | |
| } | |
| if (newBalance < orderPrice) { | |
| throw new Error('Failed to deposit enough ARIO'); | |
| } | |
| // ... rest of logic ... | |
| } | |
| } catch (error) { | |
| console.error(error); | |
| setWorkflowComplete(true); | |
| setWorkflowError(true); | |
| eventEmitter.emit('error', error); | |
| } finally { | |
| setIsBuying(false); | |
| } | |
| } | |
| useEffect(() => { | |
| return () => { | |
| if (abortControllerRef.current) { | |
| abortControllerRef.current.abort(); | |
| } | |
| }; | |
| }, []); |
🤖 Prompt for AI Agents
In src/components/pages/ViewListing/ViewListing.tsx around lines 246 to 266, the
balance-polling loop can continue running after the component unmounts causing
memory leaks and possible state updates on an unmounted component; fix by wiring
an AbortController (or a mounted flag) into the loop: create the controller in a
surrounding useEffect and return a cleanup that calls controller.abort(), pass
controller.signal or check controller.signal.aborted (or the mounted flag)
inside the while loop and break/return when aborted, and guard any state updates
so they only run when not aborted/unmounted; alternatively move the purchase
flow into a custom hook that accepts and respects an AbortSignal and performs
the same cleanup.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
♻️ Duplicate comments (2)
package.json (2)
26-26: Alpha version still in use (duplicate concern).This was previously flagged: using
^1.35.0-alpha.2introduces instability risk. Consider switching to the latest stable release or pinning the exact alpha version if you require newer features.
36-37: Sentry version mismatch still present (duplicate concern).This was previously flagged:
@sentry/browserat^10.29.0and@sentry/reactat^7.45.0are incompatible major versions. Update both packages to use the same major version to avoid runtime conflicts.
🧹 Nitpick comments (1)
package.json (1)
60-60: Consider using caret range for consistency.
lucide-reactis pinned to an exact version0.556.0without a caret, which is inconsistent with other dependencies. Consider using^0.556.0to allow patch and minor updates, unless the exact pinning is intentional to prevent breaking changes.
📜 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
⏰ 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 (1)
package.json (1)
23-23: Verify the necessity of using an alpha version.
@ar.io/sdkis set to^3.23.0-alpha.3, which introduces instability risk. The latest stable release is3.19.0; alpha releases can have breaking changes, and the caret operator allows auto-upgrading to newer alpha versions. Ensure this alpha version is required for specific features; otherwise, use the stable release or pin the exact alpha version.
| "lottie-react": "^2.4.0", | ||
| "lucide-react": "^0.438.0", | ||
| "lucide-react": "0.556.0", | ||
| "permaweb-deploy": "^2.5.1", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Move deployment tool to devDependencies.
permaweb-deploy is a deployment tool used only in build scripts (lines 8-9), not in runtime code. It should be in devDependencies rather than dependencies to avoid unnecessarily increasing the production bundle size and installation footprint.
🔎 Proposed fix
Move permaweb-deploy from dependencies to devDependencies:
"lucide-react": "0.556.0",
- "permaweb-deploy": "^2.5.1",
"plimit-lit": "^3.0.1",And add it to devDependencies section (around line 93):
"http-server": "^14.1.1",
"husky": "^9.1.7",
+ "permaweb-deploy": "^2.5.1",
"lint-staged": "^13.1.0",📝 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.
| "permaweb-deploy": "^2.5.1", | |
| // Dependencies section (with permaweb-deploy removed): | |
| "lucide-react": "0.556.0", | |
| "plimit-lit": "^3.0.1", | |
| // DevDependencies section (with permaweb-deploy added): | |
| "http-server": "^14.1.1", | |
| "husky": "^9.1.7", | |
| "permaweb-deploy": "^2.5.1", | |
| "lint-staged": "^13.1.0", |
🤖 Prompt for AI Agents
In package.json around line 61, permaweb-deploy is listed under "dependencies"
but is only used by build/deploy scripts; remove the "permaweb-deploy": "^2.5.1"
entry from "dependencies" and add the same version string under
"devDependencies" (around line ~93) so it’s installed only for development/build
environments; after editing, run your package manager (npm/yarn/pnpm) to update
node_modules and the lockfile so CI and installs reflect the change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 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.
| 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove/guard console.log in production paths (balances, events).
These will be noisy and can leak operational details; prefer a debug logger gated by env, or remove.
Also applies to: 464-465
🤖 Prompt for AI Agents
In @src/components/modals/ListNameForSaleModal/ListNameForSaleModal.tsx around
lines 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.
| // 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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',
});| <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.'} | ||
| </p> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix incorrect completion messages for seller workflow.
Lines 845-846 show messages that don't match the seller's cancellation workflow:
- Line 845: "There was an error completing your purchase" should say "...your cancellation" for sellers
- Line 846: "{name} is now yours!" doesn't apply when a seller successfully cancels their listing
🔧 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 your listing.'
+ : 'There was an error completing your purchase.'
+ : isUserSeller
+ ? 'Your listing has been removed from the marketplace.'
+ : `${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 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.
…ddresses and safe ario formatting
fix(marketplace): bug fixes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Summary by CodeRabbit
New Features
Removed Features
✏️ Tip: You can customize this high-level summary in your review settings.