feat: xp nfts contract skeleton #164
Conversation
andresaiello
changed the title
Co-authored-by: Lucas <ljanon@gmail.com>
Co-authored-by: Lucas <ljanon@gmail.com>
Co-authored-by: Lucas <ljanon@gmail.com>
andresaiello
changed the title
andresaiello
changed the title
Co-authored-by: Lucas <ljanon@gmail.com>
WalkthroughWalkthroughThe changes introduce the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ZetaXP
participant Signer
User->>ZetaXP: mintNFT(data, signature)
ZetaXP->>Signer: verifySignature(data, signature)
Signer-->>ZetaXP: verificationSuccess
ZetaXP-->>User: emit NewNFTMinted event
User->>ZetaXP: updateNFT(tokenId, updateData, signature)
ZetaXP->>Signer: verifySignature(updateData, signature)
Signer-->>ZetaXP: verificationSuccess
ZetaXP-->>User: emit NFTUpdated event
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Outside diff range, codebase verification and nitpick comments (2)
packages/zevm-app-contracts/contracts/xp-nft/test/xpNFTV2.sol (1)
1-2: Use a more recent version of Solidity.Consider using a more recent version of Solidity if possible to leverage the latest features and security improvements. The current version 0.8.7 is stable, but newer versions might offer additional benefits.
- pragma solidity 0.8.7; + pragma solidity ^0.8.7;packages/zevm-app-contracts/contracts/xp-nft/xpNFT.sol (1)
1-2: Use a more recent version of Solidity.Consider using a more recent version of Solidity if possible to leverage the latest features and security improvements. The current version 0.8.7 is stable, but newer versions might offer additional benefits.
- pragma solidity 0.8.7; + pragma solidity ^0.8.7;
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
Files selected for processing (6)
- packages/zevm-app-contracts/contracts/xp-nft/test/xpNFTV2.sol (1 hunks)
- packages/zevm-app-contracts/contracts/xp-nft/xpNFT.sol (1 hunks)
- packages/zevm-app-contracts/hardhat.config.ts (1 hunks)
- packages/zevm-app-contracts/package.json (1 hunks)
- packages/zevm-app-contracts/test/xp-nft/test.helpers.ts (1 hunks)
- packages/zevm-app-contracts/test/xp-nft/xp-nft.ts (1 hunks)
Files skipped from review due to trivial changes (2)
- packages/zevm-app-contracts/hardhat.config.ts
- packages/zevm-app-contracts/package.json
Additional comments not posted (39)
packages/zevm-app-contracts/contracts/xp-nft/test/xpNFTV2.sol (1)
6-10: LGTM!The
ZetaXPV2contract correctly extendsZetaXPand overrides theversionfunction to return "2.0.0".packages/zevm-app-contracts/test/xp-nft/test.helpers.ts (4)
1-2: LGTM!The imports are appropriate and necessary for the functionality provided in this file.
4-8: LGTM!The
Signatureinterface is well-defined and necessary for the signature handling functionality.
10-14: LGTM!The
NFTinterface is well-defined and necessary for representing NFT data.
16-19: LGTM!The
UpdateParaminterface extendsNFTand includes additional properties for updates, which is appropriate.packages/zevm-app-contracts/contracts/xp-nft/xpNFT.sol (18)
4-5: LGTM!The imports from OpenZeppelin are appropriate and necessary for the contract functionality.
7-13: LGTM!The
Signaturestruct is well-defined and necessary for signature handling.
15-21: LGTM!The
UpdateDatastruct is well-defined and necessary for handling update data.
23-24: LGTM!The mappings for
lastUpdateTimestampByTokenIdandsignedUpByTokenIdare appropriate for tracking update timestamps and signed-up status.
26-28: LGTM!The
baseTokenURIandsignerAddressvariables are appropriately declared as public.
30-33: LGTM!The events
NewNFTMintedandNFTUpdatedare appropriately defined for emitting events on minting and updating NFTs.
35-38: LGTM!The custom errors
InvalidSigner,LengthMismatch,TransferNotAllowed, andOutdatedSignatureare well-defined and improve readability and error handling.
40-50: Initialize function is well-structured.The
initializefunction is well-structured and correctly initializes the contract with the provided parameters.
52-54: LGTM!The
versionfunction correctly returns the contract version.
56-65: LGTM!The overrides for
tokenURIandsupportsInterfaceare correctly implemented.
67-87: LGTM!The
_uint2strhelper function is correctly implemented to convert uint to string.
89-102: LGTM!The
_verifyfunction is well-implemented to verify the signature and timestamp.
104-114: LGTM!The
_calculateHashfunction correctly computes the hash of the update data.
116-120: LGTM!The
_updateNFTfunction correctly updates the NFT data after verification.
122-129: LGTM!The
mintNFTfunction correctly mints a new NFT and updates its data.
131-138: LGTM!The
updateNFTfunction correctly updates an existing NFT and emits the appropriate event.
140-143: LGTM!The
setBaseURIfunction correctly sets the base URI for tokens and is restricted to the contract owner.
145-147: LGTM!The
_transferfunction correctly prevents transfers by reverting with theTransferNotAllowederror.packages/zevm-app-contracts/test/xp-nft/xp-nft.ts (16)
1-3: LGTM!The imports are appropriate and necessary for the test cases.
4-5: LGTM!The imports for
SignerWithAddress,ethers, andupgradesare appropriate and necessary for the test cases.
7-9: LGTM!The imports for
ZetaXP,getSignature,NFT, andUpdateParamare appropriate and necessary for the test cases.
10-10: LGTM!The constant
ZETA_BASE_URLis appropriately defined.
12-29: LGTM!The
describeblock andbeforeEachsetup are well-structured and correctly initialize the necessary variables and contracts.
31-37: LGTM!The
validateNFThelper function is correctly implemented to validate the owner and URL of an NFT.
39-54: LGTM!The test case for minting an NFT is well-structured and correctly validates the minted NFT.
56-69: LGTM!The test case for emitting an event on minting is well-structured and correctly validates the event emission.
71-85: LGTM!The test case for reverting if the signature is not correct is well-structured and correctly validates the error.
87-121: LGTM!The test case for updating an NFT is well-structured and correctly validates the updated NFT.
123-144: LGTM!The test case for updating the base URL is well-structured and correctly validates the updated base URL and token URI.
146-186: LGTM!The test case for updating the base URL for minted tokens is well-structured and correctly validates the updated base URL and token URIs for multiple tokens.
188-191: LGTM!The test case for reverting if a non-owner tries to update the base URL is well-structured and correctly validates the error.
193-210: LGTM!The test case for reverting if trying to transfer an NFT is well-structured and correctly validates the error.
212-228: LGTM!The test case for reverting if trying to use the same signature twice is well-structured and correctly validates the error.
230-239: LGTM!The test case for upgrading the contract is well-structured and correctly validates the version before and after the upgrade.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| string memory name, | ||
| string memory symbol, | ||
| string memory baseTokenURI_, | ||
| address signerAddress_ |
Check notice
Code scanning / Slither
Missing zero address validation Low
| function tokenURI(uint256 tokenId) public view override(ERC721Upgradeable) returns (string memory) { | ||
| _requireMinted(tokenId); | ||
|
|
||
| return string(abi.encodePacked(baseTokenURI, _uint2str(tokenId))); | ||
| } |
Check failure
Code scanning / Slither
ABI encodePacked Collision High
There was a problem hiding this comment.
Actionable comments posted: 4
Outside diff range, codebase verification and nitpick comments (2)
packages/zevm-app-contracts/scripts/xp-nft/deploy.ts (1)
12-22: Ensure robust error handling and logging in the deployment script.The deployment logic for
ZetaXPlooks correct. However, consider adding more detailed error handling and logging to capture potential issues during deployment. This could help in diagnosing deployment failures.+ try { + const zetaXP = await upgrades.deployProxy(ZetaXPFactory, ["ZETA NFT", "ZNFT", ZETA_BASE_URL, signer]); + await zetaXP.deployed(); + console.log("ZetaXP deployed to:", zetaXP.address); + saveAddress("ZetaXP", zetaXP.address, networkName); + } catch (error) { + console.error("Deployment failed:", error); + throw error; + }packages/zevm-app-contracts/hardhat.config.ts (1)
61-61: Solidity Version 0.8.9 Not Found in ContractsThe current Solidity contracts do not use version 0.8.9. The majority of contracts are using version 0.8.7, with a few using older versions like 0.6.6 and 0.5.10. You should update the contracts to ensure compatibility with Solidity 0.8.9, as specified in the configuration.
- Consider updating the pragma statements in your Solidity files to
pragma solidity ^0.8.9;.- Ensure that all contracts are tested for compatibility with this version after updating.
Analysis chain
Validate the addition of Solidity version 0.8.9.
The inclusion of Solidity version 0.8.9 is beneficial for accessing the latest features. Ensure that all contracts are tested for compatibility with this version.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify Solidity version compatibility in contracts. # Test: Search for contracts using Solidity version 0.8.9. Expect: Compatibility across all contracts. rg --type solidity "pragma solidity ^0.8.9"Length of output: 78
Script:
#!/bin/bash # Find all Solidity files and check for pragma statements with version 0.8.9 fd --extension sol --exec rg "pragma solidity ^0.8.9" {}Length of output: 60
Script:
#!/bin/bash # List all Solidity files and display their pragma statements fd --extension sol --exec rg "pragma solidity" {}Length of output: 701
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (5)
- packages/zevm-app-contracts/contracts/xp-nft/xpNFT.sol (1 hunks)
- packages/zevm-app-contracts/data/addresses.json (1 hunks)
- packages/zevm-app-contracts/hardhat.config.ts (2 hunks)
- packages/zevm-app-contracts/scripts/xp-nft/deploy.ts (1 hunks)
- packages/zevm-app-contracts/test/xp-nft/xp-nft.ts (1 hunks)
Additional context used
GitHub Check: Slither
packages/zevm-app-contracts/contracts/xp-nft/xpNFT.sol
[notice] 44-44: Missing zero address validation
ZetaXP.initialize(string,string,string,address).signerAddress_ (contracts/xp-nft/xpNFT.sol#44) lacks a zero-check on :
- signerAddress = signerAddress_ (contracts/xp-nft/xpNFT.sol#49)
[failure] 57-61: ABI encodePacked Collision
ZetaXP.tokenURI(uint256) (contracts/xp-nft/xpNFT.sol#57-61) calls abi.encodePacked() with multiple dynamic arguments:
- string(abi.encodePacked(baseTokenURI,_uint2str(tokenId))) (contracts/xp-nft/xpNFT.sol#60)
[warning] 141-141: Conformance to Solidity naming conventions
Parameter ZetaXP.setBaseURI(string)._uri (contracts/xp-nft/xpNFT.sol#141) is not in mixedCase
Additional comments not posted (13)
packages/zevm-app-contracts/data/addresses.json (1)
10-10: Verify the usage of the newZetaXPaddress.The addition of the
ZetaXPaddress appears correct. Ensure that this address is used appropriately throughout the codebase and that any integrations or dependencies are updated accordingly.Verification successful
The
ZetaXPaddress is used appropriately in the codebase.The
ZetaXPkey is utilized in deployment and testing scripts, ensuring that the address is integrated into the application logic. The address is dynamically assigned and logged during deployment, which aligns with typical deployment practices.
- Files Involved:
packages/zevm-app-contracts/test/xp-nft/xp-nft.tspackages/zevm-app-contracts/scripts/xp-nft/deploy.tsThe integration appears correct, and no further action is required.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of the new `ZetaXP` address in the codebase. # Test: Search for occurrences of the `ZetaXP` address. Expect: Consistent usage across the codebase. rg --type json --type ts --type js "0x80ECE9a08ba893e1B863C0c6B3c098268C146E40"Length of output: 188
Script:
#!/bin/bash # Description: Verify the usage of the `ZetaXP` key in the codebase. # Test: Search for occurrences of the `ZetaXP` key. Expect: Usage in the codebase indicating integration. rg --type ts --type js "ZetaXP"Length of output: 1440
packages/zevm-app-contracts/hardhat.config.ts (1)
4-4: Confirm the necessity of the OpenZeppelin Hardhat Upgrades plugin.The addition of the OpenZeppelin Hardhat Upgrades plugin is appropriate for managing contract upgrades. Ensure that all relevant contracts are compatible with the upgradeable pattern.
packages/zevm-app-contracts/test/xp-nft/xp-nft.ts (11)
16-29: Test setup looks good.The
beforeEachsetup correctly initializes the contract and signers for testing.
39-54: Mint NFT test case is comprehensive.The test case for minting an NFT covers the necessary steps and validations.
56-69: Event emission test case is well-structured.The test case correctly verifies the emission of the
NFTMintedevent.
71-85: Signature verification test case is effective.The test case correctly checks for signature validity and reverts on invalid signatures.
87-121: NFT update test case is thorough.The test case ensures that NFT updates are correctly processed and validated.
123-144: Base URL update test case is accurate.The test case correctly verifies the update of the base URL and its effect on token URIs.
146-186: Base URL update for minted tokens test case is complete.The test case effectively checks the impact of base URL updates on existing and new tokens.
188-191: Ownership test case is correct.The test case ensures that only the owner can update the base URL.
193-210: Transfer restriction test case is valid.The test case correctly verifies that token transfers are not allowed.
212-228: Duplicate signature test case is effective.The test case ensures that using the same signature twice results in a revert.
230-239: Upgrade test case is comprehensive.The test case correctly verifies the upgradeability of the contract.
Summary
Summary by CodeRabbit
New Features
ZetaXPsmart contract for advanced NFT minting, updating, and ownership verification.Tests
Chores