fix(jsonpb): eliminate non-deterministic behaviour in unmarshalling

[aarmoa]

Two independent sources of non-determinism existed in the JSON unmarshaller, both caused by iterating over Go maps whose key order is randomised per program execution.

Oneof conflict (critical)
sprops.OneofTypes is a map[string]*OneofProperties. When the JSON input contains multiple keys that belong to the same oneof group (e.g. {"title":"foo","salary":31000} for MsgWithOneof.union), every matched alternative overwrote the same interface field via target.Field(oop.Field).Set(nv). The winner depended on which key the runtime happened to visit last, making the decoded value non- deterministic.
The fix collects the map keys, sorts them with slices.Sort, and tracks which oneof interface fields have been set. If a second key from the same group is encountered an error is returned, consistent with the behaviour of the text-format parser (text_parser.go:589) and the proto oneof contract (at most one field may be set at a time).

Extension map iteration (low)
proto.RegisteredExtensions returns a map[int32]*ExtensionDesc. The unmarshaller iterated it without sorting. Extension names are unique, so this did not produce an incorrect result, but it was structurally inconsistent with the marshalling side (lines 401-408) which explicitly sorts extension IDs for stable output. The fix applies the same pattern: collect IDs, sort with sort.Sort(int32Slice(extIDs)), then iterate.

Tests
Following TDD, the tests were added first and confirmed to fail before the logic changes were applied:

• Two new entries in unmarshalingShouldError cover the oneof-conflict cases (orig_name and camelCase variants). TestUnmarshalingBadInput now asserts that both inputs produce a non-nil error.
• TestUnmarshalOneofConflictDeterminism runs the same conflicting JSON through the unmarshaller 100 times and asserts only one unique outcome is observed, directly reproducing the non-determinism that existed before the fix.

Ref: cosmos#161

Summary by CodeRabbit

• Bug Fixes

  • JSON unmarshaling now rejects conflicting values that target the same oneof group instead of overwriting a previously set member.
  • Explicit JSON null for oneof wrapper fields is ignored and no longer causes that oneof variant to be selected.
  • Proto2 extension unmarshaling is deterministic, producing consistent results across runs.

• Tests

  • Added tests validating oneof-null behavior and deterministic handling of conflicting oneof inputs.

[linear]

C-669 Report non-deterministic JSON from Authz MsgExecCompat to cosmos team

[coderabbitai]

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 6f9cfd67-f9a3-4714-92ae-1bb36f17dcc8

📥 Commits

Reviewing files that changed from the base of the PR and between a3ff2af and 66700c8.

📒 Files selected for processing (2)

• jsonpb/jsonpb.go
• jsonpb/jsonpb_test.go

---

📝 Walkthrough

Walkthrough

Oneof deserialization in jsonpb now deterministically orders oneof candidate keys, tracks which oneof member was set, treats explicit JSON null for oneof wrapper pointers as “ignore” (does not select that member), and errors if multiple JSON entries target the same oneof. Proto2 extension unmarshal iterates registered extensions in sorted ID order.

Changes

Cohort / File(s)	Summary
Core Unmarshaling Logic jsonpb/jsonpb.go	Updated unmarshaling to build deterministically sorted oneof key lists, track already-set oneof members, ignore JSON null for oneof wrapper pointers (so null does not select the member), and return an error when multiple keys map to the same oneof. Also iterates proto2 extensions by sorted extension ID.
Test Coverage jsonpb/jsonpb_test.go	Added test cases asserting errors when JSON sets multiple keys for the same oneof (including camelCase/orig_name variants), added TestUnmarshalOneofNullIsIgnored to verify null is ignored for oneof wrappers, and added TestUnmarshalOneofConflictDeterminism to assert deterministic behavior across repeated unmarshals.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

Hop-hop, keys sorted neat and clear,
Nulls now whisper, "I won't appear",
Oneof choices no longer collide,
Extensions march in ordered stride,
A rabbit cheers for deterministic pride! 🐰

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title directly and clearly identifies the main change: eliminating non-deterministic behavior in unmarshalling, which is the core focus of the PR that addresses two sources of non-determinism in JSON unmarshalling.

_{✏️ 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 c-669/fix-non-deterministic-json-from-authz-msgexeccompat

---

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