L2 contracts re-deployment#119
L2 contracts re-deployment#119JoE11-y merged 4 commits intoExplore-Beyond-Innovations:mainfrom truthixify:fix-117
Conversation
WalkthroughBumped StarkNet SDK, updated Sepolia RPC and deployment config, refreshed Sepolia deployment records, added ABI JSONs for L2 contracts, updated deploy script to fetch/save ABIs and pass a DeploymentConfig, and removed three debug println! calls from ZeroXBridgeL2.cairo. Changes
Sequence Diagram(s)sequenceDiagram
participant DeployScript as deploy_l2.ts
participant StarknetRPC as RpcProvider
participant FS as FileSystem
Note over DeployScript,StarknetRPC: For each contract (declare → deploy → fetch ABI)
DeployScript->>StarknetRPC: declare(contractClass)
StarknetRPC-->>DeployScript: classHash / declareTx
DeployScript->>StarknetRPC: deploy(contract, constructor_args)
StarknetRPC-->>DeployScript: deploymentTx / address
DeployScript->>StarknetRPC: getClassAt(address) %% fetch ABI
StarknetRPC-->>DeployScript: classDefinition / abi
DeployScript->>FS: writeFile("contracts/L2/abi/{contractName}.abi.json", abi)
FS-->>DeployScript: write confirmation
Note right of DeployScript: logs "Processing {contractName}" and "Saving ABI for {contractName} to {outPath}"
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Possibly related PRs
Suggested reviewers
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (1)
🧰 Additional context used🧬 Code graph analysis (1)contracts/L2/scripts/deploy_l2.ts (1)
🔇 Additional comments (6)
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
contracts/L2/scripts/deploy_l2.ts (1)
14-16: RPC defaults updated to v0_8 are appropriate; consider tightening devnet URL and adding a chain sanity check.
- Good update to v0_8 endpoints for sepolia/mainnet.
- Optional: Devnet often expects the /rpc path; using it avoids 404s on some builds.
- Optional: Log chainId early to fail fast if a wrong RPC is supplied.
Apply this minimal tweak if you want the devnet default to be explicit:
const DEFAULT_RPC_URLS = { - sepolia: "https://starknet-sepolia.public.blastapi.io/rpc/v0_8", - mainnet: "https://starknet-mainnet.public.blastapi.io/rpc/v0_8", - devnet: "http://127.0.0.1:5050" + sepolia: "https://starknet-sepolia.public.blastapi.io/rpc/v0_8", + mainnet: "https://starknet-mainnet.public.blastapi.io/rpc/v0_8", + devnet: "http://127.0.0.1:5050/rpc" };If you decide to add a chain sanity check (outside this hunk), you can do:
// After constructing provider const { chainId } = await provider.getChainId(); console.log(`Connected to chainId: ${chainId}`);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (2)
contracts/L2/package-lock.jsonis excluded by!**/package-lock.jsoncontracts/L2/yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (5)
contracts/L2/package.json(1 hunks)contracts/L2/scripts/.deploy-config-sepolia.json(1 hunks)contracts/L2/scripts/deploy_l2.ts(2 hunks)contracts/L2/scripts/deployments-sepolia.json(1 hunks)contracts/L2/src/core/ZeroXBridgeL2.cairo(0 hunks)
💤 Files with no reviewable changes (1)
- contracts/L2/src/core/ZeroXBridgeL2.cairo
🧰 Additional context used
🧬 Code graph analysis (4)
contracts/L2/scripts/.deploy-config-sepolia.json (1)
contracts/L1/scripts/deploy_l1.ts (2)
deploymentTestnet(298-321)DeploymentConfig(34-38)
contracts/L2/scripts/deploy_l2.ts (1)
contracts/L1/scripts/deploy_l1.ts (1)
deploymentTestnet(298-321)
contracts/L2/package.json (1)
contracts/L1/scripts/deploy_l1.ts (1)
deployAllContracts(250-293)
contracts/L2/scripts/deployments-sepolia.json (1)
contracts/L1/scripts/deploy_l1.ts (3)
deployAllContracts(250-293)deploymentTestnet(298-321)main(375-407)
🔇 Additional comments (4)
contracts/L2/package.json (1)
24-24: No action needed:starknet@7.6.4is fully compatible with your current setupAll checks pass for CJS/ESM support and Node version:
- Runtime Node version is v24.3.0 (≥18), so conditional exports are supported.
contracts/L2/package.jsonhas no"type"field (defaults to CJS), matching your CommonJS build.contracts/L2/tsconfig.jsonuses
•compilerOptions.module = "CommonJS"
•compilerOptions.moduleResolution = "node"
which will correctly resolve the CJS entry (dist/index.js) under"exports".requirerun1- The published
starknet@7.6.4package declares:
•"main": "dist/index.js"
•"module": "dist/index.mjs"
• Conditional exports for bothimport(ESM) andrequire(CJS) entrypoints run2Your deploy scripts and TS/ts-node setup can continue importing via CommonJS without any ESM-only or Node-version constraints.
contracts/L2/scripts/deploy_l2.ts (1)
363-363: Per-contract header log is a nice DX improvement.The additional header improves traceability across declare/deploy steps during long runs.
contracts/L2/scripts/.deploy-config-sepolia.json (1)
3-5: ✔ Configuration Consistency VerifiedAll automated checks passed for
contracts/L2/scripts/.deploy-config-sepolia.json:
- minRate (1 × 10¹⁸) ≤ maxRate (2 × 10¹⁸)
accountAddressandownerare valid hex strings and match each otherPlease ensure your
STARKNET_PRIVATE_KEYcorresponds to the configuredaccountAddressbefore deploying.contracts/L2/scripts/deployments-sepolia.json (1)
3-8: Review completed: timestamps and formatting validated
- No timestamps exceed the current UTC time (all ≤ 1756214192510), so there are no future-dated entries.
- Every
address,classHash, anddeploymentTxfield is correctly formatted as a hex string.- ZeroXBridgeL2 still has
declareTx: "already_declared"but a newclassHash. Please verify on-chain via StarkNet RPC (e.g.starknet_getClassHashAt) that this reflects an intentionally pre-declared class and not a deployment mismatch.
|
Hi @truthixify help update https://github.com/Explore-Beyond-Innovations/ZeroXBridge_Contracts/blob/main/contracts/L2/README.md With the latest contracts. And you forgot to dump the abi files |
JoE11-y
left a comment
JoE11-y
left a comment
There was a problem hiding this comment.
Just finish it up and this is ready to merge
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 (2)
contracts/L2/scripts/deploy_l2.ts (2)
334-337: Constructor calldata must be compiled; raw u256 objects will fail.account.deployContract expects constructorCalldata as a flattened array (felt[]). Passing objects like {low, high} for u256 without compilation will break especially for ZeroXBridgeL2’s min_rate/max_rate.
Compile the args before deploy:
- const deployResponse = await account.deployContract({ - classHash: classHash, - constructorCalldata: constructorArgs, - }); + const constructorCalldata = CallData.compile(constructorArgs); + const deployResponse = await account.deployContract({ + classHash, + constructorCalldata, + });No other call sites need changing; this covers L2Oracle/xZBERC20 (felt) and ZeroXBridgeL2 (u256 structs) uniformly.
Also applies to: 483-490
1-1: Ensure README.md Includes All Deployed Contract AddressesThe automated consistency check against
contracts/L2/scripts/deployments-sepolia.jsonreveals that L2Oracle and ProofRegistry are missing fromcontracts/L2/README.md, causing a mismatch in documented vs. actual deployed addresses.Please update
contracts/L2/README.mdto include the following entries (sorted to match the JSON):L2Bridge=0x58bdc4e4f39b648c01761324247d98ea68bc9fd5e3541e732d146385e575a63 xZBERC20=0x26cfc43ee6e47541c1de22e91382c14c2c524de52d6abdd1275e7b37cb01276 +L2Oracle=0x1e4acab001e40d194f15927bb0e832887a219809d0be6cf308f3e41a4cfd246 +ProofRegistry=0x3a5d9d2fce8a9dac6a6b5c8cade5668b19e30248611159c7788236a49ce8b9a– File to update:
•contracts/L2/README.md
🧹 Nitpick comments (13)
contracts/L2/README.md (2)
62-63: RPC bump to v0_8 looks good, but .env example still shows v0_7 — please align.Default RPC URLs are now v0_8. The “Environment Setup” example earlier still uses
/v0_7, which can mislead deployers.Update the .env snippet to v0_8 (illustrative snippet):
# Starknet Configuration STARKNET_PRIVATE_KEY=your_private_key_here STARKNET_ACCOUNT_ADDRESS=your_account_address_here STARKNET_RPC_URL=https://starknet-sepolia.public.blastapi.io/rpc/v0_8
71-77: Verified consistency of deployed addresses – All four contract addresses incontracts/L2/README.mdmatch those incontracts/L2/scripts/deployments-sepolia.json, so there’s no drift at the moment.You may still want to automate or simplify this section to prevent future mismatches:
- Option 1: Generate the markdown table (or list) directly from
deployments-sepolia.jsonin CI (e.g. via a small script) so it’s always up to date.- Option 2: Link to the JSON file and include only a brief summary in the README (for example, “See the full list of deployed addresses in
deployments-sepolia.json”).contracts/L2/abi/ProofRegistry.abi.json (1)
21-77: Confirm external mutator access policy for register_deposit_proof.ABI alone doesn’t convey authorization. If anyone can call and write the same commitment twice, ensure idempotency and proper revert on conflicts. If restricted, please confirm the guard (e.g., proof submitter allowlist).
contracts/L2/abi/L2Oracle.abi.json (1)
35-79: Please confirm only-owner enforcement on setters.Given set_total_tvl and set_relayer are external, verify the implementation enforces onlyOwner (or equivalent) and emits the corresponding events consistently.
contracts/L2/abi/xZBERC20.abi.json (1)
118-121: AccessControl and ERC20 mixins present — verify mint/burn gating matches bridge design.Please confirm:
- mint is restricted to the bridge (or a dedicated MINTER_ROLE),
- burn is restricted appropriately (caller’s own balance vs. bridge-controlled burn-from),
- set_bridge_address is owner- or admin-only.
Also applies to: 355-549
contracts/L2/scripts/deployments-sepolia.json (1)
1-33: Consider adding metadata for reproducibility and CI drift checks.Optional enhancements:
- Add compilerVersion and cairo/oz component versions used,
- Add abiSaved: true and abiCommit hash to tie ABIs to deployments,
- Add a simple schemaVersion field.
contracts/L2/scripts/deploy_l2.ts (7)
19-21: Make state/config file paths deterministic regardless of cwd.getConfigFilename/getDeploymentsFilename return relative scripts/*.json, which depends on process cwd. Running the CLI from repo root will write to ./scripts instead of contracts/L2/scripts.
-const getConfigFilename = (network: string) => `scripts/.deploy-config-${network}.json`; -const getDeploymentsFilename = (network: string) => `scripts/deployments-${network}.json`; +const getConfigFilename = (network: string) => + path.join(__dirname, `.deploy-config-${network}.json`); +const getDeploymentsFilename = (network: string) => + path.join(__dirname, `deployments-${network}.json`);save/load functions already ensure directories; this change makes outputs consistently land under contracts/L2/scripts/.
Also applies to: 153-170, 190-199
283-285: Use utf8 when reading JSON files.toString("ascii") risks mangling non-ASCII characters. utf8 is standard and safer for JSON.
-const sierraContract = json.parse(fs.readFileSync(sierraPath).toString("ascii")); -const casmContract = json.parse(fs.readFileSync(casmPath).toString("ascii")); +const sierraContract = json.parse(fs.readFileSync(sierraPath, "utf8")); +const casmContract = json.parse(fs.readFileSync(casmPath, "utf8"));
301-317: Broaden "already declared" handling to be more robust.Relying on error.message string matching is brittle. starknet.js can surface provider error structures with different wording.
- Check for error.code/classAlreadyDeclared-like markers if present (e.g., error.response?.data?.error?.code).
- As a fallback, if declare fails, query getClassByHash to confirm existence and proceed.
If you want, I can draft a helper normalizeStarknetError(e) to centralize this.
531-541: Optional: reuse the existing provider via account.provider.You don’t need to instantiate an extra RpcProvider in declareAndDeploy; account.provider implements getClassAt.
-const rpc = new RpcProvider({ nodeUrl: config.starknetRpcUrl! }); -const { abi } = await rpc.getClassAt(address); +// Reuse the existing provider +const { abi } = await (account as any).provider.getClassAt(address);If you prefer type safety, type the provider as ProviderInterface.
500-527: Post-deploy relationship setup — LGTM with a minor improvement.execute + wait is correct. Consider also logging the transaction hash to aid debugging and explorers.
- spinner.info("Bridge address set in xZBERC20 contract"); + spinner.info(`Bridge address set in xZBERC20 contract (tx: ${response.transaction_hash})`);
590-602: Interactive flows are fine; consider a non-interactive mode for CI.You already support --no-config. A future enhancement is to accept all config via env/flags to run headless in CI, skipping ask() prompts.
If useful, I can add a --ci flag that forces defaults/envs and errors out if required values are missing instead of prompting.
Also applies to: 607-627, 632-643
581-585: Tiny UX nit: use path.resolve for displayed filenames.Printing absolute paths reduces confusion about where files were written if cwd varies.
- console.log(`💾 Deployment state saved to: ${deploymentsFilename}`); - console.log(`⚙️ Configuration saved to: ${configFilename}\n`); + console.log(`💾 Deployment state saved to: ${path.resolve(deploymentsFilename)}`); + console.log(`⚙️ Configuration saved to: ${path.resolve(configFilename)}\n`);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (7)
contracts/L2/README.md(1 hunks)contracts/L2/abi/L2Oracle.abi.json(1 hunks)contracts/L2/abi/ProofRegistry.abi.json(1 hunks)contracts/L2/abi/ZeroXBridgeL2.abi.json(1 hunks)contracts/L2/abi/xZBERC20.abi.json(1 hunks)contracts/L2/scripts/deploy_l2.ts(9 hunks)contracts/L2/scripts/deployments-sepolia.json(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
contracts/L2/scripts/deployments-sepolia.json (1)
contracts/L1/scripts/deploy_l1.ts (2)
deployAllContracts(250-293)main(375-407)
contracts/L2/scripts/deploy_l2.ts (1)
contracts/L1/scripts/deploy_l1.ts (1)
DeploymentConfig(34-38)
🪛 LanguageTool
contracts/L2/README.md
[grammar] ~62-~62: There might be a mistake here.
Context: ... #### Default RPC URLs: - Sepolia: https://starknet-sepolia.public.blastapi.io/rpc/v0_8 - Mainnet: `https://starknet-mainnet.pub...
(QB_NEW_EN)
[grammar] ~63-~63: There might be a mistake here.
Context: ...ic.blastapi.io/rpc/v0_8- **Mainnet**:https://starknet-mainnet.public.blastapi.io/rpc/v0_8` - Devnet: http://127.0.0.1:5050 ## D...
(QB_NEW_EN)
⏰ 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: Cairo starknet foundry tests
🔇 Additional comments (11)
contracts/L2/abi/ProofRegistry.abi.json (1)
1-7: ABI structure is coherent and minimal — good separation of concerns.The interface cleanly exposes read functions and a single write path; event naming is precise.
contracts/L2/abi/L2Oracle.abi.json (2)
1-7: LGTM on public surface and Ownable integration.The ABI exposes clear TVL getters/setters and relayer management with Ownable mixin present.
150-251: Event surface is complete and consistent.Event typing and the unified Event enum composition are well-structured; this should ease indexer integration.
contracts/L2/abi/xZBERC20.abi.json (1)
740-765: Unified Event enum is well-flattened for indexers.Nice flattening across ERC20, AccessControl, SRC5, and Upgradeable events — this aids analytics.
contracts/L2/scripts/deployments-sepolia.json (2)
1-33: Cross-check with README — currently consistent.Oracle/ProofRegistry/xZBERC20/ZeroXBridgeL2 addresses here match the README. Good job keeping them in sync.
3-8: Timestamps sanity check: actual UTC times
I converted each millisecond value in contracts/L2/scripts/deployments-sepolia.json to UTC and got:
- ProofRegistry (line 3): 2025-08-26T13:34:20Z
- L2Oracle (line 4): 2025-08-26T13:34:53Z
- xZBERC20 (line 5): 2025-08-26T13:35:25Z
- ZeroXBridgeL2 (line 6): 2025-08-26T13:36:37Z
These all fall minutes ago on Aug 26 2025. Please confirm they match your actual Sepolia deployment logs (and aren’t placeholder or accidentally future-dated) before merging.
contracts/L2/scripts/deploy_l2.ts (4)
14-15: RPC bumped to v0_8 — LGTM.Matches the README and aligns with starknet.js 7.x. No changes needed.
548-558: Call-site updates look consistent.All deploy* functions now pass config through to declareAndDeploy. Good propagation and consistent logging.
Also applies to: 403-416, 432-436, 453-456, 492-495
106-147: Private key guard — good. Also default behavior on blank input is sensible.The “load config?” default to yes when input is blank is a nice UX touch.
532-585: Nice final summary output.Clear, readable deployment recap and file locations. No action needed.
contracts/L2/abi/ZeroXBridgeL2.abi.json (1)
1-679: ABI surface looks coherent and complete for Cairo 1.Interfaces/events cover mint/burn, dynamic rates, Merkle verification, upgradeability, and ownership. No structural issues spotted. Good to commit.
2 participants
This PR fixes an issue with L2 contract deployment.
Changes
println!statements that caused contract declaration failuresCloses #117
Summary by CodeRabbit
New Features
Refactor
Documentation
Chores