Skip to content

Add disclaimer to erc721 components#1638

Merged
immrsd merged 7 commits intomainfrom
feat/add-disclaimer-to-erc721-components
Jan 30, 2026
Merged

Add disclaimer to erc721 components#1638
immrsd merged 7 commits intomainfrom
feat/add-disclaimer-to-erc721-components

Conversation

@ericnordelo
Copy link
Copy Markdown
Member

@ericnordelo ericnordelo commented Jan 26, 2026

Summary by CodeRabbit

  • Documentation
    • Added caution notes clarifying that batch-minted tokens bypass per-token update logic and do not trigger associated hooks.
    • Added warning about incompatibilities when combining ERC721Consecutive with ERC721Enumerable due to custom balance tracking logic.

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

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Jan 26, 2026

Important

Review skipped

Auto incremental reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

  • 🔍 Trigger a full review

Walkthrough

Documentation additions to ERC721 extension files, inserting CAUTION notes regarding batch minting behavior and enumeration compatibility. No functional code changes; only descriptive comments added to warn developers about limitations and required hook invocations.

Changes

Cohort / File(s) Summary
Documentation Updates
packages/token/src/erc721/extensions/erc721_consecutive.cairo, packages/token/src/erc721/extensions/erc721_enumerable/erc721_enumerable.cairo
Added CAUTION warnings documenting batch minting limitations: tokens minted via mint_consecutive bypass per-token update logic, hooks must be manually invoked post-transfer/mint/burn, and onERC721Received is not triggered. Noted incompatibility between custom balanceOf implementations (e.g., ERC721Consecutive) and ERC721Enumerable.

Estimated Code Review Effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Poem

🐰 In tokenland where minting runs fast,
We've marked the paths with notes so vast,
"Beware!" we cry, "batch operations soar,
Hooks and updates, there's much more!"
Documentation blooms, developers now see,
The careful dance of ERC721 decree! 📜✨

🚥 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 pull request title 'Add disclaimer to erc721 components' accurately describes the main change—adding CAUTION documentation/disclaimers to ERC721-related components.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

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

✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feat/add-disclaimer-to-erc721-components

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

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Jan 26, 2026

🧪 Cairo Contract Size Benchmark Diff

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

Contract Old New Δ Note
ERC1155URIStorageMock 11325 +11325 ✅ NEW
ERC721URIStorageMock 12720 13068 🟢 +348

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

Contract Old New Δ Note
ERC1155URIStorageMock 256026 +256026 ✅ NEW
ERC721URIStorageMock 275404 283591 🟢 +8187

This comment was generated automatically from benchmark diffs.

let search_key = checkpoint.key - 1;
let found_value = mock_trace.lower_lookup(search_key);
assert_eq!(found_value, checkpoint.value);
// If search_key equals the previous checkpoint's key, lower_lookup returns that value.
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

The test was incorrectly assuming that checkpoint.key - 1 would always fall between checkpoints, but when the fuzzer generates consecutive keys (key_step = 1), search_key actually matches the previous checkpoint's key exactly, so lower_lookup correctly returns the previous checkpoint's value.

@codecov
Copy link
Copy Markdown

codecov bot commented Jan 26, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.10%. Comparing base (5ef63f2) to head (dc48248).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1638   +/-   ##
=======================================
  Coverage   94.10%   94.10%           
=======================================
  Files          96       96           
  Lines        2391     2391           
=======================================
  Hits         2250     2250           
  Misses        141      141           
Files with missing lines Coverage Δ
...ken/src/erc721/extensions/erc721_consecutive.cairo 97.67% <ø> (ø)
...tensions/erc721_enumerable/erc721_enumerable.cairo 98.00% <ø> (ø)

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 5ef63f2...dc48248. 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.

Looking good, left just one suggestion

@immrsd immrsd merged commit 911cc9a into main Jan 30, 2026
13 checks passed
@ericnordelo ericnordelo deleted the feat/add-disclaimer-to-erc721-components branch February 3, 2026 16:13
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