Cw721 timelock #9
Conversation
WalkthroughThe Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CW721Contract
participant CW721TimelockContract
User ->> CW721Contract: Transfer NFT
CW721Contract ->> CW721TimelockContract: Send NFT with Timelock Details
CW721TimelockContract ->> CW721TimelockContract: Store Timelock Information
User ->> CW721TimelockContract: Query Unlock Time
CW721TimelockContract ->> CW721TimelockContract: Retrieve Unlock Time
CW721TimelockContract ->> User: Return Unlock Time
User ->> CW721TimelockContract: Claim NFT
CW721TimelockContract ->> CW721TimelockContract: Check Unlock Time
CW721TimelockContract ->> User: Transfer NFT
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: 0
Outside diff range and nitpick comments (1)
cw721-timelock/README.md (1)
4-4: Please correct the markdown formatting issues:
- Adjust heading levels to increment by one level at a time.
- Ensure fenced code blocks are surrounded by blank lines.
Also applies to: 7-7, 25-25, 35-35, 44-44, 55-55, 63-63, 73-73, 84-84, 91-91
Tools
Markdownlint
4-4: Expected: h2; Actual: h3 (MD001, heading-increment)
Heading levels should only increment by one level at a time
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (1)
cw721-timelock/Cargo.lockis excluded by!**/*.lock
Files selected for processing (19)
- cw721-timelock/.cargo/config (1 hunks)
- cw721-timelock/.circleci/config.yml (1 hunks)
- cw721-timelock/.editorconfig (1 hunks)
- cw721-timelock/.github/workflows/Basic.yml (1 hunks)
- cw721-timelock/.github/workflows/Release.yml (1 hunks)
- cw721-timelock/.gitignore (1 hunks)
- cw721-timelock/Cargo.toml (1 hunks)
- cw721-timelock/LICENSE (1 hunks)
- cw721-timelock/NOTICE (1 hunks)
- cw721-timelock/README.md (1 hunks)
- cw721-timelock/examples/schema.rs (1 hunks)
- cw721-timelock/src/contract.rs (1 hunks)
- cw721-timelock/src/lib.rs (1 hunks)
- cw721-timelock/src/mock.rs (1 hunks)
- cw721-timelock/src/msg.rs (1 hunks)
- cw721-timelock/src/state.rs (1 hunks)
- cw721-timelock/src/testing/mock_querier.rs (1 hunks)
- cw721-timelock/src/testing/mod.rs (1 hunks)
- cw721-timelock/src/testing/tests.rs (1 hunks)
Files skipped from review due to trivial changes (8)
- cw721-timelock/.cargo/config
- cw721-timelock/.editorconfig
- cw721-timelock/.github/workflows/Basic.yml
- cw721-timelock/.github/workflows/Release.yml
- cw721-timelock/.gitignore
- cw721-timelock/LICENSE
- cw721-timelock/NOTICE
- cw721-timelock/src/testing/mod.rs
Additional context used
LanguageTool
cw721-timelock/README.md
[uncategorized] ~2-~2: Possible missing comma found. (AI_HYDRA_LEO_MISSING_COMMA)
Context: ...nt. Each locked NFT has a specific lock ID comprising the CW721 contract address c...
[uncategorized] ~62-~62: Possible missing comma found. (AI_HYDRA_LEO_MISSING_COMMA)
Context: ...ails**: Returns the details of a locked NFT including the unlock time and the recip...
Markdownlint
cw721-timelock/README.md
4-4: Expected: h2; Actual: h3 (MD001, heading-increment)
Heading levels should only increment by one level at a time
17-17: Expected: 1; Actual: 2 (MD012, no-multiple-blanks)
Multiple consecutive blank lines
18-18: Expected: 1; Actual: 3 (MD012, no-multiple-blanks)
Multiple consecutive blank lines
39-39: Expected: 1; Actual: 2 (MD012, no-multiple-blanks)
Multiple consecutive blank lines
40-40: Expected: 1; Actual: 3 (MD012, no-multiple-blanks)
Multiple consecutive blank lines
1-1: Expected: 1; Actual: 0; Below (MD022, blanks-around-headings)
Headings should be surrounded by blank lines
82-82: Expected: 1; Actual: 0; Below (MD022, blanks-around-headings)
Headings should be surrounded by blank lines
7-7: null (MD031, blanks-around-fences)
Fenced code blocks should be surrounded by blank lines
25-25: null (MD031, blanks-around-fences)
Fenced code blocks should be surrounded by blank lines
35-35: null (MD031, blanks-around-fences)
Fenced code blocks should be surrounded by blank lines
44-44: null (MD031, blanks-around-fences)
Fenced code blocks should be surrounded by blank lines
55-55: null (MD031, blanks-around-fences)
Fenced code blocks should be surrounded by blank lines
59-59: null (MD031, blanks-around-fences)
Fenced code blocks should be surrounded by blank lines
63-63: null (MD031, blanks-around-fences)
Fenced code blocks should be surrounded by blank lines
68-68: null (MD031, blanks-around-fences)
Fenced code blocks should be surrounded by blank lines
73-73: null (MD031, blanks-around-fences)
Fenced code blocks should be surrounded by blank lines
78-78: null (MD031, blanks-around-fences)
Fenced code blocks should be surrounded by blank lines
84-84: null (MD031, blanks-around-fences)
Fenced code blocks should be surrounded by blank lines
91-91: null (MD031, blanks-around-fences)
Fenced code blocks should be surrounded by blank lines
43-43: null (MD032, blanks-around-lists)
Lists should be surrounded by blank lines
62-62: null (MD032, blanks-around-lists)
Lists should be surrounded by blank lines
7-7: null (MD040, fenced-code-language)
Fenced code blocks should have a language specified
23-23: null (MD040, fenced-code-language)
Fenced code blocks should have a language specified
30-30: null (MD040, fenced-code-language)
Fenced code blocks should have a language specified
44-44: null (MD040, fenced-code-language)
Fenced code blocks should have a language specified
55-55: null (MD040, fenced-code-language)
Fenced code blocks should have a language specified
63-63: null (MD040, fenced-code-language)
Fenced code blocks should have a language specified
73-73: null (MD040, fenced-code-language)
Fenced code blocks should have a language specified
84-84: null (MD040, fenced-code-language)
Fenced code blocks should have a language specified
Additional comments not posted (48)
cw721-timelock/src/lib.rs (5)
1-1: Thecontractmodule declaration is correctly placed and essential for the core functionality.
2-2: Themsgmodule is well-defined, ensuring all message handling is centralized.
3-3: Thestatemodule is appropriately defined for managing the contract's state.
6-6: Thetestingmodule is correctly included, facilitating thorough testing of the contract.
9-9: Themockmodule is well-placed for non-WASM testing, enhancing test coverage.cw721-timelock/examples/schema.rs (1)
7-16: The main function is well-implemented, correctly setting up the schema directory and generating the necessary schemas.cw721-timelock/src/state.rs (2)
10-14: TheTimelockInfostruct is well-defined, usingMillisecondsExpirationfor precise time management.
8-8: TheTIMELOCKSmap is correctly implemented, ensuring efficient storage and retrieval of timelock information.cw721-timelock/src/msg.rs (6)
12-14: TheInstantiateMsgstruct is appropriately designed to allow optional specification of authorized token addresses, providing flexibility in contract initialization.
18-24: TheExecuteMsgenum is well-structured, clearly defining the operations for receiving and claiming NFTs.
27-32: TheCw721HookMsgenum is crucial for the timelock functionality, correctly specifying parameters like lock duration and recipient.
37-48: TheQueryMsgenum is effectively designed, facilitating queries for unlock times and NFT details.
51-59: The response structuresUnlockTimeResponseandNftDetailsResponseare well-defined, ensuring clear and informative responses to queries.
62-64: TheTokenExtensionstruct is a thoughtful addition, enhancing token functionality by including a publisher field.cw721-timelock/Cargo.toml (4)
1-6: The package information is correctly specified.
17-18: The library configuration is appropriate for a Rust project targeting both Rust and C-compatible libraries.
20-26: Please confirm if thelibraryfeature is intentionally left empty to disable default exports.
28-42: Dependencies are correctly specified and align with the project's requirements.cw721-timelock/src/mock.rs (3)
18-41: TheMockCw721Timelockstruct and its instantiation function are correctly implemented for testing purposes.
44-47: Themock_andromeda_timelockfunction correctly wraps the contract logic for testing.
49-54: Please confirm if theInstantiateMsgis intentionally left empty for testing default behaviors.cw721-timelock/.circleci/config.yml (2)
8-26: Thedocker-imagejob is correctly configured to build and push Docker images based on themasterbranch condition.
28-61: Thedocker-taggedjob is correctly configured to handle Docker image tagging and pushing based on Git tags.cw721-timelock/README.md (2)
1-12: The introduction and message structures in the README are clearly documented and provide a good overview of the CW721 Timelock functionality.Tools
LanguageTool
[uncategorized] ~2-~2: Possible missing comma found. (AI_HYDRA_LEO_MISSING_COMMA)
Context: ...nt. Each locked NFT has a specific lock ID comprising the CW721 contract address c...Markdownlint
4-4: Expected: h2; Actual: h3 (MD001, heading-increment)
Heading levels should only increment by one level at a time
1-1: Expected: 1; Actual: 0; Below (MD022, blanks-around-headings)
Headings should be surrounded by blank lines
7-7: null (MD031, blanks-around-fences)
Fenced code blocks should be surrounded by blank lines
7-7: null (MD040, fenced-code-language)
Fenced code blocks should have a language specified
82-94: The state documentation is clear and effectively explains how the contract manages the state of locked NFTs.Tools
Markdownlint
82-82: Expected: 1; Actual: 0; Below (MD022, blanks-around-headings)
Headings should be surrounded by blank lines
84-84: null (MD031, blanks-around-fences)
Fenced code blocks should be surrounded by blank lines
91-91: null (MD031, blanks-around-fences)
Fenced code blocks should be surrounded by blank lines
84-84: null (MD040, fenced-code-language)
Fenced code blocks should have a language specifiedcw721-timelock/src/testing/mock_querier.rs (6)
20-51: The functionmock_dependencies_customcorrectly sets up a custom testing environment with initialized contract values. Good use of default and custom querier initialization.
53-73: TheWasmMockQuerierstruct is well-implemented, providing necessary functionality to extendMockQuerierfor custom query handling.
82-117: Thehandle_querymethod effectively routes queries to the appropriate handlers and manages unsupported queries gracefully.
119-128: Thehandle_app_querymethod correctly handles app-specific queries with validation against a list of identifiers, ensuring data integrity.
130-151: Thehandle_token_querymethod is effectively implemented to handle token-specific queries, correctly differentiating between claimed and unclaimed tokens.
153-160: The constructor forWasmMockQuerieris well-implemented, correctly initializing the querier with default values for testing.cw721-timelock/src/contract.rs (9)
29-74: Theinstantiatefunction is well-implemented, correctly setting the contract version and initializing the contract with appropriate permissions for authorized token addresses.
76-91: Theexecutefunction effectively handles the execution of messages, correctly routing them based on the message type and using a context object for state management.
93-103: Thehandle_executefunction is effectively implemented, routing execution messages to their respective handlers based on the message type.
105-126: Thehandle_receive_cw721function correctly handles the receipt of CW721 tokens, setting up the timelock with necessary permission checks.
128-169: Theexecute_timelock_cw721function is well-implemented, correctly setting up the timelock for a CW721 token with validation and state management.
171-210: Theexecute_claim_cw721function correctly handles the claiming of a CW721 token after the timelock period, with necessary checks and token transfer.
212-223: Thequeryfunction effectively handles query messages, correctly routing them based on the message type and encoding the response.
225-235: Thequery_unlock_timefunction correctly retrieves and returns the unlock time for a CW721 token, effectively accessing the state for timelock information.
237-248: Thequery_nft_detailsfunction correctly retrieves and returns the details of a locked CW721 token, effectively accessing the state for timelock information.cw721-timelock/src/testing/tests.rs (8)
20-33: Thetest_instantiatefunction correctly tests the contract instantiation with default values, verifying that no messages are sent during the process.
35-91: Thetest_timelock_cw721function correctly tests the timelock setup for a CW721 token, verifying the response attributes and the unlock time through a query.
93-149: Thetest_claim_cw721function correctly tests the claiming process for a CW721 token after the timelock period, verifying the response attributes and simulating ownership transfer through a raw query.
151-175: Thetest_too_short_lock_durationfunction correctly tests the error handling for a lock duration that is too short, verifying that the appropriate error is returned.
177-201: Thetest_too_long_lock_durationfunction correctly tests the error handling for a lock duration that is too long, verifying that the appropriate error is returned.
203-242: Thetest_locked_nftfunction correctly tests the error handling when attempting to claim a CW721 token before the unlock time, verifying that the appropriate error is returned.
244-279: Thetest_query_nft_detailsfunction correctly tests the querying of details for a locked CW721 token, verifying the unlock time and recipient address through a query.
281-314: Thetest_query_unlocktimefunction correctly tests the querying of the unlock time for a CW721 token, verifying the unlock time through a query.
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (1)
cw721-timelock/Cargo.lockis excluded by!**/*.lock
Files selected for processing (7)
- cw721-timelock/Cargo.toml (1 hunks)
- cw721-timelock/src/contract.rs (1 hunks)
- cw721-timelock/src/lib.rs (1 hunks)
- cw721-timelock/src/msg.rs (1 hunks)
- cw721-timelock/src/testing/integration_tests.rs (1 hunks)
- cw721-timelock/src/testing/mod.rs (1 hunks)
- cw721-timelock/src/testing/tests.rs (1 hunks)
Files skipped from review as they are similar to previous changes (4)
- cw721-timelock/Cargo.toml
- cw721-timelock/src/lib.rs
- cw721-timelock/src/msg.rs
- cw721-timelock/src/testing/tests.rs
Additional comments not posted (9)
cw721-timelock/src/testing/mod.rs (3)
1-1: Modulemock_queriercorrectly declared for testing purposes.
2-2: Moduletestscorrectly declared for unit testing.
3-3: Moduleintegration_testscorrectly declared for integration testing.cw721-timelock/src/testing/integration_tests.rs (3)
17-19: Functionmock_appcorrectly initializes a mock application environment for testing.
21-24: Functioncontract_cw721_timelockcorrectly wraps the contract execution, instantiation, and query functions.
31-177: Integration testcw721_timelock_testis comprehensive, covering instantiation, querying, and transaction scenarios. Ensure all edge cases are covered, such as invalid inputs and unauthorized access attempts.cw721-timelock/src/contract.rs (3)
29-73: Functioninstantiatecorrectly sets up the contract with versioning and permissions. Ensure that the permissions setup is secure and follows best practices.
212-223: Functionquerycorrectly handles different query messages. Ensure that all query types are correctly processed and that error handling is robust.Verification successful
The
QueryMsgenum incw721-timelock/src/msg.rsincludes the following variants:
UnlockTime { cw721_contract: AndrAddr, token_id: String }NftDetails { cw721_contract: AndrAddr, token_id: String }The
queryfunction incw721-timelock/src/contract.rshandles both of these variants correctly. There are no additional variants in theQueryMsgenum that need to be handled.
UnlockTimeis handled byquery_unlock_time.NftDetailsis handled byquery_nft_details.No issues were found with the handling of query types in the
queryfunction.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all query types are handled correctly. # Test: Search for query handling logic. rg --type rust $'match msg' cw721-timelock/src/contract.rsLength of output: 107
Script:
#!/bin/bash # Description: Find the definition and variants of the QueryMsg enum. # Search for the QueryMsg enum definition and its variants. ast-grep --lang rust --pattern 'enum QueryMsg { $$$ }'Length of output: 56
Script:
#!/bin/bash # Description: Search for the QueryMsg keyword in the codebase to locate its definition. # Search for the QueryMsg keyword in the codebase. rg 'QueryMsg'Length of output: 2456
Script:
#!/bin/bash # Description: Extract the QueryMsg enum definition from cw721-timelock/src/msg.rs. # Extract the QueryMsg enum definition. ast-grep --lang rust --pattern 'enum QueryMsg { $$$ }' cw721-timelock/src/msg.rsLength of output: 82
Script:
#!/bin/bash # Description: Search for the QueryMsg keyword in the specific file cw721-timelock/src/msg.rs to locate its definition. # Search for the QueryMsg keyword in the specific file. rg 'QueryMsg' cw721-timelock/src/msg.rsLength of output: 59
Script:
#!/bin/bash # Description: Extract the full definition of the QueryMsg enum from cw721-timelock/src/msg.rs. # Extract the full definition of the QueryMsg enum. sed -n '/pub enum QueryMsg {/,/}/p' cw721-timelock/src/msg.rsLength of output: 202
Script:
#!/bin/bash # Description: Extract the complete definition of the QueryMsg enum from cw721-timelock/src/msg.rs. # Extract the complete definition of the QueryMsg enum. sed -n '/pub enum QueryMsg {/,/}/p' cw721-timelock/src/msg.rsLength of output: 202
Script:
#!/bin/bash # Description: Extract the complete definition of the QueryMsg enum from cw721-timelock/src/msg.rs. # Extract the complete definition of the QueryMsg enum. sed -n '/pub enum QueryMsg {/,/}/p' cw721-timelock/src/msg.rsLength of output: 202
Script:
#!/bin/bash # Description: Display the contents of cw721-timelock/src/msg.rs to manually verify the QueryMsg enum definition. # Display the contents of the file. cat cw721-timelock/src/msg.rsLength of output: 1264
76-91: Functionexecutecorrectly handles different contract messages. Ensure that all message types are correctly processed and that error handling is robust.Verification successful
The
ExecuteMsgenum has the following variants:
ReceiveNft(Cw721ReceiveMsg)ClaimNft { cw721_contract: AndrAddr, token_id: String }Additionally, the
AMPReceivevariant is handled in theexecutefunction, but it is not part of theExecuteMsgenum. This suggests thatAMPReceivemight be part of another enum or struct.The
executefunction handles the following variants:
ExecuteMsg::AMPReceive(pkt)ExecuteMsg::ReceiveNft(msg)ExecuteMsg::ClaimNftSince all variants of the
ExecuteMsgenum are handled in theexecutefunction, the message handling logic appears to be correctly implemented.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all message types are handled correctly. # Test: Search for message handling logic. rg --type rust $'match msg' cw721-timelock/src/contract.rsLength of output: 107
Script:
#!/bin/bash # Description: Identify all variants of the `ExecuteMsg` enum. # Search for the definition of the `ExecuteMsg` enum. rg --type rust 'enum ExecuteMsg' # List all variants of the `ExecuteMsg` enum. rg --type rust 'ExecuteMsg::'Length of output: 1973
Script:
#!/bin/bash # Description: Extract the full definition of the `ExecuteMsg` enum. # Extract the definition of the `ExecuteMsg` enum from `cw721-timelock/src/msg.rs`. rg --type rust -A 20 'pub enum ExecuteMsg' cw721-timelock/src/msg.rsLength of output: 455
…tead of a valid address
There was a problem hiding this comment.
Actionable comments posted: 15
Outside diff range and nitpick comments (1)
cw721-timelock/src/contract.rs (1)
Line range hint
210-314: Review thequeryfunction and specific query handlers for correctness.The
queryfunction correctly routes query messages to their respective handlers. Thequery_unlock_timeandquery_nft_detailsfunctions correctly retrieve and return the requested information. Ensure that the queries handle errors such as missing or invalid data gracefully.Consider adding more detailed logging or attributes to help trace the query execution and improve debuggability.
pub fn query( deps: Deps, env: Env, msg: QueryMsg, ) -> Result<Binary, ContractError> { + // Log the received query message type for better traceability match msg { QueryMsg::UnlockTime { cw721_contract, token_id } => encode_binary(&query_unlock_time(deps, cw721_contract, token_id)?), ... } }
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (4)
- cw721-timelock/src/contract.rs (1 hunks)
- cw721-timelock/src/msg.rs (1 hunks)
- cw721-timelock/src/testing/integration_tests.rs (1 hunks)
- cw721-timelock/src/testing/tests.rs (1 hunks)
Additional comments not posted (8)
cw721-timelock/src/msg.rs (4)
1-7: Ensure consistent import grouping.Consider grouping imports from the same module together for better readability.
- use cosmwasm_std::{Addr}; - use cosmwasm_schema::{cw_serde, QueryResponses}; - use andromeda_std::{ - andr_exec, andr_instantiate, andr_query, - amp::AndrAddr, - common::{milliseconds::MillisecondsDuration}, - }; + use cosmwasm_std::Addr; + use cosmwasm_schema::{cw_serde, QueryResponses}; + use andromeda_std::{andr_exec, andr_instantiate, andr_query, amp::AndrAddr, common::milliseconds::MillisecondsDuration};
18-24: Review enumExecuteMsgfor completeness and correctness.The
ExecuteMsgenum correctly handlesReceiveNftandClaimNftas per the PR description. Ensure that the fields and types used match the requirements specified for these operations.
34-48: Review query enums for completeness.The
QueryMsgenum correctly definesUnlockTimeandNftDetailswith the appropriate return types and fields as described in the PR. Ensure that the handling of these queries in the contract matches these definitions.
50-59: Check response structs for completeness.The
UnlockTimeResponseandNftDetailsResponsestructs are well-defined and match the descriptions in the PR. Ensure that all necessary fields are included and correctly typed.cw721-timelock/src/testing/integration_tests.rs (2)
1-9: Ensure correct use of imports and modules.The imports are correctly structured and relevant to the tests being performed. Ensure that no unnecessary imports are included and that all necessary modules are referenced.
21-29: Check contract wrapper functions for correctness.The
contract_cw721_timelockandcontract_cw721functions correctly wrap the contract execution, instantiation, and query handlers. Ensure these are correctly linked to the respective contract implementations.cw721-timelock/src/contract.rs (1)
1-4: Check global configuration and imports.The global configuration and imports are set correctly for a non-library build. Ensure that all necessary libraries and modules are imported for the contract's functionality.
cw721-timelock/src/testing/tests.rs (1)
1-11: Ensure correct use of imports and modules in unit tests.The imports are correctly structured and relevant to the tests being performed. Ensure that no unnecessary imports are included and that all necessary modules are referenced.
…instead of a valid address
There was a problem hiding this comment.
Actionable comments posted: 10
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (4)
- cw721-timelock/src/contract.rs (1 hunks)
- cw721-timelock/src/msg.rs (1 hunks)
- cw721-timelock/src/testing/integration_tests.rs (1 hunks)
- cw721-timelock/src/testing/tests.rs (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- cw721-timelock/src/msg.rs
Additional comments not posted (1)
cw721-timelock/src/testing/tests.rs (1)
20-33: Enhance thetest_instantiatefunction with detailed comments.The
test_instantiatefunction correctly tests the instantiation of the contract with minimal parameters. Adding more detailed comments or descriptions to each test block will improve maintainability and understandability of the test code.
[REFACTOR_Suggestion]#[test] fn test_instantiate() { + // Test the instantiation of the CW721 Timelock contract with minimal parameters let mut deps = mock_dependencies_custom(&[]); ... }
There was a problem hiding this comment.
Actionable comments posted: 4
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (4)
- cw721-timelock/src/contract.rs (1 hunks)
- cw721-timelock/src/msg.rs (1 hunks)
- cw721-timelock/src/testing/integration_tests.rs (1 hunks)
- cw721-timelock/src/testing/tests.rs (1 hunks)
Files skipped from review as they are similar to previous changes (2)
- cw721-timelock/src/msg.rs
- cw721-timelock/src/testing/tests.rs
Additional comments not posted (2)
cw721-timelock/src/testing/integration_tests.rs (1)
1-12: Review import statements for relevance and correctness.All imports seem relevant to the testing context. Ensure that
MOCK_KERNEL_CONTRACTis appropriately used and that its value is correctly set for the testing environment.cw721-timelock/src/contract.rs (1)
1-12: Review import and configuration statements for correctness and relevance.Ensure that all imports are necessary for the contract's functionality. The configuration constants (
CONTRACT_NAME,CONTRACT_VERSION,ONE_DAY,ONE_YEAR) are correctly defined and used throughout the contract.
There was a problem hiding this comment.
Actionable comments posted: 6
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- cw721-timelock/src/contract.rs (1 hunks)
Additional comments not posted (1)
cw721-timelock/src/contract.rs (1)
1-12: Ensure appropriate feature gating and library usage.The use of
#[cfg(not(feature = "library"))]andentry_pointannotations are correct, ensuring that the contract entry points are only compiled when not used as a library. This is a best practice in Rust for conditional compilation.
|
Looks good to me, would be nice to have it tried on testnet. With each step documented along its |
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- cw721-timelock/src/contract.rs (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- cw721-timelock/src/contract.rs
…ed comments of code base
There was a problem hiding this comment.
Actionable comments posted: 2
Outside diff range and nitpick comments (9)
cw721-timelock/src/testing/tests.rs (9)
23-36: Review thetest_instantiatefunction for coverage and correctness.The
test_instantiatefunction correctly tests the instantiation of the contract with minimal parameters. Ensure that all necessary assertions are made to verify the correct initialization of the contract.Consider adding more detailed comments or descriptions to each test block to explain the purpose and expected outcome of the tests. This will improve maintainability and understandability of the test code.
#[test] fn test_instantiate() { + // Test the instantiation of the CW721 Timelock contract with minimal parameters let mut deps = mock_dependencies_custom(&[]); ... }
38-74: Review thetest_timelock_cw721function for coverage and correctness.This test covers the functionality of locking an NFT. Ensure that all assertions are correct and that edge cases, such as incorrect lock durations or unauthorized access, are tested.
Consider adding more detailed comments or descriptions to each test block to explain the purpose and expected outcome of the tests. This will improve maintainability and understandability of the test code.
#[test] fn test_timelock_cw721() { + // Test the locking of an NFT in the CW721 Timelock contract let mut deps = mock_dependencies_custom(&[]); ... }
76-136: Review thetest_claim_cw721function for coverage and correctness.This test covers the functionality of claiming an NFT after the lock period. Ensure that all assertions are correct and that edge cases, such as attempting to claim before the unlock time, are tested.
Consider adding more detailed comments or descriptions to each test block to explain the purpose and expected outcome of the tests. This will improve maintainability and understandability of the test code.
#[test] fn test_claim_cw721() { + // Test the claiming of an NFT after the lock period in the CW721 Timelock contract let mut deps = mock_dependencies_custom(&[]); ... }
138-188: Review the error condition teststest_too_short_lock_durationandtest_too_long_lock_durationfor coverage and correctness.These tests correctly handle the scenarios where the lock duration is too short or too long. Ensure that the error handling is robust and that all necessary assertions are made.
Consider adding more detailed comments or descriptions to each test block to explain the purpose and expected outcome of the tests. This will improve maintainability and understandability of the test code.
#[test] fn test_too_short_lock_duration() { + // Test the error condition when the lock duration is too short let mut deps = mock_dependencies_custom(&[]); ... } #[test] fn test_too_long_lock_duration() { + // Test the error condition when the lock duration is too long let mut deps = mock_dependencies_custom(&[]); ... }
190-229: Review thetest_locked_nftfunction for coverage and correctness.This test covers the scenarios of a locked NFT. Ensure that all assertions are correct and that the tests cover the necessary scenarios as described in the PR.
Consider adding more detailed comments or descriptions to each test block to explain the purpose and expected outcome of the tests. This will improve maintainability and understandability of the test code.
#[test] fn test_locked_nft() { + // Test the scenario where an NFT is locked in the CW721 Timelock contract let mut deps = mock_dependencies_custom(&[]); ... }
231-257: Review thetest_claim_non_existent_timelocked_nftfunction for coverage and correctness.This test covers the scenario of attempting to claim a non-existent time-locked NFT. Ensure that all assertions are correct and that the test covers the necessary scenarios as described in the PR.
Consider adding more detailed comments or descriptions to each test block to explain the purpose and expected outcome of the tests. This will improve maintainability and understandability of the test code.
#[test] fn test_claim_non_existent_timelocked_nft() { + // Test the scenario of attempting to claim a non-existent time-locked NFT in the CW721 Timelock contract let mut deps = mock_dependencies_custom(&[]); ... }
259-294: Review thetest_query_nft_detailsfunction for coverage and correctness.This test covers the scenarios of querying NFT details. Ensure that all assertions are correct and that the tests cover the necessary scenarios as described in the PR.
Consider adding more detailed comments or descriptions to each test block to explain the purpose and expected outcome of the tests. This will improve maintainability and understandability of the test code.
#[test] fn test_query_nft_details() { + // Test querying the details of a locked NFT in the CW721 Timelock contract let mut deps = mock_dependencies_custom(&[]); ... }
296-329: Review thetest_query_unlocktimefunction for coverage and correctness.This test covers the functionality of querying the unlock time for an NFT. Ensure that all assertions are correct and that the test covers the necessary scenarios as described in the PR.
Consider adding more detailed comments or descriptions to each test block to explain the purpose and expected outcome of the tests. This will improve maintainability and understandability of the test code.
#[test] fn test_query_unlocktime() { + // Test querying the unlock time for an NFT in the CW721 Timelock contract let mut deps = mock_dependencies_custom(&[]); ... }
331-368: Review thetest_query_is_lockedfunction for coverage and correctness.This test covers the functionality of querying whether an NFT is locked. Ensure that all assertions are correct and that the test covers the necessary scenarios as described in the PR.
Consider adding more detailed comments or descriptions to each test block to explain the purpose and expected outcome of the tests. This will improve maintainability and understandability of the test code.
#[test] fn test_query_is_locked() { + // Test querying whether an NFT is locked in the CW721 Timelock contract let mut deps = mock_dependencies_custom(&[]); ... }
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (4)
- cw721-timelock/Cargo.toml (1 hunks)
- cw721-timelock/src/contract.rs (1 hunks)
- cw721-timelock/src/state.rs (1 hunks)
- cw721-timelock/src/testing/tests.rs (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- cw721-timelock/src/contract.rs
Additional comments not posted (5)
cw721-timelock/src/state.rs (1)
8-12: Struct definition looks good.The
TimelockInfostruct is well-defined and follows best practices.cw721-timelock/Cargo.toml (4)
4-4: Ensure that the email address is correct.Verify that the author's email address is correct and up-to-date.
6-6: Ensure compatibility with the specified Rust version.Verify that the codebase is compatible with Rust version
1.78.0.
8-13: Review the excluded files.Ensure that the excluded files are correctly specified and necessary for the project.
27-47: Dependency list looks good.The dependencies are correctly specified and necessary for the project.
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- cw721-timelock/src/contract.rs (1 hunks)
- cw721-timelock/src/state.rs (1 hunks)
- cw721-timelock/src/testing/tests.rs (1 hunks)
Files skipped from review as they are similar to previous changes (3)
- cw721-timelock/src/contract.rs
- cw721-timelock/src/state.rs
- cw721-timelock/src/testing/tests.rs
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- cw721-timelock/src/testing/tests.rs (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- cw721-timelock/src/testing/tests.rs
|
@coderabbitai review |
Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 17
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (7)
- cw721-timelock/src/contract.rs (1 hunks)
- cw721-timelock/src/msg.rs (1 hunks)
- cw721-timelock/src/state.rs (1 hunks)
- cw721-timelock/src/testing/integration_tests.rs (1 hunks)
- cw721-timelock/src/testing/mock_querier.rs (1 hunks)
- cw721-timelock/src/testing/mod.rs (1 hunks)
- cw721-timelock/src/testing/tests.rs (1 hunks)
Files skipped from review due to trivial changes (1)
- cw721-timelock/src/testing/mod.rs
Additional comments not posted (10)
cw721-timelock/src/state.rs (2)
6-6: Approved: Use of tuple key forTIMELOCKSmap.The use of a tuple key
(&Addr, &str)for theTIMELOCKSmap is appropriate for distinguishing between different contracts and token IDs.
8-12: Approved:TimelockInfostruct definition.The
TimelockInfostruct is correctly defined withunlock_timeandrecipientfields.cw721-timelock/src/msg.rs (2)
18-24: Approved:ExecuteMsgenum definition.The
ExecuteMsgenum is correctly defined withReceiveNftandClaimNftvariants.
37-52: Approved:QueryMsgenum definition.The
QueryMsgenum is correctly defined withUnlockTime,NftDetails, andIsLockedvariants.cw721-timelock/src/testing/mock_querier.rs (4)
20-51: Approved:mock_dependencies_customfunction definition.The
mock_dependencies_customfunction is correctly defined and initializes custom mock dependencies for testing.
53-57: Approved:WasmMockQuerierstruct definition.The
WasmMockQuerierstruct is correctly defined withbase,contract_address, andtokens_left_to_burnfields.
83-117: Approved:WasmMockQuerier::handle_queryfunction definition.The
handle_queryfunction is correctly defined and handles different types of queries appropriately.
130-151: Approved:WasmMockQuerier::handle_token_queryfunction definition.The
handle_token_queryfunction is correctly defined and handles token queries for the mock CW721 contract appropriately.cw721-timelock/src/testing/integration_tests.rs (2)
28-31: LGTM!The function correctly creates a new
ContractWrapperfor the CW721 Timelock contract.
33-36: LGTM!The function correctly creates a new
ContractWrapperfor the CW721 contract.
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- cw721-timelock/src/msg.rs (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- cw721-timelock/src/msg.rs
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (19)
- cw721-timelock/.gitignore (1 hunks)
- cw721-timelock/schema/andromeda-cw721-timelock.json (1 hunks)
- cw721-timelock/schema/cw721receive.json (1 hunks)
- cw721-timelock/schema/raw/execute.json (1 hunks)
- cw721-timelock/schema/raw/instantiate.json (1 hunks)
- cw721-timelock/schema/raw/query.json (1 hunks)
- cw721-timelock/schema/raw/response_to_app_contract.json (1 hunks)
- cw721-timelock/schema/raw/response_to_block_height_upon_creation.json (1 hunks)
- cw721-timelock/schema/raw/response_to_is_locked.json (1 hunks)
- cw721-timelock/schema/raw/response_to_kernel_address.json (1 hunks)
- cw721-timelock/schema/raw/response_to_nft_details.json (1 hunks)
- cw721-timelock/schema/raw/response_to_original_publisher.json (1 hunks)
- cw721-timelock/schema/raw/response_to_owner.json (1 hunks)
- cw721-timelock/schema/raw/response_to_ownership_request.json (1 hunks)
- cw721-timelock/schema/raw/response_to_permissioned_actions.json (1 hunks)
- cw721-timelock/schema/raw/response_to_permissions.json (1 hunks)
- cw721-timelock/schema/raw/response_to_type.json (1 hunks)
- cw721-timelock/schema/raw/response_to_unlock_time.json (1 hunks)
- cw721-timelock/schema/raw/response_to_version.json (1 hunks)
Files skipped from review due to trivial changes (6)
- cw721-timelock/.gitignore
- cw721-timelock/schema/raw/response_to_is_locked.json
- cw721-timelock/schema/raw/response_to_original_publisher.json
- cw721-timelock/schema/raw/response_to_owner.json
- cw721-timelock/schema/raw/response_to_permissioned_actions.json
- cw721-timelock/schema/raw/response_to_type.json
Additional comments not posted (17)
cw721-timelock/schema/raw/response_to_version.json (1)
1-14: Schema looks good.The
VersionResponseschema is well-defined with a requiredversionproperty. The use ofadditionalProperties: falseis a good practice to ensure strict validation.cw721-timelock/schema/raw/response_to_unlock_time.json (1)
1-16: Schema looks good.The
UnlockTimeResponseschema is well-defined with a requiredunlock_timeproperty. The constraints andadditionalProperties: falseensure proper validation.cw721-timelock/schema/raw/response_to_block_height_upon_creation.json (1)
1-16: Schema looks good.The
BlockHeightResponseschema is well-defined with a requiredblock_heightproperty. The constraints andadditionalProperties: falseensure proper validation.cw721-timelock/schema/raw/response_to_app_contract.json (1)
1-20: Schema definition looks good.The JSON schema for
AppContractResponseis well-defined and includes necessary properties and definitions.cw721-timelock/schema/raw/response_to_kernel_address.json (1)
1-20: Schema definition looks good.The JSON schema for
KernelAddressResponseis well-defined and includes necessary properties and definitions.cw721-timelock/schema/raw/response_to_nft_details.json (1)
1-26: Schema definition looks good.The JSON schema for
NftDetailsResponseis well-defined and includes necessary properties and definitions.cw721-timelock/schema/raw/instantiate.json (1)
1-36: Schema forInstantiateMsgis well-structured.The JSON schema is well-defined with clear constraints and appropriate use of
$reffor reusability. The pattern forAndrAddris comprehensive, covering various address formats.cw721-timelock/schema/raw/response_to_ownership_request.json (1)
1-40: Schema forContractPotentialOwnerResponseis clear and well-documented.The use of
anyOffor optional properties is appropriate, and the definitions forAddrandMillisecondsare clear and informative.cw721-timelock/schema/raw/response_to_permissions.json (1)
1-112: Schema forArray_of_PermissionInfois detailed and flexible.The schema is well-structured with comprehensive definitions for permissions. The use of
oneOffor thePermissionenum provides flexibility and clarity in defining permissions.cw721-timelock/schema/cw721receive.json (1)
1-81: Schema is well-defined and complete.The JSON schema for
cw721receiveis comprehensive and includes necessary properties and constraints. The definitions are detailed and clear.cw721-timelock/schema/raw/query.json (1)
1-240: Schema is well-defined and complete.The JSON schema for
QueryMsgis comprehensive and includes necessary properties and constraints. The definitions are detailed and clear.cw721-timelock/schema/raw/execute.json (1)
1-592: Schema is well-defined and complete.The JSON schema for
ExecuteMsgis comprehensive and includes necessary properties and constraints. The definitions are detailed and clear.cw721-timelock/schema/andromeda-cw721-timelock.json (5)
1-4: Contract metadata looks good.The contract name, version, and IDL version are correctly defined.
5-40: Instantiate message schema is well-defined.The properties and constraints for initializing the contract are clearly specified.
41-160: Execute message schema is comprehensive and clear.The operations and their respective properties are well-defined and validated.
635-865: Query message schema is well-structured.The queries and their properties are clearly defined and appropriately constrained.
877-1205: Response schemas are well-defined.Each response type is clearly specified with appropriate properties and constraints.
Contact Information
Summary: With the CW721 Timelock, you can lock an NFT (CW721) with a contract for a certain amount of time (currently between one day & one year). Once the timelock has expired, anyone can call the claim function to send the NFT to the defined recipient. Each locked NFT has a specific lock ID comprising the CW721 contract address concatenated with the token_id.
This is the solution of #7
Messages
Instantiation (What is specified and stored at instantiation)
authorized_token_addresses: An optional vector of addresses that are authorized to interact with the contract. If not specified, any address can interact.
Execute Messages (What are the messages that can be executed)
Cw721ReceiveMsg: The message received when an NFT is sent to this contract. This message includes the sender, the token ID, and a message for further handling.
cw721_contract: The address of the CW721 contract.
token_id: The ID of the token to be claimed.
Query Messages (What are the messages that can be queried, what does each return)
cw721_contract: The address of the CW721 contract.
token_id: The ID of the token.
Returns:
unlock_time: The time at which the NFT can be claimed.
cw721_contract: The address of the CW721 contract.
token_id: The ID of the token.
Returns:
unlock_time: The time at which the NFT can be claimed.
recipient: The address of the recipient who can claim the NFT after the unlock time.
State
The contract maintains the following state:
TimelockInfo: Structure holding the unlock time and the recipient address for each locked NFT.
TIMELOCKS: A mapping from token IDs to their respective TimelockInfo.
This state ensures that each NFT has its own lock period and designated recipient.
Summary by CodeRabbit
New Features
Documentation
Tests