Skip to content

refactor(vm): replace custom JSON types with standard Amino encoding (alternative to #4972)#5274

Open
jaekwon wants to merge 14 commits intomasterfrom
feat/jae/json-vm-specs3
Open

refactor(vm): replace custom JSON types with standard Amino encoding (alternative to #4972)#5274
jaekwon wants to merge 14 commits intomasterfrom
feat/jae/json-vm-specs3

Conversation

@jaekwon
Copy link
Contributor

@jaekwon jaekwon commented Mar 11, 2026

Summary

This builds on @gfanton's PR #4972.

  • Remove JSONField, JSONStructValue, JSONArrayValue, JSONMapValue, JSONMapEntry, JSONObjectInfo types from gnolang
  • Use ExportValues() + amino.MarshalJSON() instead — the same encoding already used for persistence
  • Make tryGetError panic-safe: re-panics OOG, gracefully degrades on other panics (omits @error field)
  • Net ~960 lines removed

Follow-up PR

A separate PR for a state explorer will add:

  • A vm/qtype query endpoint for fetching type definitions (including struct field names)
  • A client-side JavaScript library for converting the Amino JSON into human-readable format

Test plan

  • All convert_test.go tests pass (primitives, structs, slices, pointers, maps, declared types, cycles, errors)
  • All keeper_test.go tests pass (persisted objects, nested object traversal, JSON formatting)
  • All values_export_test.go tests pass
  • Panicking .Error() gracefully omits @error field
  • OOG during .Error() properly re-panics

🤖 Generated with Claude Code

@Gno2D2 Gno2D2 requested review from alexiscolin and gfanton March 11, 2026 21:35
@github-actions github-actions bot added 📖 documentation Improvements or additions to documentation 🧾 package/realm Tag used for new Realms or Packages. 📦 🤖 gnovm Issues or PRs gnovm related 📦 🌐 tendermint v2 Issues or PRs tm2 related 📦 ⛰️ gno.land Issues or PRs gno.land package related 🌍 gnoweb Issues & PRs related to gnoweb and render 🐳 devops 🛠️ gnodev labels Mar 11, 2026
@Gno2D2
Copy link
Collaborator

Gno2D2 commented Mar 11, 2026

🛠 PR Checks Summary

All Automated Checks passed. ✅

Manual Checks (for Reviewers):
  • IGNORE the bot requirements for this PR (force green CI check)
Read More

🤖 This bot helps streamline PR reviews by verifying automated checks and providing guidance for contributors and reviewers.

✅ Automated Checks (for Contributors):

No automated checks match this pull request.

☑️ Contributor Actions:
  1. Fix any issues flagged by automated checks.
  2. Follow the Contributor Checklist to ensure your PR is ready for review.
    • Add new tests, or document why they are unnecessary.
    • Provide clear examples/screenshots, if necessary.
    • Update documentation, if required.
    • Ensure no breaking changes, or include BREAKING CHANGE notes.
    • Link related issues/PRs, where applicable.
☑️ Reviewer Actions:
  1. Complete manual checks for the PR, including the guidelines and additional checks if applicable.
📚 Resources:
Debug
Manual Checks
**IGNORE** the bot requirements for this PR (force green CI check)

If

🟢 Condition met
└── 🟢 On every pull request

Can be checked by

  • Any user with comment edit permission

@jaekwon jaekwon force-pushed the feat/jae/json-vm-specs3 branch from 6bb8194 to 31b0729 Compare March 11, 2026 21:56
@codecov
Copy link

codecov bot commented Mar 11, 2026

Use ExportRefValue with ":N" IDs for cycle-breaking references
instead of RefValue with synthetic ObjectIDs.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@jaekwon jaekwon force-pushed the feat/jae/json-vm-specs3 branch from 31b0729 to f3bed45 Compare March 11, 2026 22:23
jaekwon and others added 3 commits March 11, 2026 15:31
Wire up the QueryEvalJSON and QueryObject keeper methods to the ABCI
query handler so they are accessible via vm/qeval_json and vm/qobject
endpoints. Also adds both methods to the VMKeeperI interface.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Regenerate gnolang.proto to include ExportRefValue message
- Fix gofmt formatting in values_export.go
- Add QueryEvalJSON and QueryObject to mockVMKeeper

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@jaekwon jaekwon changed the title refactor(vm): replace custom JSON types with standard Amino encoding refactor(vm): replace custom JSON types with standard Amino encoding (alternative to #4972) Mar 11, 2026
@jaekwon jaekwon removed the request for review from alexiscolin March 11, 2026 23:16
jaekwon and others added 6 commits March 11, 2026 16:26
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Factor out exportObject helper shared by JSON and binary variants.
Add vm/qobject_binary route, QueryObjectBinary keeper method, and
interface/mock updates.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add txtar integration tests exercising the new query endpoints end-to-end:
- qeval_json: primitives, strings, structs, slices, multiple returns, errors
- qobject_json/qobject_binary: error handling for invalid/missing ObjectIDs,
  RefValue presence for persisted pointers

Also register ObjectNotFoundError with Amino and regenerate vm.proto.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…rface tests

Add tests for uncovered export paths: maps, functions, closures,
interfaces, int slices, list arrays, multiple returns, ExportObject,
and heap items (captured variables).

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add handler tests for qeval_json, qobject_json, qobject_binary, and
unknown endpoint queries. Add additional export value tests for bound
methods, declared types, map[string]struct, nested slices, type values,
structs with methods, and recursive structs.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
jaekwon and others added 3 commits March 11, 2026 17:58
Add handler success path tests for qobject_json and qobject_binary.
Add export type tests for InterfaceType, ChanType, tupleType, uint
values, BigintValue nil, and Block objects. Add WillSetParam tests
for auth and bank params. Add helper.go tests for MustParam functions.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Remove TestExportValuesClosure (duplicate of HeapItem),
TestExportValuesStructWithMethods (duplicate of struct value),
TestExportValuesTypeValue (weak assertions), TestExportCopyValue_UintValues
(covered by Primitive table), TestExportValuesNestedSlice (no new branch),
TestExportValuesMapStringStruct (map already covered), and
TestVmHandlerQuery_ObjectBinary error-only test (same parsing as JSON).

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@Kouteki Kouteki moved this from Triage to In Review in 🧙‍♂️Gno.land development Mar 12, 2026
@jefft0
Copy link
Contributor

jefft0 commented Mar 12, 2026

The PR description says "Add vm/qtype query endpoint for fetching type definitions (including struct field names)". Note that there is already the vm/qdoc endpoint which returns package information including types. For example:

gnokey query vm/qdoc -data "gno.land/p/nt/avl/v0" -remote https://rpc.gno.land:443

This includes the type information for the Node struct:

    {
      "name": "Node",
      "type": "struct {\n\tkey       string // key is the unique identifier for the node.\n\tvalue     any    // value is the data stored in the node.\n\theight    int8   // height is the height of the node in the tree.\n\tsize      int    // size is the number of nodes in the subtree rooted at this node.\n\tleftNode  *Node  // leftNode is the left child of the node.\n\trightNode *Node  // rightNode is the right child of the node.\n}",
      "doc": "Node represents a node in an AVL tree.\n",
      "alias": false,
      "kind": "struct",
      "fields": [
        {
          "name": "key",
          "type": "string",
          "doc": "// key is the unique identifier for the node.\n"
        },
        {
          "name": "value",
          "type": "any",
          "doc": "// value is the data stored in the node.\n"
        },
        {
          "name": "height",
          "type": "int8",
          "doc": "// height is the height of the node in the tree.\n"
        },
        {
          "name": "size",
          "type": "int",
          "doc": "// size is the number of nodes in the subtree rooted at this node.\n"
        },
        {
          "name": "leftNode",
          "type": "*Node",
          "doc": "// leftNode is the left child of the node.\n"
        },
        {
          "name": "rightNode",
          "type": "*Node",
          "doc": "// rightNode is the right child of the node.\n"
        }
      ]
    }

Comment on lines +9 to +10
stdout '"results":\['
stdout 'PrimitiveType'
Copy link
Member

Choose a reason for hiding this comment

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

It seems like a good idea to test against the entire reply rather than checking for substrings. (whole file)

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

please push

@jaekwon
Copy link
Contributor Author

jaekwon commented Mar 13, 2026

@gfanton ok. lets keep both for now but migrate over time to orthogonal APIs.

qdocs returning types is not orthogonal (it's composite) because well, a package doc isn't doc about any type, it's about names. So really qdoc should change to not return type information, and qtypes should be called separately to get type information without docs.

For now this is what we need for #5283 (new state explorer) MVP. I don't think the state explorer should be showing docs though.

For JS library UX the misc/gnojs javascript library is where we can make composite APIs built upon the orthogonal APIs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

📖 documentation Improvements or additions to documentation 🐳 devops 🛠️ gnodev 🌍 gnoweb Issues & PRs related to gnoweb and render 📦 🌐 tendermint v2 Issues or PRs tm2 related 📦 ⛰️ gno.land Issues or PRs gno.land package related 📦 🤖 gnovm Issues or PRs gnovm related 🧾 package/realm Tag used for new Realms or Packages.

Projects

Status: No status
Status: In Review

Development

Successfully merging this pull request may close these issues.

4 participants