fix: renamed market modules #385
Conversation
WalkthroughThis update refactors market conversion logic and test fixtures to unify and clarify the process of converting between human-readable and chain formats. It reorganizes imports, adjusts quantization and scaling order in market classes, synchronizes test logic, and updates fixture definitions. The package version is incremented to 1.11.0-rc4. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant MarketClass
participant Token
User->>MarketClass: Provide human-readable value (qty/price/margin)
MarketClass->>MarketClass: Scale by token decimals
MarketClass->>MarketClass: Quantize value (floor to tick size)
MarketClass->>MarketClass: Apply additional decimal scaling
MarketClass->>User: Return chain format value
Poem
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
✨ Finishing Touches
🪧 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
Documentation and Community
|
There was a problem hiding this comment.
Pull Request Overview
This PR renames market modules and updates related conversion logic and tick size parameters across tests and implementation files. The main changes include updating import paths from legacy modules to the new market modules, revising tick size values and conversion formula orders in fixtures and core functions, and bumping the package version.
Reviewed Changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/test_composer_deprecation_warnings.py | Updated import path to use new markets fixtures |
| tests/test_composer.py | Updated import path to use new markets fixtures |
| tests/model_fixtures/markets_v2_fixtures.py | Replaced references from chain_formatted_market to market_v2 and adjusted tick size values |
| tests/model_fixtures/markets_fixtures.py | Switched token fixture imports to use markets_v2 fixtures and reverted tick size values |
| tests/core/* | Updated tests to reflect changes in market conversion formulas and new module references |
| pyproject.toml | Bumped version from 1.11.0-rc3 to 1.11.0-rc4 |
| pyinjective/core/market_v2.py, market.py | Revised conversion formulas ordering in quantity and price conversion functions |
| pyinjective/composer.py, async_client*.py | Updated import paths to reference the appropriate market modules |
Comments suppressed due to low confidence (2)
tests/model_fixtures/markets_v2_fixtures.py:65
- Verify that the updated tick size values (e.g. changing from a very small value to 0.01) are correct and consistently aligned with market_v2 expectations.
min_price_tick_size=Decimal("0.01"),
pyinjective/core/market_v2.py:25
- The reordering of quantization and multiplication in the quantity_to_chain_format function changes the evaluation order; please ensure that this new ordering still meets the intended conversion logic.
quantized_value = human_readable_value // self.min_quantity_tick_size * self.min_quantity_tick_size
| min_price_tick_size=Decimal("0.01"), | ||
| min_quantity_tick_size=Decimal("0.001"), | ||
| min_notional=Decimal("1"), | ||
| min_price_tick_size=Decimal("0.000000000000001"), |
There was a problem hiding this comment.
The tick size values here differ significantly from those in the markets_v2_fixtures; consider consolidating these definitions to avoid potential inconsistencies in test behavior.
| min_price_tick_size=Decimal("0.000000000000001"), | |
| min_price_tick_size=markets_v2_fixtures.INJ_USDT_SPOT_MIN_PRICE_TICK_SIZE, |
![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 (2)
pyinjective/async_client.py (1)
19-29: Fix isort-reported import ordering
pre-commitfailed because the newly-addedcore.marketimport is not alphabetically sorted within the local-package block. Re-order thepyinjective.*imports to keep CI green.-from pyinjective.client.model.pagination import PaginationOption -from pyinjective.composer import Composer -from pyinjective.constant import GAS_PRICE -from pyinjective.core.market import BinaryOptionMarket, DerivativeMarket, SpotMarket -from pyinjective.core.ibc.channel.grpc.ibc_channel_grpc_api import IBCChannelGrpcApi +from pyinjective.client.model.pagination import PaginationOption +from pyinjective.composer import Composer +from pyinjective.constant import GAS_PRICE +from pyinjective.core.ibc.channel.grpc.ibc_channel_grpc_api import IBCChannelGrpcApi +from pyinjective.core.ibc.client.grpc.ibc_client_grpc_api import IBCClientGrpcApi +from pyinjective.core.ibc.connection.grpc.ibc_connection_grpc_api import IBCConnectionGrpcApi +from pyinjective.core.ibc.transfer.grpc.ibc_transfer_grpc_api import IBCTransferGrpcApi +from pyinjective.core.market import BinaryOptionMarket, DerivativeMarket, SpotMarket +from pyinjective.core.network import Network +from pyinjective.core.tendermint.grpc.tendermint_grpc_api import TendermintGrpcApi +from pyinjective.core.token import Token +from pyinjective.core.tokens_file_loader import TokensFileLoader +from pyinjective.core.tx.grpc.tx_grpc_api import TxGrpcApi -from pyinjective.core.ibc.client.grpc.ibc_client_grpc_api import IBCClientGrpcApi -from pyinjective.core.ibc.connection.grpc.ibc_connection_grpc_api import IBCConnectionGrpcApi -from pyinjective.core.ibc.transfer.grpc.ibc_transfer_grpc_api import IBCTransferGrpcApi -from pyinjective.core.network import Network -from pyinjective.core.tendermint.grpc.tendermint_grpc_api import TendermintGrpcApi -from pyinjective.core.token import Token -from pyinjective.core.tokens_file_loader import TokensFileLoader -from pyinjective.core.tx.grpc.tx_grpc_api import TxGrpcApiAfter applying, run
isort --apply(orpre-commit run --all-files) to verify.tests/test_composer.py (1)
1138-1166: Duplicate test method is shadowing a previous oneThe method
test_msg_cancel_derivative_orderis already defined earlier in this class (lines 899-938).
Because class attributes are overridden on re-assignment, the earlier test is silently dropped and never executed by pytest, reducing coverage.- def test_msg_cancel_derivative_order( + # Cancelling a binary-options order – give the test its own name + def test_msg_cancel_binary_options_order(Rename (or remove) one of the duplicates so both scenarios are exercised.
🧹 Nitpick comments (4)
tests/test_composer.py (1)
11-18: Fixture-import path changed – verify remaining referencesThe import now points to
tests.model_fixtures.markets_fixtures, which matches the module-rename in this PR.
Please run a quick grep to ensure no test module still references the oldchain_formatted_markets_fixturespath.rg 'chain_formatted_markets_fixtures' testsIf the command returns anything, update those files as well.
tests/core/test_gas_limit_estimator.py (1)
25-32: Trim unused fixtures to drop# noqa: F401noise
inj_token,usdt_perp_token, andusdt_tokenaren’t referenced in this test module. Keeping them forces the# noqa: F401suppression and slightly slows collection. Consider importing only the fixtures you actually use:-from tests.model_fixtures.markets_v2_fixtures import ( # noqa: F401 - btc_usdt_perp_market, - first_match_bet_market, - inj_token, - inj_usdt_spot_market, - usdt_perp_token, - usdt_token, -) +from tests.model_fixtures.markets_v2_fixtures import ( + btc_usdt_perp_market, + first_match_bet_market, + inj_usdt_spot_market, +)This keeps the import list minimal and removes the need for
# noqa.tests/model_fixtures/markets_fixtures.py (1)
6-8: Remove unused import.The
usdt_tokenimport is flagged as unused by static analysis. Please verify if it's actually needed or remove it.-from tests.model_fixtures.markets_v2_fixtures import usdt_token # noqa: F401; noqa: F401pyinjective/core/market_v2.py (1)
42-42: Fix typo in variable name.There's a typo:
quantized_balueshould bequantized_value.- quantized_balue = chain_formatted_value.quantize(Decimal("1"), rounding=ROUND_UP) + quantized_value = chain_formatted_value.quantize(Decimal("1"), rounding=ROUND_UP)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
pyinjective/async_client.py(1 hunks)pyinjective/async_client_v2.py(1 hunks)pyinjective/composer.py(1 hunks)pyinjective/core/market.py(6 hunks)pyinjective/core/market_v2.py(6 hunks)pyproject.toml(1 hunks)tests/core/test_gas_heuristics_gas_limit_estimator.py(1 hunks)tests/core/test_gas_limit_estimator.py(1 hunks)tests/core/test_market.py(16 hunks)tests/core/test_market_v2.py(16 hunks)tests/core/test_token.py(1 hunks)tests/model_fixtures/markets_fixtures.py(4 hunks)tests/model_fixtures/markets_v2_fixtures.py(4 hunks)tests/test_composer.py(1 hunks)tests/test_composer_deprecation_warnings.py(1 hunks)
🧰 Additional context used
🪛 GitHub Actions: pre-commit
pyinjective/async_client.py
[error] 19-29: isort formatting check failed. The import statements were not correctly sorted. Run 'isort --apply' to fix import order issues.
🪛 Ruff (0.11.9)
tests/core/test_market_v2.py
17-17: Redefinition of unused inj_usdt_spot_market from line 10
(F811)
tests/core/test_market.py
11-11: tests.model_fixtures.markets_fixtures.usdt_token imported but unused
Remove unused import: tests.model_fixtures.markets_fixtures.usdt_token
(F401)
15-15: Redefinition of unused inj_usdt_spot_market from line 9
(F811)
tests/model_fixtures/markets_fixtures.py
8-8: tests.model_fixtures.markets_v2_fixtures.usdt_token imported but unused
Remove unused import: tests.model_fixtures.markets_v2_fixtures.usdt_token
(F401)
🪛 Flake8 (7.2.0)
tests/core/test_market_v2.py
[error] 17-17: redefinition of unused 'inj_usdt_spot_market' from line 6
(F811)
tests/core/test_market.py
[error] 15-15: redefinition of unused 'inj_usdt_spot_market' from line 9
(F811)
🔇 Additional comments (28)
pyproject.toml (1)
3-3: Sync the version bump across all version references
pyproject.tomlnow declares1.11.0-rc4, but some parts of the repo (e.g.pyinjective/__init__.py, any generated docs, CI release scripts, CHANGELOG) may still read1.11.0-rc3. Mismatched versions lead to hard-to-trace packaging issues and incorrect artefact tagging.Run a quick sweep to confirm every version reference is updated:
#!/bin/bash # List all hard-coded version strings that are *not* rc4. rg -n --fixed-strings '"1.11.0-rc3"' || echo "No stale rc3 strings found ✅" # Check for an __version__ attribute that must match. ast-grep --pattern $'__version__ = "1.11.0-rc4"'If any stale occurrences appear, update them before merging.
tests/core/test_token.py (1)
3-3: Import path update looks correct – confirm other tests follow suitThe fixture path switch to
markets_v2_fixturesaligns with the module-renaming effort. Verify that every other test now imports from the same location to avoidModuleNotFoundErrors.#!/bin/bash # Detect any lingering references to the old fixtures module rg -n $'tests\\.model_fixtures\\.markets_fixtures' testspyinjective/composer.py (1)
11-11: Moved tocore.market– hunt for stray legacy importsThe import swap is consistent with the new module layout. Ensure
pyinjective.core.chain_formatted_marketis fully retired; stray imports would silently break runtime dependency graphs.#!/bin/bash # Search the entire repo for obsolete references rg -n $'pyinjective\\.core\\.chain_formatted_market'tests/test_composer_deprecation_warnings.py (1)
7-14: Import path update looks goodThe fixture import path was updated in line with the module rename. No further issues here.
pyinjective/async_client_v2.py (1)
27-27: Ensure complete migration away from legacypyinjective.core.marketimports
BinaryOptionMarket,DerivativeMarket, andSpotMarketare now sourced frompyinjective.core.market_v2, but other parts of the codebase (or downstream projects) might still import from the old module, leading to duplicated class definitions at runtime.#!/bin/bash # Verify that no Python files still import from the legacy module rg --line-number --no-heading $'from pyinjective\\.core\\.market' | headExpect zero matches. Any remaining occurrences should be updated to
market_v2for consistency and to prevent subtle bugs caused by using different class objects.tests/core/test_gas_limit_estimator.py (1)
20-20: Import path switch looks correctImporting
BinaryOptionMarketfrommarket_v2aligns the test suite with the refactored market classes.tests/core/test_gas_heuristics_gas_limit_estimator.py (1)
35-42: LGTM! Clean import update to align with refactored fixtures.The import change from
markets_fixturestomarkets_v2_fixturesis consistent with the module refactoring objectives.tests/model_fixtures/markets_fixtures.py (1)
22-24: Verify updated market parameters align with v2 logic.The tick size and notional parameters have been significantly updated (e.g.,
min_price_tick_sizefrom scientific notation to decimal values). Ensure these changes are consistent with the new quantization logic in market_v2.Run the following script to verify the parameter changes are consistent across fixtures:
#!/bin/bash # Description: Compare market parameters between v1 and v2 fixtures to ensure consistency # Check for differences in market parameter patterns echo "Checking market parameter patterns in fixtures..." rg -A 3 -B 1 "min_price_tick_size|min_quantity_tick_size|min_notional" tests/model_fixtures/tests/core/test_market_v2.py (2)
21-27: Quantization logic correctly updated for v2.The test correctly reflects the new quantization-first approach: quantize the human-readable value, then scale by decimals, then apply additional chain format decimals.
135-139: Consistent quantization pattern applied.The quantization-first pattern is consistently applied across derivative market tests, which aligns with the v2 implementation approach.
tests/model_fixtures/markets_v2_fixtures.py (2)
9-51: Well-structured token fixtures with comprehensive attributes.The token fixtures are properly defined with all necessary attributes (name, symbol, denom, address, decimals, logo, updated timestamp). This centralizes token definitions effectively.
65-67: Standardized market parameters improve consistency.The updated tick sizes and notional values (e.g.,
min_price_tick_size=Decimal("0.01")) are more realistic and consistent compared to the scientific notation values in the original fixtures.pyinjective/core/market_v2.py (4)
24-29: Quantization-first approach correctly implemented.The new logic quantizes the human-readable value first, then scales by token decimals and additional chain format decimals. This approach is mathematically sound and may provide better precision control.
32-37: Price conversion logic consistently updated.The price conversion follows the same quantization-first pattern, correctly handling the decimal difference between quote and base tokens.
111-116: Simplified margin calculation improves clarity.The margin calculation now works directly with human-readable values before quantization and scaling, which is cleaner and more intuitive than the previous approach.
218-224: Binary options margin calculation follows consistent pattern.The margin calculation for binary options correctly implements the quantization-first approach and properly handles the buy/sell price difference for binary options.
tests/core/test_market.py (5)
19-23: LGTM: Test logic correctly updated for scaling-before-quantization approach.The test logic has been properly updated to match the new conversion implementation where human-readable values are first scaled by token decimals before quantization and additional chain format scaling.
32-36: LGTM: Price conversion test logic correctly updated.The test properly reflects the new order of operations: scale by price decimals first, then quantize, then apply additional chain format decimals.
132-136: LGTM: Derivative market price conversion test updated correctly.The test logic correctly mirrors the implementation changes for derivative market price conversions.
373-381: LGTM: Binary option margin calculation tests updated correctly.The margin calculation tests have been properly updated to reflect the new approach where quantity and price are scaled by their respective decimals before multiplication and quantization. This aligns with the implementation changes in the
BinaryOptionMarketclass.Also applies to: 396-401, 416-421
251-253: Verify the updated fixed denom parameter values.The fixed denom parameters have been updated from smaller values (1) to larger values (100, 10000, 0). Ensure these values are appropriate for the test scenarios and reflect realistic market conditions.
#!/bin/bash # Description: Verify if these fixed denom values are used consistently across the codebase # Expected: Find other occurrences of these specific denom values to ensure consistency echo "Searching for fixed denom parameter patterns..." rg -A 3 -B 3 "min_quantity_tick_size=100" rg -A 3 -B 3 "min_price_tick_size=10000" rg -A 3 -B 3 "min_notional=0"Also applies to: 286-288, 323-325, 361-363, 441-443, 471-473, 509-511, 545-547
pyinjective/core/market.py (7)
25-27: LGTM: Improved conversion logic with scaling-before-quantization.The reordering of operations is a good improvement. Scaling the human-readable value by token decimals before quantization ensures more accurate conversions, especially when dealing with tokens that have different decimal places.
33-35: LGTM: Consistent application of scaling-before-quantization for price conversion.The price conversion logic correctly follows the same pattern as quantity conversion, maintaining consistency across the codebase.
91-93: LGTM: Appropriate handling for derivative markets without base token.The derivative market quantity conversion correctly handles the case where there's no base token (hence no decimal scaling needed), while still applying the quantization and additional chain format scaling.
99-101: LGTM: Derivative market price conversion updated consistently.The price conversion for derivative markets properly applies the scaling-before-quantization approach using the quote token decimals.
111-119: LGTM: Margin calculation properly handles separate scaling of components.The margin calculation correctly scales quantity and price by their respective decimals before multiplication. This ensures accurate margin computation in chain format.
183-185: LGTM: Binary option market conversions consistently implemented.All binary option market conversion methods (quantity, price, margin) consistently apply the scaling-before-quantization approach with proper handling of special denominations.
Also applies to: 192-194, 203-205
216-228: LGTM: Binary option margin calculation handles buy/sell logic correctly.The margin calculation properly handles the different logic for buy vs sell orders (using
1 - pricefor sell orders) and correctly scales both quantity and price components before computing the margin.
| from tests.model_fixtures.markets_v2_fixtures import ( # noqa: F401 | ||
| btc_usdt_perp_market, | ||
| first_match_bet_market, | ||
| inj_token, | ||
| inj_usdt_spot_market, | ||
| usdt_perp_token, | ||
| usdt_token, | ||
| ) |
There was a problem hiding this comment.
Fix fixture import redefinition.
Static analysis indicates inj_usdt_spot_market is redefined, suggesting a conflict between imported fixtures and test parameters.
Verify the fixture imports are correct and resolve any naming conflicts:
#!/bin/bash
# Description: Check for fixture redefinition issues
# Search for duplicate fixture definitions
ast-grep --pattern 'def $fixture_name($_):'
rg -n "inj_usdt_spot_market" tests/core/test_market_v2.py🤖 Prompt for AI Agents
In tests/core/test_market_v2.py around lines 6 to 13, there is a fixture import
redefinition issue with inj_usdt_spot_market causing conflicts. Review the
imported fixtures and test parameters to identify where inj_usdt_spot_market is
redefined. Remove or rename the duplicate fixture or adjust imports to ensure
each fixture name is unique and not re-imported or redefined within the test
file.
| from tests.model_fixtures.markets_fixtures import btc_usdt_perp_market # noqa: F401 | ||
| from tests.model_fixtures.markets_fixtures import first_match_bet_market # noqa: F401 | ||
| from tests.model_fixtures.markets_fixtures import inj_token # noqa: F401 | ||
| from tests.model_fixtures.markets_fixtures import inj_usdt_spot_market # noqa: F401 | ||
| from tests.model_fixtures.markets_fixtures import usdt_perp_token # noqa: F401 | ||
| from tests.model_fixtures.markets_fixtures import usdt_token # noqa: F401; noqa: F401 |
There was a problem hiding this comment.
Address static analysis issues with imports.
The import reorganization is good, but there are unused imports and redefinition issues that need to be resolved.
Apply this diff to fix the static analysis issues:
-from tests.model_fixtures.markets_fixtures import usdt_token # noqa: F401; noqa: F401
+# Remove unused import - usdt_token is not used in this fileAlso, there's a redefinition issue with inj_usdt_spot_market being imported on line 9 and then redefined as a parameter on line 15. Consider renaming the import or parameter to avoid confusion.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| from tests.model_fixtures.markets_fixtures import btc_usdt_perp_market # noqa: F401 | |
| from tests.model_fixtures.markets_fixtures import first_match_bet_market # noqa: F401 | |
| from tests.model_fixtures.markets_fixtures import inj_token # noqa: F401 | |
| from tests.model_fixtures.markets_fixtures import inj_usdt_spot_market # noqa: F401 | |
| from tests.model_fixtures.markets_fixtures import usdt_perp_token # noqa: F401 | |
| from tests.model_fixtures.markets_fixtures import usdt_token # noqa: F401; noqa: F401 | |
| from tests.model_fixtures.markets_fixtures import btc_usdt_perp_market # noqa: F401 | |
| from tests.model_fixtures.markets_fixtures import first_match_bet_market # noqa: F401 | |
| from tests.model_fixtures.markets_fixtures import inj_token # noqa: F401 | |
| from tests.model_fixtures.markets_fixtures import inj_usdt_spot_market # noqa: F401 | |
| from tests.model_fixtures.markets_fixtures import usdt_perp_token # noqa: F401 | |
| # Remove unused import - usdt_token is not used in this file |
🧰 Tools
🪛 Ruff (0.11.9)
11-11: tests.model_fixtures.markets_fixtures.usdt_token imported but unused
Remove unused import: tests.model_fixtures.markets_fixtures.usdt_token
(F401)
🤖 Prompt for AI Agents
In tests/core/test_market.py around lines 6 to 11, there are unused imports and
a redefinition issue with inj_usdt_spot_market being imported and also used as a
parameter later. Remove any unused imports to clean up static analysis warnings
and rename either the inj_usdt_spot_market import or the parameter on line 15 to
avoid naming conflicts and confusion.
Summary by CodeRabbit