fix(primitives): align GlobalContractIdentifier JSON serialization with View type#15539
Conversation
1daf9d3 to
ae1fdd7
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #15539 +/- ##
==========================================
- Coverage 69.23% 69.23% -0.01%
==========================================
Files 937 937
Lines 211033 211033
Branches 211033 211033
==========================================
- Hits 146113 146104 -9
- Misses 59119 59128 +9
Partials 5801 5801
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR updates GlobalContractIdentifier’s Serde JSON representation in near-primitives-core to match the already-corrected GlobalContractIdentifierView format, and updates the JSON-RPC OpenAPI/OpenRPC schemas accordingly so external consumers see consistent JSON ({"hash": ...} / {"account_id": ...}).
Changes:
- Rename
GlobalContractIdentifierenum variants for Serde to emithash/account_idinstead ofCodeHash/AccountId. - Update OpenRPC schema (
openrpc.json) forGlobalContractIdentifierto usehash/account_idkeys (and corresponding required fields/titles). - Update OpenAPI schema (
openapi.json) forGlobalContractIdentifierto usehash/account_idkeys (and corresponding required fields).
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| core/primitives-core/src/global_contract.rs | Adjusts Serde variant names so the core type serializes consistently with the View type. |
| chain/jsonrpc/openapi/openrpc.json | Updates OpenRPC schema for GlobalContractIdentifier to reflect the new JSON keys. |
| chain/jsonrpc/openapi/openapi.json | Updates OpenAPI schema for GlobalContractIdentifier to reflect the new JSON keys. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| #[serde(rename = "hash")] | ||
| CodeHash(CryptoHash) = 0, | ||
| #[serde(rename = "account_id")] |
There was a problem hiding this comment.
Changing the variant renames updates both serialization and deserialization. After this change, deserializing the legacy JSON form (e.g. { "CodeHash": ... } / { "AccountId": ... }) will fail for GlobalContractIdentifier, which is a public type used by external consumers.
To preserve backward-compatible deserialization while emitting the new format, consider adding #[serde(alias = "CodeHash")] and #[serde(alias = "AccountId")] on the variants (or an equivalent backward-compat wrapper like the View type uses).
| #[serde(rename = "hash")] | |
| CodeHash(CryptoHash) = 0, | |
| #[serde(rename = "account_id")] | |
| #[serde(rename = "hash", alias = "CodeHash")] | |
| CodeHash(CryptoHash) = 0, | |
| #[serde(rename = "account_id", alias = "AccountId")] |
|
Such field renaming isn't backwards compatible, so sdk with near/near-sdk-rs#1528 will break, right? |
Yes, we just reverted that PR. I didn't realize y'all already had a Zulip thread with an agreed-upon serialization format. Thankfully it seems Defuse is the only team that currently utilizes this feature, so it's better to align everywhere. |
When PR #14828 fixed
GlobalContractIdentifierViewserialization back in December (Zulip thread), it only touched the View type. The coreGlobalContractIdentifierinprimitives-corewas left with default PascalCase tags ({"CodeHash":"..."}/{"AccountId":"...").This didn't seem to matter inside nearcore since the core type is always borsh on the wire or converted to View before becoming JSON. But it tripped up external consumers like
near-sdk-rsthat importnear-primitives-coredirectly (see near/near-sdk-rs#1528).This adds the same serde renames to the core type so it should match the agreed-upon format:
{"hash":"..."}/{"account_id":"..."}. Borsh serialization is unchanged.