Redeploy 3 #26
Redeploy 3 #26
Conversation
📝 WalkthroughWalkthroughThis pull request updates the Zylith contract and USDC token addresses across the ASP protocol stack following a Sepolia contract migration. Changes include circuit artifact path relocations, frontend component refactoring, test data alignment, and configuration updates throughout documentation and setup files. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 (1)
frontend/src/components/StarknetProvider.tsx (1)
55-60: Hardcoded RPC URL removes environment flexibility and conflicts with comments.Several concerns with this change:
- Stale documentation: Comments mention using a "proxy API route to avoid CORS issues" but the code now directly uses a public RPC endpoint, which may reintroduce CORS issues.
- Environment inflexibility: The hardcoded Sepolia endpoint cannot be changed per environment (dev/staging/prod) without code modifications.
- Network mismatch: The URL is Sepolia-specific, but
StarknetConfigincludes bothsepoliaandmainnetchains (line 68). This provider will return the Sepolia endpoint regardless of which chain is active.- No fallback: Removes any fallback mechanism if the public endpoint is unavailable.
- Testing limitations: Makes it difficult to use local or custom RPC endpoints for testing.
🔎 Suggested approach: Use CONFIG or environment variables
Update the comments and restore environment-based configuration:
- // Create custom RPC provider using proxy API route to avoid CORS issues - // The proxy route (/api/rpc) forwards requests from server (no CORS) + // Create custom RPC provider with configurable endpoint const provider = useMemo(() => { - // Use proxy API route for RPC calls to avoid CORS - const proxyUrl = "https://starknet-sepolia-rpc.publicnode.com"; - + const proxyUrl = CONFIG.rpcUrl || "https://starknet-sepolia-rpc.publicnode.com"; const customProvider = new RpcProvider({ nodeUrl: proxyUrl }); // Return a function that matches the provider interface expected by StarknetConfig return (chain: any) => customProvider; - }, []); + }, [CONFIG.rpcUrl]);Alternatively, use environment variables:
const proxyUrl = process.env.NEXT_PUBLIC_RPC_URL || "https://starknet-sepolia-rpc.publicnode.com";
🧹 Nitpick comments (3)
zylith/CONTRACT_ADDRESS.md (1)
25-26: Consider wrapping URLs in markdown link syntax.The bare URLs trigger markdownlint warnings (MD034). Consider using proper markdown link syntax for better readability and compliance with markdown standards.
🔎 Proposed markdown formatting fix
-🔗 **Links** - -- Class → https://sepolia.starkscan.co/class/0x4bb0d80090bb558d1b6bf5562992ee7f4daf3f8d077bcad1bc87c413fa19ed0 -- Contract → https://sepolia.starkscan.co/contract/0x07fd7386f3b91ec5e130aafb85da7fe3cbfa069beb080789150c4b75efc5c9ef +🔗 **Links** + +- [Class](https://sepolia.starkscan.co/class/0x4bb0d80090bb558d1b6bf5562992ee7f4daf3f8d077bcad1bc87c413fa19ed0) +- [Contract](https://sepolia.starkscan.co/contract/0x07fd7386f3b91ec5e130aafb85da7fe3cbfa069beb080789150c4b75efc5c9ef)Based on static analysis hints.
frontend/src/components/swap/SwapInterface.tsx (2)
178-209: Consider removing debug logging for production.The
useEffectfor debug logging includesbuttonStatein its dependency array. SincegetButtonState()returns a new object on each render, this effect will run on every render. For production, consider removing this debug block entirely or memoizingbuttonState.🔎 Option: Memoize buttonState if keeping debug logs
+import { useMemo } from "react"; - const buttonState = getButtonState(); + const buttonState = useMemo(() => getButtonState(), [ + isLoading, + isValidAmount, + isPrivate, + availableNotes.length, + selectedNote, + amount, + inputToken.symbol, + ]);
496-547: Debug UI section visible in production.The debug info section is intentionally always visible ("Always show for troubleshooting"). For a Sepolia testnet deployment this is reasonable, but ensure this is removed or hidden behind a feature flag before mainnet deployment.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
frontend/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (17)
asp/README.mdasp/src/abis/zylith-abi.jsonasp/src/bin/initialize_pool.rsasp/src/main.rsasp/src/proof.rsasp/start.shdocs/integration/frontend-integration-guide.mdfrontend/README.mdfrontend/package.jsonfrontend/src/components/StarknetProvider.tsxfrontend/src/components/portfolio/__tests__/NotesList.test.tsxfrontend/src/components/swap/SwapInterface.tsxfrontend/src/hooks/__tests__/use-private-deposit.test.tsfrontend/src/lib/config.tsfrontend/src/lib/proof-service.tszylith/CONTRACT_ADDRESS.mdzylith/src/zylith.cairo
🧰 Additional context used
🧬 Code graph analysis (2)
frontend/src/components/StarknetProvider.tsx (1)
frontend/src/components/shared/NotificationToast.tsx (2)
useNotifications(110-127)NotificationToast(27-105)
frontend/src/lib/proof-service.ts (1)
circuits/scripts/generate_proof.js (1)
circuitName(146-146)
🪛 LanguageTool
asp/README.md
[grammar] ~35-~35: Corrige la mayúscula.
Context: ...scripción | Valor por Defecto | | ------------------ | --------...
(QB_NEW_ES_OTHER_ERROR_IDS_REPLACEMENT_ORTHOGRAPHY_UPPERCASE)
🪛 markdownlint-cli2 (0.18.1)
zylith/CONTRACT_ADDRESS.md
25-25: Bare URL used
(MD034, no-bare-urls)
26-26: Bare URL used
(MD034, no-bare-urls)
🔇 Additional comments (18)
frontend/src/lib/config.ts (1)
5-5: LGTM! Contract address updates are consistent.The ZYLITH_CONTRACT and USDC configuration updates align with the Sepolia contract migration mentioned in the PR summary. The changes are consistent across the repository.
Also applies to: 49-52
zylith/src/zylith.cairo (1)
182-191: LGTM! Formatting-only changes.These changes only reflow long comments without modifying any logic or functionality. The code behavior remains unchanged.
Also applies to: 234-245, 247-258
asp/start.sh (1)
34-34: LGTM! Default contract address updated.The default CONTRACT_ADDRESS has been updated to match the new Sepolia deployment, consistent with changes in other configuration files.
zylith/CONTRACT_ADDRESS.md (1)
19-20: LGTM! Contract address documentation updated.The Class Hash and Contract addresses have been updated to reflect the new Sepolia deployment, consistent with changes throughout the repository.
asp/src/main.rs (1)
66-66: LGTM! Default addresses updated for Sepolia deployment.Both the ZYLITH_CONTRACT default address (line 66) and USDC token address (line 1133) have been updated consistently with the contract migration reflected throughout the repository.
Also applies to: 1133-1133
docs/integration/frontend-integration-guide.md (1)
47-47: LGTM! Documentation updated to reflect new contract addresses.The integration guide has been updated with the new Sepolia contract addresses, ensuring the documentation stays synchronized with the deployed contracts.
Also applies to: 243-243
asp/src/proof.rs (1)
34-35: The circuit artifact paths are correctly configured. The.zkeyand.wasmfiles are intentionally git-ignored build artifacts (defined in.gitignore), not repository files. They are generated during the circuit compilation process. The code already validates these paths exist at runtime (lines 42-47) with appropriate error handling, so no further verification is needed.frontend/package.json (1)
49-49: The ESLint 9 upgrade appears to be properly implemented. The project useseslint.config.mjsin the correct flat config format, with proper integration ofeslint-config-nextusing ESLint 9'sdefineConfigAPI. Version 16.1.1 ofeslint-config-nextofficially supports ESLint 9, and no legacy.eslintrc.*files exist to create conflicts. No further action needed.asp/src/abis/zylith-abi.json (1)
1-763: ABI structure normalization looks correct.The changes are purely formatting/schema adjustments (reordering
typekeys, adding"kind": "data"annotations to event members). The contract interface, function signatures, inputs/outputs, and state mutability remain unchanged.frontend/src/lib/proof-service.ts (1)
89-149: Proof generation logic looks correct.The error handling, logging, and proof formatting are well-structured. The validation and proof generation flow correctly handles both small and large circuits with appropriate progress logging.
frontend/src/components/swap/SwapInterface.tsx (2)
44-68: Hydration safety pattern correctly implemented.Using
isMountedstate withuseEffectto defer client-side state initialization prevents React hydration mismatches. The conditional access togetNotesByTokenandgetTotalBalanceis appropriate.
74-117: BigInt decimal parsing logic is well-implemented.The handling of decimal amounts with proper string manipulation to avoid floating-point precision loss is correct. Edge cases like empty strings, "0", "0.0", and invalid inputs are properly handled.
frontend/README.md (1)
47-50: Contract address updated correctly in documentation.The
NEXT_PUBLIC_ZYLITH_CONTRACTenvironment variable example reflects the new contract address, consistent with the redeployment changes across the codebase.frontend/src/components/portfolio/__tests__/NotesList.test.tsx (1)
126-164: Test mock data correctly updated with new contract address.The
from_addressvalues in mock events are updated to match the new Zylith contract address, ensuring test consistency with the redeployment.frontend/src/hooks/__tests__/use-private-deposit.test.ts (1)
21-26: Mock configuration correctly updated with new contract address.The mocked
CONFIG.ZYLITH_CONTRACTvalue is updated to the new deployment address, maintaining test validity.asp/src/bin/initialize_pool.rs (1)
19-37: Configuration defaults updated correctly for redeployment.The default
ZYLITH_CONTRACTandUSDCtoken addresses are updated to match the new Sepolia deployment. Good security practice maintaining required environment variables forPRIVATE_KEYandACCOUNT_ADDRESSwithout defaults.asp/README.md (1)
21-46: Contract address and documentation updated correctly.The
CONTRACT_ADDRESSvalues are consistently updated to the new deployment address. The table formatting improvements are clean.Regarding the static analysis hint about capitalization of "Defecto" - this is a table header in Spanish where title-case capitalization is acceptable. This appears to be a false positive.
frontend/src/components/StarknetProvider.tsx (1)
15-18: Formatting improvements look good.The formatting changes across imports, destructuring, and JSX improve readability and follow common React conventions. No behavioral impact.
Also applies to: 23-24, 35-38, 46-46, 50-50, 75-75
| useNotifications, | ||
| } from "@/components/shared/NotificationToast"; | ||
| import { setNotificationHandler } from "@/hooks/use-contract-events"; | ||
| import { CONFIG } from "@/lib/config"; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if CONFIG is used anywhere in the StarknetProvider.tsx file
rg -n '\bCONFIG\b' frontend/src/components/StarknetProvider.tsxRepository: SunsetLabs-Game/Zylith
Length of output: 107
🏁 Script executed:
cat -n frontend/src/components/StarknetProvider.tsxRepository: SunsetLabs-Game/Zylith
Length of output: 3033
Remove unused import and fix misleading comments.
Line 20: Remove the unused CONFIG import from @/lib/config.
Lines 55–58: The comments mention a proxy API route (/api/rpc) to avoid CORS, but the code directly uses the hardcoded public endpoint https://starknet-sepolia-rpc.publicnode.com. Update comments to reflect the actual implementation, or consider whether a configurable RPC endpoint would be preferable.
🤖 Prompt for AI Agents
In frontend/src/components/StarknetProvider.tsx around line 20 and lines 55–58,
remove the unused import "CONFIG" from "@/lib/config" (it's not referenced
anywhere) and update the misleading comment about using a proxy API route;
either change the comment to accurately state that the code uses the hardcoded
public RPC endpoint "https://starknet-sepolia-rpc.publicnode.com" or refactor to
read the RPC URL from props/environment (e.g., a configurable env var or
passed-in prop) and use that value instead, ensuring comments reflect the chosen
approach.
| // Circuit paths match the structure from generate_proof.js | ||
| // Format: out/{circuit}_js/{circuit}.wasm and out/{circuit}_final.zkey | ||
| const wasmPath = path.join(this.circuitsPath, "out", `${circuitName}_js`, `${circuitName}.wasm`); | ||
| const zkeyPath = path.join(this.circuitsPath, "out", `${circuitName}_final.zkey`); | ||
| const wasmPath = path.join( | ||
| this.circuitsPath, | ||
| "build", | ||
| `${circuitName}`, | ||
| `${circuitName}_js`, | ||
| `${circuitName}.wasm` | ||
| ); | ||
| const zkeyPath = path.join( | ||
| this.circuitsPath, | ||
| "build", | ||
| "zkey", | ||
| `${circuitName}.zkey` | ||
| ); |
There was a problem hiding this comment.
Stale comment references old path structure.
The comment on lines 61-62 still references out/{circuit}_js/{circuit}.wasm and out/{circuit}_final.zkey, but the actual paths now use build/{circuit}/{circuit}_js/{circuit}.wasm and build/zkey/{circuit}.zkey.
🔎 Proposed fix
/**
* Generate a Groth16 proof using snarkjs
*/
async generateProof(circuitName: string, inputs: any): Promise<ProofResult> {
- // Circuit paths match the structure from generate_proof.js
- // Format: out/{circuit}_js/{circuit}.wasm and out/{circuit}_final.zkey
+ // Circuit paths match the structure from generate_proof.js
+ // Format: build/{circuit}/{circuit}_js/{circuit}.wasm and build/zkey/{circuit}.zkey
const wasmPath = path.join(📝 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.
| // Circuit paths match the structure from generate_proof.js | |
| // Format: out/{circuit}_js/{circuit}.wasm and out/{circuit}_final.zkey | |
| const wasmPath = path.join(this.circuitsPath, "out", `${circuitName}_js`, `${circuitName}.wasm`); | |
| const zkeyPath = path.join(this.circuitsPath, "out", `${circuitName}_final.zkey`); | |
| const wasmPath = path.join( | |
| this.circuitsPath, | |
| "build", | |
| `${circuitName}`, | |
| `${circuitName}_js`, | |
| `${circuitName}.wasm` | |
| ); | |
| const zkeyPath = path.join( | |
| this.circuitsPath, | |
| "build", | |
| "zkey", | |
| `${circuitName}.zkey` | |
| ); | |
| // Circuit paths match the structure from generate_proof.js | |
| // Format: build/{circuit}/{circuit}_js/{circuit}.wasm and build/zkey/{circuit}.zkey | |
| const wasmPath = path.join( | |
| this.circuitsPath, | |
| "build", | |
| `${circuitName}`, | |
| `${circuitName}_js`, | |
| `${circuitName}.wasm` | |
| ); | |
| const zkeyPath = path.join( | |
| this.circuitsPath, | |
| "build", | |
| "zkey", | |
| `${circuitName}.zkey` | |
| ); |
🤖 Prompt for AI Agents
In frontend/src/lib/proof-service.ts around lines 61 to 75, the inline comment
describing circuit output paths is stale (mentions
out/{circuit}_js/{circuit}.wasm and out/{circuit}_final.zkey) while the code
builds paths using build/{circuit}/{circuit}_js/{circuit}.wasm and
build/zkey/{circuit}.zkey; update the comment to reflect the current path
structure (build/{circuit}/{circuit}_js/{circuit}.wasm and
build/zkey/{circuit}.zkey) and remove any mention of the old out/... and
_final.zkey naming so the comment matches the code.
2 participants
Summary by CodeRabbit
New Features
Documentation
Chores
✏️ Tip: You can customize this high-level summary in your review settings.