fix(network): migrate governance proposal queries to gov v1 endpoints - #3573
fix(network): migrate governance proposal queries to gov v1 endpoints#3573baktun14 wants to merge 4 commits into
Conversation
The chain's legacy /cosmos/gov/v1beta1/proposals endpoint returns 500 once any on-chain proposal has a message count other than one (proposals 322 and 329 on mainnet), which broke GET /v1/proposals in production. Query the gov v1 endpoints instead and map the v1 shape (top-level title/summary, *_count tally fields, param changes extracted from MsgExecLegacyContent messages) while keeping the public API response contract unchanged.
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 17 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughChangesCosmos Gov v1 migration
Estimated code review effort: 3 (Moderate) | ~25 minutes Suggested reviewers: ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3573 +/- ##
==========================================
- Coverage 76.33% 75.71% -0.62%
==========================================
Files 1134 1070 -64
Lines 29598 27680 -1918
Branches 7352 6962 -390
==========================================
- Hits 22593 20959 -1634
+ Misses 6174 5917 -257
+ Partials 831 804 -27
*This pull request uses carry forward flags. Click here to find out more.
🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/api/src/proposal/services/proposal/proposal.service.ts (1)
68-72: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueConsider tolerating a non-JSON param value.
JSON.parse(change.value)throws if a legacy param change stores a plain string rather than JSON. The throw escapes the 404 branch and surfaces as a 500 for that proposal. Param values are JSON-encoded by convention, so this is an edge case, but the fallback is cheap.♻️ Proposed refactor
.map(change => ({ subspace: change.subspace, key: change.key, - value: JSON.parse(change.value) + value: this.parseParamValue(change.value) }))Add the helper to the class:
private parseParamValue(value: string): unknown { try { return JSON.parse(value); } catch { return value; } }🤖 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 `@apps/api/src/proposal/services/proposal/proposal.service.ts` around lines 68 - 72, Update the parameter mapping to tolerate legacy non-JSON values by adding a private parseParamValue helper on the proposal service that returns JSON.parse(value) when valid and the original string when parsing fails, then use it instead of JSON.parse(change.value).
🤖 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/proposal/services/proposal/proposal.service.ts`:
- Around line 65-67: Update the proposal conversion logic around paramChanges to
safely handle omitted messages by defaulting proposalFromCosmos.messages to an
empty array before filtering. Mark CosmosGovProposal.messages optional so
zero-message Cosmos responses are represented correctly while preserving
existing change extraction.
---
Nitpick comments:
In `@apps/api/src/proposal/services/proposal/proposal.service.ts`:
- Around line 68-72: Update the parameter mapping to tolerate legacy non-JSON
values by adding a private parseParamValue helper on the proposal service that
returns JSON.parse(value) when valid and the original string when parsing fails,
then use it instead of JSON.parse(change.value).
🪄 Autofix
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: 1d23ae1b-bb1c-4d49-b411-a20702d5e8be
📒 Files selected for processing (5)
apps/api/src/proposal/services/proposal/proposal.service.tsapps/api/test/functional/proposals.spec.tspackages/http-sdk/src/cosmos/cosmos-http.service.spec.tspackages/http-sdk/src/cosmos/cosmos-http.service.tspackages/http-sdk/src/cosmos/types.ts
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
LGTM. Correct, well-tested migration from gov v1beta1 to v1 for the three proposal queries, matching the described prod 500 root cause.
Extended reasoning...
Overview
The PR migrates CosmosHttpService's three governance-proposal queries (list, get-by-id, tally) from /cosmos/gov/v1beta1/* to /cosmos/gov/v1/*, and updates ProposalService to map the new v1 response shape (top-level id/title/summary, *_count tally fields, messages[] instead of content, nullable voting timestamps) back onto the unchanged public API contract (zod schemas untouched). Types in packages/http-sdk are rewritten to the v1 shape; I confirmed via grep that ProposalService is the only internal consumer of these exported types, so the acknowledged breaking type change has no other blast radius in this repo.
Security risks
None. This is a read-only proxy of public governance data from a REST full node; no auth, no user input reaches the changed code paths beyond a validated numeric proposal id.
Level of scrutiny
Medium — production-impacting bug fix (upstream 500s) but logically simple field remapping in a single service, with no schema/contract changes. I traced each field mapping (id, title/summary, tally *_count, voting_start/end_time null-coalescing to the sentinel 0001-01-01T00:00:00Z, and paramChanges extraction from MsgExecLegacyContent messages) against both the diff and the new functional test fixtures and found the logic consistent and correctly covered, including the two prod-breaking shapes (zero-message and multi-message proposals) called out in the PR description.
Other factors
Test coverage is strong: new functional tests exercise the zero-message/null-voting-time case, the multi-message case, and the legacy param-change + final-tally path, plus a new unit spec for CosmosHttpService verifying the new endpoint paths. Codecov reports full coverage of changed lines. No outstanding review comments in the timeline to address.
There was a problem hiding this comment.
Looks good overall; the one inline nit (unguarded total_deposit[0]) mirrors a class of issue the author already fixed for messages in this same PR, so I'd trust it to get picked up similarly.
Extended reasoning...
Overview
This PR migrates three governance proposal queries (list, by-id, tally) from the deprecated gov/v1beta1 REST endpoints to gov/v1, updating the shared http-sdk types and the apps/api ProposalService mapping logic accordingly. It's a targeted fix for a live production 500 caused by the chain node rejecting non-v1beta1-representable proposals. The public API response contract (zod schemas, generated types) is explicitly unchanged, and the PR includes solid functional/unit test coverage for the new v1 shapes, including edge cases like proposals with zero, multiple, or legacy-content messages, and null voting timestamps.
Security risks
No security-sensitive surface here — this is read-only data mapping from a public blockchain REST node into the console's existing public API contract. No auth, injection, or data-exposure concerns.
Level of scrutiny
This is a production-critical bug fix (fixing an active 500 in /v1/proposals), and the mapping logic involves several protojson-quirk edge cases (omitted repeated fields, nullable timestamps) that are easy to get subtly wrong. That said, the diff is well-scoped, mechanical in nature (renaming/remapping fields), and backed by good test coverage exercising the exact prod-breaking shapes. The one inline finding (unguarded total_deposit[0] access, mirroring the already-fixed messages guard) is flagged as a nit by the bug hunter itself — current mainnet behavior doesn't trigger it, and the fix is a one-line mirror of the pattern already applied elsewhere in this file.
Other factors
The author was responsive and thorough during CodeRabbit's review, promptly fixing the analogous messages-omission issue and adding regression coverage for it. That track record, plus the triviality and non-blocking nature of the total_deposit nit, gives me confidence a human reviewer doesn't need to gate on it, though the inline comment leaves a clear trail for a follow-up if desired.
There was a problem hiding this comment.
Beyond the two previously-fixed omitted-field issues (messages, total_deposit), I also checked the tally fields (yes_count/abstain_count/no_count/no_with_veto_count) and content?.changes for the same protojson-omits-empty/default-field risk — both are already read through || 0 or ?? [] fallbacks, so they don't share the crash risk. No new issues found this run; deferring given this migration has already required two rounds of edge-case fixes for a production-critical endpoint.
Extended reasoning...
No new bugs were found by the bug hunting system or in my own review this run. This is a follow-up note only: given the pattern of two real omitted-repeated-field bugs already found and fixed in this same PR (messages, total_deposit), I checked the remaining scalar/optional-chain accesses in ProposalService (tally counts, param-change changes extraction) for the same protojson-default-omission failure mode and found them already safely guarded with || 0 / ?? [] fallbacks.
1 participant
Why
Production
GET /v1/proposals(andGET /v1/proposals/{id}for some ids) returns 500. The upstream chain REST node rejects the legacy governance endpoint with:On-chain proposals #322 (2 messages) and #329 (0 messages) cannot be represented in the legacy gov v1beta1 shape, so the node 500s the entire
/cosmos/gov/v1beta1/proposalslist response (reproduced live againstapi.akashnet.net). As a side effect, the shared chain HTTP client retries 5xx GETs 3×, so every console request was also tripling load on the failing upstream endpoint.What
Migrate the three governance queries in
CosmosHttpServicefrom/cosmos/gov/v1beta1/*to/cosmos/gov/v1/*and adapt to the v1 response shape:packages/http-sdk: rewrite the gov response types to the v1 shape (id,messages[], top-leveltitle/summary,*_counttally fields, nullable voting timestamps). Type names are unchanged; note the shapes are breaking for external consumers of these exported types — though the old shapes described an endpoint that now 500s.apps/apiProposalService: maptitle/summaryfrom the v1 top-level fields, tally from*_countfields, extractparamChangesfromMsgExecLegacyContentmessages (multi-/zero-message proposals yield[]), and coalesce null voting timestamps to0001-01-01T00:00:00Z— exactly what the v1beta1 endpoint emitted for unset times./v1/proposalsresponse contract is unchanged (zod schemas and generatedconsole-api-typesuntouched).Verified end-to-end by booting the built API locally against the live mainnet node:
/v1/proposalsreturns 200 with all 161 proposals (currently 500 in prod),/v1/proposals/322and/v1/proposals/329return 200, legacy param-change proposals still surface parsedparamChanges, and unknown ids still 404.Summary by CodeRabbit
Bug Fixes
Tests