-
Notifications
You must be signed in to change notification settings - Fork 11
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
feat(contracts): create airdrop vesting contract with manager list #124
Conversation
WalkthroughThe project introduced a comprehensive update focused on token vesting functionalities, particularly enhancing the airdrop token vesting mechanism with optional cliffs for airdrop scenarios. It encompasses the addition of a new directory for airdrop token vesting, updates to documentation, error handling, message structuring, state management, and extensive testing for various contract interactions including instantiation, execution, and querying of vesting accounts. Changes
Related issues
Poem
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 Configration File (
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files
|
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.
Review Status
Actionable comments generated: 4
Configuration used: CodeRabbit UI
Files ignored due to path filters (8)
Cargo.lock
is excluded by:!**/*.lock
contracts/airdrop-token-vesting/Cargo.lock
is excluded by:!**/*.lock
contracts/airdrop-token-vesting/Cargo.toml
is excluded by:!**/*.toml
contracts/airdrop-token-vesting/schema/cw20_hook_msg.json
is excluded by:!**/*.json
contracts/airdrop-token-vesting/schema/execute_msg.json
is excluded by:!**/*.json
contracts/airdrop-token-vesting/schema/instantiate_msg.json
is excluded by:!**/*.json
contracts/airdrop-token-vesting/schema/query_msg.json
is excluded by:!**/*.json
contracts/airdrop-token-vesting/schema/vesting_account_response.json
is excluded by:!**/*.json
Files selected for processing (13)
- Cookbook.md (1 hunks)
- README.md (1 hunks)
- contracts/README.md (1 hunks)
- contracts/airdrop-token-vesting/.cargo/config (1 hunks)
- contracts/airdrop-token-vesting/.gitignore (1 hunks)
- contracts/airdrop-token-vesting/README.md (1 hunks)
- contracts/airdrop-token-vesting/examples/schema.rs (1 hunks)
- contracts/airdrop-token-vesting/src/contract.rs (1 hunks)
- contracts/airdrop-token-vesting/src/errors.rs (1 hunks)
- contracts/airdrop-token-vesting/src/lib.rs (1 hunks)
- contracts/airdrop-token-vesting/src/msg.rs (1 hunks)
- contracts/airdrop-token-vesting/src/state.rs (1 hunks)
- contracts/airdrop-token-vesting/src/testing.rs (1 hunks)
Files skipped from review due to trivial changes (3)
- contracts/airdrop-token-vesting/.cargo/config
- contracts/airdrop-token-vesting/.gitignore
- contracts/airdrop-token-vesting/examples/schema.rs
Additional comments: 27
contracts/airdrop-token-vesting/src/lib.rs (1)
- 1-7: The module declarations and structure of
lib.rs
are clear and follow Rust's conventions for organizing code. Separating the contract logic, error handling, message definitions, and state management into their respective modules, along with a dedicated testing module, enhances maintainability and readability.contracts/README.md (1)
- 32-32: The addition of the "airdrop-token-vesting" section in the
README.md
file is a valuable update, ensuring that the documentation reflects the new features introduced in the project. It's crucial for users and developers to have easy access to information about all available contracts.contracts/airdrop-token-vesting/src/errors.rs (1)
- 1-51: The custom error types defined in
errors.rs
are well-structured and leverage thethiserror
crate effectively, facilitating clear and manageable error handling within the airdrop token vesting contract. This approach enhances the contract's robustness and developer experience by providing specific, descriptive error messages.README.md (1)
- 17-17: The update to the root
README.md
to include theairdrop-token-vesting
directory is a valuable addition, ensuring that the project documentation provides a comprehensive overview of its structure and features. Keeping the root README updated is essential for project navigation and understanding.contracts/airdrop-token-vesting/src/state.rs (1)
- 1-76: The state management defined in
state.rs
is well-structured, utilizingcw_storage_plus
effectively for managing contract state. TheWhitelist
andVestingAccount
structures are clearly defined, and the utility functiondenom_to_key
provides a practical solution for handling different token denominations. This setup facilitates clear and efficient state management within the contract.contracts/airdrop-token-vesting/README.md (1)
- 1-70: The documentation provided in the
README.md
file within theairdrop-token-vesting
directory is comprehensive and well-structured, covering key aspects of the contract's functionality, including master operations and vesting account operations. The inclusion of a section for deployed contract information, marked as TODO, is a good placeholder for future updates. This level of documentation is crucial for enabling developers and users to effectively understand and interact with the contract.contracts/airdrop-token-vesting/src/msg.rs (1)
- 1-301: The message structures and enums defined in
msg.rs
are clearly designed and cover a comprehensive range of interactions with the airdrop token vesting contract. The use ofcw_serde
for serialization and the detailed definitions of vesting schedules demonstrate a thoughtful approach to contract message design. This setup facilitates clear and flexible interactions with the contract, enhancing its usability and functionality.contracts/airdrop-token-vesting/src/contract.rs (13)
- 31-40: The validation for the deposit ensures only one type of token is deposited and that the amount is non-zero. This is a good practice for ensuring contract integrity and preventing misuse.
- 118-121: The authorization check for
reward_users
function is essential for security. It ensures that only whitelisted members or the admin can execute this action. This is a good implementation of access control.- 132-137: Checking if the total requested amount exceeds the unallocated amount before proceeding with rewarding users is a good practice. It prevents the contract from committing to more rewards than it can handle, ensuring the integrity of the vesting process.
- 209-211: Checking for the existence of a vesting account before creating a new one is a good practice to avoid overwriting existing data. This ensures data integrity and prevents accidental loss of vesting information.
- 265-268: The authorization check in
deregister_vesting_account
ensures that only the master address associated with the vesting account can deregister it. This is a critical security measure to prevent unauthorized access and manipulation of vesting accounts.- 281-305: The logic for transferring the claimable and left vesting amounts during account deregistration is well-implemented. It ensures that vested tokens are correctly distributed according to the vesting schedule and remaining tokens are returned. This maintains the integrity of the token distribution process.
- 350-352: The check for zero claimable amount in the
claim
function is a good optimization. It prevents unnecessary operations and network fees for transactions that would have no effect.- 355-363: Removing the vesting account from storage if the claimed amount equals the vesting amount is a good practice for cleaning up state and optimizing contract storage usage. This helps in managing the contract's state efficiently.
- 391-403: The
build_send_msg
function encapsulates the creation of aBankMsg::Send
message, making the code more modular and readable. This is a good example of encapsulating functionality for reuse and clarity.- 426-471: The
vesting_account
query function implementation, including pagination and filtering bymin_denom
, is well-designed. It provides a flexible and efficient way to retrieve vesting account information, adhering to best practices for smart contract queries.- 513-539: The test case
deregister_err_nonexistent_vesting_account
correctly checks for the error scenario where a non-existent vesting account is attempted to be deregistered. This is a good practice for ensuring the contract behaves as expected in error conditions.- 542-588: The test case
deregister_err_unauthorized_vesting_account
verifies that only the master address can deregister a vesting account, which is crucial for contract security. This test ensures that the contract's access control mechanisms are functioning as intended.- 591-632: The test case
deregister_successful
demonstrates the successful deregistration of a vesting account by the master address. It's important to have positive test cases like this to confirm that the contract's intended functionality works as expected.contracts/airdrop-token-vesting/src/testing.rs (7)
- 19-31: The test
proper_initialization
correctly sets up a mock environment and dependencies, and asserts that the contract can be instantiated with valid parameters. This test ensures that the contract's initialization logic is functioning as expected.- 34-81: The test
invalid_coin_sent_instantiation
effectively checks for errors when instantiating the contract with invalid coin amounts (no coins or multiple coin types). It's good practice to validate input constraints, ensuring robust error handling in the contract.- 84-160: The test
invalid_manangers_initialization
covers various scenarios of invalid manager configurations during contract instantiation, such as empty managers list, too short manager addresses, and an empty admin address. These tests are crucial for verifying input validation and error handling related to manager configurations.- 212-337: The test
register_cliff_vesting_account_with_native_token
thoroughly checks various error scenarios related to registering vesting accounts with cliffs, including zero vesting amount, invalid cliff times, and insufficient funds. These tests are essential for ensuring the contract's robustness in handling edge cases and error conditions during vesting account registration.- 340-354: The utility function
require_error
is a helpful abstraction for asserting expected errors in test scenarios. It simplifies the test code and enhances readability by encapsulating the pattern of executing a contract function and matching the result against an expected error.- 356-521: The test
register_vesting_account_with_native_token
covers scenarios for registering vesting accounts without cliffs, including zero vesting amount, excessive vesting amounts, and a successful registration with valid parameters. It also includes a query to verify the vesting account's state post-registration. This test is comprehensive and ensures that vesting account registration and state querying work as intended.- 524-680: The test
claim_native
validates the claiming process for vested tokens, including error handling for non-existent vesting entries and successful claims with correct amounts. It also verifies the state of the vesting account post-claim. This test is crucial for ensuring the correctness of the claiming logic and the contract's ability to update vesting account states accurately.
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.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- Cookbook.md (6 hunks)
Files skipped from review as they are similar to previous changes (1)
- Cookbook.md
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.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files ignored due to path filters (5)
Cargo.lock
is excluded by:!**/*.lock
contracts/airdrop-token-vesting/schema/execute_msg.json
is excluded by:!**/*.json
contracts/airdrop-token-vesting/schema/instantiate_msg.json
is excluded by:!**/*.json
contracts/airdrop-token-vesting/schema/query_msg.json
is excluded by:!**/*.json
contracts/airdrop/Cargo.toml
is excluded by:!**/*.toml
Files selected for processing (4)
- contracts/airdrop-token-vesting/src/contract.rs (1 hunks)
- contracts/airdrop-token-vesting/src/msg.rs (1 hunks)
- contracts/airdrop-token-vesting/src/state.rs (1 hunks)
- contracts/airdrop-token-vesting/src/testing.rs (1 hunks)
Files skipped from review as they are similar to previous changes (4)
- contracts/airdrop-token-vesting/src/contract.rs
- contracts/airdrop-token-vesting/src/msg.rs
- contracts/airdrop-token-vesting/src/state.rs
- contracts/airdrop-token-vesting/src/testing.rs
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.
Review Status
Actionable comments generated: 3
Configuration used: CodeRabbit UI
Files ignored due to path filters (1)
artifacts/airdrop_token_vesting.wasm
is excluded by:!**/*.wasm
Files selected for processing (1)
- Cookbook.md (6 hunks)
{ | ||
} |
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 instantiation message for the core token vesting contract is mentioned as not existing, which might be intentional to indicate that no specific parameters are required for instantiation. However, it's crucial to ensure that this is clearly communicated to avoid confusion. If instantiation does not require parameters, consider adding a brief explanation to clarify this to users.
{
+ "Note": "This contract does not require any parameters for instantiation."
}
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
{ | |
} | |
{ | |
"Note": "This contract does not require any parameters for instantiation." | |
} |
…u into mat/airdrop-vesting
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.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (2)
- Cookbook.md (6 hunks)
- contracts/airdrop-token-vesting/src/contract.rs (1 hunks)
Files skipped from review as they are similar to previous changes (2)
- Cookbook.md
- contracts/airdrop-token-vesting/src/contract.rs
…u into mat/airdrop-vesting
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.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (3)
- .gitignore (1 hunks)
- contracts/airdrop-token-vesting/src/msg.rs (1 hunks)
- contracts/nibi-stargate/README.md (2 hunks)
Files skipped from review due to trivial changes (1)
- contracts/nibi-stargate/README.md
Files skipped from review as they are similar to previous changes (1)
- contracts/airdrop-token-vesting/src/msg.rs
Additional comments: 1
.gitignore (1)
- 23-23: The addition of
.idea/
to the.gitignore
file is a good practice to prevent IDE-specific settings from cluttering the repository. This ensures that project settings specific to JetBrains IDEs, which might be used by some developers working on this Rust project, are not accidentally committed. Keeping such settings out of version control helps maintain a clean and conflict-free repository, which is beneficial for collaboration across different development environments.
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.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (2)
- contracts/airdrop-token-vesting/src/contract.rs (1 hunks)
- contracts/airdrop-token-vesting/src/msg.rs (1 hunks)
Files skipped from review as they are similar to previous changes (2)
- contracts/airdrop-token-vesting/src/contract.rs
- contracts/airdrop-token-vesting/src/msg.rs
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.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- contracts/airdrop-token-vesting/src/contract.rs (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- contracts/airdrop-token-vesting/src/contract.rs
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.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- contracts/airdrop-token-vesting/README.md (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- contracts/airdrop-token-vesting/README.md
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.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files ignored due to path filters (4)
artifacts/airdrop_token_vesting.wasm
is excluded by:!**/*.wasm
contracts/airdrop-token-vesting/schema/execute_msg.json
is excluded by:!**/*.json
contracts/airdrop-token-vesting/schema/query_msg.json
is excluded by:!**/*.json
contracts/airdrop-token-vesting/schema/vesting_account_response.json
is excluded by:!**/*.json
Files selected for processing (6)
- Cookbook.md (6 hunks)
- artifacts/checksums.txt (1 hunks)
- contracts/airdrop-token-vesting/src/contract.rs (1 hunks)
- contracts/airdrop-token-vesting/src/msg.rs (1 hunks)
- contracts/airdrop-token-vesting/src/state.rs (1 hunks)
- contracts/airdrop-token-vesting/src/testing.rs (1 hunks)
Files skipped from review as they are similar to previous changes (5)
- Cookbook.md
- contracts/airdrop-token-vesting/src/contract.rs
- contracts/airdrop-token-vesting/src/msg.rs
- contracts/airdrop-token-vesting/src/state.rs
- contracts/airdrop-token-vesting/src/testing.rs
Additional comments: 1
artifacts/checksums.txt (1)
- 2-2: The addition of the checksum for
airdrop_token_vesting.wasm
aligns with the PR's objective of introducing a new airdrop vesting contract. Ensure the checksum is verified against the actual.wasm
file to confirm its integrity.
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.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- contracts/airdrop-token-vesting/README.md (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- contracts/airdrop-token-vesting/README.md
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.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- contracts/airdrop-token-vesting/README.md (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- contracts/airdrop-token-vesting/README.md
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.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- contracts/airdrop-token-vesting/src/contract.rs (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- contracts/airdrop-token-vesting/src/contract.rs
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.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files ignored due to path filters (1)
Cargo.lock
is excluded by:!**/*.lock
Files selected for processing (1)
- contracts/airdrop-token-vesting/README.md (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- contracts/airdrop-token-vesting/README.md
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.
Could we also revert the query interface back to the same as core-token-vesting
, so that the frontend can keep using the same query patterns?
StdError::generic_err("Insufficient funds for all rewards").into() | ||
); | ||
} | ||
vesting_schedule.validate_time(env.block.time)?; |
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.
Is this time validation necessary?
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.
It depends. It is a little arbitrary because there is no consistency. Do we want to allow deploying without taking care of if the cliff or the start time is in the past?
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.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files ignored due to path filters (3)
Cargo.lock
is excluded by:!**/*.lock
artifacts/airdrop_token_vesting.wasm
is excluded by:!**/*.wasm
contracts/airdrop-token-vesting/schema/query_msg.json
is excluded by:!**/*.json
Files selected for processing (4)
- artifacts/checksums.txt (1 hunks)
- contracts/airdrop-token-vesting/src/contract.rs (1 hunks)
- contracts/airdrop-token-vesting/src/msg.rs (1 hunks)
- contracts/airdrop-token-vesting/src/testing.rs (1 hunks)
Files skipped from review as they are similar to previous changes (4)
- artifacts/checksums.txt
- contracts/airdrop-token-vesting/src/contract.rs
- contracts/airdrop-token-vesting/src/msg.rs
- contracts/airdrop-token-vesting/src/testing.rs
Summary by CodeRabbit
.gitignore
files to exclude unnecessary files.airdrop_token_vesting.wasm
artifact.This can be tested against a live chain with this script: