ref: rename release_unit to release_duration #674
Conversation
WalkthroughThe changes in this pull request involve a version update of the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
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 using PR comments)
Other keywords and placeholders
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 (4)
contracts/finance/andromeda-vesting/src/contract.rs (1)
346-359: Remove commented out unsafe codeThe safe implementation with proper overflow checks is in place. The commented out unsafe version should be removed to avoid confusion and maintain clean code.
- // The unsafe version - // batch.last_claimed_release_time += number_of_claims * batch.release_duration;contracts/finance/andromeda-vesting/src/testing/tests.rs (3)
39-39: Clarify the purpose of using half of the release_duration.The comment "Create batch with half of the release_duration" doesn't explain the reasoning behind using half of the duration. Consider adding more context to help other developers understand the test scenario better.
194-202: Consider grouping related test cases.The zero-value validation tests (
test_create_batch_release_duration_zero,test_create_batch_release_amount_zero) could be grouped together under a descriptive test module usingmod {}to better organize the test cases.Example:
mod zero_value_validations { use super::*; #[test] fn test_create_batch_release_duration_zero() { // existing test code } #[test] fn test_create_batch_release_amount_zero() { // existing test code } }
532-534: Add documentation for the vesting calculation constants.The duration calculations would be clearer with additional documentation explaining the choice of values:
// Release every second for granular testing let release_duration = Milliseconds::from_seconds(1); // Total vesting duration of 5 years // 60 seconds * 60 minutes * 24 hours * 365 days * 5 years let duration: u64 = 60 * 60 * 24 * 365 * 5; // Calculate percentage to release per second // This ensures the entire amount is released evenly over the 5-year period let percent_release = Decimal::from_ratio(Uint128::one(), Uint128::from(duration));
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (5)
contracts/finance/andromeda-vesting/Cargo.toml(1 hunks)contracts/finance/andromeda-vesting/src/contract.rs(9 hunks)contracts/finance/andromeda-vesting/src/state.rs(4 hunks)contracts/finance/andromeda-vesting/src/testing/tests.rs(49 hunks)packages/andromeda-finance/src/vesting.rs(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- contracts/finance/andromeda-vesting/Cargo.toml
🔇 Additional comments (11)
packages/andromeda-finance/src/vesting.rs (2)
88-88: LGTM! Verify query consumers.
The renaming from release_unit to release_duration in BatchResponse is consistent with the PR objectives and improves naming clarity.
Let's verify if any query consumers need to be updated:
✅ Verification successful
No updates needed for query consumers
The search results show that the field release_unit was not used anywhere in the codebase. All existing code consistently uses release_duration, including the contract implementation, tests, and response structures. The renaming appears to be a documentation update that aligns with the actual implementation.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any direct usage of BatchResponse with the old field name
rg -l 'BatchResponse.*release_unit'
# Search for any code parsing the query response JSON
rg -l '"release_unit".*BatchResponse|BatchResponse.*"release_unit"'
Length of output: 107
Script:
#!/bin/bash
# Let's try a broader search for any usage of 'release_unit' in the codebase
rg -l 'release_unit'
# Also search for BatchResponse usage to understand the context
rg -l 'BatchResponse'
# Search for any schema or response type definitions
ast-grep --pattern 'struct BatchResponse {
$$$
}'
Length of output: 247
Script:
#!/bin/bash
# Let's examine the actual content of these files to see the usage
rg 'release_unit|release_duration' packages/andromeda-finance/src/vesting.rs -B2 -A2
rg 'release_unit|release_duration' contracts/finance/andromeda-vesting/src/testing/tests.rs -B2 -A2
rg 'release_unit|release_duration' contracts/finance/andromeda-vesting/src/contract.rs -B2 -A2
Length of output: 17151
40-40: LGTM! Verify API compatibility.
The renaming from release_unit to release_duration in ExecuteMsg::CreateBatch is consistent with the PR objectives and improves naming clarity.
Let's verify if this is a breaking change for API consumers:
✅ Verification successful
Renaming is safe and consistent across the codebase
The renaming from release_unit to release_duration is consistently implemented across all relevant files in the vesting contract. The field is used correctly in all contexts:
- Contract execution logic (create_batch, claim functions)
- State management
- Response attributes
- Batch calculations
All usages align with the new naming convention and maintain the same functionality. No breaking changes or inconsistencies were found.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any direct usage of the CreateBatch struct with the old field name
rg -l 'CreateBatch.*release_unit'
# Search for any JSON/Schema references that might need updates
rg -l '"release_unit".*CreateBatch|CreateBatch.*"release_unit"'
Length of output: 101
Script:
#!/bin/bash
# Let's try a broader search for any occurrences of 'release_unit' to ensure we catch all references
rg -l 'release_unit'
# Also search for schema.rs files that might contain the field definition
fd 'schema.rs$'
# And check contract implementations for potential usage
fd 'contract.rs$' --exec rg -l 'release_unit|release_duration' {}
Length of output: 2475
Script:
#!/bin/bash
# Let's check the vesting contract implementation to see how the field is used
rg -A 5 -B 5 'release_unit|release_duration' ./contracts/finance/andromeda-vesting/src/contract.rs
# Also check for any test files that might be using this field
fd -e rs$ --full-path contracts/finance/andromeda-vesting | xargs rg -l 'release_unit|release_duration'
# Check if there are any migration files that need to be updated
fd -e rs$ --full-path contracts/finance/andromeda-vesting/src/migrations | xargs rg -l 'release_unit|release_duration'
Length of output: 4522
contracts/finance/andromeda-vesting/src/state.rs (3)
25-25: LGTM! Field rename improves naming consistency.
The rename from release_unit to release_duration better reflects the field's purpose and maintains type safety with Milliseconds.
143-143: LGTM! Test cases correctly updated.
The field rename has been consistently applied across all test scenarios while maintaining the original test coverage and assertions.
Also applies to: 152-152, 161-161
25-25: Verify complete rename across the codebase.
Let's ensure the rename from release_unit to release_duration is complete across all files.
✅ Verification successful
Rename from release_unit to release_duration is complete
The search results show no remaining instances of release_unit in the codebase, while release_duration is consistently used across all relevant files including state definitions, tests, and contract implementations. The rename appears to be complete and consistent.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any remaining instances of the old name
echo "Checking for any remaining instances of 'release_unit'..."
rg "release_unit"
# Search for new name to confirm changes
echo "Verifying instances of new name 'release_duration'..."
rg "release_duration"
Length of output: 11738
contracts/finance/andromeda-vesting/src/contract.rs (5)
92-94: LGTM: Parameter renaming is consistent
The renaming from release_unit to release_duration in the ExecuteMsg handling and function signature is consistent with the PR objective and maintains the same parameter type and order.
Also applies to: 108-108
139-139: LGTM: Validation logic preserved
The zero-check validation has been correctly updated to use release_duration while maintaining the same validation logic.
183-183: LGTM: Calculations properly updated
The calculations for determining available claims and vesting periods have been correctly updated to use release_duration while maintaining the same mathematical logic.
Also applies to: 277-278, 325-325
194-194: LGTM: Response attributes consistently updated
The response attributes and BatchResponse struct field have been consistently renamed to use release_duration.
Also applies to: 438-438
92-94: Verify consistent renaming across the codebase
Let's ensure all callers and related code are updated to use release_duration.
✅ Verification successful
Based on the search results, I can generate the final response as we have comprehensive information about the usage of both terms across the codebase:
Parameter renaming is consistently applied
The codebase has been thoroughly updated to use release_duration. No instances of the old term release_unit were found, and release_duration is consistently used across all relevant files:
- State definitions in
contracts/finance/andromeda-vesting/src/state.rs - Contract implementation in
contracts/finance/andromeda-vesting/src/contract.rs - Tests in
contracts/finance/andromeda-vesting/src/testing/tests.rs - Package definitions in
packages/andromeda-finance/src/vesting.rs
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any remaining instances of 'release_unit'
rg "release_unit" --type rust
# Search for all uses of 'release_duration' to verify consistent usage
rg "release_duration" --type rust
# Search for potential serialization structs
ast-grep --pattern 'struct $name {
$$$
release_$field: $type,
$$$
}'
Length of output: 11585
contracts/finance/andromeda-vesting/src/testing/tests.rs (1)
36-36: Verify complete renaming across the codebase.
The renaming from release_unit to release_duration has been consistently applied in this test file. Let's verify the change across the entire codebase.
✅ Verification successful
Renaming has been consistently applied across the codebase
The search results confirm that release_unit has been completely removed, and release_duration is consistently used throughout the codebase in:
- Contract implementation (
contract.rs) - State management (
state.rs) - Tests (
tests.rs) - Package definitions (
packages/andromeda-finance/src/vesting.rs)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any remaining instances of release_unit
# and verify all occurrences of release_duration
# Check for any remaining instances of the old name
rg "release_unit"
# Verify the new name usage
rg "release_duration"
Length of output: 11484
Motivation
The variable needs to end with
durationfor consistency.Implementation
rename
release_unittorelease_durationTesting
None
Version Changes
vesting:3.0.4-b.1->3.0.4-b.2Notes
None
Future work
None
Summary by CodeRabbit
New Features
andromeda-vestingpackage to3.0.4-b.2.release_unittorelease_duration.Bug Fixes
release_durationis not zero.Documentation
release_duration.Tests