feat(dpp): unify JSON/Value conversion traits + comprehensive round-trip tests#3573
Open
shumkov wants to merge 139 commits into
Open
feat(dpp): unify JSON/Value conversion traits + comprehensive round-trip tests#3573shumkov wants to merge 139 commits into
shumkov wants to merge 139 commits into
Conversation
Adds JsonConvertible / ValueConvertible impls (canonical traits in
packages/rs-dpp/src/serialization/serialization_traits.rs) to the
domain types catalogued in docs/json-value-conversion-inventory.md.
This is the unification first pass — round-trip correctness, tagged-
enum tag preservation, and integer-precision tests are deferred to the
second pass per the plan. Some impls may produce broken JSON or fail
round-trip until pass 2 fixes them; that's expected.
Coverage:
- Symmetrize V-only and J-only types (15+1).
- Add J+V to types missing both: top priorities (DataContract,
StateTransition, BatchTransition, Document, AssetLockProof,
AddressCreditWithdrawalTransition, Pooling) plus 22 batch transitions
and 19 leaf serde types.
Skipped: types without serde derives, lifetime-param refs, and the
wasm-dpp legacy crate per minimum-touch policy.
Approach: derive(JsonConvertible/ValueConvertible) where the type
already opts into the json_safe_fields macro ecosystem; empty manual
impl X {} (§6 escape hatch) elsewhere to bypass the JsonSafeFields
cascade. Both paths use the trait's default serde-delegating methods.
Adds planning docs:
- docs/json-value-conversion-inventory.md — structural inventory.
- docs/json-value-unification-plan.md — phased plan with critical
findings and per-mechanism deprecation decisions.
cargo check -p dpp passes with --features=json-conversion,value-conversion,serde-conversion.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Updates the unification plan with: - Progress table tracking the 5 passes (1 done, 2 in progress). - Phase B/C status updated: ~80 types now have canonical impls. - Skip-list rationale for types we deliberately did NOT migrate (no serde derives, lifetime params, internal indirection). - Section 11 "Lessons learned from pass 1" — the JsonSafeFields cascade, BTreeMap-of-enum-keys serde helpers, what shipped in the 481 commits we pulled, test-fixture pattern, sandbox/sccache/gpg gotchas. - Reference to pass-1 commit 9f23d67. Companion doc gets a status banner pointing back to the plan. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ity) Adding empty impl JsonConvertible/ValueConvertible for DataContract in pass 1 collided with the existing DataContractJsonConversionMethodsV0:: to_json(&self, &PlatformVersion) at every call site that passes a PlatformVersion — Rust E0034 (multiple applicable items in scope). Per the unification plan §3.11 step 10, DataContract is KEEP-AS-EXCEPTION (version-aware serde via DataContractInSerializationFormat). The proper unification path renames the legacy methods to *_versioned first, then the canonical traits can layer on. That's a follow-up. For now, leave a comment in data_contract/mod.rs explaining the absence and pointing readers at DataContractInSerializationFormat (which DOES have the canonical traits) when they need a JSON shape. cargo test -p dpp --features=json-conversion,value-conversion,serde-conversion --lib json_convertible_tests now passes (10/10 — the 5 address-transition round-trip + tag-preservation tests from pass 1). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds json_round_trip + value_round_trip tests for 11 types covered by the pass-1 unification commit (9f23d67). All 28 tests in the new modules pass; no regressions in the existing 3432 dpp lib tests. Types covered: - Identity, IdentityV0, IdentityPublicKey - AddressCreditWithdrawalTransition - TokenContractInfo, TokenPaymentInfo - Document - Pooling - GroupStateTransitionInfo Types skipped with TODO (V0 inner lacks Default): - AssetLockValue (AssetLockValueV0) - GroupAction (GroupActionV0 has GroupActionEvent field with no Default) Pass-2 work continues: more types to follow, then bug discovery (StateTransition untagged, ExtendedDocument bug, Critical-1 / -2 / -4). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds round-trip tests for TokenEmergencyAction, GasFeesPaidBy, and YesNoAbstainVoteChoice — all flat enums with derive(Default). Also marks TokenMarketplaceRules and other types whose V0 lacks Default with TODO(unification pass 2) comments — they need explicit fixtures. 34 json_convertible_tests pass, no regressions. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…DistributionType (pass 2) DocumentPatch has Default and J+V impls — round-trips cleanly. TokenDistributionType has Default but the J+V impls are on its variants (TokenDistributionTypeWithResolvedRecipient, TokenDistributionInfo), neither of which has Default — left as TODO for explicit fixture. 36/36 json_convertible_tests pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…perty assertions Per user direction, every J/V test must: 1. Use a NON-DEFAULT fixture (distinguishable values per field). 2. Round-trip via to_json/from_json (and to_object/from_object). 3. Assert each field of the recovered value individually — catches silent field drops, type narrowing, and PartialEq quirks that whole-struct equality can miss. IdentityCreateFromAddressesTransition is the canonical example — fixture has 6 non-default fields including a 2-entry inputs map with both P2PKH+P2SH addresses, a populated public key, two witness types, custom fee strategy, and non-zero user_fee_increase. All three tests pass (json_round_trip, value_round_trip, format_version_tag). Plan §8 updated with the new mandatory convention and rationale. Existing tests with Default fixtures are now legacy and will be upgraded as we revisit each type in pass 2. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…sferToAddresses tests Apply the new mandatory convention (non-default fixture + per-property assertions + round-trip) to two more address transitions. Both fixtures use distinguishable values for every field (identity_id, recipient_addresses, nonce, signature, fee strategy, witnesses, etc.) so the per-property assertions actually exercise data preservation. 3/5 address transitions now on the new convention. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Upgrade AddressFundingFromAssetLockTransition, AddressFundsTransferTransition,
and AddressCreditWithdrawalTransition tests to non-default fixture +
per-property assertions per the new convention.
Bug surfaced: AddressFundingFromAssetLockTransition.value_round_trip
fails — `OutPoint` inside `ChainAssetLockProof` cannot deserialize from
`platform_value::Value::Map` ("invalid type: map, expected an OutPoint").
JSON round-trip works fine. Marked the value test #[ignore] with the
reason and logged in plan §10b for pass-3 fix.
5/5 address transitions now on the new convention. 46 json_convertible_tests
pass, 3 ignored (1 OutPoint bug + 2 StateTransition untagged-enum known
failures).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…erty assertions Replaces the legacy Identity::default() fixture with one that has: - id: Identifier::new([0x42; 32]) - balance: 1_000_000 - revision: 7 - public_keys: BTreeMap with 2 distinct entries Per-property assertions check id, balance, revision, and public_keys count. Removes the duplicate empty-fixture test module that was leftover. 401 dpp lib tests pass (filtered to identity::identity). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… tests Apply non-default fixture + per-property assertion convention to: - IdentityPublicKey (8 distinguishable fields incl. disabled_at, contract_bounds) - TokenContractInfo (contract_id + token_contract_position; note: untagged enum) - Pooling (test all 3 variants — Never/IfAvailable/Standard) 48 json_convertible_tests pass, 3 ignored (1 OutPoint bug, 2 StateTransition). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Replaces single-Default-fixture tests for unit enums with each_variant() pattern that exercises all variants in turn. This is the per-property-assertion equivalent for unit-only enums where each discriminant is the only "field". Upgrades: - TokenEmergencyAction (Pause, Resume) - GasFeesPaidBy (DocumentOwner, ContractOwner, PreferContractOwner) - YesNoAbstainVoteChoice (YES, NO, ABSTAIN) 48 json_convertible_tests pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Apply non-default fixture + per-property assertion convention to: - GroupStateTransitionInfo (group_contract_position=5, action_id=[0x33;32], action_is_proposer=true) - DocumentPatch (id=[0x77;32], 2 properties, revision=3, updated_at=1.7T) 48 json_convertible_tests pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…per-property 5-field fixture with all Option fields populated and gas_fees_paid_by set to a non-default variant. Per-property assertion verifies each field preserves through round-trip. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…er-property 5-field fixture (owner_id, transitions, user_fee_increase, signature_public_key_id, signature) with distinguishable values. transitions vec is empty since DocumentTransition sub-types are tested in their own modules. Per-property assertion verifies each field preserves through round-trip. 49 json_convertible_tests pass, 3 ignored. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…rk list Updates the plan with: - Pass-2 status table — 17/~80 types upgraded, 1 bug surfaced. - Explicit list of types still on Default fixtures or without tests. - Cost estimate: ~10-15 hours of focused work to finish pass 2. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds basic round-trip + format version tag tests for: - IdentityCreateTransition (json/value tests #[ignore]: V0::default() has structurally invalid asset_lock_proof — needs explicit fixture) - IdentityTopUpTransition - IdentityCreditTransferTransition - MasternodeVoteTransition - IdentityPublicKeyInCreation - IdentityUpdateTransition - IdentityCreditWithdrawalTransition DataContractCreateTransition and DataContractUpdateTransition skipped: their V0 inners lack Default — needs explicit fixtures (TODO). 68 json_convertible_tests pass, 5 ignored (3 prior + 2 new IdentityCreateTransition pending real fixture). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds basic round-trip tests using Default fixture for: - BlockInfo (struct with Default) - Vote (manual Default impl) - VotePoll (manual Default impl) - ResourceVoteChoice (derived Default with #[default] variant) - InstantAssetLockProof (manual Default impl) Marks 6 types as TODO (no Default — needs explicit fixture): - ContractBoundSpecification, ChainAssetLockProof, - ExtendedBlockInfo, ExtendedEpochInfo, FinalizedEpochInfo, - IdentityTokenInfo, TokenStatus. 78 json_convertible_tests pass, 5 ignored. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Replaces TODOs with hand-built fixtures for:
- IdentityTokenInfo (frozen=true)
- TokenStatus (paused=true)
- ExtendedEpochInfo (6 fields, distinguishable values)
- FinalizedEpochInfo (12 fields incl. block_proposers map)
- ExtendedBlockInfo (8 fields incl. signature [u8;96])
Bug surfaced: ExtendedBlockInfo value_round_trip fails on signature
field round-trip via platform_value::Value ("Invalid symbol 17"). JSON
works. Marked #[ignore] and logged in plan §10b.
87 conversion tests pass, 6 ignored (3 prior + 1 new bug + 2 needs-fixture).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
AssetLockValue uses AssetLockValue::new() factory (V0 fields are pub(super), can't be set directly). ChainAssetLockProof uses OutPoint::from_str factory; value test ignored due to known OutPoint round-trip bug. 90 conversion tests pass, 7 ignored. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…IndexInformation)
…ourceVotePoll + ContestedDocumentVotePollWinnerInfo 102 conversion tests pass, 7 ignored. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ansition Both use fully-qualified trait syntax to disambiguate from legacy StateTransitionValueConvert::to_object/to_json methods on the same type — known E0034 ambiguity per plan §3.11. 106 conversion tests pass, 7 ignored. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
DocumentReplaceTransition, DocumentTransferTransition, DocumentPurchaseTransition, DocumentUpdatePriceTransition — all use fully-qualified trait syntax to disambiguate from legacy methods. 116 conversion tests pass, 7 ignored. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…nMint 122 conversion tests pass, 7 ignored. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…troyFrozenFunds 128 tests pass, 7 ignored. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…y/Claim/DirectPurchase/SetPrice) 136 conversion tests pass, 7 ignored. All 17 of 19 batch sub-transitions now tested (only TokenConfigUpdate remaining — needs TokenConfigurationChangeItem fixture). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…odsV0 entirely After Phase D step 5 trimmed the trait down to a single method (\`from_object(value, &platform_version)\`), examination shows the platform_version arg is dead scaffolding for V1+ that doesn't exist: - The V0 inner impl explicitly ignores it (\`_platform_version\`). - The outer dispatch only ever maps to V0 (the only structure version). - For V0 the result is byte-identical to canonical \`ValueConvertible::from_object(value)\` — both produce the same \`IdentityPublicKey\` because the outer enum is internally tagged with \`\$formatVersion\` and all V0 values carry that tag. Future-V1 migration scenarios — where "platform decides version" might diverge from "value decides version" — are speculative; if/when V1 ships, canonical's tag-driven dispatch is the correct semantic (deserialize the value as it claims to be). The legacy "override-the-tag-by-arg" form is unused everywhere it's currently called. ## Changes - Deleted \`identity_public_key/conversion/platform_value/v0/mod.rs\` (trait def, was a 1-method trait). - Deleted \`identity_public_key/v0/conversion/platform_value.rs\` (V0 impl + the \`TryFrom<&IdentityPublicKeyV0> for Value\` and \`TryFrom<Value> for IdentityPublicKeyV0\` helpers that lived alongside it — they were leftover scaffolding). - Outer impl on \`IdentityPublicKey\` removed; the file now contains only the test module exercising canonical \`ValueConvertible\` round trips. - Removed \`mod platform_value\` from \`identity_public_key/v0/conversion/mod.rs\`. ## Caller migration - wasm-dpp2 \`partial_identity.rs:387\`: switched from \`<IdentityPublicKey as IdentityPublicKeyPlatformValueConversionMethodsV0>::from_object(value, platform_version)\` to \`<IdentityPublicKey as ValueConvertible>::from_object(value)\`. - wasm-dpp2 \`public_key.rs:412\`: \`fromObject\` keeps the JS API signature \`(value, platformVersion)\` for SDK consistency, but routes through canonical \`ValueConvertible::from_object\`. The \`platform_version\` arg is now \`_platform_version\` — reserved for future use, not load-bearing. - rs-dpp \`identity_public_key/v0/conversion/json.rs\`: \`from_json_object\` switched from \`Self::from_object(value, platform_version)\` to \`platform_value::from_value(value)\` (the TryFrom impls that previously routed through serde lived in the deleted file). ## Test results - rs-dpp: 3704 lib tests passing (was 3712 — 8 fewer: dropped delegation tests that exercised the now-deleted trait surface). - wasm-dpp2: 1120 / 0 unchanged. - cargo check: clean across rs-dpp / wasm-dpp / wasm-dpp2. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…bject Same dead-scaffolding pattern as the value-path cleanup (\`IdentityPublicKeyPlatformValueConversionMethodsV0::from_object\`): \`from_json_object\` took a \`&PlatformVersion\` arg, the V0 inner impl ignored it (\`_platform_version\`), and the outer dispatcher always mapped to V0 (the only structure version that exists). For V0 only, the result is byte-identical regardless of the arg. Trim the signature to \`from_json_object(raw_object) -> ProtocolError\`. The trait still keeps \`to_json_object\` (validating-JSON shape) and \`from_json_object\` (binary-field replacement) since both have real semantic content distinct from canonical \`JsonConvertible\`. Updated wasm-dpp2 callers (\`IdentityPublicKey.fromJSON\`, \`PartialIdentity.fromJSON\`) to drop the unused arg. The wasm wrapper JS API still accepts \`platformVersion\` for SDK consistency, prefixed \`_\` since it's reserved for future use. Test results: - rs-dpp: 3704 lib tests passing. - wasm-dpp2: 1120 / 0 unchanged. Also expanded the trait doc comment so its remaining purpose (validating-JSON shape support, complementary to canonical JsonConvertible) is unambiguous. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…cy traits
The wasm-dpp2 \`IdentityPublicKey.toJSON\` / \`fromJSON\` JS API was
exposing a deviant wire shape: validating-JSON form with binary fields
as JSON arrays of u8 values. Every other rs-dpp type's canonical JSON
shape uses base64 strings — including the embedded public keys inside
\`IdentityWasm.toJSON\`. The two paths produced *different* shapes for
the same data depending on whether you serialized the wrapping
Identity or the standalone IdentityPublicKey.
Switch \`IdentityPublicKeyWasm.toJSON\` / \`fromJSON\` and
\`PartialIdentityWasm\`'s inner-key deserialization to canonical
\`JsonConvertible\`. Now: same shape everywhere, base64 strings for
binary, base58 strings for identifiers.
That eliminates the only production reason to keep the legacy JSON
conversion traits. Audit confirmed:
- \`IdentityJsonConversionMethodsV0\` (Identity, not IPK): zero
non-test callers anywhere in the workspace. Was already dead.
- \`IdentityPublicKeyJsonConversionMethodsV0\` (IPK): only callers
were the wasm-dpp2 sites we just migrated.
## Deletions
Six files removed:
- \`identity/conversion/json/v0/mod.rs\` (\`IdentityJsonConversionMethodsV0\` trait def)
- \`identity/conversion/json/mod.rs\` (outer wrapper, was \`pub use v0::*;\`)
- \`identity/v0/conversion/json.rs\` (V0 impl + tests, ~165 lines)
- \`identity_public_key/conversion/json/v0/mod.rs\` (\`IdentityPublicKeyJsonConversionMethodsV0\` trait def)
- \`identity_public_key/conversion/json/mod.rs\` (outer impl + tests, ~85 lines)
- \`identity_public_key/v0/conversion/json.rs\` (V0 impl + \`TryFrom<&str>\` + tests, ~170 lines)
Module declarations removed from:
- \`identity/conversion/mod.rs\` (\`pub mod json\`)
- \`identity/v0/conversion/mod.rs\` (\`pub mod json\`)
- \`identity_public_key/conversion/mod.rs\` (\`pub mod json\`)
- \`identity_public_key/v0/conversion/mod.rs\` (\`mod json\`)
## Test fixture updates
- \`wasm-dpp2/tests/unit/IdentityPublicKey.spec.ts\`:
\`toJSON\` test was asserting \`Array.from(json.data).deep.equal(...)\`
against byte-array form. Switched to
\`expect(json.data).to.equal('A2o5...=')\` for the canonical base64
string. The \`fromJSON\` test was already passing a base64 string
fixture (\`data: 'A2o5...'\`) — it kept working through the legacy
\`from_json_object\` because \`replace_at_paths\` accepts both
shapes; now it goes through canonical \`JsonConvertible::from_json\`
which expects base64 directly.
## Test results
- rs-dpp: 3686 lib tests passing (was 3704 — lost 18 from the deleted
trait test modules, all of which were exercising methods that no
longer exist).
- wasm-dpp2: 1120 / 0 passing (1 fixture updated, full green).
- cargo check on rs-dpp / wasm-dpp / wasm-dpp2: clean.
Net: −425 lines of legacy trait code, single canonical JSON wire shape
across the entire SDK surface for identity-family types.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…yInto impls
Phase D step 6 cleanup. The Critical-2 fix from earlier on this branch
already gave \`AssetLockProof\` a tagged-enum representation
(\`#[serde(tag = "type")]\`) and a manual \`Deserialize\` that routes
through \`RawAssetLockProof\` for the instant-lock raw-bytes shape.
What remained was three asymmetric helpers that produced *untagged*
\`platform_value::Value\` (dropping the variant tag entirely), so their
output couldn't round-trip through the manual \`Deserialize\`:
- \`AssetLockProof::to_raw_object\` (inherent method)
- \`TryInto<Value> for AssetLockProof\`
- \`TryInto<Value> for &AssetLockProof\`
All three had **zero production callers** workspace-wide (verified
across rs-dpp, rs-drive, rs-drive-abci, rs-sdk, wasm-dpp, wasm-dpp2,
wasm-sdk). The only callers were tests in the same module exercising
the dead helpers.
Deleted them. Use canonical \`ValueConvertible::to_object\` instead —
it produces the correctly-tagged shape (\`{type: "instant" | "chain",
...fields}\`) that round-trips through \`Deserialize\`.
The lingering \`TryFrom<&Value>\` / \`TryFrom<Value> for AssetLockProof\`
hacks (which accept legacy integer-tag and externally-tagged shapes)
are KEPT for now — they have 2 production callers in
state-transition value-conversion paths, and migrating them requires
auditing every upstream that produces a state transition's raw Value.
A TODO comment in the source already flagged this. Follow-up work.
Test fixture updates:
- \`chain_proof_to_raw_object\` test renamed to
\`chain_proof_to_object_canonical\`, switched to
\`ValueConvertible::to_object\`.
- \`chain_proof_value_round_trip\` rewritten to exercise the canonical
\`to_object\` -> \`from_object\` round trip with a wire-shape
assertion that the result has \`type: "chain"\` (not the legacy
integer tag form).
- \`try_into_value\` test module deleted (exercised the now-deleted
\`TryInto<Value>\` impls).
Test results:
- rs-dpp: 3684 lib tests passing (was 3686 — lost 2 tests that
exercised the deleted impls).
- wasm-dpp2: 1120 / 0 unchanged.
- cargo check: clean across rs-dpp, wasm-dpp, wasm-dpp2.
Net: ~50 lines of asymmetric dead code removed.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…from_value
Phase D step 6, completing what the in-source TODO comment promised:
// todo: replace with
// from_value(value.clone()).map_err(ProtocolError::ValueError)
The hack accepted two pre-Critical-2 wire shapes that no longer flow:
1. Integer-tagged: \`{type: 0|1, ...fields}\` (used \`AssetLockProofType\`
enum's u8 discriminant)
2. Externally-tagged: \`{Instant: {...fields}}\` / \`{Chain: {...}}\`
(variant name as map key)
After the Critical-2 fix earlier in this branch made AssetLockProof
internally tagged with \`#[serde(tag = "type")]\`, the canonical
serialization produces \`{type: "instant" | "chain", ...fields}\` —
incompatible with both legacy shapes. The hack was simultaneously
broken for canonical input (its \`get_optional_integer("type")\` call
errors when \`type\` is a string) AND unreachable for legacy input
(nothing in the workspace produces those shapes anymore).
Audit:
- Production callers of \`AssetLockProof::try_from(&Value)\` /
\`try_from(Value)\`: only 2 — \`IdentityTopUpTransitionV0::from_object\`
and \`IdentityCreateTransitionV0::from_object\`, both legacy
\`StateTransitionValueConvert\` (A1) trait methods.
- External callers of those legacy A1 \`from_object\` methods:
zero in rs-drive / rs-drive-abci / rs-sdk / wasm-dpp / wasm-dpp2 /
wasm-sdk for these specific transitions. (wasm-dpp uses A1 for
DataContract create/update only.)
- All 3684 rs-dpp lib tests still pass with the hack replaced —
confirming nothing was relying on the legacy-shape acceptance.
Replaced both \`TryFrom\` impls with one-line \`platform_value::from_value\`
calls. The \`Deserialize\` impl handles the routing through
\`RawAssetLockProof\` for the instant-lock raw-bytes shape.
Test results:
- rs-dpp: 3684 / 0
- wasm-dpp / wasm-dpp2 / wasm-sdk: cargo check clean
- wasm-dpp2: 1120 / 0
Net: −56 lines of dead-on-arrival code.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…notes ## Step 5 — Identity family canonical migration ✅ DONE Updates the plan doc to reflect that ALL FOUR legacy identity-family conversion traits have been deleted entirely (across multiple commits on this branch): - \`IdentityPlatformValueConversionMethodsV0\` (1-method, redundant after step 4 \`skip_serializing_if\`) - \`IdentityPublicKeyPlatformValueConversionMethodsV0\` (4 methods, including the formerly-defended \`from_object(value, &platform_version)\` whose platform_version dispatch turned out to be dead scaffolding — V0 ignored the arg, only V0 exists, canonical tag-driven dispatch is byte-identical) - \`IdentityJsonConversionMethodsV0\` (3 methods, zero non-test callers) - \`IdentityPublicKeyJsonConversionMethodsV0\` (3 methods including validating-JSON shape — wasm-dpp2 IdentityPublicKey JS API switched to canonical base64 strings, matching every other rs-dpp type) Documents what shipped + what didn't (the validating-JSON byte-array shape was deliberately dropped — every other type's canonical JSON output uses base64 strings, the IdentityPublicKey deviation was an SDK API inconsistency). ## Step 6 — AssetLockProof tagged-enum + dead helpers ✅ DONE Documents the two-part work: the original Critical-2 fix (tagged-enum representation, manual Deserialize via RawAssetLockProof, canonical traits) plus the asymmetric-helper deletion (\`to_raw_object\`, \`TryInto<Value>\`, the \`TryFrom\` hack that accepted pre-Critical-2 legacy shapes which no longer flow anywhere). ## Step 8 — Document family ⬜ DEFERRED Captures the audit findings for the follow-up PR: production callers in rs-drive's document-update consensus path, the genuine semantic divergence of \`to_map_value\` (returns BTreeMap, not Value), \`from_json_value\` (manual ingest), and \`to_json_with_identifiers_using_bytes\` (validating-JSON shape). Lists the specific call sites and the sequence of steps a follow-up PR should take, including the hash-equivalence audit needed for rs-drive. No code changes — pure documentation update. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ferent methods Phase D step 8 slice A. The two legacy Document conversion traits had a mix of redundant methods (1:1 canonical equivalents) and methods with genuine semantic content. Trim the redundant ones, keep the rest with clearer documentation of why they exist. ## DocumentPlatformValueMethodsV0 (A11) Deleted from trait + V0 impl + outer impl + ExtendedDocumentV0 impl: - \`to_object\` — 1:1 equivalent of canonical \`ValueConvertible::to_object\`. - \`into_value\` — 1:1 equivalent of canonical \`ValueConvertible::into_object\`. Kept (with expanded doc comments explaining why): - \`to_map_value\` / \`into_map_value\` — return \`BTreeMap<String, Value>\`, used by ExtendedDocument and DocumentWasm to compose Document plus metadata fields. Canonical returns \`Value::Map(Vec<...>)\`, not the map directly. - \`from_platform_value(value, &platform_version)\` — **legacy-shape ingest**. Accepts un-tagged Document values (no \`\$formatVersion\`). Symmetric with \`from_json_value\` on the JSON side. Used to ingest DPNS / DashPay legacy JSON fixtures and older stored shapes that predate \`#[serde(tag = "\$formatVersion")]\`. Initially audited as "dead scaffolding for V1+" — actually load-bearing for legacy ingest. ExtendedDocument's \`from_trusted_platform_value\` / \`from_untrusted_platform_value\` and the \`json_should_generate_human_readable_binaries\` test rely on it. ## DocumentJsonMethodsV0 (A10) Deleted from trait + V0 impl + outer impl + ExtendedDocumentV0 impl: - \`to_json\` — 1:1 equivalent of canonical \`JsonConvertible::to_json\`. Its body was \`self.to_object()?.try_into()\` — same as canonical. Kept: - \`to_json_with_identifiers_using_bytes\` — validating-JSON wire shape (bs58 string identifiers + binary fields as JSON arrays of u8). Used by JSON Schema validators. - \`from_json_value<S, E>\` — generic over identifier deserialization type, accepts JSON without \`\$formatVersion\`. Legacy-shape ingest. ## Internal call-site updates - \`document/v0/cbor_conversion.rs\`: \`self.to_object()\` (deleted V0 trait method) -> inlined \`platform_value::to_value(self)\`. CBOR conversion goes through the same path. - \`document/v0/json_conversion.rs\` tests: \`doc.to_json(...)\` -> \`serde_json::to_value(&doc)\` (DocumentV0 doesn't derive JsonConvertible directly). - \`extended_document/v0/mod.rs\`: \`pub fn to_pretty_json\` inlined what legacy \`to_json\` body did (\`to_object()?.try_into()\`) — goes through platform_value as an intermediate, which is what the test fixture asserts (binary as base64, identifiers as bs58). Canonical \`JsonConvertible::to_json\` would go directly via serde_json (one-step), producing a subtly different shape due to Critical-1 (is_human_readable divergence). - \`extended_document/mod.rs\`: \`pub fn to_json\` (outer inherent, takes &PlatformVersion) routes through canonical \`JsonConvertible::to_json\` — platform_version arg kept for API compatibility but isn't load-bearing. ## Test results - rs-dpp: 3677 lib tests passing (was 3684 — lost 7 tests that exercised the deleted methods; round-trip coverage moved to canonical-trait round-trip tests where applicable). - wasm-dpp2: 1120 / 0 unchanged. - rs-drive: cargo check clean. Net: ~−170 lines of redundant code; clearer trait surface. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Step 8 slice A shipped in commit 678121a. Update the plan doc with: 1. What landed: deleted the 1:1 canonical-equivalent methods from both Document family legacy traits (\`to_object\`, \`into_value\` from A11; \`to_json\` from A10), kept the rest with expanded doc comments. 2. Audit course-correction on \`from_platform_value\`: my initial pass dismissed it as "dead scaffolding for V1+" — wrong. The method accepts un-tagged Document values that canonical \`ValueConvertible::from_object\` errors on (DPNS legacy fixtures, ExtendedDocument's \`from_trusted_platform_value\`). Reverted that part of the migration; kept the method as legacy-shape ingest (symmetric with \`from_json_value\`). 3. Slice B deferred work: wasm-dpp2 DocumentWasm.fromObject migration, rs-drive test-fixture migration, \`to_map_value\` API decision. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Phase D step 8 slice B finishes the Document-family canonical migration
by removing both legacy ingest methods and routing every caller through
canonical ValueConvertible / JsonConvertible.
Trait deletions (rs-dpp):
- DocumentPlatformValueMethodsV0::from_platform_value
- DocumentJsonMethodsV0::from_json_value<S, E>
Traits now hold only the methods with no canonical equivalent:
A11 -> to_map_value / into_map_value (BTreeMap<String,Value> shape)
A10 -> to_json_with_identifiers_using_bytes (validating-JSON shape)
Caller migrations:
- wasm-dpp2 DocumentWasm.toObject emits \$formatVersion: \"0\";
fromObject/fromJSON use canonical from_object / from_json.
- ExtendedDocumentV0 from_trusted_platform_value /
from_untrusted_platform_value insert \$formatVersion via the
new ensure_document_format_version helper, then delegate to
canonical Document::from_object. Public API unchanged.
- wasm-dpp legacy DocumentWasm.fromObject inserts \$formatVersion
inline before canonical (minimum-touch, JS surface unchanged).
- rs-drive 4 sites in drive/document/update/mod.rs and 7 sites in
tests/query_tests.rs migrated to local document_from_legacy_value
helpers using the same insert-tag-then-canonical pattern.
- Removed obsolete tests in document/v0/json_conversion.rs that
targeted the deleted from_json_value method; canonical round-trip
is covered in serialization_traits and v0/serialize.rs.
Verified:
cargo test -p dpp --features all_features_without_client --lib
-> 3670 passed, 0 failed, 8 ignored
cargo test -p drive --lib drive::document::update::tests
-> 39 passed
cargo check -p dpp -p drive -p wasm-dpp -p wasm-dpp2 --tests
clean (only pre-existing warnings)
Net for Phase D step 8 (slice A + B): legacy A10 / A11 traits reduced
from 6 methods to 3, with the 3 survivors documenting why they stay.
Phase D step 9 removes both legacy state-transition trait families
entirely. The audit reframed the work after three findings:
1. Signing is bincode (PlatformSignable -> signable_bytes()), not
JSON canonical. The to_canonical_object / to_canonical_cleaned_object
machinery on A1 was vestigial JS-DPP-era scaffolding with zero
production callers - only its own tautological tests called it.
2. Outer enums already had canonical JsonConvertible / ValueConvertible
derives from Phase C; A1 / A2 were running in parallel doing the
same work.
3. Cross-package use was tiny: 2 wasm-dpp legacy files used
to_cleaned_object. wasm-dpp2 had zero A1 / A2 callers.
What landed (4 slices in one commit, after the audit collapsed
"step 9 long pole" to a deletion):
Slice 1 + 3: deleted both traits and 68 impl files
- traits/state_transition_value_convert.rs
- traits/state_transition_json_convert.rs
- 34 value_conversion.rs + 34 json_conversion.rs (per transition x
{inner, outer} x {V0, V1})
- removed mod declarations from 34 parent mod.rs files
- removed re-exports from traits/mod.rs
Slice 2: migrated 2 wasm-dpp legacy callers to canonical
- DataContractCreateTransitionWasm / DataContractUpdateTransitionWasm
constructor + toObject use canonical ValueConvertible::to_object()
plus manual signature path stripping for skip_signature
- constructors use insert-$formatVersion-then-canonical pattern
matching the Document migration
- state_transition_facade.rs is dead (module not exposed); no fix
needed there
Slice 4 (subset): added #[json_safe_fields] to 5 V0 inners that lacked it
- AddressCreditWithdrawalTransitionV0
- AddressFundingFromAssetLockTransitionV0
- AddressFundsTransferTransitionV0
- IdentityCreateFromAddressesTransitionV0
- IdentityTopUpFromAddressesTransitionV0
Slice 4 (deferred): BatchTransitionV0 / V1 cannot get #[json_safe_fields]
yet - the attribute generates compile-time JsonSafeFields assertions
on field types and DocumentTransition / BatchedTransition (and their
sub-transitions) need their own JsonSafeFields impls first. Follow-up
for the BatchTransition family migration.
Test cleanup: subagent deleted 76 tautology tests across 13 test
modules that were exercising the removed methods. Canonical round-trip
on outer enums via json_convertible_tests / value_convertible_tests
covers correctness.
Verification:
cargo test -p dpp --features all_features_without_client --lib
-> 3594 passed, 0 failed, 8 ignored (was 3670; -76 tautology)
cargo check -p drive -p wasm-dpp -p wasm-dpp2 -p dash-sdk -p drive-abci --tests
clean (only pre-existing warnings)
Net: 219 lines added, 5051 lines deleted across 108 files.
…lpers Plan-doc refresh: - Updated progress table (now 2026-05-09): Phase D 1-9 done, steps 10-11 remain. Pass 4 (wasm-dpp2 _serde! migration) reframed: step 9 audit showed wasm-dpp2 had no actual A1/A2 blockers, so the remaining _serde! sites need re-survey to identify actual blockers. - Critical-5 (sorted-keys-for-signing) marked FALSIFIED — signing is bincode, the canonical-object machinery had zero production callers. - A1/A2 rows in §3.1 trait table struck out with deletion commit ref. - §3.5 catalogue: state_transition trait + impl files marked deleted. - §3.10 affected-type total: pre-Phase D ~90 non-canonical types collapsed to ~10-15 remaining (DataContract KEEP-AS-EXCEPTION, AddressWitness/ContestedIndexFieldMatch step 11, wasm-dpp legacy). - §3.11 step 7 (ExtendedDocument C1) marked DONE with commit ref. - Phase D summary section now lists step-by-step status. Code: deleted abstract_state_transition.rs - the state_transition_helpers module had become dead after step 9 removed all callers (the trait methods were the only consumers of to_object/to_cleaned_object/to_json helpers). Verified zero remaining users via grep, then removed the mod declaration and pub use re-export from state_transition/mod.rs. Verification: 3594/3594 dpp lib tests pass; workspace --tests check clean.
Step 9 follow-up: completes the JS-safe integer protection for the
BatchTransition tree, deferred at step 9 because it required a deeper
walk through the document/token transition sub-tree.
Attribute applied to V0 inners that were missing it (8 leaves):
- DocumentDeleteTransitionV0
- TokenFreezeTransitionV0
- TokenUnfreezeTransitionV0
- TokenDestroyFrozenFundsTransitionV0
- TokenClaimTransitionV0
- TokenEmergencyActionTransitionV0
- TokenConfigUpdateTransitionV0
- TokenSetPriceForDirectPurchaseTransitionV0
Plus BatchTransitionV0 / BatchTransitionV1 themselves (deferred at
step 9 with explanatory comments; comments removed, attribute applied).
Manual JsonSafeFields impls added in safe_fields.rs for 7 types where
the `#[json_safe_fields]` macro doesn't reach (variant-internal u64s
or wrapper enums with manual `impl JsonConvertible`):
- DocumentTransition, TokenTransition, BatchedTransition (wrapper
enums - their inner V0 leaves are all json_safe_fields-annotated,
so safe by induction)
- TokenEmergencyAction (unit-variant enum)
- TokenDistributionType (unit-variant enum)
- TokenPricingSchedule (escape-hatch pattern - tuple variants hold
Credits / BTreeMap<TokenAmount, Credits>; matches existing
TokenEvent pattern)
- TokenConfigurationChangeItem (escape-hatch - tuple variants hold
Option<TokenAmount> / Option<GroupContractPosition>)
Also: made DocumentTransition / TokenTransition enum re-exports public
in batch_transition/batched_transition/mod.rs (were `use`, now `pub use`)
so safe_fields.rs can name them via `crate::...::batched_transition::X`.
Verification:
cargo test -p dpp --features all_features_without_client --lib
-> 3594 passed, 0 failed, 8 ignored
cargo check -p drive -p wasm-dpp -p wasm-dpp2 -p dash-sdk -p drive-abci --tests
clean (only pre-existing warnings)
Plan / memory updated to mark step 9 follow-up done. Phase D steps
1-9 + follow-up are now complete; remaining: step 10 (DataContract
KEEP-AS-EXCEPTION rename pass) and step 11 (AddressWitness /
ContestedIndexFieldMatch manual-impl refactor).
…dexFieldMatch
Phase D step 11 — replace custom Serialize/Deserialize impls with serde
attributes, gated on round-trip + wire-shape parity tests.
AddressWitness (address_funds/witness.rs):
- Replaced ~115 lines of manual serde with `#[serde(tag = "type")]`
internal tagging.
- Explicit `rename = "p2pkh"` / `rename = "p2sh"` on variants
(camelCase rule is ambiguous for `P2pkh` / `P2sh`).
- `redeem_script` field gets explicit `rename = "redeemScript"`.
- Behavior change: `MAX_P2SH_SIGNATURES` no longer enforced on the
JSON/Value deserialize path. The bincode `Decode` impl still
enforces it (the load-bearing wire format). Documented in the
type's doc comment. JSON/Value is dev/SDK-facing; downstream
consumers must validate signature counts before re-serializing.
- Wire shape unchanged. The existing 4 round-trip tests in
`json_convertible_tests` (P2PKH / P2SH x JSON / Value) keep
passing — byte-for-byte parity confirmed.
ContestedIndexFieldMatch (data_contract/document_type/index/mod.rs):
- Replaced ~95 lines of manual serde with
`#[serde(rename_all = "snake_case")]` externally-tagged enum.
- LazyRegex gets `serde(from = "String", into = "String")` so it
round-trips as a bare string (the `regex: OnceLock<Regex>` field
is reconstructed lazily on use).
- Bug fix: previous custom Serialize emitted `{"Regex": ...}`
(PascalCase) while custom Deserialize expected `{"regex": ...}`
(snake_case) — the type was non-round-trippable through serde.
New impl is consistently snake_case in both directions.
- No production callers identified — production data-contract
loading uses the unrelated Value-walking `regexPattern` path.
- Added 4 round-trip + wire-shape parity tests:
`json_round_trip_contested_index_field_match_regex`
`json_round_trip_contested_index_field_match_positive_integer`
`value_round_trip_contested_index_field_match_regex`
`value_round_trip_contested_index_field_match_positive_integer`
First two assert exact JSON shape `{"regex": "..."}` /
`{"positive_integer_match": N}`.
Verification:
cargo test -p dpp --features all_features_without_client --lib
-> 3598 passed (was 3594; +4 new), 0 failed, 8 ignored
cargo check -p drive -p wasm-dpp -p wasm-dpp2 -p dash-sdk -p drive-abci --tests
clean (only pre-existing warnings)
Net: ~-210 lines, both types now go through pure serde derive. Phase D
steps 4-9 + 11 all complete; only step 10 (DataContract — KEEP-AS-EXCEPTION
rename pass) remains in the deprecation order.
Step 11 follow-up: my initial migration picked snake_case for the
ContestedIndexFieldMatch wire shape, justified as "preserve what the
existing Deserialize accepted." That was the wrong call:
1. The previous serde impl was already non-round-trippable (Serialize
emitted PascalCase, Deserialize expected snake_case), so there's no
real wire shape to preserve.
2. There are zero production callers — data-contract loading uses an
unrelated Value-walking `regexPattern` path.
3. The codebase convention is camelCase for JSON wire shapes
(`$formatVersion`, `redeemScript`, etc.). snake_case sticks out.
Changed `serde(rename_all = "snake_case")` -> `serde(rename_all =
"camelCase")` on the enum. Updated the parity test fixture from
`{"positive_integer_match": 42}` to `{"positiveIntegerMatch": 42}`.
The `Regex` variant is unchanged (single-word lowercase identical
between snake_case and camelCase).
Verified: 3598/3598 dpp lib tests pass.
…al-4
Phase D step 10 — three pieces landing together:
1. Critical-4 pinned in regression tests
(data_contract/conversion/serde/mod.rs::data_contract_serde_pins_critical_4):
- data_contract_round_trips_through_serde_json: serde_json round-trip
works at the active platform version.
- data_contract_serialize_matches_serialization_format_at_current_version:
DataContract::serialize is byte-equivalent to
DataContractInSerializationFormat::serialize at LATEST_PLATFORM_VERSION
— proves the manual impl is a thin format-routing wrapper, not a
custom shape.
- data_contract_deserialize_rejects_invalid_schema_via_full_validation:
DataContract::deserialize rejects a contract with an indices entry
referencing a nonexistent property — proves the hardcoded
full_validation=true runs on the JSON/Value/CBOR ingest path.
Module-level doc explains the impurity rationale (Critical-4 in the
unification plan) so future readers / refactors understand the
constraint.
2. Method rename pass to disambiguate from canonical traits:
DataContractJsonConversionMethodsV0::
from_json -> from_json_versioned
to_json -> to_json_versioned
to_validating_json (unchanged — no clash)
DataContractValueConversionMethodsV0::
from_value -> from_value_versioned
to_value -> to_value_versioned
into_value -> into_value_versioned
The `_versioned` suffix makes it visually obvious that these are NOT
canonical JsonConvertible::to_json (zero args) but the version-aware
path that takes &PlatformVersion and a full_validation flag. ~26 call
sites updated across rs-dpp / rs-drive / rs-drive-abci / wasm-dpp /
wasm-dpp2 / dash-sdk / rs-sdk-ffi.
3. KEEP-AS-EXCEPTION rationale documented at three sites:
- conversion/json/v0/mod.rs (trait def)
- conversion/value/v0/mod.rs (trait def)
- data_contract/mod.rs:112-120 (outer enum, already had a comment;
updated to reference the new method names)
The traits stay because DataContract is a versioned enum routed
through DataContractInSerializationFormat. Both the platform version
and full_validation flag are inputs to the conversion that canonical
`JsonConvertible` / `ValueConvertible` (with their parameter-free
signatures) cannot express. The rename UNBLOCKS adding canonical
traits in the future if we choose to (no longer ambiguous), but
doesn't do so now.
Drive-by: rs-sdk-ffi/src/document/queries/search.rs needed an explicit
`use ValueConvertible;` import — pre-existing E0599 surfaced during the
verification cargo check across the workspace, fixed inline since it
was a one-line block on getting the workspace clean.
Verification:
cargo test -p dpp --features all_features_without_client --lib
-> 3601 passed, 0 failed, 8 ignored (was 3598; +3 from new
Critical-4 pin tests)
cargo check -p dpp -p drive -p drive-abci -p wasm-dpp -p wasm-dpp2 -p dash-sdk -p rs-sdk-ffi --tests
clean (only pre-existing warnings)
Phase D unification plan steps 1-11 are now complete. The plan's
"deprecate non-canonical mechanisms" pass is done for all non-DataContract
types; DataContract intentionally retains its version-aware traits as
the documented exception.
Step 10 follow-up. Flips the hardcoded full_validation=true in the
manual `Deserialize` impl on DataContract to false, making schema
validation opt-in instead of a hidden default.
Why:
- Most production callsites load already-validated contracts from
storage and were paying validation cost twice.
- Hidden behavior in serde Deserialize doesn't match the rest of the
codebase's serde semantics (where Deserialize means "structurally
well-formed").
- Trust-but-verify boundaries (SDK ingest, JSON fixtures, gRPC
handlers) were already plumbing the `full_validation` flag
explicitly through `from_*_versioned` and don't depend on the
implicit canonical-Deserialize default.
Audit confirmed canonical `Deserialize` had zero production callers
depending on its implicit validation: only ExtendedDocument's nested
round-trip and the Critical-4 pin tests themselves exercised it.
Production callers using `from_*_versioned(_, true, _)` keep working
unchanged (they explicitly opt in); those using `from_*_versioned(_,
false, _)` keep working unchanged (they explicitly opt out); only the
canonical path semantics change, and they change to match the rest of
the codebase.
Critical-4 pin test renamed
`data_contract_deserialize_does_not_validate_by_default` and now
asserts both halves of the new contract:
- canonical `serde_json::from_value::<DataContract>(...)` ACCEPTS a
contract with an invalid `indices` schema entry (no validation).
- explicit `DataContract::from_json_versioned(_, true, _)` REJECTS
the same payload (opt-in validation runs).
Module-level doc on conversion/serde/mod.rs updated:
- "Validation policy: opt-in, not default" section explains the new
contract.
- Critical-4 framing updated to focus on the platform-version coupling
(which stays) rather than the hardcoded validation (which is gone).
Verification:
cargo test -p dpp --features all_features_without_client --lib
-> 3601 passed, 0 failed, 8 ignored (no count change)
cargo check -p drive -p drive-abci -p wasm-dpp -p wasm-dpp2 -p dash-sdk -p rs-sdk-ffi --tests
clean (only pre-existing warnings)
Plan + memory updated to reflect the four-piece Step 10 landing
(Critical-4 pin + rename + KEEP-AS-EXCEPTION docs + validation flip).
User-directed cleanup of step 10's earlier rename pass. The `_versioned`
naming was misleading — every conversion path uses the platform version,
including canonical (via `get_current()`). The actual semantic split is
**validation: yes / no**.
Final API:
WITHOUT validation (the cheap path):
serde_json::from_value::<DataContract>(...)
platform_value::from_value::<DataContract>(...)
serde_json::to_value(&dc)
platform_value::to_value(&dc)
WITH validation (opt-in, trust-boundary path):
DataContract::from_json_validated(json, &pv)
DataContract::from_value_validated(value, &pv)
Kept (different concept):
to_validating_json(&pv) — JSON Schema-compatible output with
binary fields as u8 arrays
Deleted entirely:
- DataContractJsonConversionMethodsV0::from_json_versioned (had a
bool full_validation that's now baked into the method name choice)
- DataContractJsonConversionMethodsV0::to_json_versioned (canonical
serde_json::to_value does the same via get_current())
- DataContractValueConversionMethodsV0::from_value_versioned
- DataContractValueConversionMethodsV0::to_value_versioned
- DataContractValueConversionMethodsV0::into_value_versioned
Caller migration (~30 sites across rs-dpp / rs-drive / rs-drive-abci /
wasm-dpp / wasm-dpp2 / dash-sdk / rs-sdk-ffi):
- from_*_versioned(value, true, &pv) -> from_*_validated(value, &pv)
- from_*_versioned(value, false, &pv) -> canonical serde/platform_value
from_value (caller's pv is
usually current; for the few
sites that need pv to control
variant dispatch, deserialize
via DataContractInSerializationFormat
then try_from_platform_versioned
— same internal shape the old
method had)
- from_*_versioned(value, dynamic_bool, &pv) -> if/else bridge between
validated and canonical
(preserves dynamism)
- to_*_versioned(&pv) / into_value_versioned(&pv) -> canonical to_value /
serde_json::to_value
(DataContract doesn't
implement canonical
JsonConvertible/
ValueConvertible directly
per its doc comment;
use the function form)
Notable judgment call: tests/json_document.rs test-fixture loader needs
caller-provided &pv to control which DataContract variant comes back
(history-replay scenarios). Bridge's else-branch deserializes through
DataContractInSerializationFormat (serde(tag = "$formatVersion") enum
dispatches by tag), then calls try_from_platform_versioned(format,
false, &mut vec![], pv) for caller-pv variant dispatch. This preserves
the exact internal shape the old from_json_versioned(value, false, pv)
had.
Verification:
cargo test -p dpp --features all_features_without_client --lib
-> 3601 passed, 0 failed, 8 ignored (no count change)
cargo check -p dpp -p drive -p drive-abci -p wasm-dpp -p wasm-dpp2 -p dash-sdk -p rs-sdk-ffi --tests
clean
Plan + memory updated. Phase D unification work complete; DataContract
no longer has the misleading `_versioned` ceremony — validation is the
real axis, and it's expressed in the type system via method name.
…verage
Phase D step 9 deleted `make_withdrawal_v1_no_script` along with its
2 tests when removing the legacy StateTransitionValueConvert traits.
The unique V1 behavior these covered — `output_script: Option<CoreScript>`
round-tripping with `None` — was no longer exercised; the surviving
`json_convertible_tests::fixture` only covers the V0 variant with
a mandatory script.
Added in `identity_credit_withdrawal_transition/mod.rs::json_convertible_tests`:
- fixture_v1_no_script() — V1 variant with output_script: None
- json_round_trip_v1_with_none_output_script — locks the JSON wire
shape (`"outputScript": null`, `pooling: "standard"`, all sized-int
fields explicit)
- value_round_trip_v1_with_none_output_script — locks the
platform_value wire shape (`Pooling::Standard` -> U8(2), Null variant)
Lessons learned during fixture creation:
- Pooling discriminants: Never=0, IfAvailable=1, Standard=2.
- `platform_value!` macro doesn't resolve bare `Null` — needs the
qualified `platform_value::Value::Null`.
- serde_json::Value object comparisons in `assert_eq!` panic with
debug-identical-looking output when there's a single-char difference
in a long base64 string. Use `echo -n "..." | wc -c` +
`printf '\xXX%.0s' {1..N} | base64` to verify the expected encoding
before committing — the test loop iterates much faster than guessing.
Verification:
cargo test -p dpp --features all_features_without_client --lib
-> 3603 passed, 0 failed, 8 ignored (was 3601; +2 new)
Plan doc updated: marked this coverage gap done, left the remaining
two flagged Phase D follow-ups (unknown $formatVersion error coverage,
json_preserves_format_version_tag symmetry on DataContractUpdateTransition).
…dispatch
Adds representative coverage for the canonical serde-tag dispatch error
path. There are ~70 `$formatVersion`-tagged outer enums in rs-dpp; their
unknown-version handling is uniform (serde's enum dispatch errors on any
tag value not matching a declared variant), so per-enum tests would be
mechanical noise.
One good representative on `IdentityCreditWithdrawalTransition` (the
multi-variant V0+V1 enum) demonstrates the contract for both directions:
from_json_rejects_unknown_format_version — JsonConvertible path
from_object_rejects_unknown_format_version — ValueConvertible path
Both feed a structurally-well-formed payload with `$formatVersion: "99"`
and assert the error. If a future refactor switches an outer enum away
from `serde(tag = "$formatVersion")` (e.g., to untagged or
externally-tagged), unknown-version inputs would silently match a
variant via field-shape inference instead of erroring — this test fails
loudly when that happens.
Per the plan doc this closes the "unknown $formatVersion error coverage"
follow-up. Only `json_preserves_format_version_tag` symmetry on
DataContractUpdateTransition remains in the branch-scoped Phase D
follow-ups.
Verification:
cargo test -p dpp --features all_features_without_client --lib
-> 3605 passed, 0 failed, 8 ignored (was 3603; +2 new)
The plan flagged this as a missing test on DataContractUpdateTransition. Auditing the code confirms it's already present at data_contract_update_transition/mod.rs:297, matching the create twin. The earlier note was stale. All branch-scoped Phase D follow-ups are now closed. Remaining items (Critical-1 docstrings, Critical-2 explicit array→bytes test) are separate non-Phase-D work, not gated on this branch shipping.
…lueConvertible Adds Critical-1 documentation to both canonical traits in serialization/serialization_traits.rs. The two traits LOOK like mirrors but serialize in different is_human_readable() modes (serde_json = HR true; platform_value = HR false), which means types that branch on is_human_readable() in their Serialize impl produce structurally different output between the paths: to_json: "5bV6jUfh..." (bs58 string) to_object: Value::Identifier(...) (raw bytes variant) A reader looking at the trait signatures naturally assumes self.to_object().try_into_json() ≡ self.to_json() — false for any binary-bearing type. We hit this concretely on this branch during Step 8 (to_pretty_json), Critical-3 (ExtendedDocument $entropy round-trip), and the ContentDeserializer-quirk fixes (Bytes32, serde_bytes, OutPoint). The new doc block on each trait covers: - The HR/non-HR mode difference with a Type × Path table for the affected types (Identifier, BinaryData, Bytes20/32/36, CoreScript). - "Do not assume to_object().try_into_json() ≡ to_json()" warning. - The ContentDeserializer caveat: serde's internal Content type used by #[serde(tag = "...")] enums always reports is_human_readable: true regardless of the original source. Manual Deserialize impls that branch on is_human_readable() need a dual-shape visitor in the HR branch via deserialize_any. - Pointers to Bytes32::deserialize and serde_bytes.rs as canonical examples of the dual-shape visitor recipe. The original G3 task also called for a property test enforcing to_json() vs to_object().try_into() equivalence across all types. That's deferred: adding equivalence checks across ~80 types is high-cost / low-marginal-value when the divergences are intentional (JSON has no native byte type) and the existing per-type round-trip tests already catch concrete regressions. The docstring is the high-leverage part — future contributors find the rule at the only place they'd naturally look. Plan doc updated to mark G3 done with the deferral note for the property test.
The `From<JsonValue> for Value` impls had a JS-DPP-era heuristic that
silently reclassified any JSON array of length ≥ 10 with all u8-range
elements as `Value::Bytes`. Source carried a `//todo: hacky solution,
to fix` comment.
The heuristic was load-bearing only for JS-DPP-style clients sending
binary as `[u8, ...]` arrays. After the canonical-trait unification:
HR (serde_json): binary → base64 string
nHR (platform_value): binary → Value::Bytes
BinaryData/Identifier/Bytes* Deserialize impls accept both forms.
The heuristic was no longer needed and was actively corrupting
genuine document properties typed as `array of small integers` of
length 10+ — they'd silently become Value::Bytes, then round-trip
back to JSON as a base64 string instead of the original array.
Fix:
- Removed the heuristic from both `From<JsonValue> for Value` impls
(owned + borrowed) in `rs-platform-value/src/converter/serde_json.rs`.
Conversion is now faithful: JSON array → Value::Array.
- Migrated 2 test fixtures in rs-dpp that depended on it:
- `extended_document/mod.rs::json_should_generate_human_readable_binaries`
- `document_create_transition/v0/mod.rs::convert_to_object_from_json_value_with_dynamic_binary_paths`
Both used `vec![u8; N]` literals in json!() macros; migrated to
base64 strings for binary fields and bs58 strings for identifier-typed
fields (which the contract schema marks via contentMediaType).
- Pin tests in platform-value:
- `from_json_array_10_u8_range_stays_array_not_bytes` — old assertion
flipped (was `..._becomes_bytes`, now asserts array form).
- `from_json_array_all_255_stays_array_not_bytes` — same.
- `from_json_long_byte_like_array_stays_array_not_bytes` — 1000-element
round-trip pin (JSON array → Value → JSON), proving no silent
corruption.
- `from_json_ref_array_stays_array_not_bytes` — borrowed-variant mirror.
Audit method: removed the heuristic, ran `cargo test --workspace`,
categorized breakage. Production paths (drive/drive-abci/wasm-dpp/
wasm-dpp2/dash-sdk/rs-sdk-ffi) all clean — they already use canonical
encodings. The 2 broken fixtures were the entire migration cost.
Notable judgment call in test #2: the data_contract fixture's field
names are misleading — `alphaBinary` has `contentMediaType: identifier`
in the schema (decoded as bs58) and `alphaIdentifier` is plain
byteArray (decoded as base64). Added a comment in the test explaining
this.
Verification:
cargo test -p platform-value --lib
-> 1036 passed, 0 failed
cargo test -p dpp --features all_features_without_client --lib
-> 3605 passed, 0 failed, 8 ignored
cargo check -p dpp -p drive -p drive-abci -p wasm-dpp -p wasm-dpp2 -p dash-sdk -p rs-sdk-ffi --tests
clean (only pre-existing warnings)
Plan doc updated: Critical-2 marked RESOLVED. G2 prerequisite marked
DONE. Critical-2 was the last unfixed Critical finding in §3.0.
Updates the plan-doc header to reflect the final state after Critical-2 landed: - Status line: passes 1 + 2 + tag-shape + Phase D (all 11 steps) + all 5 Critical findings complete. - Pass 3 row: DataContract family final shape documented (canonical + from_*_validated; no more _versioned). - New 'All 5 Critical findings resolved' table — single glance to confirm none remain open. - Date bump to 2026-05-11. No code changes.
Re-survey on 2026-05-11 found 17 callers in wasm-dpp2, all
in state_transitions/proof_result/{voting,token,shielded,identity}.rs.
Verified each inner type (VerifiedShieldedPoolState, VerifiedIdentity,
VerifiedTokenBalance, etc.) is defined IN wasm-dpp2 — no rs-dpp
counterpart exists.
This matches Phase E's earlier conclusion exactly: these are wasm-only
DTOs decomposing StateTransitionProofResult tuple variants into
named-field JS classes. Migrating would require inventing rs-dpp types
just for proof-result decomposition, with no reuse outside the wasm
boundary.
The Pass 4 row header was the only stale piece — said 'not started'
when Phase E had already done the work and reached the verdict.
Updated to reflect: migration done within reach; the 17 remaining
are confirmed-infeasible (with the count + locations re-verified
today).
…ible-address-transitions
dashcore PRs #708 (OutPoint dual-shape visitor) and #729 (hashes::serde_macros::SerdeHash dual-shape visitor) merged upstream on 2026-05-06. The v3.1-dev base branch already bumped the dashcore rev (d6dd5da1) to include both fixes; merging v3.1-dev into this branch pulled the new rev. The local workarounds we'd added on this branch are no longer needed: - Deleted the local `outpoint_serde` mod in `chain/chain_asset_lock_proof.rs` (was ~150 lines). dashcore's upstream Deserialize now correctly handles `ContentDeserializer` HR-quirk for OutPoint through tagged enums. - Unignored `Validator::value_round_trip_with_full_wire_shape` — the ProTxHash/PubkeyHash hex/bytes dual-shape is now upstream via #729. - Re-ignored `ValidatorSet::value_round_trip_with_full_wire_shape` with an updated comment: the test still fails because its fixture has a non-None BLS `threshold_public_key`, which routes through the local `bls_pubkey_serde` wrapper. That wrapper depends on a separate blstrs_plus upstream PR (not dashcore #708/#729). Once blstrs_plus lands, drop the wrapper and unignore this test too. Drive-by: the merge from v3.1-dev introduced `test_countable_allowing_offset_variant_end_to_end` in `drive/src/query/drive_document_count_query/tests.rs` which used the now-deleted `DataContract::from_json(_, false, _)` legacy method. Migrated to canonical `serde_json::from_value::<DataContract>(...)` (no-validation default matches the false flag the test passed). Verification: cargo test -p dpp --features all_features_without_client --lib -> 3619 passed, 0 failed, 7 ignored (was 3618 post-merge with 8 ignored; 1 unignore + zero new failures) cargo check -p drive -p drive-abci -p wasm-dpp -p wasm-dpp2 -p dash-sdk -p rs-sdk-ffi --tests clean (only pre-existing warnings) Plan doc updated: final test count refreshed to 3619/7. Upstream PRs status section reflects: dashcore #708/#729 merged + integrated; blstrs_plus still pending.
Collaborator
Review GateCommit:
|
The Rust workspace `Tests (macOS)` CI job was failing on
`cargo fmt --check` — 25 files had formatting drift accumulated
across the long-running unification work. Auto-fixed via
`cargo fmt --all`. No behavior changes.
Verified after:
cargo fmt --all -- --check clean
cargo test -p dpp --features all_features_without_client --lib
-> 3619 passed (unchanged)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Unification of JSON/Value conversion across rs-dpp + wasm-dpp2: canonical
JsonConvertible/ValueConvertibletraits on every domain type, ~80 trait impls + ~200 round-trip tests, and a coordinated deprecation sweep that removed all 5 documented Critical bugs and most legacy non-canonical conversion mechanisms.What landed (high-level)
JsonConvertible/ValueConvertibleimpls on ~80 rs-dpp domain types.json_convertible_tests/value_convertible_testsmodules with full wire-shape assertions per type.StateTransitionValueConvert/StateTransitionJsonConverttraits + 68 impl files, A6/A7/A8/A9 identity traits, A10/A11 document traits down to one helper each, AssetLockProof asymmetric methods, and the_versionedDataContract method family. Replaced with canonical + (for DataContract)from_*_validated(value, &pv)opt-in validation.outpoint_serdewrapper. The blstrs_plus PR is still pending (oneValidatorSetvalue-round-trip test remains#[ignore]).Critical findings status
is_human_readableHR/non-HR divergenceserialization_traits.rswith the divergence table +ContentDeserializercaveat + pointer to canonical dual-shape visitor examples.array→bytescoercion inFrom<JsonValue> for Valuers-platform-value/src/converter/serde_json.rs). Faithful conversion: JSON array →Value::Array. Pin tests added.ExtendedDocumentnon-round-trippable#[serde(tag = "$extendedFormatVersion")]derive; round-trip tests added.DataContractserde impurityDeserializeno longer hardcodesfull_validation=true); KEEP-AS-EXCEPTION rationale documented at all 3 sites.to_canonical_objectsorts keys (assumed signature-load-bearing)PlatformSignablederive). Methods had zero production callers; deleted along with A1/A2.DataContract API — final shape
The deprecation sweep collapsed the
_versionedmethod family into a clear split by validation policy:serde_json::from_value::<DataContract>(json)/platform_value::from_value::<DataContract>(v)/serde_json::to_value(&dc)/platform_value::to_value(&dc). Use for storage reads, internal round-trips.DataContract::from_json_validated(json, &pv)/from_value_validated(value, &pv). Use on trust boundaries (SDK ingest, fixture loaders). No bool param — name implies always-validates.to_validating_json(&pv)— different concept (produces JSON-Schema-compatible output with binary as u8 arrays).to_*_versioned,into_value_versioned,from_*_versioned(_, full_validation, _).Test results
recursive_schema_validatorignores; 1 isValidatorSet::value_round_trip_with_full_wire_shape(pending blstrs_plus upstream PR).cargo check --testsclean acrossdpp/drive/drive-abci/wasm-dpp/wasm-dpp2/dash-sdk/rs-sdk-ffi.Architectural conventions established
#[serde(tag = "$formatVersion")]; all discriminated enums use a$-prefixed key ($type,$action,$transition); zero adjacent-tagged enums remain.#[json_safe_fields]rollout: 25+ V0/V1 struct leaves carry the attribute. JS-safe large-integer protection at every serialization site.json!{}/platform_value!{}literal assertions in every round-trip test (~85 tests on this convention).impl_wasm_conversions_inner!(45 sites in wasm-dpp2) for rs-dpp domain types using canonical traits;impl_wasm_conversions_serde!(20 sites) for wasm-only DTOs without rs-dpp counterparts — pattern documented and re-audited.Cross-package audit (just before shipping)
Serialize/Deserialize, 0 references to deleted legacy APIs, 38impl_wasm_serde_conversions!applications. All DTOs follow canonical patterns.IdentifierWasm,PoolingWasm,PlatformAddressWasm) — all documented production-required adapters for lenient JS-facing parsing; rest of crate flows through canonical helpers.Out of scope (separate work)
PublicKeydual-shape deserialize — pending upstream. OneValidatorSetvalue-round-trip test remains#[ignore]until it lands.wasm-dppcrate) — blocked on team decision.Docs
docs/json-value-unification-plan.md— the live plan + status doc (regularly updated through the work).docs/json-value-conversion-inventory.md— pre-pass-1 structural inventory.docs/json-value-conversion-canonical-pattern.md— the canonical-trait usage pattern, kept up to date.Test plan
cargo test -p dpp --features all_features_without_client --liband sees 3619 pass / 0 fail.docs/json-value-unification-plan.mdto confirm the architectural decisions (validation-opt-in, KEEP-AS-EXCEPTION for DataContract, tag-shape conventions) match team intent.IdentityCreditWithdrawalTransitionWasm) round-trips correctly from the JS SDK perspective.🤖 Generated with Claude Code