feat: sdk example app#519
Open
valentinludu wants to merge 14 commits into
Open
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughAdds a ChangesSDK: Cover Metadata Module and Quote Refactor
Next.js wagmi Example App
Sequence Diagram(s)sequenceDiagram
participant User
participant BuyCoverPage
participant NexusSDK
participant NexusAPI
participant CoverBrokerContract
rect rgba(70, 130, 180, 0.5)
note over User,NexusSDK: Quote flow
User->>BuyCoverPage: Enter productId/amount/period/asset → Get Quote
BuyCoverPage->>NexusSDK: getQuoteAndBuyCoverInputs({ coverMetadata? })
NexusSDK->>NexusAPI: GET product + productType
NexusSDK->>NexusAPI: POST /cover-metadata (if coverMetadata present)
NexusAPI-->>NexusSDK: { cid }
NexusSDK->>NexusAPI: GET /quote
NexusAPI-->>NexusSDK: quote + buyCoverInput
NexusSDK-->>BuyCoverPage: displayInfo + buyCoverParams
end
rect rgba(34, 139, 34, 0.5)
note over User,CoverBrokerContract: Buy flow
User->>BuyCoverPage: Confirm Buy Cover
BuyCoverPage->>CoverBrokerContract: buyCover(params) [value=ETH if ETH asset]
CoverBrokerContract-->>BuyCoverPage: transaction receipt
BuyCoverPage-->>User: Etherscan link
end
sequenceDiagram
participant User
participant EditMetadataPage
participant wagmiSignTypedData
participant CoverData
User->>EditMetadataPage: Enter coverMetadataId + ProofOfLossEntry value → Submit
EditMetadataPage->>wagmiSignTypedData: signTypedData(buildCoverMetadataAuthMessage())
wagmiSignTypedData-->>EditMetadataPage: signature string
EditMetadataPage->>CoverData: editCoverMetadata({ coverMetadataId, proofOfLoss, signature })
CoverData->>CoverData: buildAuthHeaders(signature) → x-auth-* headers
CoverData-->>EditMetadataPage: { result } or { error }
EditMetadataPage-->>User: success panel or error message
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
Comment |
Deploying sdk with
|
| Latest commit: |
47e8cfd
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://e03106e2.sdk-9yp.pages.dev |
| Branch Preview URL: | https://feat-sdk-example-app.sdk-9yp.pages.dev |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (7)
src/types/product.ts (2)
33-33: ⚡ Quick winConsider documenting validation behavior.
The
proofOfLossInputTypesfield specifies which proof types are required when proof of loss data is provided. Consider adding a JSDoc comment to clarify that:
- These types are validated only when
coverMetadata.proofOfLossis provided- All specified types must be present (at least one entry per type)
- This is independent of
ProductType.isProofOfLossRequired📝 Suggested documentation
+ /** + * Required proof of loss input types for this product. + * When coverMetadata.proofOfLoss is provided, all types listed here must be present. + * Validated in conjunction with ProductType.isProofOfLossRequired. + */ proofOfLossInputTypes?: ProofOfLossEntry['type'][];🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/types/product.ts` at line 33, Add a JSDoc comment above the proofOfLossInputTypes field to document its validation behavior. The comment should clarify that these types are validated only when coverMetadata.proofOfLoss is provided, that all specified types must be present with at least one entry per type, and that this validation is independent of ProductType.isProofOfLossRequired. This will help other developers understand the field's purpose and behavior constraints.
15-15: ⚡ Quick winConsider documenting the relationship with
proofOfLossInputTypes.The
isProofOfLossRequiredfield works in conjunction withProduct.proofOfLossInputTypes. Their relationship isn't immediately clear from the type definitions:
isProofOfLossRequired: Whether proof of loss data is mandatory for this product typeproofOfLossInputTypes: Which specific proof types must be present (validated only if proof is provided)Consider adding a JSDoc comment explaining this relationship for future maintainers.
📝 Suggested documentation
+ /** + * Whether proof of loss is required for this product type. + * When true, coverMetadata.proofOfLoss must be provided and non-empty. + * Used in conjunction with Product.proofOfLossInputTypes to validate proof data. + */ isProofOfLossRequired?: boolean;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/types/product.ts` at line 15, Add a JSDoc comment to the `isProofOfLossRequired` field that documents its relationship with the `proofOfLossInputTypes` field. The comment should clarify that `isProofOfLossRequired` indicates whether proof of loss data is mandatory for this product type, while `proofOfLossInputTypes` specifies which specific proof types must be present (only validated when proof is actually provided). This will help future maintainers understand how these two fields work together.examples/nextjs-wagmi/app/page.tsx (1)
59-88: ⚡ Quick winAdd error handling for wallet connection failures.
The
ConnectButtoncallsconnect({ connector: injected() })without handling potential errors. If the connection fails (e.g., user rejects, MetaMask not installed), the user receives no feedback.♻️ Proposed fix to add error state
function ConnectButton() { const { address, isConnected } = useAccount(); const { mutate: connect } = useConnect(); const { mutate: disconnect } = useDisconnect(); + const [error, setError] = useState<string>(); if (isConnected && address) { return ( <div className="flex items-center gap-3"> <span className="rounded-full bg-primary-light px-3 py-1.5 text-sm font-mono text-primary"> {address.slice(0, 6)}...{address.slice(-4)} </span> <button onClick={() => disconnect()} className="rounded-lg border border-card-border px-4 py-2 text-sm font-medium text-muted transition-colors hover:border-primary hover:text-foreground" > Disconnect </button> </div> ); } return ( - <button - onClick={() => connect({ connector: injected() })} - className="rounded-lg bg-primary px-5 py-2.5 text-sm font-semibold text-white transition-colors hover:bg-primary-hover" - > - Connect Wallet - </button> + <div> + <button + onClick={() => { + setError(undefined); + connect( + { connector: injected() }, + { + onError: (err) => setError(err.message), + } + ); + }} + className="rounded-lg bg-primary px-5 py-2.5 text-sm font-semibold text-white transition-colors hover:bg-primary-hover" + > + Connect Wallet + </button> + {error && ( + <p className="mt-2 text-sm text-red-600">{error}</p> + )} + </div> ); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@examples/nextjs-wagmi/app/page.tsx` around lines 59 - 88, The ConnectButton component needs error handling for the wallet connection process. Modify the useConnect hook destructuring to also capture the error state that it returns, then add a local state variable to track connection errors. When the user clicks the connect button and the mutation fails, capture and display the error message to the user in the UI. Consider adding error state management alongside the connection logic in the onClick handler for the Connect Wallet button and displaying the error message conditionally in the component's return statement.examples/nextjs-wagmi/app/view-metadata/page.tsx (1)
17-21: ⚡ Quick winHandle invalid cover ID input explicitly.
Line 20 converts
coverIdto a number without validation. If the user bypasses client-side checks orcoverIdcontains non-numeric characters,Number(coverId)returnsNaN, which would pass an invalid value togetCover().While the UI's
canFetchcheck (line 61) prevents the button from being enabled whencoverIdis invalid, adding explicit validation in the mutation function would make the error case clearer and provide better error messages.♻️ Proposed fix to add explicit validation
const viewMetadata = useMutation({ mutationFn: async (): Promise<ViewCoverMetadataResponse> => { + const coverIdNum = Number(coverId); + if (!Number.isInteger(coverIdNum) || coverIdNum <= 0) { + throw new Error('Cover ID must be a positive integer'); + } + // Step 1: Get cover data (includes coverMetadataId) - const coverResponse = await sdk.cover.getCover(Number(coverId)); + const coverResponse = await sdk.cover.getCover(coverIdNum); if (coverResponse.error) { throw new Error(coverResponse.error.message); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@examples/nextjs-wagmi/app/view-metadata/page.tsx` around lines 17 - 21, Add explicit validation for the coverId parameter in the mutationFn function of the viewMetadata useMutation hook before passing it to sdk.cover.getCover(). Instead of directly calling Number(coverId), first validate that coverId is a non-empty string and contains only numeric characters, then parse it. If validation fails, throw an error with a clear message indicating what went wrong (e.g., "Invalid cover ID provided"). This ensures that invalid inputs are caught early in the mutation function with better error messaging rather than relying solely on the UI-level canFetch check.examples/nextjs-wagmi/app/edit-metadata/page.tsx (1)
28-41: ⚖️ Poor tradeoffConsider validating entry values before building proof of loss.
The
buildProofOfLossfunction constructs entries without validating the format ofentryValue. For example, whenentryTypeis'address', the function doesn't verify thatentryValueis a valid Ethereum address. Adding validation would prevent submission of malformed data.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@examples/nextjs-wagmi/app/edit-metadata/page.tsx` around lines 28 - 41, The buildProofOfLoss function constructs proof of loss entries without validating that entryValue conforms to the expected format for the given entryType. Add validation logic at the beginning of the buildProofOfLoss function that checks entryValue against each case type, such as validating that entryValue is a valid Ethereum address when entryType is 'address', validating API key format when entryType is 'api_key', and so on. This validation should occur before the switch statement and should either throw an error or return an invalid state to prevent malformed data from being submitted.examples/nextjs-wagmi/app/buy-cover/page.tsx (1)
91-95: 💤 Low valueReview the exhaustive-deps disable.
The
useEffectdisables thereact-hooks/exhaustive-depsrule becausequoteMutation.resetis not included in the dependency array. This pattern can lead to stale closure bugs if the mutation object identity changes.Consider whether
quoteMutation.resetshould be added to the deps array or wrap it in auseCallbackto stabilize its identity.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@examples/nextjs-wagmi/app/buy-cover/page.tsx` around lines 91 - 95, The useEffect hook disables the exhaustive-deps ESLint rule because quoteMutation.reset is used within the effect but not included in the dependency array, which can cause stale closure bugs. Fix this by either adding quoteMutation.reset to the dependency array along with debouncedProductId, or by wrapping quoteMutation.reset in a useCallback hook to stabilize its identity, then remove the eslint-disable comment once the dependency issue is resolved.examples/nextjs-wagmi/config/wagmi.ts (1)
1-11: 💤 Low valueConsider adding a comment about RPC fallback for public accessibility.
The config uses http() with no URL, which falls back to a public RPC, though an authenticated RPC URL is highly recommended to prevent rate-limiting. For an example app, this is acceptable, but a brief comment clarifying this trade-off would improve maintainability.
export const config = createConfig({ chains: [mainnet], connectors: [injected()], transports: { + // Falls back to public RPC; use authenticated URL for production [mainnet.id]: http(), }, });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@examples/nextjs-wagmi/config/wagmi.ts` around lines 1 - 11, The http() function in the transports configuration lacks documentation explaining its behavior. Add a comment above the transports object that clarifies that http() without a URL argument falls back to a public RPC endpoint, which may be rate-limited, and note that while acceptable for example applications, an authenticated RPC URL should be used in production environments.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@examples/nextjs-wagmi/app/buy-cover/page.tsx`:
- Line 75: The code on line 75 uses a non-null assertion on response.result
without first verifying that the result property exists. While the error check
on lines 71-73 guards against response.error, it does not guarantee that
response.result is present. Add an explicit check to verify that response.result
is defined and not null before returning it, and handle the case where result is
missing appropriately instead of relying on the non-null assertion operator.
- Line 350: The href attribute in the transaction link is hardcoded to the
mainnet Etherscan URL, which breaks for users on other networks. Instead of
using the hardcoded https://etherscan.io/tx/ string, dynamically construct the
explorer URL by accessing the current chain information from wagmi (typically
available through useNetwork or from the chain object in your wagmi config) and
use the appropriate blockExplorers property from the chain data to build the
correct explorer URL for the current network before appending the txHash.
In `@examples/nextjs-wagmi/app/edit-metadata/page.tsx`:
- Around line 53-69: Remove all debug console.log statements from the
editCoverMetadata function block in the page.tsx file. Specifically, delete the
four console.log calls: the one logging the signature ("enters sig"), the one
logging proofOfLoss, the one logging the response, and the one logging the
error. Keep all the actual business logic intact, including the buildProofOfLoss
call, the sdk.cover.editCoverMetadata API call, and the error handling
conditional check. Only remove the logging statements to prevent sensitive data
like signatures and proof of loss information from being exposed to browser
consoles.
In `@examples/nextjs-wagmi/package.json`:
- Line 26: The eslint-config-next package version set to 16.2.9 does not align
with the actual Next.js version being used in the project. Verify the Next.js
version specified in the dependencies of the package.json file and update the
eslint-config-next version to match the corresponding compatible version for
that Next.js release, ensuring consistency across both packages.
In `@src/cover/Cover.ts`:
- Around line 20-31: The validation check in the getCover method currently does
not prevent Infinity and -Infinity values from passing through since these are
not caught by the `!coverId || coverId <= 0` condition. Replace or enhance the
validation condition to also check that coverId is a finite number by using
Number.isFinite(coverId) or by adding explicit checks to reject Infinity and
-Infinity values alongside the existing checks for zero, negative numbers, and
NaN.
---
Nitpick comments:
In `@examples/nextjs-wagmi/app/buy-cover/page.tsx`:
- Around line 91-95: The useEffect hook disables the exhaustive-deps ESLint rule
because quoteMutation.reset is used within the effect but not included in the
dependency array, which can cause stale closure bugs. Fix this by either adding
quoteMutation.reset to the dependency array along with debouncedProductId, or by
wrapping quoteMutation.reset in a useCallback hook to stabilize its identity,
then remove the eslint-disable comment once the dependency issue is resolved.
In `@examples/nextjs-wagmi/app/edit-metadata/page.tsx`:
- Around line 28-41: The buildProofOfLoss function constructs proof of loss
entries without validating that entryValue conforms to the expected format for
the given entryType. Add validation logic at the beginning of the
buildProofOfLoss function that checks entryValue against each case type, such as
validating that entryValue is a valid Ethereum address when entryType is
'address', validating API key format when entryType is 'api_key', and so on.
This validation should occur before the switch statement and should either throw
an error or return an invalid state to prevent malformed data from being
submitted.
In `@examples/nextjs-wagmi/app/page.tsx`:
- Around line 59-88: The ConnectButton component needs error handling for the
wallet connection process. Modify the useConnect hook destructuring to also
capture the error state that it returns, then add a local state variable to
track connection errors. When the user clicks the connect button and the
mutation fails, capture and display the error message to the user in the UI.
Consider adding error state management alongside the connection logic in the
onClick handler for the Connect Wallet button and displaying the error message
conditionally in the component's return statement.
In `@examples/nextjs-wagmi/app/view-metadata/page.tsx`:
- Around line 17-21: Add explicit validation for the coverId parameter in the
mutationFn function of the viewMetadata useMutation hook before passing it to
sdk.cover.getCover(). Instead of directly calling Number(coverId), first
validate that coverId is a non-empty string and contains only numeric
characters, then parse it. If validation fails, throw an error with a clear
message indicating what went wrong (e.g., "Invalid cover ID provided"). This
ensures that invalid inputs are caught early in the mutation function with
better error messaging rather than relying solely on the UI-level canFetch
check.
In `@examples/nextjs-wagmi/config/wagmi.ts`:
- Around line 1-11: The http() function in the transports configuration lacks
documentation explaining its behavior. Add a comment above the transports object
that clarifies that http() without a URL argument falls back to a public RPC
endpoint, which may be rate-limited, and note that while acceptable for example
applications, an authenticated RPC URL should be used in production
environments.
In `@src/types/product.ts`:
- Line 33: Add a JSDoc comment above the proofOfLossInputTypes field to document
its validation behavior. The comment should clarify that these types are
validated only when coverMetadata.proofOfLoss is provided, that all specified
types must be present with at least one entry per type, and that this validation
is independent of ProductType.isProofOfLossRequired. This will help other
developers understand the field's purpose and behavior constraints.
- Line 15: Add a JSDoc comment to the `isProofOfLossRequired` field that
documents its relationship with the `proofOfLossInputTypes` field. The comment
should clarify that `isProofOfLossRequired` indicates whether proof of loss data
is mandatory for this product type, while `proofOfLossInputTypes` specifies
which specific proof types must be present (only validated when proof is
actually provided). This will help future maintainers understand how these two
fields work together.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 33f26520-26d1-4a93-9511-e2a9127874bb
⛔ Files ignored due to path filters (4)
examples/nextjs-wagmi/app/favicon.icois excluded by!**/*.icoexamples/nextjs-wagmi/pnpm-lock.yamlis excluded by!**/pnpm-lock.yamlexamples/nextjs-wagmi/public/globe.svgis excluded by!**/*.svgexamples/nextjs-wagmi/public/vercel.svgis excluded by!**/*.svg
📒 Files selected for processing (37)
.npmignoreexamples/nextjs-wagmi/.gitignoreexamples/nextjs-wagmi/README.mdexamples/nextjs-wagmi/app/buy-cover/page.tsxexamples/nextjs-wagmi/app/edit-metadata/page.tsxexamples/nextjs-wagmi/app/globals.cssexamples/nextjs-wagmi/app/layout.tsxexamples/nextjs-wagmi/app/page.tsxexamples/nextjs-wagmi/app/providers.tsxexamples/nextjs-wagmi/app/view-metadata/page.tsxexamples/nextjs-wagmi/config/sdk.tsexamples/nextjs-wagmi/config/wagmi.tsexamples/nextjs-wagmi/eslint.config.mjsexamples/nextjs-wagmi/hooks/useDebounce.tsexamples/nextjs-wagmi/next.config.tsexamples/nextjs-wagmi/package.jsonexamples/nextjs-wagmi/pnpm-workspace.yamlexamples/nextjs-wagmi/postcss.config.mjsexamples/nextjs-wagmi/tsconfig.jsonsrc/auth.test.tssrc/auth.tssrc/cover/Cover.test.tssrc/cover/Cover.tssrc/cover/index.tssrc/index.tssrc/ipfs/Ipfs.tssrc/ipfs/schemas.tssrc/ipfs/uploadIPFSContent.test.tssrc/ipfs/validateIPFSContent.test.tssrc/nexus-sdk.tssrc/quote/Quote.tssrc/quote/getQuoteAndBuyCoverInputs.test.tssrc/types/cover-metadata.tssrc/types/index.tssrc/types/ipfs.tssrc/types/product.tssrc/types/sdk.ts
💤 Files with no reviewable changes (4)
- src/ipfs/validateIPFSContent.test.ts
- src/ipfs/schemas.ts
- src/ipfs/Ipfs.ts
- src/types/ipfs.ts
| throw new Error(response.error.message); | ||
| } | ||
|
|
||
| return response.result!; |
There was a problem hiding this comment.
Non-null assertion without verification.
The code uses the non-null assertion operator (response.result!) without first checking that response.result is defined. While Line 71-73 checks for response.error, the absence of an error doesn't guarantee result is present.
🛡️ Proposed fix to add explicit result check
if (response.error) {
throw new Error(response.error.message);
}
+ if (!response.result) {
+ throw new Error('Quote failed: no result returned');
+ }
return response.result!;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| return response.result!; | |
| if (response.error) { | |
| throw new Error(response.error.message); | |
| } | |
| if (!response.result) { | |
| throw new Error('Quote failed: no result returned'); | |
| } | |
| return response.result!; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@examples/nextjs-wagmi/app/buy-cover/page.tsx` at line 75, The code on line 75
uses a non-null assertion on response.result without first verifying that the
result property exists. While the error check on lines 71-73 guards against
response.error, it does not guarantee that response.result is present. Add an
explicit check to verify that response.result is defined and not null before
returning it, and handle the case where result is missing appropriately instead
of relying on the non-null assertion operator.
| <div className="rounded-xl border border-green-200 bg-green-50 p-4 dark:border-green-900 dark:bg-green-950"> | ||
| <p className="text-sm font-medium text-green-700 dark:text-green-300">Cover purchased!</p> | ||
| <a | ||
| href={`https://etherscan.io/tx/${txHash}`} |
There was a problem hiding this comment.
Hardcoded mainnet Etherscan URL.
The Etherscan link is hardcoded to https://etherscan.io, which only works on mainnet. If the wagmi config supports multiple networks, users will be directed to the wrong block explorer.
🔗 Suggested improvement to use chain-aware explorer
Use wagmi's chain information to construct the correct explorer URL:
+import { useAccount, useWriteContract, useWaitForTransactionReceipt, useChainId } from 'wagmi';
+import { mainnet } from 'wagmi/chains';
export default function BuyCoverPage() {
- const { address, isConnected } = useAccount();
+ const { address, isConnected, chain } = useAccount();
+ const explorerUrl = chain?.blockExplorers?.default?.url ?? 'https://etherscan.io';
// ...
{isConfirmed && txHash && (
<div className="rounded-xl border border-green-200 bg-green-50 p-4 dark:border-green-900 dark:bg-green-950">
<p className="text-sm font-medium text-green-700 dark:text-green-300">Cover purchased!</p>
<a
- href={`https://etherscan.io/tx/${txHash}`}
+ href={`${explorerUrl}/tx/${txHash}`}
target="_blank"
rel="noopener noreferrer"
className="mt-1 block text-sm text-green-600 underline dark:text-green-400"
>
View on Etherscan📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| href={`https://etherscan.io/tx/${txHash}`} | |
| import { useAccount, useWriteContract, useWaitForTransactionReceipt, useChainId } from 'wagmi'; | |
| import { mainnet } from 'wagmi/chains'; | |
| export default function BuyCoverPage() { | |
| const { address, isConnected, chain } = useAccount(); | |
| const explorerUrl = chain?.blockExplorers?.default?.url ?? 'https://etherscan.io'; | |
| // ... rest of component ... | |
| {isConfirmed && txHash && ( | |
| <div className="rounded-xl border border-green-200 bg-green-50 p-4 dark:border-green-900 dark:bg-green-950"> | |
| <p className="text-sm font-medium text-green-700 dark:text-green-300">Cover purchased!</p> | |
| <a | |
| href={`${explorerUrl}/tx/${txHash}`} | |
| target="_blank" | |
| rel="noopener noreferrer" | |
| className="mt-1 block text-sm text-green-600 underline dark:text-green-400" | |
| > | |
| View on Etherscan | |
| </a> | |
| </div> | |
| )} |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@examples/nextjs-wagmi/app/buy-cover/page.tsx` at line 350, The href attribute
in the transaction link is hardcoded to the mainnet Etherscan URL, which breaks
for users on other networks. Instead of using the hardcoded
https://etherscan.io/tx/ string, dynamically construct the explorer URL by
accessing the current chain information from wagmi (typically available through
useNetwork or from the chain object in your wagmi config) and use the
appropriate blockExplorers property from the chain data to build the correct
explorer URL for the current network before appending the txHash.
Comment on lines
+53
to
+69
| console.log('enters sig', { signature }); | ||
| const proofOfLoss = [buildProofOfLoss()]; | ||
| console.log('proof of loss', { proofOfLoss }); | ||
|
|
||
| const response = await sdk.cover.editCoverMetadata({ | ||
| coverMetadataId, | ||
| proofOfLoss, | ||
| signature: { | ||
| signature, | ||
| payload: typedData.value, | ||
| }, | ||
| }); | ||
|
|
||
| console.log('response', { response }); | ||
|
|
||
| if (response.error) { | ||
| console.log('error', { error: response.error }); |
There was a problem hiding this comment.
Remove debug console.log statements.
Multiple console.log statements are present in the production code (lines 53, 55, 66, 69). These should be removed before merging as they can leak sensitive information (signatures, proof of loss data) to browser consoles.
🧹 Proposed cleanup
});
- console.log('enters sig', { signature });
const proofOfLoss = [buildProofOfLoss()];
- console.log('proof of loss', { proofOfLoss });
const response = await sdk.cover.editCoverMetadata({
coverMetadataId,
proofOfLoss,
signature: {
signature,
payload: typedData.value,
},
});
- console.log('response', { response });
-
if (response.error) {
- console.log('error', { error: response.error });
throw new Error(response.error.message);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| console.log('enters sig', { signature }); | |
| const proofOfLoss = [buildProofOfLoss()]; | |
| console.log('proof of loss', { proofOfLoss }); | |
| const response = await sdk.cover.editCoverMetadata({ | |
| coverMetadataId, | |
| proofOfLoss, | |
| signature: { | |
| signature, | |
| payload: typedData.value, | |
| }, | |
| }); | |
| console.log('response', { response }); | |
| if (response.error) { | |
| console.log('error', { error: response.error }); | |
| const proofOfLoss = [buildProofOfLoss()]; | |
| const response = await sdk.cover.editCoverMetadata({ | |
| coverMetadataId, | |
| proofOfLoss, | |
| signature: { | |
| signature, | |
| payload: typedData.value, | |
| }, | |
| }); | |
| if (response.error) { | |
| throw new Error(response.error.message); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@examples/nextjs-wagmi/app/edit-metadata/page.tsx` around lines 53 - 69,
Remove all debug console.log statements from the editCoverMetadata function
block in the page.tsx file. Specifically, delete the four console.log calls: the
one logging the signature ("enters sig"), the one logging proofOfLoss, the one
logging the response, and the one logging the error. Keep all the actual
business logic intact, including the buildProofOfLoss call, the
sdk.cover.editCoverMetadata API call, and the error handling conditional check.
Only remove the logging statements to prevent sensitive data like signatures and
proof of loss information from being exposed to browser consoles.
| "@types/react": "^19", | ||
| "@types/react-dom": "^19", | ||
| "eslint": "^9", | ||
| "eslint-config-next": "16.2.9", |
There was a problem hiding this comment.
Verify eslint-config-next version compatibility.
The eslint-config-next version is set to 16.2.9, which matches the invalid Next.js version. This package version should align with the actual Next.js version being used.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@examples/nextjs-wagmi/package.json` at line 26, The eslint-config-next
package version set to 16.2.9 does not align with the actual Next.js version
being used in the project. Verify the Next.js version specified in the
dependencies of the package.json file and update the eslint-config-next version
to match the corresponding compatible version for that Next.js release, ensuring
consistency across both packages.
Comment on lines
+20
to
+31
| public async getCover(coverId: number): Promise<GetCoverApiResponse | ErrorApiResponse> { | ||
| if (!coverId || coverId <= 0) { | ||
| return { result: undefined, error: { message: 'Invalid coverId: must be a positive number' } }; | ||
| } | ||
|
|
||
| try { | ||
| const response = await this.sendRequest<GetCoverResponse>(`/cover/${coverId}`); | ||
| return { result: response, error: undefined }; | ||
| } catch (error: unknown) { | ||
| return { result: undefined, error: { message: (error as Error).message || 'Failed to fetch cover' } }; | ||
| } | ||
| } |
There was a problem hiding this comment.
Add validation for Infinity and -Infinity.
The current check !coverId || coverId <= 0 correctly rejects 0, NaN, and negative numbers, but allows Infinity to pass through. While the API would likely reject /cover/Infinity with a 404 or 400, catching this at the SDK layer provides a clearer error message to the caller.
🛡️ Suggested fix
- if (!coverId || coverId <= 0) {
- return { result: undefined, error: { message: 'Invalid coverId: must be a positive number' } };
+ if (!coverId || coverId <= 0 || !isFinite(coverId)) {
+ return { result: undefined, error: { message: 'Invalid coverId: must be a positive finite number' } };
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/cover/Cover.ts` around lines 20 - 31, The validation check in the
getCover method currently does not prevent Infinity and -Infinity values from
passing through since these are not caught by the `!coverId || coverId <= 0`
condition. Replace or enhance the validation condition to also check that
coverId is a finite number by using Number.isFinite(coverId) or by adding
explicit checks to reject Infinity and -Infinity values alongside the existing
checks for zero, negative numbers, and NaN.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Webapp built with Next.js, wagmi, tailwind and nexusmutual/sdk as a proof of concept for developers who will integrate our sdk
Summary by CodeRabbit
Release Notes
New Features
Updates