[IC-1055] create-python-sdk-final-release-for-v120-testnet#415
Conversation
…0-alpha.3 and injective-indexer v1.19.41. Generated the MAINTAINERS.md file with documentation of the actions to execute when updating the SDK for a new chain version.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughAdds MsgBatchLiquidatePositions and Composer enum helpers with flexible resolvers, extends market builders with cross-margin flags, regenerates many protobuf/grpc bindings for updated core/indexer, adds maintainer docs, examples, and tests; bumps version to 1.15.0-rc1. ChangesBatch Liquidation and Cross-Margin Eligibility Release
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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 `@CHANGELOG.md`:
- Line 11: Update the CHANGELOG entry so the version string matches the intended
proto targets: replace the current line "Updated all compiled protos for
compatibility with Injective core v1.20.0 and Indexer v1.19.41ß" by correcting
the typo and exact versions to read "Updated all compiled protos for
compatibility with Injective core v1.20.0 and Indexer v1.19.41" (remove the
stray ß and ensure both version numbers match the PR description).
In `@examples/chain_client/exchange/31_MsgBatchLiquidatePositions.py`:
- Around line 17-18: The code reads INJECTIVE_PRIVATE_KEY into
private_key_in_hexa without validating it; add an explicit presence check
immediately after the getenv (for variable private_key_in_hexa) and fail fast
with a clear error (raise a RuntimeError or call sys.exit(1) with a message like
"INJECTIVE_PRIVATE_KEY environment variable is required") so the script stops
with an actionable message; apply the same presence check pattern for the other
similar getenv usages around the 30-33 and 38 lines in this example.
In `@MAINTAINERS.md`:
- Line 17: Update the MAINTAINERS.md table row that currently reads "Python
3.9+" to match the project's actual supported Python range: locate the table
cell containing "`Python 3.9+`" and replace it with the supported version
specifier found in the project's configuration (e.g., pyproject.toml /
runtime.txt / CI matrix). Ensure the runtime column entry remains the same and
only the Python version cell is updated so the table accurately reflects the
project's current interpreter requirement.
In `@tests/test_composer_v2.py`:
- Around line 54-56: The failing E501 is due to an overly long assert comparing
Composer.CROSS_MARGIN_ELIGIBILITY[name].value to
injective_proposal_v2_pb.CrossMarginEligibility.Value(name); to fix, split the
long line into two by assigning one or both sides to short local variables
(e.g., left = Composer.CROSS_MARGIN_ELIGIBILITY[name].value and right =
injective_proposal_v2_pb.CrossMarginEligibility.Value(name)) inside the same
test function test_order_type_enum_accepted_for_order_type_param, then assert
left == right so the line length stays under 120 chars and pre-commit lint
passes.
- Around line 30-37: The test test_permissions_action_enum_mirrors_proto relies
on iteration order when comparing [m.name for m in Composer.PERMISSIONS_ACTION]
to proto_non_zero_keys; instead, build normalized mappings (e.g., dicts) of
non-zero proto names to values and of Composer.PERMISSIONS_ACTION names to
values (filter out zero and handle aliases) and assert equality of these dicts
so the comparison is order-independent; reference Composer.PERMISSIONS_ACTION
and permissions_pb.Action when locating and replacing the list-based comparisons
with dict-based comparisons.
🪄 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: de7e1eaf-cece-46ed-95b8-3bccff6540e5
📒 Files selected for processing (38)
CHANGELOG.mdMAINTAINERS.mdMakefileREADME.mdbuf.gen.yamlexamples/chain_client/exchange/25_MsgUpdateDerivativeMarket.pyexamples/chain_client/exchange/31_MsgBatchLiquidatePositions.pyexamples/chain_client/exchange/4_MsgInstantPerpetualMarketLaunch.pyexamples/chain_client/exchange/5_MsgInstantExpiryFuturesMarketLaunch.pypyinjective/composer_v2.pypyinjective/proto/exchange/injective_rfq_gw_rpc_pb2.pypyinjective/proto/exchange/injective_rfq_gw_rpc_pb2_grpc.pypyinjective/proto/exchange/injective_rfq_rpc_pb2.pypyinjective/proto/exchange/injective_rfq_rpc_pb2_grpc.pypyinjective/proto/exchange/injective_tc_derivatives_rpc_pb2.pypyinjective/proto/injective/exchange/v1beta1/events_pb2.pypyinjective/proto/injective/exchange/v2/events_pb2.pypyinjective/proto/injective/exchange/v2/exchange_pb2.pypyinjective/proto/injective/exchange/v2/genesis_pb2.pypyinjective/proto/injective/exchange/v2/market_pb2.pypyinjective/proto/injective/exchange/v2/proposal_pb2.pypyinjective/proto/injective/exchange/v2/query_pb2.pypyinjective/proto/injective/exchange/v2/query_pb2_grpc.pypyinjective/proto/injective/exchange/v2/tx_pb2.pypyinjective/proto/injective/exchange/v2/tx_pb2_grpc.pypyinjective/proto/injective/oracle/v1beta1/events_pb2.pypyinjective/proto/injective/oracle/v1beta1/genesis_pb2.pypyinjective/proto/injective/oracle/v1beta1/oracle_pb2.pypyinjective/proto/injective/oracle/v1beta1/query_pb2.pypyinjective/proto/injective/oracle/v1beta1/query_pb2_grpc.pypyinjective/proto/injective/oracle/v1beta1/tx_pb2.pypyinjective/proto/injective/oracle/v1beta1/tx_pb2_grpc.pypyinjective/proto/injective/peggy/v1/params_pb2.pypyinjective/proto/injective/stream/v2/query_pb2.pypyproject.tomltests/client/chain/grpc/test_chain_grpc_exchange_v2_api.pytests/client/chain/stream_grpc/test_chain_grpc_chain_stream.pytests/test_composer_v2.py
💤 Files with no reviewable changes (1)
- pyinjective/proto/exchange/injective_rfq_rpc_pb2_grpc.py
| private_key_in_hexa = os.getenv("INJECTIVE_PRIVATE_KEY") | ||
|
|
There was a problem hiding this comment.
Add an explicit private key presence check before use.
If INJECTIVE_PRIVATE_KEY is unset, this example fails later with a less actionable error. Fail fast with a clear message.
Proposed fix
async def main() -> None:
dotenv.load_dotenv()
private_key_in_hexa = os.getenv("INJECTIVE_PRIVATE_KEY")
+ if not private_key_in_hexa:
+ raise ValueError("INJECTIVE_PRIVATE_KEY is not set")Also applies to: 30-33, 38-38
🤖 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 `@examples/chain_client/exchange/31_MsgBatchLiquidatePositions.py` around lines
17 - 18, The code reads INJECTIVE_PRIVATE_KEY into private_key_in_hexa without
validating it; add an explicit presence check immediately after the getenv (for
variable private_key_in_hexa) and fail fast with a clear error (raise a
RuntimeError or call sys.exit(1) with a message like "INJECTIVE_PRIVATE_KEY
environment variable is required") so the script stops with an actionable
message; apply the same presence check pattern for the other similar getenv
usages around the 30-33 and 38 lines in this example.
| def test_permissions_action_enum_mirrors_proto(self): | ||
| # IntFlag excludes zero-valued members from iteration (zero means "no flags set"), | ||
| # so we compare only the non-zero proto keys. | ||
| proto_non_zero_keys = [n for n in permissions_pb.Action.keys() if permissions_pb.Action.Value(n) != 0] | ||
| assert [m.name for m in Composer.PERMISSIONS_ACTION] == proto_non_zero_keys | ||
| for name in proto_non_zero_keys: | ||
| assert Composer.PERMISSIONS_ACTION[name].value == permissions_pb.Action.Value(name) | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
python - <<'PY'
from pyinjective.composer_v2 import Composer
from pyinjective.proto.injective.permissions.v1beta1 import permissions_pb2 as permissions_pb
print("Composer iteration:", [m.name for m in Composer.PERMISSIONS_ACTION])
print("Proto keys:", list(permissions_pb.Action.keys()))
print("Proto non-zero:", [n for n in permissions_pb.Action.keys() if permissions_pb.Action.Value(n) != 0])
members = Composer.PERMISSIONS_ACTION.__members__
normalized = {
n: m.value
for n, m in members.items()
if m.value != 0 and m.name == n
}
print("Composer normalized non-zero non-alias:", normalized)
PYRepository: InjectiveLabs/sdk-python
Length of output: 585
🏁 Script executed:
# First, locate and examine the test file
find . -name "test_composer_v2.py" -type fRepository: InjectiveLabs/sdk-python
Length of output: 96
🏁 Script executed:
# Find the Composer class definition
find . -name "composer_v2.py" -o -name "composer.py" | head -20Repository: InjectiveLabs/sdk-python
Length of output: 97
🏁 Script executed:
# Search for PERMISSIONS_ACTION in the codebase
rg "PERMISSIONS_ACTION" --type py -A 5 -B 5Repository: InjectiveLabs/sdk-python
Length of output: 13318
🏁 Script executed:
# Read the test file to see the full test context
cat -n tests/test_composer_v2.py | head -50Repository: InjectiveLabs/sdk-python
Length of output: 2606
🏁 Script executed:
# Search for imports in the test file to understand what permissions_pb is
rg "permissions_pb|import" tests/test_composer_v2.py | head -20Repository: InjectiveLabs/sdk-python
Length of output: 1265
🏁 Script executed:
# Look at the proto file to understand the Action enum
find . -name "*permissions*" -type f -path "*/proto/*" | grep -E "\.(proto|py)$"Repository: InjectiveLabs/sdk-python
Length of output: 211
🏁 Script executed:
# Check if there are Python proto files
rg "class Action" --type py | head -20Repository: InjectiveLabs/sdk-python
Length of output: 50
🏁 Script executed:
# Examine the proto generated file to understand Action enum
rg "Action = " pyinjective/proto/injective/permissions/v1beta1/permissions_pb2.py -A 50 | head -100Repository: InjectiveLabs/sdk-python
Length of output: 50
🏁 Script executed:
# Check the structure of the proto Action enum
cat pyinjective/proto/injective/permissions/v1beta1/permissions_pb2.py | head -200 | tail -100Repository: InjectiveLabs/sdk-python
Length of output: 5430
🏁 Script executed:
# Find the Action enum definition more specifically
grep -n "class Action\|Action = " pyinjective/proto/injective/permissions/v1beta1/permissions_pb2.py | head -20Repository: InjectiveLabs/sdk-python
Length of output: 50
🏁 Script executed:
# Get the full proto file to see all enum members
cat pyinjective/proto/injective/permissions/v1beta1/permissions_pb2.py | tail -100Repository: InjectiveLabs/sdk-python
Length of output: 5430
🏁 Script executed:
# Search for how Action enum members are defined
rg "UNSPECIFIED|MINT|RECEIVE|BURN" pyinjective/proto/injective/permissions/v1beta1/permissions_pb2.pyRepository: InjectiveLabs/sdk-python
Length of output: 3013
🏁 Script executed:
# Look at the composer_v2.py file more closely to see the IntFlag definition
cat -n pyinjective/composer_v2.py | grep -A 10 -B 5 "PERMISSIONS_ACTION"Repository: InjectiveLabs/sdk-python
Length of output: 662
🏁 Script executed:
# Get full context of the Composer PERMISSIONS_ACTION definition and how it's used
wc -l pyinjective/composer_v2.pyRepository: InjectiveLabs/sdk-python
Length of output: 100
🏁 Script executed:
# Read more context around PERMISSIONS_ACTION definition
sed -n '300,330p' pyinjective/composer_v2.pyRepository: InjectiveLabs/sdk-python
Length of output: 1104
🏁 Script executed:
# Check imports in composer_v2.py to understand IntFlag usage
head -50 pyinjective/composer_v2.pyRepository: InjectiveLabs/sdk-python
Length of output: 2680
🏁 Script executed:
# Check if there could be alias issues with matching proto structure
python3 << 'PYTHON'
# Verify enum iteration semantics
from enum import IntFlag
# Create IntFlag similar to the test scenario
test_enum = IntFlag('TEST_ENUM', [
('ZERO', 0),
('ONE', 1),
('TWO', 2),
('FOUR', 4),
])
print("IntFlag iteration (should exclude ZERO):")
print([m.name for m in test_enum])
print("\nIntFlag members (includes ZERO):")
print([m.name for m in test_enum.__members__])
print("\nDirect __members__ dict:")
print({n: m.value for n, m in test_enum.__members__.items()})
PYTHONRepository: InjectiveLabs/sdk-python
Length of output: 352
🏁 Script executed:
# Check the test file for any additional context about the failure
grep -n "def test_permissions" tests/test_composer_v2.py -A 15Repository: InjectiveLabs/sdk-python
Length of output: 1082
🏁 Script executed:
# Verify the similar tests to understand the pattern
grep -n "def test_.*_enum_mirrors_proto" tests/test_composer_v2.py -A 5Repository: InjectiveLabs/sdk-python
Length of output: 1599
🏁 Script executed:
# Test the actual enum behavior with dict comparison
python3 << 'PYTHON'
from enum import IntFlag
# Simulate the enum creation pattern from the code
members = [
('ZERO', 0),
('ONE', 1),
('TWO', 2),
('FOUR', 4),
]
test_enum = IntFlag('TEST_ENUM', members)
print("Iteration order:")
print([m.name for m in test_enum])
print("\nMembers dict keys:")
print(list(test_enum.__members__.keys()))
print("\nNormalized non-zero non-alias (proposed approach):")
normalized = {
n: m.value
for n, m in test_enum.__members__.items()
if m.value != 0 and m.name == n
}
print(normalized)
print("\nComparison of iteration vs normalized:")
print("Iteration matches list order?", [m.name for m in test_enum] == ['ONE', 'TWO', 'FOUR'])
PYTHONRepository: InjectiveLabs/sdk-python
Length of output: 318
Refactor the test to compare normalized enum members rather than iteration order.
The current test relies on iteration order matching between proto keys and IntFlag iteration, which is fragile and order-dependent. IntFlag iteration excludes zero-valued members correctly, but comparing iteration results directly is brittle. Use normalized dict comparison instead to make the test resilient to iteration order differences and to explicitly handle zero filtering and alias members.
Proposed fix
def test_permissions_action_enum_mirrors_proto(self):
- # IntFlag excludes zero-valued members from iteration (zero means "no flags set"),
- # so we compare only the non-zero proto keys.
- proto_non_zero_keys = [n for n in permissions_pb.Action.keys() if permissions_pb.Action.Value(n) != 0]
- assert [m.name for m in Composer.PERMISSIONS_ACTION] == proto_non_zero_keys
- for name in proto_non_zero_keys:
- assert Composer.PERMISSIONS_ACTION[name].value == permissions_pb.Action.Value(name)
+ proto_non_zero = {
+ name: permissions_pb.Action.Value(name)
+ for name in permissions_pb.Action.keys()
+ if permissions_pb.Action.Value(name) != 0
+ }
+ composer_non_zero = {
+ name: member.value
+ for name, member in Composer.PERMISSIONS_ACTION.__members__.items()
+ if member.value != 0 and member.name == name
+ }
+ assert composer_non_zero == proto_non_zero🧰 Tools
🪛 GitHub Actions: pre-commit / 0_pre-commit.txt
[error] pre-commit hook 'black' failed; files were reformatted (tests/test_composer_v2.py was modified). Run black to apply formatting changes.
🪛 GitHub Actions: pre-commit / pre-commit
[warning] pre-commit hook 'black' failed because it modified files (reformat required). Changes applied to tests/test_composer_v2.py.
🪛 GitHub Actions: run-tests / 7_run-tests (3.10, ubuntu-latest).txt
[error] 34-34: Test failed: TestComposer.test_permissions_action_enum_mirrors_proto. AssertionError: Composer.PERMISSIONS_ACTION enum ordering/contents do not match proto_non_zero_keys (unexpected 'UNSPECIFIED' at index 0; left contains extra item 'MODIFY_ROLE_MANAGERS').
🪛 GitHub Actions: run-tests / run-tests (3.10, ubuntu-latest)
[error] 34-34: Test failed: TestComposer.test_permissions_action_enum_mirrors_proto assertion mismatch. Expected Composer.PERMISSIONS_ACTION enum names (excluding proto non-zero keys) to match proto Non-zero Action keys; diff shows 'UNSPECIFIED' present at index 0 and 'MODIFY_ROLE_MANAGERS' count mismatch.
🤖 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/test_composer_v2.py` around lines 30 - 37, The test
test_permissions_action_enum_mirrors_proto relies on iteration order when
comparing [m.name for m in Composer.PERMISSIONS_ACTION] to proto_non_zero_keys;
instead, build normalized mappings (e.g., dicts) of non-zero proto names to
values and of Composer.PERMISSIONS_ACTION names to values (filter out zero and
handle aliases) and assert equality of these dicts so the comparison is
order-independent; reference Composer.PERMISSIONS_ACTION and
permissions_pb.Action when locating and replacing the list-based comparisons
with dict-based comparisons.
Summary by CodeRabbit