Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughReplaces scattered token/contract+chain parameters with a unified Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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 |
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
lib/moment/fetchComments.ts (1)
8-12: Fix type mismatch: chainId parameter not in MomentCommentsInput.The function destructures
chainIdfrom the parameters, butMomentCommentsInput(line 4) only definesmomentandoffset. This creates a type safety violation.Additionally, since
Momentnow has an optionalchainIdfield (line 7 of types/moment.ts), you should either:
- Use
moment.chainIddirectly, or- Update
MomentCommentsInputto includechainIdas a separate parameterRecommended solution: Use
moment.chainIddirectly:async function fetchComments({ moment, - chainId, offset, }: MomentCommentsInput): Promise<MintComment[]> { try { + const chainId = moment.chainId; + if (!chainId) { + throw new Error("chainId is required"); + } const params: CommentsQueryParams = { contractAddress: moment.collectionAddress, tokenId: moment.tokenId, chainId, offset, };hooks/useComments.ts (2)
15-26: Critical bug: Property name mismatch in moment object.Line 20 passes an object with
contractAddressproperty tofetchComments, but theMomenttype (types/moment.ts line 5) defines this ascollectionAddress. This will causemoment.collectionAddressto beundefinedin fetchComments.ts line 15, leading to incorrect API calls.Apply this fix:
queryFn: ({ pageParam = 0 }) => fetchComments({ moment: { - contractAddress: contractAddress!, + collectionAddress: collectionAddress!, tokenId: tokenId!, }, chainId: chainId!, offset: pageParam as number, }),Note: This also depends on fixing the type mismatch in fetchComments.ts where
chainIdshould be part ofmomentor the type definition should be updated.
47-68: Property name mismatch in addComment cache key.The same issue exists in the cache update logic. Since
contractAddressshould becollectionAddress, the variable names should be updated throughout.This will be fixed by applying the refactor suggested on line 13 to use
collectionAddressconsistently.hooks/useMomentCollect.ts (2)
14-29: ExtractmintOnSmartWalletto a separate lib file.Per coding guidelines, hooks should focus on state management and data fetching. The
mintOnSmartWalletfunction contains API call logic that should be extracted to a lib file.Create a new file
lib/smartwallet/mintOnSmartWallet.ts:export async function mintOnSmartWallet(parameters: any) { const response = await fetch(`/api/smartwallet/sendUserOperation`, { method: "POST", headers: { "content-type": "application/json", accept: "application/json", }, body: JSON.stringify({ parameters, }), }); const data = await response.json(); return data.transactionHash; }Then import and use it in this hook:
+import { mintOnSmartWallet } from "@/lib/smartwallet/mintOnSmartWallet"; -const mintOnSmartWallet = async (parameters: any) => { - const response = await fetch(`/api/smartwallet/sendUserOperation`, { - method: "POST", - headers: { - "content-type": "application/json", - accept: "application/json", - }, - body: JSON.stringify({ - parameters, - }), - }); - - const data = await response.json(); - - return data.transactionHash; -};As per coding guidelines, business logic and API calls should be in lib files.
31-101: File exceeds 100-line limit.This file is 103 lines, exceeding the coding guideline limit of 100 lines. Consider splitting the business logic into separate lib files as suggested above.
As per coding guidelines, keep all files under 100 lines of code to maintain readability and follow single responsibility principle.
🧹 Nitpick comments (1)
hooks/useUpdateMomentURI.ts (1)
13-96: File exceeds 100-line limit.This file is 104 lines, exceeding the coding guideline limit of 100 lines. Consider extracting the metadata assembly logic (lines 37-57) into a separate lib function.
As per coding guidelines, keep all files under 100 lines. Consider creating
lib/moment/assembleUpdatedMetadata.tsfor the metadata construction logic.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (25)
app/api/moment/update-uri/route.ts(1 hunks)components/CreatedMoment/CreatedMomentAirdrop.tsx(1 hunks)components/TokenManagePage/TokenManagePage.tsx(1 hunks)components/TokenPage/TokenPage.tsx(1 hunks)hooks/useAirdrop.ts(2 hunks)hooks/useBalanceOf.ts(1 hunks)hooks/useComments.ts(1 hunks)hooks/useMomentCollect.ts(3 hunks)hooks/useSaleConfig.ts(2 hunks)hooks/useShareMoment.ts(1 hunks)hooks/useTokenInfo.ts(1 hunks)hooks/useUpdateMomentURI.ts(4 hunks)lib/moment/airdropMoment.ts(3 hunks)lib/moment/callUpdateMomentURI.ts(3 hunks)lib/moment/collectMoment.ts(1 hunks)lib/moment/collectMomentApi.ts(1 hunks)lib/moment/executeAirdrop.ts(3 hunks)lib/moment/fetchComments.ts(1 hunks)lib/moment/getMomentApi.ts(1 hunks)lib/moment/updateMomentURI.ts(2 hunks)lib/schema/airdropMomentSchema.ts(1 hunks)lib/schema/collectSchema.ts(1 hunks)providers/MomentProvider.tsx(1 hunks)types/moment.ts(1 hunks)types/token.ts(0 hunks)
💤 Files with no reviewable changes (1)
- types/token.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{ts,tsx}: Minimize code to absolute minimum needed - avoid unnecessary abstractions, intermediate steps, or redundant patterns
Write self-documenting code through precise naming (verbs for functions, nouns for variables), single-responsibility components, obvious data flow, and add short comments only when necessary
Apply Single Responsibility Principle (SRP): each function/component should have ONE clear responsibility - extract functions into separate files when they handle distinct concerns - hooks focus on state management, business logic goes in lib files - each function should be in its own file when it has a distinct purpose
Use strict TypeScript types with zero 'any' - prioritize type-exact definitions
Implement built-in security for authentication and data handling - follow secure coding practices throughout the application
Follow Next.js optimization guides for performance - implement proper caching, code splitting, and rendering strategies
Keep all files under 100 lines of code to maintain readability and follow single responsibility principle - split hooks/components when they grow too large or handle multiple concerns
Before coding, make a plan inside a tag: identify core requirement, consider 3 implementation approaches, choose simplest that meets needs, verify if code can be split into smaller functions, check for unnecessary abstractions, and ensure clarity for junior developers
Files:
lib/schema/airdropMomentSchema.tstypes/moment.tshooks/useTokenInfo.tslib/schema/collectSchema.tshooks/useMomentCollect.tshooks/useBalanceOf.tslib/moment/executeAirdrop.tshooks/useUpdateMomentURI.tsapp/api/moment/update-uri/route.tshooks/useSaleConfig.tslib/moment/updateMomentURI.tslib/moment/fetchComments.tscomponents/CreatedMoment/CreatedMomentAirdrop.tsxlib/moment/getMomentApi.tslib/moment/callUpdateMomentURI.tslib/moment/airdropMoment.tslib/moment/collectMoment.tshooks/useShareMoment.tshooks/useComments.tslib/moment/collectMomentApi.tsproviders/MomentProvider.tsxhooks/useAirdrop.tscomponents/TokenManagePage/TokenManagePage.tsxcomponents/TokenPage/TokenPage.tsx
lib/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
Library files should contain pure business logic, data transformation, validation, and API calls - never include React state management or UI concerns
Files:
lib/schema/airdropMomentSchema.tslib/schema/collectSchema.tslib/moment/executeAirdrop.tslib/moment/updateMomentURI.tslib/moment/fetchComments.tslib/moment/getMomentApi.tslib/moment/callUpdateMomentURI.tslib/moment/airdropMoment.tslib/moment/collectMoment.tslib/moment/collectMomentApi.ts
hooks/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
Hooks should focus exclusively on state management and data fetching - extract business logic, validation, and data transformation into separate lib files
Files:
hooks/useTokenInfo.tshooks/useMomentCollect.tshooks/useBalanceOf.tshooks/useUpdateMomentURI.tshooks/useSaleConfig.tshooks/useShareMoment.tshooks/useComments.tshooks/useAirdrop.ts
🧬 Code graph analysis (15)
hooks/useTokenInfo.ts (3)
types/moment.ts (1)
Moment(4-8)providers/UserProvider.tsx (1)
useUserProvider(38-44)lib/moment/getMomentApi.ts (1)
getMomentApi(3-15)
lib/schema/collectSchema.ts (2)
lib/schema/commentsSchema.ts (1)
momentSchema(4-7)lib/consts.ts (1)
CHAIN_ID(20-20)
lib/moment/executeAirdrop.ts (2)
types/moment.ts (1)
Moment(4-8)lib/consts.ts (1)
PERMISSION_BIT_ADMIN(29-29)
hooks/useUpdateMomentURI.ts (3)
providers/MomentProvider.tsx (1)
useMomentProvider(28-34)lib/consts.ts (1)
CHAIN_ID(20-20)lib/mux/migrateMuxToArweaveApi.ts (1)
migrateMuxToArweaveApi(18-43)
hooks/useSaleConfig.ts (4)
providers/MomentProvider.tsx (1)
useMomentProvider(28-34)lib/viem/publicClient.ts (2)
publicClient(16-16)getPublicClient(7-14)lib/consts.ts (1)
CHAIN_ID(20-20)lib/protocolSdk/constants.ts (1)
zoraCreatorFixedPriceSaleStrategyAddress(55-68)
lib/moment/updateMomentURI.ts (2)
types/moment.ts (1)
Moment(4-8)lib/coinbase/getOrCreateSmartWallet.ts (1)
getOrCreateSmartWallet(6-19)
components/CreatedMoment/CreatedMomentAirdrop.tsx (1)
lib/consts.ts (1)
CHAIN_ID(20-20)
lib/moment/getMomentApi.ts (1)
types/moment.ts (1)
Moment(4-8)
lib/moment/callUpdateMomentURI.ts (1)
types/moment.ts (1)
Moment(4-8)
lib/moment/airdropMoment.ts (1)
lib/consts.ts (1)
PERMISSION_BIT_ADMIN(29-29)
lib/moment/collectMoment.ts (3)
lib/coinbase/getOrCreateSmartWallet.ts (1)
getOrCreateSmartWallet(6-19)lib/consts.ts (1)
CHAIN_ID(20-20)lib/moment/validateBalanceAndAllowance.ts (1)
validateBalanceAndAllowance(15-40)
hooks/useShareMoment.ts (2)
providers/MomentProvider.tsx (1)
useMomentProvider(28-34)lib/consts.ts (2)
CHAIN(19-19)SITE_ORIGINAL_URL(36-38)
hooks/useComments.ts (1)
providers/MomentProvider.tsx (1)
useMomentProvider(28-34)
hooks/useAirdrop.ts (1)
providers/MomentProvider.tsx (1)
useMomentProvider(28-34)
components/TokenPage/TokenPage.tsx (1)
providers/MomentProvider.tsx (1)
MomentProvider(13-26)
🔇 Additional comments (27)
hooks/useSaleConfig.ts (1)
14-70: LGTM on the migration to moment-based architecture.The refactor from token-based to moment-based data flow is clean and maintains all existing functionality. The hook properly destructures moment data and uses appropriate fallbacks for optional chainId.
types/moment.ts (1)
4-8: LGTM on the Moment type updates.The rename from
contractAddresstocollectionAddressbetter represents the purpose, and adding optionalchainIdsupports the multi-chain architecture.hooks/useShareMoment.ts (1)
6-19: LGTM on the moment-based share functionality.Correctly uses
moment.collectionAddressandmoment.tokenIdfrom the MomentProvider.app/api/moment/update-uri/route.ts (1)
39-47: LGTM on the moment object construction.The API route correctly constructs the moment object with
collectionAddress,tokenId, andchainIdfrom the request payload, maintaining proper type safety.hooks/useAirdrop.ts (1)
18-91: LGTM on the moment-based airdrop implementation.The hook correctly migrates to using the
momentobject from MomentProvider, properly accessingcollectionAddressand passing the entire moment toexecuteAirdrop.lib/schema/airdropMomentSchema.ts (1)
12-15: LGTM on the schema field rename.Renaming
momentContracttocollectionAddressaligns with the Moment type definition and improves clarity.lib/moment/collectMomentApi.ts (1)
16-20: LGTM! Clean refactor to moment-centric payload.The API payload now directly sends the moment object, simplifying the structure and aligning with the broader moment-centric architecture.
hooks/useBalanceOf.ts (1)
10-26: LGTM! Consistent migration to moment-based data.All references correctly updated from
tokentomoment, with proper guard conditions and effect dependencies.components/TokenManagePage/TokenManagePage.tsx (1)
20-26: LGTM! MomentProvider correctly updated.The moment object structure properly includes
collectionAddress,tokenId, andchainId, aligning with the new Moment type definition.lib/moment/airdropMoment.ts (2)
23-44: LGTM! Consistent parameter renaming.All references to
momentContractcorrectly updated tocollectionAddressthroughout permission checks and function signature.
66-69: LGTM! Transaction target correctly updated.The transaction recipient field now uses
collectionAddress, maintaining consistency with the refactored naming.lib/schema/collectSchema.ts (1)
5-9: LGTM! Schema correctly updated with collectionAddress and chainId.The schema now includes
collectionAddress(renamed fromcontractAddress) and an optionalchainIdwith a sensible default, aligning with the moment-centric refactor.lib/moment/collectMoment.ts (1)
32-51: LGTM! Clean destructuring and parameter updates.The code correctly extracts
collectionAddressandtokenIdfrom the moment object and passes them to downstream functions, maintaining clean separation of concerns.lib/moment/executeAirdrop.ts (3)
11-16: LGTM! Interface correctly updated to moment-based structure.The
ExecuteAirdropParamsinterface now accepts a unifiedmomentobject instead of separatetokenIdandmomentContractparameters, improving API ergonomics.
26-43: LGTM! Permission checks correctly updated.All permission checks now use
moment.collectionAddressandmoment.tokenId, maintaining consistency with the moment-centric refactor.
45-56: LGTM! API payload correctly structured.The API request properly sends
collectionAddressextracted from the moment object.components/TokenPage/TokenPage.tsx (2)
21-25: LGTM! Moment object correctly structured.The moment object properly includes
collectionAddress,tokenId, andchainId, with appropriate type casting for the address.
30-30: LGTM! MomentProvider correctly updated.The provider now receives a single
momentprop instead of separatetokenandchainIdprops, aligning with the new API.components/CreatedMoment/CreatedMomentAirdrop.tsx (1)
17-21: LGTM! Clean refactoring to moment-based API.The moment object is properly constructed with all required fields (
collectionAddress,tokenId) and the optionalchainId. The type casting and structure align with the newMomenttype definition.lib/moment/callUpdateMomentURI.ts (1)
1-39: LGTM! Excellent lib file structure.This file properly follows coding guidelines:
- Pure business logic without React state management
- Clean type definitions with
Momenttype- Proper error handling
- Well under 100-line limit
The refactoring to accept a
momentobject is consistent with the broader architectural change.lib/moment/getMomentApi.ts (1)
3-15: LGTM! Clean API function with proper optional chainId handling.The function correctly:
- Accepts a unified
Momentobject- Conditionally includes
chainIdin query params only when present (line 7)- Handles error responses with a fallback error message
The refactoring maintains backward compatibility through optional
chainIdwhile simplifying the API signature.lib/moment/updateMomentURI.ts (1)
23-48: LGTM! Proper lib file with moment-based refactoring.The function correctly:
- Uses the
Momentobject for token context- Maintains clean business logic without React dependencies
- Properly extracts
collectionAddressandtokenIdfrom moment (lines 32-36)- Returns structured result with hash and chainId
The refactoring is consistent with the architectural change and follows coding guidelines for lib files.
hooks/useTokenInfo.ts (1)
8-52: LGTM! Well-structured hook with proper moment-based data fetching.The hook correctly:
- Accepts a unified
Momentparameter- Uses moment fields in queryKey for proper cache management (line 12)
- Has appropriate enabled condition checking required fields (line 14)
- Focuses on state management without business logic
The refactoring maintains query optimization (staleTime, retry logic) while simplifying the API to a single
momentparameter.providers/MomentProvider.tsx (2)
13-26: LGTM! Clean provider refactoring with improved API.The provider correctly:
- Consolidates
tokenandchainIdinto a singlemomentprop (line 13)- Passes
momenttouseTokenInfofor data fetching (line 14)- Spreads moment data into context value (lines 19-20)
- Maintains proper TypeScript typing
This is a breaking change but improves the API by reducing the number of props and enforcing consistency through the
Momenttype.
28-34: LGTM! Proper context hook with error handling.The
useMomentProviderhook correctly:
- Uses
useContextto access the context- Throws a descriptive error if used outside provider
- Exports a clean, type-safe API
The implementation follows React context best practices.
hooks/useMomentCollect.ts (1)
36-36: I'll help you verify the review comment about themomentobject validation inuseMomentCollect.ts. Let me start by examining the relevant files.
<function_calls>
#!/bin/bashFirst, let's find and examine the MomentProvider and useMomentCollect files
fd -t f "MomentProvider.tsx" -x cat {}
echo "---FILE_SEPARATOR---"
fd -t f "useMomentCollect.ts" -x cat {} | head -100
</function_calls>
#!/bin/bashSearch for MomentProvider context to understand how moment is provided
rg -nP --type=ts "useMomentProvider|MomentProvider" --context=5 | head -150
</function_calls>
#!/bin/bashCheck the full useMomentCollect.ts file to see all moment usages
wc -l hooks/useMomentCollect.ts
echo "---"
cat -n hooks/useMomentCollect.ts
</function_calls>Let me search more specifically for these files:
<function_calls>
#!/bin/bashList all files in providers directory
fd -t f . providers/ 2>/dev/null | head -20
echo "---"List all files in hooks directory
fd -t f . hooks/ 2>/dev/null | head -20
</function_calls><function_calls>
#!/bin/bashSearch for MomentProvider and useMomentCollect more broadly
find . -name "Moment" -type f 2>/dev/null | grep -E ".(ts|tsx)$"
</function_calls><function_calls>
#!/bin/bashSearch for files containing useMomentProvider or MomentProvider definitions
rg -l "useMomentProvider|createContext.*Moment|MomentProvider" --type=ts --type=tsx
</function_calls><function_calls>
#!/bin/bashCheck the structure to understand the repo layout
git ls-files | grep -E "(hook|provider)" | head -30
</function_calls>hooks/useUpdateMomentURI.ts (1)
30-34: Chainid fallback is appropriate and verified.All upstream callers of
MomentProviderexplicitly setchainId(TokenPage.tsx, TokenManagePage.tsx, and CreatedMomentAirdrop.tsx). SincechainIdis optional in theMomenttype (types/moment.ts line 7), the fallback toCHAIN_IDis both justified and correct.CHAIN_IDis environment-aware (IS_TESTNET ? baseSepolia : base), making it a proper default for all cases.
| args: [ | ||
| moment.tokenId, | ||
| zoraCreatorFixedPriceSaleStrategyAddress[moment.chainId || CHAIN_ID], | ||
| calldata, | ||
| ], |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's examine the useSaleConfig.ts file completely
fd useSaleConfig -type fRepository: sweetmantech/in_process
Length of output: 237
🏁 Script executed:
# Find the useSaleConfig.ts file
fd useSaleConfigRepository: sweetmantech/in_process
Length of output: 89
🏁 Script executed:
# Read the useSaleConfig.ts file
wc -l hooks/useSaleConfig.tsRepository: sweetmantech/in_process
Length of output: 92
🏁 Script executed:
# Read the complete useSaleConfig.ts file
cat -n hooks/useSaleConfig.tsRepository: sweetmantech/in_process
Length of output: 2760
🏁 Script executed:
# Find the constants file with zoraCreatorFixedPriceSaleStrategyAddress
fd constants -type f | grep -E 'protocolSdk|lib'Repository: sweetmantech/in_process
Length of output: 237
🏁 Script executed:
# Find the constants file
fd constants | grep -E 'protocolSdk|lib'Repository: sweetmantech/in_process
Length of output: 135
🏁 Script executed:
# Read the constants file
cat -n lib/protocolSdk/constants.tsRepository: sweetmantech/in_process
Length of output: 3492
🏁 Script executed:
# Find CHAIN_ID constant
fd consts -o cat | head -100Repository: sweetmantech/in_process
Length of output: 192
🏁 Script executed:
# Find files named consts
fd constsRepository: sweetmantech/in_process
Length of output: 97
🏁 Script executed:
# Read the main consts file
cat -n lib/consts.tsRepository: sweetmantech/in_process
Length of output: 6613
🏁 Script executed:
# Find MomentProvider
fd MomentProviderRepository: sweetmantech/in_process
Length of output: 95
🏁 Script executed:
# Check the size of MomentProvider.tsx first
wc -l providers/MomentProvider.tsxRepository: sweetmantech/in_process
Length of output: 98
🏁 Script executed:
# Read MomentProvider.tsx
cat -n providers/MomentProvider.tsxRepository: sweetmantech/in_process
Length of output: 1176
🏁 Script executed:
# Find the Moment type
fd moment.tsRepository: sweetmantech/in_process
Length of output: 401
🏁 Script executed:
# Read the Moment type definition
cat -n types/moment.tsRepository: sweetmantech/in_process
Length of output: 1661
🏁 Script executed:
# Search for where useSaleConfig is called to see if there's upstream validation
rg -n "useSaleConfig" --type ts --type tsxRepository: sweetmantech/in_process
Length of output: 94
Verify that chainId exists in zoraCreatorFixedPriceSaleStrategyAddress before using it as a dictionary key.
The code accesses zoraCreatorFixedPriceSaleStrategyAddress[moment.chainId || CHAIN_ID] without verifying the chainId exists as a key. The Moment interface defines chainId as an optional number with no constraint to supported chain IDs (1, 10, 999, 8453, 42161, 81457, 84532, 421614, 7777777, 11155111, 168587773, 999999999). If moment.chainId contains an unsupported value, this will silently pass undefined to the contract call.
Add a runtime check or TypeScript constraint:
+ const chainId = moment.chainId || CHAIN_ID;
+ const strategyAddress = zoraCreatorFixedPriceSaleStrategyAddress[chainId as keyof typeof zoraCreatorFixedPriceSaleStrategyAddress];
+ if (!strategyAddress) {
+ throw new Error(`Unsupported chain ID: ${chainId}`);
+ }
+
const hash = await signTransaction({
address: moment.collectionAddress,
abi: zoraCreator1155ImplABI,
functionName: "callSale",
args: [
moment.tokenId,
- zoraCreatorFixedPriceSaleStrategyAddress[moment.chainId || CHAIN_ID],
+ strategyAddress,
calldata,
],📝 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.
| args: [ | |
| moment.tokenId, | |
| zoraCreatorFixedPriceSaleStrategyAddress[moment.chainId || CHAIN_ID], | |
| calldata, | |
| ], | |
| args: [ | |
| moment.tokenId, | |
| zoraCreatorFixedPriceSaleStrategyAddress[moment.chainId || CHAIN_ID], | |
| calldata, | |
| ], | |
| ]; | |
| const chainId = moment.chainId || CHAIN_ID; | |
| const strategyAddress = zoraCreatorFixedPriceSaleStrategyAddress[chainId as keyof typeof zoraCreatorFixedPriceSaleStrategyAddress]; | |
| if (!strategyAddress) { | |
| throw new Error(`Unsupported chain ID: ${chainId}`); | |
| } | |
| const hash = await signTransaction({ | |
| address: moment.collectionAddress, | |
| abi: zoraCreator1155ImplABI, | |
| functionName: "callSale", | |
| args: [ | |
| moment.tokenId, | |
| strategyAddress, | |
| calldata, | |
| ], |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
lib/supabase/in_process_moments/selectMoments.ts (1)
13-18: Address type/runtime inconsistency and unsafe number conversion.Two concerns:
Type mismatch: The
Momentinterface definescollectionAddressandtokenIdas required fields, yet the code treats them as optional with conditional checks. Either make these fields optional in the type (collectionAddress?,tokenId?) or remove the unnecessary checks.Unsafe conversion:
Number(tokenId)on line 17 will produceNaNfor non-numeric strings (e.g.,Number("abc")→NaN), which could cause database query failures or incorrect filtering. Consider validating the tokenId or using a safer conversion.Apply this diff to add validation:
if (tokenId) { - query = query.eq("token_id", Number(tokenId)); + const tokenIdNum = Number(tokenId); + if (isNaN(tokenIdNum)) { + throw new Error(`Invalid tokenId: ${tokenId} cannot be converted to a number`); + } + query = query.eq("token_id", tokenIdNum); }Additionally, consider updating the
Momentinterface to reflect the actual optionality:// In types/moment.ts export interface Moment { collectionAddress?: Address; tokenId?: string; chainId?: number; }app/api/moment/comments/route.ts (1)
21-28: Mismatch between query parameter structure and schema/consumer expectations.The
chainIdis extracted at the top level (line 26), butmomentSchemaincommentsSchema.tsexpectschainIdinside themomentobject (with a default). Meanwhile,momentComments.tsreadsmoment.chainId. This means the user-providedchainIdquery parameter is ignored, and only the schema default is used.Either include
chainIdinside themomentobject:const queryParams = { moment: { collectionAddress: searchParams.get("collectionAddress") as Address, tokenId: searchParams.get("tokenId") || "1", + chainId: Number(searchParams.get("chainId")) || CHAIN_ID, }, - chainId: Number(searchParams.get("chainId")) || CHAIN_ID, offset: Number(searchParams.get("offset")) || 0, };Or update
commentsSchemato keepchainIdat the top level and havemomentCommentsread it from there.lib/schema/commentsSchema.ts (1)
5-15: DuplicatechainIdin schema creates confusion.The schema defines
chainIdin two places:
- Inside
momentSchema(line 8) - optional with default- At the top level of
commentsSchema(line 13) - requiredThis duplication causes ambiguity about which value takes precedence. The route passes
chainIdat the top level, butmomentComments.tsreadsmoment.chainId.Consider aligning with
collectSchema.tswhich useschainIdonly insidemomentSchema:export const commentsSchema = z.object({ moment: momentSchema, - chainId: z.number(), offset: z.number(), });
♻️ Duplicate comments (1)
hooks/useSaleConfig.ts (1)
38-44: Missing runtime validation for unsupported chainId values.The type cast
as keyof typeof zoraCreatorFixedPriceSaleStrategyAddresssuppresses TypeScript errors but doesn't prevent runtime issues. Ifmoment.chainIdis an unsupported value, the lookup returnsundefined, which will be passed to the contract call.This issue was flagged in a previous review. Apply the suggested runtime check:
+ const chainId = moment.chainId || CHAIN_ID; + const strategyAddress = zoraCreatorFixedPriceSaleStrategyAddress[chainId as keyof typeof zoraCreatorFixedPriceSaleStrategyAddress]; + if (!strategyAddress) { + throw new Error(`Unsupported chain ID: ${chainId}`); + } + const hash = await signTransaction({ address: moment.collectionAddress, abi: zoraCreator1155ImplABI, functionName: "callSale", args: [ moment.tokenId, - zoraCreatorFixedPriceSaleStrategyAddress[ - moment.chainId as keyof typeof zoraCreatorFixedPriceSaleStrategyAddress - ], + strategyAddress, calldata, ],
🧹 Nitpick comments (1)
app/api/moment/comments/route.ts (1)
45-48: Avoidanytype per coding guidelines.Use
unknownand narrow the type for error handling.- } catch (e: any) { - console.log(e); - const message = e?.message ?? "failed to get comments"; + } catch (e: unknown) { + console.error(e); + const message = e instanceof Error ? e.message : "failed to get comments"; return Response.json({ message }, { status: 500, headers: corsHeaders }); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
app/api/moment/comments/route.ts(1 hunks)hooks/useComments.ts(1 hunks)hooks/useSaleConfig.ts(2 hunks)lib/moment/fetchComments.ts(1 hunks)lib/moment/momentComments.ts(1 hunks)lib/schema/commentsSchema.ts(1 hunks)lib/supabase/in_process_moments/selectMoments.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- hooks/useComments.ts
- lib/moment/fetchComments.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{ts,tsx}: Minimize code to absolute minimum needed - avoid unnecessary abstractions, intermediate steps, or redundant patterns
Write self-documenting code through precise naming (verbs for functions, nouns for variables), single-responsibility components, obvious data flow, and add short comments only when necessary
Apply Single Responsibility Principle (SRP): each function/component should have ONE clear responsibility - extract functions into separate files when they handle distinct concerns - hooks focus on state management, business logic goes in lib files - each function should be in its own file when it has a distinct purpose
Use strict TypeScript types with zero 'any' - prioritize type-exact definitions
Implement built-in security for authentication and data handling - follow secure coding practices throughout the application
Follow Next.js optimization guides for performance - implement proper caching, code splitting, and rendering strategies
Keep all files under 100 lines of code to maintain readability and follow single responsibility principle - split hooks/components when they grow too large or handle multiple concerns
Before coding, make a plan inside a tag: identify core requirement, consider 3 implementation approaches, choose simplest that meets needs, verify if code can be split into smaller functions, check for unnecessary abstractions, and ensure clarity for junior developers
Files:
app/api/moment/comments/route.tslib/schema/commentsSchema.tshooks/useSaleConfig.tslib/moment/momentComments.tslib/supabase/in_process_moments/selectMoments.ts
lib/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
Library files should contain pure business logic, data transformation, validation, and API calls - never include React state management or UI concerns
Files:
lib/schema/commentsSchema.tslib/moment/momentComments.tslib/supabase/in_process_moments/selectMoments.ts
hooks/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
Hooks should focus exclusively on state management and data fetching - extract business logic, validation, and data transformation into separate lib files
Files:
hooks/useSaleConfig.ts
🧬 Code graph analysis (2)
lib/schema/commentsSchema.ts (2)
lib/schema/collectSchema.ts (1)
momentSchema(5-9)lib/consts.ts (1)
CHAIN_ID(20-20)
lib/supabase/in_process_moments/selectMoments.ts (1)
types/moment.ts (1)
Moment(4-8)
🔇 Additional comments (3)
lib/supabase/in_process_moments/selectMoments.ts (1)
4-5: LGTM! Clean refactor to unified Moment parameter.The function signature change from separate parameters to a single
Momentobject improves API consistency and aligns with the PR's goal of standardizing on the Moment shape across the codebase.lib/moment/momentComments.ts (1)
20-41: LGTM!The error handling for missing moment data and the comment formatting logic are well-implemented.
hooks/useSaleConfig.ts (1)
54-70: LGTM!The useEffect for initializing saleStart and the hook's return structure are well-implemented.
| const publicClient = getPublicClient(moment.chainId || CHAIN_ID); | ||
| const hash = await signTransaction({ | ||
| address: token.tokenContractAddress, | ||
| address: moment.collectionAddress, | ||
| abi: zoraCreator1155ImplABI, | ||
| functionName: "callSale", | ||
| args: [token.tokenId, zoraCreatorFixedPriceSaleStrategyAddress[CHAIN_ID], calldata], | ||
| args: [ | ||
| moment.tokenId, | ||
| zoraCreatorFixedPriceSaleStrategyAddress[ | ||
| moment.chainId as keyof typeof zoraCreatorFixedPriceSaleStrategyAddress | ||
| ], | ||
| calldata, | ||
| ], | ||
| account: connectedAddress as Address, | ||
| chain: CHAIN, | ||
| }); |
There was a problem hiding this comment.
Inconsistent chain usage between publicClient and transaction signing.
Line 33 derives the public client from moment.chainId || CHAIN_ID, but line 46 hardcodes chain: CHAIN. If moment.chainId differs from CHAIN_ID, the transaction will be signed for a different chain than the one used to wait for the receipt.
Consider deriving the chain consistently:
+import { getChain } from "@/lib/viem/getChain"; // or appropriate helper
+
const publicClient = getPublicClient(moment.chainId || CHAIN_ID);
+ const targetChain = getChain(moment.chainId || CHAIN_ID);
const hash = await signTransaction({
// ...
account: connectedAddress as Address,
- chain: CHAIN,
+ chain: targetChain,
});Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In hooks/useSaleConfig.ts around lines 33 to 47, the code builds publicClient
using moment.chainId || CHAIN_ID but calls signTransaction with a hardcoded
chain: CHAIN, causing mismatched chains; fix by deriving a single chain
identifier (e.g. const activeChainId = moment.chainId || CHAIN_ID and map it to
the appropriate Chain value if needed), use that same activeChainId/Chain when
calling getPublicClient and pass the same derived Chain into signTransaction,
and ensure types align (cast or map numeric chainId to the expected Chain type)
so both public client and transaction signing use the identical chain.
| export async function momentComments({ | ||
| moment, | ||
| chainId, | ||
| offset, | ||
| }: GetCommentsInput): Promise<MomentCommentsResult> { | ||
| const data = await selectMoments({ | ||
| collectionAddress: moment.contractAddress as Address, | ||
| collectionAddress: moment.collectionAddress as Address, | ||
| tokenId: moment.tokenId, | ||
| chainId, | ||
| chainId: moment.chainId, | ||
| }); |
There was a problem hiding this comment.
Function ignores top-level chainId from input schema.
GetCommentsInput (inferred from commentsSchema) includes both moment.chainId and a top-level chainId, but only moment.chainId is used. This creates ambiguity about which chainId takes precedence and may cause the user-provided value to be ignored (see related issue in the route file).
Consider either:
- Removing the redundant
chainIdfromcommentsSchemaand relying solely onmoment.chainId, or - Using the top-level
chainIdhere if that's the intended source of truth.
🤖 Prompt for AI Agents
In lib/moment/momentComments.ts around lines 10 to 18, the function always uses
moment.chainId and ignores a top-level chainId from GetCommentsInput; decide and
implement a single source of truth: either remove the redundant top-level
chainId from the commentsSchema and leave the call as-is, or update this
function to prefer the top-level chainId (e.g., use the top-level chainId when
present, falling back to moment.chainId) when calling selectMoments; make the
change consistently and update any related validation/comments to avoid
ambiguity.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.