fix: normalize query output for vesting contracts #130
Conversation
WalkthroughThe comprehensive updates in the airdrop token vesting contract enhance functionality by introducing a whitelist, managing token denomination, refining vesting schedule calculations, and optimizing the Changes
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 (
|
matthiasmatt
changed the title
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files ignored due to path filters (3)
Cargo.lockis excluded by:!**/*.lockcontracts/airdrop-token-vesting/schema/execute_msg.jsonis excluded by:!**/*.jsoncontracts/airdrop-token-vesting/schema/vesting_account_response.jsonis excluded by:!**/*.json
Files selected for processing (3)
- contracts/airdrop-token-vesting/src/contract.rs (2 hunks)
- contracts/airdrop-token-vesting/src/msg.rs (3 hunks)
- contracts/airdrop-token-vesting/src/testing.rs (4 hunks)
Additional comments: 7
contracts/airdrop-token-vesting/src/msg.rs (3)
- 104-107: The addition of new fields
master_address,vesting_denom,vesting_amount, andvesting_scheduleto theVestingDatastruct aligns with the PR objectives to enhance the contract's functionality and maintain consistency in query output formats. However, ensure that all instances whereVestingDatais constructed or modified throughout the codebase are updated to include these new fields.- 124-132: The introduction of the
VestingScheduleQueryOutputenum is a significant enhancement. It provides a structured way to handle legacy queries and ensures that the vesting schedule can be easily converted into a query-friendly output format. This change supports the PR's goal of maintaining consistency in query outputs while improving the contract's flexibility.- 134-152: The
from_vesting_to_query_outputfunction is a crucial addition for convertingVestingScheduleinstances into the newVestingScheduleQueryOutputformat. This function directly supports the PR's objective of enhancing the contract's functionality and maintaining consistency in query output formats. Ensure that this function is used wherever necessary to convert vesting schedules for query outputs.contracts/airdrop-token-vesting/src/contract.rs (1)
- 423-452: The modifications to the
vesting_accountfunction, including the addition ofwhitelistanddenomloading, a newvesting_schedule_querycalculation, and adjustments to theVestingDatastruct fields, are well-aligned with the PR objectives. These changes enhance the contract's data handling capabilities and ensure the return format of vesting entries is consistent with previous outputs. However, ensure comprehensive testing is conducted to verify the correct behavior of these modifications, especially in edge cases and failure scenarios.contracts/airdrop-token-vesting/src/testing.rs (3)
- 7-7: The addition of
VestingScheduleQueryOutputin the import statements indicates the introduction of a new enum or struct to handle vesting schedules in a query-friendly manner. This aligns with the PR objectives to enhance the contract's flexibility and interoperability with different query formats. Ensure that this new type is thoroughly tested and documented to facilitate understanding and maintenance.- 682-697: The modifications to the
VestingDatastructure withinVestingAccountResponseinclude the addition ofmaster_address,vesting_schedule,vesting_denom, and adjustments to related fields likevesting_amount,cliff_amount, andvested_amountfor native token vesting. This change is crucial for aligning the testing framework with the new contract specifications and ensuring that testing accurately reflects the contract's operational logic. It's important to verify that these changes do not introduce any inconsistencies in the data model and that they are compatible with existing contract functionalities. Additionally, ensure that all new fields are properly validated and handled in the contract logic to prevent potential issues.- 798-813: Similar to the previous comment, the modifications to the
VestingDatastructure withinVestingAccountResponsefor theclaim_nativetest case include the addition ofmaster_address,vesting_schedule,vesting_denom, and adjustments to related fields likevesting_amount,cliff_amount,vested_amount, andclaimable_amountfor native token vesting. This ensures that the testing framework is aligned with the new contract specifications. It's essential to verify that these changes are consistent across all test cases and that they accurately reflect the intended contract behavior. Additionally, consider adding more test cases to cover edge cases and ensure comprehensive coverage of the new functionalities introduced in the contract.
|
Building artifacts |
There was a problem hiding this comment.
Review Status
Actionable comments generated: 2
Configuration used: CodeRabbit UI
Files ignored due to path filters (6)
Cargo.lockis excluded by:!**/*.lockcontracts/core-token-vesting/schema/cw20_hook_msg.jsonis excluded by:!**/*.jsoncontracts/core-token-vesting/schema/execute_msg.jsonis excluded by:!**/*.jsoncontracts/core-token-vesting/schema/instantiate_msg.jsonis excluded by:!**/*.jsoncontracts/core-token-vesting/schema/query_msg.jsonis excluded by:!**/*.jsoncontracts/core-token-vesting/schema/vesting_account_response.jsonis excluded by:!**/*.json
Files selected for processing (3)
- .gitignore (1 hunks)
- Cookbook.md (25 hunks)
- contracts/core-cw3-flex-msig/README.md (4 hunks)
Additional comments: 7
.gitignore (1)
- 1-4: The additions to the
.gitignorefile are appropriate and ensure that generated smart contract schema JSON types are ignored by version control. This is a good practice for maintaining a clean repository.contracts/core-cw3-flex-msig/README.md (2)
- 3-17: The documentation provides a comprehensive overview of the CW3 Flexible Multisig smart contract, including its advantages and how it builds upon the cw3-fixed-multisig. The examples and explanations are clear and informative.
- 1-205: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [22-245]
The usage guide, including instantiation, execution, and query examples, is well-detailed and provides clear instructions for interacting with the contract. This will be very helpful for developers looking to implement or interact with the contract.
Cookbook.md (4)
- 3-45: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [6-95]
The introduction of new contracts and their respective sections in the Cookbook provides clear and detailed instructions for instantiation, execution, and querying. This is valuable for developers looking to understand or interact with these contracts.
- 86-114: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [99-163]
The section on "Core token vesting" is well-documented, providing clear examples for executing and querying vesting accounts. It's important to ensure that the examples provided match the actual contract implementation.
- 245-251: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [238-313]
The documentation for "Nibi Stargate Perp" and "Nusd Valuator" contracts is comprehensive, providing detailed examples for various operations. Ensure that the examples are consistent with the contract's code and functionality.
- 405-422: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [343-422]
The "Nusd Valuator" section provides clear instructions for instantiation, execution, and querying. It's important to verify that the examples and functionalities described match the actual contract implementation.
Unique-Divine
added
type: bug/fix
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files ignored due to path filters (10)
artifacts/airdrop_token_vesting.wasmis excluded by:!**/*.wasmartifacts/cw3_flex_multisig.wasmis excluded by:!**/*.wasmartifacts/incentives.wasmis excluded by:!**/*.wasmartifacts/lockup.wasmis excluded by:!**/*.wasmartifacts/nibi_stargate.wasmis excluded by:!**/*.wasmartifacts/nibi_stargate_perp.wasmis excluded by:!**/*.wasmartifacts/nusd_valuator.wasmis excluded by:!**/*.wasmartifacts/pricefeed.wasmis excluded by:!**/*.wasmartifacts/shifter.wasmis excluded by:!**/*.wasmartifacts/token_vesting.wasmis excluded by:!**/*.wasm
Files selected for processing (3)
- artifacts/checksums.txt (1 hunks)
- contracts/airdrop-token-vesting/src/contract.rs (7 hunks)
- nibiru-std/src/proto/buf/nibiru.oracle.v1.rs (14 hunks)
Files skipped from review due to trivial changes (1)
- nibiru-std/src/proto/buf/nibiru.oracle.v1.rs
Additional comments: 6
artifacts/checksums.txt (1)
- 1-11: Checksum updates for various
.wasmfiles are noted. This is a standard procedure following changes in the contract code to ensure that the deployed artifacts are up to date. Ensure that these new checksums are correctly generated from the updated.wasmfiles.contracts/airdrop-token-vesting/src/contract.rs (5)
- 34-37: Ensure that the error messages provided are clear and informative for the end-user. For instance, specifying the type of token expected can make the error message more helpful.
- "must deposit exactly one type of token" + "must deposit exactly one type of token, e.g., 'UST'" - "must deposit some token" + "must deposit a non-zero amount of the token"
- 41-41: Validating that the
managerslist is not empty is crucial for ensuring that there is at least one manager who can perform necessary operations. This is a good practice for maintaining the integrity of the contract's administrative functions.- 130-137: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [119-134]
The withdrawal functionality correctly checks for authorization and ensures that the amount to be withdrawn does not exceed the unallocated amount. Additionally, it properly updates the unallocated amount after withdrawal. This logic is crucial for maintaining the contract's financial integrity.
- 319-319: The
send_if_amount_is_not_zerofunction is a good practice to avoid unnecessary transactions and gas usage. It ensures that tokens are only transferred when there is a non-zero amount to send, optimizing contract performance and cost.- 419-448: The
vesting_accountquery function has been updated to return multiple vesting entries in a vector format, aligning with the PR's objectives to ensure consistency in query output formats. This change enhances the flexibility and usability of the contract's query interface.
Purpose
Align the query entry points between the various vesting and airdrop smart contracts
Summary by CodeRabbit
New Features
master_address,vesting_denom,vesting_amount, and a detailedvesting_schedule.Refactor
VestingDatastruct and related query outputs for improved clarity and functionality.