Skip to content

Enable missing components in macros#1635

Merged
bidzyyys merged 2 commits intomainfrom
feat/enable-missing-components-in-macros
Jan 20, 2026
Merged

Enable missing components in macros#1635
bidzyyys merged 2 commits intomainfrom
feat/enable-missing-components-in-macros

Conversation

@ericnordelo
Copy link
Copy Markdown
Member

@ericnordelo ericnordelo commented Jan 19, 2026

Summary by CodeRabbit

  • New Features

    • Added support for ERC20Wrapper and ERC721Wrapper components for enhanced token contract composition.
  • Bug Fixes & Validation

    • Added validation warnings for missing hook implementations in ERC1155Supply and ERC721Enumerable components to catch integration errors.
  • Tests

    • Extended test coverage for wrapper component variants and hook integration scenarios.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Jan 19, 2026

Walkthrough

The PR extends the macro system to support ERC20Wrapper and ERC721Wrapper components by adding enum variants, deserialization logic, and metadata. It also introduces validation checks for hook requirements in ERC1155Supply and ERC721Enumerable components, along with comprehensive test coverage and mock contract updates to use the new macro annotations.

Changes

Cohort / File(s) Summary
Macro Component Support
packages/macros/src/attribute/with_components/components.rs
Introduces two new enum variants (ERC20Wrapper, ERC721Wrapper) to AllowedComponents. Extends from_str deserialization to recognize "ERC20Wrapper" and "ERC721Wrapper" strings. Adds ComponentInfo metadata for both variants in get_info method, defining name, path, storage, event, initializer flag, immutable config, and internal impls.
Diagnostic Warnings
packages/macros/src/attribute/with_components/diagnostics.rs
Adds two public string constants: ERC1155_SUPPLY_HOOKS_MISSING (indicates missing ERC1155Supply::after_update hook call) and ERC721_ENUMERABLE_HOOKS_MISSING (indicates missing ERC721Enumerable::before_update hook call).
Hook Validation Logic
packages/macros/src/attribute/with_components/parser.rs
Extends add_per_component_warnings with component-specific checks: for ERC1155Supply, emits warning if neither erc1155_supply.after_update() nor ERC1155SupplyInternalImpl::after_update() is found; for ERC721Enumerable, emits warning if neither erc721_enumerable.before_update() nor ERC721EnumerableInternalImpl::before_update() is found.
Test Coverage
packages/macros/src/tests/test_with_components.rs
Adds comprehensive test cases for ERC20Wrapper, ERC721Wrapper, and ERC721Enumerable variants, including scenarios with and without initializers. Tests validate wrapper initialization, hook integration with ERC721Component, and proper forwarding of before_update behavior.
Mock Contract Updates
packages/test_common/src/mocks/erc20.cairo
Updates ERC20WrapperMock to use #[with_components(ERC20, ERC20Wrapper)] annotation. Removes manual ERC20Component::Storage and ERC20WrapperComponent::Storage declarations, replacing storage and event enums with empty structs.
Mock Contract Updates
packages/test_common/src/mocks/erc721.cairo
Updates ERC721WrapperMock to use #[with_components(ERC721, SRC5, ERC721Wrapper)] annotation. Removes explicit ERC721WrapperComponent::Storage and ERC721WrapperComponent::Event declarations, replacing with empty structs while maintaining ERC721WrapperImpl and ERC721WrapperReceiverImpl implementations.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • Add ERC721Wrapper #1625: Introduces ERC721Wrapper component and related mocks/tests/interfaces; the macro system changes in this PR directly enable and support the ERC721Wrapper introduced in that PR.
  • Add ERC20WrapperComponent #1617: Implements ERC20WrapperComponent, interface, and wrapper tests; this PR adds the macro-level support (AllowedComponents::ERC20Wrapper, metadata, and test/mock integration) that pairs with that component implementation.

Suggested reviewers

  • immrsd
  • bidzyyys

Poem

🐰 Wrappers bloom in macro fields so green,
ERC20 and ERC721 now seen,
Hooks validate with careful care,
Storage simplified, light as air!
The contracts dance in harmony true,
Macro magic makes them new! ✨

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Enable missing components in macros' accurately summarizes the main change: adding support for ERC20Wrapper, ERC721Wrapper, and ERC721Enumerable components in the macro system.
Docstring Coverage ✅ Passed Docstring coverage is 85.71% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feat/enable-missing-components-in-macros

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions
Copy link
Copy Markdown
Contributor

🧪 Cairo Contract Size Benchmark Diff

BYTECODE SIZE (felts) (limit: 81,920 felts)

No changes in felts.

SIERRA CONTRACT CLASS SIZE (bytes) (limit: 4,089,446 bytes)

No changes in bytes.

This comment was generated automatically from benchmark diffs.

Copy link
Copy Markdown
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

🤖 Fix all issues with AI agents
In `@packages/macros/src/attribute/with_components/components.rs`:
- Around line 267-284: The ComponentInfo metadata for
AllowedComponents::ERC20Wrapper and AllowedComponents::ERC721Wrapper is
incorrect: update the ComponentInfo 'event' field to reflect actual component
definitions—for AllowedComponents::ERC20Wrapper (ERC20WrapperComponent) set
event to "Event" to match the enum name defined in ERC20 wrapper, and for
AllowedComponents::ERC721Wrapper (ERC721WrapperComponent) either remove the
'event' field entirely from that ComponentInfo or add a matching #[event] enum
to the ERC721 component; adjust only the ComponentInfo entries (in the
ComponentInfo initializer for ERC20WrapperComponent and ERC721WrapperComponent)
so metadata and component symbols align.

Comment thread packages/macros/src/attribute/with_components/components.rs
@codecov
Copy link
Copy Markdown

codecov Bot commented Jan 19, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.74%. Comparing base (06319c7) to head (bf82d49).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1635   +/-   ##
=======================================
  Coverage   93.74%   93.74%           
=======================================
  Files          88       88           
  Lines        2190     2190           
=======================================
  Hits         2053     2053           
  Misses        137      137           

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 06319c7...bf82d49. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Copy Markdown
Collaborator

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

Good job, LGTM!

Comment on lines +311 to +318
AllowedComponents::ERC1155Supply => {
let hook_called = code.contains("erc1155_supply.after_update(")
|| code.contains("ERC1155SupplyInternalImpl::after_update(");
if !hook_called {
let warning = Diagnostic::warn(warnings::ERC1155_SUPPLY_HOOKS_MISSING);
warnings.push(warning);
}
}
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

👍

Copy link
Copy Markdown
Collaborator

@bidzyyys bidzyyys left a comment

Choose a reason for hiding this comment

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

LGTM!

@bidzyyys bidzyyys merged commit 8093a17 into main Jan 20, 2026
15 checks passed
@bidzyyys bidzyyys deleted the feat/enable-missing-components-in-macros branch January 20, 2026 09:18
This was referenced Jan 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants