fix(deployment): upgrade chain-sdk to alpha.41 and adapt bigint resource decode - #3545
fix(deployment): upgrade chain-sdk to alpha.41 and adapt bigint resource decode#3545baktun14 wants to merge 3 commits into
Conversation
…rce decode chain-sdk alpha.40 changed ResourceValue.val (cpu/memory/storage/gpu quantities in deployment and bid protobuf messages) from Uint8Array to bigint. Adapt the two direct chain-sdk decoders (indexer v1beta4 deployment handler, api gpu-price v1beta5 bids) and the affected test fixtures. alpha.40 also made manifest Service.interconnectGroup required.
|
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 as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThe update upgrades the chain SDK, changes resource fixtures to use bigint values, and adds bigint-compatible resource decoding. GPU pricing retains byte-array support. The indexer uses direct numeric conversion with zero defaults. ChangesResource Unit Migration
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested reviewers: ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3545 +/- ##
==========================================
+ Coverage 74.79% 74.82% +0.02%
==========================================
Files 1155 1155
Lines 30092 30095 +3
Branches 7509 7513 +4
==========================================
+ Hits 22508 22519 +11
+ Misses 6701 6689 -12
- Partials 883 887 +4
🚀 New features to boost your workflow:
|
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
No bugs found, but I'd like a human to confirm this dependency bump before merge — it adapts a breaking chain-sdk decode change (Uint8Array → bigint) in two runtime-critical paths: GPU bid pricing (apps/api) and the blockchain stats indexer (apps/indexer), both of which affect financial/on-chain data with no schema-level type safety on the affected call sites (they're any-typed).
Extended reasoning...
Overview
This PR bumps @akashnetwork/chain-sdk from alpha.39 to alpha.41 across 9 workspaces and adapts to a breaking change where ResourceValue.val moved from Uint8Array to bigint in v1beta5 protobuf messages. Two call sites are affected: apps/indexer's handleCreateDeploymentV4 (a hard compile error, now fixed via Number(...)) and apps/api's gpu-price.service.ts (a runtime-only break masked by any typing, fixed via a new resourceValToInt helper that branches on typeof val === 'bigint'). Test fixtures across three spec files and one functional spec were updated to build bigint values instead of Uint8Array/Buffer, and a new regression test explicitly asserts decoded GPU bid units for v1beta5.
Security risks
None identified. This is a dependency version bump plus decode-path adaptation; no auth, crypto, or permission logic is touched. The changes are purely about correctly parsing already-trusted on-chain protobuf data.
Level of scrutiny
Despite being labeled size S, this isn't a purely mechanical bump — alpha.40 introduced a silent runtime behavior change that a type-unsafe (any) code path did not catch at compile time, and it feeds two consequential systems: GPU price calculation (used for pricing displayed to users) and the on-chain stats indexer (persists cpu/memory/storage/gpu quantities to the database). An incorrect decode here would not throw — it would silently produce wrong numbers in production. Given that risk profile plus the number of touched workspaces (9 package.json bumps, 1 lockfile, 2 runtime decode fixes, 4 test files), I believe this warrants a human review rather than automatic sign-off, even though I did not find a specific bug in the code.
Other factors
The PR author added a dedicated unit test ("parses v1beta5 resource units that decode as bigint") that pins the exact regression this PR fixes, and updated the functional gpu spec fixtures to match the new bigint shape. The sibling v1beta1/2/3 indexer handlers (still using @akashnetwork/akash-api, unaffected by the breaking change) were correctly left untouched. Test coverage looks appropriately targeted at the risky decode paths, which increases my confidence, but doesn't change my view that a human with chain-sdk context should confirm the interpretation of the new bigint semantics before this ships.
…oJSON chain-sdk alpha.40 changed GroupSpec.toJSON to emit ResourceValue.val as a decimal integer string instead of base64. Drop the now-incorrect atob() in the screening test assertion and refresh the stale doc comment. The screening endpoint accepts both encodings, so runtime behavior is unchanged.
There was a problem hiding this comment.
LGTM — this is a careful, well-tested adaptation to a breaking chain-sdk change, not a blind version bump.
What was reviewed: the two production code paths affected by the alpha.40 ResourceValue.val bigint change (indexer's handleCreateDeploymentV4 and api's gpu-price.service.ts), confirming the v1beta1/2/3 (akash-api, still Uint8Array) handlers were correctly left untouched. Also checked the CI codecov failure reported earlier in this PR's history (buildPlacementScreeningRequest producing "×M4" instead of "1000") — that failure traces to a stale intermediate commit that still called atob() on what is now a plain decimal string; the PR's final commit removes the atob() call and the test now asserts the decimal string directly, which resolves it.
Extended reasoning...
Overview
This PR bumps @akashnetwork/chain-sdk from alpha.39 to alpha.41 across all 9 workspace packages and adapts the two call sites that read ResourceValue.val directly from chain-sdk protobuf messages, which changed from Uint8Array to bigint in alpha.40: the indexer's v1beta4 deployment handler (now uses Number(...)) and the api's gpu-price service (routed through a new resourceValToInt helper that accepts both bigint and Uint8Array, since v1beta4 bids still come from akash-api as Uint8Array while v1beta5 bids come from chain-sdk as bigint). It also updates a manifest test fixture for a newly-required interconnectGroup field and switches several test fixtures from Uint8Array/TextEncoder encodings to BigInt(...).
Security risks
None. This is a dependency version bump plus adaptation to a decode-path type change; no auth, crypto, or permission logic is touched.
Level of scrutiny
Moderate-high given it touches billing-adjacent GPU pricing and blockchain indexing (data correctness), but the actual logic change is narrow and well-isolated: only two production files change behavior, both guarded by new/updated tests that assert decoded values numerically. The bulk of the diff is mechanical (package.json bumps, lockfile, test fixture updates).
Other factors
I independently traced the CI codecov failure visible in the PR timeline (test expected "1000", got "×M4") to confirm it is not a live issue: "×M4" is exactly what atob("1000")" decodes to, meaning the failure occurred on an intermediate commit that still base64-decoded a value that had already become a plain decimal string. The final commit in this PR removes that stale atob()call and the spec now asserts the decimal string directly. I also confirmedtrial-validation.service.tsandblocked-gpu.service.tsonly read GPU attribute strings (not.val`), explaining why only their test fixtures needed updates, not production code. Given no bugs were found by the bug-hunting system and my own targeted checks came back clean, I'm comfortable shadow-approving.
![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
🤖 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 `@apps/api/src/gpu/services/gpu-price/gpu-price.service.spec.ts`:
- Around line 142-145: Extend the assertions in the best-bid test around
result.models[0].providersWithBestBid![0].bestBid to validate the converted
deployment.storageUnits value from the v1beta5 fixture. Keep the existing CPU,
memory, and GPU assertions, and assert the expected storage quantity explicitly.
In `@apps/api/src/gpu/services/gpu-price/gpu-price.service.ts`:
- Around line 23-27: Update resourceValToInt to check for an empty Uint8Array
before calling uint8arrayToString or parseInt, returning 0 for empty legacy
values while preserving bigint and non-empty array handling.
🪄 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: 5cbeefa7-f6a4-444e-bb2e-13b57843255d
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (18)
apps/api/package.jsonapps/api/src/billing/services/trial-validation/trial-validation.service.spec.tsapps/api/src/deployment/services/blocked-gpu/blocked-gpu.service.spec.tsapps/api/src/gpu/services/gpu-price/gpu-price.service.spec.tsapps/api/src/gpu/services/gpu-price/gpu-price.service.tsapps/api/test/functional/gpu.spec.tsapps/deploy-web/package.jsonapps/deploy-web/src/queries/useScreenedProviders.spec.tsxapps/deploy-web/src/queries/useScreenedProviders.tsapps/deploy-web/src/services/provider-proxy/provider-proxy.service.spec.tsapps/indexer/package.jsonapps/indexer/src/indexers/akashStatsIndexer.tsapps/notifications/package.jsonapps/provider-inventory/package.jsonapps/provider-proxy/package.jsonapps/stats-web/package.jsonapps/tx-signer/package.jsonpackages/network-store/package.json
Empty Uint8Array(0) is truthy and decodes to "", so parseInt returned NaN and propagated through cpu/memory/storage reduce sums. Guard on length and add explicit radix. Also assert storageUnits decode in the bigint test.
There was a problem hiding this comment.
Not approving — this is a non-mechanical breaking-change adaptation across billing/pricing-critical resource-decode paths (GPU pricing, indexer stats, screening request encoding), so it warrants a human look even though no new bugs surfaced this run.
What was reviewed: the resourceValToInt bigint/legacy-Uint8Array handling in gpu-price.service.ts (including the empty-array NaN guard fixed in be2dd5c); the v1beta4-only bigint switch in the indexer's handleCreateDeploymentV4, confirming sibling v1/v2/v3 handlers correctly keep uint8arrayToString; and the useScreenedProviders decimal-string encoding change — verified against apps/api/src/bid-screening/http-schemas/bid-screening.schema.ts, whose ResourceValueSchema already accepts both decimal-integer and base64 strings, so the new encoding is backward compatible with the screening endpoint.
Extended reasoning...
Overview
This PR bumps @akashnetwork/chain-sdk from alpha.39 to alpha.41 across 9 workspace packages and adapts the two call sites that read ResourceValue.val directly from chain-sdk protobuf messages, which changed from Uint8Array to bigint in alpha.40: apps/api's GPU price service (v1beta5 bid decode) and apps/indexer's v1beta4 deployment handler. It also updates a deploy-web screening request builder that previously base64-encoded resource values and now emits decimal strings, plus fixture updates across several spec files.
Security risks
None identified. No auth, crypto, or permission logic is touched. The one externally-facing surface — the bid-screening request body encoding change in useScreenedProviders.ts — was cross-checked against the receiving schema (apps/api/src/bid-screening/http-schemas/bid-screening.schema.ts), which accepts both the old base64 format and the new decimal-string format, so it's backward compatible.
Level of scrutiny
The PR description itself calls out that this is 'not purely mechanical' since it adapts runtime decode logic, and one path (the GPU price service) is any-typed so the break wasn't caught by tsc — it would have crashed GPU pricing for every v1beta5 bid at runtime. That combination (compiler-invisible break + billing/pricing-adjacent code + a dependency bump touching 9 packages) puts this above the bar for a mechanical version bump, even though the diff itself is small (size: S).
Other factors
CodeRabbit flagged two real issues (missing storageUnits assertion, and NaN propagation from empty legacy Uint8Arrays in resourceValToInt) — both were fixed by the author in be2dd5c and confirmed inline, and a human reviewer (baktun14) already confirmed the version bump itself is intentional. Test coverage is solid: a new v1beta5 bigint unit test, updated fixtures across api/deploy-web spec files, and Codecov reports full coverage of modified lines. Given the resolved feedback and coverage, the main residual risk is the runtime-only (non-tsc-checked) nature of the original break, which is why I'd still like a human's eyes on this before merge.
1 participant
Why
Bumps
@akashnetwork/chain-sdkfrom1.0.0-alpha.39to1.0.0-alpha.41(the latest alpha, published from this release run). alpha.40 shipped a breaking change to how resource quantities decode, so the bump is not purely mechanical; this PR adapts the affected code.What
Bumps chain-sdk to
alpha.41across all 9 workspace packages plus the lockfile.chain-sdk breaking change (alpha.40, chain-sdk#347):
ResourceValue.val(the cpu/memory/storage/gpu quantity fields inside deployment and bid protobuf messages) changed fromUint8Arraytobigint. Only the two places that decode chain-sdk protobuf and read.valdirectly are affected:apps/indexerhandleCreateDeploymentV4(v1beta4 deployment decode): was a hardtsccompile error. Now reads the bigint directly viaNumber(...). The sibling v1beta1/2/3 handlers decode@akashnetwork/akash-apimessages (stillUint8Array) and are left untouched.apps/apigpu-price service (v1beta5 bid decode): the callbacks areany-typed, sotscdid not catch this, but at runtimeuint8arrayToString(bigint)throws, crashing GPU pricing for every v1beta5 GPU bid. Routed the four call sites through a small helper that accepts bothbigint(chain-sdk v1beta5) andUint8Array(akash-api v1beta4). Added a v1beta5 unit test that asserts the decoded units as a regression guard for this compiler-invisible break.alpha.40 also made manifest
Service.interconnectGrouprequired (GPU interconnect multinode, chain-sdk#315); updated the one hand-built manifest fixture. Test fixtures that built chain-sdk resources withUint8Arrayval were switched tobigint.Everything else in the repo reads
.valvia@akashnetwork/http-sdkREST types (plain strings), so it is unaffected. No production output contract changes: resource units are still emitted as the same integers.Verification
tsc --noEmitacross all 9 affected apps: zero new errors (every remaining error is pre-existing baseline; all modified files compile clean).apps/apiunit suite (1224 tests) and the affectedapps/deploy-webprovider-proxy spec pass; lint clean on all changed files.apps/apifunctional gpu spec type-checks; not run locally (no Docker), will run in CI.Summary by CodeRabbit
Bug Fixes
Documentation