feat(platform)!: subscribe to committed state transitions matching document and address filters

[PastaPastaPasta]

Issue being fixed or feature implemented

Closes #5274.

Applications that react to Platform activity poll for it today, for example Dash Forge's inbox, CI runners and webhook relay. A wallet watching its addresses does the same. Polling cost grows with users times feeds, runs into the gateway's per-IP limit, and notices changes late.

This adds subscribeToStateTransitions, a server-streaming Platform RPC. A client describes what it wants to hear about, and DAPI streams each committed, successfully executed state transition that matches, with checkpoints to resume from without gaps. It covers:

• a platform address getting funds or spending them;
• a document appearing on a contract or document type;
• a document matching a query being created, or changing.

Drive has had DriveDocumentQueryFilter for this purpose (#2761, #2781, #5114), but nothing used it. The earlier attempt #2795 was closed.

What was done?

The commits are meant to be reviewed one at a time:

1. feat(drive): DriveDocumentQueryFilter matches canonical values.
   • Encodings: different encodings of one value never compared equal before. For example, an identifier as bytes vs base58 vs Identifier, or a u8 field vs a U64 operand. Clause values and the transition values a clause reads are now both brought to the form the schema stores, element by element on typed arrays, which keeps feat(platform)!: index the elements of typed arrays (PV14) #5370's typed-array matching.
   • Outside the schema: clauses on fields outside the schema are refused.
   • Null: null never satisfies an ordered comparison.
   • Canonical operands: validate() refuses operands that canonicalize_clause_values() would still change, so a filter used without that step fails loudly instead of missing matches.
   • One-time build: the filter is generic over how it holds its contract, so a subscription builds it once rather than per transition.
2. feat(dapi)!: the RPC, regenerated clients and Drive's unimplemented stub.
   • Request: 1–16 filters, each a DocumentFilter or an AddressFilter, plus an optional from_block_height.
   • Response: a match (height, time, protocol version, hash, bytes, which filters and batch positions matched) or a checkpoint.
3. feat(sdk): dash_platform_queries::subscriptions.
   • Filters: filter types, wire conversion, and ResolvedFilters, which binds document filters to their contracts and matches transitions.
   • Document filters: contract, optional document type, actions (create, replace, delete, transfer, update price, purchase), each with clauses on the new data, $id of the original, the recipient or buyer, the new price, and the batch owner.
   • Address filters: up to 256 addresses on the SENDER, RECIPIENT or ANY side, classified in one exhaustive match over every StateTransition variant.
4. feat(rs-dapi): serving the stream from the Tenderdash block store.
   • Scanning: each subscription walks committed heights with its own cursor, up to the highest height whose execution results are saved. History and live blocks are read the same way, and new-block events only wake the scan, so a missed event can't lose a block.
   • Shared reads: reads are single-flight and shared through a cache bounded by a conservative estimate of decoded memory, with at most 8 Tenderdash calls in flight. Matches share the cached transaction bytes rather than copying them.
   • Never skips a height: an undecodable transaction, an unknown protocol version or a pruned height ends the stream at that height.
   • Limits:
     • 256 subscriptions per node and 16 per source, keyed the same way as the shielded proof budget's SourceKey;
     • starts must be within 50k blocks behind and 1k ahead of the tip;
     • a client that stops reading for 60 s has its buffered messages dropped and its stream ended with RESOURCE_EXHAUSTED.
5. feat(dashmate): an Envoy route with the Core streams' timeouts. It replaces the route for subscribePlatformEvents, which has no RPC.
6. feat(sdk): Sdk::subscribe_to_state_transitions(filters, from_block_height, config).
   • Checks: verifies each transition's hash and decodes it with a bounded value depth.
   • Resuming: resumes from the last checkpoint on another node with capped backoff.
   • Bad nodes: a node that sends a wrong hash, bad match indexes or a regressing checkpoint is banned.
   • Undecodable transitions: these end the subscription with an "upgrade the SDK" error instead of banning nodes, since every node sends the same bytes.

Design choices worth a look

• Matching reads only the transition. Of a document before a replace, delete, transfer, price update or purchase, only $id is known, so clauses on the original document other than $id are refused up front. The one exception is the delete of an indexOnly document, which carries its values.
• Filters are bound once, to the contract version current at subscribe time. From protocol 14, validate_update v1 makes property types, integer widths, encodings and document-type flags immutable, and nothing can be removed. A later update therefore can't change what an admitted filter matches. On earlier protocol versions, an update that widens an integer field may need a resubscribe; the endpoint doc says so.
• The SDK doesn't re-match or fetch contracts. It has no way to verify inclusion, so a dishonest node could fabricate a matching transition anyway. The documented contract is: delivery is at least once and may be incomplete; act on an event with a proved read; catch up from a proved read's metadata.height + 1.
• Status codes tell a client what to do.
  • OUT_OF_RANGE: the start is behind every node's window; catch up with a proved read.
  • FAILED_PRECONDITION: this node lacks or can't decode the height; try another node.
  • UNAVAILABLE: retry.
• Where-clause wire decoding. It lives in dash_platform_queries::documents::from_proto. drive-abci keeps its existing copy rather than taking dash-platform-queries as a runtime dependency.
• Size. +20.4k lines, of which 14.9k are regenerated gRPC clients. The rest is about 3.1k lines of code, proto and docs, and 2.4k lines of tests.

Split out: the Value Display panic fix is #5434.

Not in this PR

• wasm-sdk / js-evo-sdk bindings. The matcher and the Rust SDK already build for wasm32, and rs-dapi-client's grpc-web transport supports server streaming.
• Identity, token and data-contract filters. The request's oneof takes them later without breaking existing clients.

How Has This Been Tested?

• cargo test -p drive --lib query::filter: 84 passed. Covers canonical encodings, long values, typed-array elements, null ordering, fields outside the schema, and wrong-typed operands refused without formatting them.
• cargo test -p dash-platform-queries --lib: 82 passed. Covers address roles across transition variants, batch attribution, $id original-document matching, refusal of undecidable clauses, limits and the wire round trip.
• cargo test -p rs-dapi --lib: 364 passed. The subscription tests run the scan loop against an in-memory chain:
  • replay then live, without gaps;
  • live and future starts, and out-of-window starts;
  • idle checkpoints;
  • a tip published only once its results are saved;
  • bounded concurrent Tenderdash fetches;
  • a 2 MiB block staying cached;
  • per-source limits;
  • an undecodable transaction ending the stream;
  • stalled or failed streams holding their slot until their buffer is read or discarded.
• cargo test -p dash-sdk --lib subscriptions: 17 passed. Covers hash checks, depth bounds, node faults vs client faults, resume heights, backoff reset, transient vs fatal codes, the first open behaving like a reopen, and the idle window on paused time.
• Clippy is clean for every touched crate. dash-sdk builds for wasm32-unknown-unknown. check-grpc-coverage.py passes.
• Not done: an end-to-end run on a local dashmate devnet. The network-only test tests/fetch/state_transition_subscriptions.rs compiles under network-testing but has not been run against a live network.

Breaking Changes

• Rust gRPC server API (dapi-grpc): platform_server::Platform gains a required associated type subscribeToStateTransitionsStream and method subscribe_to_state_transitions, because tonic generates no default stubs. Downstream implementations must add both; drive-abci's QueryService returning unimplemented is the minimal migration.
• Rust API (drive): DriveDocumentQueryFilter<'a> becomes DriveDocumentQueryFilter<C: Deref<Target = DataContract>>, so a subscription can own or share its contract. Code that names the type must change DriveDocumentQueryFilter<'a> to DriveDocumentQueryFilter<&'a DataContract>. Code that constructs it is unchanged.
• Wire and consensus: on the protobuf wire the RPC and its messages are purely additive. The Drive filter changes affect only DriveDocumentQueryFilter, which nothing else uses and which is not consensus code. No shipped protocol generation changes behaviour.

Checklist:

• I have performed a self-review of my own code
• I have commented my code, particularly in hard-to-understand areas
• I have added or updated relevant unit/integration/functional/e2e tests
• I have added "!" to the title and described breaking changes in the corresponding section if my code contains any
• I have made corresponding changes to the documentation if needed
• If I added or changed GroveDB structure, I described it in the area's structure.rs, regenerated grovedb-structure.json, and checked the structure viewer link posted on this pull request

For repository code-owners and collaborators only

• I have assigned this pull request to a milestone

🤖 Generated with Claude Code

[coderabbitai]

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

• Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[thepastaclaw]

✅ Final review complete — no blockers (commit bbd00dc) · triage: critical

[thepastaclaw]

Preliminary review — Phase 1 blocker gate

Verified the supplied findings against head 5681b07. Seven in-scope suggestions remain, including incomplete SDK contract following, unknown-version handling, and historical schema binding; none violates the review policy's consensus-blocking invariants. This was a static review: the supplied CI snapshot shows Rust workspace tests skipped and PR Hygiene pending, so it does not establish that the subscription tests passed.

The Phase-1 findings confirmed by a fresh verifier are above the gate budget. Phase 2 is deferred until a fresh same-head revalidation brings them under it.

🟡 7 suggestion(s)

Review provenance

Source: reviewer 1: muse-spark-1.3-contributor (agent: phase1-reviewer, role: general); reviewer 2: muse-spark-1.3-contributor (agent: phase1-reviewer, role: rust-quality); final verifier: gpt-6.1-sol (agent: sol-gate-verifier, role: verifier)

• Triage: normal by gpt-6.1-sol (effort low) — This is a large, intricate streaming RPC and SDK change, but the diff changes client-facing query/filter handling rather than consensus rules, funds movement, cryptography, peer-to-peer deserialization, or storage migrations.
• Phase 1 reviewers: muse-spark-1.3-contributor — general (completed, effort xhigh); agent phase1-reviewer, muse-spark-1.3-contributor — rust-quality (completed, effort xhigh); agent phase1-reviewer
• Phase 1 model: muse-spark-1.3-contributor — not quota-gated; passed over gemini-3.8-flash-high (antigravity below 15% reserve: weekly 15% left, 5h 100% left), glm-5.3-flash (not used above high effort; tier asks max)
• Fresh verifier: gpt-6.1-sol — verifier; agent sol-gate-verifier
• Phase 2 reviewers: not run (deferred by the Phase-1 gate)

🤖 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.

In `packages/rs-sdk/src/platform/subscriptions.rs`:
- [SUGGESTION] packages/rs-sdk/src/platform/subscriptions.rs:122-128: SDK silently drops its contract-follow filter when the request is full
  When a request contains 16 filters and includes document filters, this branch omits the internal contract-update filter but still accepts the subscription. DAPI calls follow() for every successfully executed transition it scans, including updates that match none of the caller's filters; the SDK can call follow() only for transitions it receives. Unless another caller filter happens to match those updates, the SDK retains its old contract binding while DAPI rebinds. This contradicts the method's guarantee that both sides follow contract updates and can cause local matching to diverge after an update. Reject requests that leave no room for the internal filter, or ensure the caller's existing filters cover all required contract updates. This is a client subscription correctness issue, not a consensus blocker.
- [SUGGESTION] packages/rs-sdk/src/platform/subscriptions.rs:279-280: SDK falls back to latest() for unknown protocol versions instead of skipping
  An unknown protocol version is replaced with PlatformVersion::latest(), after which the SDK evaluates filters and follows contract updates using tables that do not describe the advertised version. Hash verification and successful decoding do not establish that this substitution is valid. DAPI explicitly fails closed for unknown block versions, whereas the SDK can emit an event carrying an unsupported protocol version or mutate its filter bindings under guessed rules. Reject or skip the event with a diagnostic rather than selecting a different protocol version.

In `packages/dash-platform-queries/src/subscriptions/resolved.rs`:
- [SUGGESTION] packages/dash-platform-queries/src/subscriptions/resolved.rs:174-192: Historical replay starts with the current contract rather than the historical binding
  DAPI fetches the current contract before starting the scan, and follow() changes that binding only when a contract update is encountered. Consequently, the initial historical segment is evaluated using the current schema. Backward-compatible updates protect many existing-field filters, so ordinary field additions do not automatically imply incorrect matching. However, matching also calls data_as_stored(), which generates omitted generatedFrom properties using the bound schema. A generated property added after the replayed block can therefore be synthesized for an old transition and satisfy a clause even though that property was not generated when the transition executed. Bind replay to the contract version active at its starting height, or explicitly document that historical document clauses use the initial current-schema binding rather than execution-time schema semantics.
- [SUGGESTION] packages/dash-platform-queries/src/subscriptions/resolved.rs:297-312: Document clause matching deep-clones resolved clauses on the scan hot path
  For each relevant inner document transition, this branch clones the document type name and clones each attempted DocumentActionMatchClauses alternative into a temporary DriveDocumentQueryFilter. The clause structure owns collections and values, so these are deep clones rather than borrowed views. Block decoding is shared, but matching is performed separately for every subscription, multiplying these allocations across subscriptions, transitions, and alternatives. Prebuild reusable match state when resolving the filter or provide a borrowing matcher so evaluation does not clone the resolved clauses.

In `packages/dapi-grpc/protos/platform/v0/platform.proto`:
- [SUGGESTION] packages/dapi-grpc/protos/platform/v0/platform.proto:4247-4249: Document ActionMatch cannot distinguish a missing action from CREATE
  ActionMatch.action is a non-optional proto3 enum, and CREATE is zero. An omitted action therefore decodes as CREATE, and action_match_from_proto converts it to Some(DocumentAction::Create). The shared Rust validator explicitly rejects action alternatives without a named action, but wire requests cannot express that missing state. A client that sends clauses without naming an action silently receives a create-only subscription instead of an invalid-argument response. Make the field optional and validate its presence, or introduce a rejected unspecified enum value before publishing the new API.

In `packages/rs-dapi/src/services/platform_service/subscribe_to_state_transitions/block_source.rs`:
- [SUGGESTION] packages/rs-dapi/src/services/platform_service/subscribe_to_state_transitions/block_source.rs:202: block_results failures discard the cause before retrying
  Every block_results error becomes ReadMiss::NotYet, including persistent transport or configuration failures. Retrying is appropriate when results lag behind block storage, but after the unreadable deadline the client receives only a generic message and the underlying cause has not been recorded here. The block-fetch half preserves its error, making failures in the results half substantially harder to diagnose. Record the error before mapping it to the retryable state.

In `packages/rs-sdk/tests/fetch/state_transition_subscriptions.rs`:
- [SUGGESTION] packages/rs-sdk/tests/fetch/state_transition_subscriptions.rs:9-34: SDK event verification has no offline test coverage
  The new network test checks only the opening checkpoint. The unit test in subscriptions.rs checks retry-status classification, not accept(), so neither exercises hash rejection, decoding rejection, local filter matching, contract following, or removal of the internal filter's matches. These checks implement the advertised SDK verification behavior and can regress without either test failing. Add offline unit tests in the subscriptions module that construct response messages and verify rejected events, internal-update suppression, caller-visible match indexes, and unsupported protocol versions. The PR states that the network test was not run, and the supplied CI snapshot shows Rust workspace tests skipped.

[PastaPastaPasta]

Thanks. All seven were checked against the code; six are addressed in f2c4819:

• Contract-follow filter dropped when the request is full (subscriptions.rs): the SDK now refuses a request with document filters that leaves no room for its contract-update filter, instead of going on without it.
• Unknown protocol version falls back to latest(): the SDK now fails closed with an error, as DAPI does. A transition it cannot decode is also an error now rather than a skip; both mean the SDK needs upgrading. Wrong-hash and non-matching transitions are still skipped as node misbehaviour.
• Historical replay uses the current contract: valid. Recovering the version in force at an arbitrary start height is not generally possible (contract history exists only for contracts that keep it), so the behaviour is now documented on ResolvedFilters::resolve and in the endpoint doc, including the generatedFrom case.
• ActionMatch.action cannot be missing: the field is now optional Action action; an unset action is rejected with INVALID_ARGUMENT (new wire test). Clients regenerated.
• block_results failures lose their cause: logged at debug before the height is retried.
• No offline coverage of the SDK's verification: added unit tests that drive accept() with constructed responses: delivery with caller filter indexes, wrong hash, non-matching, internal-filter-only matches, unknown protocol version and undecodable transition, and the resume height.

Not changed: deep clones in document matching (resolved.rs). The clone happens only after a transition has passed the contract and document-type check, so only transitions on the subscribed type pay it, for at most 12 alternatives. Removing it needs DriveDocumentQueryFilter to borrow its clauses instead of owning them, a wider refactor of the drive type I'd rather leave to a follow-up.

---

🤖 Posted autonomously by Codex on behalf of pasta.

[thepastaclaw]

Re-review — Final validation — Phase 1 + Phase 2

Static verification at f2c4819 confirms one blocking SDK trust-boundary defect and five non-blocking issues involving resource limits, failure handling, and rebinding coverage. Six prior findings are fixed; the bounded clause-cloning optimization can be deferred to a separate follow-up. No builds or tests were run; the supplied exact-head CI snapshot shows Rust workspace tests skipped and PR Hygiene pending.

🔴 1 blocking | 🟡 5 suggestion(s)

Review provenance

Source: reviewer 1: muse-spark-1.3-contributor (agent: phase1-reviewer, role: general); reviewer 2: muse-spark-1.3-contributor (agent: phase1-reviewer, role: rust-quality); reviewer 3: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 4: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 5: gpt-6.1-sol (agent: phase2-reviewer, role: platform-versioning); reviewer 6: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 7: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)

• Triage: normal by gpt-6.1-sol (effort low) — This is a large, intricate streaming RPC and SDK change, but it does not clearly alter consensus, funds movement, cryptography, peer-facing network deserialization, or storage migrations.
• Phase 1 reviewers: muse-spark-1.3-contributor — general (completed, effort xhigh); agent phase1-reviewer, muse-spark-1.3-contributor — rust-quality (completed, effort xhigh); agent phase1-reviewer
• Phase 1 model: muse-spark-1.3-contributor — not quota-gated; passed over gemini-3.8-flash-high (antigravity below 15% reserve: weekly 15% left, 5h 100% left), glm-5.3-flash (not used above high effort; tier asks max)
• Fresh verifier: gpt-6.1-sol — final-verifier; agent sol-verifier
• Phase 2 reviewers: gpt-6.1-sol — general (completed, effort high); agent phase2-reviewer, gpt-6.1-sol — architecture-layering (completed, effort high); agent phase2-reviewer, gpt-6.1-sol — platform-versioning (completed, effort high); agent phase2-reviewer, gpt-6.1-sol — rust-quality (completed, effort high); agent phase2-reviewer, gpt-6.1-sol — security-auditor (completed, effort high); agent phase2-reviewer

🤖 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.

In `packages/rs-sdk/src/platform/subscriptions.rs`:
- [BLOCKING] packages/rs-sdk/src/platform/subscriptions.rs:285-288: Authenticate contract updates before replacing proved filter bindings
  Initial document bindings come from DataContract::fetch(), whose proof-verification path authenticates the contract. This call to follow() instead installs the contract embedded in an unproved network event: the shared implementation converts the payload and rebinds filters by contract ID without authenticating its commitment. A malicious node can fabricate an update for a watched contract and compute the accompanying hash itself. Changing a generatedFrom rule can then make later document events match a schema that was never proved, while the internal update remains hidden from the caller. This violates the documented proved-schema matching guarantee, independently of the explicitly unproved completeness and block metadata. Treat updates as refresh notifications and obtain an authenticated binding through a proved query or verified contract history before rebinding. If the required historical version cannot be authenticated, fail closed or expose explicitly unverified semantics. Add a regression showing that a fabricated update cannot replace the proved binding.
- [SUGGESTION] packages/rs-sdk/src/platform/subscriptions.rs:260-263: Fail explicitly on an unsupported response envelope
  An absent or unrecognized response envelope produces Ok(None), even though unsupported protocol versions and undecodable transitions now return errors because the SDK cannot interpret them safely. Unknown protobuf oneof fields are discarded during decoding, so a newer unsupported envelope also reaches this branch as None. If a stream keeps delivering such responses, next() keeps waiting without yielding events or advancing its resume height; those messages also reset the consecutive-failure counter. Return an explicit unsupported-envelope error rather than treating the response as a locally rejected transition, and add an offline accept() assertion for this case.

In `packages/rs-dapi/src/services/platform_service/subscribe_to_state_transitions/stream.rs`:
- [SUGGESTION] packages/rs-dapi/src/services/platform_service/subscribe_to_state_transitions/stream.rs:322-326: Reacquire replay capacity after releasing it for client sends
  Releasing replay_permit before sending correctly prevents slow consumers from holding catch-up capacity. However, acquisition occurs only in the outer loop. After a matching block releases the permit, the inner page loop reads all remaining transaction-bearing heights without reacquiring it; the periodic-checkpoint branch has the same effect. Subscribers replaying distinct ranges can therefore exceed max_replaying concurrent historical reads, because single-flight sharing only combines reads of the same height. Reacquire capacity before subsequent historical reads while continuing to release it before client sends. Extend the stalled-client coverage with distinct replay ranges and a blocking/counting source that verifies the concurrency bound after a matching block.

In `packages/rs-dapi/src/services/platform_service/subscribe_to_state_transitions/mod.rs`:
- [SUGGESTION] packages/rs-dapi/src/services/platform_service/subscribe_to_state_transitions/mod.rs:71-82: Reject oversized filter lists before fetching their contracts
  The 1–16 filter bound is enforced by ResolvedFilters::resolve() only after this loop fetches every distinct document contract. from_proto() does not validate the request-level count, and nonexistent contracts return Ok(None), so they do not stop the loop. The 128 KiB transport limit still permits thousands of minimal document filters, turning a request that must be rejected into thousands of sequential Drive lookups. Admission bounds concurrent requests, not backend work within each request. Validate the count before conversion and contract fetching, and test that an oversized request performs no Drive calls. Apply early cardinality validation in the SDK as well, where contract fetching currently precedes rejection.

In `packages/dash-platform-queries/src/subscriptions/tests.rs`:
- [SUGGESTION] packages/dash-platform-queries/src/subscriptions/tests.rs:493-505: The rebinding test cannot detect a no-op implementation
  This test resolves its filter against contract and then rebinds to a clone of that same contract. The unconstrained document-type filter already matches the transition before rebinding, so removing rebind_data_contract()'s implementation would leave the assertion passing. The test also bypasses follow(), including its conversion of the update payload. Use distinct contract versions and assert a matching outcome that changes only after rebinding, then exercise an actual DataContractUpdate through follow(). The new SDK accept() tests cover useful verification branches, but their internal-filter fixture uses an identity filter and does not test actual contract following.

In `packages/dash-platform-queries/src/subscriptions/resolved.rs`:
- [SUGGESTION] packages/dash-platform-queries/src/subscriptions/resolved.rs:188-196: Report contract conversion failures instead of silently ignoring them
  Deserializing a StateTransition does not establish that its embedded contract can be constructed. DataContract::try_from_platform_versioned() still performs fallible document-type construction and required-since checks when full_validation is false. This if-let discards any error, leaving the previous binding without a diagnostic, so the SDK can continue after receiving an update it could not interpret. Handle the conversion failure explicitly: return an error where following must fail closed, or at minimum report the error and contract ID if retaining the previous binding is intentional. Keep this distinct from the documented historical-replay case where a successfully converted older contract cannot serve a particular filter.

Out-of-scope follow-up suggestions (1)

These are valid observations, but they are outside this PR's scope and should be handled in separate issues or author/maintainer-requested PRs rather than blocking this review.

• Direct connections bypass the per-IP subscription limit — The bypass is conditional on exposing DAPI outside the repository's gateway boundary. packages/dashmate/docker-compose.yml and templates/dynamic-compose.yml.dot expose port 3010 only within the container network rather than publishing it; the Envoy route reaches that internal service, and use_remote_address is enabled. client_ip() explicitly documents that a missing forwarded header denotes a non-gateway request. Without evidence of an untrusted direct-access path in the supported deployment, requesting an additional direct-client policy would be speculative hardening rather than a demonstrated defect in this PR.
  • Follow-up: Consider creating a separate issue or author/maintainer-requested PR for this.

[thepastaclaw]

Re-review — Final validation — Phase 1 + Phase 2

Verified the supplied findings against head 9085160: one blocking CPU-exhaustion issue and three correctness suggestions remain. Twelve prior findings are fixed; the bounded document-matching allocation optimization remains intentionally deferred. This was static verification only: the supplied CI snapshot shows Rust workspace tests skipped, with NPM release validation and PR Hygiene still pending.

🔴 1 blocking | 🟡 3 suggestion(s)

Review provenance

Source: reviewer 1: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 2: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 3: muse-spark-1.3-contributor (agent: phase1-reviewer, role: general); reviewer 4: gpt-6.1-sol (agent: phase2-reviewer, role: platform-versioning); reviewer 5: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 6: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); reviewer 7: muse-spark-1.3-contributor (agent: phase1-reviewer, role: rust-quality); reviewer 8: gpt-6-astra (agent: phase2-reviewer, role: general); reviewer 9: gpt-6-astra (agent: phase2-reviewer, role: architecture-layering); reviewer 10: gpt-6-astra (agent: phase2-reviewer, role: platform-versioning); reviewer 11: gpt-6-astra (agent: phase2-reviewer, role: rust-quality); reviewer 12: gpt-6-astra (agent: phase2-reviewer, role: security-auditor); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)

• Triage: critical by gpt-6.1-sol (effort low) — This intricate streaming feature adds peer-facing network deserialization in packages/dash-platform-queries/src/subscriptions/proto.rs, including StateTransitionFilter::from_proto and document clause decoding, meeting the critical-surface criterion.
• Phase 1 reviewers: muse-spark-1.3-contributor — general (completed, effort xhigh); agent phase1-reviewer, muse-spark-1.3-contributor — rust-quality (completed, effort xhigh); agent phase1-reviewer
• Phase 1 model: muse-spark-1.3-contributor — not quota-gated; passed over gemini-3.8-flash-high (antigravity below 15% reserve: weekly 15% left, 5h 100% left), glm-5.3-flash (not used above high effort; tier asks max)
• Single stage: Phase 1 and Phase 2 reviewed this head side by side, with no blocker gate between them (triage tier)
• Fresh verifier: gpt-6.1-sol — final-verifier; agent sol-verifier
• Phase 2 reviewers: gpt-6.1-sol — general (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — architecture-layering (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — platform-versioning (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — rust-quality (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — security-auditor (completed, effort xhigh); agent phase2-reviewer, gpt-6-astra — general (completed, effort xhigh); agent phase2-reviewer, gpt-6-astra — architecture-layering (completed, effort xhigh); agent phase2-reviewer, gpt-6-astra — platform-versioning (completed, effort xhigh); agent phase2-reviewer, gpt-6-astra — rust-quality (completed, effort xhigh); agent phase2-reviewer, gpt-6-astra — security-auditor (completed, effort xhigh); agent phase2-reviewer
• Model comparison: every Phase-2 reviewer also ran on gpt-6-astra; the verifier weighed both sets without knowing which model wrote which

🤖 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.

In `packages/rs-drive/src/query/filter.rs`:
- [BLOCKING] packages/rs-drive/src/query/filter.rs:827-829: Bound Base58 identifier operands before canonicalization
  An unauthenticated subscription request can contain a document DELETE filter for an existing contract/type with an original-document `$id EQUAL` operand consisting of 120,000 `z` characters. It fits the 128 KiB transport limit and passes the structural checks. This call then reaches serialize_value_for_key_v0(), which invokes Value::to_identifier_bytes() and bs58::decode(text).into_vec() before checking that the decoded identifier is 32 bytes. The pinned bs58 0.5.1 decoder processes each character against the growing decoded buffer, making this input quadratic CPU work. DAPI resolves filters synchronously on a Tokio worker before replay throttling applies; concurrent admitted requests can monopolize workers, and asynchronous timeouts cannot interrupt the decode. Bound identifier text before invoking the codec, or decode into a fixed 32-byte destination. Cover system identifiers, schema identifier properties, and list operands, while preserving genuinely long string-property operands.
- [SUGGESTION] packages/rs-drive/src/query/filter.rs:818-825: Preserve empty byte-array operands during canonicalization
  The index-key codec is not lossless for empty byte arrays: encode_value_for_tree_keys(Value::Bytes(vec![])) returns an empty buffer, and decode_value_for_tree_keys() interprets that buffer as Value::Null. A valid equality operand therefore becomes null and is rejected by validate_against_schema(), which requires Value::Bytes for byte-array equality. IN operands containing an empty byte array fail for the same reason. The existing withByteArrays fixture permits this value because byteArrayField has maxItems but no positive minItems. Preserve the byte-array type during normalization instead of decoding its empty index-key sentinel, and add EQUAL and IN regressions containing an empty byte array.

In `packages/rs-sdk/src/platform/subscriptions.rs`:
- [SUGGESTION] packages/rs-sdk/src/platform/subscriptions.rs:268-270: Retain pending contract refreshes across cancellation
  pop_first() removes the refresh obligation before awaiting DataContract::fetch(). If a caller drops a pending next() future through tokio::select! or a timeout, the subscription survives but the popped identifier is lost. The update notification has already been consumed, so a subsequent next() can read later events and checkpoints using the previous binding without retrying the required proved refresh. Keep the identifier queued until fetching and rebinding complete. Add an offline cancellation regression that suspends the fetch, drops the next() future, and verifies that the following call still refreshes the contract before reading another message.
- [SUGGESTION] packages/rs-sdk/src/platform/subscriptions.rs:306: Reject checkpoints with no representable resume height
  Checkpoint heights are unproved node input, and protobuf permits block_height = u64::MAX. Adding one panics with overflow checks enabled and wraps to zero without them, producing a resume cursor that DAPI explicitly rejects on reconnect. Unauthenticated checkpoint semantics do not require accepting metadata that panics the SDK or corrupts its cursor. Use checked_add() before updating resume_from, and add an offline accept() regression asserting that the maximum-height checkpoint returns an error without changing the previous cursor.

[thepastaclaw]

Re-review — Final validation — Phase 1 + Phase 2

Verified the combined reviewer findings against head be0b69d. Of the 17 prior findings, 15 are fixed, one remains blocking because derived identifier fields bypass the Base58 input bound, and one performance optimization remains intentionally deferred; three additional suggestions are confirmed. This was static verification only: the supplied CI snapshot shows Rust workspace tests skipped, with NPM release build validation and PR Hygiene pending.

🔴 1 blocking | 🟡 3 suggestion(s)

1 carried-forward finding(s) already raised on this PR; not re-posting as new inline comments.

Review provenance

Source: reviewer 1: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 2: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 3: gpt-6.1-sol (agent: phase2-reviewer, role: platform-versioning); reviewer 4: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 5: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); reviewer 6: gpt-6-astra (agent: phase2-reviewer, role: general); reviewer 7: muse-spark-1.3-contributor (agent: phase1-reviewer, role: general); reviewer 8: gpt-6-astra (agent: phase2-reviewer, role: architecture-layering); reviewer 9: gpt-6-astra (agent: phase2-reviewer, role: platform-versioning); reviewer 10: gpt-6-astra (agent: phase2-reviewer, role: rust-quality); reviewer 11: muse-spark-1.3-contributor (agent: phase1-reviewer, role: rust-quality); reviewer 12: gpt-6-astra (agent: phase2-reviewer, role: security-auditor); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)

• Triage: critical by gpt-6.1-sol (effort low) — The change is intricate and adds peer-facing network deserialization in packages/dash-platform-queries/src/subscriptions/proto.rs::StateTransitionFilter::from_proto alongside a new public streaming RPC and decoding of remotely supplied block and transition data.
• Phase 1 reviewers: muse-spark-1.3-contributor — general (completed, effort xhigh); agent phase1-reviewer, muse-spark-1.3-contributor — rust-quality (completed, effort xhigh); agent phase1-reviewer
• Phase 1 model: muse-spark-1.3-contributor — not quota-gated; passed over gemini-3.8-flash-high (antigravity below 15% reserve: weekly 15% left, 5h 100% left), glm-5.3-flash (not used above high effort; tier asks max)
• Single stage: Phase 1 and Phase 2 reviewed this head side by side, with no blocker gate between them (triage tier)
• Fresh verifier: gpt-6.1-sol — final-verifier; agent sol-verifier
• Phase 2 reviewers: gpt-6.1-sol — general (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — architecture-layering (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — platform-versioning (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — rust-quality (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — security-auditor (completed, effort xhigh); agent phase2-reviewer, gpt-6-astra — general (completed, effort xhigh); agent phase2-reviewer, gpt-6-astra — architecture-layering (completed, effort xhigh); agent phase2-reviewer, gpt-6-astra — platform-versioning (completed, effort xhigh); agent phase2-reviewer, gpt-6-astra — rust-quality (completed, effort xhigh); agent phase2-reviewer, gpt-6-astra — security-auditor (completed, effort xhigh); agent phase2-reviewer
• Model comparison: every Phase-2 reviewer also ran on gpt-6-astra; the verifier weighed both sets without knowing which model wrote which

🤖 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.

In `packages/rs-drive/src/query/filter.rs`:
- [SUGGESTION] packages/rs-drive/src/query/filter.rs:833-842: Preserve string values without round-tripping index sentinels
  The string index-key codec is not lossless: both empty text and a one-character U+0000 string encode as `[0]`, which `decode_value_for_tree_keys()` converts to empty text. Document serialization preserves their distinction with a length prefix, and the fixture's unrestricted `niceDocument.name` permits both. Because this normalizer runs on operands and transition values, an EQUAL filter for empty text also matches a transition containing only U+0000, and SDK re-matching repeats the same false positive. An IN list containing both strings becomes a duplicate list and is rejected; a STARTS_WITH operand containing only U+0000 becomes an empty prefix and is rejected. Preserve typed string values directly rather than using the index-key representation as a lossless canonicalizer. Add EQUAL, IN, and STARTS_WITH regressions distinguishing empty text from U+0000 without changing the shipped index encoding.
- [SUGGESTION] packages/rs-drive/src/query/filter.rs:871-882: Canonicalize packed U8 IN operands as candidate lists
  `WhereClause::in_values()`, schema validation, and `WhereOperator::eval()` explicitly support `Value::Bytes` as a packed IN list for U8 properties. The subscription wire decoder also preserves `BytesValue` as `Value::Bytes`. This match handles only `Value::Array` element-wise, so a valid U8 operand such as `IN Value::Bytes(vec![1, 2])` falls through to scalar conversion. The U8 property codec then calls `to_integer()` on the entire byte vector and rejects it, causing both SDK resolution and DAPI to refuse a clause that the existing validator and evaluator support. Handle packed byte IN lists before scalar conversion while retaining their U8-only field restriction and keeping byte-array scalar operands distinct. Cover native resolution, wire round-trip, and matching against U8 values.
- [BLOCKING] packages/rs-drive/src/query/filter.rs:817-832: Bound Base58 identifier operands before canonicalization
  (existing thread: https://github.com/dashpay/platform/pull/5285#discussion_r4186329460)
  The new bound protects the listed system identifiers and flattened identifier properties, but derived identifier index fields still bypass it. DPP supports a field such as `postId.$ownerId` with effective type `Identifier`; that field is absent from `flattened_properties()`, so `identifier` is false here. The fallback at lines 844–846 calls `serialize_value_for_key_v0()`, which resolves `derived_index_property_type(key)` and reaches `to_identifier_bytes()` → `bs58::decode(text).into_vec()`. A client can submit a CREATE filter with a new-document equality clause on that field and 120,000 `z` characters against a contract declaring the derived index. The request fits the 128 KiB transport cap, and `action_clauses()` canonicalizes it before schema validation rejects the unsupported field. The pinned decoder iterates over its growing output for each input character, so this still performs quadratic synchronous work on a DAPI worker before replay throttling. Reject derived-index-only fields before invoking their codec, since transition matching cannot read their values, or include their effective types in the pre-decode identifier bound. Add EQUAL and IN regressions using a derived identifier index.

In `packages/dapi-grpc/protos/platform/v0/platform.proto`:
- [SUGGESTION] packages/dapi-grpc/protos/platform/v0/platform.proto:166-167: Record the Rust server-trait break in release metadata
  The RPC is additive on the protobuf wire, but it changes the exported Rust server API. `dapi-grpc` generates servers without default stubs, so tonic adds a required `subscribeToStateTransitionsStream` associated type and `subscribe_to_state_transitions` method to `platform_server::Platform`. Existing downstream service or mock implementations no longer compile until they add these items; the new implementations in `QueryService` and `PlatformServiceImpl` show the migration. The PR's Breaking Changes section currently says there are none. Document this server-side Rust source break and apply the repository's breaking-change title/release convention so downstream implementers receive notice. This is package API release treatment, not a PlatformVersion activation change.

[thepastaclaw]

Re-review — Final validation — Phase 1 + Phase 2

At fbcd07a, verification confirms one blocking untrusted-input panic and one numeric range-normalization defect. Nineteen prior findings are fixed; the bounded document-clause cloning optimization remains intentionally deferred. Validation was static only: the supplied CI snapshot shows Rust workspace tests skipped, with NPM release validation and PR Hygiene pending.

🔴 1 blocking | 🟡 1 suggestion(s)

Review provenance

Source: reviewer 1: muse-spark-1.3-contributor (agent: phase1-reviewer, role: general); reviewer 2: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 3: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 4: muse-spark-1.3-contributor (agent: phase1-reviewer, role: rust-quality); reviewer 5: gpt-6.1-sol (agent: phase2-reviewer, role: platform-versioning); reviewer 6: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 7: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); reviewer 8: gpt-6-astra (agent: phase2-reviewer, role: general); reviewer 9: gpt-6-astra (agent: phase2-reviewer, role: architecture-layering); reviewer 10: gpt-6-astra (agent: phase2-reviewer, role: platform-versioning); reviewer 11: gpt-6-astra (agent: phase2-reviewer, role: rust-quality); reviewer 12: gpt-6-astra (agent: phase2-reviewer, role: security-auditor); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)

• Triage: critical by gpt-6.1-sol (effort low) — The intricate streaming feature adds peer-facing network deserialization and validation in packages/dash-platform-queries/src/subscriptions/proto.rs, notably StateTransitionFilter::from_proto, alongside new decoding of remotely supplied blocks and transitions.
• Phase 1 reviewers: muse-spark-1.3-contributor — general (completed, effort xhigh); agent phase1-reviewer, muse-spark-1.3-contributor — rust-quality (completed, effort xhigh); agent phase1-reviewer
• Phase 1 model: muse-spark-1.3-contributor — not quota-gated; passed over gemini-3.8-flash-high (antigravity below 15% reserve: weekly 15% left, 5h 100% left), glm-5.3-flash (not used above high effort; tier asks max)
• Single stage: Phase 1 and Phase 2 reviewed this head side by side, with no blocker gate between them (triage tier)
• Fresh verifier: gpt-6.1-sol — final-verifier; agent sol-verifier
• Phase 2 reviewers: gpt-6.1-sol — general (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — architecture-layering (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — platform-versioning (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — rust-quality (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — security-auditor (completed, effort xhigh); agent phase2-reviewer, gpt-6-astra — general (completed, effort xhigh); agent phase2-reviewer, gpt-6-astra — architecture-layering (completed, effort xhigh); agent phase2-reviewer, gpt-6-astra — platform-versioning (completed, effort xhigh); agent phase2-reviewer, gpt-6-astra — rust-quality (completed, effort xhigh); agent phase2-reviewer, gpt-6-astra — security-auditor (completed, effort xhigh); agent phase2-reviewer
• Model comparison: every Phase-2 reviewer also ran on gpt-6-astra; the verifier weighed both sets without knowing which model wrote which

🤖 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.

In `packages/rs-drive/src/query/filter.rs`:
- [BLOCKING] packages/rs-drive/src/query/filter.rs:865-866: Prevent malformed event values from panicking the SDK matcher
  A subscription node can fabricate a binary-decodable document CREATE for the watched contract/type, supply its correct SHA-256 hash, and encode an integer property as Value::Text("a".repeat(19) + "é"). SDK accept() deserializes the transition without schema or signature validation and passes its properties into this new canonicalization path. The integer codec calls Value::to_integer(), whose wrong-type error formats the value through Display. In packages/rs-platform-value/src/display.rs, that formatter calls text.split_at(20): byte 20 is inside the final UTF-8 character, so it panics before the codec returns an error. The .ok()? does not catch a panic, which escapes subscription.next(), terminating the consuming task or aborting a panic=abort client. Although the formatter predates this PR, the new matcher exposes it to fabricated network events; malformed subscription operands also reach it through canonical_operand(). Make value truncation UTF-8-safe and add regressions for both a rejected Unicode operand and a fabricated SDK event with a wrong-typed Unicode property.

In `packages/dash-platform-queries/src/subscriptions/resolved.rs`:
- [SUGGESTION] packages/dash-platform-queries/src/subscriptions/resolved.rs:556-561: Canonicalize numeric bounds before grouping range clauses
  These clauses are extracted and grouped before schema canonicalization at lines 612–614. Grouping paired bounds calls WhereClause::less_than(), which accepts only matching Value variants. On a U8 property, a CREATE filter containing stars >= Value::I64(1) and stars <= Value::U64(5) therefore fails with RangeClausesNotGroupable, although both values individually canonicalize to U8 and the equivalent BETWEEN operand succeeds. The wire decoder preserves int64_value and uint64_value as I64 and U64, so this affects wire requests as well as native SDK requests using different integer widths. Normalize raw operands against the document schema before extraction/grouping, retaining the identifier-length and schema-field guards rather than changing shipped grouping implementations. Add native and wire-resolution regressions for mixed-encoding bounds.

[thepastaclaw]

Re-review — Final validation — Phase 1 + Phase 2

At head 7cfbb22, the 22 prior findings reconcile to 21 fixed and one intentionally deferred performance refactor. Four non-blocking issues remain: null values can satisfy numeric ranges, accepted native numeric operands can fail after wire encoding, and cache accounting plus eager response copies understate subscription memory use. Verification was static only; the supplied CI snapshot shows Rust workspace and Dashmate E2E tests skipped, with NPM release validation and PR Hygiene pending.

🟡 4 suggestion(s)

Review provenance

Source: reviewer 1: muse-spark-1.3-contributor (agent: phase1-reviewer, role: general); reviewer 2: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 3: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 4: gpt-6.1-sol (agent: phase2-reviewer, role: platform-versioning); reviewer 5: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 6: muse-spark-1.3-contributor (agent: phase1-reviewer, role: rust-quality); reviewer 7: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); reviewer 8: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 9: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 10: gpt-6.1-sol (agent: phase2-reviewer, role: platform-versioning); reviewer 11: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 12: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)

• Triage: critical by gpt-6.1-sol (effort low) — This large, intricate change introduces peer-facing network deserialization in StateTransitionFilter::from_proto in packages/dash-platform-queries/src/subscriptions/proto.rs and a new public streaming RPC with request validation and resource admission.
• Phase 1 reviewers: muse-spark-1.3-contributor — general (completed, effort xhigh); agent phase1-reviewer, muse-spark-1.3-contributor — rust-quality (completed, effort xhigh); agent phase1-reviewer
• Phase 1 model: muse-spark-1.3-contributor — not quota-gated; passed over gemini-3.8-flash-high (antigravity below 15% reserve: weekly 15% left, 5h 100% left), glm-5.3-flash (not used above high effort; tier asks max)
• Single stage: Phase 1 and Phase 2 reviewed this head side by side, with no blocker gate between them (triage tier)
• Fresh final gate: an independent Phase-2 review ran after iterative findings were reconciled
• Fresh verifier: gpt-6.1-sol — final-verifier; agent sol-verifier
• Phase 2 reviewers: gpt-6.1-sol — general (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — architecture-layering (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — platform-versioning (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — rust-quality (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — security-auditor (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — general (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — architecture-layering (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — platform-versioning (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — rust-quality (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — security-auditor (completed, effort xhigh); agent phase2-reviewer

🤖 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.

In `packages/rs-drive/src/query/filter.rs`:
- [SUGGESTION] packages/rs-drive/src/query/filter.rs:880-888: Reject null event values before evaluating numeric ranges
  The property codec returns an empty encoding for Value::Null and decodes that encoding back to Null. Because the empty-encoding guard excludes null inputs, this path returns Some(Null), which evaluate_clauses passes to WhereOperator::eval(). Its ordered comparisons use Value's derived PartialOrd, where Null sorts after every numeric variant. A CREATE filter such as stars > U64(3) on the U8 rating field therefore matches a transition carrying stars: Null. data_as_stored() does not validate this property, and SDK accept() can deliver a fabricated, binary-decodable event with a correct hash through this path. Prevent null from participating in numeric ordered comparisons, while preserving any intentionally supported null semantics elsewhere, and add matcher and offline SDK accept() regressions. This is newly exposed subscription-matching behavior, not consensus execution.

In `packages/dash-platform-queries/src/subscriptions/proto.rs`:
- [SUGGESTION] packages/dash-platform-queries/src/subscriptions/proto.rs:164-168: Preserve normalized operand semantics when encoding subscriptions
  The SDK resolves a clone of the filters but retains and transmits their original operands. Native resolution accepts an UPDATE_PRICE equality operand Value::U128(50), normalizing it to U64(50). This encoder instead reuses value_to_proto(), which emits Text("50") for U128; the subscription decoder preserves Text, and DAPI's strict to_integer::<u64>() rejects it. The document-clause encoder has the same discrepancy for a U8 field queried with U128(3), so this is not limited to unsupported 128-bit schema fields. Encode accepted operands from their normalized representation, or add subscription-specific integer encoding that preserves representable numeric values without converting them to text. Add native/wire round-trip resolution tests for both price and document clauses.

In `packages/rs-dapi/src/services/platform_service/subscribe_to_state_transitions/stream.rs`:
- [SUGGESTION] packages/rs-dapi/src/services/platform_service/subscribe_to_state_transitions/stream.rs:415-425: Defer transaction payload copies until responses are sent
  match_block() constructs all matching responses before the first send, and tx.bytes.as_ref().clone() deep-copies each serialized transaction from its shared Arc<Vec<u8>>. The 32-message channel bound does not bound these unsent copies: the send loop's iterator retains every remaining response while awaiting a slow consumer. Subscribers matching the same block therefore retain approximately subscriber count times matching payload bytes in private buffers, outside the shared block-cache budget, until delivery or the send deadline. Precompute lightweight transaction indexes and match descriptors while preserving contract-follow order, then materialize each payload when sending it. Add a stalled-reader regression with more matches than STREAM_BUFFER to check that unsent responses do not eagerly duplicate the entire block.

In `packages/rs-dapi/src/services/platform_service/subscribe_to_state_transitions/block_source.rs`:
- [SUGGESTION] packages/rs-dapi/src/services/platform_service/subscribe_to_state_transitions/block_source.rs:128-131: Include decoded allocations in the block-cache memory budget
  Each cached transaction retains both its serialized bytes and its decoded StateTransition, but this weight charges only one additional serialized length for the decoded representation. Compact Value arrays can substantially exceed that estimate: protocol 14 permits 1024-element boolean typed-array properties, whose transition encoding uses two bytes per boolean while the decoded Vec allocates a full Value slot per element. Several such properties fit within the 20 KiB transition limit, yielding decoded allocations many times larger than their charged weight. BlockSource caches successful transactions before applying subscriber filters, so a historical subscription with a non-matching identity filter can populate this amplified cache; replay concurrency and per-IP admission limits do not correct the retained-memory accounting. Include decoded collection capacities and nested allocations in the weight, or use a demonstrably conservative bound, and add a compact-array regression. The current nominal 64 MiB budget is not a conservative bound on retained block memory.

[thepastaclaw]

Re-review — Final validation — Phase 1 + Phase 2

At head 0c67483, 24 of the 26 prior findings are fixed, one remains partially unfixed, and the clause-cloning optimization remains intentionally deferred. Verification also confirms a blocking memory-retention issue in stalled subscriptions and a numeric-null regression coverage gap. This was a static review only; the supplied head-specific CI snapshot shows Rust workspace tests skipped and other checks pending, queued, or running.

🔴 1 blocking | 🟡 2 suggestion(s)

1 carried-forward finding(s) already raised on this PR; not re-posting as new inline comments.

Review provenance

Source: reviewer 1: muse-spark-1.3-contributor (agent: phase1-reviewer, role: general); reviewer 2: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 3: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 4: gpt-6.1-sol (agent: phase2-reviewer, role: platform-versioning); reviewer 5: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 6: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); reviewer 7: muse-spark-1.3-contributor (agent: phase1-reviewer, role: rust-quality); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)

• Triage: critical by gpt-6.1-sol (effort low) — The intricate new subscription endpoint changes peer-facing network deserialization in packages/dash-platform-queries/src/subscriptions/proto.rs through StateTransitionFilter::from_proto and its document-clause decoders, meeting the critical-surface bar.
• Phase 1 reviewers: muse-spark-1.3-contributor — general (completed, effort xhigh); agent phase1-reviewer, muse-spark-1.3-contributor — rust-quality (completed, effort xhigh); agent phase1-reviewer
• Phase 1 model: muse-spark-1.3-contributor — not quota-gated; passed over gemini-3.8-flash-high (antigravity below 15% reserve: weekly 15% left, 5h 100% left), glm-5.3-flash (not used above high effort; tier asks max)
• Single stage: Phase 1 and Phase 2 reviewed this head side by side, with no blocker gate between them (triage tier)
• Fresh verifier: gpt-6.1-sol — final-verifier; agent sol-verifier
• Phase 2 reviewers: gpt-6.1-sol — general (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — architecture-layering (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — platform-versioning (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — rust-quality (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — security-auditor (completed, effort xhigh); agent phase2-reviewer

🤖 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.

In `packages/rs-dapi/src/services/platform_service/subscribe_to_state_transitions/stream.rs`:
- [BLOCKING] packages/rs-dapi/src/services/platform_service/subscribe_to_state_transitions/stream.rs:327-333: Account for decoded blocks pinned by stalled subscriptions
  The scan retains its Arc<CommittedBlock>, including every decoded transaction, throughout the send loop. Cache eviction cannot free a block still owned by a scan, and the replay permit is released before a backpressured send suspends it. Consequently, unauthenticated subscriptions replaying distinct transaction-heavy heights can each pin a different decoded block while additional subscriptions continue reading; the 128 MiB cache budget and eight-reader limit do not bound these allocations. Compact boolean arrays expand into Vec<Value> allocations many times their encoded size, so even the gateway's default 100 concurrent requests can retain several GiB on a sufficiently transaction-heavy history before the send deadlines expire. Charge in-flight block ownership against a shared memory budget, including blocks not retained by the cache, or retain only bounded serialized payload references after matching and release the decoded block before awaiting sends. Add a stalled-reader regression across distinct heavy heights and cache eviction.

In `packages/rs-drive/src/query/filter.rs`:
- [SUGGESTION] packages/rs-drive/src/query/filter.rs:3747-3753: Make the null regression exercise a numeric field
  This regression uses the string property niceDocument.name. canonical_property_value() already rejects Null for string properties because its String branch accepts only Text, so removing the new null guard from evaluate_clauses() would leave the test passing. The numeric path that required the guard instead encodes Null as an empty buffer and decodes it back to Null, allowing Value's derived ordering to satisfy a greater-than bound. Use the U8 rating.stars fixture with stars > U64(3) and a transition carrying stars: Null, so removing the guard makes the regression fail.

In `packages/dash-platform-queries/src/subscriptions/proto.rs`:
- [SUGGESTION] packages/dash-platform-queries/src/subscriptions/proto.rs:145: Preserve normalized operand semantics when encoding subscriptions
  (existing thread: https://github.com/dashpay/platform/pull/5285#discussion_r4187638043)
  The new narrowing fixes the reported U128 examples, but nonnegative I128 operands above i64::MAX still lose numeric semantics even when they fit u64. For example, an UPDATE_PRICE equality operand Value::I128(1_i128 << 63) resolves natively because price canonicalization calls to_integer::<u64>() and produces U64. Here, i64::try_from() fails and preserves I128, so the shared encoder emits decimal Text; the subscription decoder preserves that Text and DAPI rejects it as a non-integer. The SDK transmits the original filters, not its resolved copy, so a locally accepted subscription fails when opened. Fall back to unsigned-64 narrowing when signed narrowing fails, and extend the native/wire regression to i64::MAX + 1, including list operands.

[thepastaclaw]

Re-review — Final validation — Phase 1 + Phase 2

Static verification of the complete PR range at a181760 found four non-consensus suggestions and no remaining blockers. Of the 28 prior findings, 26 are fixed, operand serialization remains partly unresolved, and the bounded document-clause cloning optimization remains intentionally deferred. No builds or tests were run; the supplied head-specific CI snapshot reports Rust workspace tests skipped and several checks pending or queued.

🟡 4 suggestion(s)

1 carried-forward finding(s) already raised on this PR; not re-posting as new inline comments.

Review provenance

Source: reviewer 1: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 2: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 3: muse-spark-1.3-contributor (agent: phase1-reviewer, role: general); reviewer 4: gpt-6.1-sol (agent: phase2-reviewer, role: platform-versioning); reviewer 5: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 6: muse-spark-1.3-contributor (agent: phase1-reviewer, role: rust-quality); reviewer 7: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); reviewer 8: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 9: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 10: gpt-6.1-sol (agent: phase2-reviewer, role: platform-versioning); reviewer 11: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 12: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)

• Triage: critical by gpt-6.1-sol (effort low) — The change is intricate and adds peer-facing network deserialization in packages/dash-platform-queries/src/subscriptions/proto.rs (StateTransitionFilter::from_proto), alongside a new remotely accessible streaming endpoint with substantial input validation and resource-management logic.
• Phase 1 reviewers: muse-spark-1.3-contributor — general (completed, effort xhigh); agent phase1-reviewer, muse-spark-1.3-contributor — rust-quality (completed, effort xhigh); agent phase1-reviewer
• Phase 1 model: muse-spark-1.3-contributor — not quota-gated; passed over gemini-3.8-flash-high (antigravity below 15% reserve: weekly 15% left, 5h 100% left), glm-5.3-flash (not used above high effort; tier asks max)
• Single stage: Phase 1 and Phase 2 reviewed this head side by side, with no blocker gate between them (triage tier)
• Fresh final gate: an independent Phase-2 review ran after iterative findings were reconciled
• Fresh verifier: gpt-6.1-sol — final-verifier; agent sol-verifier
• Phase 2 reviewers: gpt-6.1-sol — general (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — architecture-layering (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — platform-versioning (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — rust-quality (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — security-auditor (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — general (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — architecture-layering (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — platform-versioning (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — rust-quality (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — security-auditor (completed, effort xhigh); agent phase2-reviewer

🤖 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.

In `packages/dash-platform-queries/src/subscriptions/resolved.rs`:
- [SUGGESTION] packages/dash-platform-queries/src/subscriptions/resolved.rs:619-621: Bound price IN candidate lists during subscription resolution
  Price IN operands bypass the candidate bounds used by ordinary document clauses. This branch forwards the price clause to DriveDocumentQueryFilter, whose price validation checks only that array elements are integers; it does not call WhereClause::in_values(), which rejects empty lists, more than 100 candidates, and duplicates. A wire request with 20,000 repeated small uint64 candidates fits the 128 KiB request limit and resolves successfully. Every relevant document transition then deep-clones that list, and an UPDATE_PRICE with an absent price scans it through array.contains(). Replay admission limits concurrent readers, not this per-transition matching work. Validate price IN cardinality and duplicates at the subscription boundary, consistently with ordinary IN clauses, and add oversized native and wire request regressions. This does not require the deferred borrowing-matcher refactor.

In `packages/rs-sdk/src/platform/subscriptions.rs`:
- [SUGGESTION] packages/rs-sdk/src/platform/subscriptions.rs:202-206: Evict a failing stream's node before reconnecting
  Failures after a stream opens bypass the executor's address-failure handling. open() discards ExecutionResponse.address through into_inner(), and next() clears only the stream before retrying. AddressList deliberately uses a sticky active set, so with with_active_set_size(1), a node that sends the opening checkpoint and then OUT_OF_RANGE for pruned history is selected again on every reopen. The five-failure budget expires before its 5–7.5-minute rotation slot, even when a healthy standby retains the requested block. Preserve the selected address and evict it from rotation for node-specific resumable failures, without treating ordinary stream-lifetime expiry as node misbehavior. Add deterministic offline reconnect coverage with a failing active node and a healthy standby; verify the checkpoint-derived replacement height, replay of an interrupted block, backoff, and the failure limit for repeated opening-checkpoint-only failures.

In `packages/dash-platform-queries/src/subscriptions/participants.rs`:
- [SUGGESTION] packages/dash-platform-queries/src/subscriptions/participants.rs:204-209: Classify a document purchaser as a recipient
  document_recipient() handles only Transfer, so an identity filter with Role::Recipient never matches a document PURCHASE by that identity. The buyer is available without a state lookup: the batch transformer passes its owner_id as purchaser_id, and the purchase action assigns that identity as the document's new owner. The document purchase owner clause already uses the batch owner, and token_recipient() likewise classifies a DirectPurchase buyer as Recipient. Currently participants() records the document buyer only as Sender, while matches_batched() relies on this helper for recipient matching. Pass the batch owner into document_recipient(), return it for Purchase, and add a regression asserting that the buyer's recipient filter matches and reports the purchase's batch position.

In `packages/dash-platform-queries/src/subscriptions/proto.rs`:
- [SUGGESTION] packages/dash-platform-queries/src/subscriptions/proto.rs:170-175: Preserve normalized operand semantics when encoding subscriptions
  (existing thread: https://github.com/dashpay/platform/pull/5285#discussion_r4187638043)
  The I128 unsigned fallback fixes the reported price boundary, but the SDK still resolves a clone and serializes the original operands. For an F64 document property, EQUAL U128(1_u128 << 64) resolves natively because the property's codec converts it to Float. narrowed() cannot represent it as u64, so the shared encoder emits decimal Text; the subscription decoder preserves Text, which the F64 codec rejects. There is another instance of the same mismatch: a byte-array equality operand Array([U8(1), U8(2)]) resolves to Bytes natively, but serialization and decoding produce Array([U64(1), U64(2)]), which to_binary_bytes() rejects. Encode subscription operands from their schema-normalized representation using the existing Drive canonicalization, while leaving the separately documented getDocuments encoder behavior unchanged. Add native/wire resolution regressions for wide F64 operands, including IN, and byte-array operands supplied as U8 arrays.

[thepastaclaw]

Re-review — Final validation — Phase 1 + Phase 2

At head 8d4bba6, verification of the complete diff confirms two non-blocking subscription issues: resumed stream-opening refusals can repeatedly select the same unsuitable node, and heartbeat ticks discard replay semaphore queue positions. All 31 prior findings were revalidated: 30 are addressed, and the bounded clause-cloning optimization remains intentionally deferred; no blocking finding remains. This was static verification only: the supplied exact-head CI snapshot shows Rust workspace tests skipped and several other checks pending, queued, or running, so passing validation at this head is not established.

🟡 2 suggestion(s)

Review provenance

Source: reviewer 1: muse-spark-1.3-contributor (agent: phase1-reviewer, role: general); reviewer 2: muse-spark-1.3-contributor (agent: phase1-reviewer, role: rust-quality); reviewer 3: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 4: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 5: gpt-6.1-sol (agent: phase2-reviewer, role: platform-versioning); reviewer 6: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 7: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); reviewer 8: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 9: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 10: gpt-6.1-sol (agent: phase2-reviewer, role: platform-versioning); reviewer 11: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 12: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)

• Triage: normal by gpt-6.1-sol (effort low) — This is a large, intricate streaming API and SDK change, but its filtering and committed-block replay do not change consensus, funds movement, cryptography, peer-to-peer deserialization, or storage migrations.
• Phase 1 reviewers: muse-spark-1.3-contributor — general (completed, effort xhigh); agent phase1-reviewer, muse-spark-1.3-contributor — rust-quality (completed, effort xhigh); agent phase1-reviewer
• Phase 1 model: muse-spark-1.3-contributor — not quota-gated; passed over gemini-3.8-flash-high (antigravity below 15% reserve: weekly 15% left, 5h 100% left), glm-5.3-flash (not used above high effort; tier asks max)
• Fresh final gate: an independent Phase-2 review ran after iterative findings were reconciled
• Fresh verifier: gpt-6.1-sol — final-verifier; agent sol-verifier
• Phase 2 reviewers: gpt-6.1-sol — general (completed, effort high); agent phase2-reviewer, gpt-6.1-sol — architecture-layering (completed, effort high); agent phase2-reviewer, gpt-6.1-sol — platform-versioning (completed, effort high); agent phase2-reviewer, gpt-6.1-sol — rust-quality (completed, effort high); agent phase2-reviewer, gpt-6.1-sol — security-auditor (completed, effort high); agent phase2-reviewer, gpt-6.1-sol — general (completed, effort high); agent phase2-reviewer, gpt-6.1-sol — architecture-layering (completed, effort high); agent phase2-reviewer, gpt-6.1-sol — platform-versioning (completed, effort high); agent phase2-reviewer, gpt-6.1-sol — rust-quality (completed, effort high); agent phase2-reviewer, gpt-6.1-sol — security-auditor (completed, effort high); agent phase2-reviewer

🤖 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.

In `packages/rs-sdk/src/platform/subscriptions.rs`:
- [SUGGESTION] packages/rs-sdk/src/platform/subscriptions.rs:258-267: Fail over when a node refuses a resumed subscription before opening
  open() discards ExecutionError.address on failure, so subscription-specific failover applies only after a stream opens. A replacement node more than 1,000 blocks behind a valid checkpoint-derived resume cursor refuses the request with OUT_OF_RANGE in SubscriptionService::start(). The generic gRPC CanRetry implementation excludes OUT_OF_RANGE and FAILED_PRECONDITION, so the executor neither retries elsewhere nor bans that address. reopen() nevertheless treats these statuses as resumable and retries them; with an active-set size of one, it repeatedly selects the same unsuitable node until the five-failure budget expires, even when a healthy standby can serve the cursor. Retain the failed address and evict or ban it for node-specific opening failures during reconnection, while keeping genuinely invalid initial requests terminal. Add deterministic coverage in which a lagging replacement refuses the open and a healthy standby receives the unchanged resume cursor.

In `packages/rs-dapi/src/services/platform_service/subscribe_to_state_transitions/stream.rs`:
- [SUGGESTION] packages/rs-dapi/src/services/platform_service/subscribe_to_state_transitions/stream.rs:377-385: Preserve replay semaphore queue position across heartbeat ticks
  Each loop iteration constructs a new acquire_owned() future inside tokio::select!. When the checkpoint timer wins, that acquisition is cancelled; Tokio explicitly documents that cancellation loses its FIFO queue position. Historical reads can remain in flight beyond the 10-second heartbeat interval—the Tenderdash request timeout is 30 seconds—so an older waiter can repeatedly move behind newer subscriptions while emitting unchanged checkpoints. Under sustained arrivals, this can starve its replay despite permits periodically becoming available. Preserve the acquisition's queue position across heartbeat ticks, and ensure a backpressured checkpoint send cannot retain an acquired or reserved permit. Add a paused-time regression with staggered waiters that crosses a heartbeat interval before releasing capacity and asserts that the oldest healthy waiter still acquires first.

[thepastaclaw]

Re-review — Final validation — Phase 1 + Phase 2

Verified the complete diff at 1314918 and revalidated all 57 prior findings: 56 are fixed, and the clause-cloning optimization remains intentionally deferred. Two new performance suggestions are confirmed; no in-scope blockers remain. Validation was static only: the supplied head-matched CI snapshot shows Rust workspace and functional tests skipped and PR Hygiene pending, and the PR reports no live end-to-end validation.

🟡 2 suggestion(s)

Review provenance

Source: reviewer 1: muse-spark-1.3-contributor (agent: phase1-reviewer, role: general); reviewer 2: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 3: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 4: gpt-6.1-sol (agent: phase2-reviewer, role: platform-versioning); reviewer 5: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 6: muse-spark-1.3-contributor (agent: phase1-reviewer, role: rust-quality); reviewer 7: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); reviewer 8: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 9: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 10: gpt-6.1-sol (agent: phase2-reviewer, role: platform-versioning); reviewer 11: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 12: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)

• Triage: critical by gpt-6.1-sol (effort low) — The change is intricate and modifies peer-facing network deserialization in packages/rs-dpp/src/state_transition/mod.rs through deserialize_from_bytes_untrusted_in_version_with_max_value_depth, used to bound state-transition values received from nodes.
• Phase 1 reviewers: muse-spark-1.3-contributor — general (completed, effort xhigh); agent phase1-reviewer, muse-spark-1.3-contributor — rust-quality (completed, effort xhigh); agent phase1-reviewer
• Phase 1 model: muse-spark-1.3-contributor — not quota-gated; passed over gemini-3.8-flash-high (antigravity below 15% reserve: weekly 15% left, 5h 100% left), glm-5.3-flash (not used above high effort; tier asks max)
• Single stage: Phase 1 and Phase 2 reviewed this head side by side, with no blocker gate between them (triage tier)
• Fresh final gate: an independent Phase-2 review ran after iterative findings were reconciled
• Fresh verifier: gpt-6.1-sol — final-verifier; agent sol-verifier
• Phase 2 reviewers: gpt-6.1-sol — general (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — architecture-layering (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — platform-versioning (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — rust-quality (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — security-auditor (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — general (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — architecture-layering (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — platform-versioning (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — rust-quality (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — security-auditor (completed, effort xhigh); agent phase2-reviewer

🤖 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.

In `packages/rs-dapi/src/services/platform_service/subscribe_to_state_transitions/block_source.rs`:
- [SUGGESTION] packages/rs-dapi/src/services/platform_service/subscribe_to_state_transitions/block_source.rs:160-164: Account for per-shard admission limits when configuring the block cache
  The pinned quick_cache 0.6.23 divides this budget among independently sized shards. Its default OptionsBuilder requests four shards per detected core, and replace_placeholder refuses entries heavier than a shard's hot allocation. With eight detected cores, the 4096-item estimate permits 32 shards, leaving approximately 4 MiB per shard. A block containing 100 KiB of successful transaction bytes exceeds that admission threshold under BlockWeighter's decoded-value multiplier, despite fitting comfortably within the overall 128 MiB budget. The loader returns the block and shares it with existing waiters, but does not retain it; a subsequent reader fetches and decodes that height again. This undermines the PR's decoded-block reuse guarantee for transaction-bearing blocks. Set an explicit shard count appropriate for the block weights you intend to retain, or use tighter conservative retained-memory accounting while preserving the memory bound. Add a counting-source regression that reads a realistically sized block twice after the first load completes and verifies that the second read performs no Tenderdash calls.

In `packages/rs-drive/src/query/filter.rs`:
- [SUGGESTION] packages/rs-drive/src/query/filter.rs:929-930: Avoid index-key heap allocations for numeric event values
  Integer properties still take this codec round trip even when the event value already has the property's canonical variant. For example, encode_u8 and encode_u16 allocate a Vec, and their decoders allocate another Vec to reverse the sign-bit transformation before constructing the scalar Value. evaluate_clauses invokes this normalization for each evaluated numeric predicate on each relevant subscription, so sharing decoded blocks does not amortize these allocations. Add borrowed fast paths for canonical U8/U16 values and convert other accepted integer representations directly with to_integer, preserving the existing null and out-of-range behavior. This is a local optimization of the new canonicalization path and does not require the deferred borrowing-clause refactor.

[thepastaclaw]

Re-review — Final validation — Phase 1 + Phase 2

At dfb5df1, source inspection confirms that 58 prior findings are addressed and the document-clause borrowing optimization remains intentionally deferred. One non-blocking SDK issue remains: into_stream() discards the subscription after contract-refresh errors that next() explicitly permits callers to retry. This was static validation only; the supplied CI snapshot shows Rust workspace tests skipped, NPM release validation and PR Hygiene pending, and the PR reports no completed devnet end-to-end run.

🟡 1 suggestion(s)

Review provenance

Source: reviewer 1: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 2: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 3: gpt-6.1-sol (agent: phase2-reviewer, role: platform-versioning); reviewer 4: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 5: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); reviewer 6: gpt-6-astra (agent: phase2-reviewer, role: general); reviewer 7: gpt-6-astra (agent: phase2-reviewer, role: architecture-layering); reviewer 8: muse-spark-1.3-contributor (agent: phase1-reviewer, role: general); reviewer 9: muse-spark-1.3-contributor (agent: phase1-reviewer, role: rust-quality); reviewer 10: gpt-6-astra (agent: phase2-reviewer, role: platform-versioning); reviewer 11: gpt-6-astra (agent: phase2-reviewer, role: rust-quality); reviewer 12: gpt-6-astra (agent: phase2-reviewer, role: security-auditor); reviewer 13: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 14: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 15: gpt-6.1-sol (agent: phase2-reviewer, role: platform-versioning); reviewer 16: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 17: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)

• Triage: critical by gpt-6.1-sol (effort low) — The change is large and intricate and adds peer-facing network deserialization in packages/dash-platform-queries/src/subscriptions/proto.rs, notably StateTransitionFilter::from_proto, alongside a new public streaming RPC.
• Phase 1 reviewers: muse-spark-1.3-contributor — general (completed, effort xhigh); agent phase1-reviewer, muse-spark-1.3-contributor — rust-quality (completed, effort xhigh); agent phase1-reviewer
• Phase 1 model: muse-spark-1.3-contributor — not quota-gated; passed over gemini-3.8-flash-high (antigravity below 15% reserve: weekly 15% left, 5h 100% left), glm-5.3-flash (not used above high effort; tier asks max)
• Single stage: Phase 1 and Phase 2 reviewed this head side by side, with no blocker gate between them (triage tier)
• Fresh final gate: an independent Phase-2 review ran after iterative findings were reconciled
• Fresh verifier: gpt-6.1-sol — final-verifier; agent sol-verifier
• Phase 2 reviewers: gpt-6.1-sol — general (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — architecture-layering (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — platform-versioning (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — rust-quality (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — security-auditor (completed, effort xhigh); agent phase2-reviewer, gpt-6-astra — general (completed, effort xhigh); agent phase2-reviewer, gpt-6-astra — architecture-layering (completed, effort xhigh); agent phase2-reviewer, gpt-6-astra — platform-versioning (completed, effort xhigh); agent phase2-reviewer, gpt-6-astra — rust-quality (completed, effort xhigh); agent phase2-reviewer, gpt-6-astra — security-auditor (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — general (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — architecture-layering (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — platform-versioning (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — rust-quality (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — security-auditor (completed, effort xhigh); agent phase2-reviewer
• Model comparison: every Phase-2 reviewer also ran on gpt-6-astra; the verifier weighed both sets without knowing which model wrote which

🤖 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.

In `packages/rs-sdk/src/platform/subscriptions.rs`:
- [SUGGESTION] packages/rs-sdk/src/platform/subscriptions.rs:375: Preserve the subscription after retryable refresh errors in into_stream
  next() returns contract-refresh errors before setting terminated, preserving the pending refresh and allowing another call to retry. into_stream() instead supplies None as the unfold state for every error, so it drops the subscription after yielding that error. A temporary proved-fetch failure or the retryable stale-contract error therefore permanently ends this adapter: handling the error and polling again returns EOF, with no way to recover the retained refresh obligation or checkpoint from the consumed subscription. Keep the unfold state when terminated is unset, and end it only for latched terminal errors. Add an offline adapter regression that yields a refresh error, makes a sufficient contract available, and verifies that another poll retries rather than immediately returning EOF.

[thepastaclaw]

Re-review — Final validation — Phase 1 + Phase 2

At fbee8b5, the complete review range and all 60 prior findings were revalidated: 59 are fixed, and the owned-clause cloning refactor remains intentionally deferred. One new, non-blocking allocation issue is confirmed in action-only CREATE matching; no remaining blocking findings were confirmed. Verification was static only: the supplied head-specific CI snapshot shows Rust workspace tests skipped and NPM release validation and PR Hygiene pending, and the live-network/devnet subscription path remains unvalidated.

🟡 1 suggestion(s)

Review provenance

Source: reviewer 1: muse-spark-1.3-contributor (agent: phase1-reviewer, role: general); reviewer 2: muse-spark-1.3-contributor (agent: phase1-reviewer, role: rust-quality); reviewer 3: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 4: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 5: gpt-6.1-sol (agent: phase2-reviewer, role: platform-versioning); reviewer 6: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 7: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); reviewer 8: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 9: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 10: gpt-6.1-sol (agent: phase2-reviewer, role: platform-versioning); reviewer 11: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 12: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)

• Triage: critical by gpt-6.1-sol (effort low) — This intricate cross-cutting change modifies peer-facing network deserialization through StateTransition::deserialize_from_bytes_untrusted_in_version_with_max_value_depth in packages/rs-dpp/src/state_transition/mod.rs, alongside the new subscription stream and its client-side decoding.
• Phase 1 reviewers: muse-spark-1.3-contributor — general (completed, effort xhigh); agent phase1-reviewer, muse-spark-1.3-contributor — rust-quality (completed, effort xhigh); agent phase1-reviewer
• Phase 1 model: muse-spark-1.3-contributor — not quota-gated; passed over gemini-3.8-flash-high (antigravity below 15% reserve: weekly 15% left, 5h 100% left), glm-5.3-flash (not used above high effort; tier asks max)
• Single stage: Phase 1 and Phase 2 reviewed this head side by side, with no blocker gate between them (triage tier)
• Fresh final gate: an independent Phase-2 review ran after iterative findings were reconciled
• Fresh verifier: gpt-6.1-sol — final-verifier; agent sol-verifier
• Phase 2 reviewers: gpt-6.1-sol — general (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — architecture-layering (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — platform-versioning (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — rust-quality (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — security-auditor (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — general (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — architecture-layering (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — platform-versioning (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — rust-quality (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — security-auditor (completed, effort xhigh); agent phase2-reviewer

🤖 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.

In `packages/rs-drive/src/query/filter.rs`:
- [SUGGESTION] packages/rs-drive/src/query/filter.rs:212-214: Skip stored-data materialization for action-only CREATE matches
  A document filter specifying a type and CREATE, but no document clauses, resolves to the Clauses branch and reaches this call for every matching document. If that document type declares generatedFrom properties, DocumentTypeBasicMethods::data_as_stored() deep-clones the entire data BTreeMap and runs property generation, although the empty clause set never reads any properties. The new subscription consumer repeats this work independently for each subscriber, so shared block decoding does not amortize these payload copies. Return Pass for an empty new_document_clauses set before materializing stored data, as the Replace branch already does. This local shortcut is separate from the intentionally deferred refactor of owned clause alternatives.

[thepastaclaw]

Re-review — Final validation — Phase 1 + Phase 2

Static verification of the complete diff at ebdfda3 confirms one non-blocking correctness issue: contract following can change the clause grammar and terminate an otherwise valid subscription. All 61 prior findings were independently revalidated: 60 are fixed, and the bounded clause-cloning refactor remains intentionally deferred. No builds or tests were run; the supplied head-specific CI snapshot shows Rust workspace tests skipped, NPM release validation and PR Hygiene pending, and no reported devnet end-to-end run.

🟡 1 suggestion(s)

Review provenance

Source: reviewer 1: muse-spark-1.3-contributor (agent: phase1-reviewer, role: general); reviewer 2: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 3: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 4: gpt-6.1-sol (agent: phase2-reviewer, role: platform-versioning); reviewer 5: muse-spark-1.3-contributor (agent: phase1-reviewer, role: rust-quality); reviewer 6: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 7: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); reviewer 8: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 9: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 10: gpt-6.1-sol (agent: phase2-reviewer, role: platform-versioning); reviewer 11: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 12: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)

• Triage: critical by gpt-6.1-sol (effort low) — This intricate, cross-layer streaming feature changes network-facing untrusted state-transition deserialization through StateTransition::deserialize_untagged_untrusted_exact in packages/rs-dpp/src/state_transition/serialization.rs, meeting the critical-surface criterion.
• Phase 1 reviewers: muse-spark-1.3-contributor — general (completed, effort xhigh); agent phase1-reviewer, muse-spark-1.3-contributor — rust-quality (completed, effort xhigh); agent phase1-reviewer
• Phase 1 model: muse-spark-1.3-contributor — not quota-gated; passed over gemini-3.8-flash-high (antigravity below 15% reserve: weekly 15% left, 5h 100% left), glm-5.3-flash (not used above high effort; tier asks max)
• Single stage: Phase 1 and Phase 2 reviewed this head side by side, with no blocker gate between them (triage tier)
• Fresh final gate: an independent Phase-2 review ran after iterative findings were reconciled
• Fresh verifier: gpt-6.1-sol — final-verifier; agent sol-verifier
• Phase 2 reviewers: gpt-6.1-sol — general (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — architecture-layering (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — platform-versioning (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — rust-quality (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — security-auditor (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — general (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — architecture-layering (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — platform-versioning (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — rust-quality (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — security-auditor (completed, effort xhigh); agent phase2-reviewer

🤖 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.

In `packages/dash-platform-queries/src/subscriptions/resolved.rs`:
- [SUGGESTION] packages/dash-platform-queries/src/subscriptions/resolved.rs:218-219: Keep the subscription clause grammar stable during contract following
  DAPI initially resolves subscription filters with PlatformVersion::latest(), but follow() passes the update block's protocol version into rebind_data_contract(). Re-resolution then uses that version in action_clauses() → InternalClauses::extract_from_clauses(). These select different grammars: latest uses where_clause_grouping v1 and accepts multiple non-primary-key IN clauses, whereas protocol 13 uses v0 and rejects them with MultipleInClauses. On a protocol-13 network, a CREATE filter containing `stars IN [1, 2]` and `votes IN [3, 4]` can therefore be accepted and match normally, then fail after a compatible contract update advances the binding from version 1 to version 2. The historical fallback does not apply to a newer contract, so DAPI terminates the stream with FAILED_PRECONDITION even though the contract and block are readable by this build. Preserve the grammar selected at initial resolution across rebinding, separately from the block-version rules used to decode contracts and evaluate stored document data. Alternatively, enforce the same grammar at initial admission and rebinding. Add a regression resolving a two-IN filter under latest and following a compatible newer contract under protocol 13; no shipped consensus query generation needs modification.

[thepastaclaw]

Re-review — Final validation — Phase 1 + Phase 2

Independent static verification of the complete PR range at 3f6dcbc found no remaining in-scope blockers or actionable suggestions: all 62 prior findings were revalidated, with 61 addressed and the bounded clause-cloning optimization intentionally deferred. The current implementation preserves admission-time filter rules, authenticates replacement contract bindings, bounds untrusted client decoding, and includes the previously requested replay and admission cleanup fixes without changing historical consensus decoding policy. No builds or tests were run; the supplied 2026-10-06T06:29:30Z CI snapshot shows Rust workspace tests skipped and NPM release validation and PR Hygiene pending, while live-network end-to-end validation remains unperformed.

🔴 0 blocking | 🟡 0 suggestion(s) | 💬 0 nitpick(s)

Review provenance

Source: reviewer 1: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 2: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 3: gpt-6.1-sol (agent: phase2-reviewer, role: platform-versioning); reviewer 4: muse-spark-1.3-contributor (agent: phase1-reviewer, role: general); reviewer 5: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 6: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); reviewer 7: muse-spark-1.3-contributor (agent: phase1-reviewer, role: rust-quality); reviewer 8: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 9: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 10: gpt-6.1-sol (agent: phase2-reviewer, role: platform-versioning); reviewer 11: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 12: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)

• Triage: critical by gpt-6.1-sol (effort low) — The change is large and intricate and adds network-facing deserialization in packages/dash-platform-queries/src/subscriptions/proto.rs (StateTransitionFilter::from_proto), alongside new streaming request validation and response handling.
• Phase 1 reviewers: muse-spark-1.3-contributor — general (completed, effort xhigh); agent phase1-reviewer, muse-spark-1.3-contributor — rust-quality (completed, effort xhigh); agent phase1-reviewer
• Phase 1 model: muse-spark-1.3-contributor — not quota-gated; passed over gemini-3.8-flash-high (antigravity below 15% reserve: weekly 15% left, 5h 100% left), glm-5.3-flash (not used above high effort; tier asks max)
• Single stage: Phase 1 and Phase 2 reviewed this head side by side, with no blocker gate between them (triage tier)
• Fresh final gate: an independent Phase-2 review ran after iterative findings were reconciled
• Fresh verifier: gpt-6.1-sol — final-verifier; agent sol-verifier
• Phase 2 reviewers: gpt-6.1-sol — general (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — architecture-layering (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — platform-versioning (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — rust-quality (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — security-auditor (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — general (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — architecture-layering (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — platform-versioning (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — rust-quality (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — security-auditor (completed, effort xhigh); agent phase2-reviewer

🤖 Prompt for all review comments with AI agents

These findings are from an automated code review. Verify the current code and confirm that no unresolved issues remain.

No unresolved findings remain from the prior review on this head.

[thepastaclaw]

Re-review — Final validation — Phase 1 + Phase 2

Verified the supplied claims against head 013e557: nine prior findings remain applicable, and ended slow-reader streams introduce a separate blocking retained-memory issue. Two additional, lower-priority suggestions were omitted under the 10-finding comment budget. This was static verification only; no builds or tests were run, and the supplied head-matched CI snapshot shows Rust workspace tests skipped, NPM validation in progress, and policy reconciliation queued.

🔴 2 blocking | 🟡 8 suggestion(s)

9 carried-forward finding(s) already raised on this PR; not re-posting as new inline comments.

Review provenance

Source: reviewer 1: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 2: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 3: gpt-6.1-sol (agent: phase2-reviewer, role: platform-versioning); reviewer 4: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 5: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); reviewer 6: muse-spark-1.3-contributor (agent: phase1-reviewer, role: general); reviewer 7: muse-spark-1.3-contributor (agent: phase1-reviewer, role: rust-quality); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)

• Triage: critical by gpt-6.1-sol (effort low) — The change is large and intricate and directly adds peer-facing network deserialization in packages/rs-sdk/src/platform/subscriptions.rs::decode, including protocol-version-dependent decoding and depth limits for untrusted streamed state transitions.
• Phase 1 reviewers: muse-spark-1.3-contributor — general (completed, effort xhigh); agent phase1-reviewer, muse-spark-1.3-contributor — rust-quality (completed, effort xhigh); agent phase1-reviewer
• Phase 1 model: muse-spark-1.3-contributor — not quota-gated; passed over gemini-3.8-flash-high (antigravity below 15% reserve: weekly 14% left, 5h 100% left), glm-5.3-flash (not used above high effort; tier asks max)
• Single stage: Phase 1 and Phase 2 reviewed this head side by side, with no blocker gate between them (triage tier)
• Fresh verifier: gpt-6.1-sol — final-verifier; agent sol-verifier
• Phase 2 reviewers: gpt-6.1-sol — general (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — architecture-layering (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — platform-versioning (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — rust-quality (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — security-auditor (completed, effort xhigh); agent phase2-reviewer

🤖 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.

In `packages/rs-dapi/src/services/platform_service/subscribe_to_state_transitions/stream.rs`:
- [BLOCKING] packages/rs-dapi/src/services/platform_service/subscribe_to_state_transitions/stream.rs:305-307: Budget buffered responses until the response stream is released
  Releasing Admission stops accounting for the subscription but does not release its queued payloads: SubscriptionStream retains its ReceiverStream and drains all buffered messages before reporting the terminal status. A client on a directly exposed gRPC listener can fill its permitted streams from transaction-heavy history, stop granting HTTP/2 receive capacity, wait for the 60-second send deadline, and open another batch while keeping the old streams connected. Each ended stream can still retain queued transaction payloads, so successive batches bypass both admission limits and accumulate memory. The configured Tower timeout bounds obtaining the response, not consumption of its streaming body; the gateway's lifetime limit does not protect direct connections. Release scan capacity promptly, but retain separate accounting for response-buffer memory until the body is dropped, or make slow-reader cancellation reclaim those buffers. Extend the regression to keep multiple timed-out streams undrained while opening replacements and assert a shared retained-memory bound.
- [SUGGESTION] packages/rs-dapi/src/services/platform_service/subscribe_to_state_transitions/stream.rs:391-406: Defer transaction payload copies until responses are sent
  (existing thread: https://github.com/dashpay/platform/pull/5285#discussion_r4187638052)
  The collect() deep-copies every matching transaction into an owned response before the first send. While a send waits on a full channel, the iterator retains all remaining payload copies outside STREAM_BUFFER and independently for every matching subscriber. Dropping the decoded block correctly repairs its lifetime, but does not preserve the earlier per-send materialization fix. Keep lightweight match descriptors with shared Arc<Vec<u8>> payload references, release the decoded block, and construct each payload-owning protobuf response immediately before sending it. A stalled-reader regression with more than STREAM_BUFFER matches in one block should check that unsent payloads remain shared.

In `packages/rs-drive/src/query/filter.rs`:
- [BLOCKING] packages/rs-drive/src/query/filter.rs:811-814: Prevent malformed event values from panicking the SDK matcher
  (existing thread: https://github.com/dashpay/platform/pull/5285#discussion_r4187240683)
  Removing SDK re-matching eliminates the fabricated-event route, but the new unauthenticated request path still exposes this panic. A CREATE equality operand on an integer field containing Text("a".repeat(19) + "é") passes wire decoding and grouping, then reaches this integer codec during filter resolution. Value::to_integer() formats its wrong-type error through Display, whose current implementation calls text.split_at(20); byte 20 falls inside the final UTF-8 character. The request therefore panics instead of returning INVALID_ARGUMENT, and .ok() cannot intercept it. The formatter repair described in the earlier reply is absent from this head; splitting it into #5434 does not remove this endpoint's dependency on safe formatting. Include that repair before exposing the endpoint, or reject incompatible operand variants without invoking the formatter, and retain a malformed wire-operand regression through subscription resolution.
- [SUGGESTION] packages/rs-drive/src/query/filter.rs:811-814: Avoid index-key heap allocations for numeric event values
  (existing thread: https://github.com/dashpay/platform/pull/5285#discussion_r4191509703)
  Integer event values again pass through the storage-key encoder and decoder, including values already in the property's canonical variant. For example, the U8 and U16 encoders allocate a Vec, and their decoders copy it to reverse the key transformation. evaluate_clauses() repeats this for every evaluated numeric predicate and relevant subscription, so shared block decoding does not amortize the allocations. The direct conversions described in your reply are absent. Restore canonical-integer fast paths and direct to_integer() conversions for other accepted widths, preserving null and range behavior without modifying shipped codecs.
- [SUGGESTION] packages/rs-drive/src/query/filter.rs:790-791: Borrow already-canonical text during event matching
  (existing thread: https://github.com/dashpay/platform/pull/5285#discussion_r4189770472)
  canonical_property_value() returns an owned Value and clones the complete String even when evaluating already-canonical transition data. evaluate_clauses() only compares and immediately discards that temporary, so each relevant subscription copies long unindexed text independently. Prebuilding clause alternatives does not remove these event-value copies. The Cow-based implementation described in your reply is absent from this head. Return a borrowed/owned representation or provide a borrowed evaluation path, taking ownership only when retaining normalized operands.
- [SUGGESTION] packages/rs-drive/src/query/filter.rs:794-795: Borrow byte-array values that are already canonical
  (existing thread: https://github.com/dashpay/platform/pull/5285#discussion_r4190643841)
  The direct byte conversion preserves empty byte arrays and removes the former codec round trip, but to_binary_bytes() still clones an existing Value::Bytes buffer. Matching performs that payload-sized allocation for each evaluated byte-array predicate and relevant subscription, then immediately drops it after comparison. The borrowed fast path described in your reply is no longer present. Borrow already-canonical Bytes during evaluation, including empty arrays, while retaining owned conversion for alternate representations and operand preparation.

In `packages/rs-dapi/src/services/platform_service/subscribe_to_state_transitions/block_source.rs`:
- [SUGGESTION] packages/rs-dapi/src/services/platform_service/subscribe_to_state_transitions/block_source.rs:103-107: Include decoded allocations in the block-cache memory budget
  (existing thread: https://github.com/dashpay/platform/pull/5285#discussion_r4187638064)
  The current weight charges four serialized lengths plus fixed overhead, but cached transactions retain decoded StateTransition allocations as well. Compact boolean arrays encode each element as a small variant discriminant and boolean, while decoding allocates a full Value slot per element; Value must accommodate variants such as Bytes36 and u128. Several valid arrays therefore retain substantially more memory than this charge permits. Blocks enter the cache before subscriber matching, so even a nonmatching historical subscription can populate amplified entries. The conservative Value-based accounting described in your earlier reply is no longer present, and the current two-MiB retention fixture uses arbitrary transaction bytes rather than successfully decoded compact collections. Account for decoded collection capacities and nested allocations, or use a demonstrably conservative bound, with a valid compact-array regression.

In `packages/dash-platform-queries/src/subscriptions/resolved.rs`:
- [SUGGESTION] packages/dash-platform-queries/src/subscriptions/resolved.rs:327-332: Canonicalize numeric bounds before grouping range clauses
  (existing thread: https://github.com/dashpay/platform/pull/5285#discussion_r4187240689)
  drive_filter() groups raw operands before canonicalize_clause_values() runs. Grouping paired bounds calls WhereClause::less_than(), which accepts matching Value variants only, so stars >= I64(1) together with stars <= U64(5) on a U8 field fails with RangeClausesNotGroupable. Both operands individually normalize to U8, and the equivalent BETWEEN clause succeeds. Wire decoding preserves the signed/unsigned distinction, so this affects both native and wire callers. The pre-grouping normalization described in your reply is absent from this head. Normalize raw clauses against their schema before extraction, preserving the schema-field and identifier-length guards and leaving shipped grouping generations unchanged; restore native/wire mixed-width coverage.

In `packages/dash-platform-queries/src/subscriptions/proto.rs`:
- [SUGGESTION] packages/dash-platform-queries/src/subscriptions/proto.rs:107-128: Preserve normalized operand semantics when encoding subscriptions
  (existing thread: https://github.com/dashpay/platform/pull/5285#discussion_r4187638043)
  narrowed() repairs integers representable in 64 bits, but still changes operands accepted by native resolution into wire values that DAPI rejects. U128(1 << 64) on an F64 field normalizes natively through to_float(), yet is encoded as decimal Text and rejected after decoding. Likewise, byte-array equality with Array([U8(1)]) resolves natively, but its wire round trip produces Array([U64(1)]), which to_binary_bytes() rejects because array elements must be U8. The canonical_operands API, documented composition, and SDK preparation described in your latest reply are all absent; the SDK sends its original filters directly. Restore a schema-aware preparation path, or explicitly validate and reject representations without equivalent wire semantics, with native/wire regressions for both examples.

In `packages/rs-sdk/src/platform/subscriptions.rs`:
- [SUGGESTION] packages/rs-sdk/src/platform/subscriptions.rs:372-374: Evict a failing stream's node before reconnecting
  (existing thread: https://github.com/dashpay/platform/pull/5285#discussion_r4188462004)
  The established-stream failure path moves off the serving address only for FAILED_PRECONDITION. Streamed UNAVAILABLE, INTERNAL, and UNKNOWN errors are retried without banning or evicting that address. These occur after execute() has successfully returned the stream, so its ordinary address-failure handling cannot apply. With active_set_size(1), a node that repeatedly accepts subscriptions and then reports a local Tenderdash read failure remains selected until its 5–7.5-minute rotation expires, even with a healthy standby. The broader handling described in your earlier reply has narrowed in this head. Evict the address from rotation for appropriate post-open server failures; AddressList::evict_from_rotation() supports this without a health ban or treating an offline client's idle watchdog as misconduct. Cover actual selection between a failing active address and a healthy standby.

[thepastaclaw]

Re-review — Final validation — Phase 1 + Phase 2

Verified the combined reviewer claims and all 63 prior findings against the exact head. Four prior issues remain, including an admission-accounting bypass through failure-ended response buffers; two additional suggestions concern startup cursor readiness and public API migration documentation. This was a static review: no builds or tests were run, and the supplied CI snapshot reports Rust workspace tests skipped, with NPM validation and several policy/generation checks unfinished.

🔴 1 blocking | 🟡 5 suggestion(s)

4 carried-forward finding(s) already raised on this PR; not re-posting as new inline comments.

Review provenance

Source: reviewer 1: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 2: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 3: gpt-6.1-sol (agent: phase2-reviewer, role: platform-versioning); reviewer 4: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 5: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); reviewer 6: gpt-6-astra (agent: phase2-reviewer, role: architecture-layering); reviewer 7: muse-spark-1.3-contributor (agent: phase1-reviewer, role: general); reviewer 8: gpt-6-astra (agent: phase2-reviewer, role: platform-versioning); reviewer 9: gpt-6-astra (agent: phase2-reviewer, role: rust-quality); reviewer 10: gpt-6-astra (agent: phase2-reviewer, role: security-auditor); reviewer 11: muse-spark-1.3-contributor (agent: phase1-reviewer, role: rust-quality); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)

• Triage: critical by gpt-6.1-sol (effort low) — The change is intricate and adds peer-facing network deserialization in packages/dash-platform-queries/src/subscriptions/proto.rs (StateTransitionFilter::from_proto) alongside a new streaming RPC with concurrent scanning and resource controls.
• Phase 1 reviewers: muse-spark-1.3-contributor — general (completed, effort xhigh); agent phase1-reviewer, muse-spark-1.3-contributor — rust-quality (completed, effort xhigh); agent phase1-reviewer
• Phase 1 model: muse-spark-1.3-contributor — not quota-gated; passed over gemini-3.8-flash-high (antigravity below 15% reserve: weekly 14% left, 5h 100% left), glm-5.3-flash (not used above high effort; tier asks max)
• Single stage: Phase 1 and Phase 2 reviewed this head side by side, with no blocker gate between them (triage tier)
• Fresh verifier: gpt-6.1-sol — final-verifier; agent sol-verifier
• Phase 2 reviewers: gpt-6.1-sol — general (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — architecture-layering (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — platform-versioning (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — rust-quality (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — security-auditor (completed, effort xhigh); agent phase2-reviewer, gpt-6-astra — architecture-layering (completed, effort xhigh); agent phase2-reviewer, gpt-6-astra — platform-versioning (completed, effort xhigh); agent phase2-reviewer, gpt-6-astra — rust-quality (completed, effort xhigh); agent phase2-reviewer, gpt-6-astra — security-auditor (completed, effort xhigh); agent phase2-reviewer
• Model comparison: every Phase-2 reviewer also ran on gpt-6-astra; the verifier weighed both sets without knowing which model wrote which

🤖 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.

In `packages/rs-dapi/src/services/platform_service/subscribe_to_state_transitions/stream.rs`:
- [SUGGESTION] packages/rs-dapi/src/services/platform_service/subscribe_to_state_transitions/stream.rs:242-250: Use the readable tip when initializing a live subscription
  When the tip watch is still zero, current_tip() uses latest_height() directly, although BlockSource documents that this height's execution results may not yet be saved. This bypasses publish_tip()'s readiness policy. At startup, if Tenderdash has stored H but only H - 1 is readable, this path starts a live subscription at H + 1 and sends an opening checkpoint at H; the tracker would instead publish H - 1 and include H once its results become available. The stream never revisits H. Derive the fallback from the same results-readiness policy, or wait for/refuse startup until the tracker establishes a readable boundary. Add a startup regression with the watch at zero and the latest height's results withheld.
- [BLOCKING] packages/rs-dapi/src/services/platform_service/subscribe_to_state_transitions/stream.rs:315-318: Budget buffered responses until the response stream is released
  (existing thread: https://github.com/dashpay/platform/pull/5285#discussion_r4279564139)
  The weak-buffer cleanup reclaims queued messages for Stop::SlowClient, but Stop::Fail explicitly preserves them after Admission has been released. For example, the opening checkpoint, 30 matching transactions, and their checkpoint fit the 32-message channel; if the next height fails reading or decoding, run() exits without encountering a blocked send. An undrained response body then owns the queued messages indefinitely, with no remaining task enforcing SEND_DEADLINE, while replacement subscriptions are admitted. Repeated failures can accumulate queue/hash/match-index allocations and pin serialized transaction buffers beyond cache eviction, outside both subscription admission limits. Preserve separate response-buffer accounting until the body drains or is dropped, or provide bounded reclamation for failure-ended buffers while allowing healthy readers to drain them. Extend the replacement-admission regression with a matching block followed by a failing height, keeping the failed streams undrained.
- [SUGGESTION] packages/rs-dapi/src/services/platform_service/subscribe_to_state_transitions/stream.rs:390-395: Preserve slow-reader cleanup while waiting for replay capacity
  (existing thread: https://github.com/dashpay/platform/pull/5285#discussion_r4189391800)
  Removing the replay-specific semaphore did not remove capacity waits: blocks.read() can queue on BlockSource's eight-slot fetch semaphore or wait behind another single-flight loader, and meta_page() has the same behavior. These awaits do not inspect the response buffer or apply SEND_DEADLINE. If a preceding block's checkpoint fills the channel exactly, an undrained client can retain its full buffer and Admission beyond 60 seconds while its next read waits behind other subscriptions; bounding individual Tenderdash calls does not bound semaphore queueing time. Keep subscription-level backpressure enforcement active during backend waits without imposing a client-specific timeout on the shared BlockSource or repeatedly cancelling and rejoining the acquisition queue. Add paused-time coverage that fills the response channel, withholds fetch capacity, and verifies buffer and admission reclamation before a read completes.

In `packages/rs-drive/src/query/filter.rs`:
- [SUGGESTION] packages/rs-drive/src/query/filter.rs:70-72: Document the public Drive filter type-parameter migration
  The exported type changes from DriveDocumentQueryFilter<'a> to DriveDocumentQueryFilter<C>, which is a Rust source-compatibility break independent of the documented tonic server-trait additions. Downstream signatures, struct fields, and aliases explicitly naming DriveDocumentQueryFilter<'a> must migrate to DriveDocumentQueryFilter<&'a DataContract>, or select an owned/shared contract parameter. The PR explains the generic ownership design but its Breaking Changes section does not identify this required source migration. Add it alongside the server-trait migration. This is public crate API documentation; it does not require a PlatformVersion or GroveVersion change.

In `packages/rs-dapi/src/services/platform_service/subscribe_to_state_transitions/block_source.rs`:
- [SUGGESTION] packages/rs-dapi/src/services/platform_service/subscribe_to_state_transitions/block_source.rs:110-115: Include decoded allocations in the block-cache memory budget
  (existing thread: https://github.com/dashpay/platform/pull/5285#discussion_r4187638064)
  The one-Value-per-byte calculation bounds initialized elements, not retained collection capacity. platform-value's untrusted decoder starts arrays at zero capacity and grows them with try_reserve(1), returning the vector without shrinking it. An array of 8,193 Null values encodes in approximately 8 KiB but ordinarily retains 16,384 Value slots: 768 KiB at the documented 48-byte Value size, versus approximately 392 KiB charged for those encoded bytes. Small nested arrays also retain minimum-capacity slack. These shapes can occur in successful contract transitions: DataContractInSerializationFormat retains raw document schemas, and scalar-property examples annotations allow arbitrary array items. BlockSource caches successful transitions before filtering, so the nominal 512 MiB weight limit is not a conservative retained-memory bound. Account for decoded collection capacities and nested allocations, or establish a conservative bound covering their growth behavior. Add a null-array regression just beyond a capacity boundary; the boolean test's len-based assertion does not cover this case.

In `packages/dash-platform-queries/src/subscriptions/proto.rs`:
- [SUGGESTION] packages/dash-platform-queries/src/subscriptions/proto.rs:130-137: Preserve normalized operand semantics when encoding subscriptions
  (existing thread: https://github.com/dashpay/platform/pull/5285#discussion_r4187638043)
  Identifier byte-list operands still change their accepted representation across the wire. A typed DELETE filter with original_document_where containing `$id EQUAL Value::Array(vec![Value::U8(1); 32])` resolves natively: Identifier canonicalization delegates to to_identifier_bytes(), which accepts U8 arrays. narrowed() preserves that array, value_to_proto emits its elements as uint64_value, and value_from_proto reconstructs U64 elements. The same Identifier canonicalizer then rejects the encoded filter because to_identifier_bytes() accepts only U8 array elements. Schema Identifier and IdentifierWithReference properties have the same discrepancy. The current byte-list fix applies only to ByteArray properties. Extend checked byte-list normalization to identifier operands in the shared Drive canonicalizer, preserving the 32-byte check, or encode these operands canonically before transmission. Add native/wire resolution regressions for both `$id` and an identifier property.

[thepastaclaw]

Re-review — Final validation — Phase 1 + Phase 2

Verified the complete change range at f84c5cd and revalidated all 65 prior findings; no blocking findings remain. Three suggestions remain: an accepted packed typed-array filter can miss matches, disconnected scans retain admission during queued backend reads, and non-transfer address classification lacks focused regression coverage. Validation was static only: the supplied head-matched CI snapshot shows Rust workspace tests skipped, with NPM release validation and PR Hygiene pending.

🟡 3 suggestion(s)

Review provenance

Source: reviewer 1: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 2: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 3: gpt-6.1-sol (agent: phase2-reviewer, role: platform-versioning); reviewer 4: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 5: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); reviewer 6: muse-spark-1.3-contributor (agent: phase1-reviewer, role: general); reviewer 7: muse-spark-1.3-contributor (agent: phase1-reviewer, role: rust-quality); reviewer 8: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 9: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 10: gpt-6.1-sol (agent: phase2-reviewer, role: platform-versioning); reviewer 11: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 12: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)

• Triage: critical by gpt-6.1-sol (effort low) — This intricate cross-layer feature changes peer-facing network deserialization in packages/dash-platform-queries/src/subscriptions/proto.rs through StateTransitionFilter::from_proto and adds SDK decoding and validation of streamed state transitions.
• Phase 1 reviewers: muse-spark-1.3-contributor — general (completed, effort xhigh); agent phase1-reviewer, muse-spark-1.3-contributor — rust-quality (completed, effort xhigh); agent phase1-reviewer
• Phase 1 model: muse-spark-1.3-contributor — not quota-gated; passed over gemini-3.8-flash-high (antigravity below 15% reserve: weekly 14% left, 5h 100% left), glm-5.3-flash (not used above high effort; tier asks max)
• Single stage: Phase 1 and Phase 2 reviewed this head side by side, with no blocker gate between them (triage tier)
• Fresh final gate: an independent Phase-2 review ran after iterative findings were reconciled
• Fresh verifier: gpt-6.1-sol — final-verifier; agent sol-verifier
• Phase 2 reviewers: gpt-6.1-sol — general (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — architecture-layering (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — platform-versioning (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — rust-quality (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — security-auditor (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — general (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — architecture-layering (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — platform-versioning (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — rust-quality (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — security-auditor (completed, effort xhigh); agent phase2-reviewer

🤖 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.

In `packages/rs-drive/src/query/filter.rs`:
- [SUGGESTION] packages/rs-drive/src/query/filter.rs:772-775: Normalize packed containsAny candidates for non-u8 elements
  This shortcut also handles ContainsAny after the typed-array branch maps it to In, but typed-array validation does not enforce scalar IN's U8-only restriction. A ContainsAny Bytes([1, 2]) clause on a U16 typed array is accepted: validate_element_values() expands the bytes into U8 candidates and verifies that each encodes as U16. Matching then canonicalizes the document's elements to U16 while leaving the operand packed; ContainsAny delegates to In.eval(), whose Bytes branch accepts only Value::U8. An array containing 1 therefore fails the admitted filter, and DAPI checkpoints past the omitted match. The base matcher encoded both sides through the element codec and handled this case. Expand and canonicalize packed candidates for non-U8 element types, or reject that representation explicitly during resolution. Add native and wire-resolution regressions using a U16 typed array.

In `packages/rs-dapi/src/services/platform_service/subscribe_to_state_transitions/stream.rs`:
- [SUGGESTION] packages/rs-dapi/src/services/platform_service/subscribe_to_state_transitions/stream.rs:365-368: Release disconnected scans while backend reads are queued
  The preceding is_closed() check does not observe a disconnect during this await, and scan_block() similarly awaits blocks.read() without observing sender.closed(). These reads can queue behind BlockSource's fetch semaphore, whose waiting time is outside the 30-second HTTP request timeout. Subscriptions replaying distinct uncached pages can therefore retain node-wide and per-source admission for multiple timeout waves after their response streams are dropped, and still perform backend reads for absent clients. This is separate from the intentionally deferred slow-reader deadline: a dropped receiver needs neither buffered delivery nor a new send deadline. Race channel closure against the source waits, or otherwise detach the per-client scan lifetime from shared loading. Add a delayed-source regression that drops queued streams and verifies admission is reclaimed without waiting for their reads to execute.

In `packages/dash-platform-queries/src/subscriptions/tests.rs`:
- [SUGGESTION] packages/dash-platform-queries/src/subscriptions/tests.rs:155-160: Cover the non-transfer address participant branches
  The focused address-role test constructs only AddressFundsTransfer, and the server scan tests use the same variant. The new classifier's optional change outputs, asset-lock surplus output, Unshield recipient, Shield sender-only behavior, and identity-creation fallback address consequently have no focused matching coverage. Exhaustive matching ensures every transition variant is listed, but does not establish that the correct addresses and roles are contributed. Add table-driven ResolvedFilters matching tests for these branches, including Some/None optional outputs and negative Sender/Recipient assertions. These tests can remain entirely offline.

[thepastaclaw]

Re-review — Final validation — Phase 1 + Phase 2

Verified the complete diff at b167c11 and reconciled all 68 prior findings against the current implementation: none remains blocking, while four non-blocking improvements remain intentionally deferred. Four new suggestions are confirmed concerning direct Drive filter preparation, configured banning policy, reconnect cancellation, and Tenderdash JSON test coverage. This was static verification only; the supplied head-matched CI snapshot skips Rust workspace tests and leaves NPM release validation and PR Hygiene pending.

🟡 4 suggestion(s)

Review provenance

Source: reviewer 1: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 2: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 3: gpt-6.1-sol (agent: phase2-reviewer, role: platform-versioning); reviewer 4: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 5: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); reviewer 6: gpt-6-astra (agent: phase2-reviewer, role: general); reviewer 7: gpt-6-astra (agent: phase2-reviewer, role: architecture-layering); reviewer 8: gpt-6-astra (agent: phase2-reviewer, role: platform-versioning); reviewer 9: gpt-6-astra (agent: phase2-reviewer, role: rust-quality); reviewer 10: gpt-6-astra (agent: phase2-reviewer, role: security-auditor); reviewer 11: muse-spark-1.3-contributor (agent: phase1-reviewer, role: general); reviewer 12: muse-spark-1.3-contributor (agent: phase1-reviewer, role: rust-quality); reviewer 13: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 14: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 15: gpt-6.1-sol (agent: phase2-reviewer, role: platform-versioning); reviewer 16: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 17: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)

• Triage: critical by gpt-6.1-sol (effort low) — This intricate cross-layer feature adds peer-facing network deserialization in packages/dash-platform-queries/src/subscriptions/proto.rs::StateTransitionFilter::from_proto and new streaming request processing with concurrent shared block reads.
• Phase 1 reviewers: muse-spark-1.3-contributor — general (completed, effort xhigh); agent phase1-reviewer, muse-spark-1.3-contributor — rust-quality (completed, effort xhigh); agent phase1-reviewer
• Phase 1 model: muse-spark-1.3-contributor — not quota-gated; passed over gemini-3.8-flash-high (antigravity below 15% reserve: weekly 14% left, 5h 100% left), glm-5.3-flash (not used above high effort; tier asks max)
• Single stage: Phase 1 and Phase 2 reviewed this head side by side, with no blocker gate between them (triage tier)
• Fresh final gate: an independent Phase-2 review ran after iterative findings were reconciled
• Fresh verifier: gpt-6.1-sol — final-verifier; agent sol-verifier
• Phase 2 reviewers: gpt-6.1-sol — general (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — architecture-layering (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — platform-versioning (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — rust-quality (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — security-auditor (completed, effort xhigh); agent phase2-reviewer, gpt-6-astra — general (completed, effort xhigh); agent phase2-reviewer, gpt-6-astra — architecture-layering (completed, effort xhigh); agent phase2-reviewer, gpt-6-astra — platform-versioning (completed, effort xhigh); agent phase2-reviewer, gpt-6-astra — rust-quality (completed, effort xhigh); agent phase2-reviewer, gpt-6-astra — security-auditor (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — general (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — architecture-layering (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — platform-versioning (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — rust-quality (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — security-auditor (completed, effort xhigh); agent phase2-reviewer
• Model comparison: every Phase-2 reviewer also ran on gpt-6-astra; the verifier weighed both sets without knowing which model wrote which

🤖 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.

In `packages/rs-sdk/src/platform/subscriptions.rs`:
- [SUGGESTION] packages/rs-sdk/src/platform/subscriptions.rs:427-428: Retain the pending stream-open future across cancellation
  The backoff timer survives cancellation, but the subsequent open future belongs only to this invocation of reopen(). Cancelling next() while this await is pending drops the RPC and its executor timeout/retry state. The following call finds both stream and retry unset and starts a new RPC. A caller multiplexing next() with a regularly winning select branch can therefore prevent a healthy replacement stream from ever opening, despite next() being documented as cancel-safe. Store the pending open future in subscription-owned state until it completes. Add an injectable-Opener regression that repeatedly cancels next() during a delayed open and verifies that the same attempt eventually completes.
- [SUGGESTION] packages/rs-sdk/src/platform/subscriptions.rs:438-440: Apply subscription failures through the client reputation policy
  This path unconditionally bans a node reporting FAILED_PRECONDITION, and the streamed-failure path calls the same unconditional ban helper. Neither checks the effective ban_failed_address setting. Consequently, setting Some(false) through SdkBuilder or SubscriptionConfig still allows a node that lacks the requested historical height to be banned from the shared AddressList, affecting unrelated fetches and broadcasts. rs-dapi-client already distinguishes banning from evict_from_rotation() when banning is disabled. Apply that policy using the effective merged settings for subscription-specific failures as well, without changing FAILED_PRECONDITION's global retry classification. Add opening and streamed-failure tests that verify failover without banning when the setting is false.

In `packages/rs-drive/src/query/filter.rs`:
- [SUGGESTION] packages/rs-drive/src/query/filter.rs:481-483: Make canonical operands an invariant of the Drive filter
  Matching now canonicalizes document values but relies on a separate caller operation to canonicalize operands. The public filter remains directly constructible, and validate() can accept operands that this matcher then compares incorrectly. For example, CONTAINS Bytes(id) on an identifier typed array passes element validation, but the document element becomes Identifier(id) and is compared with the unchanged Bytes operand, producing a false negative. The previous matcher encoded both sides and handled that case. ResolvedFilters and the modified matching test helper explicitly call canonicalize_clause_values(), so the subscription path works, but direct consumers using the existing construct/validate/match sequence regress. Although the new helper documents its required ordering, the owning API does not enforce it. Provide a fallible prepared-filter constructor/type, reject unprepared representations, or preserve symmetric normalization for directly constructed filters, and cover the direct public validation/matching path.

In `packages/rs-dapi/src/clients/tenderdash_client.rs`:
- [SUGGESTION] packages/rs-dapi/src/clients/tenderdash_client.rs:374-378: Exercise the new Tenderdash RPCs with raw JSON fixtures
  The subscription tests implement TenderdashBlocks and return constructed Rust response types, bypassing TenderdashClient::post() and the new Serde response definitions. They therefore do not exercise string-encoded header numbers, null transaction/result lists, or the blockchain metadata envelope. A decoding regression at this boundary can prevent tip publication or transaction-bearing block reads while all scan-loop tests remain green. Add offline representative Tenderdash JSON fixtures for block, block_results, and blockchain, deserialized through TenderdashResponse<T>, covering empty blocks and mixed successful/failed transaction results. This does not require a live devnet.

[thepastaclaw]

Re-review — Final validation — Phase 1 + Phase 2

The current implementation addresses or supersedes the prior correctness and security findings; all 72 prior identities were independently reconciled, with four performance/backpressure suggestions intentionally deferred. One non-blocking issue remains: readiness probes bypass the shared Tenderdash concurrency budget. This was a static review only; the supplied CI snapshot shows Rust workspace tests skipped, NPM release validation and PR Hygiene pending, and the PR reports no live-devnet end-to-end validation.

🟡 1 suggestion(s)

Review provenance

Source: reviewer 1: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 2: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 3: gpt-6.1-sol (agent: phase2-reviewer, role: platform-versioning); reviewer 4: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 5: muse-spark-1.3-contributor (agent: phase1-reviewer, role: general); reviewer 6: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); reviewer 7: muse-spark-1.3-contributor (agent: phase1-reviewer, role: rust-quality); reviewer 8: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 9: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 10: gpt-6.1-sol (agent: phase2-reviewer, role: platform-versioning); reviewer 11: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 12: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)

• Triage: critical by gpt-6.1-sol (effort low) — The change is large and intricate and adds peer-facing network deserialization in packages/dash-platform-queries/src/subscriptions/proto.rs, including StateTransitionFilter::from_proto and document-filter operand decoding.
• Phase 1 reviewers: muse-spark-1.3-contributor — general (completed, effort xhigh); agent phase1-reviewer, muse-spark-1.3-contributor — rust-quality (completed, effort xhigh); agent phase1-reviewer
• Phase 1 model: muse-spark-1.3-contributor — not quota-gated; passed over gemini-3.8-flash-high (antigravity below 15% reserve: weekly 14% left, 5h 100% left), glm-5.3-flash (not used above high effort; tier asks max)
• Single stage: Phase 1 and Phase 2 reviewed this head side by side, with no blocker gate between them (triage tier)
• Fresh final gate: an independent Phase-2 review ran after iterative findings were reconciled
• Fresh verifier: gpt-6.1-sol — final-verifier; agent sol-verifier
• Phase 2 reviewers: gpt-6.1-sol — general (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — architecture-layering (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — platform-versioning (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — rust-quality (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — security-auditor (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — general (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — architecture-layering (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — platform-versioning (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — rust-quality (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — security-auditor (completed, effort xhigh); agent phase2-reviewer

🤖 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.

In `packages/rs-dapi/src/services/platform_service/subscribe_to_state_transitions/block_source.rs`:
- [SUGGESTION] packages/rs-dapi/src/services/platform_service/subscribe_to_state_transitions/block_source.rs:151-158: Apply the shared Tenderdash budget to readiness probes
  latest_height() and results_saved() call Tenderdash directly without acquiring fetches. While the tip watch is zero, each admitted SubscriptionService::start() independently runs current_tip() → readable_tip(), so slow startup or unavailable results can produce up to 256 concurrent readiness probes outside the eight-fetch budget, in addition to the tracker and ordinary reads. Subscription admission bounds clients, not shared backend concurrency. The budgeted fetch() path also runs block() and block_results() concurrently under one permit, so it does not enforce the PR description's limit of eight actual Tenderdash calls. Centralize backend admission in BlockSource and route readiness probes and cached loads through it, using per-RPC accounting if eight is the intended call limit. Share concurrent readiness probes where practical. Add a regression that counts status, results, block, and metadata RPCs during simultaneous cold-start subscriptions and replay; the existing concurrency test counts only block() calls.

[thepastaclaw]

Re-review — Final validation — Phase 1 + Phase 2

Static verification of the complete diff at bbd00dc found no remaining actionable in-scope findings, and all 73 prior findings were independently reconciled against the current source. Prepared filters eliminate the previously deferred clause-set cloning, while the slow-reader deadline during backend queue waits remains intentionally deferred—not obsolete—with admission and buffer limits retained. No builds or tests were run; the supplied exact-head CI snapshot shows Rust workspace tests skipped and NPM release validation and PR Hygiene pending.

🔴 0 blocking | 🟡 0 suggestion(s) | 💬 0 nitpick(s)

Review provenance

Source: reviewer 1: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 2: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 3: gpt-6.1-sol (agent: phase2-reviewer, role: platform-versioning); reviewer 4: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 5: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); reviewer 6: muse-spark-1.3-contributor (agent: phase1-reviewer, role: general); reviewer 7: muse-spark-1.3-contributor (agent: phase1-reviewer, role: rust-quality); reviewer 8: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 9: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 10: gpt-6.1-sol (agent: phase2-reviewer, role: platform-versioning); reviewer 11: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 12: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)

• Triage: critical by gpt-6.1-sol (effort low) — This intricate cross-cutting change adds peer-facing network deserialization in packages/dash-platform-queries/src/subscriptions/proto.rs, including StateTransitionFilter::from_proto, and introduces a new remotely accessible streaming service.
• Phase 1 reviewers: muse-spark-1.3-contributor — general (completed, effort xhigh); agent phase1-reviewer, muse-spark-1.3-contributor — rust-quality (completed, effort xhigh); agent phase1-reviewer
• Phase 1 model: muse-spark-1.3-contributor — not quota-gated; passed over gemini-3.8-flash-high (antigravity below 15% reserve: weekly 14% left, 5h 100% left), glm-5.3-flash (not used above high effort; tier asks max)
• Single stage: Phase 1 and Phase 2 reviewed this head side by side, with no blocker gate between them (triage tier)
• Fresh final gate: an independent Phase-2 review ran after iterative findings were reconciled
• Fresh verifier: gpt-6.1-sol — final-verifier; agent sol-verifier
• Phase 2 reviewers: gpt-6.1-sol — general (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — architecture-layering (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — platform-versioning (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — rust-quality (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — security-auditor (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — general (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — architecture-layering (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — platform-versioning (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — rust-quality (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — security-auditor (completed, effort xhigh); agent phase2-reviewer

🤖 Prompt for all review comments with AI agents

These findings are from an automated code review. Verify the current code and confirm that no unresolved issues remain.

No unresolved findings remain from the prior review on this head.