Public minting fee recipient control #16
Conversation
jdubpark
requested review from
kingster-will,
Ramarti,
leeren,
Spablob,
LeoHChen and
AndyBoWu
as code owners
WalkthroughThe updates focused on enhancing the minting functionality of the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant StoryProtocolGateway
participant SPGNFT
User ->> StoryProtocolGateway: createCollection(name, symbol, maxSupply, ...)
StoryProtocolGateway ->> SPGNFT: initialize(owner, mintFeeRecipient, mintOpen, isPublicMinting, ...)
User ->> SPGNFT: mint(to)
SPGNFT ->> SPGNFT: Check mintOpen, publicMinting, and fee
SPGNFT ->> User: Mint success or revert with error
Poem
Tip Early access features: disabledWe are currently testing the following features in early access:
Note:
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 Configration 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: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (9)
- contracts/SPGNFT.sol (7 hunks)
- contracts/StoryProtocolGateway.sol (1 hunks)
- contracts/interfaces/ISPGNFT.sol (2 hunks)
- contracts/interfaces/IStoryProtocolGateway.sol (1 hunks)
- contracts/lib/Errors.sol (1 hunks)
- package.json (2 hunks)
- test/SPGNFT.t.sol (3 hunks)
- test/StoryProtocolGateway.t.sol (3 hunks)
- test/utils/BaseTest.t.sol (6 hunks)
Files skipped from review due to trivial changes (1)
- package.json
Additional comments not posted (22)
contracts/lib/Errors.sol (3)
26-26: New error type for minting denial is clear and specific.The
SPGNFT__MintingDeniederror is well-named and clearly describes a scenario where minting is denied due to the public minting status being false and the caller lacking the minter role.
32-32: New error type for fee recipient verification is clear and specific.The
SPGNFT__CallerNotFeeRecipienterror is appropriately named and clearly indicates that the caller is not the designated fee recipient, which is a crucial check for certain financial operations.
35-35: New error type for closed minting is clear and specific.The
SPGNFT__MintingClosederror is well-defined and accurately reflects a scenario where an attempt is made to mint while the minting status is closed.contracts/interfaces/ISPGNFT.sol (4)
10-20: Initialization parameters are comprehensive and well-documented.The added parameters in the
initializefunction (mintFeeRecipient,mintOpen,isPublicMinting) are crucial for the new functionalities introduced, allowing for better control over the minting process.
36-50: Getter methods for minting properties are correctly implemented.The added getter methods (
mintFee,mintFeeToken,mintFeeRecipient,mintOpen,publicMinting) provide necessary visibility into the minting settings of the collection, which aligns with the goals of enhancing control and transparency.
61-74: Setter methods for minting properties are correctly implemented and restricted.The added setter methods (
setMintFee,setMintFeeToken,setMintFeeRecipient,setMintOpen,setPublicMinting) are correctly restricted to appropriate roles, ensuring that only authorized users can change critical settings.
87-89: Withdraw function is correctly updated to handle specific tokens.The update to the
withdrawTokenfunction to allow specifying the token is a necessary improvement for handling multiple token types, enhancing flexibility in financial operations.test/SPGNFT.t.sol (4)
18-35: Setup function correctly initializes test variables and contract.The
setUpfunction is well-organized, setting up necessary test variables and correctly initializing the SPGNFT contract with the new parameters. This ensures that the contract is ready for testing with the intended settings.
51-65: Test forinitializefunction covers new parameters and roles.The
test_SPGNFT_initializefunction effectively tests the initialization of another SPGNFT contract instance with the new parameters, ensuring that roles and minting settings are correctly applied.
80-96: Error handling tests for initialization parameters are comprehensive.The tests
test_SPGNFT_initialize_revert_zeroParamsandtest_SPGNFT_initialize_revert_zeroMaxSupplycorrectly anticipate and handle potential errors due to invalid parameters, ensuring robustness in contract initialization.
206-223: Test forwithdrawTokenfunction correctly simulates fee handling and token withdrawal.The
test_SPGNFT_withdrawTokenfunction effectively simulates the process of setting a mint fee recipient, minting an NFT, and withdrawing tokens, ensuring that the token transfer logic works as expected under normal and edge cases.contracts/SPGNFT.sol (5)
19-29: Storage structure for SPGNFT is well-defined and comprehensive.The
SPGNFTStoragestruct is correctly updated to include new fields (mintFeeRecipient,mintOpen,publicMinting), which are essential for managing minting operations and fees. This structure is crucial for the new functionalities introduced.
58-72: Initialization function is robust and well-documented.The
initializefunction in the SPGNFT contract is comprehensive, including checks for non-zero mint fees and zero max supply. It also correctly sets up roles and minting settings, ensuring the contract is initialized with the correct parameters and controls.
102-125: Getter methods for minting properties are implemented correctly.The getter methods (
mintFee,mintFeeToken,mintFeeRecipient,mintOpen,publicMinting) provide necessary visibility into the contract's minting settings, ensuring transparency and control over the minting process.
Line range hint
127-162: Setter methods for minting properties are secure and appropriately restricted.The setter methods (
setMintFee,setMintFeeToken,setMintFeeRecipient,setMintOpen,setPublicMinting) are correctly restricted to appropriate roles, ensuring that only authorized users can modify critical settings, which is crucial for maintaining integrity and security in minting operations.
183-186: Withdraw function is implemented correctly for handling token transfers.The
withdrawTokenfunction is effectively designed to transfer tokens to the mint fee recipient, utilizing the correct checks and balances. This ensures that token withdrawals are handled securely and appropriately.contracts/interfaces/IStoryProtocolGateway.sol (1)
49-64: Review ofcreateCollectionfunction signature changes:The function signature for
createCollectionhas been expanded to include new parameters such asmintFeeRecipient,owner,mintOpen, andisPublicMinting. These changes align with the PR objectives and summary, aiming to enhance control over minting fees, ownership, and permissions.
mintFeeRecipientallows specifying where minting fees should be sent.ownerdesignates the owner of the collection.mintOpenandisPublicMintingcontrol the minting status and public accessibility.This expansion provides greater flexibility and control over the NFT collection creation process.
test/utils/BaseTest.t.sol (2)
Line range hint
122-147: Review of instantiation changes forAccessControllerandLicenseRegistry:The instantiation of
AccessControllerandLicenseRegistryhas been modified to include additional setup steps. These changes are critical for ensuring that the test environment mirrors the production setup closely, allowing for accurate testing of the new features introduced in the PR.
- The instantiation now includes additional checks to ensure that the deployed addresses match expected values, enhancing the robustness of the setup process.
- The use of
TestProxyHelper.deployUUPSProxysuggests that these components are intended to be upgradeable, aligning with best practices for contract flexibility.Overall, these changes enhance the test setup's accuracy and robustness, ensuring that new features can be thoroughly tested.
Line range hint
230-312: Review ofLicenseTokensetup and subsequent configuration changes:The setup for
LicenseTokeninvolves deploying a new proxy instance and setting various module interactions. This setup is crucial for testing the interactions between licensing and other modules within the system.
- The configuration of
LicenseTokenwith URLs and interactions with other modules likeLicensingModuleandDisputeModuleis essential for ensuring that licensing operations are tested in an environment that closely resembles production.- The method calls to
setAddressesandsetLicensingModuleon various contracts ensure that the system components are correctly interconnected, which is vital for end-to-end functionality testing.These changes are well-aligned with the PR's goals of enhancing control and flexibility in the licensing operations of the protocol.
test/StoryProtocolGateway.t.sol (2)
Line range hint
29-61: Review of setup changes and new tests inStoryProtocolGatewayTest:The setup for testing has been expanded to include new variables such as
feeRecipientand modifications to thecreateCollectionmethod to incorporate new parameters likemintFeeRecipientandisPublicMinting.
- The addition of
feeRecipientand its use in thecreateCollectioncall is crucial for testing the new minting fee recipient functionality.- The expanded parameters in
createCollectionallow for comprehensive testing of the collection creation process with all the new features like mint control and public minting status.These changes ensure that the tests cover the new functionalities introduced in the PR, providing a robust framework for verifying the behavior of the updated contract.
72-76: Testing of minting parameters and control flags:The tests now include assertions for minting-related parameters and control flags such as
mintFee,mintFeeToken,mintFeeRecipient,mintOpen, andpublicMinting.
- Assertions for
mintFee,mintFeeToken, andmintFeeRecipientensure that the minting fee logic is functioning as expected.- Checks for
mintOpenandpublicMintingvalidate the control mechanisms over the minting process.These tests are essential for ensuring that the minting process is correctly implemented with all the new control features, providing confidence in the system's functionality.
contracts/StoryProtocolGateway.sol (1)
119-147: Implementation of enhancedcreateCollectionmethod inStoryProtocolGateway:The
createCollectionmethod has been significantly expanded to include new parameters such asmintFeeRecipient,owner,mintOpen, andisPublicMinting. These additions are designed to provide enhanced control over the minting process and access to the collections.
- The method now correctly initializes the NFT collection with all the new parameters, ensuring that the collection's setup is fully customizable.
- The use of
BeaconProxyfor collection instantiation suggests a flexible, upgradeable approach to managing NFT collections.This implementation aligns with the PR's objectives of enhancing minting functionality and control, ensuring that the gateway can manage NFT collections effectively with the new features.
sebsadface
force-pushed
the
public-minting-fee-recipient-control
branch
from
902def1 to
32d1524
Compare
Description
This PR closes
Tests
Tests have been modified to support the modified SPG and SPG NFT contracts and the protocol core v1.0.0. More unit and integration tests are needed for the new features added by 1, 2, and 3 above.
Summary by CodeRabbit
New Features
createCollectionto include minting configuration parameters.Bug Fixes
Chores
Tests