Skip to content

Conversation

@immrsd
Copy link
Contributor

@immrsd immrsd commented Aug 12, 2025

Summary by CodeRabbit

  • New Features

    • Added support for OpenZeppelin Contracts for Cairo 3.0.0-alpha.0.
  • Bug Fixes

    • Corrected alpha-version detection in the UI using proper semantic version comparison.
    • Ensured Votes component correctly forwards the optional section parameter.
  • Documentation

    • Added an Unreleased changelog entry noting the breaking upgrade to Cairo 3.0.0-alpha.0.
    • Updated snapshot headers to reflect Cairo 3.0.0-alpha.0 compatibility.
  • Chores

    • Bumped package and tool versions (Cairo/Scarb/starknet) to 2.12.0 and aligned dependencies with Cairo 3.0.0-alpha.0.

@immrsd immrsd self-assigned this Aug 12, 2025
@immrsd immrsd requested review from a team as code owners August 12, 2025 12:05
@socket-security
Copy link

socket-security bot commented Aug 12, 2025

Review the following changes in direct dependencies. Learn more about Socket for GitHub.

Diff Package Supply Chain
Security
Vulnerability Quality Maintenance License
Addedjszip@​3.10.1100100837770
Addedsolidity-ast@​0.4.6010010010080100
Updatedhardhat@​2.26.2 ⏵ 2.26.394 +110090 +1100 +180
Added@​openzeppelin/​contracts-upgradeable@​5.4.010010010094100

View full report

Copy link
Member

@ericnordelo ericnordelo left a comment

Choose a reason for hiding this comment

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

Content LGTM! I think we are missing formatting for workflows to pass.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Aug 12, 2025

Summary by CodeRabbit

  • New Features

    • Added support for OpenZeppelin Contracts for Cairo 3.0.0-alpha.0.
  • Bug Fixes

    • Corrected alpha-version detection in the UI using proper semantic version comparison.
    • Ensured Votes component correctly forwards the optional section parameter.
  • Documentation

    • Added an Unreleased changelog entry noting the breaking upgrade to Cairo 3.0.0-alpha.0.
    • Updated snapshot headers to reflect Cairo 3.0.0-alpha.0 compatibility.
  • Chores

    • Bumped package and tool versions (Cairo/Scarb/starknet) to 2.12.0 and aligned dependencies with Cairo 3.0.0-alpha.0.

Walkthrough

The update bumps Cairo/OpenZeppelin Contracts to 3.0.0-alpha.0 across the Cairo alpha package, updates version pins (Cairo/Scarb), adjusts snapshot headers, renames a parameter in addVotesComponent (both cairo and cairo_alpha), adds an ERC6372TimestampClock use clause, updates version constants, modifies selection logic to use semver.compare, and updates Scarb.toml dependencies.

Changes

Cohort / File(s) Summary
Versioning and constants
packages/core/cairo_alpha/package.json, packages/core/cairo_alpha/src/utils/version.ts, packages/core/cairo_alpha/test_project/Scarb.toml
Bump package version to 3.0.0-alpha.0; update contractsVersion to 3.0.0-alpha.0; cairo/scarb versions to 2.12.0; Scarb.toml pins starknet 2.12.0 and openzeppelin v3.0.0-alpha.0.
Changelog
packages/core/cairo_alpha/CHANGELOG.md
Add Unreleased section noting Cairo 3.0.0-alpha.0 migration (breaking change).
Common components (API tweak)
packages/core/cairo/src/common-components.ts, packages/core/cairo_alpha/src/common-components.ts
Rename addVotesComponent param: section? → snip12MetadataSection?; update internal call. cairo_alpha also adds use clause for openzeppelin::utils::contract_clock ERC6372TimestampClock.
UI selection logic
packages/ui/src/main.ts
Change Cairo branch comparison to use semver.compare(cairoVersion, cairoAlphaVersion) !== 0 instead of direct inequality.
Snapshot header updates (text only)
packages/core/cairo_alpha/src/account.test.ts.md, .../contract.test.ts.md, .../custom.test.ts.md, .../erc1155.test.ts.md, .../erc721.test.ts.md, .../governor.test.ts.md, .../multisig.test.ts.md, .../vesting.test.ts.md
Replace “Compatible with OpenZeppelin Contracts for Cairo 2.0.0” with “3.0.0-alpha.0” across snapshots; no logic changes.
ERC20 snapshots with structural inserts
packages/core/cairo_alpha/src/erc20.test.ts.md
Beyond header updates, add SNIP12Metadata/SNIP12MetadataImpl blocks, VotesImpl embeddings, and ERC6372TimestampClock imports across multiple snapshots.

Sequence Diagram(s)

sequenceDiagram
  participant User
  participant UI as UI evaluateSelection
  participant Semver as semver
  User->>UI: Select Cairo toolchain
  UI->>Semver: compare(cairoVersion, cairoAlphaVersion)
  Semver-->>UI: -1 / 0 / 1
  alt result !== 0
    UI-->>User: Select cairo_alpha
  else result === 0
    UI-->>User: Select cairo
  end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

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.

✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 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.

@socket-security
Copy link

socket-security bot commented Aug 12, 2025

All alerts resolved. Learn more about Socket for GitHub.

This PR previously contained dependency changes with security issues that have been resolved, removed, or ignored.

View full report

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

🔭 Outside diff range comments (1)
packages/core/cairo_alpha/src/common-components.ts (1)

72-81: Propagate section to VotesImpl for consistent code sectioning

addSRC5Component propagates the optional section to its impl; do the same here to keep generated layout deterministic.

 export function addVotesComponent(c: ContractBuilder, name: string, version: string, section?: string) {
   addSNIP12Metadata(c, name, version, section);
   c.addComponent(components.NoncesComponent, [], false);
   c.addComponent(components.VotesComponent, [], false);
   c.addImplToComponent(components.VotesComponent, {
     name: 'VotesImpl',
-    value: `VotesComponent::VotesImpl<ContractState>`,
-  });
+    value: `VotesComponent::VotesImpl<ContractState>`,
+    section,
+  });
   c.addUseClause('openzeppelin::utils::contract_clock', 'ERC6372TimestampClock');
 }
🧹 Nitpick comments (4)
packages/core/cairo_alpha/test_project/Scarb.toml (1)

5-10: Version pins updated to Cairo/Scarb 2.12.0 and OZ Cairo v3.0.0-alpha.0 — looks good

Pins align with the target toolchain. Consider pinning the git dependency to a specific commit for stronger reproducibility.

-openzeppelin = { git = "https://github.com/OpenZeppelin/cairo-contracts", tag = "v3.0.0-alpha.0" }
+openzeppelin = { git = "https://github.com/OpenZeppelin/cairo-contracts", tag = "v3.0.0-alpha.0", rev = "<commit-sha-for-tag>" }

Replace with the exact commit of the tag to guard against retags.

packages/core/cairo_alpha/CHANGELOG.md (1)

3-7: Changelog entry added; consider noting pinned Cairo/Scarb versions for clarity.
Optionally include “Requires Cairo 2.12.0 and Scarb 2.12.0” in the Unreleased notes to help users align toolchains with this alpha bump.

packages/ui/src/main.ts (2)

73-75: Prefer semver comparison over string casts for clarity and type-safety

Casting to string silences TS literal-type warnings but obscures intent. Using semver.compare (or eq/neq) avoids casts and clarifies semantics.

Apply this diff:

-        (semver.satisfies(requestedVersion, cairoAlphaSemver) &&
-          (cairoVersion as string) !== (cairoAlphaVersion as string))
+        (semver.satisfies(requestedVersion, cairoAlphaSemver) &&
+          semver.compare(cairoVersion, cairoAlphaVersion) !== 0)

13-17: Nit: alias name can mislead (contracts vs compiler version)

contractsVersion as cairoVersion and the same for alpha can be misread as compiler version. Consider aliasing to cairoContractsVersion/cairoAlphaContractsVersion for clarity in future touch-ups.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 374d325 and 5cb478b.

⛔ 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 (15)
  • packages/core/cairo_alpha/CHANGELOG.md (1 hunks)
  • packages/core/cairo_alpha/package.json (1 hunks)
  • packages/core/cairo_alpha/src/account.test.ts.md (26 hunks)
  • packages/core/cairo_alpha/src/common-components.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 (15 hunks)
  • packages/core/cairo_alpha/src/erc20.test.ts.md (16 hunks)
  • packages/core/cairo_alpha/src/erc721.test.ts.md (19 hunks)
  • packages/core/cairo_alpha/src/governor.test.ts.md (10 hunks)
  • packages/core/cairo_alpha/src/multisig.test.ts.md (5 hunks)
  • packages/core/cairo_alpha/src/utils/version.ts (1 hunks)
  • packages/core/cairo_alpha/src/vesting.test.ts.md (6 hunks)
  • packages/core/cairo_alpha/test_project/Scarb.toml (1 hunks)
  • packages/ui/src/main.ts (1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
packages/ui/src/main.ts (2)
packages/core/cairo/src/utils/version.ts (1)
  • cairoVersion (11-11)
packages/core/cairo_alpha/src/utils/version.ts (1)
  • cairoVersion (11-11)
packages/core/cairo_alpha/src/utils/version.ts (1)
packages/core/cairo/src/utils/version.ts (6)
  • contractsVersion (4-4)
  • contractsVersionTag (5-5)
  • edition (10-10)
  • cairoVersion (11-11)
  • scarbVersion (12-12)
  • compatibleContractsSemver (19-19)
⏰ 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). (5)
  • GitHub Check: Socket Security: Pull Request Alerts
  • GitHub Check: build (solidity, default)
  • GitHub Check: build (stellar, compile)
  • GitHub Check: validate-cairo-alpha
  • GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (13)
packages/core/cairo_alpha/src/common-components.ts (1)

80-80: Confirm deduplication and single clock import

  • The addUseClause implementation in packages/core/cairo_alpha/src/contract.ts keys imports by unique name (alias or component name), so duplicate calls are ignored.
  • A search shows only one invocation of
    c.addUseClause('openzeppelin::utils::contract_clock', 'ERC6372TimestampClock')
    in packages/core/cairo_alpha/src/common-components.ts.
  • Please double-check that the openzeppelin::utils::contract_clock module is present in the pinned OpenZeppelin Cairo v3.0.0-alpha.0 release.
packages/core/cairo_alpha/src/multisig.test.ts.md (1)

12-12: LGTM: Snapshot headers updated to Cairo 3.0.0-alpha.0

Pure textual updates align with the version bump. No concerns.

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

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

12-12: LGTM: Vesting snapshots reflect Cairo 3.0.0-alpha.0

Text-only header updates are consistent with the migration.

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

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

12-12: LGTM: Contract snapshot headers bumped to Cairo 3.0.0-alpha.0

All updated headers look consistent with the broader PR.

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


12-12: All compatibility headers updated to 3.0.0-alpha.0
Verified that no “Compatible with OpenZeppelin Contracts for Cairo 2” headers remain, and confirmed 114 occurrences of “Compatible with OpenZeppelin Contracts for Cairo 3.0.0-alpha.0” in packages/core/cairo_alpha/src. Everything is up to date.

packages/core/cairo_alpha/package.json (1)

4-4: Version bump to 3.0.0-alpha.0 – cross-file consistency verified

All instances of 3.0.0-alpha.0 (143 matches) are up to date in packages/core/cairo_alpha, and there are no lingering references to:

  • “Compatible with OpenZeppelin Contracts for Cairo 2.0.0”
  • contractsVersion: "2.0.0"
  • Cairo/Scarb pin 2.11.4

LGTM.

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

12-12: All snapshot headers updated to 3.0.0-alpha.0

No occurrences of the legacy Compatible with OpenZeppelin Contracts for Cairo 2.0.0 header remain in packages/core/cairo_alpha/src.

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

12-12: Governor snapshots correctly updated to 3.0.0-alpha.0.
No structural changes detected in the snippets; only header text updated.

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

12-12: ERC20 snapshot headers updated to 3.0.0-alpha.0 — no leftover references

The search confirms there are no lingering “Compatible with OpenZeppelin Contracts for Cairo 2.0.0” headers in packages/core/cairo_alpha/src/erc20.test.ts.md. All snapshots consistently reflect 3.0.0-alpha.0.

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

12-12: All cairo_alpha snapshot headers updated to Cairo 3.0.0-alpha.0
Verified with rg—no remaining “Compatible with OpenZeppelin Contracts for Cairo 2.” banners under packages/core/cairo_alpha/src. LGTM.

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

12-12: Docs-only snapshot banner updates — all good

Version headers updated to 3.0.0-alpha.0 consistently. No runtime or API implications.

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

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

12-12: Consistent snapshot banner bump across ERC1155 variants — looks correct

All snapshot headers reflect Cairo 3.0.0-alpha.0 and align with the version constants. No logic affected.

Also applies to: 84-84, 176-176, 285-285, 362-362, 487-487, 613-613, 740-740, 887-887, 979-979, 1089-1089, 1215-1215, 1329-1329, 1459-1459, 1670-1670

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

4-4: Version constants correctly updated and pinned for alpha

  • contractsVersion → 3.0.0-alpha.0
  • cairo/scarb → 2.12.0
  • compatibleContractsSemver pinned to the exact alpha (correct per the comment)
    This matches the broader repo changes and UI logic.

Also applies to: 11-12, 19-19

@immrsd immrsd requested a review from ericglau August 12, 2025 14:56
@ericglau
Copy link
Member

Hi @immrsd, I think we should apply the following changes suggested in #623 (review)

  • packages/core/cairo_alpha/src/common-components.ts (1)
    72-81: Propagate section to VotesImpl for consistent code sectioning

    • Also should do this for cairo stable
  • packages/ui/src/main.ts (2)
    73-75: Prefer semver comparison over string casts for clarity and type-safety

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: 0

🧹 Nitpick comments (2)
packages/core/cairo/src/common-components.ts (1)

77-79: Nit: Prefer single quotes over a template literal for a static string

The value is a static string; using a template literal is unnecessary and slightly inconsistent with nearby literals.

-    value: `VotesComponent::VotesImpl<ContractState>`,
+    value: 'VotesComponent::VotesImpl<ContractState>',
packages/ui/src/main.ts (1)

73-75: Semver comparison change is correct; consider semver.neq for readability

Using semver to compare versions is the right move. For readability, semver.neq expresses the intent directly.

-        (semver.satisfies(requestedVersion, cairoAlphaSemver) &&
-          semver.compare(cairoVersion, cairoAlphaVersion) !== 0)
+        (semver.satisfies(requestedVersion, cairoAlphaSemver) &&
+          semver.neq(cairoVersion, cairoAlphaVersion))
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5cb478b and 3bb66fb.

📒 Files selected for processing (3)
  • packages/core/cairo/src/common-components.ts (1 hunks)
  • packages/core/cairo_alpha/src/common-components.ts (1 hunks)
  • packages/ui/src/main.ts (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/core/cairo_alpha/src/common-components.ts
🧰 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.717Z
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.
📚 Learning: 2025-08-12T14:48:12.717Z
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.717Z
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/ui/src/main.ts
🧬 Code Graph Analysis (1)
packages/ui/src/main.ts (2)
packages/core/cairo/src/utils/version.ts (1)
  • cairoVersion (11-11)
packages/core/cairo_alpha/src/utils/version.ts (1)
  • cairoVersion (11-11)
⏰ 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: validate-cairo
  • GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (1)
packages/core/cairo/src/common-components.ts (1)

76-80: Forwarding section to VotesImpl — LGTM

Passing the optional section through to VotesImpl aligns with how SRC5Impl is handled and keeps code sectioning consistent. No functional concerns.

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

📥 Commits

Reviewing files that changed from the base of the PR and between 3bb66fb and 87f59d7.

📒 Files selected for processing (1)
  • .changeset/config.json (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.717Z
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.
📚 Learning: 2025-08-12T14:48:12.717Z
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.717Z
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:

  • .changeset/config.json
⏰ 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: validate-cairo
  • GitHub Check: semgrep-cloud-platform/scan

@ericnordelo
Copy link
Member

@immrsd let's get this PR merged as soon as possible since we have already released 3.0.0-alpha.1 and this PR enables 3.0.0-alpha.0. I will prioritize work on enabling 3.0.0-alpha.1 right after this get's merged. cc @ericglau

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: 0

🧹 Nitpick comments (4)
packages/core/cairo/src/erc20.test.ts.md (1)

908-911: VotesImpl moved under a section — behavior unchanged, snapshot alignment looks good

Repositioning #[abi(embed_v0)] impl VotesImpl = VotesComponent::VotesImpl<ContractState>; is consistent with the intent to propagate section metadata. Functionality is unchanged; snapshots should now reflect the new sectioning.

Tiny nit: the inline comment reads “SNIP12 Metadata” right before the Votes impl. That header label may mislead readers since the line that follows is about Votes. Consider labeling that local section as “Votes” or relocating the comment to the actual SNIP12 block to avoid duplicating “SNIP12 Metadata” headings.

-        // SNIP12 Metadata
+        // Votes
         #[abi(embed_v0)]
         impl VotesImpl = VotesComponent::VotesImpl<ContractState>;

Also applies to: 1026-1028, 1133-1135, 1230-1232, 1379-1381

packages/core/cairo/src/erc721.test.ts.md (1)

1132-1135: Consistent section propagation for VotesImpl — snapshots updated correctly

Same pattern here: VotesImpl embedding is grouped under a nearby section header. This aligns snapshots with the generator’s sectioning without affecting semantics.

To reduce confusion, prefer a “Votes” header adjacent to VotesImpl, and reserve “SNIP12 Metadata” for the actual SNIP12 metadata block.

-        // SNIP12 Metadata
+        // Votes
         #[abi(embed_v0)]
         impl VotesImpl = VotesComponent::VotesImpl<ContractState>;

Also applies to: 1400-1402, 1570-1572, 1694-1696, 1810-1812, 1932-1934

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

1401-1404: VotesImpl embedding grouped with SNIP12 marker — ok, with minor labeling nit

Embedding VotesImpl is correct and matches the generator changes that propagate the section. See earlier note about preferring a “Votes” header here to avoid duplicate “SNIP12 Metadata” labels.

-        // SNIP12 Metadata
+        // Votes
         #[abi(embed_v0)]
         impl VotesImpl = VotesComponent::VotesImpl<ContractState>;

Also applies to: 1572-1574, 1697-1699, 1814-1816, 1937-1939

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

910-913: VotesImpl repositioned under the nearby section — approved; tiny labeling nit

The embedding location and attribute are correct. Same minor nit on the local section comment label as elsewhere.

-        // SNIP12 Metadata
+        // Votes
         #[abi(embed_v0)]
         impl VotesImpl = VotesComponent::VotesImpl<ContractState>;

Also applies to: 1029-1031, 1136-1138, 1234-1236, 1384-1386, 1557-1559

📜 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 fd091f9 and 634f666.

⛔ Files ignored due to path filters (4)
  • packages/core/cairo/src/erc20.test.ts.snap is excluded by !**/*.snap
  • packages/core/cairo/src/erc721.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
📒 Files selected for processing (6)
  • .changeset/fluffy-clubs-write.md (1 hunks)
  • packages/core/cairo/src/erc20.test.ts.md (6 hunks)
  • packages/core/cairo/src/erc721.test.ts.md (5 hunks)
  • packages/core/cairo_alpha/src/erc20.test.ts.md (26 hunks)
  • packages/core/cairo_alpha/src/erc721.test.ts.md (28 hunks)
  • packages/ui/src/main.ts (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • .changeset/fluffy-clubs-write.md
  • packages/ui/src/main.ts
🧰 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.
📚 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/erc721.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). (7)
  • 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: validate-cairo
  • GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (4)
packages/core/cairo_alpha/src/erc721.test.ts.md (2)

12-12: Update compatibility header to 3.0.0-alpha.0 — approved

All snapshot headers now reflect “Compatible with OpenZeppelin Contracts for Cairo 3.0.0-alpha.0.” This is consistent with the alpha bump goal.

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, 1349-1349, 1536-1536, 1661-1661, 1786-1786, 1881-1881


1361-1361: ERC6372TimestampClock import added — consistent with Votes/clock integration

Bringing ERC6372TimestampClock into scope matches the 3.0.0-alpha.0 expectations for ERC-6372-based voting clocks.

If the clock is auto-wired by Votes internally, the use may be required even without an explicit reference. Otherwise, consider removing unused imports to keep generated code strict. Please confirm whether these imports are required by the OZ Cairo 3.0.0-alpha.0 modules in all shown variants.

Also applies to: 1546-1546, 1671-1671, 1793-1793, 1895-1895

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

12-12: Alpha compatibility headers updated to 3.0.0-alpha.0 — approved

Headers consistently reflect the alpha version across ERC20 variants.

Also applies to: 52-52, 123-123, 203-203, 311-311, 438-438, 551-551, 624-624, 695-695, 776-776, 876-876, 995-995, 1112-1112, 1199-1199, 1345-1345, 1511-1511


885-885: ERC6372TimestampClock import added — aligns with Votes clock expectations

Including the timestamp clock import across Votes-enabled variants is consistent with 3.0.0-alpha.0. Looks good.

Double-check whether every imported clock is required to compile in all variants; if any are redundant, consider pruning unused imports in the generator to keep outputs tidy.

Also applies to: 1004-1004, 1119-1119, 1207-1207, 1355-1355, 1526-1526

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

🧹 Nitpick comments (1)
packages/core/cairo_alpha/src/erc20.test.ts.md (1)

885-885: ERC6372TimestampClock import added for Votes — looks correct; please confirm necessity

Assuming OZ Cairo v3 Votes relies on a “use clause” to select the timestamp-based ERC-6372 clock, these imports are appropriate. If the compiler warns on unused imports in some permutations, consider gating generation to only the Votes-enabled variants (which appears to be the case here).

Can you confirm that v3 requires only this import (no additional trait impls) and that no unused-import warnings surface across all snapshots?

Also applies to: 1001-1001, 1114-1114, 1201-1201, 1347-1347, 1516-1516

📜 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 634f666 and 063d8ae.

⛔ Files ignored due to path filters (2)
  • 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
📒 Files selected for processing (4)
  • packages/core/cairo/src/common-components.ts (1 hunks)
  • packages/core/cairo_alpha/src/common-components.ts (1 hunks)
  • packages/core/cairo_alpha/src/erc20.test.ts.md (20 hunks)
  • packages/core/cairo_alpha/src/erc721.test.ts.md (23 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
  • packages/core/cairo_alpha/src/erc721.test.ts.md
  • packages/core/cairo/src/common-components.ts
  • packages/core/cairo_alpha/src/common-components.ts
🧰 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.
📚 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/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). (7)
  • GitHub Check: semgrep-cloud-platform/scan
  • GitHub Check: validate-cairo
  • GitHub Check: validate-cairo-alpha
  • GitHub Check: Redirect rules - openzeppelin-contracts-wizard
  • GitHub Check: build (solidity, default)
  • GitHub Check: Header rules - openzeppelin-contracts-wizard
  • GitHub Check: Pages changed - openzeppelin-contracts-wizard
🔇 Additional comments (3)
packages/core/cairo_alpha/src/erc20.test.ts.md (3)

12-12: Cairo compatibility header bumped to 3.0.0-alpha.0 — LGTM

All ERC20-related snapshots consistently reflect the new Cairo/OpenZeppelin Contracts version in the header.

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


897-904: Ensure VotesImpl receives the intended “section” when generated

Per PR discussion, the section prop should propagate to VotesImpl for consistent code sectioning. Snapshots don’t show section identifiers, so just double-check the generator is passing the section to VotesImpl in both cairo_alpha and cairo packages.

Also applies to: 1016-1019, 1123-1129, 1218-1223, 1367-1370, 1538-1541


963-971: SNIP12 metadata embedding — LGTM

  • Default votes variants return version 'v1', and the “version” variants return 'MY_DAPP_VERSION'. This aligns with expected test permutations.
  • Consistent implementation across upgradeable and non-upgradeable/full variants.

Also applies to: 1079-1087, 1176-1184, 1320-1328, 1472-1480, 1658-1666

Comment on lines 1112 to 1117
use openzeppelin::governance::votes::VotesComponent;␊
use openzeppelin::token::erc20::{DefaultConfig, ERC20Component};␊
use openzeppelin::utils::contract_clock::ERC6372TimestampClock;␊
use openzeppelin::utils::cryptography::nonces::NoncesComponent;␊
use openzeppelin::utils::cryptography::snip12::SNIP12Metadata;␊
Copy link
Contributor

@coderabbitai coderabbitai bot Aug 18, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue

Add missing starknet::ContractAddress import in “erc20 votes, non-upgradeable”

ContractAddress is used in the ERC20 hooks signatures below but isn’t imported in this module, which would break compilation for the generated contract.

Apply this diff near the other use lines in this block:

         use openzeppelin::governance::votes::VotesComponent;
         use openzeppelin::token::erc20::{DefaultConfig, ERC20Component};
         use openzeppelin::utils::contract_clock::ERC6372TimestampClock;
         use openzeppelin::utils::cryptography::nonces::NoncesComponent;
         use openzeppelin::utils::cryptography::snip12::SNIP12Metadata;
+        use starknet::ContractAddress;
📝 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
use openzeppelin::governance::votes::VotesComponent;
use openzeppelin::token::erc20::{DefaultConfig, ERC20Component};
use openzeppelin::utils::contract_clock::ERC6372TimestampClock;
use openzeppelin::utils::cryptography::nonces::NoncesComponent;
use openzeppelin::utils::cryptography::snip12::SNIP12Metadata;
use openzeppelin::governance::votes::VotesComponent;
use openzeppelin::token::erc20::{DefaultConfig, ERC20Component};
use openzeppelin::utils::contract_clock::ERC6372TimestampClock;
use openzeppelin::utils::cryptography::nonces::NoncesComponent;
use openzeppelin::utils::cryptography::snip12::SNIP12Metadata;
use starknet::ContractAddress;
🤖 Prompt for AI Agents
In packages/core/cairo_alpha/src/erc20.test.ts.md around lines 1112 to 1117, the
module uses starknet::ContractAddress in ERC20 hook signatures but never imports
it; add a use starknet::ContractAddress; line alongside the other use statements
in that block to ensure the type is available and the generated contract
compiles.

Copy link
Member

Choose a reason for hiding this comment

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

Will evaluate in a separate PR if applicable.

Copy link
Contributor

Choose a reason for hiding this comment

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

@ericglau Understood! That's a reasonable approach to evaluate and address this potential import issue in a separate PR, especially since this PR is focused on the Cairo 3.0.0-alpha.0 upgrade.

If you'd like me to create a follow-up issue to track this potential compilation issue with the missing ContractAddress import in the "erc20 votes, non-upgradeable" snapshot, just let me know and I can open one for you.


🧠 Learnings used
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.

@ericglau ericglau merged commit a5e5808 into OpenZeppelin:master Aug 18, 2025
20 of 21 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Aug 18, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants