feat(dpp): unify JSON/Value conversion traits + comprehensive round-trip tests

[shumkov]

Summary

Unification of JSON/Value conversion across rs-dpp + wasm-dpp2: canonical JsonConvertible / ValueConvertible traits 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)

• Pass 1: canonical JsonConvertible / ValueConvertible impls on ~80 rs-dpp domain types.
• Pass 2: ~200 dedicated json_convertible_tests / value_convertible_tests modules with full wire-shape assertions per type.
• Phase D (steps 1–11): deprecation and deletion of non-canonical conversion mechanisms. Removed StateTransitionValueConvert/StateTransitionJsonConvert traits + 68 impl files, A6/A7/A8/A9 identity traits, A10/A11 document traits down to one helper each, AssetLockProof asymmetric methods, and the _versioned DataContract method family. Replaced with canonical + (for DataContract) from_*_validated(value, &pv) opt-in validation.
• All 5 Critical findings resolved (see table below).
• Upstream PRs landed — dashcore fix: Starcounter-Jack JSON-Patch Prototype Pollution vulnerability #708 + feat(drive): log number of refunded epochs #729 merged; this branch dropped the local outpoint_serde wrapper. The blstrs_plus PR is still pending (one ValidatorSet value-round-trip test remains #[ignore]).

Critical findings status

#	Finding	Resolution
Critical-1	is_human_readable HR/non-HR divergence	Documented on both canonical traits in serialization_traits.rs with the divergence table + ContentDeserializer caveat + pointer to canonical dual-shape visitor examples.
Critical-2	Silent array→bytes coercion in From<JsonValue> for Value	Heuristic removed (rs-platform-value/src/converter/serde_json.rs). Faithful conversion: JSON array → Value::Array. Pin tests added.
Critical-3	ExtendedDocument non-round-trippable	Replaced broken manual serde with #[serde(tag = "$extendedFormatVersion")] derive; round-trip tests added.
Critical-4	DataContract serde impurity	Platform-version coupling pinned in 3 regression tests; validation flipped to opt-in (Deserialize no longer hardcodes full_validation=true); KEEP-AS-EXCEPTION rationale documented at all 3 sites.
Critical-5	to_canonical_object sorts keys (assumed signature-load-bearing)	Falsified — signing uses bincode (PlatformSignable derive). Methods had zero production callers; deleted along with A1/A2.

DataContract API — final shape

The deprecation sweep collapsed the _versioned method family into a clear split by validation policy:

• No validation (the new default): canonical 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.
• Opt-in validation: 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.
• Kept: to_validating_json(&pv) — different concept (produces JSON-Schema-compatible output with binary as u8 arrays).
• Deleted entirely: to_*_versioned, into_value_versioned, from_*_versioned(_, full_validation, _).

Test results

• rs-dpp: 3619/3619 lib tests pass, 0 failed, 7 ignored. Of the 7 ignored: 6 are pre-existing recursive_schema_validator ignores; 1 is ValidatorSet::value_round_trip_with_full_wire_shape (pending blstrs_plus upstream PR).
• rs-platform-value: 1036/1036 lib tests pass.
• rs-drive: 3040/3040 lib tests pass.
• rs-sdk: 117/117 lib tests pass.
• rs-sdk-ffi: 252/252 lib tests pass.
• Workspace cargo check --tests clean across dpp / drive / drive-abci / wasm-dpp / wasm-dpp2 / dash-sdk / rs-sdk-ffi.

Architectural conventions established

• Tag-shape rules: all versioned outer enums use #[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.
• Wire-shape test convention: json!{} / platform_value!{} literal assertions in every round-trip test (~85 tests on this convention).
• Wasm-side adapter pattern: 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)

• wasm-sdk: 0 manual Serialize/Deserialize, 0 references to deleted legacy APIs, 38 impl_wasm_serde_conversions! applications. All DTOs follow canonical patterns.
• wasm-dpp2: 3 manual serde impls (IdentifierWasm, PoolingWasm, PlatformAddressWasm) — all documented production-required adapters for lenient JS-facing parsing; rest of crate flows through canonical helpers.
• rs-sdk / rs-sdk-ffi / swift-sdk: no breakage; no references to deleted APIs.

Out of scope (separate work)

• blstrs_plus BLS PublicKey dual-shape deserialize — pending upstream. One ValidatorSet value-round-trip test remains #[ignore] until it lands.
• Pass 5 (delete the legacy wasm-dpp crate) — 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

• CI green (currently 3 success / 12 skipped / 0 failed).
• Reviewer runs cargo test -p dpp --features all_features_without_client --lib and sees 3619 pass / 0 fail.
• Reviewer skims docs/json-value-unification-plan.md to confirm the architectural decisions (validation-opt-in, KEEP-AS-EXCEPTION for DataContract, tag-shape conventions) match team intent.
• Spot-check a wasm-dpp2 wrapper migration (e.g., IdentityCreditWithdrawalTransitionWasm) round-trips correctly from the JS SDK perspective.

🤖 Generated with Claude Code

[thepastaclaw]

Review Gate

Commit: b6110d68

• ✅ Debounce: 10301m ago (need 30m)

• ⏳ CI checks: build failure: JS packages (@dashevo/wasm-sdk) / Tests, JS packages (dash) / Tests, JS packages (@dashevo/dapi-client) / Tests

• ✅ CodeRabbit review: comment found

• ✅ Off-peak hours: off-peak (03:39 AM PT Tuesday)

• Run review now (check to override)

[codecov]

Codecov Report

❌ Patch coverage is 95.32520% with 207 lines in your changes missing coverage. Please review.
✅ Project coverage is 87.33%. Comparing base (12fd1a3) to head (5ae8c26).

Files with missing lines	Patch %	Lines
...ages/rs-dpp/src/serialization/json/safe_integer.rs	22.79%	105 Missing ⚠️
packages/rs-dpp/src/serialization/serde_bytes.rs	78.75%	17 Missing ⚠️
...s/rs-dpp/src/data_contract/conversion/serde/mod.rs	82.45%	10 Missing ⚠️
...ckages/rs-dpp/src/serialization/serde_bytes_var.rs	63.63%	8 Missing ⚠️
packages/rs-dpp/src/state_transition/mod.rs	95.59%	7 Missing ⚠️
...ed_transition/document_create_transition/v0/mod.rs	86.00%	7 Missing ⚠️
...ackages/rs-dpp/src/data_contract/factory/v0/mod.rs	70.00%	6 Missing ⚠️
...ges/rs-dpp/src/data_contract/v0/conversion/json.rs	0.00%	6 Missing ⚠️
...kages/rs-dpp/src/document/extended_document/mod.rs	95.65%	5 Missing ⚠️
...es/rs-dpp/src/data_contract/conversion/json/mod.rs	50.00%	4 Missing ⚠️
... and 18 more

Additional details and impacted files

@@             Coverage Diff              @@
##           v3.1-dev    #3573      +/-   ##
============================================
+ Coverage     87.16%   87.33%   +0.17%     
============================================
  Files          2607     2601       -6     
  Lines        319420   322859    +3439     
============================================
+ Hits         278413   281982    +3569     
+ Misses        41007    40877     -130     

Components	Coverage Δ
dpp	88.31% <95.32%> (+0.64%)	⬆️
drive	85.95% <ø> (+<0.01%)	⬆️
drive-abci	89.60% <ø> (-0.01%)	⬇️
sdk	∅ <ø> (∅)
dapi-client	∅ <ø> (∅)
platform-version	∅ <ø> (∅)
platform-value	92.77% <ø> (+0.60%)	⬆️
platform-wallet	∅ <ø> (∅)
drive-proof-verifier	49.16% <ø> (ø)

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[thepastaclaw]

Code Review

No prior findings were carried forward for PR #3573, so there are no STILL VALID, FIXED, OUTDATED, or INTENTIONALLY DEFERRED prior items to report. Fresh review of head 358fe02f9a618fc1358328d6d2ac552f85e417ec found four new suggestion-level correctness issues: two byte-deserialization gaps caused by the new JSON-array preservation, one silent float corruption path in the validating-JSON bridge, and one WASM boundary bug where the exposed platformVersion argument is ignored on non-validating contract ingest.

_Note: Inline posting failed (command failed (1): python3 scripts/review_poster.py dashpay/platform 3573 358fe02 --dry-run
STDOUT:

STDERR:
Traceback (most recent call last):
File "/Users/claw/.openclaw/workspace/scripts/review_poster.py", line 138, in
result = post_review(repo, pr), so I posted the same verified findings as a top-level review body._

Reviewed commit: 358fe02

🟡 4 suggestion(s)

Verified findings

suggestion: Fixed-size byte helper cannot deserialize `Value::Array` on the non-human-readable path

packages/rs-dpp/src/serialization/serde_bytes.rs (line 90)

The helper now documents that it accepts base64 strings, byte buffers, and u8 sequences regardless of is_human_readable(), but the non-human-readable branch still calls only deserialize_byte_buf. In platform_value, deserialize_byte_buf delegates to deserialize_bytes, which accepts Value::Bytes* and Identifier but rejects Value::Array. After this PR changed serde_json::Value::Array conversion to stay as Value::Array, any platform_value::from_value(...) call that targets a field using #[serde(with = "crate::serialization::serde_bytes")] will now fail for array-shaped byte input even though the helper advertises that shape as supported.

suggestion: `Vec` helper has the same non-human-readable `Value::Array` deserialization hole

packages/rs-dpp/src/serialization/serde_bytes_var.rs (line 68)

This helper has the same structural problem as the fixed-size variant. The human-readable branch uses deserialize_any, but the non-human-readable branch uses only deserialize_byte_buf, and platform_value does not map Value::Array through that entry point. As a result, platform_value::from_value(Value::Array([...])) cannot deserialize a Vec<u8> field annotated with #[serde(with = "crate::serialization::serde_bytes_var")], even though the helper comment says u8 sequences are accepted independent of the deserializer mode.

suggestion: Validating-JSON conversion silently rewrites non-finite floats to `0`

packages/rs-platform-value/src/converter/serde_json.rs (line 44)

try_into_validating_json and its borrowed counterpart call Number::from_f64(...).unwrap_or(0.into()). serde_json::Number::from_f64 returns None for NaN and infinities, so these paths silently replace an invalid float with 0 instead of returning an error. This is an actual data-corruption path, not just a test artifact: the new validating_json_float_nan_becomes_zero test pins the rewrite in place, and any caller using the validating-JSON bridge can observe a different value than the one stored in Value::Float.

suggestion: `DataContract.fromJSON` and `fromObject` ignore the caller-supplied `platformVersion` when `fullValidation` is false

packages/wasm-dpp2/src/data_contract/model.rs (line 282)

The WASM API exposes platformVersion on both entry points, but the non-validating branches deserialize with serde_json::from_value::<DataContract> and platform_value::from_value::<DataContract> directly. DataContract's manual Deserialize implementation does not use the caller-supplied version; it resolves through PlatformVersion::get_version_or_current_or_latest(None) instead. That makes the JS API asymmetric: fullValidation=true respects the explicit version, while fullValidation=false follows Rust global state, so the same JS input can be interpreted under a different contract structure version than the one the caller requested.

🤖 Prompt for all review comments with AI agents

These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.

- [SUGGESTION] In `packages/rs-dpp/src/serialization/serde_bytes.rs`:90-101: Fixed-size byte helper cannot deserialize `Value::Array` on the non-human-readable path
  The helper now documents that it accepts base64 strings, byte buffers, and `u8` sequences regardless of `is_human_readable()`, but the non-human-readable branch still calls only `deserialize_byte_buf`. In `platform_value`, `deserialize_byte_buf` delegates to `deserialize_bytes`, which accepts `Value::Bytes*` and `Identifier` but rejects `Value::Array`. After this PR changed `serde_json::Value::Array` conversion to stay as `Value::Array`, any `platform_value::from_value(...)` call that targets a field using `#[serde(with = "crate::serialization::serde_bytes")]` will now fail for array-shaped byte input even though the helper advertises that shape as supported.
- [SUGGESTION] In `packages/rs-dpp/src/serialization/serde_bytes_var.rs`:68-76: `Vec<u8>` helper has the same non-human-readable `Value::Array` deserialization hole
  This helper has the same structural problem as the fixed-size variant. The human-readable branch uses `deserialize_any`, but the non-human-readable branch uses only `deserialize_byte_buf`, and `platform_value` does not map `Value::Array` through that entry point. As a result, `platform_value::from_value(Value::Array([...]))` cannot deserialize a `Vec<u8>` field annotated with `#[serde(with = "crate::serialization::serde_bytes_var")]`, even though the helper comment says `u8` sequences are accepted independent of the deserializer mode.
- [SUGGESTION] In `packages/rs-platform-value/src/converter/serde_json.rs`:44-44: Validating-JSON conversion silently rewrites non-finite floats to `0`
  `try_into_validating_json` and its borrowed counterpart call `Number::from_f64(...).unwrap_or(0.into())`. `serde_json::Number::from_f64` returns `None` for `NaN` and infinities, so these paths silently replace an invalid float with `0` instead of returning an error. This is an actual data-corruption path, not just a test artifact: the new `validating_json_float_nan_becomes_zero` test pins the rewrite in place, and any caller using the validating-JSON bridge can observe a different value than the one stored in `Value::Float`.
- [SUGGESTION] In `packages/wasm-dpp2/src/data_contract/model.rs`:282-309: `DataContract.fromJSON` and `fromObject` ignore the caller-supplied `platformVersion` when `fullValidation` is false
  The WASM API exposes `platformVersion` on both entry points, but the non-validating branches deserialize with `serde_json::from_value::<DataContract>` and `platform_value::from_value::<DataContract>` directly. `DataContract`'s manual `Deserialize` implementation does not use the caller-supplied version; it resolves through `PlatformVersion::get_version_or_current_or_latest(None)` instead. That makes the JS API asymmetric: `fullValidation=true` respects the explicit version, while `fullValidation=false` follows Rust global state, so the same JS input can be interpreted under a different contract structure version than the one the caller requested.

[thepastaclaw]

Code Review

The latest delta from 358fe02f9a618fc1358328d6d2ac552f85e417ec to ba8a6d888ae9bbeceb78c5ff1d5ef5b7b31f7adc only adds an unrelated Clippy allow, so there are no new findings in the latest delta. Prior finding 1 (fixed-size byte helper array handling): STILL VALID; prior finding 2 (Vec<u8> helper array handling): STILL VALID; prior finding 3 (non-finite float conversion): STILL VALID; prior finding 4 (WASM platformVersion ignored on non-validating DataContract ingest): STILL VALID. The carried-forward findings below are all confirmed against the current head; there are no resolved prior findings and no additional issues introduced by this diff.

_Note: Inline posting failed (command failed (1): python3 scripts/review_poster.py dashpay/platform 3573 ba8a6d8 --dry-run
STDOUT:

STDERR:
Traceback (most recent call last):
File "/Users/claw/.openclaw/workspace/scripts/review_poster.py", line 138, in
result = post_review(repo, pr), so I posted the same verified findings as a top-level review body._

Reviewed commit: ba8a6d8

🟡 4 suggestion(s)

Verified findings

suggestion: Fixed-size byte helper still rejects `Value::Array` on the non-human-readable path

packages/rs-dpp/src/serialization/serde_bytes.rs (line 90)

The helper now claims to accept base64 strings, byte buffers, byte slices, and u8 sequences regardless of is_human_readable(), but the non-human-readable branch still hard-codes deserialize_byte_buf(). In platform_value, is_human_readable() is false, deserialize_byte_buf() is only an alias for deserialize_bytes(), and that matcher accepts Value::Bytes* and Value::Identifier but not Value::Array (packages/rs-platform-value/src/value_serialization/de.rs:334-358,497). As a result, JS/WASM object input that crosses the boundary as a numeric array still fails for [u8; N] fields even though the visitor itself supports sequences and the helper documentation says sequence input is accepted.

suggestion: `Vec` helper has the same non-human-readable `Value::Array` gap

packages/rs-dpp/src/serialization/serde_bytes_var.rs (line 68)

This helper has the same structural bug as the fixed-size version. Its human-readable branch uses deserialize_any(), but the non-human-readable branch still uses deserialize_byte_buf() only. On the platform_value path that means Value::Array is rejected before the visitor can consume it as a sequence, because deserialize_byte_buf() delegates to deserialize_bytes() and does not match arrays (packages/rs-platform-value/src/value_serialization/de.rs:334-358,497). That leaves plain JS byte arrays accepted on one ingress shape and rejected on the other, despite the helper claiming that sequence input is supported.

suggestion: JSON conversion still rewrites non-finite floats to `0` instead of rejecting them

packages/rs-platform-value/src/converter/serde_json.rs (line 44)

try_into_validating_json() and try_to_validating_json() still call Number::from_f64(...).unwrap_or(0.into()) at lines 44 and 140, and the general TryInto<JsonValue> path repeats the same fallback at line 289. serde_json::Number::from_f64() returns None for NaN and infinities, so these helpers silently change an invalid float into the integer 0 instead of surfacing an error. This is not hypothetical: the current test suite explicitly pins Value::Float(f64::NAN).try_into_validating_json() to json!(0), which confirms the lossy rewrite is still the intended runtime behavior at head. Because these paths are used to produce JSON for validation and interchange, silently mutating the payload can make invalid data look valid and breaks round-trip expectations.

suggestion: `DataContract.fromJSON` and `fromObject` still drop the caller-supplied `platformVersion` when validation is disabled

packages/wasm-dpp2/src/data_contract/model.rs (line 282)

Both WASM entry points accept a platformVersion, but the full_validation == false branch still deserializes with plain serde_json::from_value::<DataContract>() or platform_value::from_value::<DataContract>(). DataContract's manual Deserialize implementation does not use the caller-supplied version; it resolves protocol behavior internally with PlatformVersion::get_version_or_current_or_latest(None) and then calls try_from_platform_versioned(..., false, ..., current_version) (packages/rs-dpp/src/data_contract/conversion/serde/mod.rs:60-77). The documented keep-as-exception behavior in that serde module does not make this safe here, because these WASM APIs explicitly advertise a caller-controlled version parameter and then ignore it on the non-validating path. That means JS callers can request older-version semantics and still get current/latest decoding rules whenever they disable validation.

🤖 Prompt for all review comments with AI agents

These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.

- [SUGGESTION] In `packages/rs-dpp/src/serialization/serde_bytes.rs`:90-101: Fixed-size byte helper still rejects `Value::Array` on the non-human-readable path
  The helper now claims to accept base64 strings, byte buffers, byte slices, and `u8` sequences regardless of `is_human_readable()`, but the non-human-readable branch still hard-codes `deserialize_byte_buf()`. In `platform_value`, `is_human_readable()` is `false`, `deserialize_byte_buf()` is only an alias for `deserialize_bytes()`, and that matcher accepts `Value::Bytes*` and `Value::Identifier` but not `Value::Array` (`packages/rs-platform-value/src/value_serialization/de.rs:334-358,497`). As a result, JS/WASM object input that crosses the boundary as a numeric array still fails for `[u8; N]` fields even though the visitor itself supports sequences and the helper documentation says sequence input is accepted.
- [SUGGESTION] In `packages/rs-dpp/src/serialization/serde_bytes_var.rs`:68-75: `Vec<u8>` helper has the same non-human-readable `Value::Array` gap
  This helper has the same structural bug as the fixed-size version. Its human-readable branch uses `deserialize_any()`, but the non-human-readable branch still uses `deserialize_byte_buf()` only. On the `platform_value` path that means `Value::Array` is rejected before the visitor can consume it as a sequence, because `deserialize_byte_buf()` delegates to `deserialize_bytes()` and does not match arrays (`packages/rs-platform-value/src/value_serialization/de.rs:334-358,497`). That leaves plain JS byte arrays accepted on one ingress shape and rejected on the other, despite the helper claiming that sequence input is supported.
- [SUGGESTION] In `packages/rs-platform-value/src/converter/serde_json.rs`:44-44: JSON conversion still rewrites non-finite floats to `0` instead of rejecting them
  `try_into_validating_json()` and `try_to_validating_json()` still call `Number::from_f64(...).unwrap_or(0.into())` at lines 44 and 140, and the general `TryInto<JsonValue>` path repeats the same fallback at line 289. `serde_json::Number::from_f64()` returns `None` for `NaN` and infinities, so these helpers silently change an invalid float into the integer `0` instead of surfacing an error. This is not hypothetical: the current test suite explicitly pins `Value::Float(f64::NAN).try_into_validating_json()` to `json!(0)`, which confirms the lossy rewrite is still the intended runtime behavior at head. Because these paths are used to produce JSON for validation and interchange, silently mutating the payload can make invalid data look valid and breaks round-trip expectations.
- [SUGGESTION] In `packages/wasm-dpp2/src/data_contract/model.rs`:282-309: `DataContract.fromJSON` and `fromObject` still drop the caller-supplied `platformVersion` when validation is disabled
  Both WASM entry points accept a `platformVersion`, but the `full_validation == false` branch still deserializes with plain `serde_json::from_value::<DataContract>()` or `platform_value::from_value::<DataContract>()`. `DataContract`'s manual `Deserialize` implementation does not use the caller-supplied version; it resolves protocol behavior internally with `PlatformVersion::get_version_or_current_or_latest(None)` and then calls `try_from_platform_versioned(..., false, ..., current_version)` (`packages/rs-dpp/src/data_contract/conversion/serde/mod.rs:60-77`). The documented keep-as-exception behavior in that serde module does not make this safe here, because these WASM APIs explicitly advertise a caller-controlled version parameter and then ignore it on the non-validating path. That means JS callers can request older-version semantics and still get current/latest decoding rules whenever they disable validation.

[thepastaclaw]

Code Review

Three carried findings from the prior review are still present on 4ae0b95ddb71d47dacc8535de3af1bb0135caa9b: both byte deserializers still reject Value::Array on the non-human-readable path, and non-finite floats are still normalized to JSON 0. The prior DataContract.fromJSON/fromObject platform-version issue is still real at the code level, but the current head explicitly documents and tests that behavior as a deliberate keep-as-exception, so I classified it as intentionally deferred rather than an active review finding. I did not confirm any additional issue introduced by the ba8a6d88..4ae0b95d delta itself; the extra PartialIdentity key-ID coercion bug exists on the current head but already existed at ba8a6d88.

_Note: Inline posting failed (command failed (1): gh pr diff 3573 --repo dashpay/platform
STDOUT:

STDERR:
could not find pull request diff: HTTP 406: Sorry, the diff exceeded the maximum number of files (300). Consider using 'List pull requests files' API or locally cloning the repository instead. (https://api.github.com/repos/), so I posted the same verified findings as a top-level review body._

🔴 1 blocking | 🟡 3 suggestion(s)

4 additional finding(s)

blocking: Non-finite floats are validated as JSON `0` instead of being rejected

packages/rs-platform-value/src/converter/serde_json.rs (line 44)

Value::Float still converts through Number::from_f64(...).unwrap_or(0.into()) here and at lines 140 and 289. serde_json::Number::from_f64() returns None for NaN and infinities, so try_into_validating_json() rewrites those values to 0 before schema validation. That means validation runs against a different value than the original platform_value::Value, which is reachable from the JS/WASM boundary because js_value_to_platform_value() accepts arbitrary non-integer JS numbers as Value::Float. The current test at lines 1136-1139 pins this rewrite, so this is an active trust-boundary bug, not a hypothetical edge case.

suggestion: Fixed-size byte helper still rejects `Value::Array` on the non-human-readable path

packages/rs-dpp/src/serialization/serde_bytes.rs (line 90)

The visitor accepts visit_seq(), and the module comment says it accepts byte buffers, byte slices, base64 strings, and u8 sequences regardless of is_human_readable(). That is not true on the non-human-readable branch: it still calls deserialize_byte_buf() only. In platform_value deserialization, deserialize_byte_buf() delegates to deserialize_bytes() and accepts only Value::Bytes* and Value::Identifier, while Value::Array(...) is rejected before visit_seq() can run (packages/rs-platform-value/src/value_serialization/de.rs:334-358). JS object inputs that become Value::Array therefore still fail for fixed-size byte fields.

suggestion: `Vec` helper has the same `Value::Array` gap in non-human-readable deserialization

packages/rs-dpp/src/serialization/serde_bytes_var.rs (line 68)

This helper has the same structural bug as the fixed-size version. Its visitor supports visit_seq(), but the non-human-readable branch still hard-codes deserialize_byte_buf(). On the platform_value path, that rejects Value::Array(...) instead of delegating to deserialize_seq() (packages/rs-platform-value/src/value_serialization/de.rs:334-358), so JS number[] object inputs cannot reach the code that is supposed to accept byte sequences.

suggestion: `PartialIdentity` key-ID parsers silently coerce invalid numbers to different key IDs

packages/wasm-dpp2/src/identity/partial_identity.rs (line 330)

All three key-ID parsing helpers here bypass try_to_u32() and convert through f64 casts instead. option_array_to_not_found() accepts NaN, negative numbers, and fractional values and then casts them with as KeyID; value_to_loaded_public_keys_from_object() and value_to_loaded_public_keys_from_json() parse object-key strings as f64 and cast with as u32. In Rust, these casts silently saturate or truncate, so inputs such as NaN, -1, or 1.5 are remapped to different key IDs instead of being rejected. That can make PartialIdentity load or report the wrong public keys from caller-supplied JS data.

🤖 Prompt for all review comments with AI agents

These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.

- [BLOCKING] In `packages/rs-platform-value/src/converter/serde_json.rs`:44-44: Non-finite floats are validated as JSON `0` instead of being rejected
  `Value::Float` still converts through `Number::from_f64(...).unwrap_or(0.into())` here and at lines 140 and 289. `serde_json::Number::from_f64()` returns `None` for `NaN` and infinities, so `try_into_validating_json()` rewrites those values to `0` before schema validation. That means validation runs against a different value than the original `platform_value::Value`, which is reachable from the JS/WASM boundary because `js_value_to_platform_value()` accepts arbitrary non-integer JS numbers as `Value::Float`. The current test at lines 1136-1139 pins this rewrite, so this is an active trust-boundary bug, not a hypothetical edge case.
- [SUGGESTION] In `packages/rs-dpp/src/serialization/serde_bytes.rs`:90-101: Fixed-size byte helper still rejects `Value::Array` on the non-human-readable path
  The visitor accepts `visit_seq()`, and the module comment says it accepts byte buffers, byte slices, base64 strings, and `u8` sequences regardless of `is_human_readable()`. That is not true on the non-human-readable branch: it still calls `deserialize_byte_buf()` only. In `platform_value` deserialization, `deserialize_byte_buf()` delegates to `deserialize_bytes()` and accepts only `Value::Bytes*` and `Value::Identifier`, while `Value::Array(...)` is rejected before `visit_seq()` can run (`packages/rs-platform-value/src/value_serialization/de.rs:334-358`). JS object inputs that become `Value::Array` therefore still fail for fixed-size byte fields.
- [SUGGESTION] In `packages/rs-dpp/src/serialization/serde_bytes_var.rs`:68-75: `Vec<u8>` helper has the same `Value::Array` gap in non-human-readable deserialization
  This helper has the same structural bug as the fixed-size version. Its visitor supports `visit_seq()`, but the non-human-readable branch still hard-codes `deserialize_byte_buf()`. On the `platform_value` path, that rejects `Value::Array(...)` instead of delegating to `deserialize_seq()` (`packages/rs-platform-value/src/value_serialization/de.rs:334-358`), so JS `number[]` object inputs cannot reach the code that is supposed to accept byte sequences.
- [SUGGESTION] In `packages/wasm-dpp2/src/identity/partial_identity.rs`:330-424: `PartialIdentity` key-ID parsers silently coerce invalid numbers to different key IDs
  All three key-ID parsing helpers here bypass `try_to_u32()` and convert through `f64` casts instead. `option_array_to_not_found()` accepts `NaN`, negative numbers, and fractional values and then casts them with `as KeyID`; `value_to_loaded_public_keys_from_object()` and `value_to_loaded_public_keys_from_json()` parse object-key strings as `f64` and cast with `as u32`. In Rust, these casts silently saturate or truncate, so inputs such as `NaN`, `-1`, or `1.5` are remapped to different key IDs instead of being rejected. That can make `PartialIdentity` load or report the wrong public keys from caller-supplied JS data.

[thepastaclaw]

Code Review

All four prior findings remain valid on current head 123c00c3251946fa290af5939e531b7edc532ba6; none were fixed or intentionally deferred in the reviewed range. The 4ae0b95d..123c00c3 delta also introduces one new blocking regression by removing ExtendedDocument.toObject() / toJSON() from legacy wasm-dpp while current callers still invoke those methods.

_Note: Inline posting failed (command failed (1): gh pr diff 3573 --repo dashpay/platform
STDOUT:

STDERR:
could not find pull request diff: HTTP 406: Sorry, the diff exceeded the maximum number of files (300). Consider using 'List pull requests files' API or locally cloning the repository instead. (https://api.github.com/repos/), so I posted the same verified findings as a top-level review body._

🔴 2 blocking | 🟡 3 suggestion(s)

5 additional finding(s)

blocking: Non-finite floats are still silently rewritten to JSON `0`

packages/rs-platform-value/src/converter/serde_json.rs (line 44)

try_into_validating_json() still converts Value::Float with Number::from_f64(float).unwrap_or(0.into()), and the same fallback is still present in try_to_validating_json() at line 140 and TryInto<JsonValue> at line 289. serde_json::Number::from_f64() returns None for NaN and infinities, so these helpers corrupt non-JSON numeric values into the valid JSON number 0 instead of rejecting them. That makes validation and serialization lossy and can accept an invalid payload as if the caller had supplied zero.

suggestion: Fixed-size byte deserialization still blocks `Value::Array([u8, ...])` on the non-human-readable path

packages/rs-dpp/src/serialization/serde_bytes.rs (line 90)

The visitor is written to accept byte buffers, byte slices, base64 strings, and u8 sequences, but the non-human-readable branch still calls deserialize_byte_buf() only. In packages/rs-platform-value/src/value_serialization/de.rs:334-351, that path accepts Value::Bytes* variants and rejects Value::Array, so Serde never reaches visit_seq() for platform_value::Value::Array([Value::U8(..), ...]). This is still reachable from JS/WASM object conversion, where plain JS arrays deserialize to Value::Array, so the helper does not actually support every shape its contract claims to support.

suggestion: Variable-length byte deserialization has the same non-human-readable `Value::Array` gap

packages/rs-dpp/src/serialization/serde_bytes_var.rs (line 68)

AnyShapeVisitor implements visit_seq(), but the non-human-readable branch still forces deserialize_byte_buf(). On the platform_value deserializer, that rejects Value::Array before the visitor can consume a u8 sequence, so Vec<u8> fields still fail to round-trip through the canonical ValueConvertible::from_object path when bytes arrive as plain arrays instead of Value::Bytes. That leaves the implementation narrower than the stated accepted shapes.

suggestion: `PartialIdentity` key-ID parsing still truncates invalid JS numbers into different key IDs

packages/wasm-dpp2/src/identity/partial_identity.rs (line 320)

option_array_to_not_found() still accepts any JsValue with as_f64(), checks only the upper bound, and then casts with as KeyID. The object-key parsers in value_to_loaded_public_keys_from_object() and value_to_loaded_public_keys_from_json() do the same after parsing string keys as f64. As a result, NaN, negative numbers, and fractional values can be silently remapped to unrelated u32 key IDs instead of being rejected at the JS boundary, which makes malformed caller input target the wrong public key.

blocking: `ExtendedDocumentWasm` no longer exports `toObject()` or `toJSON()` even though current callers still use them

packages/wasm-dpp/src/document/extended_document.rs (line 258)

The current ExtendedDocumentWasm impl now jumps from setMetadata straight to toBuffer; the previous #[wasm_bindgen(js_name=toObject)] and toJSON exports were removed. This is an ABI regression, not dead-code cleanup: non-skipped tests in packages/wasm-dpp/test/unit/document/DocumentFactory.spec.js:44-66 still call document.toObject() in beforeEach, integration tests in packages/wasm-dpp/test/integration/document/DocumentFacade.spec.js:73-79 still compare result.toObject(), and packages/bench-suite/lib/benchmarks/DocumentsBenchmark.js:73 still calls document.toJSON(). On current head those paths become runtime TypeErrors for existing wasm-dpp consumers.

🤖 Prompt for all review comments with AI agents

These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.

- [BLOCKING] In `packages/rs-platform-value/src/converter/serde_json.rs`:44-44: Non-finite floats are still silently rewritten to JSON `0`
  `try_into_validating_json()` still converts `Value::Float` with `Number::from_f64(float).unwrap_or(0.into())`, and the same fallback is still present in `try_to_validating_json()` at line 140 and `TryInto<JsonValue>` at line 289. `serde_json::Number::from_f64()` returns `None` for `NaN` and infinities, so these helpers corrupt non-JSON numeric values into the valid JSON number `0` instead of rejecting them. That makes validation and serialization lossy and can accept an invalid payload as if the caller had supplied zero.
- [SUGGESTION] In `packages/rs-dpp/src/serialization/serde_bytes.rs`:90-101: Fixed-size byte deserialization still blocks `Value::Array([u8, ...])` on the non-human-readable path
  The visitor is written to accept byte buffers, byte slices, base64 strings, and `u8` sequences, but the non-human-readable branch still calls `deserialize_byte_buf()` only. In `packages/rs-platform-value/src/value_serialization/de.rs:334-351`, that path accepts `Value::Bytes*` variants and rejects `Value::Array`, so Serde never reaches `visit_seq()` for `platform_value::Value::Array([Value::U8(..), ...])`. This is still reachable from JS/WASM object conversion, where plain JS arrays deserialize to `Value::Array`, so the helper does not actually support every shape its contract claims to support.
- [SUGGESTION] In `packages/rs-dpp/src/serialization/serde_bytes_var.rs`:68-75: Variable-length byte deserialization has the same non-human-readable `Value::Array` gap
  `AnyShapeVisitor` implements `visit_seq()`, but the non-human-readable branch still forces `deserialize_byte_buf()`. On the `platform_value` deserializer, that rejects `Value::Array` before the visitor can consume a `u8` sequence, so `Vec<u8>` fields still fail to round-trip through the canonical `ValueConvertible::from_object` path when bytes arrive as plain arrays instead of `Value::Bytes`. That leaves the implementation narrower than the stated accepted shapes.
- [SUGGESTION] In `packages/wasm-dpp2/src/identity/partial_identity.rs`:320-424: `PartialIdentity` key-ID parsing still truncates invalid JS numbers into different key IDs
  `option_array_to_not_found()` still accepts any `JsValue` with `as_f64()`, checks only the upper bound, and then casts with `as KeyID`. The object-key parsers in `value_to_loaded_public_keys_from_object()` and `value_to_loaded_public_keys_from_json()` do the same after parsing string keys as `f64`. As a result, `NaN`, negative numbers, and fractional values can be silently remapped to unrelated `u32` key IDs instead of being rejected at the JS boundary, which makes malformed caller input target the wrong public key.
- [BLOCKING] In `packages/wasm-dpp/src/document/extended_document.rs`:258-271: `ExtendedDocumentWasm` no longer exports `toObject()` or `toJSON()` even though current callers still use them
  The current `ExtendedDocumentWasm` impl now jumps from `setMetadata` straight to `toBuffer`; the previous `#[wasm_bindgen(js_name=toObject)]` and `toJSON` exports were removed. This is an ABI regression, not dead-code cleanup: non-skipped tests in `packages/wasm-dpp/test/unit/document/DocumentFactory.spec.js:44-66` still call `document.toObject()` in `beforeEach`, integration tests in `packages/wasm-dpp/test/integration/document/DocumentFacade.spec.js:73-79` still compare `result.toObject()`, and `packages/bench-suite/lib/benchmarks/DocumentsBenchmark.js:73` still calls `document.toJSON()`. On current head those paths become runtime `TypeError`s for existing `wasm-dpp` consumers.

[thepastaclaw]

Code Review

The current head still carries all five prior findings: prior-1 through prior-5 are STILL VALID after rechecking the source, callers, and tests at 5fd3b25768b2a83f90ffc58798889785c5422330. The 123c00c3..5fd3b257 delta only removes a blank line in an unrelated file, so it does not introduce any new findings or resolve any existing ones. Two blocking regressions remain, so this head is not ready to merge.

_Note: Inline posting failed (command failed (1): gh pr diff 3573 --repo dashpay/platform
STDOUT:

STDERR:
could not find pull request diff: HTTP 406: Sorry, the diff exceeded the maximum number of files (300). Consider using 'List pull requests files' API or locally cloning the repository instead. (https://api.github.com/repos/), so I posted the same verified findings as a top-level review body._

🔴 2 blocking | 🟡 3 suggestion(s)

5 additional finding(s)

blocking: Validating JSON conversion still rewrites non-finite floats to zero instead of rejecting them

packages/rs-platform-value/src/converter/serde_json.rs (line 44)

try_into_validating_json() still turns Value::Float into Number::from_f64(float).unwrap_or(0.into()), and the same fallback remains in try_to_validating_json() at line 140 and TryInto<JsonValue> at line 289. serde_json::Number::from_f64() returns None for NaN and infinities, so invalid numeric input is silently changed to 0 rather than rejected. That is observable data corruption at a trust boundary, and the current test at lines 1136-1139 now locks the bad behavior in by asserting that NaN becomes 0.

blocking: `ExtendedDocumentWasm` no longer exposes the `toObject()` and `toJSON()` JS ABI

packages/wasm-dpp/src/document/extended_document.rs (line 31)

The exported ExtendedDocumentWasm class no longer defines #[wasm_bindgen(js_name = toObject)] or toJSON, so those methods are absent from the generated wasm surface. That is still an ABI regression on the current head: repository callers still expect ExtendedDocument instances to support toObject(), including packages/wasm-dpp/test/unit/document/DocumentFactory.spec.js:132, :142, :202 and packages/wasm-dpp/test/integration/document/DocumentFacade.spec.js:79, :93. Existing JS consumers that round-trip documents through those methods will fail at runtime even though the Rust type still exists.

suggestion: Fixed-size byte deserialization still rejects `Value::Array` on the non-human-readable path

packages/rs-dpp/src/serialization/serde_bytes.rs (line 97)

This helper claims to accept byte buffers, byte slices, base64 strings, and u8 sequences, and its visitor does implement visit_seq(). But the non-human-readable branch always calls deserialize_byte_buf(), and platform_value::Deserializer::deserialize_byte_buf() delegates to deserialize_bytes(), which only accepts Value::Bytes* and Value::Identifier and rejects Value::Array before visit_seq() can run. As a result, [u8; N] fields still do not deserialize from array-shaped platform_value::Value inputs on the from_object path, despite the helper advertising that shape.

suggestion: Variable-length byte deserialization has the same `Value::Array` gap

packages/rs-dpp/src/serialization/serde_bytes_var.rs (line 73)

serde_bytes_var::AnyShapeVisitor also implements visit_seq(), but the non-human-readable branch still forces deserialize_byte_buf(). On platform_value::Deserializer, that path rejects Value::Array up front and only accepts byte-oriented variants, so Vec<u8> fields still cannot be reconstructed from array-shaped platform_value::Value inputs on from_object. The implementation and its comments promise more accepted shapes than the actual non-human-readable path supports.

suggestion: `PartialIdentity` key-ID parsing still truncates invalid JS numbers into different `KeyID`s

packages/wasm-dpp2/src/identity/partial_identity.rs (line 320)

option_array_to_not_found() still accepts any JsValue with as_f64(), checks only the upper bound, and then casts with as KeyID. The same lossy pattern remains in value_to_loaded_public_keys_from_object() and value_to_loaded_public_keys_from_json(), where parsed f64 keys are cast directly to u32 after only a max check. Negative values, fractional values, and NaN can therefore be silently remapped to different key IDs instead of being rejected, which makes these JS/WASM parsing paths inconsistent with the declared KeyID type and can associate key material with the wrong ID.

🤖 Prompt for all review comments with AI agents

These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.

- [BLOCKING] In `packages/rs-platform-value/src/converter/serde_json.rs`:44-44: Validating JSON conversion still rewrites non-finite floats to zero instead of rejecting them
  `try_into_validating_json()` still turns `Value::Float` into `Number::from_f64(float).unwrap_or(0.into())`, and the same fallback remains in `try_to_validating_json()` at line 140 and `TryInto<JsonValue>` at line 289. `serde_json::Number::from_f64()` returns `None` for `NaN` and infinities, so invalid numeric input is silently changed to `0` rather than rejected. That is observable data corruption at a trust boundary, and the current test at lines 1136-1139 now locks the bad behavior in by asserting that `NaN` becomes `0`.
- [BLOCKING] In `packages/wasm-dpp/src/document/extended_document.rs`:31-303: `ExtendedDocumentWasm` no longer exposes the `toObject()` and `toJSON()` JS ABI
  The exported `ExtendedDocumentWasm` class no longer defines `#[wasm_bindgen(js_name = toObject)]` or `toJSON`, so those methods are absent from the generated wasm surface. That is still an ABI regression on the current head: repository callers still expect `ExtendedDocument` instances to support `toObject()`, including `packages/wasm-dpp/test/unit/document/DocumentFactory.spec.js:132`, `:142`, `:202` and `packages/wasm-dpp/test/integration/document/DocumentFacade.spec.js:79`, `:93`. Existing JS consumers that round-trip documents through those methods will fail at runtime even though the Rust type still exists.
- [SUGGESTION] In `packages/rs-dpp/src/serialization/serde_bytes.rs`:97-101: Fixed-size byte deserialization still rejects `Value::Array` on the non-human-readable path
  This helper claims to accept byte buffers, byte slices, base64 strings, and `u8` sequences, and its visitor does implement `visit_seq()`. But the non-human-readable branch always calls `deserialize_byte_buf()`, and `platform_value::Deserializer::deserialize_byte_buf()` delegates to `deserialize_bytes()`, which only accepts `Value::Bytes*` and `Value::Identifier` and rejects `Value::Array` before `visit_seq()` can run. As a result, `[u8; N]` fields still do not deserialize from array-shaped `platform_value::Value` inputs on the `from_object` path, despite the helper advertising that shape.
- [SUGGESTION] In `packages/rs-dpp/src/serialization/serde_bytes_var.rs`:73-75: Variable-length byte deserialization has the same `Value::Array` gap
  `serde_bytes_var::AnyShapeVisitor` also implements `visit_seq()`, but the non-human-readable branch still forces `deserialize_byte_buf()`. On `platform_value::Deserializer`, that path rejects `Value::Array` up front and only accepts byte-oriented variants, so `Vec<u8>` fields still cannot be reconstructed from array-shaped `platform_value::Value` inputs on `from_object`. The implementation and its comments promise more accepted shapes than the actual non-human-readable path supports.
- [SUGGESTION] In `packages/wasm-dpp2/src/identity/partial_identity.rs`:320-424: `PartialIdentity` key-ID parsing still truncates invalid JS numbers into different `KeyID`s
  `option_array_to_not_found()` still accepts any `JsValue` with `as_f64()`, checks only the upper bound, and then casts with `as KeyID`. The same lossy pattern remains in `value_to_loaded_public_keys_from_object()` and `value_to_loaded_public_keys_from_json()`, where parsed `f64` keys are cast directly to `u32` after only a max check. Negative values, fractional values, and `NaN` can therefore be silently remapped to different key IDs instead of being rejected, which makes these JS/WASM parsing paths inconsistent with the declared `KeyID` type and can associate key material with the wrong ID.

[thepastaclaw]

Code Review

I revalidated all five carried-forward findings against head a77e40725cdd978c706585a024243dec66b03e1b and confirmed that each one still applies on the current source. The incremental delta from 5fd3b25768b2a83f90ffc58798889785c5422330 to head only removes an unused import in packages/rs-sdk-ffi/src/document/queries/search.rs; it does not introduce any additional review findings beyond the carried-forward set.

_Note: Inline posting failed (command failed (1): gh pr diff 3573 --repo dashpay/platform
STDOUT:

STDERR:
could not find pull request diff: HTTP 406: Sorry, the diff exceeded the maximum number of files (300). Consider using 'List pull requests files' API or locally cloning the repository instead. (https://api.github.com/repos/), so I posted the same verified findings as a top-level review body._

🔴 2 blocking | 🟡 3 suggestion(s)

5 additional finding(s)

blocking: Validating JSON conversion still rewrites non-finite floats to zero

packages/rs-platform-value/src/converter/serde_json.rs (line 44)

try_into_validating_json(), try_to_validating_json(), and TryInto<JsonValue> for Value still map Value::Float through Number::from_f64(...).unwrap_or(0.into()) at lines 44, 140, and 289. serde_json::Number::from_f64() returns None for NaN and infinities, so these validating/canonical JSON helpers silently rewrite invalid numeric payloads into JSON 0 instead of rejecting them. That changes untrusted data before validation, and this path is still used by document and contract validation code such as packages/rs-dpp/src/data_contract/methods/validate_document/v0/mod.rs:69. The current tree even contains a test at packages/rs-platform-value/src/converter/serde_json.rs:1136 asserting that NaN becomes 0, which confirms the behavior is live rather than hypothetical.

blocking: ExtendedDocumentWasm still omits the public toObject()/toJSON() JS ABI

packages/wasm-dpp/src/document/extended_document.rs (line 31)

The exported ExtendedDocumentWasm class still exposes constructor/getters/setters/toBuffer/hash/validate, but there are still no #[wasm_bindgen(js_name = toObject)] or #[wasm_bindgen(js_name = toJSON)] methods anywhere in the js_class=ExtendedDocument impl. Other wasm wrapper types in this repository now get those methods from impl_wasm_conversions_inner!, but ExtendedDocumentWasm does not use that path. This leaves ExtendedDocument as an inconsistent public wasm type: callers can create and receive it from the SDK, but they cannot round-trip it through the same object/JSON ABI exposed by the rest of the wasm surface. Repository tests and fixtures still treat ExtendedDocument as a serializable JS-facing type, so the missing methods remain a real ABI regression on the current head.

suggestion: Fixed-size byte deserialization still rejects Value::Array on non-human-readable deserializers

packages/rs-dpp/src/serialization/serde_bytes.rs (line 97)

This helper still advertises support for byte buffers, byte slices, base64 strings, and sequences of u8, and its visitor does implement visit_seq(). But the non-human-readable branch still unconditionally calls deserialize_byte_buf(). On platform_value::Deserializer, deserialize_byte_buf() delegates to deserialize_bytes(), which only accepts byte-oriented variants and rejects Value::Array; only deserialize_seq() accepts arrays. As a result, array-shaped dynamic values still cannot deserialize into [u8; N] through this helper even though the documented contract and visitor implementation both claim sequence support.

suggestion: Variable-length byte deserialization has the same Value::Array gap

packages/rs-dpp/src/serialization/serde_bytes_var.rs (line 73)

serde_bytes_var::deserialize() still has the same mismatch as the fixed-size helper. Its visitor implements visit_seq(), but the non-human-readable branch always calls deserialize_byte_buf(). On platform_value::Deserializer, that path rejects Value::Array before the visitor can consume the sequence, so Vec<u8> fields still cannot accept array-shaped platform_value::Value input on the non-human-readable path despite the helper advertising that shape.

suggestion: PartialIdentity key-ID parsing still lossy-casts invalid JS numbers into different KeyIDs

packages/wasm-dpp2/src/identity/partial_identity.rs (line 320)

option_array_to_not_found() still accepts any JsValue with as_f64(), checks only the upper bound, and then casts with as KeyID. The same lossy pattern remains in value_to_loaded_public_keys_from_object() and value_to_loaded_public_keys_from_json(), which parse keys as f64, reject only values above u32::MAX, and then cast with as u32. Negative, fractional, and non-finite inputs can therefore be truncated or saturated into different key IDs instead of being rejected. That is especially hard to justify here because this crate already has a strict try_to_u32() helper in packages/wasm-dpp2/src/utils.rs that validates finiteness, integrality, and non-negativity.

🤖 Prompt for all review comments with AI agents

These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.

- [BLOCKING] In `packages/rs-platform-value/src/converter/serde_json.rs`:44-44: Validating JSON conversion still rewrites non-finite floats to zero
  `try_into_validating_json()`, `try_to_validating_json()`, and `TryInto<JsonValue> for Value` still map `Value::Float` through `Number::from_f64(...).unwrap_or(0.into())` at lines 44, 140, and 289. `serde_json::Number::from_f64()` returns `None` for `NaN` and infinities, so these validating/canonical JSON helpers silently rewrite invalid numeric payloads into JSON `0` instead of rejecting them. That changes untrusted data before validation, and this path is still used by document and contract validation code such as `packages/rs-dpp/src/data_contract/methods/validate_document/v0/mod.rs:69`. The current tree even contains a test at `packages/rs-platform-value/src/converter/serde_json.rs:1136` asserting that `NaN` becomes `0`, which confirms the behavior is live rather than hypothetical.
- [BLOCKING] In `packages/wasm-dpp/src/document/extended_document.rs`:31-303: ExtendedDocumentWasm still omits the public toObject()/toJSON() JS ABI
  The exported `ExtendedDocumentWasm` class still exposes constructor/getters/setters/`toBuffer`/`hash`/`validate`, but there are still no `#[wasm_bindgen(js_name = toObject)]` or `#[wasm_bindgen(js_name = toJSON)]` methods anywhere in the `js_class=ExtendedDocument` impl. Other wasm wrapper types in this repository now get those methods from `impl_wasm_conversions_inner!`, but `ExtendedDocumentWasm` does not use that path. This leaves `ExtendedDocument` as an inconsistent public wasm type: callers can create and receive it from the SDK, but they cannot round-trip it through the same object/JSON ABI exposed by the rest of the wasm surface. Repository tests and fixtures still treat `ExtendedDocument` as a serializable JS-facing type, so the missing methods remain a real ABI regression on the current head.
- [SUGGESTION] In `packages/rs-dpp/src/serialization/serde_bytes.rs`:97-101: Fixed-size byte deserialization still rejects Value::Array on non-human-readable deserializers
  This helper still advertises support for byte buffers, byte slices, base64 strings, and sequences of `u8`, and its visitor does implement `visit_seq()`. But the non-human-readable branch still unconditionally calls `deserialize_byte_buf()`. On `platform_value::Deserializer`, `deserialize_byte_buf()` delegates to `deserialize_bytes()`, which only accepts byte-oriented variants and rejects `Value::Array`; only `deserialize_seq()` accepts arrays. As a result, array-shaped dynamic values still cannot deserialize into `[u8; N]` through this helper even though the documented contract and visitor implementation both claim sequence support.
- [SUGGESTION] In `packages/rs-dpp/src/serialization/serde_bytes_var.rs`:73-75: Variable-length byte deserialization has the same Value::Array gap
  `serde_bytes_var::deserialize()` still has the same mismatch as the fixed-size helper. Its visitor implements `visit_seq()`, but the non-human-readable branch always calls `deserialize_byte_buf()`. On `platform_value::Deserializer`, that path rejects `Value::Array` before the visitor can consume the sequence, so `Vec<u8>` fields still cannot accept array-shaped `platform_value::Value` input on the non-human-readable path despite the helper advertising that shape.
- [SUGGESTION] In `packages/wasm-dpp2/src/identity/partial_identity.rs`:320-424: PartialIdentity key-ID parsing still lossy-casts invalid JS numbers into different KeyIDs
  `option_array_to_not_found()` still accepts any `JsValue` with `as_f64()`, checks only the upper bound, and then casts with `as KeyID`. The same lossy pattern remains in `value_to_loaded_public_keys_from_object()` and `value_to_loaded_public_keys_from_json()`, which parse keys as `f64`, reject only values above `u32::MAX`, and then cast with `as u32`. Negative, fractional, and non-finite inputs can therefore be truncated or saturated into different key IDs instead of being rejected. That is especially hard to justify here because this crate already has a strict `try_to_u32()` helper in `packages/wasm-dpp2/src/utils.rs` that validates finiteness, integrality, and non-negativity.

[thepastaclaw]

Code Review

All five prior findings are STILL VALID at 7672522: the validating Value→JSON path still silently rewrites NaN/±∞ floats to 0 (lines 44, 140, 289 of serde_json.rs), ExtendedDocumentWasm still lacks the public toObject()/toJSON() WASM ABI, the new serde_bytes / serde_bytes_var helpers still reject Value::Array on non-human-readable platform_value deserializers, and wasm-dpp2 PartialIdentity key-ID parsing still lossy-casts JS numbers into different KeyIDs. The latest delta (the v3.1-dev merge and the keywords-meta-schema test migration) pulled in two further pre-existing correctness issues now sitting on this branch's head: an unreachable timeout counter in the query service retry loop and unchecked u32→u16 truncation in vote-query proof reconstruction. No new defects were introduced by the PR's own code in the a77e407..7672522 delta.

_Note: Inline posting failed (command failed (1): gh pr diff 3573 --repo dashpay/platform
STDOUT:

STDERR:
could not find pull request diff: HTTP 406: Sorry, the diff exceeded the maximum number of files (300). Consider using 'List pull requests files' API or locally cloning the repository instead. (https://api.github.com/repos/), so I posted the same verified findings as a top-level review body._

🔴 4 blocking | 🟡 3 suggestion(s)

7 additional finding(s)

blocking: Validating Value→JSON conversion silently rewrites NaN/±∞ floats to JSON 0

packages/rs-platform-value/src/converter/serde_json.rs (line 44)

Value::Float is converted via Number::from_f64(float).unwrap_or(0.into()) at line 44, and the same fallback is repeated at line 140 (try_to_validating_json) and line 289 (TryInto<JsonValue> for Value). serde_json::Number::from_f64() returns None for NaN, +∞, and -∞, so any non-finite float silently becomes the JSON integer 0 instead of producing an error. These helpers are labelled validating and are used at trust boundaries (e.g. rs-dpp/src/data_contract/methods/validate_document/v0/mod.rs:69, rs-dpp/src/document/v0/json_conversion.rs:88, rs-dpp/src/data_contract/v0/conversion/json.rs:30). The current behaviour turns an invalid float payload into a structurally-valid 0, defeating any schema/range check that should have rejected it and making two divergent upstream values (e.g. NaN vs 0) converge to the same JSON — exactly the kind of asymmetry that masks consensus/serialization bugs. All three sites should reject non-finite floats with a typed error (e.g. Error::Unsupported) consistent with the surrounding EnumU8/IntegerSizeError arms.

blocking: ExtendedDocumentWasm still omits the public toObject()/toJSON() JS ABI

packages/wasm-dpp/src/document/extended_document.rs (line 31)

The exported #[wasm_bindgen(js_class=ExtendedDocument)] impl exposes constructor / getters / setters / toBuffer / hash / validate, but contains no #[wasm_bindgen(js_name = toObject)] or #[wasm_bindgen(js_name = toJSON)] methods. Other wasm wrapper types in this crate and in wasm-dpp2 consistently expose this conversion ABI, and the PR's Critical-3 work on the rs-dpp serde layer is specifically intended to make ExtendedDocument round-trippable through these methods. As shipped, JS callers cannot exercise that fix without going through toBuffer + binary decode round trips, which is a public ABI regression for any consumer that was previously calling ExtendedDocument.toObject() / .toJSON(). Either expose the two methods that delegate to the canonical ValueConvertible / JsonConvertible impls, or document explicitly in the CHANGELOG that this wrapper does not participate in the JS object-conversion ABI.

blocking: Query retry loop never reaches its advertised 1-second timeout (counter resets each iteration)

packages/rs-drive-abci/src/query/service.rs (line 129)

The inner wait loop at line 129 is supposed to poll committed_block_height_guard for up to 1 second before setting needs_restart, but let mut counter = 0; is declared inside the loop body at line 133. That resets the counter to 0 on every iteration, so the if counter >= 100 branch at line 143 is unreachable: counter is always 1 when it is checked. If the platform state and guard stay out of sync, the request thread sleeps and loops forever instead of retrying or failing with NotServiceable. This turns a transient finalize-block race into an unbounded hang on the query path. Move let mut counter = 0; outside the loop (just before line 129) so it persists across iterations. Note: this code originates from the upstream refactor(rs-sdk): async AddressProvider callbacks change and entered this branch via the merge from origin/v3.1-dev — it is now sitting on the PR's head and should be fixed (or escalated upstream) before this branch lands.

blocking: Vote-query proof verifier truncates u32 limits/offsets to u16, breaking proof soundness

packages/rs-drive-proof-verifier/src/from_request.rs (line 89)

Several vote-related TryFromRequest impls reconstruct the query bound against the proof using unchecked casts: v.count.map(|v| v as u16) and v.offset.map(|v| v as u16) at lines 89, 199, 254, 324, 370, and 371. The server-side handlers reject overflowed values with u16::try_from(...) (see rs-drive-abci/src/query/voting/.../v0/mod.rs for contested_resources, contested_resource_vote_state, contested_resource_voters_for_identity, contested_resource_identity_votes, and vote_polls_by_end_date_query), but the client-side FromProof paths use these same try_from_request impls in rs-drive-proof-verifier/src/proof.rs:1944-2140 to rebuild the query that the GroveDB proof is verified against. A malicious node can therefore answer an out-of-range request (e.g. limit = 65536) with a proof for the wrapped/truncated query (Some(0)), and the client will accept it as a valid proof for the original request. That breaks proof soundness on these endpoints because the client validates a proof for a materially different query than the one it sent. Use u16::try_from(...) with a RequestError on overflow, mirroring the server side. (Pre-existing from 85a7117241, brought in by the v3.1-dev merge.)

suggestion: PartialIdentity key-ID parsing lossy-casts invalid JS numbers into different KeyIDs

packages/wasm-dpp2/src/identity/partial_identity.rs (line 320)

option_array_to_not_found() (lines 320-348) accepts any JsValue with as_f64(), validates only the upper bound (> u32::MAX as f64), and then casts with as KeyID at line 341. The same pattern is reused in value_to_loaded_public_keys_from_object() (lines 351-398) and value_to_loaded_public_keys_from_json() (lines 401-end), which parse object keys as f64 and cast with as u32 at lines 374 and 424. In Rust, f64 as u32 saturates and truncates: -1.0 as u32 == 0, 0.5 as u32 == 0, 1.7 as u32 == 1, f64::NAN as u32 == 0, and values just above u32::MAX are not even caught by the > u32::MAX as f64 comparison because of f64 rounding. Net effect at this WASM trust boundary: a malicious or buggy JS caller can collapse multiple distinct logical key IDs onto the same Rust KeyID (typically 0, often the master key slot) or have fractional/non-finite inputs accepted as valid keys. Anywhere the resulting PartialIdentity feeds authorization, key selection, or wallet/UI security decisions, this silently addresses the wrong key. Validate key_val.is_finite() && key_val >= 0.0 && key_val.fract() == 0.0 && key_val <= u32::MAX as f64 before casting at all three call sites.

suggestion: Fixed-size byte deserializer rejects Value::Array on non-human-readable platform_value

packages/rs-dpp/src/serialization/serde_bytes.rs (line 90)

AnyShapeVisitor advertises support for byte buffers, byte slices, base64 strings, and sequences of u8 (via visit_seq()). The human-readable branch correctly uses deserialize_any, but the non-HR branch hard-wires deserialize_byte_buf(AnyShapeVisitor::<N>). On platform_value::Deserializer, deserialize_byte_buf delegates to deserialize_bytes, which only accepts Value::Bytes/Bytes20/Bytes32/Bytes36/Identifier (see rs-platform-value/src/value_serialization/de.rs:334-352) — a Value::Array of u8 is rejected before the visitor's visit_seq() ever runs. This creates an asymmetric shape contract: HR can accept arrays, non-HR cannot, even though the visitor claims to handle both. Switch the non-HR branch to deserialize_any so the visitor's advertised shapes are actually reachable through platform_value, while bincode (which can tolerate deserialize_any for length-prefixed byte buffers) continues to work.

suggestion: Variable-length byte deserializer has the same Value::Array gap on non-human-readable input

packages/rs-dpp/src/serialization/serde_bytes_var.rs (line 68)

Same defect as serde_bytes.rs. The visitor implements visit_seq() and the doc-comment advertises 'sequence of u8' as an accepted shape, but the non-HR branch at line 75 always calls deserialize_byte_buf(AnyShapeVisitor). On platform_value::Deserializer, that path again routes through deserialize_bytes() and rejects Value::Array before the visitor can consume the sequence. As a result, Vec<u8> fields annotated with #[serde(with = "serde_bytes_var")] cannot round-trip a payload that has gone through the canonical Value::Array form via non-HR transport, even though the helper's contract claims they can. Use deserialize_any here for symmetry with the visitor's visit_seq arm.

🤖 Prompt for all review comments with AI agents

These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.

- [BLOCKING] In `packages/rs-platform-value/src/converter/serde_json.rs`:44-44: Validating Value→JSON conversion silently rewrites NaN/±∞ floats to JSON 0
  `Value::Float` is converted via `Number::from_f64(float).unwrap_or(0.into())` at line 44, and the same fallback is repeated at line 140 (`try_to_validating_json`) and line 289 (`TryInto<JsonValue> for Value`). `serde_json::Number::from_f64()` returns `None` for `NaN`, `+∞`, and `-∞`, so any non-finite float silently becomes the JSON integer `0` instead of producing an error. These helpers are labelled *validating* and are used at trust boundaries (e.g. `rs-dpp/src/data_contract/methods/validate_document/v0/mod.rs:69`, `rs-dpp/src/document/v0/json_conversion.rs:88`, `rs-dpp/src/data_contract/v0/conversion/json.rs:30`). The current behaviour turns an invalid float payload into a structurally-valid `0`, defeating any schema/range check that should have rejected it and making two divergent upstream values (e.g. NaN vs 0) converge to the same JSON — exactly the kind of asymmetry that masks consensus/serialization bugs. All three sites should reject non-finite floats with a typed error (e.g. `Error::Unsupported`) consistent with the surrounding `EnumU8`/`IntegerSizeError` arms.
- [BLOCKING] In `packages/wasm-dpp/src/document/extended_document.rs`:31-303: ExtendedDocumentWasm still omits the public toObject()/toJSON() JS ABI
  The exported `#[wasm_bindgen(js_class=ExtendedDocument)]` impl exposes constructor / getters / setters / `toBuffer` / `hash` / `validate`, but contains no `#[wasm_bindgen(js_name = toObject)]` or `#[wasm_bindgen(js_name = toJSON)]` methods. Other wasm wrapper types in this crate and in wasm-dpp2 consistently expose this conversion ABI, and the PR's Critical-3 work on the rs-dpp serde layer is specifically intended to make ExtendedDocument round-trippable through these methods. As shipped, JS callers cannot exercise that fix without going through `toBuffer` + binary decode round trips, which is a public ABI regression for any consumer that was previously calling `ExtendedDocument.toObject()` / `.toJSON()`. Either expose the two methods that delegate to the canonical `ValueConvertible` / `JsonConvertible` impls, or document explicitly in the CHANGELOG that this wrapper does not participate in the JS object-conversion ABI.
- [BLOCKING] In `packages/rs-drive-abci/src/query/service.rs`:129-148: Query retry loop never reaches its advertised 1-second timeout (counter resets each iteration)
  The inner wait loop at line 129 is supposed to poll `committed_block_height_guard` for up to 1 second before setting `needs_restart`, but `let mut counter = 0;` is declared inside the loop body at line 133. That resets the counter to `0` on every iteration, so the `if counter >= 100` branch at line 143 is unreachable: `counter` is always `1` when it is checked. If the platform state and guard stay out of sync, the request thread sleeps and loops forever instead of retrying or failing with `NotServiceable`. This turns a transient finalize-block race into an unbounded hang on the query path. Move `let mut counter = 0;` outside the loop (just before line 129) so it persists across iterations. Note: this code originates from the upstream `refactor(rs-sdk): async AddressProvider callbacks` change and entered this branch via the merge from `origin/v3.1-dev` — it is now sitting on the PR's head and should be fixed (or escalated upstream) before this branch lands.
- [BLOCKING] In `packages/rs-drive-proof-verifier/src/from_request.rs`:89-371: Vote-query proof verifier truncates u32 limits/offsets to u16, breaking proof soundness
  Several vote-related `TryFromRequest` impls reconstruct the query bound against the proof using unchecked casts: `v.count.map(|v| v as u16)` and `v.offset.map(|v| v as u16)` at lines 89, 199, 254, 324, 370, and 371. The server-side handlers reject overflowed values with `u16::try_from(...)` (see `rs-drive-abci/src/query/voting/.../v0/mod.rs` for contested_resources, contested_resource_vote_state, contested_resource_voters_for_identity, contested_resource_identity_votes, and vote_polls_by_end_date_query), but the client-side `FromProof` paths use these same `try_from_request` impls in `rs-drive-proof-verifier/src/proof.rs:1944-2140` to rebuild the query that the GroveDB proof is verified against. A malicious node can therefore answer an out-of-range request (e.g. `limit = 65536`) with a proof for the wrapped/truncated query (`Some(0)`), and the client will accept it as a valid proof for the original request. That breaks proof soundness on these endpoints because the client validates a proof for a materially different query than the one it sent. Use `u16::try_from(...)` with a `RequestError` on overflow, mirroring the server side. (Pre-existing from `85a7117241`, brought in by the v3.1-dev merge.)
- [SUGGESTION] In `packages/wasm-dpp2/src/identity/partial_identity.rs`:320-425: PartialIdentity key-ID parsing lossy-casts invalid JS numbers into different KeyIDs
  `option_array_to_not_found()` (lines 320-348) accepts any `JsValue` with `as_f64()`, validates only the upper bound (`> u32::MAX as f64`), and then casts with `as KeyID` at line 341. The same pattern is reused in `value_to_loaded_public_keys_from_object()` (lines 351-398) and `value_to_loaded_public_keys_from_json()` (lines 401-end), which parse object keys as `f64` and cast with `as u32` at lines 374 and 424. In Rust, `f64 as u32` saturates and truncates: `-1.0 as u32 == 0`, `0.5 as u32 == 0`, `1.7 as u32 == 1`, `f64::NAN as u32 == 0`, and values just above `u32::MAX` are not even caught by the `> u32::MAX as f64` comparison because of f64 rounding. Net effect at this WASM trust boundary: a malicious or buggy JS caller can collapse multiple distinct logical key IDs onto the same Rust `KeyID` (typically `0`, often the master key slot) or have fractional/non-finite inputs accepted as valid keys. Anywhere the resulting `PartialIdentity` feeds authorization, key selection, or wallet/UI security decisions, this silently addresses the wrong key. Validate `key_val.is_finite() && key_val >= 0.0 && key_val.fract() == 0.0 && key_val <= u32::MAX as f64` before casting at all three call sites.
- [SUGGESTION] In `packages/rs-dpp/src/serialization/serde_bytes.rs`:90-102: Fixed-size byte deserializer rejects Value::Array on non-human-readable platform_value
  `AnyShapeVisitor` advertises support for byte buffers, byte slices, base64 strings, and sequences of `u8` (via `visit_seq()`). The human-readable branch correctly uses `deserialize_any`, but the non-HR branch hard-wires `deserialize_byte_buf(AnyShapeVisitor::<N>)`. On `platform_value::Deserializer`, `deserialize_byte_buf` delegates to `deserialize_bytes`, which only accepts `Value::Bytes`/`Bytes20`/`Bytes32`/`Bytes36`/`Identifier` (see `rs-platform-value/src/value_serialization/de.rs:334-352`) — a `Value::Array` of `u8` is rejected before the visitor's `visit_seq()` ever runs. This creates an asymmetric shape contract: HR can accept arrays, non-HR cannot, even though the visitor claims to handle both. Switch the non-HR branch to `deserialize_any` so the visitor's advertised shapes are actually reachable through platform_value, while bincode (which can tolerate `deserialize_any` for length-prefixed byte buffers) continues to work.
- [SUGGESTION] In `packages/rs-dpp/src/serialization/serde_bytes_var.rs`:68-76: Variable-length byte deserializer has the same Value::Array gap on non-human-readable input
  Same defect as `serde_bytes.rs`. The visitor implements `visit_seq()` and the doc-comment advertises 'sequence of u8' as an accepted shape, but the non-HR branch at line 75 always calls `deserialize_byte_buf(AnyShapeVisitor)`. On `platform_value::Deserializer`, that path again routes through `deserialize_bytes()` and rejects `Value::Array` before the visitor can consume the sequence. As a result, `Vec<u8>` fields annotated with `#[serde(with = "serde_bytes_var")]` cannot round-trip a payload that has gone through the canonical `Value::Array` form via non-HR transport, even though the helper's contract claims they can. Use `deserialize_any` here for symmetry with the visitor's `visit_seq` arm.

[thepastaclaw]

Code Review

Cumulative review at cf7affb. The only delta since 7672522 is a one-line test alignment in packages/rs-platform-value/tests/coverage_tests.rs (Critical-2 array→bytes heuristic removal); it introduces no new findings. All 7 prior findings remain present at the current head and are carried forward. Codex's claim that prior-6/prior-7 are now fixed is incorrect: platform_value's deserialize_byte_buf delegates to deserialize_bytes, which still rejects Value::Array. Two NaN/Inf coercion sites and three u16-truncation sites are consolidated where they share a root cause.

_Note: Inline posting failed (command failed (1): gh pr diff 3573 --repo dashpay/platform
STDOUT:

STDERR:
could not find pull request diff: HTTP 406: Sorry, the diff exceeded the maximum number of files (300). Consider using 'List pull requests files' API or locally cloning the repository instead. (https://api.github.com/repos/), so I posted the same verified findings as a top-level review body._

🔴 4 blocking | 🟡 3 suggestion(s)

7 additional finding(s)

blocking: Value→JSON conversion silently rewrites NaN/±∞ floats to JSON 0 on all three paths

packages/rs-platform-value/src/converter/serde_json.rs (line 44)

All three Value→JSON converters use Number::from_f64(...).unwrap_or(0.into()) for Value::Float: line 44 (try_into_validating_json), line 140 (try_to_validating_json by-reference), and line 289 (TryInto<JsonValue>). serde_json::Number::from_f64() returns None for NaN, +∞, and -∞, so these helpers silently mutate non-finite floats into the valid JSON number 0 instead of rejecting them. This is a trust-boundary defect because the validating-JSON helpers are explicitly used as a sanitization step before schema validation, signing, and WASM-bound JSON emission. Any caller that places a NaN/±∞ float into a Value (including via a permissive JS or f64 deserialization path) will see the forged value 0 downstream, defeating any range/finite check that would otherwise have rejected the input. The Critical-2 coverage test added at this commit additionally locks in the silent-coercion behavior. Return Error::Unsupported (or a dedicated non-finite variant) on None instead of unwrap_or(0.into()).

blocking: ExtendedDocumentWasm omits the public toObject()/toJSON() JS ABI

packages/wasm-dpp/src/document/extended_document.rs (line 27)

The exported #[wasm_bindgen(js_class=ExtendedDocument)] impl exposes constructor, getters/setters, toBuffer, hash, clone, validate, get, and set, but no #[wasm_bindgen(js_name=toObject)] or #[wasm_bindgen(js_name=toJSON)] (verified via grep — only getData returns the inner properties map, not the full extended-document shape). Peer wrappers in this crate (e.g. IdentityWasm in packages/wasm-dpp/src/identity/identity.rs:166-220) and most wasm-dpp2 wrappers expose this conversion ABI, and the entire PR centers on standardizing canonical object/JSON conversion. JS consumers that previously relied on extendedDocument.toObject()/toJSON() — including JSON.stringify(extendedDocument), which transparently invokes toJSON — get undefined from the wasm boundary. This is a public ABI regression for JS callers. Add the canonical toObject/toJSON methods on the exported class.

blocking: Query retry loop never reaches its advertised 1-second timeout (counter reset each iteration)

packages/rs-drive-abci/src/query/service.rs (line 129)

let mut counter = 0; is declared on line 133, inside the loop { ... } body started on line 129. Every iteration zeroes the counter, runs counter += 1 once, then evaluates if counter >= 100 — which is never true (counter is always 1 at that point). The advertised 1-second timeout / query_counter increment / needs_restart = true branch is unreachable, so a query whose required height never lands keeps sleeping in 10ms increments indefinitely instead of giving up and (eventually) returning NotServiceable. Compare with query_counter, which is correctly declared outside the loop. Beyond the latency bug, this is a denial-of-service primitive: untrusted gRPC callers can pin spawn_blocking worker threads during any state/drive height inconsistency window. Declare let mut counter = 0; immediately before the loop { on line 129.

blocking: Vote-query proof verifiers truncate u32 limits/offsets to u16, breaking proof binding

packages/rs-drive-proof-verifier/src/from_request.rs (line 89)

Six unchecked as u16 casts on wire-u32 pagination fields remain at lines 89, 199, 254, 324, 370, and 371 (verified via grep). The server-side drive-abci handlers reject overflow with u16::try_from(...), but the verifier wraps silently — so a malicious or buggy server can answer a client request for count = 70000 with a proof for count = 4464 (or any count & 0xFFFF) and the verifier will happily verify it against the wrapped query. The offset case at line 371 is the strongest: an attacker can shift the entire returned window without verification failing. This is a proof-soundness break at the network trust boundary for ContestedDocumentVotePollDriveQuery, ContestedResourceVotesGivenByIdentityQuery, ContestedDocumentVotePollVotesDriveQuery, VotePollsByDocumentTypeQuery, and VotePollsByEndDateDriveQuery. Replace each v as u16 with a checked u16::try_from(v).map_err(...)? so the verifier matches the server-side validation semantics.

suggestion: PartialIdentity key-ID parsing lossy-casts invalid JS numbers into different KeyIDs

packages/wasm-dpp2/src/identity/partial_identity.rs (line 320)

option_array_to_not_found() (lines 320-348) accepts any JsValue with as_f64(), validates only the upper bound (key_val > u32::MAX as f64), then casts key_val as KeyID. Rust's f64 as u32 is saturating/truncating: -1.0 → 0, NaN → 0, -Infinity → 0, 1.9 → 1. A JS caller passing [-1, 1.5, NaN, -Infinity] silently produces {KeyID(0)} instead of being rejected — conflating invalid inputs with the legitimate key id 0. The same lossy pattern is repeated in value_to_loaded_public_keys_from_object (line 363/374, key_str.parse::<f64>() → key_val as u32) and value_to_loaded_public_keys_from_json (lines 413/424), where it additionally accepts strings like "NaN", "Infinity", "-1", "1.5" as object keys. The crate already has a stricter try_to_u32() helper in packages/wasm-dpp2/src/utils.rs that rejects non-integer/negative/non-finite inputs; the partial-identity parsers should use it (or equivalent finite + non-negative + integral checks) before casting.

suggestion: Fixed-size byte deserializer rejects Value::Array of u8 on non-human-readable platform_value

packages/rs-dpp/src/serialization/serde_bytes.rs (line 90)

AnyShapeVisitor::<N> advertises (and implements) visit_byte_buf, visit_bytes, visit_str (base64), and visit_seq (sequence of u8). The HR branch (line 96) uses deserialize_any, but the non-HR branch (line 101) hard-wires deserialize_byte_buf(AnyShapeVisitor::<N>). On platform_value::Deserializer, deserialize_byte_buf delegates to deserialize_bytes (packages/rs-platform-value/src/value_serialization/de.rs:347-352), which only accepts Value::Bytes*/Value::Identifier and rejects Value::Array with invalid_type before visit_seq can run. After the Critical-2 fix in this PR (JSON integer arrays no longer coerce to Value::Bytes), JSON→platform_value→struct paths for fixed-size byte fields now fail on the array shape — even though the visitor was written to support it. Either implement an Array<u8> fallback in platform_value::Deserializer::deserialize_byte_buf, or call deserialize_any on the non-HR branch for sources that support it (and keep deserialize_byte_buf only for bincode). Note: codex flagged this as fixed; verification shows the platform_value path still rejects arrays here.

suggestion: Variable-length byte deserializer has the same Value::Array gap on non-human-readable input

packages/rs-dpp/src/serialization/serde_bytes_var.rs (line 68)

Same defect as serde_bytes.rs: AnyShapeVisitor implements visit_seq (collecting u8 elements into Vec<u8>), but the non-HR branch unconditionally calls deserialize_byte_buf(AnyShapeVisitor), which on platform_value::Deserializer delegates to deserialize_bytes and rejects Value::Array before the seq visitor runs. Variable-length byte fields read from JSON-derived Value objects fail this path despite the visitor being designed to accept them. Same fix shape as serde_bytes.rs.

🤖 Prompt for all review comments with AI agents

These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.

- [BLOCKING] In `packages/rs-platform-value/src/converter/serde_json.rs`:44-289: Value→JSON conversion silently rewrites NaN/±∞ floats to JSON 0 on all three paths
  All three Value→JSON converters use `Number::from_f64(...).unwrap_or(0.into())` for `Value::Float`: line 44 (`try_into_validating_json`), line 140 (`try_to_validating_json` by-reference), and line 289 (`TryInto<JsonValue>`). `serde_json::Number::from_f64()` returns `None` for `NaN`, `+∞`, and `-∞`, so these helpers silently mutate non-finite floats into the valid JSON number `0` instead of rejecting them. This is a trust-boundary defect because the validating-JSON helpers are explicitly used as a sanitization step before schema validation, signing, and WASM-bound JSON emission. Any caller that places a NaN/±∞ float into a `Value` (including via a permissive JS or `f64` deserialization path) will see the forged value `0` downstream, defeating any range/finite check that would otherwise have rejected the input. The Critical-2 coverage test added at this commit additionally locks in the silent-coercion behavior. Return `Error::Unsupported` (or a dedicated non-finite variant) on `None` instead of `unwrap_or(0.into())`.
- [BLOCKING] In `packages/wasm-dpp/src/document/extended_document.rs`:27-303: ExtendedDocumentWasm omits the public toObject()/toJSON() JS ABI
  The exported `#[wasm_bindgen(js_class=ExtendedDocument)]` impl exposes constructor, getters/setters, `toBuffer`, `hash`, `clone`, `validate`, `get`, and `set`, but no `#[wasm_bindgen(js_name=toObject)]` or `#[wasm_bindgen(js_name=toJSON)]` (verified via grep — only `getData` returns the inner properties map, not the full extended-document shape). Peer wrappers in this crate (e.g. `IdentityWasm` in packages/wasm-dpp/src/identity/identity.rs:166-220) and most wasm-dpp2 wrappers expose this conversion ABI, and the entire PR centers on standardizing canonical object/JSON conversion. JS consumers that previously relied on `extendedDocument.toObject()`/`toJSON()` — including `JSON.stringify(extendedDocument)`, which transparently invokes `toJSON` — get `undefined` from the wasm boundary. This is a public ABI regression for JS callers. Add the canonical `toObject`/`toJSON` methods on the exported class.
- [BLOCKING] In `packages/rs-drive-abci/src/query/service.rs`:129-148: Query retry loop never reaches its advertised 1-second timeout (counter reset each iteration)
  `let mut counter = 0;` is declared on line 133, *inside* the `loop { ... }` body started on line 129. Every iteration zeroes the counter, runs `counter += 1` once, then evaluates `if counter >= 100` — which is never true (`counter` is always 1 at that point). The advertised 1-second timeout / `query_counter` increment / `needs_restart = true` branch is unreachable, so a query whose required height never lands keeps sleeping in 10ms increments indefinitely instead of giving up and (eventually) returning `NotServiceable`. Compare with `query_counter`, which is correctly declared outside the loop. Beyond the latency bug, this is a denial-of-service primitive: untrusted gRPC callers can pin `spawn_blocking` worker threads during any state/drive height inconsistency window. Declare `let mut counter = 0;` immediately before the `loop {` on line 129.
- [BLOCKING] In `packages/rs-drive-proof-verifier/src/from_request.rs`:89-371: Vote-query proof verifiers truncate u32 limits/offsets to u16, breaking proof binding
  Six unchecked `as u16` casts on wire-`u32` pagination fields remain at lines 89, 199, 254, 324, 370, and 371 (verified via grep). The server-side drive-abci handlers reject overflow with `u16::try_from(...)`, but the verifier wraps silently — so a malicious or buggy server can answer a client request for `count = 70000` with a proof for `count = 4464` (or any `count & 0xFFFF`) and the verifier will happily verify it against the wrapped query. The `offset` case at line 371 is the strongest: an attacker can shift the entire returned window without verification failing. This is a proof-soundness break at the network trust boundary for ContestedDocumentVotePollDriveQuery, ContestedResourceVotesGivenByIdentityQuery, ContestedDocumentVotePollVotesDriveQuery, VotePollsByDocumentTypeQuery, and VotePollsByEndDateDriveQuery. Replace each `v as u16` with a checked `u16::try_from(v).map_err(...)?` so the verifier matches the server-side validation semantics.
- [SUGGESTION] In `packages/wasm-dpp2/src/identity/partial_identity.rs`:320-425: PartialIdentity key-ID parsing lossy-casts invalid JS numbers into different KeyIDs
  `option_array_to_not_found()` (lines 320-348) accepts any `JsValue` with `as_f64()`, validates only the upper bound (`key_val > u32::MAX as f64`), then casts `key_val as KeyID`. Rust's `f64 as u32` is saturating/truncating: `-1.0 → 0`, `NaN → 0`, `-Infinity → 0`, `1.9 → 1`. A JS caller passing `[-1, 1.5, NaN, -Infinity]` silently produces `{KeyID(0)}` instead of being rejected — conflating invalid inputs with the legitimate key id 0. The same lossy pattern is repeated in `value_to_loaded_public_keys_from_object` (line 363/374, `key_str.parse::<f64>()` → `key_val as u32`) and `value_to_loaded_public_keys_from_json` (lines 413/424), where it additionally accepts strings like `"NaN"`, `"Infinity"`, `"-1"`, `"1.5"` as object keys. The crate already has a stricter `try_to_u32()` helper in packages/wasm-dpp2/src/utils.rs that rejects non-integer/negative/non-finite inputs; the partial-identity parsers should use it (or equivalent finite + non-negative + integral checks) before casting.
- [SUGGESTION] In `packages/rs-dpp/src/serialization/serde_bytes.rs`:90-102: Fixed-size byte deserializer rejects Value::Array of u8 on non-human-readable platform_value
  `AnyShapeVisitor::<N>` advertises (and implements) `visit_byte_buf`, `visit_bytes`, `visit_str` (base64), and `visit_seq` (sequence of `u8`). The HR branch (line 96) uses `deserialize_any`, but the non-HR branch (line 101) hard-wires `deserialize_byte_buf(AnyShapeVisitor::<N>)`. On `platform_value::Deserializer`, `deserialize_byte_buf` delegates to `deserialize_bytes` (packages/rs-platform-value/src/value_serialization/de.rs:347-352), which only accepts `Value::Bytes*`/`Value::Identifier` and rejects `Value::Array` with `invalid_type` before `visit_seq` can run. After the Critical-2 fix in this PR (JSON integer arrays no longer coerce to `Value::Bytes`), JSON→platform_value→struct paths for fixed-size byte fields now fail on the array shape — even though the visitor was written to support it. Either implement an `Array<u8>` fallback in `platform_value::Deserializer::deserialize_byte_buf`, or call `deserialize_any` on the non-HR branch for sources that support it (and keep `deserialize_byte_buf` only for bincode). Note: codex flagged this as fixed; verification shows the platform_value path still rejects arrays here.
- [SUGGESTION] In `packages/rs-dpp/src/serialization/serde_bytes_var.rs`:68-76: Variable-length byte deserializer has the same Value::Array gap on non-human-readable input
  Same defect as serde_bytes.rs: `AnyShapeVisitor` implements `visit_seq` (collecting `u8` elements into `Vec<u8>`), but the non-HR branch unconditionally calls `deserialize_byte_buf(AnyShapeVisitor)`, which on `platform_value::Deserializer` delegates to `deserialize_bytes` and rejects `Value::Array` before the seq visitor runs. Variable-length byte fields read from JSON-derived `Value` objects fail this path despite the visitor being designed to accept them. Same fix shape as serde_bytes.rs.

[github-actions]

✅ DashSDKFFI.xcframework built for this PR.

• Workflow run: https://github.com/dashpay/platform/actions/runs/26298470926
• Artifacts: DashSDKFFI.xcframework (folder), DashSDKFFI.xcframework.zip, xc_checksum.txt

SwiftPM (host the zip at a stable URL, then use):

.binaryTarget(
  name: "DashSDKFFI",
  url: "https://your.cdn.example/DashSDKFFI.xcframework.zip",
  checksum: "0cf5e97b98844771f6db00d567fb476c79612132efbfa7d076850f307b2c187f"
)

Xcode manual integration:

• Download 'DashSDKFFI.xcframework' artifact from the run link above.
• Drag it into your app target (Frameworks, Libraries & Embedded Content) and set Embed & Sign.
• If using the Swift wrapper package, point its binaryTarget to the xcframework location or add the package and place the xcframework at the expected path.

[thepastaclaw]

Code Review

Latest delta (5ae8c26) is a doctest-only correction in rs-platform-value/src/value_serialization/mod.rs and introduces no new defects. Cumulative re-verification confirms all seven prior findings are STILL VALID on disk at the cited locations: NaN/Inf→0 silent coercion in three Value→JSON paths (serde_json.rs:44,140,289); ExtendedDocumentWasm still lacks toObject/toJSON wasm-bindgen exports; query service.rs:133 still declares the retry counter inside the loop body; vote-query proof verifiers still truncate u32 pagination to u16 at six sites in from_request.rs; PartialIdentity (wasm-dpp2) still lossy-casts f64→KeyID; and both serde_bytes deserializers still hard-wire deserialize_byte_buf on the non-human-readable branch. Four are merge-blocking correctness/security bugs; three are suggestion-level robustness issues. No new findings introduced by the delta.

_Note: Inline posting failed (command failed (1): gh pr diff 3573 --repo dashpay/platform
STDOUT:

STDERR:
could not find pull request diff: HTTP 406: Sorry, the diff exceeded the maximum number of files (300). Consider using 'List pull requests files' API or locally cloning the repository instead. (https://api.github.com/repos/), so I posted the same verified findings as a top-level review body._

🔴 4 blocking | 🟡 3 suggestion(s)

7 additional finding(s)

blocking: Value→JSON silently rewrites NaN/±∞ floats to JSON 0 across all three converters

packages/rs-platform-value/src/converter/serde_json.rs (line 44)

All three Value→JsonValue paths in this file — try_into_validating_json (line 44), the by-reference try_to_validating_json (line 140), and TryInto<JsonValue> for Value (line 289) — still use Number::from_f64(float).unwrap_or(0.into()). serde_json::Number::from_f64() returns None for NaN, +inf, and -inf, so any non-finite float is silently rewritten to the valid JSON number 0. This is a data-integrity defect at the Rust→JSON/JS boundary: callers cannot distinguish a real zero from an unrepresentable float, and the JSON-Schema validator that runs over the converted document (e.g. validate_document_v0() calls try_into_validating_json()) validates a different value than the platform-serialized state transition actually carries. The new test validating_json_float_nan_becomes_zero (lines 1135-1139) codifies the broken behavior. All three signatures already return Result<JsonValue, Error>, so the fix is to return Err(Error::Unsupported(...)) (or a new Error::NonFiniteFloat) when Number::from_f64 returns None, and update the test accordingly.

blocking: ExtendedDocumentWasm omits the public toObject()/toJSON() JS ABI

packages/wasm-dpp/src/document/extended_document.rs (line 31)

The exported #[wasm_bindgen(js_class=ExtendedDocument)] impl exposes constructor, getters/setters, toBuffer, hash, clone, validate, get, and set, but no #[wasm_bindgen(js_name=toObject)] or #[wasm_bindgen(js_name=toJSON)]. Grep confirms zero matches in this file. Other wasm wrappers (IdentityWasm, DataContractWasm, …) expose the canonical conversion ABI, and the generic JS-side conversion helper in packages/wasm-dpp2/src/serialization/conversions.rs looks for toJSON() before normalizing. Without these methods on ExtendedDocument, JSON.stringify(extendedDocument) and extendedDocument.toObject() both fail/throw, and the rest of this PR's canonical JsonConvertible/ValueConvertible unification is undermined for the one type that needs it most. Add toObject() and toJSON() wasm-bindgen methods that round-trip through the canonical Rust-side serializers.

blocking: Query retry loop never reaches its advertised 1-second timeout (counter reset every iteration)

packages/rs-drive-abci/src/query/service.rs (line 129)

Verified at line 133: let mut counter = 0; is declared inside the loop body. On every iteration the variable is recreated as 0, then either the loop breaks (heights match) or counter is incremented to 1 and the counter >= 100 check at line 143 is evaluated — which can never be true. The comment at line 142 ("We try for up to 1 second") is unreachable, and needs_restart is never set via this path. Operationally this means a wedged committed_block_height_guard (e.g. under load or during block-processing churn) causes the inner loop to sleep-and-retry forever, parking the spawn-blocking worker indefinitely and bypassing the query_counter > 3 saturation guard at line 150. Because queries are dispatched via spawn_blocking_task_with_name_if_supported("query", ...), an adversary that can induce or wait for height divergence can saturate the blocking pool. Fix: hoist let mut counter = 0; outside the inner loop so increments accumulate.

                let mut needs_restart = false;

                let mut counter = 0;
                loop {
                    let committed_block_height_guard = platform
                        .committed_block_height_guard
                        .load(Ordering::Relaxed);
                    if platform_state.last_committed_block_height() == committed_block_height_guard
                    {
                        break;
                    } else {
                        counter += 1;
                        sleep(Duration::from_millis(10))
                    }

                    // We try for up to 1 second
                    if counter >= 100 {
                        query_counter += 1;
                        needs_restart = true;
                        break;
                    }
                }

blocking: Vote-query proof verifiers truncate wire u32 pagination to u16, breaking proof binding

packages/rs-drive-proof-verifier/src/from_request.rs (line 89)

Verified at six sites: limit: v.count.map(|v| v as u16) (lines 89, 254, 324), limit: value.limit.map(|x| x as u16) (line 199), and limit: v.limit.map(|v| v as u16), offset: v.offset.map(|v| v as u16) (lines 370-371). Server-side handlers in rs-drive-abci's voting query module reject overflow with checked u16::try_from(...), but the verifier silently wraps any u32 > 65535 (e.g. 70000_u32 as u16 == 4464, 65536_u32 as u16 == 0). A malicious or buggy DAPI server can therefore answer a request whose wire count is 70000 with a proof bound to count=4464; the verifier reconstructs its query with the same truncation and "verifies" the proof against the wrong pagination window. For light-client consumers, authenticated proofs no longer guarantee the result corresponds to the requested page. Fix: replace each as u16 with u16::try_from(v).map_err(|_| Error::RequestError { error: format!("... exceeds u16::MAX") })? so the verifier rejects oversized wire values symmetrically with the server.

suggestion: PartialIdentity key-ID parsing lossy-casts invalid JS numbers into different KeyIDs

packages/wasm-dpp2/src/identity/partial_identity.rs (line 320)

Verified at three sites: option_array_to_not_found() (lines 320-348) takes key.as_f64(), checks only the upper bound (> u32::MAX as f64), then casts with key_val as KeyID (line 341). value_to_loaded_public_keys_from_object() (line 374) and value_to_loaded_public_keys_from_json() (line 424) parse object keys as f64 and cast with as u32 after the same one-sided check. Rust's f64 as u32 saturates negatives and NaN to 0, saturates +∞ to u32::MAX, and truncates fractional values. So JS inputs like -1, NaN, Infinity, 1.5, or "4294967296.5" are silently aliased to KeyIDs 0, 0, u32::MAX, 1, and 0 respectively — none rejected. This can collide identity keys at parse time, marking the wrong key index as "loaded" or "not found". The same crate already exposes a strict helper (packages/wasm-dpp2/src/utils.rs) and uses it on the non-object path here; the object/array paths should use it too, or validate is_finite() && >= 0.0 && fract() == 0.0 && <= u32::MAX as f64 before casting. For the string-key paths, parsing as u32 directly (key_str.parse::<u32>()) naturally rejects negatives, fractions, and out-of-range values.

suggestion: Fixed-size byte deserializer rejects Value::Array on non-human-readable platform_value

packages/rs-dpp/src/serialization/serde_bytes.rs (line 90)

The AnyShapeVisitor::<N> advertises and implements four shapes — visit_byte_buf, visit_bytes, visit_str (base64), and visit_seq (sequences of u8). The human-readable branch (line 96) routes through deserialize_any which can dispatch any of them, but the non-HR branch (line 101) hard-wires deserialize_byte_buf(AnyShapeVisitor::<N>). On platform_value::Deserializer, deserialize_byte_buf delegates to deserialize_bytes, which only accepts Value::Bytes*/Identifier and rejects Value::Array before visit_seq can run. The surrounding comment claims all four shapes are accepted regardless of is_human_readable, but that is not true for the platform_value deserializer. Since this PR's Critical-2 fix makes JSON arrays flow into Value::Array faithfully (rather than being heuristically coerced to Value::Bytes), the gap is now reachable in practice for byte fields that started life as JSON arrays. Fix: probe shape via deserialize_any on platform_value as well, or fall back to deserialize_seq on type errors.

suggestion: Variable-length byte deserializer has the same Value::Array gap on non-HR input

packages/rs-dpp/src/serialization/serde_bytes_var.rs (line 68)

Same defect as serde_bytes.rs, for Vec<u8> rather than [u8; N]: the visitor implements visit_seq() and can collect u8 elements, but the non-HR branch unconditionally calls deserialize_byte_buf(AnyShapeVisitor). On platform_value::Deserializer, that path rejects Value::Array before visit_seq is reachable. Resolve identically to the fixed-size case — dispatch through deserialize_any on platform_value or fall back to deserialize_seq for non-Bytes Value variants. Same Critical-2 interaction increases reachability.

🤖 Prompt for all review comments with AI agents

These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.

- [BLOCKING] In `packages/rs-platform-value/src/converter/serde_json.rs`:44-289: Value→JSON silently rewrites NaN/±∞ floats to JSON 0 across all three converters
  All three Value→JsonValue paths in this file — `try_into_validating_json` (line 44), the by-reference `try_to_validating_json` (line 140), and `TryInto<JsonValue> for Value` (line 289) — still use `Number::from_f64(float).unwrap_or(0.into())`. `serde_json::Number::from_f64()` returns `None` for `NaN`, `+inf`, and `-inf`, so any non-finite float is silently rewritten to the valid JSON number `0`. This is a data-integrity defect at the Rust→JSON/JS boundary: callers cannot distinguish a real zero from an unrepresentable float, and the JSON-Schema validator that runs over the converted document (e.g. `validate_document_v0()` calls `try_into_validating_json()`) validates a different value than the platform-serialized state transition actually carries. The new test `validating_json_float_nan_becomes_zero` (lines 1135-1139) codifies the broken behavior. All three signatures already return `Result<JsonValue, Error>`, so the fix is to return `Err(Error::Unsupported(...))` (or a new `Error::NonFiniteFloat`) when `Number::from_f64` returns `None`, and update the test accordingly.
- [BLOCKING] In `packages/wasm-dpp/src/document/extended_document.rs`:31-303: ExtendedDocumentWasm omits the public toObject()/toJSON() JS ABI
  The exported `#[wasm_bindgen(js_class=ExtendedDocument)]` impl exposes constructor, getters/setters, `toBuffer`, `hash`, `clone`, `validate`, `get`, and `set`, but no `#[wasm_bindgen(js_name=toObject)]` or `#[wasm_bindgen(js_name=toJSON)]`. Grep confirms zero matches in this file. Other wasm wrappers (IdentityWasm, DataContractWasm, …) expose the canonical conversion ABI, and the generic JS-side conversion helper in `packages/wasm-dpp2/src/serialization/conversions.rs` looks for `toJSON()` before normalizing. Without these methods on ExtendedDocument, `JSON.stringify(extendedDocument)` and `extendedDocument.toObject()` both fail/throw, and the rest of this PR's canonical JsonConvertible/ValueConvertible unification is undermined for the one type that needs it most. Add `toObject()` and `toJSON()` wasm-bindgen methods that round-trip through the canonical Rust-side serializers.
- [BLOCKING] In `packages/rs-drive-abci/src/query/service.rs`:129-148: Query retry loop never reaches its advertised 1-second timeout (counter reset every iteration)
  Verified at line 133: `let mut counter = 0;` is declared inside the `loop` body. On every iteration the variable is recreated as 0, then either the loop breaks (heights match) or counter is incremented to 1 and the `counter >= 100` check at line 143 is evaluated — which can never be true. The comment at line 142 ("We try for up to 1 second") is unreachable, and `needs_restart` is never set via this path. Operationally this means a wedged `committed_block_height_guard` (e.g. under load or during block-processing churn) causes the inner loop to sleep-and-retry forever, parking the spawn-blocking worker indefinitely and bypassing the `query_counter > 3` saturation guard at line 150. Because queries are dispatched via `spawn_blocking_task_with_name_if_supported("query", ...)`, an adversary that can induce or wait for height divergence can saturate the blocking pool. Fix: hoist `let mut counter = 0;` outside the inner loop so increments accumulate.
- [BLOCKING] In `packages/rs-drive-proof-verifier/src/from_request.rs`:89-371: Vote-query proof verifiers truncate wire u32 pagination to u16, breaking proof binding
  Verified at six sites: `limit: v.count.map(|v| v as u16)` (lines 89, 254, 324), `limit: value.limit.map(|x| x as u16)` (line 199), and `limit: v.limit.map(|v| v as u16), offset: v.offset.map(|v| v as u16)` (lines 370-371). Server-side handlers in rs-drive-abci's voting query module reject overflow with checked `u16::try_from(...)`, but the verifier silently wraps any u32 > 65535 (e.g. `70000_u32 as u16 == 4464`, `65536_u32 as u16 == 0`). A malicious or buggy DAPI server can therefore answer a request whose wire `count` is `70000` with a proof bound to `count=4464`; the verifier reconstructs its query with the same truncation and "verifies" the proof against the wrong pagination window. For light-client consumers, authenticated proofs no longer guarantee the result corresponds to the requested page. Fix: replace each `as u16` with `u16::try_from(v).map_err(|_| Error::RequestError { error: format!("... exceeds u16::MAX") })?` so the verifier rejects oversized wire values symmetrically with the server.
- [SUGGESTION] In `packages/wasm-dpp2/src/identity/partial_identity.rs`:320-425: PartialIdentity key-ID parsing lossy-casts invalid JS numbers into different KeyIDs
  Verified at three sites: `option_array_to_not_found()` (lines 320-348) takes `key.as_f64()`, checks only the upper bound (`> u32::MAX as f64`), then casts with `key_val as KeyID` (line 341). `value_to_loaded_public_keys_from_object()` (line 374) and `value_to_loaded_public_keys_from_json()` (line 424) parse object keys as `f64` and cast with `as u32` after the same one-sided check. Rust's `f64 as u32` saturates negatives and NaN to 0, saturates +∞ to `u32::MAX`, and truncates fractional values. So JS inputs like `-1`, `NaN`, `Infinity`, `1.5`, or `"4294967296.5"` are silently aliased to KeyIDs 0, 0, `u32::MAX`, 1, and 0 respectively — none rejected. This can collide identity keys at parse time, marking the wrong key index as "loaded" or "not found". The same crate already exposes a strict helper (`packages/wasm-dpp2/src/utils.rs`) and uses it on the non-object path here; the object/array paths should use it too, or validate `is_finite() && >= 0.0 && fract() == 0.0 && <= u32::MAX as f64` before casting. For the string-key paths, parsing as `u32` directly (`key_str.parse::<u32>()`) naturally rejects negatives, fractions, and out-of-range values.
- [SUGGESTION] In `packages/rs-dpp/src/serialization/serde_bytes.rs`:90-102: Fixed-size byte deserializer rejects Value::Array<u8> on non-human-readable platform_value
  The `AnyShapeVisitor::<N>` advertises and implements four shapes — `visit_byte_buf`, `visit_bytes`, `visit_str` (base64), and `visit_seq` (sequences of `u8`). The human-readable branch (line 96) routes through `deserialize_any` which can dispatch any of them, but the non-HR branch (line 101) hard-wires `deserialize_byte_buf(AnyShapeVisitor::<N>)`. On `platform_value::Deserializer`, `deserialize_byte_buf` delegates to `deserialize_bytes`, which only accepts `Value::Bytes*`/`Identifier` and rejects `Value::Array` before `visit_seq` can run. The surrounding comment claims all four shapes are accepted regardless of `is_human_readable`, but that is not true for the platform_value deserializer. Since this PR's Critical-2 fix makes JSON arrays flow into `Value::Array` faithfully (rather than being heuristically coerced to `Value::Bytes`), the gap is now reachable in practice for byte fields that started life as JSON arrays. Fix: probe shape via `deserialize_any` on platform_value as well, or fall back to `deserialize_seq` on type errors.
- [SUGGESTION] In `packages/rs-dpp/src/serialization/serde_bytes_var.rs`:68-76: Variable-length byte deserializer has the same Value::Array gap on non-HR input
  Same defect as `serde_bytes.rs`, for `Vec<u8>` rather than `[u8; N]`: the visitor implements `visit_seq()` and can collect `u8` elements, but the non-HR branch unconditionally calls `deserialize_byte_buf(AnyShapeVisitor)`. On `platform_value::Deserializer`, that path rejects `Value::Array` before `visit_seq` is reachable. Resolve identically to the fixed-size case — dispatch through `deserialize_any` on platform_value or fall back to `deserialize_seq` for non-Bytes Value variants. Same Critical-2 interaction increases reachability.