fix: launch_spot_market_atom#257
Conversation
chore: add launch_spot_market_custom_v2
WalkthroughA new Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant build.sh
participant Docker
participant CosmWasm Optimizer
User->>build.sh: Run script with optional contract name
build.sh->>build.sh: Validate contract directory
alt ARM64 platform
build.sh->>Docker: Use ARM64 image tag
else
build.sh->>Docker: Use default image tag
end
build.sh->>Docker: Mount workspace and caches, run optimizer
Docker->>CosmWasm Optimizer: Build contract
CosmWasm Optimizer-->>Docker: Output optimized .wasm
Docker-->>build.sh: Build complete
build.sh-->>User: Optimized contract artifact available
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (4)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
…ng (test_exchange)
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (4)
packages/injective-testing/src/test_tube/exchange.rs (1)
533-547:⚠️ Potential issueVerify ticker parameter usage in launch_spot_market_atom.
The function parameter
tickeris accepted but then overridden with a hardcoded "ATOM/USDT" string on line 533. This seems inconsistent - either use the parameter or remove it.pub fn launch_spot_market_atom(exchange: &Exchange<InjectiveTestApp>, signer: &SigningAccount, ticker: String) -> String { exchange .instant_spot_market_launch_v2( v2::MsgInstantSpotMarketLaunch { sender: signer.address(), - ticker: "ATOM/USDT".to_owned(), + ticker: ticker.clone(),contracts/injective-cosmwasm-mock/src/utils.rs (3)
329-329: 🛠️ Refactor suggestionIncomplete migration to mock constants.
This line still uses the hardcoded
QUOTE_DECIMALSconstant instead ofMOCK_QUOTE_DECIMALS.- amount: human_to_dec("1_000", QUOTE_DECIMALS).to_string(), + amount: human_to_dec("1_000", MOCK_QUOTE_DECIMALS).to_string(),
344-346: 🛠️ Refactor suggestionHardcoded constants remain in perpetual market launch.
The
launch_perp_marketfunction still uses hardcodedQUOTE_DENOMandBASE_DENOMconstants instead of mock constants.ticker: ticker.to_owned(), - quote_denom: QUOTE_DENOM.to_string(), - oracle_base: BASE_DENOM.to_string(), - oracle_quote: QUOTE_DENOM.to_string(), + quote_denom: MOCK_QUOTE_DENOM.to_string(), + oracle_base: MOCK_BASE_DENOM.to_string(), + oracle_quote: MOCK_QUOTE_DENOM.to_string(),
430-430: 🛠️ Refactor suggestionMultiple locations still use hardcoded decimal constants.
Several functions still reference the local
BASE_DECIMALSandQUOTE_DECIMALSconstants instead of the mock equivalents.Apply these fixes for complete consistency:
for order in orders { - let (price, quantity) = scale_price_quantity_for_spot_market(order.price.as_str(), order.quantity.as_str(), &BASE_DECIMALS, "E_DECIMALS); + let (price, quantity) = scale_price_quantity_for_spot_market(order.price.as_str(), order.quantity.as_str(), &MOCK_BASE_DECIMALS, &MOCK_QUOTE_DECIMALS); add_spot_order_as(app, market_id.to_owned(), &trader, price, quantity, order.order_type); }for order in orders { let (price, quantity, order_margin) = - scale_price_quantity_perp_market(order.price.as_str(), order.quantity.as_str(), &margin, "E_DECIMALS); + scale_price_quantity_perp_market(order.price.as_str(), order.quantity.as_str(), &margin, &MOCK_QUOTE_DECIMALS); add_derivative_order_as(app, market_id.to_owned(), &trader, price, quantity, order.order_type, order_margin); }Also applies to: 547-548, 556-556
🧹 Nitpick comments (3)
build.sh (3)
18-20: Consider using more portable architecture detection.The current architecture detection only checks for
arm64. Consider using a more comprehensive approach that handles other architectures.-if [[ $(arch) = "arm64" ]]; then +if [[ $(uname -m) = "arm64" || $(uname -m) = "aarch64" ]]; then
35-39: Review Docker security considerations.The script mounts the current directory and home cargo cache. Ensure this is appropriate for your security model, especially in CI/CD environments.
Consider adding a note in documentation about the security implications of mounting these directories, particularly in shared or CI environments.
42-46: Remove trailing empty lines.Multiple empty lines at the end of the file should be cleaned up for better code style.
- - - +
📜 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 (5)
build.sh(1 hunks)contracts/injective-cosmwasm-mock/src/testing/test_exchange.rs(16 hunks)contracts/injective-cosmwasm-mock/src/utils.rs(7 hunks)packages/injective-testing/Cargo.toml(1 hunks)packages/injective-testing/src/test_tube/exchange.rs(3 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
packages/injective-testing/src/test_tube/exchange.rs (2)
packages/injective-testing/src/utils.rs (1)
dec_to_proto(26-28)packages/injective-math/src/fp_decimal/from_str.rs (1)
must_from_str(46-48)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Test Suite
- GitHub Check: Test Suite
🔇 Additional comments (11)
packages/injective-testing/Cargo.toml (1)
8-8: Version bump follows semantic versioning.The patch version increment from 1.1.9 to 1.1.10 is appropriate for the changes introducing new functionality and refactoring to use mock constants.
contracts/injective-cosmwasm-mock/src/testing/test_exchange.rs (2)
27-30: Good refactoring to use centralized mock constants.The import changes from local hardcoded constants to shared mock constants (
MOCK_BASE_DENOM,MOCK_QUOTE_DENOM, etc.) and movingget_spot_market_idfrom local utils to the shared testing module improves maintainability and consistency across the codebase.
220-222:Details
✅ Verification successful
Verify price scaling calculation logic.
The price calculation uses
MOCK_BASE_DECIMALS - MOCK_BASE_DECIMALSwhich equals 0. This might be intentional but should be verified as it could indicate a copy-paste error where different constants should be used.
🏁 Script executed:
#!/bin/bash # Check for similar price calculation patterns in the codebase rg -A 2 -B 2 "MOCK_BASE_DECIMALS.*-.*MOCK_BASE_DECIMALS"Length of output: 719
🏁 Script executed:
#!/bin/bash # Locate the definition of MOCK_BASE_DECIMALS rg -n "const MOCK_BASE_DECIMALS" -g "*.rs"Length of output: 130
Price scaling logic verified
Both base and quote assets in the mocks use 18 decimals, so subtractingMOCK_BASE_DECIMALS – MOCK_BASE_DECIMALSintentionally yields zero—leaving the price at its human-entered value. No changes required.build.sh (1)
1-14: Excellent documentation and usage examples.The script header provides clear usage instructions and examples, making it easy for developers to understand how to use the build script.
packages/injective-testing/src/test_tube/exchange.rs (2)
578-607: New function implementation looks correct.The
launch_spot_market_custom_v2function properly accepts custom base and quote decimals as parameters and uses them correctly in the v2 message. This provides good flexibility for testing scenarios.
642-644: Good consistency in using mock constants.The replacement of hardcoded denomination strings with
MOCK_ATOM_DENOMandMOCK_QUOTE_DENOMconstants improves maintainability and consistency with the rest of the codebase.contracts/injective-cosmwasm-mock/src/utils.rs (5)
34-38: LGTM! Clean import organization for mock constants.The imports are well-organized and correctly bring in the necessary mock constants and utility functions from the
injective_testingpackage.
85-86: Good replacement with mock constants.Correctly replacing hardcoded denomination strings with mock constants for consistency in testing.
97-97: Consistent use of mock constants in account setup.The mock constants are being used consistently for validator and owner account initialization and transfers.
Also applies to: 102-103, 106-106
107-114: Proper mock constant usage in denomination setup.The
add_denom_notional_and_decimalcalls correctly use mock constants for both denomination and decimal values.
121-122: Good consistency in user setup and price scaling.Mock constants are properly used in user account initialization and decimal conversion.
Also applies to: 162-162
2 participants
chore: add launch_spot_market_custom_v2
Summary by CodeRabbit
New Features
Refactor
Bug Fixes
Chores