ref: Fixed Amount Splitter rename #754
Conversation
|
Warning Rate limit exceeded@joemonem has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 8 minutes and 27 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughThis pull request involves a comprehensive renaming and refactoring of the Set Amount Splitter contract to Fixed Amount Splitter across multiple files and packages. The changes include updating package names, module paths, import statements, contract interfaces, mock implementations, and test cases. The core functionality remains unchanged, with the primary focus being on consistent naming and module organization within the Andromeda finance ecosystem. Changes
Suggested Labels
Poem
Possibly related PRs
Finishing Touches
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
🧹 Nitpick comments (3)
contracts/finance/andromeda-fixed-amount-splitter/src/mock.rs (1)
Line range hint
29-35: Update the app name in instantiation.The app name still uses "Andromeda Splitter" which could be more specific. Consider updating it to "Andromeda Fixed Amount Splitter" for consistency with the new naming convention.
- let res = app.instantiate_contract(code_id, sender, &msg, &[], "Andromeda Splitter", None); + let res = app.instantiate_contract(code_id, sender, &msg, &[], "Andromeda Fixed Amount Splitter", None);tests-integration/tests/fixed_amount_splitter.rs (2)
Line range hint
124-124: Update app name in instantiation.The app name still uses the old naming convention "Set Amount Splitter App".
- "Set Amount Splitter App", + "Fixed Amount Splitter App",
Line range hint
165-197: Enhance test assertions with descriptive messages.The test assertions could be more descriptive to better explain the expected behavior.
assert_eq!( router .wrap() .query_balance(andr.get_wallet("recipient1"), "uandr") .unwrap() .amount, - Uint128::from(100u128) + Uint128::from(100u128), + "Recipient1 should receive exactly 100 uandr from the fixed amount split" );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (17)
contracts/finance/andromeda-fixed-amount-splitter/Cargo.toml(1 hunks)contracts/finance/andromeda-fixed-amount-splitter/examples/schema.rs(1 hunks)contracts/finance/andromeda-fixed-amount-splitter/src/contract.rs(2 hunks)contracts/finance/andromeda-fixed-amount-splitter/src/interface.rs(1 hunks)contracts/finance/andromeda-fixed-amount-splitter/src/lib.rs(1 hunks)contracts/finance/andromeda-fixed-amount-splitter/src/mock.rs(5 hunks)contracts/finance/andromeda-fixed-amount-splitter/src/state.rs(1 hunks)contracts/finance/andromeda-fixed-amount-splitter/src/testing/tests.rs(1 hunks)contracts/finance/andromeda-set-amount-splitter/src/interface.rs(0 hunks)contracts/finance/andromeda-set-amount-splitter/src/state.rs(0 hunks)packages/andromeda-finance/src/lib.rs(1 hunks)packages/deploy/Cargo.toml(1 hunks)packages/deploy/src/contracts.rs(2 hunks)tests-integration/Cargo.toml(2 hunks)tests-integration/tests/fixed_amount_splitter.rs(8 hunks)tests-integration/tests/mod.rs(1 hunks)tests-integration/tests/splitter.rs(4 hunks)
💤 Files with no reviewable changes (2)
- contracts/finance/andromeda-set-amount-splitter/src/interface.rs
- contracts/finance/andromeda-set-amount-splitter/src/state.rs
✅ Files skipped from review due to trivial changes (5)
- tests-integration/tests/mod.rs
- contracts/finance/andromeda-fixed-amount-splitter/Cargo.toml
- contracts/finance/andromeda-fixed-amount-splitter/src/testing/tests.rs
- contracts/finance/andromeda-fixed-amount-splitter/src/contract.rs
- tests-integration/tests/splitter.rs
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: Contract Tests (ubuntu-latest, unit-test)
- GitHub Check: Contract Tests (ubuntu-latest, integration-test)
- GitHub Check: Schemas
- GitHub Check: Contracts
- GitHub Check: Summary
🔇 Additional comments (10)
contracts/finance/andromeda-fixed-amount-splitter/src/state.rs (1)
1-4: LGTM! Clean implementation of the state storage.The storage implementation correctly uses the new module path and follows the naming convention.
packages/andromeda-finance/src/lib.rs (1)
3-3: LGTM! Module declaration follows the package structure.The new module is correctly placed and maintains the alphabetical ordering.
contracts/finance/andromeda-fixed-amount-splitter/examples/schema.rs (1)
1-1: Verify schema generation with new imports.The import path has been correctly updated. Please ensure the schema generation produces the expected output.
Run this script to verify the schema generation:
✅ Verification successful
Schema generation code is correctly structured
The schema.rs file is properly configured with:
- Correct import path from andromeda-finance workspace dependency
- Standard schema generation using write_api! macro
- All necessary message types (InstantiateMsg, QueryMsg, ExecuteMsg)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify schema generation and check for any breaking changes # Find and check schema files fd -e json -e schema 'schema' contracts/finance/andromeda-fixed-amount-splitter/schema/ \ --exec sh -c 'echo "Checking {}" && cat {} | jq -r ".[\"title\"] // .title"'Length of output: 309
Script:
#!/bin/bash # First, let's understand the project structure echo "Project structure:" fd . contracts/finance/andromeda-fixed-amount-splitter/ -t d echo -e "\nLooking for schema files:" fd schema contracts/finance/andromeda-fixed-amount-splitter/ -t f echo -e "\nChecking examples directory content:" ls -la contracts/finance/andromeda-fixed-amount-splitter/examples/Length of output: 816
Script:
#!/bin/bash echo "Content of schema.rs:" cat contracts/finance/andromeda-fixed-amount-splitter/examples/schema.rs echo -e "\nVerifying imports exist in source:" rg "pub (struct|enum) (ExecuteMsg|InstantiateMsg|QueryMsg)" contracts/finance/andromeda-fixed-amount-splitter/src/Length of output: 577
Script:
#!/bin/bash # Check Cargo.toml for dependencies and workspace structure echo "Checking Cargo.toml:" cat contracts/finance/andromeda-fixed-amount-splitter/Cargo.toml echo -e "\nChecking for message type definitions:" rg -g '!target/' -g '!examples/' "mod fixed_amount_splitter" contracts/finance/Length of output: 1219
contracts/finance/andromeda-fixed-amount-splitter/src/lib.rs (1)
13-13: Verify the interface usage across the codebase.The interface export has been correctly renamed. Let's verify its usage to ensure no references to the old interface remain.
Run this script to check for any remaining references:
✅ Verification successful
Interface rename verification completed successfully
The interface has been properly renamed across the codebase. The only reference to the old name appears in the CHANGELOG.md, which is expected as it documents historical changes.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for any remaining references to SetAmountSplitter # Search for old naming references echo "Checking for old interface references..." rg -l "SetAmount(Splitter|Contract)" echo "Checking for potential missed renames..." rg -l "set.?amount.?splitter" -iLength of output: 353
Script:
#!/bin/bash # Check content of matched files echo "=== Checking test file content ===" rg "SetAmount(Splitter|Contract)" tests-integration/tests/splitter.rs tests-integration/tests/fixed_amount_splitter.rs -C 2 echo -e "\n=== Checking CHANGELOG.md content ===" rg "set.?amount.?splitter" CHANGELOG.md -i -C 2Length of output: 871
contracts/finance/andromeda-fixed-amount-splitter/src/interface.rs (1)
1-10: LGTM! Clean interface implementation.The contract interface is well-defined with proper imports and a clear contract ID that matches the new naming convention.
packages/deploy/src/contracts.rs (1)
74-74: LGTM! Proper contract deployment update.The
FixedAmountSplitterContractis correctly integrated into the deployment list, maintaining the existing deployment order.tests-integration/tests/fixed_amount_splitter.rs (1)
Line range hint
165-228: Consider adding more test scenarios.The current test coverage focuses on basic scenarios. Consider adding tests for:
- Edge cases with zero amounts
- Cases with invalid recipient configurations
- Scenarios with multiple consecutive splits
Let me help you identify any existing test coverage for these scenarios:
packages/deploy/Cargo.toml (1)
52-52: LGTM! Verify dependency update.The dependency rename from
andromeda-set-amount-splittertoandromeda-fixed-amount-splitteraligns with the PR objectives.Let's verify the dependency path exists and the package name is updated in the target Cargo.toml:
✅ Verification successful
Dependency update verified successfully
The package name and path have been correctly updated to
andromeda-fixed-amount-splitterin both the deployment configuration and the target package.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the renamed package exists and is correctly updated # Test: Check if the package name is updated in the target Cargo.toml cat contracts/finance/andromeda-fixed-amount-splitter/Cargo.toml | grep -A 2 '^\[package\]'Length of output: 166
tests-integration/Cargo.toml (2)
71-73: LGTM! Verify testing features.The dependency rename maintains consistency with the changes in deploy/Cargo.toml and preserves the testing features.
Let's verify the testing features are properly defined in the target Cargo.toml:
✅ Verification successful
Testing features are properly defined ✅
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify testing features are properly defined # Test: Check if testing features are defined in the target Cargo.toml cat contracts/finance/andromeda-fixed-amount-splitter/Cargo.toml | grep -A 5 '^\[features\]'Length of output: 339
180-180: LGTM! Verify test file rename.The test entry rename aligns with the new package name.
Let's verify the test file is also renamed:
✅ Verification successful
Test file rename verified successfully
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify test file rename # Test: Check if the test file is renamed from set_amount_splitter.rs to fixed_amount_splitter.rs ls -la tests-integration/tests/fixed_amount_splitter.rsLength of output: 155
Script:
#!/bin/bash # Check if the old file name still exists ls -la tests-integration/tests/set_amount_splitter.rsLength of output: 149
Motivation
Rename the Set Amount Splitter to Fixed Amount Splitter.
Fixed its schema.
Implementation
Standard rename across the project.
Schema was using the regular splitter's messages instead of the Fixed Amount splitter.
Testing
None
Version Changes
None
Notes
None
Future work
None
Summary by CodeRabbit
Refactor
Documentation
Chores