chore: release for chain v1.20.3 - #421
Conversation
📝 WalkthroughWalkthroughUpdated upstream proto inputs and regenerated Python protobuf/gRPC bindings. Added derivative Markets and delegation-transfer RPC plumbing, propagated trade and position fields through descriptors, updated related tests, CI configuration, and bumped the package version to 1.16.1. ChangesProtocol and RPC update
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant InjectiveTCDerivativesRPCStub
participant InjectiveTCDerivativesRPCServicer
Client->>InjectiveTCDerivativesRPCStub: Send MarketsRequest
InjectiveTCDerivativesRPCStub->>InjectiveTCDerivativesRPCServicer: Invoke Markets
InjectiveTCDerivativesRPCServicer-->>InjectiveTCDerivativesRPCStub: Return MarketsResponse or UNIMPLEMENTED
InjectiveTCDerivativesRPCStub-->>Client: Deliver response
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tests/client/chain/stream_grpc/test_chain_grpc_chain_stream.py (1)
265-265: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winUse non-default
notionalvalues in both stream fixtures.Both expected values are derived from
SpotTradeobjects whosenotionalremains the protobuf default""; a broken conversion can therefore pass unnoticed.
tests/client/chain/stream_grpc/test_chain_grpc_chain_stream.py#L265-L265: set a concrete v1spot_trade.notionaland assert it.tests/client/chain/stream_grpc/test_chain_grpc_chain_stream.py#L674-L674: set a concrete v2spot_trade.notionaland assert it.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/client/chain/stream_grpc/test_chain_grpc_chain_stream.py` at line 265, Update both stream fixtures in tests/client/chain/stream_grpc/test_chain_grpc_chain_stream.py: in the v1 fixture around lines 265 and the v2 fixture around line 674, assign concrete non-default values to each SpotTrade object's notional field and assert those same values in the expected results. Ensure both versions validate conversion of populated notional values rather than the protobuf default.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/client/indexer/grpc/test_indexer_grpc_derivative_api.py`:
- Around line 768-771: Update the position fixtures in
tests/client/indexer/grpc/test_indexer_grpc_derivative_api.py:768-771 and
tests/client/indexer/stream_grpc/test_indexer_grpc_derivative_stream.py:876-879
to assign distinct non-default values to initialLeverage, initialMargin,
initialEntryPrice, and initialQuantity, ensuring both derivative API and stream
tests validate propagation and serialization.
---
Nitpick comments:
In `@tests/client/chain/stream_grpc/test_chain_grpc_chain_stream.py`:
- Line 265: Update both stream fixtures in
tests/client/chain/stream_grpc/test_chain_grpc_chain_stream.py: in the v1
fixture around lines 265 and the v2 fixture around line 674, assign concrete
non-default values to each SpotTrade object's notional field and assert those
same values in the expected results. Ensure both versions validate conversion of
populated notional values rather than the protobuf default.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 2486631a-a712-467c-a033-ae6792b2b372
📒 Files selected for processing (16)
Makefilebuf.gen.yamlpyinjective/proto/exchange/injective_derivative_exchange_rpc_pb2.pypyinjective/proto/exchange/injective_tc_derivatives_rpc_pb2.pypyinjective/proto/exchange/injective_tc_derivatives_rpc_pb2_grpc.pypyinjective/proto/injective/exchange/v1beta1/exchange_pb2.pypyinjective/proto/injective/exchange/v1beta1/tx_pb2.pypyinjective/proto/injective/exchange/v2/exchange_pb2.pypyinjective/proto/injective/exchange/v2/tx_pb2.pypyinjective/proto/injective/exchange/v2/tx_pb2_grpc.pypyinjective/proto/injective/stream/v1beta1/query_pb2.pypyinjective/proto/injective/stream/v2/query_pb2.pypyproject.tomltests/client/chain/stream_grpc/test_chain_grpc_chain_stream.pytests/client/indexer/grpc/test_indexer_grpc_derivative_api.pytests/client/indexer/stream_grpc/test_indexer_grpc_derivative_stream.py
| "initialLeverage": position.initial_leverage, | ||
| "initialMargin": position.initial_margin, | ||
| "initialEntryPrice": position.initial_entry_price, | ||
| "initialQuantity": position.initial_quantity, |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Use non-default values for the new position fields.
Both tests currently assert protobuf defaults, so they do not reliably detect incorrect propagation or serialization.
tests/client/indexer/grpc/test_indexer_grpc_derivative_api.py#L768-L771: initialize all fourinitial_*fields with distinct values.tests/client/indexer/stream_grpc/test_indexer_grpc_derivative_stream.py#L876-L879: initialize the same four fields in the stream fixture.
📍 Affects 2 files
tests/client/indexer/grpc/test_indexer_grpc_derivative_api.py#L768-L771(this comment)tests/client/indexer/stream_grpc/test_indexer_grpc_derivative_stream.py#L876-L879
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/client/indexer/grpc/test_indexer_grpc_derivative_api.py` around lines
768 - 771, Update the position fixtures in
tests/client/indexer/grpc/test_indexer_grpc_derivative_api.py:768-771 and
tests/client/indexer/stream_grpc/test_indexer_grpc_derivative_stream.py:876-879
to assign distinct non-default values to initialLeverage, initialMargin,
initialEntryPrice, and initialQuantity, ensuring both derivative API and stream
tests validate propagation and serialization.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
.github/workflows/pre-commit.yml (1)
18-18: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse one exact, tested Poetry version across all workflows.
The
>=2.4,<3range permits tooling behavior to change between runs. Define a shared exact version and use it in pre-commit, test, and release workflows to keep validation and published artifacts reproducible.
.github/workflows/pre-commit.yml#L18-L18: pin the Poetry version used by pre-commit..github/workflows/release.yml#L17-L17: use the same pinned Poetry version for publishing..github/workflows/run-tests.yml#L30-L30: use the same pinned Poetry version for tests.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/pre-commit.yml at line 18, Define one shared exact, tested Poetry version and replace the version range in the install steps at .github/workflows/pre-commit.yml lines 18-18, .github/workflows/release.yml lines 17-17, and .github/workflows/run-tests.yml lines 30-30 with that same pinned version, ensuring all workflows use identical reproducible tooling.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/pre-commit.yml:
- Around line 20-24: Update the cache key in the cache-venv step to include the
resolved Python version configured by setup-python, ensuring virtual-environment
caches are separated by interpreter version while preserving the existing OS and
dependency-file hash components.
In @.github/workflows/release.yml:
- Around line 14-17: Set an explicit supported interpreter in the “Install
Python” setup-python step by adding the python-version configuration; do not
rely on the runner PATH or repository defaults. Keep the existing Poetry
installation step unchanged.
---
Nitpick comments:
In @.github/workflows/pre-commit.yml:
- Line 18: Define one shared exact, tested Poetry version and replace the
version range in the install steps at .github/workflows/pre-commit.yml lines
18-18, .github/workflows/release.yml lines 17-17, and
.github/workflows/run-tests.yml lines 30-30 with that same pinned version,
ensuring all workflows use identical reproducible tooling.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 61e05fb9-e087-404e-9f71-c7a72e50dd96
📒 Files selected for processing (4)
.github/workflows/pre-commit.yml.github/workflows/release.yml.github/workflows/run-tests.ymlpyproject.toml
🚧 Files skipped from review as they are similar to previous changes (1)
- pyproject.toml
| id: cache-venv | ||
| uses: actions/cache@v4 | ||
| with: | ||
| path: ./.venv | ||
| key: ${{ runner.os }}-venv-${{ hashFiles('**/poetry.lock') }} | ||
| key: ${{ runner.os }}-venv-${{ hashFiles('**/poetry.lock', '**/pyproject.toml', '**/poetry.toml') }} |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
sed -n '1,120p' .github/workflows/pre-commit.yml | cat -nRepository: InjectiveLabs/sdk-python
Length of output: 1314
🏁 Script executed:
cat -n .github/workflows/pre-commit.ymlRepository: InjectiveLabs/sdk-python
Length of output: 1314
🏁 Script executed:
python3 - <<'PY'
from pathlib import Path
p = Path('.github/workflows/pre-commit.yml')
print(p.exists())
if p.exists():
print(p.read_text())
PYRepository: InjectiveLabs/sdk-python
Length of output: 1089
Include the resolved Python version in the .venv cache key. The current key is shared across all versions that satisfy >=3.9 <3.12, so a cache built under one interpreter can be restored under another. Add the resolved setup-python version to the key, or pin the workflow to an exact version.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/pre-commit.yml around lines 20 - 24, Update the cache key
in the cache-venv step to include the resolved Python version configured by
setup-python, ensuring virtual-environment caches are separated by interpreter
version while preserving the existing OS and dependency-file hash components.
| - name: Install Python | ||
| uses: actions/setup-python@v5 | ||
| - name: Install poetry | ||
| run: python -m pip install poetry | ||
| run: python -m pip install "poetry>=2.4,<3" |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== release workflow excerpt ==="
sed -n '1,80p' .github/workflows/release.yml
echo
echo "=== python version declarations ==="
rg -n --hidden --glob '!**/.git/**' -e 'python-version|requires-python|tool\.poetry\.dependencies|Programming Language :: Python ::' pyproject.toml poetry.lock .github/workflows README* .python-version .tool-versions setup.cfg setup.py 2>/dev/null || true
echo
echo "=== candidate config files ==="
fd -H -a 'pyproject.toml|poetry.lock|.python-version|.tool-versions|setup.cfg|setup.py' .Repository: InjectiveLabs/sdk-python
Length of output: 5335
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== pyproject.toml excerpt ==="
sed -n '1,120p' pyproject.toml
echo
echo "=== workflow files using setup-python ==="
rg -n "actions/setup-python@|python-version:" .github/workflowsRepository: InjectiveLabs/sdk-python
Length of output: 2904
🌐 Web query:
actions/setup-python v5 default python-version omitted behavior documentation
💡 Result:
In actions/setup-python v5, the python-version input is optional [1][2]. If this input is omitted, the action follows a specific fallback behavior to determine which Python version to use: 1. File-based resolution: The action first attempts to read the version from a.python-version file in the repository root [1][3]. 2. PATH fallback: If no.python-version file exists, the action will use the Python or PyPy version currently available in the PATH [1][4]. Because the default version present in the PATH can vary between different runner images and may change unexpectedly over time, the official documentation strongly recommends that you always explicitly specify a Python version using the python-version or python-version-file inputs to ensure build consistency [1][5].
Citations:
- 1: https://github.com/actions/setup-python
- 2: https://git.homeinfra.org/actions/setup-python/src/branch/releases/v5/README.md
- 3: https://deepwiki.com/actions/setup-python/5-python-version-management
- 4: https://gitea.rs.h3.ink/actions/setup-python/src/tag/v5.4.0/README.md
- 5: https://github.com/actions/setup-python/blob/a26af69b/README.md
Pin the release Python version. .github/workflows/release.yml:14-17 falls back to the runner PATH here, and there’s no .python-version file in the repo, so release builds can drift with the hosted image. Set python-version explicitly to a supported release interpreter.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/release.yml around lines 14 - 17, Set an explicit
supported interpreter in the “Install Python” setup-python step by adding the
python-version configuration; do not rely on the runner PATH or repository
defaults. Keep the existing Poetry installation step unchanged.
Reflect chain changes for a new sdk-python release (v1.16.1)
Summary by CodeRabbit
Marketsretrieval to the derivatives API.notionalvalues to spot trade data.1.16.1and tightened Poetry version constraints in CI/package builds.