Skip to content

feat(platform)!: token marketplace escrow trees and token market contract (PV15) - #5526

Open
QuantumExplorer wants to merge 119 commits into
v5.1-devfrom
claude/token-market-escrow-contract
Open

QuantumExplorer wants to merge 119 commits into
v5.1-devfrom
claude/token-market-escrow-contract

Conversation

@QuantumExplorer

@QuantumExplorer QuantumExplorer commented Oct 21, 2026 •

Copy link
Copy Markdown
Member

Basic explanation

What this does: Prepares Platform's state for the token marketplace. It adds the two places where escrow will live: one tree for the tokens holders list for sale, and one for the credits buyers offer. It also adds a new system data contract, the token market contract, whose two document types, ask (a listing) and bid (an offer), will hold the open orders. Platform's check that no tokens appear or vanish is taught to count the escrowed tokens. Nothing can place an order yet: the transitions that use all this come in the next pull requests of the stack.

Value: The order book becomes readable with the document queries and proofs every SDK already has, including the depth at each price as one proven sum, without any new query endpoint. Each side's depth sums what it holds in escrow, so the sums can never overflow.

Risks: Medium. From protocol version 15 the genesis state and the upgrade both gain two trees and a contract, so every node must build them identically: a test checks that a chain born at 15 and a chain upgraded from 14 build byte-identical [Tokens] and [PreFundedSpecializedBalances] subtrees. Before 15 nothing changes: the trees, the contract and the new supply calculator are selected only by the protocol version 15 tables. Fee numbers in tests that start at the latest version move, because genesis has one more contract.

Issue being fixed or feature implemented

The second PR of the token marketplace stack (PV15), on top of #5524 (tradeable tokens), #5688 (a batch's fee estimate no longer depends on its operations' order; this PR's base) and #5604 (generatedFrom sys.tokens.decimals, which this branch merges; review #5604 first, its commits show here until it merges). A listing must hold the tokens it offers and an offer the credits it offers, so a fill always delivers. This PR adds that escrow storage, the contract that records the open orders, and the supply check that keeps counting escrowed tokens.

What was done?

Escrow trees (DRIVE_VERSION_V10, PV15)

  • [Tokens, TOKEN_ORDER_ESCROW_KEY (96)]: a BigSumTree with one SumTree per token, created with the token's first listing, of SumItems keyed by order id.
  • [PreFundedSpecializedBalances, PREFUNDED_BALANCES_FOR_TOKEN_ORDERS (32)]: a SumTree of SumItems keyed by order id.
  • Both come from one helper, Drive::insert_token_order_escrow_trees, called last by create_initial_state_structure_v5 (genesis) and by transition_to_version_15 (upgrade). Each new key becomes a leaf of its Merk, so neither root moves.
  • Primitives the order transitions will use, all versioned under DriveTokenMethodVersions::order_escrow: add, set, remove and fetch for both escrows, with cost estimation. Each is an OptionalFeatureVersion: None in DRIVE_TOKEN_METHOD_VERSIONS_V1 (so in every table before 15), Some(0) in DRIVE_TOKEN_METHOD_VERSIONS_V3; called with an earlier protocol version, each returns DriveError::VersionNotActive instead of building operations for trees that do not exist there.
  • Cost estimation: one batch keeps one description per layer, so every escrow description keeps a deeper one another write of the batch registered (util::estimated_layers). The offers' escrow key, 32, sits on the third level of the prefunded specialized balances Merk (128 over 64 and 192), so the credit escrow describes that layer at level 2. The voting balance and contract fee pot estimations describe it at level 1, their keys' depth; from 15 (DRIVE_VERSION_V10) their new generations, estimated_cost_for_prefunded_specialized_balance_update 2 and add_estimation_costs_for_contract_fee_pot_update 1, run the previous generation and put back a deeper description it replaced, so the order of registration no longer matters. Likewise a listing's escrow key sits on level 3 of the Tokens Merk, below the balances key the token balance estimation describes at level 0: from 15 for_token_balances 1 keeps a deeper description of the Tokens layer. Both escrows also sit deeper in the root tree than the identity balances (level 1) and token balances (level 2) keys, so from 15 for_balances 1, for_token_balances 1 and the voting balance estimation 2 keep a deeper description of the root too: an offer's batch, which debits the buyer and credits the escrow, is estimated with the root at level 3 whichever operation is built first. The other writes of a batch (a document, a nonce) register their own descriptions; fix(drive)!: estimate a batch's fee the same whatever its operations' order (PV15) #5688, this PR's base, keeps the rule at the batch's boundary too: from 15 apply_drive_operations 2 merges each operation's descriptions into one covering both, so no other write lowers what the escrows describe, whatever the order. Before 15 they all run unchanged.

Token conservation (calculate_total_tokens_balance 2)

  • TotalTokensBalance gains total_token_order_escrow_balances (0 from v0 and v1), and ok() / total_balances() add it to the balance side.

Tokens moved from a balance into escrow, 1,000 minted and 400 listed:

Before (calculate_total_tokens_balance 1): supply 1000, balances 600, pools 0        -> not balanced, the block-end check halts
After  (calculate_total_tokens_balance 2): supply 1000, balances 600, escrow 400     -> balanced

The credit check needs no change: the prefunded specialized balances root is a SumTree and already counts every child.

Token market contract (new package packages/token-market-contract, SystemDataContract::TokenMarket = 11, ID AE6yYq65VdfGCg2MSyV9vV7cWGiFWjbwBuy7THesDh4e)

  • ask and bid: tokenId, price (credits per whole token), amount, remaining, decimals (0 to 255, the token's decimals when the order was placed, which every fill prices it at), and for bid the escrowed escrow. decimals is declared "generatedFrom": { "function": "sys.tokens.decimals", "params": ["tokenId"] } (feat(platform)!: generatedFrom sys.tokens.decimals generates a token's decimals (PV15) #5604): generated when the order is created and kept after. Orders carry no minimum fill: the token's trade mode sets it (feat(platform)!: token marketplace listings (PV15) #5529).
  • Indexes book (tokenId, decimals, price, $createdAt), depth (tokenId, decimals, price, summable + rangeSummable: remaining for asks, escrow for bids) and byOwner. Prices are per whole token at each order's decimals, so the book and the depth sums keep each scale apart: a query names the token and its decimals, and a sum never adds prices of two scales.
  • Config sizedIntegerTypes: false: every integer is i64, as a summed property must be, and a partial fill rewrites an order without changing its size.
  • creationRestrictionMode 2; the types stay mutable and deletable (as withdrawals do) so orders carry a revision and storage flags.
  • Registered at genesis from PV15 (create_genesis_state v1 behind >= TOKEN_MARKETPLACE_INITIAL_PROTOCOL_VERSION), stored without storage flags by transition_to_version_15, served by the Drive system contract cache and the trusted context provider from 15.
  • SYSTEM_DATA_CONTRACT_VERSIONS_V4 gains token_market: 1 (0 in V1 to V3).

Data trigger bindings 3 (DRIVE_ABCI_VALIDATION_VERSIONS_V11): v2 plus reject_data_trigger on Replace and Delete of ask and bid.

A user's document transition against an order, protocol version 15:

create ask -> refused, DocumentCreationNotAllowedError (creationRestrictionMode 2)
replace ask / delete bid -> refused, DataTriggerConditionError "Action is not allowed"

Test fixtures at the latest version: a genesis at 15 holds one more contract and two more trees, so four document fee baselines move (delete and replace -47_280, DPNS create +137_760, the two DPNS v3 rows at 15 +95_920; each comment says why) and the shielded pool characterization digests of nullifier_batch_guard that cover the state root are re-pinned (ten of them, re-pinned again when the orders gained decimals). The applied and estimated fees of the same tests still match (their +171,440 estimated processing credits come from apply_drive_operations 2, in #5688).

Wiring: workspace and Yarn workspace, Dockerfile stages, package filters, CI test lists, codecov, data-contracts/dpp/rs-sdk/rs-sdk-ffi/wasm-sdk/trusted context provider features, the GroveDB structure description (.since(15)) and grovedb-structure.json, the book (data-model/token-marketplace.md, architecture overview) and v15 note 14.

How Has This Been Tested?

  • cargo test -p token-market-contract -p data-contracts: the published ID, the schema shape, no contract below 15.
  • cargo test -p dpp --features all_features --lib -- token_market_tests total_tokens_balance: the contract loads with full validation at the latest version (mutable, deletable, no user creation, i64 integers, summed depth); the escrow term balances the check.
  • cargo test -p drive --lib -- structure order_escrow should_serve_token_market calculate_total_tokens (52 passed): add/set/remove/fetch of both escrows; supply stays balanced when tokens move into escrow at 15 and reads as unbalanced with the PV14 calculator; the cache serves the contract from 15; a new structure fixture reaches every escrow node.
  • cargo test -p drive --lib -- order_escrow estimated_layers fee_pot prefunded (63 passed):
    • every one of the ten escrow methods returns VersionNotActive at protocol version 14 and works at the latest;
    • characterization: for a listing's and an offer's first write and their partial-fill update, the estimated processing fee covers the applied one on the initial state (the offer's first write: 2,687,840 estimated against 415,660 applied, with the layer at level 1 or 2 alike, so the level itself is pinned by the next test);
    • the prefunded balances layer is described at level 2 after the escrow and either sibling in either order at the latest version, and at level 1 by protocol version 14's siblings;
    • the keep-the-deeper helper inserts unless a description at least as deep exists, and puts back a deeper one an estimation overwrote.
    • a token balance removal and an escrow addition built into one estimation, in either order, leave the Tokens layer at the escrow's level 3; with protocol version 14's balance estimation built last it is 0, as before;
    • an identity balance debit and a credit escrow addition built into one estimation, in either order, leave the root at level 3 and the prefunded layer at 2; with protocol version 14's balance estimation built last the root is 1, as before;
    • three listings at one price, two at decimals 8 and one at decimals 2: the depth sum at decimals 8 is 500 and at 2 is 50, and the proven per-price sum at 8 verifies against the client's path query with one bucket of 500;
    • the order bindings through data_trigger_bindings_list at the latest version: each replace and delete of an ask and a bid matches exactly one binding, and running it refuses with DataTriggerConditionError; protocol version 14 binds none of them.
  • cargo test -p drive-abci --lib -- batch::tests::token batch::tests::document::{deletion,replacement,dpns} --skip shielded (344 passed): the PV15 token balance estimation moves no pinned fee.
  • yarn workspace @dashevo/token-market-contract test: decimals is required, 0 and 255 pass, 256 is refused (run before minFill was dropped; the spec now has no minFill cases and has not been rerun).
  • After minFill was dropped and decimals declared generated: cargo test -p token-market-contract, cargo test -p drive --lib order_escrow, cargo test -p drive-abci --lib -- token_market create_genesis_state (19 + 8 passed).
  • cargo test -p drive-abci --lib -- create_genesis_state perform_events_on_first_block_of_protocol_change data_triggers batch::tests::document::{deletion,replacement,dpns,creation,index_only}:
    • the contract is in a genesis born at 15 and not at 14;
    • transition_to_version_15 inserts it, served by the cache afterwards;
    • genesis at 15 and an upgrade from 14 build byte-identical [Tokens] and [PreFundedSpecializedBalances] subtrees and root elements;
    • bindings 3 keep every v2 binding and reject the four order actions.
  • cargo test -p drive-abci --lib -- batch::tests::token state_transitions::data_contract_create state_transitions::data_contract_update (569 passed before the two re-pinned shielded pool digests; both pass after).
  • Trusted context provider: the contract is served from 15 and not at 14, and a contract preloaded under its id is refused before 15 in every build, with or without the contract's feature (should_refuse_a_preloaded_token_market_id_before_protocol_15_in_any_build).

In-place changes to shipped generations

The new behaviour comes through new generations: the voting balance, contract fee pot, token balance and identity balance estimations (2, 1, 1 and 1) are selected only by DRIVE_VERSION_V10 (protocol version 15), and the escrow fields of DRIVE_TOKEN_METHOD_VERSIONS_V1 are new and None (apply_drive_operations 2 and its in-place edits are #5688's). Code that earlier protocol versions run is edited in two places, each inert there, with a comment at the edit saying why:

  • calculate_total_tokens_balance 0 (protocol versions 1 to 13) and 1 (14): TotalTokensBalance gains total_token_order_escrow_balances, and both set it to 0, so the totals they report are unchanged.
  • TotalTokensBalance::ok() and total_balances() (dpp, unversioned), run at every protocol version by the block-end supply check (validate_token_aggregated_balance 0): they also refuse a negative escrow total and add it to the balance side. Below 15 the escrow total is always 0, so the refusal cannot fire and the sum is unchanged; only the overflow error's text differs, on an invariant that has already failed. The block-end check's comment now counts the escrow too.
  • From feat(platform)!: generatedFrom sys.tokens.decimals generates a token's decimals (PV15) #5604 (generatedFrom sys.tokens.decimals), which this branch merges: the Drive document create and replace transformers (selected at every protocol version) call generate_state_read_properties; validate_generated_from_properties_v0 (14) skips the value comparison for a function that reads state; validate_generated_from_declarations (parser generation 3, 14) checks an identifier param, indexOnly and contested indexes for a token function; verify_state_transition_was_executed_with_proof v0 copies a state-generated value from the proved document (adopt_state_generated_properties); the create transformer adds its contest fee to the generation fee instead of assigning it. Each is inert before protocol version 15 by construction: only apply_generated_from 1 (15) admits a sys.tokens function, meta-schema v3 and generation 0 refuse it, so no type parsed earlier declares one, the generation returns at once with a zero fee, and nothing is skipped or copied. feat(platform)!: generatedFrom sys.tokens.decimals generates a token's decimals (PV15) #5604's description lists them too.

Breaking Changes

Consensus, from protocol version 15: new state trees at genesis and on the upgrade, a new system data contract, a new token conservation calculator and new data trigger bindings. Earlier protocol versions are unchanged.

Checklist:

  • I have performed a self-review of my own code
  • My Rust, FFI, WASM, Swift or Kotlin changes follow the coding conventions
  • 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

PR Hygiene · 707af07

  • Bots — thepastaclaw requested changes — dismiss the review or push a fix, 2 threads unresolved — resolve them
  • Self-review — post /self-reviewed
  • Build failed
  • Approvals
    • files with no dedicated owner — you own it
    • github (.github/package-filters/js-packages-direct.yml, .github/package-filters/js-packages-no-workflows.yml, .github/package-filters/js-packages.yml and 6 more) — ktechmidas or shumkov
    • system-contracts — you own it
    • dpp — you own it
    • rs-drive-abci — you own it
    • rs-drive — you own it
    • rust-sdk-ffi (packages/rs-sdk-ffi/Cargo.toml) — lklimek or shumkov
    • rust-sdk (packages/rs-sdk/Cargo.toml) — lklimek or shumkov
    • js-wasm-sdk (packages/wasm-sdk/Cargo.toml) — shumkov

When every merge requirement is met, the PR Hygiene check passes. Reviewer limits do not block merging; other required GitHub checks and protections still apply.

QuantumExplorer and others added 30 commits October 8, 2026 16:17
…a keywords

From protocol version 14 the DPNS `domain` create is checked by the
contract's own keywords instead of most of its data trigger:

- `normalizedLabel` and `normalizedParentDomainName` are generated from
  `label` and `parentDomainName` (`generatedFrom` homographSafeASCII);
- `preorderSalt` reveals the writer's own preorder, made in an earlier
  block, and deletes it (`refersTo` with a sha256d `findBy`, `where`
  `$ownerId`, `minimumAgeBlocks: 1`, `consume`); the preorder's storage
  is refunded to the writer;
- the `recordsIdentityIsOwner` rule holds a new name's `records.identity`
  to its owner until the name first changes hands;
- `domain` sets `canBeDeleted: false`; `preorder` requires
  `$createdAtBlockHeight`. Positions, transient list and encoding are v2's.

`create_domain_data_trigger` 2 keeps only the parent domain checks, and
data trigger bindings 2 drop the `domain` Replace and Delete rejects.
`transition_to_version_14` re-stores DPNS. A preorder recording no
`$createdAtBlockHeight` (made before the upgrade) counts as old enough
for `minimumAgeBlocks`. The legacy JS SDK hashes the preorder as the
platform does: normalized label, ".", parent as sent.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A document recording no `$createdAtBlockHeight` is known only to come from
a block before the protocol upgrade that re-stored its system contract, so
it now meets `minimumAgeBlocks: 1` (the DPNS reveal's) and no larger
minimum. Another contract revealing DPNS preorders with a larger minimum
refuses a pre-upgrade preorder; a test covers it.

Also: a trigger-refused DPNS reveal keeps its preorder for a retry (test),
and the check_tx contract update fee pin follows the DPNS v3 preorder type
the test copies in.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…riscoll-44c981

# Conflicts:
#	packages/rs-platform-version/src/version/v14.rs
`User.DASH` hashes `user.DASH`, the parent as sent, while the domain
carries `parentDomainName: "DASH"` and `normalizedParentDomainName: "dash"`.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…riscoll-44c981

# Conflicts:
#	packages/rs-platform-version/src/version/v14.rs
Short `description`s and no `$comment`s in DPNS v3; the rules they
restated are in the schema keywords. The check_tx contract update fee pin
follows the smaller preorder type it copies in.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…owner

A copy of a pending preorder no longer blocks it, and the copier can no
longer reveal the name with the copy once the owner's reveal shows the salt:

- `preorder` is unique on (`$ownerId`, `saltedDomainHash`), replacing the
  unique index on the hash alone;
- the domain's `preorderSalt` finds it by `$ownerId` and the sha256d of the
  writer's id, the salt, the normalized label, "." and the parent.

The parser (meta-schema v3, parser generation 3) admits `"$ownerId"` as a
findBy function param (`LookupKeyParam::Writer`), a `$ownerId` findBy key on
a transferable type beside a function, and that key for `consume`.

At protocol version 14 every existing preorder is deleted and the type is
rebuilt as a fresh v3 chain lays it out
(`Drive::apply_contract_rebuilding_document_types`), so the minimumAgeBlocks
allowance for preorders recording no creation height is reverted.

rs-sdk and js-dash-sdk compute the preorder hash from the contract's own
declaration, so either side of the upgrade hashes as the platform does.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…riscoll-44c981

# Conflicts:
#	packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/mod.rs
#	packages/rs-platform-version/src/version/v14.rs
A registration took the preorder's hash form from the DPNS contract the
client already held: the context provider's for the SDK's protocol version
(13 on a fresh SDK), or js-dash-sdk's app cache. Either could still be DPNS
v2 after the network moved to v3, so the preorder committed to the old hash
and the domain create could never find it (40120), after the preorder was
paid.

rs-sdk now refreshes the protocol version (proven) and fetches DPNS from
state before hashing, fetching again when the response raised the version
the contract was parsed under. A preorderSalt reference without a findBy
function is an error instead of a silent fall back to the old hash.
js-dash-sdk fetches the contract with a new `skipCache` option, which also
replaces the cached contract both documents are created against.

Tests: the SDK hash against the real DPNS v2 and v3 contracts, a missing
hashed property, and a mock SDK seeded at 13 with a provider holding v2;
js specs for a cache warmed with v2 and for `skipCache`.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…arsed version

Comparing the SDK's protocol version before and after the fetch missed a
node behind the network: after the refresh learned protocol 14, a node at
the block before activation could still prove DPNS v2 at 13, the ratchet
ignores the lower version, and the registration would hash the old way.

The registration now reads each DPNS contract response with its metadata
and takes the first one proved at the version the SDK parsed it under, at
most 3 times. A response proved at a newer version was parsed under an
older one (decoding DPNS v3 under 13 drops the salt's reference) and has
raised the SDK's version, so the next is parsed under it; one proved at an
older version is skipped. Otherwise the registration fails before paying.

The document building moves into `dpns_registration_documents`, so an
offline test drives it against a context provider holding v2 and checks
the preorder's owner-bound hash and the domain's salt. Scripted tests cover
a response parsed under an older version (rebuilt from serialized bytes as
a proof decode does), a node behind the network, and every node behind.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
`apply_contract_rebuilding_document_types` stored the new contract
through `update_contract_add_operations`, which only builds operations,
so unlike an update it never put the rewritten contract in the block
cache. Until the protocol change events refresh every system contract at
their end, a read in the block could fall back to a global cache entry a
committed-state read had filled with the old contract. The rebuild now
fetches what it stored and seeds it with `insert_rewritten`, as
`update_contract` does. Test: DPNS v2 warm in the global cache, the
preorder type rebuilt, and a read in the block's transaction gets v3.

The book's same-block transfer case says the rule still applies.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…anwhile

The version a registration compared a DPNS contract response with was
read before the fetch awaited the network. SDK clones share that version,
so another request could raise it from 13 to 14 while the fetch waited; a
response from a node still at 13 proving DPNS v2 then matched the stale
13 and was taken, and the preorder hashed the old way.

A response is now taken only when the SDK's version is unchanged across
the fetch as well as equal to the version the node proved it at. The
version only rises, so one unchanged across the fetch is the version the
response was parsed under. Regression: the first fetch raises the shared
version to the latest while returning DPNS v2 proved at 13, which is
skipped, and the next response, v3, is taken.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…riscoll-44c981

# Conflicts:
#	packages/rs-platform-version/src/version/v14.rs
Now that `identifier` and `bytes` + `size` are in v5.0-dev (#5355), the
DPNS v3 schema uses them: `preorderSalt` and `saltedDomainHash` are
`"type": "bytes", "size": 32`, `records.identity` is `"type":
"identifier"`. The parsed types are unchanged; the stored contract keeps
the schema as written, so the check_tx contract-update fee pin moves
(27003089990 to 27003077990).

Protocol 13 cannot decode the shorthands, so the SDK test of a response
parsed under an older version now uses DPNS v2 as the contract read
under 13.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Clients:
- js-dash-sdk refuses a records.identity other than the registering
  identity before paying for the preorder when the contract declares
  recordsIdentityIsOwner; the legacy (pre-v3) hash of a top-level name is
  of the salt and the label as sent, as trigger v1 hashes it; the "no
  DPNS app" spec now reaches the code it names.
- rs-sdk `salted_domain_hash` reads the salt and label from the domain's
  properties instead of separate parameters.

Drive and drive-abci:
- the preorder rebuild refuses a type with trees outside its own subtree
  (indexOnly, contested index, moderator deletions, ttl), deletes without
  the backward-reference scan Drive never needs, and drops the
  always-None storage flags parameter.
- trigger v2 queries the parent under the top level directly (a parent
  domain name holds no dot) and gains in-module tests.

Parser and docs:
- the consume check moves into `writer_owns_found_document`, so the
  parse_reference_target match is no longer reflowed.
- `LOOKUP_KEY_WRITER` gives way to the existing `OWNER_ID`; the unused
  `preorder::properties::SALTED_DOMAIN_HASH` and `hash_double` import go.
- meta-schema v3, reference_lookup.rs, wasm-dpp2 and the book describe
  the findBy `$ownerId` entry (40120 for another identity's commitment),
  the per-owner preorder index and the legacy top-level hash.

Tests:
- the strategy tests hash preorders with the shared
  `dpns_salted_domain_hash` (now outside cfg(test)) instead of the rule
  under test, and lose the stale "disable document triggers" comments;
  dead `dpns < 3` arms, the pasted height-1 comments and the derivable
  flag go; the protocol 13 test is renamed `should_...`; preorders are
  read by id; the award test uses `fast_forward_to_block`; the upgrade
  layout reference is a bare genesis state.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
`DocumentPropertyReferenceTarget::lookup_hash_key` returns the computed
key of the `findBy` function a target declares, the chain
`as_any_document_reference` -> `lookup` -> `hash_key` that rs-sdk's DPNS
preorder hash and document reference validation 0 each spelled out.

Reference validation 0 is a shipped generation edited in place: the
accessor returns exactly what the inline chain returned, so nothing
changes at any protocol version.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ties

The register spec mocks `crypto.randomBytes` to zeros for the whole
suite, so two `generateRandomIdentifier()` calls gave the same identity
and the refusal never fired. The spec now uses two fixed, different ids.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…s stay deletable

documents.md stated without exception that a findBy `"$ownerId"` source
needs a referring type that cannot be transferred or traded, for a
property's findBy and for creatorRefersTo; beside a function the lookup
is judged on the create alone and the source is allowed, as DPNS v3
declares. The deletion chapter's onlyWhenConsumed preorder example now
says DPNS v3's own preorder keeps `canBeDeleted: true`.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Three Drive tests used the latest DPNS fixture for writes or proofs at
protocol version 13. DPNS v3 is now written with the `bytes` and
`identifier` type shorthands, which the protocol 13 parser refuses
("unsupported property type: bytes"). A chain at 13 stores DPNS v2, which
every later version reads too, so these tests now load it: the two
contested-poll tests (`..._poll_started_before_protocol_version_14`) and
`verify_batch_document_proof_before_protocol_version_14_carries_no_balance`.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ion the response reports

A fresh SDK starts at its network's floor (protocol version 13 on
mainnet, testnet and regtest) and learns the network's version only
from a verified response. DPNS v3 is written with the `bytes` and
`identifier` type shorthands, which the protocol version 13 parser
refuses, so on a network at 14 the first proof holding DPNS failed
("unsupported property type: bytes") before the SDK could learn 14,
every time.

When parsing under the SDK's version fails on an unpinned SDK below the
newest version it knows, `parse_proof_at_the_reported_version` parses
under that newest version, which verifies the proof and its quorum
signature over a state id that signs the response's protocol version,
and, when the reported version is another known one, parses again under
it. A pinned SDK, or one at the newest version, reports the first
failure. The fetch traits require cloneable responses (all are) for the
retry.

Tests: a stand-in proof readable only from protocol version 14 parses on
an SDK seeded at 13 and fails on one pinned at 13.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Conflicts with #5313, which verifies a response again after fetching a
missing quorum key, in the same clone-and-retry spot as the protocol
version fallback:
- fetch.rs, fetch_many.rs: both sides require a cloneable response; the
  doc names both retries.
- sdk.rs: the Dapi arm takes #5313's `verify_fetching_quorum_key`, whose
  two verifications now go through `parse_proof_at_the_reported_version`
  (the version passed in, so both start from the same one). A failure of
  the quorum signature or its key is reported as it is under either
  version (`fails_after_reading_the_proof`): the signature is checked
  once the proof was read, so a missing key still reaches the fetch and
  no extra quorum lookup is made.

Test: `should_report_a_missing_quorum_key_found_under_the_newest_version`.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… verification

`parse_proof_at_the_reported_version` borrows the request and response
and copies them once per parse, so `verify_fetching_quorum_key` keeps one
copy for both of its verifications instead of copying it again before
the first.

The probe is now scripted by its request (the first version that decodes
it, the version its response reports, the versions whose signature or
key fails) and records every version it is parsed under. New tests:
- a reported version between the SDK's and the newest is parsed again
  under it, and its failure there is reported;
- a signature failure under the newest version is reported over the
  first decoding error, and one under the SDK's version is not retried;
- through `verify_fetching_quorum_key`, a fresh SDK reads the proof and
  learns the reported version, and learns none when the signature fails.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…h the version fallback

The probe now asks the provider for its quorum's key instead of failing
outright, so a key the provider fetches makes the second verification
succeed. A new test drives `verify_fetching_quorum_key` with an SDK two
versions back and a provider holding no key: one fetch, the parse
sequence current, newest, current, newest, reported, and the SDK learns
the reported version. A regression of the second verification to a
direct parse at the SDK's version would fail it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…riscoll-44c981

# Conflicts:
#	book/src/contract-keywords/refers-to-lookup.md
#	book/src/data-model/documents.md
#	packages/rs-dpp/schema/meta_schemas/document/v3/document-meta.json
#	packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/action_validation/document/document_reference_validation/v0/mod.rs
#	packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/state/v0/fetch_documents.rs
…riscoll-44c981

# Conflicts:
#	book/src/contract-keywords/refers-to-lookup.md
#	book/src/data-model/documents.md
#	packages/rs-dpp/schema/meta_schemas/document/v3/document-meta.json
#	packages/rs-dpp/src/data_contract/document_type/property/reference_lookup.rs
…ut a salt

A DPNS v3 preorder now holds `ownerNameHash`, the sha256d of the owner's
id, the normalized label, "." and the normalized parent, unique per owner
(`ownerAndNameHash`), and `nameHash`, the same without the owner, indexed
with its creation height (`nameHashAndHeight`). It carries no salt and
lives an hour (`ttl` 3600, so it refunds nothing when consumed). The
domain finds it through `creatorRefersTo` (findBy `$ownerId` and the
`ownerNameHash` function, minimumAgeBlocks 1, consume). `preorderSalt`
stays declared on the domain, transient and no longer required, so
domains stored under v1 and v2 decode.

`nameHash` is there for a first-committer refersTo keyword built in a
separate PR: with it, a name whose older valid preorder belongs to
someone else is refused, which stops a watcher who reads a pending name
from the mempool from preordering and revealing it first under full
blocks. No salted design can check that before the owner reveals.

Without a salt an owner holds one pending preorder of a name: a second
is refused as a duplicate (40105) while the first stands.
`Sdk::register_dpns_name` reveals a pending one instead of preordering
again.

- Drive: `apply_contract_rebuilding_document_types` leaves a type
  already stored as declared (so the v14 upgrade stays idempotent), and
  rebuilds into a type with a ttl, which holds no document and so no
  expiry entry; a stored type with a ttl is still refused.
- rs-sdk and js-dash-sdk build the preorder from the domain's creator
  reference (`nameHash` from its params without `$ownerId`) and send no
  salt from v3; v1 and v2 keep the salted hash.
- Tests: dpp DPNS v3 shape and hash; drive rebuild (ttl new type, type
  left as stored, stored ttl refused); drive-abci dpns_v3 (one pending
  preorder per name, the contested retry reveals the same preorder,
  preorder and domain fees pinned: 62,554,803 credits a name at 14
  against 76,862,280 at 13); the check_tx update pin gains the 0.01 Dash
  non-unique index; rs-sdk and js-dash-sdk preorder shapes.
- Docs: v14 note 90, the book (commit and reveal, documents, deletion,
  transient).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…st, and reuses a pending preorder

- rs-sdk registers the DPNS contract it fetched and built the documents
  against with its context provider, so the preorder and domain
  confirmations decode under it. A known-contracts cache warmed with
  DPNS v2 would otherwise decode the v3 preorder's proof under v2's single
  property and refuse it, after the preorder was paid for.
- The v3 preorder property names are spelled in the module rather than
  imported from the bundled DPNS contract, which the SDK builds only with
  that contract's feature (`cargo check -p dash-sdk --no-default-features`
  builds).
- The pending preorder lookup is a shared query builder
  (`pending_preorder_query`).
- js-dash-sdk looks for a pending v3 preorder by its owner and
  `ownerNameHash` before preordering, reveals it instead, and propagates
  a failed query; v1 and v2 keep the fresh salt.

Tests: rs-sdk (the exact predicates, a pending preorder returned, none in
a proven absence, a failed query propagated, no query for a salted
preorder, the registration leaving a writable v2-seeded provider holding
v3); js-dash-sdk (the query, the reuse, a failed query, no query under
v2).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…its cache

`WasmTrustedContext` forwarded `get_data_contract` to its inner
`TrustedHttpContextProvider` but not `register_data_contract`, so the
trait's no-op ran. A DPNS registration that registers the v3 contract it
built against left a cache seeded with DPNS v2 (by `addKnownContract`, a
contract fetch or a restored store) in place, and the preorder's
confirmation decoded under v2 and failed after the preorder was paid for.
The call is now forwarded.

Test: a context seeded with DPNS v2 resolves DPNS to v3 once v3 is
registered.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Conflicts:
- drive contract migration modules: #5350's
  `rekey_unsigned_integer_index_values` beside this branch's
  `apply_contract_rebuilding_document_types`.
- v14 notes: #5350 took note 90 (unsigned integers keyed in value
  order), so the DPNS v3 note moves to 92, after the shorthands note 91;
  the upgrade's comment cites it as 92.

#5369 gave the document reference targets `where_conditions`: the
owner-keyed reveal test's expected `DeletableDocumentLookup` sets it
empty.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
#5378 wrote the book's examples with the type shorthands, among them
the salted DPNS preorder and its reveal in refers-to-lookup, documents
and deletion. This branch already writes them with the shorthands, for
the unsalted DPNS v3 (`ownerNameHash`, `nameHash`, `creatorRefersTo`),
so its versions are kept; documents.md no longer points at "the salt
above".

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
QuantumExplorer and others added 2 commits October 31, 2026 04:57
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
With the decision proposals and token market contracts both in the genesis
contracts tree, the token contract's node is billed 48_640 more bytes to
reach than at protocol 14 (burn 4_404_840, direct purchase 699_868_001_200),
and the commit-reveal fixture's contract is read for as many bytes as at 14.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@thepastaclaw thepastaclaw left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

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

At head 096a2cf, no blocking defect was verified; two generated-property correctness gaps and one historical-parser test-coverage gap remain as suggestions. All 36 prior findings were revalidated, including withdrawal of the proof-adoption security claim and the cross-genesis consensus claim. This was a static review only: current CI checks pass, including Kotlin, but the listed checks do not establish full Rust or JavaScript suite results.

🟡 3 suggestion(s)

1 finding(s) not shown inline (the lines are not part of this PR's diff)

🟡 Suggestion: Account for state-generated fields in document transition filters
packages/rs-drive/src/query/filter.rs:418-432

This helper delegates to DocumentTypeBasicMethods::data_as_stored, which only calls fill_generated_properties. That stateless path cannot populate sys.tokens.decimals, while client transition generation deliberately removes state-generated properties. On a PV15 document type declaring generated decimals, a create that omits decimals therefore returns Fail for a decimals == 6 predicate even when its resulting document stores 6; replacement new-document clauses fail before the API can request any document. Distinguish unresolved generated fields from definite mismatches by deferring those predicates to resolved action data or the resulting stored document, or reject unsupported predicates during filter validation. Do not reconstruct retained replacement values from the current token configuration, since the stored snapshot may differ. Add omitted-value create and replace regressions. No production caller of this filtering primitive was found at this revision, so the issue is nonblocking API correctness, not a demonstrated authorization or notification failure.

source: muse-spark-1.3-contributor (phase1-reviewer: general, rust-quality); gpt-6.1-sol (phase2-reviewer: general, architecture-layering, rust-quality)

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: muse-spark-1.3-contributor (agent: phase1-reviewer, role: general); reviewer 7: gpt-6.1-sol (agent: phase2-reviewer, role: tenderdash-consensus-security); reviewer 8: muse-spark-1.3-contributor (agent: phase1-reviewer, role: rust-quality); reviewer 9: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 10: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 11: gpt-6.1-sol (agent: phase2-reviewer, role: platform-versioning); reviewer 12: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 13: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); reviewer 14: gpt-6.1-sol (agent: phase2-reviewer, role: tenderdash-consensus-security); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)

  • Triage: critical by gpt-6.1-sol (effort low) — The large, intricate diff changes consensus-persisted state through transition_to_version_15 and create_initial_state_structure_v5, adds escrow storage primitives, and changes token supply accounting in calculate_total_tokens_balance/v2.
  • 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 quota lookup failed), 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 — tenderdash-consensus-security (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, gpt-6.1-sol — tenderdash-consensus-security (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:418-432: Account for state-generated fields in document transition filters
  This helper delegates to DocumentTypeBasicMethods::data_as_stored, which only calls fill_generated_properties. That stateless path cannot populate sys.tokens.decimals, while client transition generation deliberately removes state-generated properties. On a PV15 document type declaring generated decimals, a create that omits decimals therefore returns Fail for a decimals == 6 predicate even when its resulting document stores 6; replacement new-document clauses fail before the API can request any document. Distinguish unresolved generated fields from definite mismatches by deferring those predicates to resolved action data or the resulting stored document, or reject unsupported predicates during filter validation. Do not reconstruct retained replacement values from the current token configuration, since the stored snapshot may differ. Add omitted-value create and replace regressions. No production caller of this filtering primitive was found at this revision, so the issue is nonblocking API correctness, not a demonstrated authorization or notification failure.

In `packages/rs-platform-version/src/version/dpp_versions/dpp_contract_versions/v7.rs`:
- [SUGGESTION] packages/rs-platform-version/src/version/dpp_versions/dpp_contract_versions/v7.rs:45: Preserve protocol-14 coverage when selecting the new generatedFrom parser
  Selecting apply_generated_from generation 1 redirects the existing string-transformation registration tests because parse and expect_refused in try_from_schema/v3/generated_from_tests.rs still use PlatformVersion::latest(). Their nested declarations, function restrictions, malformed parameters, and parameter-count cases now exercise generation 1 rather than the generation 0 parser retained by protocol 14. The new protocol-14 token rejection case does not preserve that coverage: its nonvalidating parse exits at the integer-property restriction before reaching function and parameter parsing. Parameterize the existing registration cases across protocol 14 and latest, or retain an explicit protocol-14 suite alongside the new-generation tests. This follows the book's requirement to preserve older-generation tests when introducing a new generation; it is not evidence that historical behavior currently differs.

In `packages/rs-dpp/src/data_contract/document_type/class_methods/try_from_schema/mod.rs`:
- [SUGGESTION] packages/rs-dpp/src/data_contract/document_type/class_methods/try_from_schema/mod.rs:3198-3206: Reject all constraints that can exclude token decimal values
  (existing thread: https://github.com/dashpay/platform/pull/5526#discussion_r4428620520)
  The declaration guard claims to require an integer property admitting every value from 0 through 255, but checks only minimum, maximum, and enum. The native registration grammar also accepts exclusiveMinimum, exclusiveMaximum, multipleOf, and const, and the compiled document validator preserves and enforces them. For example, adding multipleOf: 2 to an otherwise valid generated-decimals declaration passes registration but rejects a document referencing a token with three decimals; exclusiveMinimum: 0 excludes a supported boundary in the same way. Complete the admission check in apply_generated_from_v1, either by checking the supported constraints or validating all 256 possible outputs, and add validating and nonvalidating registration regressions. The deployed marketplace schemas do not contain these restrictions, and later validation rejects incompatible documents normally, so this is a nonblocking registration-consistency defect rather than a demonstrated consensus or security failure.

parse_property_constraints: Some(1), // changed: countOf and sumOf match byte array keys
// changed: documents are checked against the typed validation schema this builds
validation_schema: Some(0),
apply_generated_from: Some(1), // changed: admits sys.tokens.decimals

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

🟡 Suggestion: Preserve protocol-14 coverage when selecting the new generatedFrom parser

Selecting apply_generated_from generation 1 redirects the existing string-transformation registration tests because parse and expect_refused in try_from_schema/v3/generated_from_tests.rs still use PlatformVersion::latest(). Their nested declarations, function restrictions, malformed parameters, and parameter-count cases now exercise generation 1 rather than the generation 0 parser retained by protocol 14. The new protocol-14 token rejection case does not preserve that coverage: its nonvalidating parse exits at the integer-property restriction before reaching function and parameter parsing. Parameterize the existing registration cases across protocol 14 and latest, or retain an explicit protocol-14 suite alongside the new-generation tests. This follows the book's requirement to preserve older-generation tests when introducing a new generation; it is not evidence that historical behavior currently differs.

source: gpt-6.1-sol (phase2-reviewer: rust-quality)

QuantumExplorer and others added 2 commits October 31, 2026 06:48
…pendent-batch-estimates

Private document types (#5689) took v15 note 13; the batch estimate is now
note 14. valid-payment keeps the base's validation digest and this branch's
settled digest.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…market-escrow-contract

Private document types took v15 note 13; generatedFrom is now note 15 and the
marketplace state note 16, with their slot comments and references.
CONTRACT_VERSIONS_V7 selects both parse_private_document 0 and
apply_generated_from 1.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@thepastaclaw thepastaclaw left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

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

Verified the combined reviewer claims and all 37 prior findings against head 22de2dc. Three nonblocking issues remain: incomplete generated-decimals declaration checks, unresolved generated values in transition filters, and lost PV14 parser coverage; the alleged proof-verification vulnerability is not substantiated. This was a static review without builds or tests; current listed CI checks pass, but they do not establish execution of the core Rust or token-market JavaScript suites.

🟡 3 suggestion(s)

1 finding(s) not shown inline (the lines are not part of this PR's diff)

🟡 Suggestion: Account for state-generated fields in document transition filters
packages/rs-drive/src/query/filter.rs:418-432

This helper describes its result as the stored representation, but DocumentTypeBasicMethods::data_as_stored only runs fill_generated_properties, which cannot resolve sys.tokens.decimals. A valid create that omits decimals therefore returns Fail for a new-document predicate such as decimals == 6 even when execution stores 6; explicitly supplying the same value can produce a different filter result. Replace has the same mismatch, including replacements that must retain the original document's decimals rather than read the token's current configuration. Keep the resolution in Drive: reject unsupported predicates during filter validation, represent an unresolved result, or evaluate against resolved document data. Add omitted-versus-supplied create and replace coverage while preserving stateless string-generation behavior. The book acknowledges the limitation, but the API still silently produces false negatives; no production caller was found at this head, so this remains nonblocking API integration work.

source: muse-spark-1.3-contributor (phase1-reviewer: general, rust-quality); gpt-6.1-sol (phase2-reviewer: general, rust-quality, tenderdash-consensus-security)

2 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.1-sol (agent: phase2-reviewer, role: tenderdash-consensus-security); reviewer 7: muse-spark-1.3-contributor (agent: phase1-reviewer, role: general); reviewer 8: muse-spark-1.3-contributor (agent: phase1-reviewer, role: rust-quality); reviewer 9: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 10: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 11: gpt-6.1-sol (agent: phase2-reviewer, role: platform-versioning); reviewer 12: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 13: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); reviewer 14: gpt-6.1-sol (agent: phase2-reviewer, role: tenderdash-consensus-security); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)

  • Triage: critical by gpt-6.1-sol (effort low) — This large, intricate diff changes consensus-persisted storage through transition_to_version_15 and create_initial_state_structure_v5, and changes token supply accounting in packages/rs-drive/src/drive/tokens/calculate_total_tokens_balance/v2/mod.rs.
  • 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 quota lookup failed), 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 — tenderdash-consensus-security (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, gpt-6.1-sol — tenderdash-consensus-security (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:418-432: Account for state-generated fields in document transition filters
  This helper describes its result as the stored representation, but DocumentTypeBasicMethods::data_as_stored only runs fill_generated_properties, which cannot resolve sys.tokens.decimals. A valid create that omits decimals therefore returns Fail for a new-document predicate such as decimals == 6 even when execution stores 6; explicitly supplying the same value can produce a different filter result. Replace has the same mismatch, including replacements that must retain the original document's decimals rather than read the token's current configuration. Keep the resolution in Drive: reject unsupported predicates during filter validation, represent an unresolved result, or evaluate against resolved document data. Add omitted-versus-supplied create and replace coverage while preserving stateless string-generation behavior. The book acknowledges the limitation, but the API still silently produces false negatives; no production caller was found at this head, so this remains nonblocking API integration work.

In `packages/rs-dpp/src/data_contract/document_type/class_methods/try_from_schema/mod.rs`:
- [SUGGESTION] packages/rs-dpp/src/data_contract/document_type/class_methods/try_from_schema/mod.rs:3200-3208: Reject all constraints that can exclude token decimal values
  (existing thread: https://github.com/dashpay/platform/pull/5526#discussion_r4428620520)
  The generation-1 parser checks the storage type, inclusive minimum/maximum, and enum, but that does not establish its advertised invariant that the property admits every integer from 0 through 255. The native schema checker also accepts exclusiveMinimum, exclusiveMaximum, multipleOf, and const, and the document validator enforces them. For example, adding multipleOf: 2 to an otherwise valid decimals declaration passes registration but rejects a generated value of 9; exclusiveMinimum: 0 excludes zero-decimal tokens. Enforce the full domain invariant in DPP's declaration parser, either by checking these restrictions or validating all 256 possible outputs, and add registration regressions. Subsequent action validation deterministically rejects incompatible documents before application, and the marketplace schemas do not contain these restrictions, so this is a registration-consistency issue rather than a validation bypass or demonstrated consensus failure.

In `packages/rs-platform-version/src/version/dpp_versions/dpp_contract_versions/v7.rs`:
- [SUGGESTION] packages/rs-platform-version/src/version/dpp_versions/dpp_contract_versions/v7.rs:49: Preserve protocol-14 coverage when selecting the new generatedFrom parser
  (existing thread: https://github.com/dashpay/platform/pull/5526#discussion_r4430261637)
  Selecting apply_generated_from generation 1 redirects the existing string-generation registration tests because generated_from_tests.rs still uses PlatformVersion::latest() in parse(), expect_refused(), and related registration cases. Their accepted nested declarations, malformed parameters, function restrictions, and parameter-count cases therefore no longer exercise generation 0 retained by PV14. The new PV14 token-function rejection does not replace that coverage: its nonvalidating parse exits at generation 0's integer-property restriction before parsing the function and parameters. Parameterize the applicable existing registration cases across PV14 and latest, or preserve a PV14-pinned suite alongside generation-1 coverage, as required by the book's testing conventions. This is a historical regression-coverage gap, not evidence that PV14 execution currently changed.

QuantumExplorer and others added 4 commits October 31, 2026 09:12
…nested ones included (PV15)

Generation 2 of apply_drive_operations lowered each operation with a map of
its own, but a map holds one description per path, so the nested estimators
of one operation still overwrote each other before the batch saw them: a
token mint's balance estimator (the root at level 2) replaced its supply
estimator's description (level 3), and the mint was priced below its supply's
own description.

Every estimator now writes into EstimatedLayers instead of a bare HashMap.
EstimatedLayers::new keeps the description written last, exactly as the map
did, so every estimate and protocol version but apply_drive_operations 2 is
unchanged. Generation 2 builds the batch's estimate with
EstimatedLayers::keeping_every_description, which also keeps every description
any write gives a path, and resolves it once the whole batch is lowered to the
costliest description of each layer. The per-operation maps go away. The two
estimators that write a description only when none is there use
insert_if_absent, and an unresolved estimate is refused at the GroveDB
boundary. The mechanical type change touches the signatures of shipped
estimator generations; their behaviour is the map's.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… claude/token-market-escrow-contract

The token market's estimators, escrow tests and keep-deeper helpers write into
EstimatedLayers. insert_estimated_layer_keeping_deeper and
register_keeping_deeper_estimated_layers go through
EstimatedLayers::insert_keeping_deeper, which keeps the deeper description as
before and, in an estimate keeping every description, also keeps the
shallower one. The composite-mint regression shows the description written
last at protocol version 14, as this branch's for_token_balances 1 keeps the
deeper root at the latest. The closing mint group's settled digest is
re-measured.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The estimate parameters' docs still called them hashmaps; they name
EstimatedLayers now. CHANGELOG notes the Rust API change apart from the
protocol change: Drive's estimators and DriveLowLevelOperationConverter take
EstimatedLayers. The composite-mint regression shows the description written
last at protocol version 14, where generation 1 ships it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@thepastaclaw thepastaclaw left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

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

Static verification of ea186c5..ce348ac found no confirmed blocking defect; four nonblocking findings remain after revalidating all 37 prior finding identities. The escrow activation, conservation accounting, order protections and estimation changes are coherent, and the alleged generated-decimals proof vulnerability is not substantiated. Current listed CI checks pass, but they do not establish a full Rust workspace or token-market JavaScript suite result; no builds or tests were run locally.

🟡 4 suggestion(s)

1 finding(s) not shown inline (the lines are not part of this PR's diff)

🟡 Suggestion: Account for state-generated fields in document transition filters
packages/rs-drive/src/query/filter.rs:418-432

This matcher treats DocumentTypeBasicMethods::data_as_stored as the complete stored representation, but that helper only fills stateless generated properties. A create or replacement that omits sys.tokens.decimals leaves the field absent here, although action transformation supplies its value before storage. A predicate such as decimals == 6 can consequently return Fail for a document that is successfully stored with decimals 6. Keep resolution in Drive: evaluate these predicates using materialized action/document properties, or explicitly report that state-generated data is unavailable instead of treating it as absent. Replacement coverage should include retaining creation-time decimals after the token configuration changes; fetching the current configuration alone would give the wrong value in that case. No production caller of this matcher was found, so this remains a preparatory API correctness issue.

source: muse-spark-1.3-contributor (phase1-reviewer: general, rust-quality); gpt-6.1-sol (phase2-reviewer: general, architecture-layering, rust-quality, tenderdash-consensus-security)

2 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.1-sol (agent: phase2-reviewer, role: tenderdash-consensus-security); reviewer 7: muse-spark-1.3-contributor (agent: phase1-reviewer, role: general); reviewer 8: muse-spark-1.3-contributor (agent: phase1-reviewer, role: rust-quality); reviewer 9: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 10: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 11: gpt-6.1-sol (agent: phase2-reviewer, role: platform-versioning); reviewer 12: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 13: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); reviewer 14: gpt-6.1-sol (agent: phase2-reviewer, role: tenderdash-consensus-security); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)

  • Triage: critical by gpt-6.1-sol (effort low) — This large, intricate diff changes consensus-persisted state through create_initial_state_structure_v5 and transition_to_version_15, adds escrow storage primitives, and changes token-supply accounting in packages/rs-drive/src/drive/tokens/calculate_total_tokens_balance/v2/mod.rs.
  • 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 quota lookup failed), 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 — tenderdash-consensus-security (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, gpt-6.1-sol — tenderdash-consensus-security (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:418-432: Account for state-generated fields in document transition filters
  This matcher treats DocumentTypeBasicMethods::data_as_stored as the complete stored representation, but that helper only fills stateless generated properties. A create or replacement that omits sys.tokens.decimals leaves the field absent here, although action transformation supplies its value before storage. A predicate such as decimals == 6 can consequently return Fail for a document that is successfully stored with decimals 6. Keep resolution in Drive: evaluate these predicates using materialized action/document properties, or explicitly report that state-generated data is unavailable instead of treating it as absent. Replacement coverage should include retaining creation-time decimals after the token configuration changes; fetching the current configuration alone would give the wrong value in that case. No production caller of this matcher was found, so this remains a preparatory API correctness issue.

In `packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/document/generated_from_token.rs`:
- [SUGGESTION] packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/document/generated_from_token.rs:326-334: Authenticate the reconstructed root in generated-value proof tests
  OrderFixture::verify_proof discards the root returned by verify_state_transition_was_executed_with_proof. That API reconstructs a root but receives no expected root; production authentication happens separately in the proof-verifier layer. These new tests therefore check document reconstruction and generated-value adoption without asserting that the reconstructed root equals the committed fixture state containing the documents. Retain the returned root and compare it with self.platform.drive.grove.root_hash at the selected platform version, following the marketplace depth-proof regression. This strengthens the regression test and does not imply that production adoption from an authenticated document is unsafe.

In `packages/rs-dpp/src/data_contract/document_type/class_methods/try_from_schema/mod.rs`:
- [SUGGESTION] packages/rs-dpp/src/data_contract/document_type/class_methods/try_from_schema/mod.rs:3200-3208: Reject all constraints that can exclude token decimal values
  (existing thread: https://github.com/dashpay/platform/pull/5526#discussion_r4428620520)
  The admission rule requires this property to accept every generated integer from 0 through 255, but checks only minimum, maximum and enum. The native registration checker also accepts exclusiveMinimum, exclusiveMaximum, multipleOf and const, and the native document validator enforces them. An integer declaration with minimum 0, maximum 255 and multipleOf 2 therefore passes registration, but a create referencing a token with decimals 3 is filled with 3 and subsequently rejected. Enforce the complete admissible domain in apply_generated_from_v1, either by checking these restrictions or refusing constraints not proven to admit the full range, and add validating and nonvalidating registration regressions. This is nonblocking registration consistency: downstream validation prevents invalid data from being stored, and the marketplace schema itself does not use these narrowing constraints.

In `packages/rs-platform-version/src/version/dpp_versions/dpp_contract_versions/v7.rs`:
- [SUGGESTION] packages/rs-platform-version/src/version/dpp_versions/dpp_contract_versions/v7.rs:49: Preserve protocol-14 coverage when selecting the new generatedFrom parser
  (existing thread: https://github.com/dashpay/platform/pull/5526#discussion_r4430261637)
  Selecting apply_generated_from generation 1 redirects the existing registration cases in try_from_schema/v3/generated_from_tests.rs because their parse and expect_refused helpers use PlatformVersion::latest(). Those string-transformation, nested-declaration, malformed-function and parameter-validation cases therefore no longer exercise generation 0, which PV14 still selects. The new PV14 token rejection test does not replace this coverage: its nonvalidating integer-property case returns before generation 0's function and parameter parsing. Parameterize the applicable registration cases across PV14 and latest, or retain a PV14-pinned suite. The book's Tests section explicitly requires pinning the older generation's tests when introducing its successor; this is a coverage gap, not evidence that PV14 execution has changed.

Comment on lines +326 to +334
let (_, outcome) = Drive::verify_state_transition_was_executed_with_proof(
transition,
&BlockInfo::default(),
&proof,
&|id| Ok(known_contracts.get(id).cloned().map(Arc::new)),
platform_version,
)
.expect("expected the proof to verify");
outcome.into_result()

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

🟡 Suggestion: Authenticate the reconstructed root in generated-value proof tests

OrderFixture::verify_proof discards the root returned by verify_state_transition_was_executed_with_proof. That API reconstructs a root but receives no expected root; production authentication happens separately in the proof-verifier layer. These new tests therefore check document reconstruction and generated-value adoption without asserting that the reconstructed root equals the committed fixture state containing the documents. Retain the returned root and compare it with self.platform.drive.grove.root_hash at the selected platform version, following the marketplace depth-proof regression. This strengthens the regression test and does not imply that production adoption from an authenticated document is unsafe.

source: gpt-6.1-sol (phase2-reviewer: rust-quality)

QuantumExplorer and others added 4 commits October 31, 2026 11:36
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…estimate

The pick of apply_drive_operations 2 ranked a layer's descriptions by
propagation price alone, and a description of items alone prices above one of
subtrees under the same keys. A total system credits update describes Misc as
its credits item, and a token mint describes it as holding the token supplies
tree, so the batch's estimate kept the items description and GroveDB refused
to price the supplies tree's new root key (layered_flags_size), in either
order. When a write describes a layer as holding subtrees, the pick is now
made among the descriptions that do; the depth is still the deepest any write
gives. Generation 1 priced the same batch that way whenever the mint came last,
and failed with the mint first.

No pinned estimate moves. The docs now say no write is priced at a shallower
depth than its own description, which is what still holds.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@thepastaclaw thepastaclaw left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

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

Static verification of fa9e3b4 found four nonblocking generated-property validation, filtering, and regression-coverage issues, with no confirmed blocking defect. The claimed production proof-authentication bypass is refuted by the authenticated-root check in the production proof consumer. Current CI checks pass, but they do not establish targeted Rust or token-market JavaScript suite results; no builds or tests were run locally.

🟡 4 suggestion(s)

1 finding(s) not shown inline (the lines are not part of this PR's diff)

🟡 Suggestion: Account for state-generated fields in document transition filters
packages/rs-drive/src/query/filter.rs:418-432

This helper delegates to data_as_stored, which runs stateless generation and leaves an omitted sys.tokens.decimals property unresolved. Consequently, matches_document_transition returns Fail for a valid create whose resulting document satisfies a predicate such as decimals == 8. Replacement predicates have the same problem, and NeedsOriginal cannot repair it because new-document clauses fail first. Give the filter an explicit resolved-document evaluation path or a resolution-required result for affected predicates; alternatively, reject unsupported predicates during filter validation. Do not reconstruct replacements from the token's current configuration, because an unchanged token retains the document's earlier decimals snapshot. No production subscription consumer was found, so this remains nonblocking API preparation.

source: muse-spark-1.3-contributor (phase1-reviewer: general, rust-quality); gpt-6.1-sol (phase2-reviewer: general, architecture-layering, rust-quality, tenderdash-consensus-security); gpt-6-astra (phase2-reviewer: general, rust-quality)

3 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: 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: tenderdash-consensus-security); reviewer 9: gpt-6-astra (agent: phase2-reviewer, role: general); 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); reviewer 18: gpt-6.1-sol (agent: phase2-reviewer, role: tenderdash-consensus-security); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)

  • Triage: critical by gpt-6.1-sol (effort low) — The large, intricate diff changes consensus-critical persisted state through transition_to_version_15 and create_initial_state_structure_v5, adds escrow storage operations, and changes token supply accounting in calculate_total_tokens_balance/v2.
  • 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 quota lookup failed), 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 — tenderdash-consensus-security (completed, effort xhigh); agent phase2-reviewer, gpt-6-astra — general (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, gpt-6.1-sol — tenderdash-consensus-security (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:418-432: Account for state-generated fields in document transition filters
  This helper delegates to data_as_stored, which runs stateless generation and leaves an omitted sys.tokens.decimals property unresolved. Consequently, matches_document_transition returns Fail for a valid create whose resulting document satisfies a predicate such as decimals == 8. Replacement predicates have the same problem, and NeedsOriginal cannot repair it because new-document clauses fail first. Give the filter an explicit resolved-document evaluation path or a resolution-required result for affected predicates; alternatively, reject unsupported predicates during filter validation. Do not reconstruct replacements from the token's current configuration, because an unchanged token retains the document's earlier decimals snapshot. No production subscription consumer was found, so this remains nonblocking API preparation.

In `packages/rs-dpp/src/data_contract/document_type/class_methods/try_from_schema/mod.rs`:
- [SUGGESTION] packages/rs-dpp/src/data_contract/document_type/class_methods/try_from_schema/mod.rs:3200-3208: Reject all constraints that can exclude token decimal values
  (existing thread: https://github.com/dashpay/platform/pull/5526#discussion_r4428620520)
  The PV15 registration check claims that the generated property admits every integer from 0 through 255, but checks only minimum, maximum, and enum. Native property registration also accepts const, exclusiveMinimum, exclusiveMaximum, and multipleOf, and the document validator enforces them. For example, adding multipleOf: 2 to the otherwise valid decimals declaration passes this check but rejects a generated value of 3; exclusiveMinimum: 0 excludes a valid zero-decimal token. Complete the full-range check in apply_generated_from_v1 and add validating and nonvalidating registration regressions. This is nonblocking registration consistency: action-level schema validation rejects incompatible values before persistence, and the built-in marketplace schemas do not contain these restrictions.

In `packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/document/generated_from_token.rs`:
- [SUGGESTION] packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/document/generated_from_token.rs:326-334: Authenticate the reconstructed root in generated-value proof tests
  (existing thread: https://github.com/dashpay/platform/pull/5526#discussion_r4431068033)
  The shared proof helper discards the root returned by verify_state_transition_was_executed_with_proof. That API reconstructs a root without receiving an expected one, so these create/replace regressions check document reconstruction and generated-value adoption without asserting that the result authenticates the fixture's committed state. Retain the returned root and compare it with self.platform.drive.grove.root_hash at the same GroveDB version, following the corrected marketplace depth-proof regression. This is a test-coverage gap, not a production authentication bypass: the production proof consumer separately authenticates the returned root with verify_tenderdash_proof.

In `packages/rs-platform-version/src/version/dpp_versions/dpp_contract_versions/v7.rs`:
- [SUGGESTION] packages/rs-platform-version/src/version/dpp_versions/dpp_contract_versions/v7.rs:49: Preserve protocol-14 coverage when selecting the new generatedFrom parser
  (existing thread: https://github.com/dashpay/platform/pull/5526#discussion_r4430261637)
  Selecting apply_generated_from generation 1 redirects the existing generated_from_tests.rs grammar cases because their parsing and refusal helpers use PlatformVersion::latest(). The explicit PV14 token-function rejection checks activation, but does not retain generation 0's successful string declarations, malformed declarations, or parameter-validation coverage; its nonvalidating integer case exits before those branches. Parameterize the applicable existing registration cases over PV14 and latest, or retain a PV14-pinned suite alongside the new-generation cases. The dispatch is correct; the missing historical regression coverage is the issue, consistent with the book's requirement to pin a generation's tests when it becomes frozen.

QuantumExplorer and others added 5 commits November 1, 2026 11:53
… into claude/order-independent-batch-estimates

The private document store's v15 note 13 and DRIVE_VERSION_V10 bullet sit
beside the batch estimate's note 14 and bullet.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…es) into claude/token-market-escrow-contract

DRIVE_VERSION_V10 selects both the private document store's document method
versions and this branch's escrow and estimation versions.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…, deletes and Misc described as they are (PV15)

Review 2 of this pull request.

EstimatedLayers keeps one map of the description written last, which GroveDB
prices as before, and remembers for the few paths described more than one way
the descriptions it did not keep (replaced, or not written by insert_if_absent).
Only apply_drive_operations 2 resolves an estimate, so the mode flag, the second
full map, keeping_every_description, the refusal in into_layers and From<HashMap>
are gone; insert returns nothing. costliest_layer takes the same max with
max_by, and describes_subtrees is GroveDB's layered_flags_size check.

Generation 2 also keeps the root at the level of the deepest root key the batch
writes under (ROOT_KEY_LEVELS, pinned to grovedb-structure.json's root shape):
the credit shielded pool and contract groups described the root at level 3 for
keys at level 4.

Three estimators describe their layers as they are from protocol version 15:
- remove_reference_for_index_level_for_contract_operations 2: an index's
  entries for a value as references (v1 said subtrees, and the kind filter
  kept that cheaper description on a same-type consume);
- stateless_delete_of_non_tree_for_costs 1: height h gets the path's first
  h + 1 keys, the layer GroveDB deletes in there (v0 gave the one above);
- for_total_system_credits_update 1: Misc as the expirations tree, the credits
  item and the token supplies big sum tree.
v1 and v0's bodies move into helpers they call with their old description, so
their output is unchanged.

Note 14, the V10 doc, the book chapters (which still showed the HashMap API),
the CHANGELOG and the comments that described the last-written rule as
universal now say what holds. Dispatcher tests cover both sides of protocol
version 15; a DashPay profile delete's estimate pin moves 72,064,200 ->
73,034,400.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…::new)

The 54 estimate constructions the type rename expanded into five-line
`if apply { None } else { Some(EstimatedLayers::new()) }` blocks are one line,
as generation 2 and the tests already wrote it. The rename's nested backticks
in three doc comments are undone, and two debugging snippets that iterated the
estimate as a map print it with dbg! instead. No output changes.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…timates, PV15 delete and Misc descriptions) into claude/token-market-escrow-contract

The estimate container keeps one last-written map and remembers the other
descriptions of a path; insert_keeping_deeper now writes into it the same way
(the description it declines or replaces is kept among the path's others),
with the same last-written result as before. Its tests and the escrow test
build estimates with EstimatedLayers::new instead of the removed From<HashMap>.
DRIVE_VERSION_V10 keeps the marketplace's tables and gains the base's
document and system credits estimation versions; note 14 is the base's,
notes 15 and 16 and the slot comment keep the marketplace's.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Base automatically changed from claude/order-independent-batch-estimates to v5.1-dev November 1, 2026 21:52
@github-actions

github-actions Bot commented Nov 1, 2026

Copy link
Copy Markdown
Contributor

Your move: thepastaclaw left review threads unresolved; resolve them.
Full checklist in the description.

@github-actions github-actions Bot added the waiting-self-review Waiting for the author to post /self-reviewed label Nov 1, 2026
@github-actions

github-actions Bot commented Nov 1, 2026

Copy link
Copy Markdown
Contributor

Your move: thepastaclaw left review threads unresolved; resolve them.
Full checklist in the description.

…et-escrow-contract

# Conflicts:
#	packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/token/shielded/nullifier_batch_guard.rs
#	packages/rs-drive/src/util/estimated_layers.rs
#	packages/rs-platform-version/src/version/drive_versions/v10.rs
#	packages/rs-platform-version/src/version/v15.rs
@github-actions

github-actions Bot commented Nov 1, 2026 •

Copy link
Copy Markdown
Contributor

Your move: thepastaclaw left review threads unresolved; resolve them.
Full checklist in the description.

…et-escrow-contract

# Conflicts:
#	packages/rs-platform-version/src/version/drive_versions/v10.rs
#	packages/rs-platform-version/src/version/v15.rs
@github-actions

github-actions Bot commented Nov 1, 2026 •

Copy link
Copy Markdown
Contributor

Your move: thepastaclaw requested changes on this head; dismiss the review or push a fix; thepastaclaw left review threads unresolved; resolve them.
Full checklist in the description.

@thepastaclaw thepastaclaw left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

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

Verified the combined reviewer claims and all 38 prior findings against head 707af07. One blocker remains: the expanded PV15 genesis leaves a stale moderation-restore fee assertion that demonstrably fails Rust workspace CI; four nonblocking parser, filter, and regression-coverage issues also remain. This was a static review with no local builds or tests: current marketplace JS checks and builds pass, two end-to-end suites remain pending, and the separate Dashmate certificate test failure has no established connection to this diff.

🔴 1 blocking | 🟡 4 suggestion(s)

1 finding(s) not shown inline (the lines are not part of this PR's diff)

🟡 Suggestion: Account for state-generated fields in document transition filters
packages/rs-drive/src/query/filter.rs:418-432

matches_document_transition obtains create and replace data through DocumentTypeBasicMethods::data_as_stored, which only invokes the stateless fill_generated_properties. Client builders omit sys.tokens.decimals, so this path leaves the property absent and returns Fail for a clause such as decimals == 6 even when execution stores 6. A same-token replacement has the same limitation and must use the retained stored value, not necessarily the token's current configuration. Keep resolution in Drive: evaluate these clauses against the resolved action/document, or explicitly defer unsupported clauses instead of treating unavailable data as a definite mismatch. Add create and same-token-replace regressions. No production caller was found in this revision, so this is a nonblocking public-API integration gap, not demonstrated live notification loss; deferring it needs a linked tracking issue or PR.

source: muse-spark-1.3-contributor (phase1-reviewer: general); gpt-6.1-sol (phase2-reviewer: general, architecture-layering, rust-quality, tenderdash-consensus-security)

3 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.1-sol (agent: phase2-reviewer, role: tenderdash-consensus-security); reviewer 7: muse-spark-1.3-contributor (agent: phase1-reviewer, role: general); reviewer 8: muse-spark-1.3-contributor (agent: phase1-reviewer, role: rust-quality); reviewer 9: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 10: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 11: gpt-6.1-sol (agent: phase2-reviewer, role: platform-versioning); reviewer 12: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 13: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); reviewer 14: gpt-6.1-sol (agent: phase2-reviewer, role: tenderdash-consensus-security); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)

  • Triage: critical by gpt-6.1-sol (effort low) — The large, intricate diff changes consensus-persisted state through create_initial_state_structure_v5 and transition_to_version_15 and changes token supply accounting in packages/rs-drive/src/drive/tokens/calculate_total_tokens_balance/v2/mod.rs.
  • 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 quota lookup failed), 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 — tenderdash-consensus-security (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, gpt-6.1-sol — tenderdash-consensus-security (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-abci/src/execution/platform_events/initialization/create_genesis_state/v1/mod.rs`:
- [BLOCKING] packages/rs-drive-abci/src/execution/platform_events/initialization/create_genesis_state/v1/mod.rs:104-108: Update the restore-fee characterization for the expanded PV15 genesis
  Adding the genesis contract also affects the latest-version moderation-restore fee fixture, which was missed among the other genesis-dependent baseline updates. `contract_user_moderation/tests/restore_constraints.rs::should_preserve_the_rule_free_restore_fee_and_document` creates genesis at `PlatformVersion::latest()` but still asserts processing fees of 1,442,880. The completed Rust workspace job for this exact head reports 1,492,880, with storage fees unchanged at 9,612,000, and stops with 8,056 tests unexecuted: https://github.com/dashpay/platform/actions/runs/41356810363/job/125003155403. Account for the 50,000-credit change, update that exact baseline and its explanation, and validate the moderation-restore suite while preserving strict assertions and historical baselines. This is a confirmed failing CI gate, not evidence that the new production fee is incorrect.

In `packages/rs-drive/src/query/filter.rs`:
- [SUGGESTION] packages/rs-drive/src/query/filter.rs:418-432: Account for state-generated fields in document transition filters
  `matches_document_transition` obtains create and replace data through `DocumentTypeBasicMethods::data_as_stored`, which only invokes the stateless `fill_generated_properties`. Client builders omit `sys.tokens.decimals`, so this path leaves the property absent and returns `Fail` for a clause such as `decimals == 6` even when execution stores 6. A same-token replacement has the same limitation and must use the retained stored value, not necessarily the token's current configuration. Keep resolution in Drive: evaluate these clauses against the resolved action/document, or explicitly defer unsupported clauses instead of treating unavailable data as a definite mismatch. Add create and same-token-replace regressions. No production caller was found in this revision, so this is a nonblocking public-API integration gap, not demonstrated live notification loss; deferring it needs a linked tracking issue or PR.

In `packages/rs-dpp/src/data_contract/document_type/class_methods/try_from_schema/mod.rs`:
- [SUGGESTION] packages/rs-dpp/src/data_contract/document_type/class_methods/try_from_schema/mod.rs:3200-3208: Reject all constraints that can exclude token decimal values
  (existing thread: https://github.com/dashpay/platform/pull/5526#discussion_r4428620520)
  The generation-1 compatibility check does not establish its stated guarantee that the property accepts every integer from 0 through 255: it checks only `minimum`, `maximum`, and `enum`. The native registration checker also admits `const`, `exclusiveMinimum`, `exclusiveMaximum`, and `multipleOf`, and the document validator enforces them. For example, adding `multipleOf: 2` to an otherwise valid decimals declaration passes registration but rejects a generated decimals value of 3; `exclusiveMinimum: 0` excludes the valid value 0. Complete the output-domain check in DPP's generation-1 parser, with validating and non-validating registration regressions and a generated-document validation case. Leave historical generation 0 unchanged. This is nonblocking because ordinary document validation rejects incompatible values rather than accepting invalid persisted state; the supplied evidence does not establish a consensus halt or a broken marketplace schema.

In `packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/document/generated_from_token.rs`:
- [SUGGESTION] packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/document/generated_from_token.rs:326-334: Authenticate the reconstructed root in generated-value proof tests
  (existing thread: https://github.com/dashpay/platform/pull/5526#discussion_r4431068033)
  `OrderFixture::verify_proof` still destructures the verifier result as `(_, outcome)`. This lower-level API reconstructs a root but receives no expected root, so the fixture tests document reconstruction and generated-value adoption without asserting that they authenticate the state it committed. Retain the returned root and compare it with `self.platform.drive.grove.root_hash` at the same version, following the marketplace depth-proof regression. This is a regression-coverage gap, not a production proof-authentication bypass: the production `StateTransitionProofOutcome` proof consumer authenticates the returned root with `verify_tenderdash_proof` before returning the outcome.

In `packages/rs-platform-version/src/version/dpp_versions/dpp_contract_versions/v7.rs`:
- [SUGGESTION] packages/rs-platform-version/src/version/dpp_versions/dpp_contract_versions/v7.rs:49: Preserve protocol-14 coverage when selecting the new generatedFrom parser
  (existing thread: https://github.com/dashpay/platform/pull/5526#discussion_r4430261637)
  Selecting `apply_generated_from: Some(1)` redirects the existing string-generation grammar tests: the shared parsing and rejection helpers in `v3/generated_from_tests.rs` still use `PlatformVersion::latest()`, so they now exercise generation 1 rather than PV14's frozen generation 0. The new test that PV14 rejects `sys.tokens.decimals` covers activation but does not preserve the historical string parser's positive and malformed-declaration cases. Parameterize the applicable existing cases across PV14 and latest, or retain an explicitly PV14 suite alongside the latest cases. The book's testing convention explicitly requires preserving older-generation coverage when introducing a new generation.

Comment on lines +104 to +108
if platform_version.protocol_version >= TOKEN_MARKETPLACE_INITIAL_PROTOCOL_VERSION {
system_data_contract_types.insert(
SystemDataContract::TokenMarket,
system_data_contracts.load_token_market(platform_version)?,
);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

🔴 Blocking: Update the restore-fee characterization for the expanded PV15 genesis

Adding the genesis contract also affects the latest-version moderation-restore fee fixture, which was missed among the other genesis-dependent baseline updates. contract_user_moderation/tests/restore_constraints.rs::should_preserve_the_rule_free_restore_fee_and_document creates genesis at PlatformVersion::latest() but still asserts processing fees of 1,442,880. The completed Rust workspace job for this exact head reports 1,492,880, with storage fees unchanged at 9,612,000, and stops with 8,056 tests unexecuted: https://github.com/dashpay/platform/actions/runs/41356810363/job/125003155403. Account for the 50,000-credit change, update that exact baseline and its explanation, and validate the moderation-restore suite while preserving strict assertions and historical baselines. This is a confirmed failing CI gate, not evidence that the new production fee is incorrect.

source: gpt-6.1-sol (phase2-reviewer: general, architecture-layering, platform-versioning, rust-quality, security-auditor, tenderdash-consensus-security)

This branch has not been deployed

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

Labels

waiting-self-review Waiting for the author to post /self-reviewed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants