Merged
Conversation
…ory/trade-history/orders)
📝 WalkthroughWalkthroughThis pull request adds comprehensive support for TC Abacus and TC Derivatives services to the Injective SDK. It introduces new gRPC API clients, streaming clients, type definitions, transformer modules, proto package generation setup, and detailed workflow documentation for proto file synchronization across multiple proto packages. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
📝 Coding Plan
Comment Tip You can customize the tone of the review comments and chat replies.Configure the |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/sdk-ts/tsdown.config.ts (1)
40-45:⚠️ Potential issue | 🟡 MinorAdd
@injectivelabs/tc-abacus-proto-ts-v2to the external packages array.The
tcAbacusclient module uses@injectivelabs/tc-abacus-proto-ts-v2, which is defined as a dependency inpackage.json. To maintain consistency with other proto packages (e.g.,@injectivelabs/abacus-proto-ts-v2,@injectivelabs/core-proto-ts-v2), this package should be added to the external array in the proto packages section around line 45.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/sdk-ts/tsdown.config.ts` around lines 40 - 45, Add the missing proto package to the external proto packages array by inserting '@injectivelabs/tc-abacus-proto-ts-v2' into the list alongside the other entries (e.g., next to '@injectivelabs/abacus-proto-ts-v2' and '@injectivelabs/core-proto-ts-v2') so the array that exports external proto packages includes the tc-abacus proto package used by the tcAbacus client.
🧹 Nitpick comments (8)
protoV2/README.md (1)
106-106: AddtcAbacusto the runnable command/workflow examples too.Good addition in the tree, but the procedural sections still omit this package (e.g., Quick Start generate commands and “simple packages” list). Please update those lists so the docs stay internally consistent.
Suggested doc patch
# Generate specific package cd core && npm run generate cd indexer && npm run generate cd abacus && npm run generate cd mito && npm run generate cd olp && npm run generate +cd tcAbacus && npm run generate-# For simple packages (indexer, abacus, mito, olp) +# For simple packages (indexer, abacus, mito, olp, tcAbacus) ./protoV2/_scripts/update-index-template.sh indexer🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@protoV2/README.md` at line 106, Update the README to include the new tcAbacus package in all runnable examples and lists: add tcAbacus to the project tree examples, the Quick Start generate command examples, and the “simple packages” list so the docs match the new package; specifically edit the sections that show runnable commands and package lists (e.g., the Quick Start generate commands block and the “simple packages” list) to include the identifier tcAbacus alongside the existing packages.packages/sdk-ts/src/utils/ofac.ts (1)
2-2: Add an integrity test for this list (dupes + format).Since this is compliance-sensitive data updated manually, a small unit test that asserts
0x[0-9a-fA-F]{40}format and no duplicates would prevent accidental regressions in future edits.Also applies to: 43-44, 56-56, 60-60, 66-66, 71-72, 90-91
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/sdk-ts/src/utils/ofac.ts` at line 2, Add a unit test that imports the exported address list(s) from packages/sdk-ts/src/utils/ofac.ts (the array containing entries like "0x0330070fd38ec3bb94f58fa55d40368271e9e54a") and asserts every entry matches the regex /^0x[0-9a-fA-F]{40}$/ and that there are no duplicates (compare length of array to length of new Set(array)); apply this test to all exported lists in ofac.ts so changes to any of those arrays (the entries around the referenced line ranges) will fail the test if format or duplication issues are introduced.docs/workflows/align-proto-workflow.md (2)
9-17: Consider adding language specifiers to fenced code blocks.Several code blocks in this documentation are missing language specifiers (e.g., lines 9-11, 15-17). Adding language identifiers improves syntax highlighting and linting compliance.
📝 Example fix for quick-start code blocks
-``` +```bash /align-proto</details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against the current code and only fix it if needed.
In
@docs/workflows/align-proto-workflow.mdaround lines 9 - 17, Update the
fenced code blocks that show the CLI examples (the blocks containing
"/align-proto" and "/align-proto --check-only") to include a language specifier
(e.g., "bash") so syntax highlighting and linting are enabled; locate the two
code fences in the align-proto examples in the documentation (the blocks around
the "/align-proto" and "/align-proto --check-only" lines) and change their
opening triple backticks to include the language identifier.</details> --- `107-112`: **Add blank lines around fenced code blocks.** Markdown best practices recommend surrounding fenced code blocks with blank lines for consistent rendering. <details> <summary>📝 Proposed fix</summary> ```diff - **Important**: If you modified `@injectivelabs/exceptions` (e.g., added IndexerModule or AbacusModule), build it first: + ```bash cd packages/exceptions && pnpm build
- AI runs
pnpm type-checkinpackages/sdk-ts</details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against the current code and only fix it if needed.
In
@docs/workflows/align-proto-workflow.mdaround lines 107 - 112, The fenced
code block containing the command "cd packages/exceptions && pnpm build" needs
blank lines before and after it for proper Markdown rendering; update the
Markdown around that fenced block so there is an empty line above the opening
bash and an empty line after the closingso the following line "AI runs
pnpm type-checkinpackages/sdk-ts" is separated from the code block.</details> </blockquote></details> <details> <summary>packages/sdk-ts/src/client/tcAbacus/grpc/TcAbacusGrpcApi.spec.ts (1)</summary><blockquote> `4-8`: **Consider using environment variables for test configuration.** Hardcoded mainnet URL and address may cause flaky tests in CI environments without network access. Consider using environment variables or mocking the gRPC client. ```diff -const injectiveAddress = 'inj1995xnrrtnmtdgjmx0g937vf28dwefhkhy6gy5e' - -const tcAbacusGrpcApi = new TcAbacusGrpcApi( - 'https://k8s.mainnet.eu.tc-abacus.injective.network/grpc', -) +const injectiveAddress = process.env.TEST_INJECTIVE_ADDRESS ?? 'inj1995xnrrtnmtdgjmx0g937vf28dwefhkhy6gy5e' + +const tcAbacusGrpcApi = new TcAbacusGrpcApi( + process.env.TC_ABACUS_GRPC_URL ?? 'https://k8s.mainnet.eu.tc-abacus.injective.network/grpc', +) ``` <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@packages/sdk-ts/src/client/tcAbacus/grpc/TcAbacusGrpcApi.spec.ts` around lines 4 - 8, Tests currently hardcode a mainnet URL and address (injectiveAddress and the TcAbacusGrpcApi instantiation), which can cause CI flakiness; update the spec to read the gRPC endpoint and address from environment variables (e.g., process.env.TC_ABACUS_GRPC_URL and process.env.INJECTIVE_ADDRESS) with sensible test-only fallbacks or skip conditions, or replace the real TcAbacusGrpcApi instantiation with a mocked/fake client (mock the class or its methods) so tests do not perform network calls; reference injectiveAddress and new TcAbacusGrpcApi(...) when implementing the env-driven config or the mock. ``` </details> </blockquote></details> <details> <summary>packages/sdk-ts/src/client/indexer/grpc/IndexerGrpcTcDerivativesApi.ts (1)</summary><blockquote> `35-37`: **Use an explicit undefined check for `perPage`.** `if (perPage)` will skip zero values, while `withCount` at line 112 already uses `!== undefined` for consistency. Since `perPage` is typed as `number | undefined`, switching these four guards to `perPage !== undefined` ensures zero is preserved if it's a valid RPC parameter. <details> <summary>💡 Suggested change</summary> ```diff - if (perPage) { + if (perPage !== undefined) { request.perPage = perPage } ``` Apply the same guard in `fetchOrdersHistory`, `fetchTradesHistory`, `fetchPositions`, and `fetchOrders` (lines 35–37, 71–73, 116–118, 152–154). </details> <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@packages/sdk-ts/src/client/indexer/grpc/IndexerGrpcTcDerivativesApi.ts` around lines 35 - 37, Summary: Use an explicit undefined check for perPage to preserve zero values. In the four methods fetchOrdersHistory, fetchTradesHistory, fetchPositions, and fetchOrders replace the truthy guard `if (perPage)` with an explicit `perPage !== undefined` check so a value of 0 is not treated as absent; do this for each occurrence where request.perPage is set, mirroring the `withCount !== undefined` pattern used elsewhere. ``` </details> </blockquote></details> <details> <summary>packages/sdk-ts/src/client/indexer/transformers/IndexerGrpcTcDerivativesTransformer.ts (1)</summary><blockquote> `20-25`: **`zeroPositionDelta` may become unused after fixing the spread issue.** If the type mismatch fix above is applied with `undefined` for missing position deltas, this helper function will become dead code. Consider removing it, or if you intend to always provide default values, keep it and use it properly with the nested `positionDelta` property. <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@packages/sdk-ts/src/client/indexer/transformers/IndexerGrpcTcDerivativesTransformer.ts` around lines 20 - 25, The helper zeroPositionDelta is likely to become dead code if you change position delta handling to allow undefined; either remove zeroPositionDelta entirely or ensure it's used to populate the nested positionDelta property where defaults are required (e.g., inside the transformer that builds or spreads the PositionDelta object), keeping consistent types with TradeDirection and execution* fields; update any references in IndexerGrpcTcDerivativesTransformer to either call zeroPositionDelta() for missing positionDelta or delete the function and adjust callers to accept undefined. ``` </details> </blockquote></details> <details> <summary>packages/sdk-ts/src/client/indexer/grpc_stream/streamV2/IndexerGrpcTcDerivativesStreamV2.ts (1)</summary><blockquote> `33-39`: **Consider making `transport` a local variable.** The `transport` field is stored but never accessed after the constructor. If there's no plan to expose it or use it elsewhere, it could be a local variable. <details> <summary>♻️ Optional simplification</summary> ```diff export class IndexerGrpcTcDerivativesStreamV2 { private client: InjectiveTCDerivativesRPCClient - private transport: GrpcWebRpcTransport constructor(endpoint: string, metadata?: Record<string, string>) { - this.transport = new GrpcWebRpcTransport(endpoint, metadata) - this.client = new InjectiveTCDerivativesRPCClient(this.transport) + const transport = new GrpcWebRpcTransport(endpoint, metadata) + this.client = new InjectiveTCDerivativesRPCClient(transport) } ``` </details> However, if you anticipate needing access to the transport for future features (e.g., closing connections, changing metadata), keeping it as a field is fine. <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@packages/sdk-ts/src/client/indexer/grpc_stream/streamV2/IndexerGrpcTcDerivativesStreamV2.ts` around lines 33 - 39, The class IndexerGrpcTcDerivativesStreamV2 declares a private transport field that is only assigned in the constructor and never used elsewhere; remove the private transport property and instead create a local const transport inside the constructor, e.g., const transport = new GrpcWebRpcTransport(endpoint, metadata), then pass that local transport into new InjectiveTCDerivativesRPCClient(transport); update the constructor accordingly and remove the unused private transport declaration to avoid dead state. ``` </details> </blockquote></details> </blockquote></details> <details> <summary>🤖 Prompt for all review comments with AI agents</summary>Verify each finding against the current code and only fix it if needed.
Inline comments:
In@packages/sdk-ts/package.json:
- Around line 325-326: The package.json exports are missing a subpath for the
new tcAbacus client, causing consumers to be unable to import
@injectivelabs/sdk-ts/client/tcAbacus; add an exports entry mirroring the
existing client entries (e.g., the patterns used for "./client/chain",
"./client/indexer", "./client/abacus", "./client/olp", "./client/wasm") but with
the key "./client/tcAbacus" and point it to the appropriate built file name
(tcAbacus) for both require and import targets so published packages resolve
correctly.In
@packages/sdk-ts/src/client/indexer/transformers/IndexerGrpcTcDerivativesTransformer.ts:
- Around line 74-100: In grpcTradeToTrade, the derived position delta is being
spread into the top-level trade object (adding tradeDirection, executionPrice,
etc.) instead of being nested; change the return to assign the
mappedPositionDelta to the positionDelta property (i.e., use positionDelta:
mappedPositionDelta or positionDelta: zeroPositionDelta() when absent) rather
than spreading it, so IndexerGrpcTcDerivativesTransformer.grpcTradeToTrade
returns a TcDerivativeTradeHistory with positionDelta properly nested.In
@packages/sdk-ts/src/client/tcAbacus/grpc/TcAbacusGrpcApi.spec.ts:
- Around line 11-26: The tests currently swallow failures by catching errors in
the test blocks (e.g., the fetchHealthCheck test calling
tcAbacusGrpcApi.fetchHealthCheck and similar tests) and only logging them, so
remove the outer try/catch (or re-throw the caught error) in each spec (tests
around TcAbacusGrpcApi.fetchHealthCheck and the other listed tests) so that
failing API calls cause the test to fail; alternatively, if these are
flaky/integration tests, mark them with describe.skip or test.skip instead of
suppressing errors.- Around line 16-22: The test currently asserts the response against itself
using expect.objectContaining(response), which always passes; instead compute an
explicit expected shape and assert against that: call
TcAbacusGrpcTransformer.grpcHealthCheckToHealthCheck with the original gRPC
input (or build an expected object) to produce an expected value, then replace
expect(response).toEqual(expect.objectContaining(response)) with asserting
response equals or contains the explicit expected object (e.g.,
expect(response).toEqual(expected) or
expect(response).toEqual(expect.objectContaining(expected))); reference
TcAbacusGrpcTransformer.grpcHealthCheckToHealthCheck and the test variable
response to locate the change.In
@protoV2/tcAbacus/gen.sh:
- Around line 35-38: The script currently sets abacus_branch=main and clones via
git clone (the git clone command), which makes generated code non-reproducible;
change the script to pin the upstream proto checkout to an immutable ref by
replacing the mutable branch reference with a specific tag or commit hash (e.g.,
set abacus_branch to a release tag or commit SHA) or accept a provided immutable
REF env var and use that in the git clone/checkout step so the clone always
resolves to a fixed commit rather than "main".In
@protoV2/tcAbacus/graphql.stub.proto:
- Around line 1-6: The proto file declares package danielvladco.protobuf.graphql
but lives in the wrong directory; move graphql.stub.proto into a directory
structure matching the package (create danielvladco/protobuf/graphql/ and place
graphql.stub.proto there) and then update the copy command in the gen.sh script
(the copy that references graphql.stub.proto) to point to the new location so
the build/lint rule PACKAGE_DIRECTORY_MATCH no longer fails.In
@protoV2/tcAbacus/publish.sh:
- Around line 15-38: The publish script computes current version into variable v
and a new patch version v1 but then ignores them and hardcodes npm version
1.18.1; update the publish step (after cd "$SCRIPT_DIR/proto-ts") to drive npm
version from the computed value (use v1) instead of the hardcoded literal so the
bump uses the single source of truth, and ensure the published package uses that
updated package.json version (references: variables v, v1 and the npm version
call).In
@protoV2/tcAbacus/src/index.d.ts:
- Around line 1-4: The declaration file exports (PointsSvcClient, PointsSvcPb)
but the actual runtime module (index.ts) exports InjectiveTCAbacusRPCClient and
InjectiveTCAbacusRPCPb; update the mismatched exports so TypeScript consumers
see the same names: either rename the exports in this declaration file to export
InjectiveTCAbacusRPCClient and InjectiveTCAbacusRPCPb (and adjust any re-exports
like GraphqlPb/DescriptorPb if needed) to match index.ts, or change index.ts to
export PointsSvcClient and PointsSvcPb to match this declaration—make sure the
unique symbols InjectiveTCAbacusRPCClient, InjectiveTCAbacusRPCPb,
PointsSvcClient, and PointsSvcPb are consistent between index.ts and index.d.ts.
Outside diff comments:
In@packages/sdk-ts/tsdown.config.ts:
- Around line 40-45: Add the missing proto package to the external proto
packages array by inserting '@injectivelabs/tc-abacus-proto-ts-v2' into the list
alongside the other entries (e.g., next to '@injectivelabs/abacus-proto-ts-v2'
and '@injectivelabs/core-proto-ts-v2') so the array that exports external proto
packages includes the tc-abacus proto package used by the tcAbacus client.
Nitpick comments:
In@docs/workflows/align-proto-workflow.md:
- Around line 9-17: Update the fenced code blocks that show the CLI examples
(the blocks containing "/align-proto" and "/align-proto --check-only") to
include a language specifier (e.g., "bash") so syntax highlighting and linting
are enabled; locate the two code fences in the align-proto examples in the
documentation (the blocks around the "/align-proto" and "/align-proto
--check-only" lines) and change their opening triple backticks to include the
language identifier.- Around line 107-112: The fenced code block containing the command "cd
packages/exceptions && pnpm build" needs blank lines before and after it for
proper Markdown rendering; update the Markdown around that fenced block so there
is an empty line above the openingbash and an empty line after the closingso the following line "AI runspnpm type-checkinpackages/sdk-ts" is
separated from the code block.In
@packages/sdk-ts/src/client/indexer/grpc_stream/streamV2/IndexerGrpcTcDerivativesStreamV2.ts:
- Around line 33-39: The class IndexerGrpcTcDerivativesStreamV2 declares a
private transport field that is only assigned in the constructor and never used
elsewhere; remove the private transport property and instead create a local
const transport inside the constructor, e.g., const transport = new
GrpcWebRpcTransport(endpoint, metadata), then pass that local transport into new
InjectiveTCDerivativesRPCClient(transport); update the constructor accordingly
and remove the unused private transport declaration to avoid dead state.In
@packages/sdk-ts/src/client/indexer/grpc/IndexerGrpcTcDerivativesApi.ts:
- Around line 35-37: Summary: Use an explicit undefined check for perPage to
preserve zero values. In the four methods fetchOrdersHistory,
fetchTradesHistory, fetchPositions, and fetchOrders replace the truthy guardif (perPage)with an explicitperPage !== undefinedcheck so a value of 0 is not
treated as absent; do this for each occurrence where request.perPage is set,
mirroring thewithCount !== undefinedpattern used elsewhere.In
@packages/sdk-ts/src/client/indexer/transformers/IndexerGrpcTcDerivativesTransformer.ts:
- Around line 20-25: The helper zeroPositionDelta is likely to become dead code
if you change position delta handling to allow undefined; either remove
zeroPositionDelta entirely or ensure it's used to populate the nested
positionDelta property where defaults are required (e.g., inside the transformer
that builds or spreads the PositionDelta object), keeping consistent types with
TradeDirection and execution* fields; update any references in
IndexerGrpcTcDerivativesTransformer to either call zeroPositionDelta() for
missing positionDelta or delete the function and adjust callers to accept
undefined.In
@packages/sdk-ts/src/client/tcAbacus/grpc/TcAbacusGrpcApi.spec.ts:
- Around line 4-8: Tests currently hardcode a mainnet URL and address
(injectiveAddress and the TcAbacusGrpcApi instantiation), which can cause CI
flakiness; update the spec to read the gRPC endpoint and address from
environment variables (e.g., process.env.TC_ABACUS_GRPC_URL and
process.env.INJECTIVE_ADDRESS) with sensible test-only fallbacks or skip
conditions, or replace the real TcAbacusGrpcApi instantiation with a mocked/fake
client (mock the class or its methods) so tests do not perform network calls;
reference injectiveAddress and new TcAbacusGrpcApi(...) when implementing the
env-driven config or the mock.In
@packages/sdk-ts/src/utils/ofac.ts:
- Line 2: Add a unit test that imports the exported address list(s) from
packages/sdk-ts/src/utils/ofac.ts (the array containing entries like
"0x0330070fd38ec3bb94f58fa55d40368271e9e54a") and asserts every entry matches
the regex /^0x[0-9a-fA-F]{40}$/ and that there are no duplicates (compare length
of array to length of new Set(array)); apply this test to all exported lists in
ofac.ts so changes to any of those arrays (the entries around the referenced
line ranges) will fail the test if format or duplication issues are introduced.In
@protoV2/README.md:
- Line 106: Update the README to include the new tcAbacus package in all
runnable examples and lists: add tcAbacus to the project tree examples, the
Quick Start generate command examples, and the “simple packages” list so the
docs match the new package; specifically edit the sections that show runnable
commands and package lists (e.g., the Quick Start generate commands block and
the “simple packages” list) to include the identifier tcAbacus alongside the
existing packages.</details> --- <details> <summary>ℹ️ Review info</summary> <details> <summary>⚙️ Run configuration</summary> **Configuration used**: defaults **Review profile**: CHILL **Plan**: Pro **Run ID**: `b16051cb-ff50-426a-a2fd-9bfb790befb1` </details> <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between 291baf0e4cd8ac3a129ca07c06cfeca86d0b94d3 and 6e541e2d4a988877f7a80e7e4e53dfb6976664ea. </details> <details> <summary>⛔ Files ignored due to path filters (3)</summary> * `pnpm-lock.yaml` is excluded by `!**/pnpm-lock.yaml` * `protoV2/tcAbacus/package-lock.json` is excluded by `!**/package-lock.json` * `protoV2/tcAbacus/pnpm-lock.yaml` is excluded by `!**/pnpm-lock.yaml` </details> <details> <summary>📒 Files selected for processing (38)</summary> * `docs/workflows/align-proto-workflow.md` * `package.json` * `packages/exceptions/src/exceptions/types/modules.ts` * `packages/sdk-ts/package.json` * `packages/sdk-ts/src/client/index.ts` * `packages/sdk-ts/src/client/indexer/grpc/IndexerGrpcTcDerivativesApi.ts` * `packages/sdk-ts/src/client/indexer/grpc/index.ts` * `packages/sdk-ts/src/client/indexer/grpc_stream/index.ts` * `packages/sdk-ts/src/client/indexer/grpc_stream/streamV2/IndexerGrpcTcDerivativesStreamV2.ts` * `packages/sdk-ts/src/client/indexer/transformers/IndexerGrpcTcDerivativesTransformer.ts` * `packages/sdk-ts/src/client/indexer/transformers/IndexerTcDerivativesStreamTransformer.ts` * `packages/sdk-ts/src/client/indexer/transformers/index.ts` * `packages/sdk-ts/src/client/indexer/types/index.ts` * `packages/sdk-ts/src/client/indexer/types/tc-derivatives.ts` * `packages/sdk-ts/src/client/tcAbacus/grpc/TcAbacusGrpcApi.spec.ts` * `packages/sdk-ts/src/client/tcAbacus/grpc/TcAbacusGrpcApi.ts` * `packages/sdk-ts/src/client/tcAbacus/grpc/index.ts` * `packages/sdk-ts/src/client/tcAbacus/grpc/transformers/index.ts` * `packages/sdk-ts/src/client/tcAbacus/index.ts` * `packages/sdk-ts/src/client/tcAbacus/types/index.ts` * `packages/sdk-ts/src/client/tcAbacus/types/tcAbacus.ts` * `packages/sdk-ts/src/utils/ofac.ts` * `packages/sdk-ts/tsdown.config.ts` * `protoV2/README.md` * `protoV2/tcAbacus/gen.sh` * `protoV2/tcAbacus/graphql.stub.proto` * `protoV2/tcAbacus/package.json` * `protoV2/tcAbacus/package.json.template` * `protoV2/tcAbacus/publish.sh` * `protoV2/tcAbacus/src/index.d.ts` * `protoV2/tcAbacus/src/index.js` * `protoV2/tcAbacus/src/index.template.js` * `protoV2/tcAbacus/src/index.template.ts` * `protoV2/tcAbacus/src/index.ts` * `protoV2/tcAbacus/tsconfig.json` * `protoV2/tcAbacus/tsup.config.d.ts` * `protoV2/tcAbacus/tsup.config.ts` * `vitest.config.ts` </details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
packages/sdk-ts/src/client/indexer/transformers/IndexerGrpcTcDerivativesTransformer.ts
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
protoV2/tcAbacus/gen.sh (1)
35-38:⚠️ Potential issue | 🟠 MajorPin the Abacus checkout to an immutable ref.
Cloning
mainkeeps this generator non-reproducible: the same SDK commit can emit different TS artifacts as upstream moves. Accept a tag or commit SHA (for exampleABACUS_REF) and clone that instead of a mutable branch.💡 Suggested change
-abacus_branch=main +abacus_ref="${ABACUS_REF:?Set ABACUS_REF to an immutable tag or commit SHA}" @@ -git clone https://github.com/InjectiveLabs/injective-abacus.git $BUILD_DIR -b $abacus_branch --depth 1 --single-branch > /dev/null +git clone https://github.com/InjectiveLabs/injective-abacus.git "$BUILD_DIR" \ + -b "$abacus_ref" --depth 1 --single-branch > /dev/null🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@protoV2/tcAbacus/gen.sh` around lines 35 - 38, The script currently clones the Abacus repo using a mutable branch via the abacus_branch variable and git clone ... -b $abacus_branch; change this to accept and use an immutable ref (e.g., ABACUS_REF) instead of a branch: add a variable (ABACUS_REF) that defaults to a specific tag or requires an env value, and use it in the git clone command (replace abacus_branch with ABACUS_REF) so the generator pins to a commit SHA or tag rather than main; ensure any references to abacus_branch are updated or removed (look for the abacus_branch variable and the git clone invocation).protoV2/tcAbacus/danielvladco/protobuf/graphql.stub.proto (1)
1-2:⚠️ Potential issue | 🟠 MajorMove this stub into a package-matching directory.
package danielvladco.protobuf.graphqlmaps todanielvladco/protobuf/graphql/, but this file is checked in one level higher. Buf will keep failingPACKAGE_DIRECTORY_MATCHuntil the source file is moved toprotoV2/tcAbacus/danielvladco/protobuf/graphql/graphql.stub.protoand the copy path inprotoV2/tcAbacus/gen.shis updated.#!/bin/bash python - <<'PY' from pathlib import Path p = Path("protoV2/tcAbacus/danielvladco/protobuf/graphql.stub.proto") package = next( line.split()[1].rstrip(";") for line in p.read_text().splitlines() if line.startswith("package ") ) expected_dir = Path("protoV2/tcAbacus", *package.split(".")) print("actual_dir =", p.parent.as_posix()) print("expected_dir =", expected_dir.as_posix()) print("match =", p.parent == expected_dir) PY🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@protoV2/tcAbacus/danielvladco/protobuf/graphql.stub.proto` around lines 1 - 2, The proto file graphql.stub.proto declares package danielvladco.protobuf.graphql but resides one directory level too high; move graphql.stub.proto into a directory structure matching the package (create danielvladco/protobuf/graphql under protoV2/tcAbacus and place the file there) and update the copy/path reference in protoV2/tcAbacus/gen.sh to point to the new protoV2/tcAbacus/danielvladco/protobuf/graphql/graphql.stub.proto location so PACKAGE_DIRECTORY_MATCH will pass.
🧹 Nitpick comments (1)
packages/sdk-ts/src/client/indexer/types/tc-derivatives.ts (1)
34-52: Reduce type drift by composingTcDerivativeTradeHistorywithTcPositionDelta.Line 34 duplicates
tradeDirection,executionPrice,executionMargin, andexecutionQuantityfromTcPositionDelta. Extending the base type keeps both contracts in sync.♻️ Proposed refactor
-export interface TcDerivativeTradeHistory { +export interface TcDerivativeTradeHistory extends TcPositionDelta { fee: string cid: string pnl: string payout: string tradeId: string marketId: string orderHash: string executedAt: number subaccountId: string feeRecipient: string executionSide: string isLiquidation: boolean - tradeDirection: string - executionPrice: string - executionMargin: string - executionQuantity: string tradeExecutionType: string }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/sdk-ts/src/client/indexer/types/tc-derivatives.ts` around lines 34 - 52, TcDerivativeTradeHistory duplicates fields defined in TcPositionDelta (tradeDirection, executionPrice, executionMargin, executionQuantity); update TcDerivativeTradeHistory to compose with TcPositionDelta instead of re-declaring those properties — e.g., make TcDerivativeTradeHistory extend or intersect TcPositionDelta and remove the duplicated property declarations, keeping the remaining unique fields (fee, cid, pnl, payout, tradeId, marketId, orderHash, executedAt, subaccountId, feeRecipient, executionSide, isLiquidation, tradeExecutionType) so both types stay in sync.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@packages/sdk-ts/src/client/indexer/transformers/IndexerGrpcTcDerivativesTransformer.ts`:
- Line 188: The mapping currently converts response.total using a truthy check
which turns 0 into undefined; in the transformer
(IndexerGrpcTcDerivativesTransformer) where total is set (total: response.total
? Number(response.total) : undefined), replace the truthy guard with a nullish
check so that numeric 0 is preserved (e.g., use response.total ?? undefined or
check for response.total !== null && response.total !== undefined before
Number(response.total)); update the assignment in the mapping function to use
that nullish check so valid zeros remain as 0.
In `@protoV2/tcAbacus/publish.sh`:
- Around line 1-4: The script currently lacks Bash strict mode and runs npm
publish even if the version bump fails; add strict mode (e.g., set -euo
pipefail) near the top after SCRIPT_DIR to ensure the script exits on errors and
undefined variables, and change the version bump invocation to use npm version
... --no-git-tag-version so it won't attempt git commits/tags; ensure the
subsequent npm publish . is only reached if the npm version command succeeds
(strict mode enforces this) and keep references to the existing SCRIPT_DIR
variable unchanged.
---
Duplicate comments:
In `@protoV2/tcAbacus/danielvladco/protobuf/graphql.stub.proto`:
- Around line 1-2: The proto file graphql.stub.proto declares package
danielvladco.protobuf.graphql but resides one directory level too high; move
graphql.stub.proto into a directory structure matching the package (create
danielvladco/protobuf/graphql under protoV2/tcAbacus and place the file there)
and update the copy/path reference in protoV2/tcAbacus/gen.sh to point to the
new protoV2/tcAbacus/danielvladco/protobuf/graphql/graphql.stub.proto location
so PACKAGE_DIRECTORY_MATCH will pass.
In `@protoV2/tcAbacus/gen.sh`:
- Around line 35-38: The script currently clones the Abacus repo using a mutable
branch via the abacus_branch variable and git clone ... -b $abacus_branch;
change this to accept and use an immutable ref (e.g., ABACUS_REF) instead of a
branch: add a variable (ABACUS_REF) that defaults to a specific tag or requires
an env value, and use it in the git clone command (replace abacus_branch with
ABACUS_REF) so the generator pins to a commit SHA or tag rather than main;
ensure any references to abacus_branch are updated or removed (look for the
abacus_branch variable and the git clone invocation).
---
Nitpick comments:
In `@packages/sdk-ts/src/client/indexer/types/tc-derivatives.ts`:
- Around line 34-52: TcDerivativeTradeHistory duplicates fields defined in
TcPositionDelta (tradeDirection, executionPrice, executionMargin,
executionQuantity); update TcDerivativeTradeHistory to compose with
TcPositionDelta instead of re-declaring those properties — e.g., make
TcDerivativeTradeHistory extend or intersect TcPositionDelta and remove the
duplicated property declarations, keeping the remaining unique fields (fee, cid,
pnl, payout, tradeId, marketId, orderHash, executedAt, subaccountId,
feeRecipient, executionSide, isLiquidation, tradeExecutionType) so both types
stay in sync.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: bd10206c-0e0c-486a-9f65-b9c361c1f6e5
📒 Files selected for processing (7)
packages/sdk-ts/package.jsonpackages/sdk-ts/src/client/indexer/transformers/IndexerGrpcTcDerivativesTransformer.tspackages/sdk-ts/src/client/indexer/types/tc-derivatives.tsprotoV2/tcAbacus/danielvladco/protobuf/graphql.stub.protoprotoV2/tcAbacus/gen.shprotoV2/tcAbacus/publish.shprotoV2/tcAbacus/src/index.d.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- protoV2/tcAbacus/src/index.d.ts
- packages/sdk-ts/package.json
| IndexerGrpcTcDerivativesTransformer.grpcPositionToPosition, | ||
| ), | ||
| next: response.next, | ||
| total: response.total ? Number(response.total) : undefined, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate tc-derivatives proto declarations if vendored in repo
fd 'injective_tc_derivatives_rpc_pb' .
# Inspect PositionsResponse/total declarations and nearby context
rg -n -C3 --type=ts 'PositionsResponse|total\??\s*:'
# Check how `response.total` is handled in indexer transformers
rg -n -C3 --type=ts 'response\.total\s*\?' packages/sdk-ts/src/client/indexer/transformersRepository: InjectiveLabs/injective-ts
Length of output: 26561
Preserve valid zero totals in positions mapping.
Line 188 uses a truthy guard that converts numeric 0 to undefined. Since total should represent the count of items (where 0 is a valid value), use a nullish check instead.
🐛 Proposed fix
- total: response.total ? Number(response.total) : undefined,
+ total:
+ response.total === null || response.total === undefined
+ ? undefined
+ : Number(response.total),📝 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.
Suggested change
| total: response.total ? Number(response.total) : undefined, | |
| total: | |
| response.total === null || response.total === undefined | |
| ? undefined | |
| : Number(response.total), |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@packages/sdk-ts/src/client/indexer/transformers/IndexerGrpcTcDerivativesTransformer.ts`
at line 188, The mapping currently converts response.total using a truthy check
which turns 0 into undefined; in the transformer
(IndexerGrpcTcDerivativesTransformer) where total is set (total: response.total
? Number(response.total) : undefined), replace the truthy guard with a nullish
check so that numeric 0 is preserved (e.g., use response.total ?? undefined or
check for response.total !== null && response.total !== undefined before
Number(response.total)); update the assignment in the mapping function to use
that nullish check so valid zeros remain as 0.
Comment on lines
+1
to
+4
| #!/usr/bin/env bash | ||
|
|
||
| # Get the directory where the script is located | ||
| SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Does npm versioncreate a git commit/tag by default, and does--no-git-tag-version disable the clean-working-tree and git-tagging behavior?
💡 Result:
Yes—when run inside a Git repo, npm version creates a “version” Git commit and a Git tag by default. This is controlled by the git-tag-version setting, whose default is true. Setting it to false means no commit is made at all (and therefore no tag either). [1]
Also yes—passing --no-git-tag-version disables that Git integration (commit/tag) and, per the docs, this is the switch that disables the behavior that otherwise fails when the working tree isn’t clean (the “Git working directory not clean” check is part of the Git integration path). [1]
Sources
[1] npm Docs – npm version (CLI v11): git-tag-version default/meaning, git commit+tag behavior, --no-git-tag-version, and clean-tree failure note. (docs.npmjs.com)
Citations:
🏁 Script executed:
# Check if the file exists and read its contents
if [ -f "protoV2/tcAbacus/publish.sh" ]; then
echo "=== File exists ===" && wc -l "protoV2/tcAbacus/publish.sh"
echo -e "\n=== Lines 1-10 ===" && head -10 "protoV2/tcAbacus/publish.sh"
echo -e "\n=== Lines 30-40 ===" && sed -n '30,40p' "protoV2/tcAbacus/publish.sh"
else
echo "File not found at protoV2/tcAbacus/publish.sh"
echo "Searching for publish.sh files..."
fd "publish.sh"
fiRepository: InjectiveLabs/injective-ts
Length of output: 623
Add strict mode to prevent npm publish from running after a failed version bump.
npm version defaults to creating git commits and tags when run inside a git repository. When the working tree is dirty or other git conditions aren't met, this can fail. Without set -e, the failed npm version command on line 37 won't stop the subsequent npm publish . on line 38 from executing. Additionally, npm version should use --no-git-tag-version to avoid git-integration failures in a subdirectory publish script.
Suggested fix
#!/usr/bin/env bash
+set -euo pipefail
# Get the directory where the script is located
SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"-npm version $v1
+npm version "$v1" --no-git-tag-version
npm publish .🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@protoV2/tcAbacus/publish.sh` around lines 1 - 4, The script currently lacks
Bash strict mode and runs npm publish even if the version bump fails; add strict
mode (e.g., set -euo pipefail) near the top after SCRIPT_DIR to ensure the
script exits on errors and undefined variables, and change the version bump
invocation to use npm version ... --no-git-tag-version so it won't attempt git
commits/tags; ensure the subsequent npm publish . is only reached if the npm
version command succeeds (strict mode enforces this) and keep references to the
existing SCRIPT_DIR variable unchanged.
ThomasRalee
approved these changes
Mar 18, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request introduces support for "tc-derivatives" to the SDK, including both gRPC and streaming APIs, along with necessary type definitions and transformers. It also updates dependencies to include the new proto packages and exposes the new modules for external use.
New tc-derivatives support:
IndexerGrpcTcDerivativesApiclass for gRPC API access to tc-derivatives, supporting queries for orders, trades, positions, and order history. (packages/sdk-ts/src/client/indexer/grpc/IndexerGrpcTcDerivativesApi.ts)IndexerGrpcTcDerivativesStreamV2, enabling real-time updates for orders, trades, and positions. (packages/sdk-ts/src/client/indexer/grpc_stream/streamV2/IndexerGrpcTcDerivativesStreamV2.ts)packages/sdk-ts/src/client/indexer/transformers/IndexerGrpcTcDerivativesTransformer.ts,packages/sdk-ts/src/client/indexer/transformers/IndexerTcDerivativesStreamTransformer.ts) [1] [2]Type and module exposure:
packages/sdk-ts/src/client/index.ts,packages/sdk-ts/src/client/indexer/grpc/index.ts,packages/sdk-ts/src/client/indexer/grpc_stream/index.ts,packages/sdk-ts/src/client/indexer/transformers/index.ts,packages/sdk-ts/src/client/indexer/types/index.ts) [1] [2] [3] [4] [5]Dependency and configuration updates:
@injectivelabs/tc-abacus-proto-ts-v2and latest versions of related proto packages inpackage.jsonfiles. [1] [2]TcDerivativesto theIndexerErrorModuleenum for error handling support. (packages/exceptions/src/exceptions/types/modules.ts)Summary by CodeRabbit
New Features
Documentation
Dependencies