feat(treasury): add swap functionalities through osmosis #126
Conversation
WalkthroughThe recent updates involve adding new dependencies, updating existing ones, and incorporating new functionalities across various files. Key changes include transitioning to Changes
Sequence Diagram(s) (Beta)sequenceDiagram
participant User
participant StakingContract
participant Treasury
User->>StakingContract: Instantiate with `send_fees_to_treasury`
StakingContract->>StakingContract: Initialize configuration
StakingContract->>Treasury: Send fees (if `send_fees_to_treasury` is true)
User->>StakingContract: Execute transaction
StakingContract->>StakingContract: Process transaction
StakingContract->>Treasury: Send fees (if `send_fees_to_treasury` is true)
User->>StakingContract: Query data
StakingContract->>User: Return data
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Outside diff range and nitpick comments (2)
README.md (2)
Line range hint
34-34: Specify the language for fenced code blocks to enhance readability.- ```rust + ```bash
Line range hint
38-38: Specify the language for fenced code blocks to enhance readability.- ``` + ```bash
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 (19)
- Cargo.toml (2 hunks)
- README.md (1 hunks)
- contracts/staking/Cargo.toml (1 hunks)
- contracts/treasury/Cargo.toml (3 hunks)
- contracts/treasury/README.md (1 hunks)
- contracts/treasury/src/contract.rs (4 hunks)
- contracts/treasury/src/error.rs (2 hunks)
- contracts/treasury/src/execute.rs (4 hunks)
- contracts/treasury/src/helpers.rs (1 hunks)
- contracts/treasury/src/lib.rs (1 hunks)
- contracts/treasury/src/msg.rs (2 hunks)
- contracts/treasury/src/query.rs (1 hunks)
- contracts/treasury/src/state.rs (2 hunks)
- contracts/treasury/src/tests/mod.rs (1 hunks)
- contracts/treasury/src/tests/ownership_tests.rs (1 hunks)
- contracts/treasury/src/tests/send_tests.rs (1 hunks)
- contracts/treasury/src/tests/swap_tests.rs (1 hunks)
- contracts/treasury/src/tests/test_helper.rs (1 hunks)
- contracts/treasury/src/tests/update_config_test.rs (1 hunks)
Files skipped from review due to trivial changes (5)
- contracts/treasury/Cargo.toml
- contracts/treasury/src/lib.rs
- contracts/treasury/src/tests/mod.rs
- contracts/treasury/src/tests/ownership_tests.rs
- contracts/treasury/src/tests/send_tests.rs
Additional Context Used
Markdownlint (9)
README.md (2)
34: null
Fenced code blocks should have a language specified
38: null
Fenced code blocks should have a language specifiedcontracts/treasury/README.md (7)
40: null
Lists should be surrounded by blank lines
60: null
Lists should be surrounded by blank lines
69: null
Lists should be surrounded by blank lines
84: null
Lists should be surrounded by blank lines
100: null
Lists should be surrounded by blank lines
116: null
Lists should be surrounded by blank lines
133: null
Lists should be surrounded by blank lines
Additional comments not posted (22)
Cargo.toml (1)
17-26: Ensure all new dependencies are compatible with the project's existing ecosystem.contracts/staking/Cargo.toml (1)
25-41: All dependencies have been updated to use workspace versions. This is a good practice to ensure consistency across the project.contracts/treasury/src/tests/update_config_test.rs (2)
10-28: The testupdate_config_not_admin_failcorrectly checks that only the admin can update the configuration. This is crucial for maintaining secure control over contract settings.
30-50: The testupdate_configeffectively verifies the functionality of updating the trader and allowed swap routes. It's good to see comprehensive checks on the state after the operation.contracts/treasury/src/tests/test_helper.rs (1)
10-45: The initialization function is well-structured and sets up the contract with a comprehensive set of parameters. This setup is crucial for ensuring that subsequent tests run under realistic conditions.contracts/treasury/src/state.rs (2)
19-27: TheSwapRouteandConfigstructs are well-defined with appropriate documentation. The use of vectors forallowed_swap_routesinConfigprovides flexibility in defining complex swap paths.Also applies to: 29-35
39-63: The methodsassert_traderandassert_allowed_swap_routeare essential for enforcing contract rules and preventing unauthorized actions. The error handling is robust, ensuring that only valid operations proceed.contracts/treasury/src/contract.rs (3)
Line range hint
27-63: Theinstantiatefunction is well-implemented, setting up the contract state and configuration correctly. The use of optional fields with defaults ensures flexibility in contract deployment.
86-99: The execution functions for swapping and updating configurations are correctly implemented. It's good to see detailed handling of different scenarios and input validations.
104-137: Thequeryandmigratefunctions are correctly implemented. The migration logic includes version checks to prevent incorrect upgrades, which is a critical safety feature.contracts/treasury/README.md (1)
3-172: The README is comprehensive and provides clear instructions for all operations related to the treasury contract. It's important that the documentation stays up-to-date with the contract's capabilities.contracts/treasury/src/execute.rs (5)
107-113: Consider validating the address format before constructing the message.This ensures that the address format is correct before proceeding with the transaction, potentially preventing failed transactions due to address format errors.
Line range hint
117-128: Ensure proper error handling for the IBC transfer scenario.The comment mentions not using the IBC queue here and just reexecuting if this fails. It's crucial to handle potential failures in IBC transfers robustly, possibly by implementing a retry mechanism or logging the failure for manual intervention.
150-194: Ensure that the swap functionality adheres to the specified swap routes and token denominations.This function performs a swap with a specified minimum output amount. It's important to verify that the swap routes and token denominations are correctly validated against the allowed configurations to prevent unauthorized swaps.
196-240: Review the logic for executing swaps with an exact amount out.Similar to the previous function, this one needs thorough validation of swap routes and token denominations. Additionally, ensure that the logic for handling the maximum input amount is correctly implemented to prevent overspending.
242-266: Check the update configuration functionality for proper authorization and validation.This function updates the trading configuration. It's crucial to ensure that only authorized users can update the configuration and that the input data is properly validated.
contracts/treasury/src/tests/swap_tests.rs (6)
20-48: Add more detailed assertions to the test for unauthorized swaps.While the test checks for unauthorized access, it could be enhanced by adding more detailed assertions to verify that no state changes occur and no messages are sent when the swap is unauthorized.
50-61: Ensure comprehensive coverage for scenarios with empty swap routes.This test checks the behavior when the swap routes are empty. Consider adding assertions to verify that no state changes or unintended actions occur in this scenario.
63-101: Validate the handling of unauthorized pools in swap operations.This test covers scenarios where swaps are attempted with unauthorized pools. Ensure that the test adequately checks for all possible unauthorized access points and correctly handles them.
103-156: Check the validation logic for token denominations in swap operations.This series of tests checks for mismatches in token denominations. Ensure that the tests comprehensively cover all edge cases and correctly assert the expected errors.
158-194: Review the tests for unauthorized token output denominations.These tests should verify that the swap operation does not proceed with unauthorized output denominations. Ensure that the tests are comprehensive and correctly handle all scenarios.
196-543: Confirm that successful swap operations are correctly tested.These tests verify successful swap operations. Ensure that they correctly simulate the swap process, check for the correct state changes, and validate that the expected messages are sent.
There was a problem hiding this comment.
Actionable comments posted: 11
Outside diff range and nitpick comments (4)
contracts/staking/src/migrations/v0_4_19.rs (1)
Line range hint
41-61: Proper integration ofsend_fees_to_treasuryin the migration logic.Would you like me to help create a unit test to verify the migration logic?
contracts/staking/src/helpers.rs (1)
107-107: Clarify the purpose of allowing type complexity inpaginate_map.Consider adding a comment explaining why
#[allow(clippy::type_complexity)]is necessary here, for future maintainers.contracts/staking/src/contract.rs (1)
[!TIP]
Codebase VerificationThe constants
CELESTIA_ACCOUNT_PREFIXandOSMOSIS_ACCOUNT_PREFIXare defined but not used anywhere in the codebase. Please review if these constants are intended to be used or if they can be removed.
CELESTIA_ACCOUNT_PREFIXdefined atcontracts/staking/src/contract.rs:39OSMOSIS_ACCOUNT_PREFIXdefined atcontracts/staking/src/contract.rs:40Analysis chain
Line range hint
27-44: Ensure the constants are correctly defined and used throughout the contract.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the correct definition and usage of constants in the contract. # Test: Search for the constants usage in the contract. Expect: Correct definitions and consistent usage. rg --type rust $'CELESTIA_ACCOUNT_PREFIX|OSMOSIS_ACCOUNT_PREFIX|CELESTIA_VALIDATOR_PREFIX' contracts/staking/src/contract.rsLength of output: 377
Script:
#!/bin/bash # Description: Verify the usage of CELESTIA_ACCOUNT_PREFIX and OSMOSIS_ACCOUNT_PREFIX in the codebase. # Test: Search for the constants usage in the entire codebase. Expect: Correct definitions and consistent usage. rg --type rust $'CELESTIA_ACCOUNT_PREFIX|OSMOSIS_ACCOUNT_PREFIX'Length of output: 236
contracts/staking/src/execute.rs (1)
57-57: Update the memo format to include more context if needed.Consider including additional context or identifiers in the memo field to enhance traceability and debugging.
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 (25)
- contracts/staking/Cargo.toml (1 hunks)
- contracts/staking/src/ack.rs (2 hunks)
- contracts/staking/src/contract.rs (9 hunks)
- contracts/staking/src/execute.rs (7 hunks)
- contracts/staking/src/helpers.rs (7 hunks)
- contracts/staking/src/migrations/v0_4_19.rs (2 hunks)
- contracts/staking/src/msg.rs (4 hunks)
- contracts/staking/src/query.rs (3 hunks)
- contracts/staking/src/state.rs (1 hunks)
- contracts/staking/src/tests/circuit_breaker_tests.rs (1 hunks)
- contracts/staking/src/tests/ibc_transfer_tests.rs (1 hunks)
- contracts/staking/src/tests/instantiate_tests.rs (6 hunks)
- contracts/staking/src/tests/ownership_tests.rs (1 hunks)
- contracts/staking/src/tests/query_tests.rs (1 hunks)
- contracts/staking/src/tests/reward_tests.rs (1 hunks)
- contracts/staking/src/tests/stake_tests.rs (4 hunks)
- contracts/staking/src/tests/submit_batch_tests.rs (1 hunks)
- contracts/staking/src/tests/test_helper.rs (1 hunks)
- contracts/staking/src/tests/unstake_tests.rs (1 hunks)
- contracts/staking/src/tests/validator_tests.rs (1 hunks)
- contracts/staking/src/tests/withdraw_tests.rs (1 hunks)
- contracts/treasury/src/error.rs (2 hunks)
- contracts/treasury/src/helpers.rs (1 hunks)
- contracts/treasury/src/msg.rs (2 hunks)
- contracts/treasury/src/query.rs (1 hunks)
Files skipped from review due to trivial changes (5)
- contracts/staking/src/tests/circuit_breaker_tests.rs
- contracts/staking/src/tests/ownership_tests.rs
- contracts/staking/src/tests/submit_batch_tests.rs
- contracts/staking/src/tests/test_helper.rs
- contracts/staking/src/tests/validator_tests.rs
Files skipped from review as they are similar to previous changes (5)
- contracts/staking/Cargo.toml
- contracts/treasury/src/error.rs
- contracts/treasury/src/helpers.rs
- contracts/treasury/src/msg.rs
- contracts/treasury/src/query.rs
Additional comments not posted (30)
contracts/staking/src/ack.rs (1)
14-14: Change fromto_binarytoto_json_binaryaligns with modern web standards.contracts/staking/src/state.rs (1)
22-24: Clear documentation and appropriate integration ofsend_fees_to_treasuryin theConfigstruct.Verification successful
The
send_fees_to_treasuryfield is consistently integrated and used across the codebase, including inquery.rs,msg.rs,execute.rs,contract.rs, and various test files. This indicates proper handling and integration of the new field.
contracts/staking/src/query.rs: Properly includessend_fees_to_treasury.contracts/staking/src/msg.rs: Defines and usessend_fees_to_treasury.contracts/staking/src/execute.rs: Handlessend_fees_to_treasuryin execution logic.contracts/staking/src/contract.rs: Integratessend_fees_to_treasuryin contract logic.- Test files: Include tests for
send_fees_to_treasury.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify usage of `send_fees_to_treasury` across the system. # Test: Search for usage of `send_fees_to_treasury`. Expect: Proper handling in business logic. rg --type rust $'send_fees_to_treasury'Length of output: 2410
contracts/staking/src/tests/reward_tests.rs (1)
101-169: Effective testing of thereceive_rewards_and_send_fees_to_treasuryfunction, ensuring correct interaction with the treasury.Verification successful
The extracted code from
execute.rsconfirms that thetreasury_addressandsend_fees_to_treasuryconfigurations are correctly handled. The logic ensures that ifsend_fees_to_treasuryis set to true, the fees are transferred to the treasury address.Key points:
- The
treasury_addressis validated and set in the configuration.- The
send_fees_to_treasuryflag is checked, and if true, aBankMsg::Sendmessage is created to transfer the fees to the treasury address.This confirms that the integration with the treasury contract is correctly implemented.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify integration of `receive_rewards_and_send_fees_to_treasury` with the treasury contract. # Test: Search for usage of `receive_rewards_and_send_fees_to_treasury`. Expect: Proper handling of fees transfer. rg --type rust $'receive_rewards_and_send_fees_to_treasury'Length of output: 151
Script:
#!/bin/bash # Description: Verify integration with the treasury contract by searching for references to the treasury address and related configurations. # Search for references to the treasury address in the codebase. rg --type rust 'treasury_address' # Search for references to the `send_fees_to_treasury` configuration in the codebase. rg --type rust 'send_fees_to_treasury'Length of output: 4851
Script:
#!/bin/bash # Description: Extract relevant code from `execute.rs` to verify handling of `treasury_address` and `send_fees_to_treasury`. # Extract functions from `execute.rs` that handle `treasury_address` and `send_fees_to_treasury`. rg --type rust --context 10 'treasury_address|send_fees_to_treasury' contracts/staking/src/execute.rsLength of output: 4130
contracts/staking/src/msg.rs (1)
36-37: Proper addition ofsend_fees_to_treasurytoInstantiateMsgfor initial configuration.Verification successful
The integration of
send_fees_to_treasuryin the instantiation logic and tests appears to be properly handled. Here are the relevant locations:
Instantiation Logic:
contracts/staking/src/contract.rs:
- Line:
send_fees_to_treasury: msg.send_fees_to_treasury- Line:
Some(msg.send_fees_to_treasury)- Line:
send_fees_to_treasury- Line:
send_fees_to_treasury- Line:
send_fees_to_treasury- Line:
migrations::v0_4_19::migrate(deps.branch(), env, send_fees_to_treasury)?Tests:
contracts/staking/src/tests/test_helper.rs:
- Line:
send_fees_to_treasury: truecontracts/staking/src/tests/reward_tests.rs:
- Line:
config.send_fees_to_treasury = false- Line:
receive_rewards_and_send_fees_to_treasury- Line:
config.send_fees_to_treasury = truecontracts/staking/src/tests/instantiate_tests.rs:
- Line:
send_fees_to_treasury: false- Line:
send_fees_to_treasury: None- Line:
send_fees_to_treasury: None- Line:
send_fees_to_treasury: None- Line:
send_fees_to_treasury: NoneThe changes ensure that the
send_fees_to_treasuryfield is correctly integrated into the contract's instantiation logic and adequately tested.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify integration of `send_fees_to_treasury` in contract instantiation logic. # Test: Search for instantiation calls. Expect: Proper handling of the new field. rg --type rust $'InstantiateMsg'Length of output: 1384
Script:
#!/bin/bash # Description: Verify handling of `send_fees_to_treasury` in instantiation logic and tests. # Test: Search for `send_fees_to_treasury` in contract instantiation logic. rg --type rust 'send_fees_to_treasury' contracts/staking/src/contract.rs # Test: Search for `send_fees_to_treasury` in instantiation tests. rg --type rust 'send_fees_to_treasury' contracts/staking/src/testsLength of output: 1229
contracts/staking/src/helpers.rs (1)
163-166: Ensure accuracy in financial calculations inget_rates.Verify that the ratio calculations in
get_ratesfunction handle edge cases correctly, such as division by zero or extremely large values.contracts/staking/src/tests/instantiate_tests.rs (2)
49-49: Validate the addition ofsend_fees_to_treasuryinInstantiateMsg.The addition of
send_fees_to_treasurytoInstantiateMsgaligns with the PR's objective to allow the staking contract to send fees to the treasury.
160-160: Ensure consistency in handlingsend_fees_to_treasuryacross different configurations.Verify that the handling of
send_fees_to_treasuryis consistent and correctly implemented across various configurations in theupdate_configtests.Also applies to: 200-200, 226-226
Verification successful
The gathered context shows that
send_fees_to_treasuryis used in various configurations across different test files. The values assigned tosend_fees_to_treasuryincludetrue,false, andNone, indicating different scenarios being tested.Observations:
- test_helper.rs:
send_fees_to_treasuryis set totrue.- instantiate_tests.rs:
send_fees_to_treasuryis set tofalseandNonein different instances.- reward_tests.rs:
send_fees_to_treasuryis toggled betweentrueandfalseto test different behaviors.Analysis:
The usage of
send_fees_to_treasuryappears to be consistent with the intention of testing various configurations. Each test case sets this parameter to different values to verify the behavior of the system under those conditions.Conclusion:
The handling of
send_fees_to_treasuryis consistent and correctly implemented across the different configurations in theupdate_configtests.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify consistent handling of `send_fees_to_treasury`. # Test: Search for all occurrences of `send_fees_to_treasury` in test cases. # Expect: Consistent and correct implementation. rg --type rust "send_fees_to_treasury" contracts/staking/src/tests/Length of output: 866
Script:
#!/bin/bash # Extract and display the relevant lines and surrounding context for `send_fees_to_treasury` in the identified files. # Display lines around occurrences in test_helper.rs rg --type rust --context 5 "send_fees_to_treasury" contracts/staking/src/tests/test_helper.rs # Display lines around occurrences in instantiate_tests.rs rg --type rust --context 5 "send_fees_to_treasury" contracts/staking/src/tests/instantiate_tests.rs # Display lines around occurrences in reward_tests.rs rg --type rust --context 5 "send_fees_to_treasury" contracts/staking/src/tests/reward_tests.rsLength of output: 3778
contracts/staking/src/tests/withdraw_tests.rs (1)
1-10: Ensure correct imports and setup for withdrawal tests.The setup and imports for the withdrawal tests are correctly configured, providing the necessary context and functionality for the tests.
contracts/staking/src/tests/query_tests.rs (1)
1-12: Check for necessary imports and setup in query tests.The setup and imports for the query tests are correctly configured, ensuring that all dependencies are properly declared.
contracts/staking/src/tests/stake_tests.rs (3)
Line range hint
56-67: Ensure theMsgTransfermessage construction is correct and aligns with the intended functionality.Verification successful
The
MsgTransferconstruction in theproper_liquid_stakefunction appears to be correctly structured with the appropriate parameters. TheMsgTransfermessage includes thesource_channel,source_port,sender,receiver,token,timeout_height,timeout_timestamp, andmemofields, which align with the expected structure.
source_channel:CHANNEL_ID.to_string()source_port:"transfer".to_string()sender:env.contract.address.to_string()receiver:Addr::unchecked(CELESTIA1).to_string()token:Some(ibc_coin)timeout_height:Nonetimeout_timestamp:timeout.timestamp().unwrap().nanos()memo:format!("{{\"ibc_callback\":\"{}\"}}", env.contract.address)These parameters are consistent with the intended functionality of the
MsgTransfermessage.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the correct construction of `MsgTransfer` in the `proper_liquid_stake` test function. # Test: Search for the `MsgTransfer` usage in the `proper_liquid_stake` function. Expect: Correct parameters and structure. rg --type rust $'MsgTransfer' contracts/staking/src/tests/stake_tests.rsLength of output: 227
Script:
#!/bin/bash # Extract the full context of `MsgTransfer` usage in the `proper_liquid_stake` function. # This will help verify the parameters and structure. # Search for the `proper_liquid_stake` function definition and extract its body. ast-grep --lang rust --pattern $'fn proper_liquid_stake($_) { $$$ }' contracts/staking/src/tests/stake_tests.rsLength of output: 114
Script:
#!/bin/bash # Extract the full context of `MsgTransfer` usage in the `proper_liquid_stake` function. # This will help verify the parameters and structure. # Search for the `proper_liquid_stake` function definition and extract its body. rg 'fn proper_liquid_stake' -A 50 contracts/staking/src/tests/stake_tests.rsLength of output: 2283
206-206: Verify the correct usage ofderive_intermediate_senderfunction.Verification successful
The
derive_intermediate_senderfunction is defined as follows:pub fn derive_intermediate_sender( channel_id: &str, original_sender: &str, bech32_prefix: &str, ) -> Result<String, bech32::Error> { use bech32::ToBase32; let sender_str = format!("{channel_id}/{original_sender}"); let sender_hash_32 = addess_hash(SENDER_PREFIX, sender_str.as_bytes()); let sender = sender_hash_32.to_base32(); bech32::encode(bech32_prefix, sender, bech32::Variant::Bech32) }The usage of
derive_intermediate_senderin theproper_ibc_liquid_staketest function:derive_intermediate_sender(CHANNEL_ID, CELESTIA1, "osmo").unwrap();is correct as it matches the function's parameters:
channel_id,original_sender, andbech32_prefix.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the correct usage of `derive_intermediate_sender` function in the `proper_ibc_liquid_stake` test function. # Test: Search for the `derive_intermediate_sender` usage in the `proper_ibc_liquid_stake` function. Expect: Correct parameters and structure. rg --type rust $'derive_intermediate_sender' contracts/staking/src/tests/stake_tests.rsLength of output: 280
Script:
#!/bin/bash # Description: Locate the definition of the `derive_intermediate_sender` function to verify its correct usage. # Search for the definition of `derive_intermediate_sender` in the codebase. ast-grep --lang rust --pattern 'fn derive_intermediate_sender($_, $_, $_) { $$$ }'Length of output: 84
Script:
#!/bin/bash # Description: Locate the definition of the `derive_intermediate_sender` function to verify its correct usage. # Search for the definition of `derive_intermediate_sender` in the codebase. rg --type rust 'fn derive_intermediate_sender'Length of output: 116
Script:
#!/bin/bash # Description: Display the definition of the `derive_intermediate_sender` function to verify its correct usage. # Display the definition of `derive_intermediate_sender` in `contracts/staking/src/helpers.rs`. rg --type rust -A 10 'fn derive_intermediate_sender' contracts/staking/src/helpers.rsLength of output: 513
231-233: Ensure thederive_intermediate_senderfunction is correctly handling thereward_collector_addressin thereceive_rewards_before_mintingtest function.Verification successful
The
receive_rewards_before_mintingfunction is present in thestake_tests.rsfile and includes the handling ofreward_collector_address. The function usesreward_collector_addressas part of thederive_intermediate_senderfunction call.
- Location:
contracts/staking/src/tests/stake_tests.rs, lines 231-233The handling of
reward_collector_addressappears to be correctly integrated within thereceive_rewards_before_mintingfunction.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the correct handling of `reward_collector_address` in the `receive_rewards_before_minting` test function. # Test: Search for the `reward_collector_address` usage in the `receive_rewards_before_minting` function. Expect: Correct handling and structure. rg --type rust $'reward_collector_address' contracts/staking/src/tests/stake_tests.rsLength of output: 136
Script:
#!/bin/bash # Locate the `receive_rewards_before_minting` function and inspect its implementation. # Search for the definition of the `receive_rewards_before_minting` function in the `stake_tests.rs` file. ast-grep --lang rust --pattern 'fn receive_rewards_before_minting($_) { $$$ }' contracts/staking/src/tests/stake_tests.rsLength of output: 123
Script:
#!/bin/bash # List all function definitions in the `stake_tests.rs` file to identify the correct function. # Search for all function definitions in the `stake_tests.rs` file. ast-grep --lang rust --pattern 'fn $_($_) { $$$ }' contracts/staking/src/tests/stake_tests.rsLength of output: 95
Script:
#!/bin/bash # Verify the existence of the `stake_tests.rs` file and list its content. # Check if the `stake_tests.rs` file exists and list its content. if [ -f contracts/staking/src/tests/stake_tests.rs ]; then cat contracts/staking/src/tests/stake_tests.rs else echo "File not found: contracts/staking/src/tests/stake_tests.rs" fiLength of output: 11846
contracts/staking/src/contract.rs (4)
324-326: Ensure the migration logic inMigrateMsg::V0_4_18ToV0_4_19correctly handles thesend_fees_to_treasuryparameter.Verification successful
The
send_fees_to_treasuryparameter is correctly handled in the migration logic. It is passed to themigratefunction and used to set thesend_fees_to_treasuryfield in the new configuration, which is then saved.
contracts/staking/src/migrations/v0_4_19.rs:61: Thesend_fees_to_treasuryparameter is assigned to the corresponding field in the new configuration.contracts/staking/src/migrations/v0_4_19.rs:64: The new configuration, including thesend_fees_to_treasuryfield, is saved.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the correct handling of `send_fees_to_treasury` in the migration logic. # Test: Search for the `send_fees_to_treasury` usage in the migration logic. Expect: Correct handling and structure. rg --type rust $'send_fees_to_treasury' contracts/staking/src/contract.rsLength of output: 363
Script:
#!/bin/bash # Description: Verify the correct handling of `send_fees_to_treasury` in the migration logic. # Test: Search for the `send_fees_to_treasury` usage in the migration logic file. rg --type rust 'send_fees_to_treasury' $(fd migrate | grep 'migrations/v0_4_19')Length of output: 2451
Script:
#!/bin/bash # Description: Extract the `migrate` function implementation to verify the handling of `send_fees_to_treasury`. # Test: Extract the `migrate` function from the migration logic file. ast-grep --lang rust --pattern 'pub fn migrate($_, $_, $_) -> $_ { $$$ }' contracts/staking/src/migrations/v0_4_19.rsLength of output: 2917
Line range hint
98-117: Verify the correct handling ofsend_fees_to_treasuryin theinstantiatefunction.Verification successful
The
instantiateandmigratefunctions have been identified, but the extracted code does not provide enough context to verify the handling ofsend_fees_to_treasury. We need to extract more lines from these functions to ensure we capture the relevant logic.
The extracted code from the
instantiateandmigratefunctions shows thatsend_fees_to_treasuryis correctly handled in both functions. In theinstantiatefunction, it is assigned to theconfigstruct, and in themigratefunction, it is passed to the migration logic.
- In the
instantiatefunction,send_fees_to_treasuryis set as part of theConfigstruct.- In the
migratefunction,send_fees_to_treasuryis passed to themigrations::v0_4_19::migratefunction.This confirms that
send_fees_to_treasuryis correctly handled in theinstantiatefunction and related migration logic.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the correct handling of `send_fees_to_treasury` in the `instantiate` function. # Test: Search for the `send_fees_to_treasury` usage in the `instantiate` function. Expect: Correct handling and structure. rg --type rust $'send_fees_to_treasury' contracts/staking/src/contract.rsLength of output: 363
Script:
#!/bin/bash # Extract the `instantiate` function and related context to verify the handling of `send_fees_to_treasury`. # Extract the `instantiate` function ast-grep --lang rust --pattern 'fn instantiate($_, $_, $_) -> $_ { $$$ }' contracts/staking/src/contract.rs # Extract the migration function related to `send_fees_to_treasury` ast-grep --lang rust --pattern 'fn migrate($_, $_, $_) -> $_ { $$$ }' contracts/staking/src/contract.rsLength of output: 219
Script:
#!/bin/bash # Adjusted patterns to extract the `instantiate` function and related context to verify the handling of `send_fees_to_treasury`. # Extract the `instantiate` function with a more flexible pattern ast-grep --lang rust --pattern 'fn instantiate($_) -> $_ { $$$ }' contracts/staking/src/contract.rs # Extract the migration function related to `send_fees_to_treasury` with a more flexible pattern ast-grep --lang rust --pattern 'fn migrate($_) -> $_ { $$$ }' contracts/staking/src/contract.rsLength of output: 203
Script:
#!/bin/bash # Use `rg` to search for the `instantiate` and migration function definitions and extract the relevant code. # Search for the `instantiate` function definition rg --type rust 'fn instantiate' contracts/staking/src/contract.rs -A 20 # Search for the migration function definition related to `send_fees_to_treasury` rg --type rust 'fn migrate' contracts/staking/src/contract.rs -A 20Length of output: 1787
Script:
#!/bin/bash # Extract more lines from the `instantiate` and `migrate` functions to verify the handling of `send_fees_to_treasury`. # Extract more lines from the `instantiate` function rg --type rust 'fn instantiate' contracts/staking/src/contract.rs -A 50 # Extract more lines from the migration function related to `send_fees_to_treasury` rg --type rust 'fn migrate' contracts/staking/src/contract.rs -A 50Length of output: 3728
264-290: Verify the correct implementation of query functions and their alignment with the contract's functionality.Verification successful
The implementations of the query functions have been successfully extracted and reviewed. They appear to be correctly implemented and aligned with the contract's functionality.
query_config: Retrieves and formats the contract's configuration.query_state: Retrieves and formats the contract's state.query_batch: Retrieves a specific batch by ID.query_batches: Retrieves batches with optional filtering by status.query_batches_by_ids: Retrieves batches by a list of IDs.query_pending_batch: Retrieves the pending batch.query_unstake_requests: Retrieves unstake requests for a specific user.query_all_unstake_requests: Retrieves all unstake requests.query_all_unstake_requests_v2: Retrieves all unstake requests in a different format.query_ibc_queue: Retrieves the IBC queue.query_reply_queue: Retrieves the IBC reply queue.These functions are implemented in
contracts/staking/src/query.rsand appear to be functioning as intended.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the correct implementation of query functions in the contract. # Test: Search for the query functions usage in the contract. Expect: Correct implementation and alignment with contract functionality. rg --type rust $'query_config|query_state|query_batch|query_batches|query_batches_by_ids|query_pending_batch|query_unstake_requests|query_all_unstake_requests|query_all_unstake_requests_v2|query_ibc_queue|query_reply_queue' contracts/staking/src/contract.rsLength of output: 1347
Script:
#!/bin/bash # Description: Locate the definitions of query functions in the codebase. # Search for the definitions of each query function. ast-grep --lang rust --pattern 'fn query_config($_) { $$$ }' ast-grep --lang rust --pattern 'fn query_state($_) { $$$ }' ast-grep --lang rust --pattern 'fn query_batch($_) { $$$ }' ast-grep --lang rust --pattern 'fn query_batches($_) { $$$ }' ast-grep --lang rust --pattern 'fn query_batches_by_ids($_) { $$$ }' ast-grep --lang rust --pattern 'fn query_pending_batch($_) { $$$ }' ast-grep --lang rust --pattern 'fn query_unstake_requests($_) { $$$ }' ast-grep --lang rust --pattern 'fn query_all_unstake_requests($_) { $$$ }' ast-grep --lang rust --pattern 'fn query_all_unstake_requests_v2($_) { $$$ }' ast-grep --lang rust --pattern 'fn query_ibc_queue($_) { $$$ }' ast-grep --lang rust --pattern 'fn query_reply_queue($_) { $$$ }'Length of output: 755
Script:
#!/bin/bash # Description: Locate the definitions of query functions in the codebase by searching for function names directly. # Search for the definitions of each query function. rg --type rust 'fn query_config' rg --type rust 'fn query_state' rg --type rust 'fn query_batch' rg --type rust 'fn query_batches' rg --type rust 'fn query_batches_by_ids' rg --type rust 'fn query_pending_batch' rg --type rust 'fn query_unstake_requests' rg --type rust 'fn query_all_unstake_requests' rg --type rust 'fn query_all_unstake_requests_v2' rg --type rust 'fn query_ibc_queue' rg --type rust 'fn query_reply_queue'Length of output: 1833
Script:
#!/bin/bash # Description: Extract the implementations of query functions from the identified files. # Extract the implementations of each query function. rg --type rust -A 20 'pub fn query_config' contracts/staking/src/query.rs rg --type rust -A 20 'pub fn query_state' contracts/staking/src/query.rs rg --type rust -A 20 'pub fn query_batch' contracts/staking/src/query.rs rg --type rust -A 20 'pub fn query_batches' contracts/staking/src/query.rs rg --type rust -A 20 'pub fn query_batches_by_ids' contracts/staking/src/query.rs rg --type rust -A 20 'pub fn query_pending_batch' contracts/staking/src/query.rs rg --type rust -A 20 'pub fn query_unstake_requests' contracts/staking/src/query.rs rg --type rust -A 20 'pub fn query_all_unstake_requests' contracts/staking/src/query.rs rg --type rust -A 20 'pub fn query_all_unstake_requests_v2' contracts/staking/src/query.rs rg --type rust -A 20 'pub fn query_ibc_queue' contracts/staking/src/query.rs rg --type rust -A 20 'pub fn query_reply_queue' contracts/staking/src/query.rsLength of output: 9803
Line range hint
209-224: Ensure theupdate_configfunction correctly handles thesend_fees_to_treasuryparameter.Verification successful
The
update_configfunction incontracts/staking/src/execute.rscorrectly handles thesend_fees_to_treasuryparameter by updating theconfig.send_fees_to_treasuryfield if the parameter is provided.
contracts/staking/src/execute.rs, lines 206-212: Thesend_fees_to_treasuryparameter is correctly handled within theupdate_configfunction.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the correct handling of `send_fees_to_treasury` in the `update_config` function. # Test: Search for the `send_fees_to_treasury` usage in the `update_config` function. Expect: Correct handling and structure. rg --type rust $'send_fees_to_treasury' contracts/staking/src/contract.rsLength of output: 363
Script:
#!/bin/bash # Extract the `update_config` function to inspect the handling of `send_fees_to_treasury`. ast-grep --lang rust --pattern 'fn update_config($_) { $$$ }' contracts/staking/src/contract.rsLength of output: 99
Script:
#!/bin/bash # Refine the pattern to match the `update_config` function more accurately. ast-grep --lang rust --pattern 'fn update_config($$$) { $$$ }' contracts/staking/src/contract.rsLength of output: 100
Script:
#!/bin/bash # Locate the `update_config` function definition and inspect the handling of `send_fees_to_treasury`. rg 'fn update_config' contracts/staking/src/contract.rs -A 20Length of output: 63
Script:
#!/bin/bash # Search for the `update_config` function definition across the entire repository. rg 'fn update_config' -t rustLength of output: 319
Script:
#!/bin/bash # Extract the `update_config` function from `contracts/staking/src/execute.rs` to inspect the handling of `send_fees_to_treasury`. ast-grep --lang rust --pattern 'pub fn update_config($$$) { $$$ }' contracts/staking/src/execute.rsLength of output: 103
Script:
#!/bin/bash # Locate the `update_config` function definition in `contracts/staking/src/execute.rs` and inspect the handling of `send_fees_to_treasury`. rg 'fn update_config' contracts/staking/src/execute.rs -A 50Length of output: 2128
contracts/staking/src/tests/unstake_tests.rs (6)
59-130: The testdouble_liquid_unstakeeffectively checks the scenario where a user unstakes multiple times before a batch is submitted. It's crucial to verify that the state updates correctly after each unstake operation and that the batch totals reflect all unstake requests accurately.
171-187: The testinvalid_denom_liquid_unstakeis designed to verify that the contract correctly handles attempts to unstake with an invalid denomination. This is a good practice as it ensures robust error handling in the contract.
242-298: The testinvalid_amount_liquid_unstakeshould verify that the contract handles cases where the unstake amount is greater than the total liquid stake tokens. This test is crucial for ensuring that the contract does not allow unauthorized token creation or manipulation.
300-356: The testtotal_liquid_stake_token_with_zerochecks the behavior when the total liquid stake token is zero. This is an important edge case to test, ensuring that the contract behaves correctly under such conditions.
358-440: The testclaimable_batchesverifies the functionality related to querying and receiving tokens for claimable batches. It's essential to ensure that the test covers scenarios where multiple batches are claimable and that the contract handles each correctly.
18-57: The testproper_liquid_unstakeappears to be well-structured and covers the basic functionality of liquid unstaking. However, it's important to ensure that the test checks for the correct handling of edge cases, such as when the unstake amount exceeds the available tokens.Verification successful
The
proper_liquid_unstaketest is complemented by other tests such asinvalid_denom_liquid_unstakeandinvalid_amount_liquid_unstake, which handle important edge cases. Therefore, the edge cases are adequately covered in the test suite.
invalid_denom_liquid_unstake: Tests the scenario where the unstake denomination is invalid.invalid_amount_liquid_unstake: Tests the scenario where the unstake amount is invalid.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that the `proper_liquid_unstake` test handles edge cases properly. # Test: Search for additional tests handling edge cases. rg --type rust $'fn proper_liquid_unstake' contracts/staking/src/tests/Length of output: 146
Script:
#!/bin/bash # Description: List all test functions in the `unstake_tests.rs` file to check for edge cases. # List all test functions in the file. rg --type rust --pcre2 --only-matching --no-line-number --no-filename '^fn \w+' contracts/staking/src/tests/unstake_tests.rsLength of output: 327
Script:
#!/bin/bash # Description: Extract the implementations of `invalid_denom_liquid_unstake` and `invalid_amount_liquid_unstake` functions to review edge case handling. # Extract the `invalid_denom_liquid_unstake` function implementation. rg --type rust --context 10 'fn invalid_denom_liquid_unstake' contracts/staking/src/tests/unstake_tests.rs # Extract the `invalid_amount_liquid_unstake` function implementation. rg --type rust --context 10 'fn invalid_amount_liquid_unstake' contracts/staking/src/tests/unstake_tests.rsLength of output: 1490
contracts/staking/src/tests/ibc_transfer_tests.rs (7)
160-238: The testfail_ibc_queuechecks the behavior of the IBC transfer mechanism when the acknowledgment fails. This is crucial for ensuring that the contract can handle failures gracefully and correctly revert or adjust the state as necessary.
240-316: The testtimeout_ibc_queueis designed to verify the contract's behavior when an IBC transfer times out. Testing timeout scenarios is essential for ensuring that the contract can handle network delays or disruptions correctly.
318-345: The testrecover_non_paginatedchecks the recovery mechanism for non-paginated IBC transfers. It's important to verify that the recovery process correctly handles all packets, regardless of their status.
347-374: The testrecover_paginatedverifies the recovery process for paginated IBC transfers. Ensuring that pagination is handled correctly is crucial for the scalability and performance of the contract.
376-407: The testrecover_forcedexplores the forced recovery of selected IBC packets. This test is important for verifying that the contract can selectively recover packets based on specific criteria or conditions.
409-529: The testrecover_multipleandrecover_recursiveare designed to test the recovery mechanisms for multiple and recursive IBC transfers. These tests are crucial for ensuring that the contract can handle complex scenarios where multiple transfers need recovery.
16-158: The testsuccess_ibc_queueeffectively simulates a successful IBC queue operation, including the creation of an IBC transfer message and handling the reply. It's important to ensure that the test covers all aspects of the IBC lifecycle, including timeouts and error handling.Verification successful
The
ibc_transfer_tests.rsfile contains tests that cover various aspects of the IBC lifecycle, including error handling and timeouts. Specifically:
fail_ibc_queuecovers scenarios where the IBC transfer fails.timeout_ibc_queuecovers scenarios where the IBC transfer times out.These tests, along with
success_ibc_queue, ensure comprehensive coverage of the IBC lifecycle, including success, failure, and timeout scenarios.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that all aspects of the IBC lifecycle are covered in `success_ibc_queue`. # Test: Search for tests covering timeouts and error handling in IBC lifecycle. rg --type rust $'IBC_TIMEOUT' contracts/staking/src/tests/ibc_transfer_tests.rsLength of output: 267
Script:
#!/bin/bash # Description: Search for other tests in the file that might cover error handling and timeouts. # Test: Search for test functions in the file. rg --type rust '#\[test\]' contracts/staking/src/tests/ibc_transfer_tests.rs # Test: Search for error handling in the file. rg --type rust 'Err\(' contracts/staking/src/tests/ibc_transfer_tests.rsLength of output: 277
Script:
#!/bin/bash # Description: Extract the names of the test functions and the context around the error handling instances. # Extract the names of the test functions. rg --type rust --context 1 '#\[test\]' contracts/staking/src/tests/ibc_transfer_tests.rs # Extract the context around the error handling instances. rg --type rust --context 5 'Err\(' contracts/staking/src/tests/ibc_transfer_tests.rsLength of output: 1202
Script:
#!/bin/bash # Description: Extract the implementations of `fail_ibc_queue` and `timeout_ibc_queue` tests. # Extract the implementation of `fail_ibc_queue`. rg --type rust --context 50 'fn fail_ibc_queue' contracts/staking/src/tests/ibc_transfer_tests.rs # Extract the implementation of `timeout_ibc_queue`. rg --type rust --context 50 'fn timeout_ibc_queue' contracts/staking/src/tests/ibc_transfer_tests.rsLength of output: 6230
contracts/staking/src/execute.rs (1)
Line range hint
687-702: Ensure consistent handling of optional parameters inupdate_config.Also applies to: 731-733
Verification successful
The calls to
update_configincontracts/staking/src/contract.rshandle optional parameters by passingSomevalues explicitly. This ensures that the optional parameters are consistently handled.
contracts/staking/src/contract.rs, line 20:update_configis called withSomevalues for all optional parameters.contracts/staking/src/contract.rs, line 40:update_configis called with variables that are likely to beOptiontypes, ensuring proper handling of optional parameters.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that all calls to `update_config` correctly handle optional parameters. # Test: Search for the function usage. Expect: Proper handling of None values for optional parameters. rg --type rust $'update_config'Length of output: 1369
Script:
#!/bin/bash # Extract and inspect calls to `update_config` in the relevant files. # Search for calls to `update_config` in `contracts/staking/src/contract.rs` and `contracts/staking/src/execute.rs` rg --type rust 'update_config' contracts/staking/src/contract.rs contracts/staking/src/execute.rsLength of output: 469
Script:
#!/bin/bash # Extract relevant lines around the calls to `update_config` in `contracts/staking/src/contract.rs`. # Extract 10 lines before and after each call to `update_config` for context. rg --type rust 'update_config' -A 10 -B 10 contracts/staking/src/contract.rsLength of output: 1916
manu0466
force-pushed
the
manuel/update-treasury-contract
branch
from
e134a60 to
9e3b2cd
Compare
manu0466
force-pushed
the
manuel/update-treasury-contract
branch
from
9e3b2cd to
aeae749
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range and nitpick comments (1)
contracts/staking/src/migrations/v0_4_20.rs (1)
[!TIP]
Codebase VerificationThe
send_fees_to_treasuryfield is missing from theConfigstruct definition instate.rs. The migration function inv0_4_20.rsattempts to set this field, but it is not present in the struct definition. To resolve this, you need to update theConfigstruct to include thesend_fees_to_treasuryfield and ensure the migration function correctly sets this field.
- Update the
Configstruct incontracts/staking/src/state.rsto include thesend_fees_to_treasuryfield.- Ensure the migration function in
contracts/staking/src/migrations/v0_4_20.rscorrectly sets this field during migration.Analysis chain
Line range hint
41-61: Ensure proper testing of the migration logic.
The output indicates that the
send_fees_to_treasuryfield is not present in theConfigstruct after the migration. This suggests that the migration logic might not be correctly updating the struct. To verify this, we need to check the definition of theConfigstruct and ensure that thesend_fees_to_treasuryfield is included and correctly set during migration.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the migration logic by checking the state before and after migration. # Test: Check the state before and after migration. Expect: The new field should be present and correctly set after migration. # Fetch the contract state before migration echo "State before migration:" cat contracts/staking/src/state.rs | grep -A 10 "pub struct Config" # Perform the migration echo "Performing migration..." # Simulate migration command here (pseudo-command) echo "migrate --send_fees_to_treasury true" # Fetch the contract state after migration echo "State after migration:" cat contracts/staking/src/state.rs | grep -A 10 "pub struct Config"Length of output: 1203
Script:
#!/bin/bash # Verify the presence of the `send_fees_to_treasury` field in the `Config` struct definition. # Check the definition of the `Config` struct in the state file. echo "Checking Config struct definition:" cat contracts/staking/src/state.rs | grep -A 15 "pub struct Config" # Check the migration function to ensure it updates the `send_fees_to_treasury` field. echo "Checking migration function:" cat contracts/staking/src/migrations/v0_4_20.rs | grep -A 20 "pub fn migrate"Length of output: 2019
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (8)
- contracts/staking/src/ack.rs (2 hunks)
- contracts/staking/src/contract.rs (9 hunks)
- contracts/staking/src/execute.rs (7 hunks)
- contracts/staking/src/helpers.rs (7 hunks)
- contracts/staking/src/migrations/mod.rs (1 hunks)
- contracts/staking/src/migrations/v0_4_20.rs (2 hunks)
- contracts/staking/src/msg.rs (4 hunks)
- contracts/staking/src/tests/instantiate_tests.rs (8 hunks)
Files skipped from review due to trivial changes (1)
- contracts/staking/src/migrations/mod.rs
Files skipped from review as they are similar to previous changes (6)
- contracts/staking/src/ack.rs
- contracts/staking/src/contract.rs
- contracts/staking/src/execute.rs
- contracts/staking/src/helpers.rs
- contracts/staking/src/msg.rs
- contracts/staking/src/tests/instantiate_tests.rs
Overview
This PR updates the treasury and staking contracts, allowing the staking contract to send protocol fees to the treasury and enabling a
traderaccount to perform swap operations, helping the treasury hedge against asset price fluctuations.To prevent the
traderfrom converting the treasury's assets into unwanted ones, a list of swap routes can be defined. This forces thetraderto use only a predefined set of pools and only in a predefined direction. For example, if the Osmosis pool 1 has the OSMO/USDC pair, we can enforce that thetradercan only swap OSMO for USDC, but not the other way around.closes: MILKTIA-35
What changes have been made in this PR?
cargo clippyChecklist