Skip to content

Conversation

MozirDmitriy
Copy link
Contributor

This change deprecates the ERC1155InvalidApprover error in contracts/interfaces/draft-IERC6093.sol, clarifying it is non-standard for ERC-1155 which only defines operator-wide approvals via setApprovalForAll. The error is unused across the codebase and not part of ERC-6093’s canonical ERC-1155 set. The comment now directs users to ERC1155InvalidOperator and ERC1155MissingApprovalForAll to avoid confusion and align with the spec without introducing behavioral changes.

@MozirDmitriy MozirDmitriy requested a review from a team as a code owner October 15, 2025 15:40
Copy link

changeset-bot bot commented Oct 15, 2025

⚠️ No Changeset found

Latest commit: 84d2177

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Copy link

coderabbitai bot commented Oct 15, 2025

Walkthrough

The docblock for IERC1155Errors’s ERC1155InvalidApprover(error) in contracts/interfaces/draft-IERC6093.sol was updated to note it is non-standard for ERC-1155 and marked as deprecated. The error’s signature remains unchanged. No code or control-flow modifications were made, and no exported or public declarations were altered.

Suggested labels

ignore-changeset

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Title Check ✅ Passed The title succinctly captures the main change by indicating the deprecation of ERC1155InvalidApprover in the IERC1155Errors interface, matching the update in documentation without extraneous details.
Description Check ✅ Passed The description accurately describes the documentation update to deprecate the ERC1155InvalidApprover error, its context in the ERC-1155 spec, and confirms no behavioral changes, making it directly relevant to the changeset.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • 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

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

Copy link

@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 1cf1377 and 84d2177.

📒 Files selected for processing (1)
  • contracts/interfaces/draft-IERC6093.sol (1 hunks)
🧰 Additional context used
🪛 GitHub Actions: checks
contracts/interfaces/draft-IERC6093.sol

[error] 143-143: Hardhat compilation failed: DocstringParsingError: Documentation tag @deprecated not valid for errors.

🪛 GitHub Actions: formal verification
contracts/interfaces/draft-IERC6093.sol

[error] 143-143: Compiler run failed: Error (6546): Documentation tag @deprecated not valid for errors.

Comment on lines +144 to 146
* @dev Non-standard for ERC-1155. ERC-1155 only defines operator-wide approvals via {setApprovalForAll}.
* @deprecated Non-standard for ERC-1155; use {ERC1155InvalidOperator} or {ERC1155MissingApprovalForAll} instead.
* @param approver Address initiating an approval operation.
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Replace the unsupported @deprecated tag

Hardhat is rejecting this docblock (DocstringParsingError: Documentation tag @deprecated not valid for errors), so the contract no longer compiles. Please keep the deprecation note but phrase it without the NatSpec tag, e.g.:

-     * @deprecated Non-standard for ERC-1155; use {ERC1155InvalidOperator} or {ERC1155MissingApprovalForAll} instead.
+     * Deprecated: non-standard for ERC-1155; use {ERC1155InvalidOperator} or {ERC1155MissingApprovalForAll} instead.
📝 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
* @dev Non-standard for ERC-1155. ERC-1155 only defines operator-wide approvals via {setApprovalForAll}.
* @deprecated Non-standard for ERC-1155; use {ERC1155InvalidOperator} or {ERC1155MissingApprovalForAll} instead.
* @param approver Address initiating an approval operation.
* @dev Non-standard for ERC-1155. ERC-1155 only defines operator-wide approvals via {setApprovalForAll}.
* Deprecated: non-standard for ERC-1155; use {ERC1155InvalidOperator} or {ERC1155MissingApprovalForAll} instead.
* @param approver Address initiating an approval operation.
🤖 Prompt for AI Agents
contracts/interfaces/draft-IERC6093.sol around lines 144-146: the NatSpec uses
the @deprecated tag on an error docblock which Hardhat rejects; remove the
@deprecated tag and rephrase the deprecation note as plain text in the comment
(e.g., "Deprecated — Non-standard for ERC-1155; use ERC1155InvalidOperator or
ERC1155MissingApprovalForAll instead.") so it is not treated as a NatSpec tag,
keeping the param and description intact.

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.

1 participant