Skip to content

fix: convert proto enum values via *ToJSON/*FromJSON for REST and serialization consistency#158

Merged
Vritra4 merged 1 commit intomainfrom
fix/client-params
Apr 2, 2026
Merged

fix: convert proto enum values via *ToJSON/*FromJSON for REST and serialization consistency#158
Vritra4 merged 1 commit intomainfrom
fix/client-params

Conversation

@Vritra4
Copy link
Copy Markdown
Contributor

@Vritra4 Vritra4 commented Apr 2, 2026

  • Proto enum values (BondStatus, VoteOption) were being passed as raw numbers instead of strings
  • Added *ToJSON() / *FromJSON() conversion in MstakingAPI query params, Validator, MsgVote, MsgVoteLegacy, and WeightedVoteOption
  • Amino serialization intentionally keeps numeric enums (verified via broadcast test that string format fails signature verification)

Summary by CodeRabbit

  • Tests

    • Updated test assertion for validator status field type validation.
  • Refactor

    • Improved data serialization for validator status and vote option fields to ensure proper format conversion between internal representations and external API data.

@Vritra4 Vritra4 requested review from joon9823 and songwongtp April 2, 2026 05:08
@Vritra4 Vritra4 self-assigned this Apr 2, 2026
@Vritra4 Vritra4 added the bug Something isn't working label Apr 2, 2026
@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Apr 2, 2026

Walkthrough

Updates enum field serialization across REST API and governance modules to use JSON string representations. The status field in staking validators and option field in voting structures are now converted to/from JSON string format using dedicated conversion helpers, aligning internal numeric/enum values with REST API and data contract expectations.

Changes

Cohort / File(s) Summary
REST API Staking Updates
src/client/rest/api/MstakingAPI.spec.ts, src/client/rest/api/MstakingAPI.ts
Updated MstakingAPI to convert BondStatus enum to JSON string via bondStatusToJSON() before sending REST requests. Test assertion updated to expect numeric status field.
Governance Voting Serialization
src/core/gov/Vote.ts, src/core/gov/msgs/MsgVote.ts, src/core/gov/msgs/MsgVoteLegacy.ts
Updated vote option field conversions to use voteOptionFromJSON() and voteOptionToJSON() helpers. Changed WeightedVoteOption.Data.option, MsgVote.Data.option, and MsgVoteLegacy.Data.option type from VoteOption/Option to string to reflect JSON representation.
Staking Validator Status
src/core/mstaking/Validator.ts
Updated Validator to convert BondStatus enum to/from JSON string via bondStatusFromJSON() and bondStatusToJSON(). Changed Validator.Amino.status and Validator.Data.status type from BondStatus to string.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 Hop along, dear enum friends,
From numbers now to strings you bend,
JSON-bound through REST we flow,
Validators and votes will surely know!

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title directly and accurately describes the main technical change across the entire changeset: converting proto enum values using *ToJSON/*FromJSON helpers for REST and serialization consistency.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/client-params

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 2, 2026

⚠️ Deprecation Warning: The deny-licenses option is deprecated for possible removal in the next major release. For more information, see issue 997.

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

Snapshot Warnings

⚠️: No snapshots were found for the head SHA 9337239.
Ensure that dependencies are being submitted on PR branches. Re-running this action after a short time may resolve the issue. See the documentation for more information and troubleshooting advice.

Scanned Files

None

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/client/rest/api/MstakingAPI.ts`:
- Line 68: The request builder in MstakingAPI.ts currently uses a truthy check
for the enum param (e.g., "{ ...params, status: status ?
bondStatusToJSON(status) : undefined }") which drops valid enum value 0; change
the conditional to an explicit undefined/null check (e.g., check status !==
undefined and status !== null) so bondStatusToJSON(status) is used for valid
numeric values including 0; apply the same fix to the other occurrence that uses
the same truthy pattern around bondStatusToJSON.

In `@src/core/mstaking/Validator.ts`:
- Line 193: The Validator.Amino/Validator.Data types currently force status:
string which breaks callers passing numeric enum values; update the status
property type to accept both string and numeric enum inputs (e.g., string |
number) so Validator.fromAmino and any callers (see Validator.fromAmino and
bondStatusFromJSON usage) remain compatible with numeric status values; adjust
both occurrences of status in the Validator type definitions accordingly and
ensure Validator.fromAmino continues to call bondStatusFromJSON to normalize the
value.
🪄 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: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 7b4a8609-ec78-4cac-a96e-08a30ebeb873

📥 Commits

Reviewing files that changed from the base of the PR and between 40c4f3f and 9337239.

📒 Files selected for processing (6)
  • src/client/rest/api/MstakingAPI.spec.ts
  • src/client/rest/api/MstakingAPI.ts
  • src/core/gov/Vote.ts
  • src/core/gov/msgs/MsgVote.ts
  • src/core/gov/msgs/MsgVoteLegacy.ts
  • src/core/mstaking/Validator.ts

@Vritra4 Vritra4 merged commit aa40d54 into main Apr 2, 2026
5 checks passed
@Vritra4 Vritra4 deleted the fix/client-params branch April 2, 2026 05:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants