-
Notifications
You must be signed in to change notification settings - Fork 573
Update Solidity version and refactor ContractMetadata #692
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: sonnyquinn24 <[email protected]>
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughReplaces the prior ContractMetadata-focused content with a new ERC721 contract Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Owner
participant RebelByNature
participant ERC721 as ERC721 Core
note right of RebelByNature: state: nextTokenId
Owner->>RebelByNature: mintTribute(recipient)
activate RebelByNature
RebelByNature->>ERC721: _mint(recipient, nextTokenId)
RebelByNature->>RebelByNature: _setTokenURI(nextTokenId, uri)
RebelByNature-->>Owner: emit TributeMinted(nextTokenId, recipient, block.timestamp)
deactivate RebelByNature
note over RebelByNature,Owner: _canSetContractURI() returns true only for owner
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR transforms the ContractMetadata abstract contract into a complete NFT contract called "RebelByNature", updating the Solidity version and implementing ERC721 functionality with tribute minting capabilities.
- Updates Solidity version from ^0.8.0 to ^0.8.19
- Replaces abstract ContractMetadata implementation with concrete RebelByNature ERC721 contract
- Adds tribute minting functionality with event emission and owner-only access control
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
/// @author thirdweb | ||
import "@openzeppelin/contracts/token/ERC721/extensions/ERC721URIStorage.sol"; | ||
import "@openzeppelin/contracts/access/Ownable.sol"; | ||
import "./ContractMetadata.sol"; |
Copilot
AI
Oct 7, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The file is importing itself (ContractMetadata.sol), creating a circular dependency that will cause compilation to fail.
import "./ContractMetadata.sol"; |
Copilot uses AI. Check for mistakes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Update
import "./ContractMetadata.sol"; | ||
|
Copilot
AI
Oct 7, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The contract inherits from ContractMetadata which doesn't exist in this file anymore, and the circular import will prevent compilation.
import "./ContractMetadata.sol"; | |
// import "./ContractMetadata.sol"; // Removed circular import | |
// Minimal ContractMetadata contract definition | |
abstract contract ContractMetadata { | |
string public contractURI; | |
function setContractURI(string memory _uri) public virtual { | |
contractURI = _uri; | |
} | |
function _canSetContractURI() internal view virtual returns (bool); | |
} |
Copilot uses AI. Check for mistakes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
// SPDX-License-Identifier: Apache-2.0
pragma solidity ^0.8.0;
interface IContractMetadata {
function contractURI() external view returns (string memory);
function setContractURI(string memory _uri) external;
event ContractURIUpdated(string prevURI, string newURI);
}
import "./ContractMetadata.sol"; | ||
|
||
import "./interface/IContractMetadata.sol"; | ||
contract RebelByNature is ERC721URIStorage, Ownable, ContractMetadata { | ||
uint256 public nextTokenId; | ||
|
||
/** | ||
* @title Contract Metadata | ||
* @notice Thirdweb's `ContractMetadata` is a contract extension for any base contracts. It lets you set a metadata URI | ||
* for you contract. | ||
* Additionally, `ContractMetadata` is necessary for NFT contracts that want royalties to get distributed on OpenSea. | ||
*/ | ||
event TributeMinted(uint256 tokenId, address recipient, uint256 timestamp); | ||
|
||
abstract contract ContractMetadata is IContractMetadata { | ||
/// @dev The sender is not authorized to perform the action | ||
error ContractMetadataUnauthorized(); | ||
constructor() ERC721("Rebel By Nature", "SEQREB") {} | ||
|
||
/// @notice Returns the contract metadata URI. | ||
string public override contractURI; | ||
|
||
/** | ||
* @notice Lets a contract admin set the URI for contract-level metadata. | ||
* @dev Caller should be authorized to setup contractURI, e.g. contract admin. | ||
* See {_canSetContractURI}. | ||
* Emits {ContractURIUpdated Event}. | ||
* | ||
* @param _uri keccak256 hash of the role. e.g. keccak256("TRANSFER_ROLE") | ||
*/ | ||
function setContractURI(string memory _uri) external override { | ||
if (!_canSetContractURI()) { | ||
revert ContractMetadataUnauthorized(); | ||
} | ||
|
||
_setupContractURI(_uri); | ||
function mintTribute(address recipient) external onlyOwner { | ||
uint256 tokenId = nextTokenId++; | ||
_mint(recipient, tokenId); | ||
_setTokenURI(tokenId, contractURI); | ||
emit TributeMinted(tokenId, recipient, block.timestamp); | ||
} | ||
|
||
/// @dev Lets a contract admin set the URI for contract-level metadata. | ||
function _setupContractURI(string memory _uri) internal { | ||
string memory prevURI = contractURI; | ||
contractURI = _uri; | ||
|
||
emit ContractURIUpdated(prevURI, _uri); | ||
function _canSetContractURI() internal view override returns (bool) { | ||
return msg.sender == owner(); | ||
} |
Copilot
AI
Oct 7, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The variable contractURI
is not defined in this contract. Since ContractMetadata was removed, this property no longer exists.
Copilot uses AI. Check for mistakes.
Co-authored-by: Copilot <[email protected]> Signed-off-by: sonnyquinn24 <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (1)
contracts/extension/ContractMetadata.sol (1)
11-11
: Index event fields for better queryingMake
tokenId
andrecipient
indexed.- event TributeMinted(uint256 tokenId, address recipient, uint256 timestamp); + event TributeMinted(uint256 indexed tokenId, address indexed recipient, uint256 timestamp);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
contracts/extension/ContractMetadata.sol
(1 hunks)
🔇 Additional comments (2)
contracts/extension/ContractMetadata.sol (2)
2-2
: Toolchain and OpenZeppelin version are compatible
foundry.toml targets solc 0.8.23 (covers ^0.8.19) and package.json uses OZ v4.9.6 (no-arg Ownable ctor).
20-25
: Likely required: override supportsInterface due to multiple inheritanceERC721 and ContractMetadata typically both override supportsInterface. Add an override to resolve the diamond.
} function _canSetContractURI() internal view override returns (bool) { return msg.sender == owner(); } + + function supportsInterface(bytes4 interfaceId) + public + view + virtual + override(ERC721, ContractMetadata) + returns (bool) + { + return super.supportsInterface(interfaceId); + } }Likely an incorrect or invalid review comment.
import "@openzeppelin/contracts/token/ERC721/extensions/ERC721URIStorage.sol"; | ||
import "@openzeppelin/contracts/access/Ownable.sol"; | ||
import "./ContractMetadata.sol"; | ||
|
||
import "./interface/IContractMetadata.sol"; | ||
contract RebelByNature is ERC721URIStorage, Ownable, ContractMetadata { | ||
uint256 public nextTokenId; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
Blocker: Self-import and replaced extension file will break the build and downstream imports
- This file is contracts/extension/ContractMetadata.sol but now defines RebelByNature and also
import "./ContractMetadata.sol";
(self-import). This will not compile and also removes the original extension that many modules likely import. - If you intend to use the thirdweb ContractMetadata extension, place RebelByNature in a new file (e.g., contracts/tokens/RebelByNature.sol) and import the actual extension from its correct path. Restore this file to the original ContractMetadata extension.
[action]
- Move this contract to a new file and fix the import to the real ContractMetadata location.
- Revert this file back to the extension to avoid breaking changes.
Run to locate the correct ContractMetadata definition and affected importers:
🏁 Script executed:
#!/bin/bash
# Find definitions of ContractMetadata / IContractMetadata
rg -nP --hidden -C2 '(^\s*(abstract\s+)?contract\s+ContractMetadata\b)|(^\s*interface\s+IContractMetadata\b)'
# Find all imports of ContractMetadata.sol to see impact
rg -nP --hidden -C0 'import\s+.*ContractMetadata\.sol'
# Check if this file path is referenced by others (would break consumers)
rg -nP --hidden -C2 'contracts/extension/ContractMetadata\.sol'
Length of output: 6760
Restore contracts/extension/ContractMetadata.sol
and relocate RebelByNature
contracts/extension/ContractMetadata.sol
has been overwritten byRebelByNature
(with a self-import), breaking all imports of the extension.- Move
RebelByNature
into its own file (e.g.,contracts/tokens/RebelByNature.sol
), import the real extension fromcontracts/extension/upgradeable/ContractMetadata.sol
, and revertcontracts/extension/ContractMetadata.sol
back to the original extension implementation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
function mintTribute(address recipient) external onlyOwner { | ||
uint256 tokenId = nextTokenId++; | ||
_mint(recipient, tokenId); | ||
_setTokenURI(tokenId, contractURI); | ||
emit TributeMinted(tokenId, recipient, block.timestamp); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix compile error and mint safety: call contractURI(), use _safeMint
_setTokenURI(tokenId, contractURI);
likely fails:contractURI
is a function in the extension; call it ascontractURI()
.- Prefer
_safeMint
to avoid sending tokens to non-receiver contracts.
Apply:
- function mintTribute(address recipient) external onlyOwner {
- uint256 tokenId = nextTokenId++;
- _mint(recipient, tokenId);
- _setTokenURI(tokenId, contractURI);
- emit TributeMinted(tokenId, recipient, block.timestamp);
- }
+ function mintTribute(address recipient) external onlyOwner {
+ uint256 tokenId = nextTokenId++;
+ _safeMint(recipient, tokenId);
+ _setTokenURI(tokenId, contractURI());
+ emit TributeMinted(tokenId, recipient, block.timestamp);
+ }
Optional: accept a per‑token URI to avoid copying the contract‑level URI into token metadata.
function mintTribute(address recipient, string calldata uri) external onlyOwner {
uint256 tokenId = nextTokenId++;
_safeMint(recipient, tokenId);
_setTokenURI(tokenId, uri);
emit TributeMinted(tokenId, recipient, block.timestamp);
}
🤖 Prompt for AI Agents
In contracts/extension/ContractMetadata.sol around lines 15 to 19, the
mintTribute implementation currently calls contractURI as if it were a variable
and uses _mint which can send tokens to non-receiver contracts; change the call
to contractURI() if you intend to use the contract-level URI, and replace _mint
with _safeMint to ensure ERC721Receiver checks. Optionally, change the function
signature to accept a per-token URI (e.g., mintTribute(address recipient, string
calldata uri)), call _safeMint(recipient, tokenId), then _setTokenURI(tokenId,
uri) (or _setTokenURI(tokenId, contractURI()) if you want the contract-level
URI), and emit the event as before.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
There was a problem hiding this 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
♻️ Duplicate comments (2)
contracts/extension/ContractMetadata.sol (2)
7-23
: Do not overwrite the extension + restoreContractMetadata
availability.This file is the shared extension
contracts/extension/ContractMetadata.sol
. Replacing it withRebelByNature
removes the base extension and breaks every import that relies on it. MoveRebelByNature
into its own file (e.g.contracts/tokens/RebelByNature.sol
) and revert this file to the original extension so downstream contracts continue compiling.
17-18
: CallcontractURI()
when setting token URI.
contractURI
is a function in the extension; passing it without parentheses yields a function pointer and fails compilation. CallcontractURI()
(or supply a token-specific URI). While fixing this, prefer_safeMint
over_mint
to guard against non-receiver contracts.- _mint(recipient, tokenId); - _setTokenURI(tokenId, contractURI); + _safeMint(recipient, tokenId); + _setTokenURI(tokenId, contractURI());
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
contracts/extension/ContractMetadata.sol
(1 hunks)
import "@openzeppelin/contracts/access/Ownable.sol"; | ||
|
||
import "./interface/IContractMetadata.sol"; | ||
contract RebelByNature is ERC721URIStorage, Ownable, ContractMetadata { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ContractMetadata
is undefined—missing import/definition.
The contract inherits ContractMetadata
, but that type is no longer declared in this file nor imported from elsewhere, so the compiler cannot resolve it. Reintroduce the original extension declaration here or import the correct contract before using it.
🤖 Prompt for AI Agents
In contracts/extension/ContractMetadata.sol around line 7, the contract inherits
ContractMetadata which is not defined or imported; add the missing declaration
or import the correct ContractMetadata contract before using it. Restore the
original ContractMetadata extension by either reintroducing its
contract/interface implementation in this file or adding an import statement
that points to the file that declares ContractMetadata, and then ensure the
inheritance list compiles (update SPDX/pragma or paths as needed).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
// SPDX-License-Identifier: Apache-2.0
pragma solidity ^0.8.19;
import "@openzeppelin/contracts/token/ERC721/extensions/ERC721URIStorage.sol";
import "@openzeppelin/contracts/access/Ownable.sol";
import "./ContractMetadata.sol";
contract RebelByNature is ERC721URIStorage, Ownable, ContractMetadata {
uint256 public nextTokenId;
event TributeMinted(uint256 tokenId, address recipient, uint256 timestamp);
constructor() ERC721("Rebel By Nature", "RBN") {}
function mintTribute(address recipient) external onlyOwner {
uint256 tokenId = nextTokenId++;
_mint(recipient, tokenId);
_setTokenURI(tokenId, contractURI);
emit TributeMinted(tokenId, recipient, block.timestamp);
}
function _canSetContractURI() internal view override returns (bool) {
return msg.sender == owner();
}
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for ContractMetadata contract or interface definitions
echo "=== Searching for ContractMetadata definitions ==="
rg -nP --type=sol -C3 '^\s*(contract|interface|abstract contract)\s+ContractMetadata\b'
echo -e "\n=== Searching for IContractMetadata interface ==="
rg -nP --type=sol -C3 '^\s*interface\s+IContractMetadata\b'
echo -e "\n=== Check the interface directory ==="
fd -e sol . contracts/extension/interface/
echo -e "\n=== Git history of ContractMetadata.sol (before PR) ==="
git show main:contracts/extension/ContractMetadata.sol | head -50
Length of output: 4558
🏁 Script executed:
#!/bin/bash
# Get the IContractMetadata interface content
echo "=== IContractMetadata interface content ==="
cat contracts/extension/interface/IContractMetadata.sol
echo -e "\n=== Check if RebelByNature.sol already exists ==="
fd -e sol "RebelByNature" contracts/
Length of output: 930
@sonnyquinn24, the import import "./ContractMetadata.sol";
creates a self-import (the file importing itself), which will cause a circular dependency compilation error.
The Problem
You've completely replaced the ContractMetadata.sol
file, which originally contained an abstract contract extension, with your RebelByNature
contract. The file cannot import itself.
Recommended Solution: Separate Files
Restore contracts/extension/ContractMetadata.sol
to its original content and create a new file for your contract:
contracts/extension/ContractMetadata.sol (restore original):
// SPDX-License-Identifier: Apache-2.0
pragma solidity ^0.8.0;
import "./interface/IContractMetadata.sol";
abstract contract ContractMetadata is IContractMetadata {
error ContractMetadataUnauthorized();
string public override contractURI;
function setContractURI(string memory _uri) external override {
if (!_canSetContractURI()) {
revert ContractMetadataUnauthorized();
}
_setupContractURI(_uri);
}
function _setupContractURI(string memory _uri) internal {
string memory prevURI = contractURI;
contractURI = _uri;
emit ContractURIUpdated(prevURI, _uri);
}
function _canSetContractURI() internal view virtual returns (bool);
}
contracts/RebelByNature.sol (new file):
// SPDX-License-Identifier: Apache-2.0
pragma solidity ^0.8.19;
import "@openzeppelin/contracts/token/ERC721/extensions/ERC721URIStorage.sol";
import "@openzeppelin/contracts/access/Ownable.sol";
import "./extension/ContractMetadata.sol";
contract RebelByNature is ERC721URIStorage, Ownable, ContractMetadata {
uint256 public nextTokenId;
event TributeMinted(uint256 tokenId, address recipient, uint256 timestamp);
constructor() ERC721("Rebel By Nature", "RBN") Ownable(msg.sender) {}
function mintTribute(address recipient) external onlyOwner {
uint256 tokenId = nextTokenId++;
_mint(recipient, tokenId);
_setTokenURI(tokenId, contractURI);
emit TributeMinted(tokenId, recipient, block.timestamp);
}
function _canSetContractURI() internal view override returns (bool) {
return msg.sender == owner();
}
}
Note: I also added Ownable(msg.sender)
to the constructor, which is required for OpenZeppelin's Ownable in v5.x if you're using that version.
Alternative: Combined File (Not Recommended)
If you prefer a single file, include both contract definitions:
// SPDX-License-Identifier: Apache-2.0
pragma solidity ^0.8.19;
import "@openzeppelin/contracts/token/ERC721/extensions/ERC721URIStorage.sol";
import "@openzeppelin/contracts/access/Ownable.sol";
import "./interface/IContractMetadata.sol";
abstract contract ContractMetadata is IContractMetadata {
error ContractMetadataUnauthorized();
string public override contractURI;
function setContractURI(string memory _uri) external override {
if (!_canSetContractURI()) {
revert ContractMetadataUnauthorized();
}
_setupContractURI(_uri);
}
function _setupContractURI(string memory _uri) internal {
string memory prevURI = contractURI;
contractURI = _uri;
emit ContractURIUpdated(prevURI, _uri);
}
function _canSetContractURI() internal view virtual returns (bool);
}
contract RebelByNature is ERC721URIStorage, Ownable, ContractMetadata {
uint256 public nextTokenId;
event TributeMinted(uint256 tokenId, address recipient, uint256 timestamp);
constructor() ERC721("Rebel By Nature", "RBN") Ownable(msg.sender) {}
function mintTribute(address recipient) external onlyOwner {
uint256 tokenId = nextTokenId++;
_mint(recipient, tokenId);
_setTokenURI(tokenId, contractURI);
emit TributeMinted(tokenId, recipient, block.timestamp);
}
function _canSetContractURI() internal view override returns (bool) {
return msg.sender == owner();
}
}
The separate files approach is strongly recommended to maintain the repository's extension architecture.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Todo
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Todo
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Todo
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Summary by CodeRabbit