Repository navigation
feat(platform)!: token marketplace listings (PV15) - #5529
QuantumExplorer wants to merge 51 commits into
Conversation
Holders of a tradeable token can list it for sale (TokenPlaceAsk), buy part or all of a listing (TokenFillAsk) and withdraw an open order (TokenCancelOrder). Listed tokens wait in escrow; a fill moves them to the buyer and the buyer's credits to the seller in the same transition, paying the change in the listing's rounded-up running total. Each kind is refused unpaid before protocol version 15 by the batch's is_allowed gate, driven by the exhaustive TokenTransition::is_marketplace_order. New basic errors 10462-10463 and state errors 40750-40757, token events OrderPlaced/OrderFilled/OrderCancelled, proofs for each kind, wasm-dpp2 bindings, the wallet signing summary, the book and v15 note 7. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…market-listings Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A listing's tradeable token, value and expiry, and a fill's tradeable token, read nothing beyond the contract the action carries and the block, so they move from state validation to per-action structure validations 0 that the batch's advanced structure 2 runs, where check_tx runs them too. Paused, frozen and balance checks stay in state validation. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the
|
|
📖 Book Preview built successfully. Download the preview from the workflow artifacts. Updated at 2026-10-27T15:38:07.183Z |
…market-listings Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…market-listings Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
add_to_token_order_escrow_operations now takes the batch's earlier operations. A transition places one listing and its operations apply as their own batch, so the listing converter passes none. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
thepastaclaw
left a comment
There was a problem hiding this comment.
Final validation — Phase 1 + Phase 2
The marketplace flow is broadly coherent, but several newly introduced paths can either alter the economic meaning of an existing listing, reject user-controlled data as an internal error, or accept incomplete execution proofs. The verify-only feature configuration and WASM numeric setters also leave valid consumers exposed to broken proof verification or silently changed financial terms.
🔴 5 blocking | 🟡 1 suggestion(s)
1 finding(s) not shown inline (the lines are not part of this PR's diff)
🔴 Blocking: Enable the token-market contract in Drive's verify feature
packages/rs-drive/Cargo.toml:152-160
The new verifier unconditionally loads the TokenMarket system contract, but Drive's verify feature enables dpp/system_contracts, dpp/token-history-contract, and dpp/withdrawals-contract without dpp/token-market-contract. In the production wasm-drive-verify dependency configuration, default features are disabled and only Drive's verify feature is enabled; the data-contracts crate therefore uses its ContractNotIncluded("token-market") implementation. Standalone verify-only consumers cannot verify valid PlaceAsk or CancelOrder proofs even though full/server builds work through broader feature unification. Add dpp/token-market-contract to Drive's verify feature and cover verification with the verify-only feature set.
source: gpt-6.1-sol (phase2-reviewer: general, architecture-layering, ffi-engineer, platform-versioning, rust-quality, security-auditor)
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: ffi-engineer); reviewer 4: gpt-6.1-sol (agent: phase2-reviewer, role: platform-versioning); reviewer 5: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 6: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); reviewer 7: muse-spark-1.3-contributor (agent: phase1-reviewer, role: general); reviewer 8: muse-spark-1.3-contributor (agent: phase1-reviewer, role: rust-quality); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)
- Triage:
criticalbygpt-6.1-sol(effort low) — This is a large, intricate change to consensus-critical token state transitions and platform funds movement, including escrow accounting, validation, serialization, and execution across DPP, Drive, and Drive ABCI. - Phase 1 reviewers:
muse-spark-1.3-contributor— general (completed, effort xhigh); agentphase1-reviewer,muse-spark-1.3-contributor— rust-quality (completed, effort xhigh); agentphase1-reviewer - Phase 1 model:
muse-spark-1.3-contributor— not quota-gated; passed overgemini-3.8-flash-high(lane 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 verifier:
gpt-6.1-sol— final-verifier; agentsol-verifier - Phase 2 reviewers:
gpt-6.1-sol— general (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— architecture-layering (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— ffi-engineer (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— platform-versioning (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— rust-quality (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— security-auditor (completed, effort xhigh); agentphase2-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/state_transition_action/batch/batched_transition/token_transition/token_fill_ask_transition_action/v0/transformer.rs`:
- [BLOCKING] packages/rs-drive/src/state_transition_action/batch/batched_transition/token_transition/token_fill_ask_transition_action/v0/transformer.rs:159-160: Preserve the listing pricing scale across convention changes
Every fill derives the price scale from the token's current `decimals`, but `TokenConfigurationChangeItem::Conventions` can update that value after an order is placed and `TokenOrder` stores no placement-time scale. For example, an order priced at 100 credits with zero decimals charges 100 credits for its first base unit; after changing to two decimals, the next unit costs only `ceil(200 / 100) - ceil(100 / 100) = 1` credit. This lets an authorized convention update change the terms of an already-open listing without the maker signing new terms. Decreasing the decimals can also make a previously valid order exceed the representable credit bound, causing `fill_credits` to return an error that this `?` propagates as an internal failure. Persist the scale with the order or reject convention changes/fills that are incompatible with open orders, and test placement, partial fill, convention update, and subsequent fill.
In `packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/batched_transition/token_place_ask_transition/validate_structure/v0/mod.rs`:
- [BLOCKING] packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/batched_transition/token_place_ask_transition/validate_structure/v0/mod.rs:36-41: Reject expiresAt values that cannot be stored in the order document
The structural validator bounds amount, price, and min_fill but accepts any `u64` expiry. The place transformer constructs `TokenOrder::new_document` before advanced structure validation, and `TokenOrder::properties()` converts `expires_at` to signed `i64`; values above `i64::MAX` therefore return `ProtocolError::Overflow` through the transformer rather than a deterministic consensus validation error. A signed transition with `expires_at = i64::MAX + 1` can consequently take the internal-error path instead of the normal paid refusal and nonce handling. Validate the signed storage-width bound here, while retaining the block-time comparison in advanced validation, and cover `i64::MAX` and `i64::MAX + 1`.
In `packages/rs-drive/src/verify/state_transition/verify_state_transition_was_executed_with_proof/v0/mod.rs`:
- [BLOCKING] packages/rs-drive/src/verify/state_transition/verify_state_transition_was_executed_with_proof/v0/mod.rs:1362-1367: Verify that a placed listing retains its full remaining amount
PlaceAsk proof verification checks the order id, token, maker, price, amount, min_fill, and expiry, but not `order.remaining`. The transition's placement semantics require `remaining == amount`; without that check, a cryptographically valid proof of an order with the same terms but a smaller remaining amount is accepted as proof that the placement executed. Add `order.remaining != token_place_ask_transition.amount()` to the mismatch condition.
In `packages/rs-drive/Cargo.toml`:
- [BLOCKING] packages/rs-drive/Cargo.toml:152-160: Enable the token-market contract in Drive's verify feature
The new verifier unconditionally loads the TokenMarket system contract, but Drive's `verify` feature enables `dpp/system_contracts`, `dpp/token-history-contract`, and `dpp/withdrawals-contract` without `dpp/token-market-contract`. In the production `wasm-drive-verify` dependency configuration, default features are disabled and only Drive's `verify` feature is enabled; the data-contracts crate therefore uses its `ContractNotIncluded("token-market")` implementation. Standalone verify-only consumers cannot verify valid PlaceAsk or CancelOrder proofs even though full/server builds work through broader feature unification. Add `dpp/token-market-contract` to Drive's `verify` feature and cover verification with the verify-only feature set.
In `packages/wasm-dpp2/src/state_transitions/batch/token_transitions/token_place_ask.rs`:
- [BLOCKING] packages/wasm-dpp2/src/state_transitions/batch/token_transitions/token_place_ask.rs:113-133: Validate BigInt ranges in marketplace setters before mutation
The constructors convert marketplace integers with `try_to_u64`, but the new setters expose raw `u64`/`Option<u64>` WASM ABI parameters. Out-of-range JavaScript BigInts can therefore be converted by the ABI rather than rejected, silently changing the signed financial terms before the DPP validator sees them; for example, a value above `u64::MAX` can wrap to a different valid price, and the same issue affects amount, minFill, expiry, and the fill transition's minAmount, maxAmount, and maxTotalPrice setters. Existing wrappers in this crate accept `&js_sys::BigInt` and call `try_to_u64`, returning an error before mutating the transition. Apply that pattern to all new marketplace integer setters and add regression coverage that failed conversions leave the prior value unchanged.
In `packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/token/marketplace/mod.rs`:
- [SUGGESTION] packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/token/marketplace/mod.rs:827-834: Assert the specific insufficient-funds rejection
The test only asserts that the result is not `SuccessfulExecution`. It therefore also passes for an internal error, an unrelated validation error, `NotExecuted`, an empty result list, or multiple results. This does not establish that the fill's purchase credits are included in fee validation and that an unaffordable buyer receives the expected paid consensus rejection. Match exactly one expected insufficient-balance or fee error while retaining the balance and escrow assertions.
- A listing records the token's decimals when it is placed and every fill prices it at them, so a conventions update cannot reprice an open listing. Fills read no token configuration. - An expiresAt above i64::MAX is refused by the structure validation with the new basic error InvalidTokenOrderExpiryError (10464), through one validate_token_order_terms shared with offers. - A listing or a fill naming a token its contract does not define bumps the nonce with ContractHasNoTokensError or InvalidTokenPositionError instead of failing as an internal error; the token base structure validation reads the same helper (token_configuration_or_position_error). - The PlaceAsk proof check also requires remaining == amount. - Drive's verify feature enables dpp/token-market-contract, so a verify-only build has the contract the order proofs read. - wasm-dpp2 listing setters convert BigInts with try_to_u64 and leave the transition unchanged when out of range. - The cannot-pay test asserts the exact unpaid IdentityInsufficientBalanceError for a buyer short of the fee and one short of the price. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The PlaceAsk proof accepted only an untouched ask, so a client asking for the proof after a fill could not confirm its placement. The order id follows from the token, the signer and the nonce, so only this transition creates an ask with it: one holding the transition's terms proves the placement with any remaining from the amount down to 1. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
thepastaclaw
left a comment
There was a problem hiding this comment.
Re-review — Final validation — Phase 1 + Phase 2
At head d4ab767, verification confirms one blocking financial-authorization issue and two targeted test-coverage suggestions. Five prior findings are fixed; the requirement that placement proofs retain the full original amount is withdrawn because legitimate partial fills preserve evidence of placement. This was a static review without builds or tests; current listed CI checks pass, but no Rust validation result is exposed in those checks.
🔴 1 blocking | 🟡 2 suggestion(s)
Review provenance
Source: reviewer 1: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 2: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 3: gpt-6.1-sol (agent: phase2-reviewer, role: platform-versioning); reviewer 4: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 5: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); reviewer 6: muse-spark-1.3-contributor (agent: phase1-reviewer, role: general); reviewer 7: muse-spark-1.3-contributor (agent: phase1-reviewer, role: rust-quality); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)
- Triage:
criticalbygpt-6.1-sol(effort low) — This is a large, intricate diff that changes consensus-critical token state transitions and directly moves tokens and credits through escrow, including validation, serialization, protocol-version gating, storage operations, and proof verification. - Phase 1 reviewers:
muse-spark-1.3-contributor— general (completed, effort xhigh); agentphase1-reviewer,muse-spark-1.3-contributor— rust-quality (completed, effort xhigh); agentphase1-reviewer - Phase 1 model:
muse-spark-1.3-contributor— not quota-gated; passed overgemini-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 verifier:
gpt-6.1-sol— final-verifier; agentsol-verifier - Phase 2 reviewers:
gpt-6.1-sol— general (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— architecture-layering (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— platform-versioning (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— rust-quality (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— security-auditor (completed, effort xhigh); agentphase2-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/state_transition_action/batch/batched_transition/token_transition/token_place_ask_transition_action/v0/transformer.rs`:
- [BLOCKING] packages/rs-drive/src/state_transition_action/batch/batched_transition/token_transition/token_place_ask_transition_action/v0/transformer.rs:102-105: Bind the pricing scale into the seller's signed placement
This lookup derives the listing's pricing denominator from mutable configuration at execution time. Neither TokenPlaceAskTransitionV0 nor its base transition signs expected decimals or a configuration version. A controller authorized to update conventions can therefore change another holder's pending sale without the holder's signature: 1,000 base units at a signed price of 1,000,000,000 yield 10,000 credits with 8 decimals, but increasing decimals to 16 before placement makes the entire listing cost only ceil(1000 * 1000000000 / 10^16) = 1 credit. Both configurations are valid. With the controller and seller being distinct identities, the configuration update does not invalidate the seller's nonce; the controller can subsequently buy the escrow and restore the conventions. Persisted decimals protect orders only after placement, and the buyer's maximum-price limit cannot protect the seller against this reduction. Include the seller's pricing scale in the signed DPP transition, validate any required configuration agreement before escrow creation, and compare the signed scale during placement-proof verification. Add an execution regression that signs as a holder distinct from the conventions controller, applies a conventions update, and then submits the original signed listing.
In `packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/action_validation/token/token_fill_ask_transition_action/state_v0/mod.rs`:
- [SUGGESTION] packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/action_validation/token/token_fill_ask_transition_action/state_v0/mod.rs:81-89: Cover the marketplace's frozen-account branches
The new marketplace execution suite never freezes either participant. It therefore does not exercise the distinction between checking the order's maker and checking the transaction's signer, or the exception allowing receipt into a frozen balance. Add cases through process_raw_state_transitions for a frozen maker, a frozen buyer with transfers to frozen balances disabled, and a frozen buyer with them enabled. On refusals, match IdentityTokenAccountFrozenError and the affected identity, and assert that escrow and token balances remain unchanged. Also verify that the frozen maker can still cancel and recover the remaining escrow. Existing transfer and freeze tests do not cover this marketplace-specific wiring.
In `packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/token/marketplace/mod.rs`:
- [SUGGESTION] packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/token/marketplace/mod.rs:617: Assert the specific proof failure after a sell-out
After the sell-out, this assertion accepts every verifier error rather than establishing that a valid absence proof cannot prove placement. The helper already panics on proof-generation failures, but unrelated verification errors such as UnknownContract or UnexpectedResultProof would still satisfy is_err(). The placement verifier explicitly returns ProofError::IncorrectProof when the expected listing is absent; match that variant to pin the intended rejection.
A listing's price is per whole token, so the token's decimals set what it is worth, and they were read from the token's configuration when the listing executed: whoever controls the token's conventions could change them between the seller signing and execution and reprice the listing (1,000 base units at 1,000,000,000 credits per whole token are 10,000 credits at 8 decimals, 1 credit at 16). - TokenPlaceAskTransitionV0 gains a signed `decimals` (constructor, wasm-dpp2 wrapper with a required option and checked setter). - The listing's structure validation (batch advanced structure, so check_tx too) refuses it with the new TokenOrderDecimalsMismatchError (40758) unless they are the token's. The order records them, and the placement proof compares them. - Tests: the attack, by a holder other than the conventions controller, is refused; fills of a frozen maker's listing and by a frozen buyer (unless the token allows transfers to frozen balances) are refused and the frozen maker can cancel; the sell-out proof failure is IncorrectProof. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
As a listing does (#5529): TokenPlaceBidTransitionV0 gains a signed `decimals` (constructor, wasm-dpp2 wrapper). The transformer refuses the offer with TokenOrderDecimalsMismatchError (40758, check_tx too) unless they are the token's, before the escrow is worked out from them, so whoever controls the token's conventions cannot change them between signing and execution and make the buyer escrow more (1,000 base units at 1,000,000,000 credits per whole token hold 10,000 credits at 8 decimals, 100,000,000 at 4). The order records them and the placement proof compares them. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…its proof A placement is never a group action and the node refuses group information on it, but the proof verifier does not run that validation: an executed listing re-sent with group information would verify. The PlaceAsk arm refuses it first. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…t sales' frozen-account roles - The PlaceBid proof arm refuses group information first, as the PlaceAsk arm does (#5529): a placement is never a group action, and the verifier does not run the node's structure validation. - Tests: an executed offer re-sent with group information is refused and the offer itself proves; a sale into an offer is refused naming the frozen seller whatever the token allows, and naming the frozen buyer unless the token allows transfers to frozen balances (then the tokens land in the frozen balance); a frozen buyer cancels and recovers its escrow. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…d position The proof verifier read a marketplace transition by its token id without checking that the id is the one its contract and position derive, as the node's structure validation requires. The genuine proof of an executed listing verified the same request re-signed naming another position of a two-token contract (or another contract) with the same token id, a request the node refuses. Every marketplace transition (is_marketplace_order) now refuses a token id its contract and position do not derive. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
thepastaclaw
left a comment
There was a problem hiding this comment.
Re-review — Final validation — Phase 1 + Phase 2
The final head contains a Rust compile error in the new order-ID fee helper and a consensus-visible accounting flaw when duplicate fills for one listing are included in the same BatchTransition. Marketplace events also omit the signed pricing scale, leaving historical placement prices ambiguous after the order document is gone. The available CI snapshot covers Kotlin, policy, title, and book checks, but does not establish that the touched Rust code compiles or that these paths pass tests.
🔴 2 blocking | 🟡 1 suggestion(s)
Review provenance
Source: reviewer 1: muse-spark-1.3-contributor (agent: phase1-reviewer, role: general); reviewer 2: muse-spark-1.3-contributor (agent: phase1-reviewer, role: rust-quality); reviewer 3: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 4: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 5: gpt-6.1-sol (agent: phase2-reviewer, role: ffi-engineer); reviewer 6: gpt-6.1-sol (agent: phase2-reviewer, role: platform-versioning); reviewer 7: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 8: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)
- Triage:
criticalbygpt-6.1-sol(effort low) — This is a large, intricate change that modifies consensus-critical token state transitions and atomic escrowed token/credit movement across DPP, Drive, version dispatch, and WASM boundaries. - Phase 1 reviewers:
muse-spark-1.3-contributor— general (completed, effort xhigh); agentphase1-reviewer,muse-spark-1.3-contributor— rust-quality (completed, effort xhigh); agentphase1-reviewer - Phase 1 model:
muse-spark-1.3-contributor— not quota-gated; passed overgemini-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 verifier:
gpt-6.1-sol— final-verifier; agentsol-verifier - Phase 2 reviewers:
gpt-6.1-sol— general (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— architecture-layering (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— ffi-engineer (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— platform-versioning (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— rust-quality (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— security-auditor (completed, effort xhigh); agentphase2-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-dpp/src/tokens/token_order_side.rs`:
- [BLOCKING] packages/rs-dpp/src/tokens/token_order_side.rs:91: Import size_of for order ID preimage sizing
`order_id_preimage_size` calls `size_of::<IdentityNonce>()`, but this module only imports `std::fmt`; `size_of` is not in the Rust prelude. The DPP crate therefore fails to compile on the newly added code path. Qualify the call or import `std::mem::size_of`.
In `packages/rs-drive/src/state_transition_action/batch/batched_transition/token_transition/token_fill_ask_transition_action/v0/transformer.rs`:
- [BLOCKING] packages/rs-drive/src/state_transition_action/batch/batched_transition/token_transition/token_fill_ask_transition_action/v0/transformer.rs:103-160: Reject duplicate fills of one listing within a single batch
Each fill independently fetches the order document and computes its fill amount, credits, and absolute `remaining_after` before any operations from the batch are applied. `BatchTransitionTransformerV0::transform_token_transitions_within_contract_v0` maps over all token transitions and only merges the results afterward, so two fills for the same `order_id` in one signed batch both observe the original remaining amount. Their operation converters then emit separate token transfers and stale absolute escrow updates. For a 1,000-unit listing, two fills with `max_amount = 700` can both accept 700, while both writes use `remaining_after = 300`; this can release more tokens than the escrow should contain or leave inconsistent escrow and order-document state. The existing regression processes two separate raw state transitions sequentially, not two fills inside one `BatchTransition`, so it does not cover this path. Reject repeated order IDs within a batch, or transform fills against a batch-local order state updated after each fill.
In `packages/rs-dpp/src/tokens/token_event.rs`:
- [SUGGESTION] packages/rs-dpp/src/tokens/token_event.rs:213-228: Include the signed pricing scale in marketplace events
The DPP-owned `OrderPlaced` and `OrderFilled` events expose a price or moved credits without the signed `decimals` that gives the price its meaning. A placement at the same numeric price represents different economics at different scales, and once an order is filled or cancelled the order document is unavailable to recover that signed value. Add the scale to the canonical event variants and propagate it through the JSON, value/document serializers, action event construction, and WASM event projection instead of relying on mutable token configuration or the transient order document.
…nsition batch cap SystemLimits::max_transitions_in_documents_batch lists the marketplace fill among what raising the cap needs: a fill reads its order as state holds it and writes remaining and escrow as absolute values, so two fills of one order in a batch would both read it unfilled. The order id preimage size names std::mem::size_of explicitly. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
thepastaclaw
left a comment
There was a problem hiding this comment.
Re-review — Final validation — Phase 1 + Phase 2
The marketplace implementation still has a blocking fill-path correctness issue: a seller whose valid credit balance is MAX_CREDITS - 1 can receive a payment that pushes the balance to MAX_CREDITS, causing Drive's internal balance-overflow error during operation application because fill validation never checks the maker's post-payment balance. The order-read fee aggregation also uses saturating arithmetic instead of the checked FeeResult API, which can silently undercharge on overflow. The supplied CI snapshot reports all listed checks successful; no local builds or tests were run per review constraints.
🔴 1 blocking | 🟡 1 suggestion(s)
Review provenance
Source: reviewer 1: muse-spark-1.3-contributor (agent: phase1-reviewer, role: general); reviewer 2: muse-spark-1.3-contributor (agent: phase1-reviewer, role: rust-quality); reviewer 3: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 4: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 5: gpt-6.1-sol (agent: phase2-reviewer, role: ffi-engineer); reviewer 6: gpt-6.1-sol (agent: phase2-reviewer, role: platform-versioning); reviewer 7: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 8: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)
- Triage:
criticalbygpt-6.1-sol(effort low) — This is a large, intricate change that modifies consensus-critical token state transitions and validation, moves tokens and credits through escrow, changes peer-facing serialization, and adds persistent storage and proof execution paths, notably across the token transition processors and Drive operation transformers. - Phase 1 reviewers:
muse-spark-1.3-contributor— general (completed, effort xhigh); agentphase1-reviewer,muse-spark-1.3-contributor— rust-quality (completed, effort xhigh); agentphase1-reviewer - Phase 1 model:
muse-spark-1.3-contributor— not quota-gated; passed overgemini-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 verifier:
gpt-6.1-sol— final-verifier; agentsol-verifier - Phase 2 reviewers:
gpt-6.1-sol— general (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— architecture-layering (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— ffi-engineer (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— platform-versioning (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— rust-quality (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— security-auditor (completed, effort xhigh); agentphase2-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/state_transition_action/action_convert_to_operations/batch/token/token_fill_ask_transition.rs`:
- [BLOCKING] packages/rs-drive/src/state_transition_action/action_convert_to_operations/batch/token/token_fill_ask_transition.rs:65-68: Reject fills that overflow the seller's credit balance
A successful fill unconditionally emits AddToIdentityBalance for the maker, but TokenFillAskTransitionAction state validation checks token pause/freeze and the buyer's frozen-account policy without checking the maker's credit balance. Identity balances may validly be MAX_CREDITS - 1, while update_identity_balance_operation_v0 rejects any resulting balance at or above MAX_CREDITS. Therefore a seller with MAX_CREDITS - 1 can place a valid listing whose fill pays one credit; the fill passes validation and is admitted, then operation application returns the internal CriticalBalanceOverflow error instead of a deterministic consensus refusal. Add a state-validation check for the maker's post-payment balance and return an appropriate consensus error before the action is accepted.
In `packages/rs-drive/src/state_transition_action/batch/batched_transition/token_transition/token_fill_ask_transition_action/v0/transformer.rs`:
- [SUGGESTION] packages/rs-drive/src/state_transition_action/batch/batched_transition/token_transition/token_fill_ask_transition_action/v0/transformer.rs:116: Use checked fee accumulation for order-document reads
The order lookup returns a processing cost, but this line adds it with saturating arithmetic. If the already-accrued processing fee and the order-read cost exceed u64::MAX, the result silently clamps and undercharges instead of returning the typed overflow error provided by FeeResult::checked_add_assign. The same pattern appears in the cancel-order transformer at line 99. Aggregate the read cost through the checked FeeResult API in both transformers.
The fill and cancel transformations add the order document read through FeeResult::checked_add_assign rather than a saturating add, so an overflow is an error instead of an undercharge. The fill's payment to the seller notes why it cannot pass MAX_CREDITS. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
thepastaclaw
left a comment
There was a problem hiding this comment.
Re-review — Final validation — Phase 1 + Phase 2
Static review of the exact head found no in-scope blocking defects. One non-blocking versioning/documentation issue remains: the proof verifier's shipped generations were extended with marketplace handling, but the PR's in-place-change rationale documents only the token-base validator; the available CI snapshot shows successful policy, Kotlin, and book checks, with no Rust code checks listed.
🟡 1 suggestion(s)
Review provenance
Source: reviewer 1: muse-spark-1.3-contributor (agent: phase1-reviewer, role: general); reviewer 2: muse-spark-1.3-contributor (agent: phase1-reviewer, role: rust-quality); reviewer 3: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 4: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 5: gpt-6.1-sol (agent: phase2-reviewer, role: ffi-engineer); reviewer 6: gpt-6.1-sol (agent: phase2-reviewer, role: platform-versioning); reviewer 7: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 8: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); 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: ffi-engineer); reviewer 12: gpt-6.1-sol (agent: phase2-reviewer, role: platform-versioning); reviewer 13: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 14: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)
- Triage:
criticalbygpt-6.1-sol(effort low) — This large, intricate diff changes consensus-critical token state transitions and funds movement across rs-dpp, rs-drive, and rs-drive-abci, including escrow, fills, cancellations, serialization, and protocol-version activation. - Phase 1 reviewers:
muse-spark-1.3-contributor— general (completed, effort xhigh); agentphase1-reviewer,muse-spark-1.3-contributor— rust-quality (completed, effort xhigh); agentphase1-reviewer - Phase 1 model:
muse-spark-1.3-contributor— not quota-gated; passed overgemini-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; agentsol-verifier - Phase 2 reviewers:
gpt-6.1-sol— general (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— architecture-layering (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— ffi-engineer (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— platform-versioning (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— rust-quality (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— security-auditor (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— general (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— architecture-layering (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— ffi-engineer (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— platform-versioning (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— rust-quality (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— security-auditor (completed, effort xhigh); agentphase2-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/verify/state_transition/verify_state_transition_was_executed_with_proof/v0/mod.rs`:
- [SUGGESTION] packages/rs-drive/src/verify/state_transition/verify_state_transition_was_executed_with_proof/v0/mod.rs:1361-1413: Document the in-place proof-verifier generation extension
This change adds PlaceAsk, CancelOrder, and FillAsk handling to the shared v0 proof-verifier implementation. The v1 dispatcher generation also calls this shared implementation, so the behavior of both shipped verifier generations changes even though v1 itself is not edited. That is inert for protocol versions before PV15 because the marketplace transition gate rejects these transitions, but the PR's "In-place changes to shipped generations" section currently documents only the token-base validator move. Add the proof-verifier generations and their safety rationale to that section, or introduce a new verifier generation selected by the PV15 Drive version.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…efused A token's decimals are a u8, but the order arithmetic needed 10^decimals to fit in u128, so every order of a token with 39 or more decimals was refused as worth more than an order holds. From 39 decimals a whole token is more base units than any amount times any price, so an order is now worth one credit rounded up and nothing rounded down. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
thepastaclaw
left a comment
There was a problem hiding this comment.
Re-review — Final validation — Phase 1 + Phase 2
The marketplace implementation is functionally coherent, and the supplied CI snapshot is green for all listed checks. One blocking architecture issue remains: order-ID derivation determines consensus storage and proof keys but is not selected through platform-version dispatch; two documentation suggestions also remain for the PV15 snapshot and the complete list of in-place v0 extensions.
🔴 1 blocking | 🟡 2 suggestion(s)
Review provenance
Source: reviewer 1: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 2: muse-spark-1.3-contributor (agent: phase1-reviewer, role: general); reviewer 3: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 4: muse-spark-1.3-contributor (agent: phase1-reviewer, role: rust-quality); reviewer 5: gpt-6.1-sol (agent: phase2-reviewer, role: ffi-engineer); reviewer 6: gpt-6.1-sol (agent: phase2-reviewer, role: platform-versioning); reviewer 7: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 8: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)
- Triage:
criticalbygpt-6.1-sol(effort low) — This is a large, intricate change that modifies consensus-critical token state transitions, escrowed asset and credit movement, serialization, storage operations, proof verification, and protocol-version activation. - Phase 1 reviewers:
muse-spark-1.3-contributor— general (completed, effort xhigh); agentphase1-reviewer,muse-spark-1.3-contributor— rust-quality (completed, effort xhigh); agentphase1-reviewer - Phase 1 model:
muse-spark-1.3-contributor— not quota-gated; passed overgemini-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 verifier:
gpt-6.1-sol— final-verifier; agentsol-verifier - Phase 2 reviewers:
gpt-6.1-sol— general (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— architecture-layering (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— ffi-engineer (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— platform-versioning (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— rust-quality (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— security-auditor (completed, effort xhigh); agentphase2-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-dpp/src/tokens/token_order_side.rs`:
- [BLOCKING] packages/rs-dpp/src/tokens/token_order_side.rs:68-92: Version the consensus order-ID derivation through platform-version tables
TokenOrderSide::order_id directly selects Document::generate_document_id_v0, but the resulting identifier is the committed order-document key, escrow key, proof path, and marketplace event identifier. Unlike the versioned Document::generate_document_id API, this helper accepts no PlatformVersion and has no version-table slot, so a future change to the order-ID preimage or derivation cannot be selected by protocol version and could make nodes disagree about storage keys and proofs. Add an order-ID method version to the DPP token version table, dispatch through an explicit v0 implementation selected by the PV15 tables, and thread the active PlatformVersion through the matching preimage-size, fee, Drive, proof, and event call sites.
In `packages/rs-platform-version/src/version/v15.rs`:
- [SUGGESTION] packages/rs-platform-version/src/version/v15.rs:186-201: Record the >=39-decimal order rule in the PV15 snapshot
The latest implementation gives tokens with 39 or more decimals an unrepresentable whole-token unit: a nonzero ask rounds up to one credit while a bid rounds down to zero. The PV15 note describes the signed order-value bound and rounded running total but does not record this specific replay behavior, even though it changes valid placement and fill outcomes. Add a numbered PV15 changelog statement documenting the >=39-decimal rule.
In `packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/validation/validate_basic_structure/v0/mod.rs`:
- [SUGGESTION] packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/validation/validate_basic_structure/v0/mod.rs:284-292: Document all in-place gated v0 generation extensions
The diff extends additional shipped v0 paths with marketplace arms, including BatchTransition::validate_base_structure_v0 here, the batch transformer, shielded-proof handling, outputs_only_bundle_tag_v0, and accepted-token-pool-nullifier state handling. The PR description documents the token-base validator and proof-verifier changes but does not enumerate these other in-place extensions. Expand the in-place-changes section to list them and explain that the PV15 is_allowed gate makes the new arms unreachable for protocol versions 1 through 14.
| impl TokenOrderSide { | ||
| /// The id of the order of this side that `maker` places for `token_id` with the identity | ||
| /// contract nonce `nonce`: known before the transition is broadcast, and unique, since the | ||
| /// nonce is used once. It is the id of the order's document in the token market contract and | ||
| /// the key of its escrow entry. The token id takes the contract id's place, as it does in | ||
| /// the ids of token history documents. | ||
| pub fn order_id( | ||
| &self, | ||
| token_id: Identifier, | ||
| maker: Identifier, | ||
| nonce: IdentityNonce, | ||
| ) -> Identifier { | ||
| Document::generate_document_id_v0( | ||
| &token_id, | ||
| &maker, | ||
| self.document_type_name(), | ||
| nonce.to_be_bytes().as_slice(), | ||
| ) | ||
| } | ||
|
|
||
| /// The number of bytes `order_id` hashes (twice, with SHA-256): the token id and the maker's | ||
| /// id (32 bytes each), the document type name and the nonce. | ||
| pub fn order_id_preimage_size(&self) -> usize { | ||
| 2 * 32 + self.document_type_name().len() + std::mem::size_of::<IdentityNonce>() | ||
| } |
There was a problem hiding this comment.
🔴 Blocking: Version the consensus order-ID derivation through platform-version tables
TokenOrderSide::order_id directly selects Document::generate_document_id_v0, but the resulting identifier is the committed order-document key, escrow key, proof path, and marketplace event identifier. Unlike the versioned Document::generate_document_id API, this helper accepts no PlatformVersion and has no version-table slot, so a future change to the order-ID preimage or derivation cannot be selected by protocol version and could make nodes disagree about storage keys and proofs. Add an order-ID method version to the DPP token version table, dispatch through an explicit v0 implementation selected by the PV15 tables, and thread the active PlatformVersion through the matching preimage-size, fee, Drive, proof, and event call sites.
source: gpt-6.1-sol (phase2-reviewer: architecture-layering)
There was a problem hiding this comment.
Not changed. The order id follows how Platform already derives the other ids it stores under, none of which takes a version slot until its derivation changes:
- token history documents (
TokenTransition::historical_document_id); - the ids a contract create checks (
Document::generate_document_id_v0invalidate_base_structure_v0); - contested and group action ids.
The _v0 in the name already pins the derivation. A later protocol version that wanted a different order id would add order_id_v1 and select it from that version's tables, exactly as it would add a version field. Nothing changes at any existing version until then. A version field selecting the only generation that exists would be plumbing with nothing to choose between.
It also isn't a cross-node risk today: every node at a given protocol version runs the same code, and changing this function without a new protocol version would be a consensus change that review must catch, with or without a table entry. If you still want a table slot now, it's a small follow-up, and I'd keep it to dpp's token method versions and the two placement transformers.
🤖 Addressed by Claude Code
There was a problem hiding this comment.
Still applies (re-reviewed at 848d3bcb): The order ID still calls generate_document_id_v0 directly from an unversioned helper, and no DPP token-version slot selects that generation. The _v0 suffix preserves today's algorithm but does not provide the table boundary required for a future order-ID generation.
…r more decimals are priced Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
thepastaclaw
left a comment
There was a problem hiding this comment.
Re-review — Final validation — Phase 1 + Phase 2
The marketplace implementation is internally coherent and the prior escrow, fee, proof, binding, and validation issues are addressed or tracked. Two consensus-critical behaviors introduced by this PR remain outside platform-version dispatch: order-ID derivation and order pricing/fill arithmetic; both can change storage identity or execution outcomes for existing protocol versions. Kotlin and policy checks are still pending, with no failed code check shown.
🔴 2 blocking
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: ffi-engineer); reviewer 4: muse-spark-1.3-contributor (agent: phase1-reviewer, role: general); 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: muse-spark-1.3-contributor (agent: phase1-reviewer, role: rust-quality); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)
- Triage:
criticalbygpt-6.1-sol(effort low) — This large, intricate diff changes consensus-critical token state transitions and storage-backed escrow flows that move tokens and credits, including validation, serialization, protocol activation, and execution logic. - Phase 1 reviewers:
muse-spark-1.3-contributor— general (completed, effort xhigh); agentphase1-reviewer,muse-spark-1.3-contributor— rust-quality (completed, effort xhigh); agentphase1-reviewer - Phase 1 model:
muse-spark-1.3-contributor— not quota-gated; passed overgemini-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 verifier:
gpt-6.1-sol— final-verifier; agentsol-verifier - Phase 2 reviewers:
gpt-6.1-sol— general (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— architecture-layering (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— ffi-engineer (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— platform-versioning (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— rust-quality (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— security-auditor (completed, effort xhigh); agentphase2-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-dpp/src/tokens/token_order.rs`:
- [BLOCKING] packages/rs-dpp/src/tokens/token_order.rs:67-141: Version token order pricing and fill arithmetic through platform-version tables
`total_at`, `fill_amount`, `fill_credits`, and `order_value` determine consensus outcomes: they select the amount filled, the credits transferred, the rounding behavior, and whether signed order terms are admissible. DPP calls `order_value` during unpaid structural validation, while Drive calls `fill_amount` and `fill_credits` during stateful transformation. None accepts `PlatformVersion` or has a method-version slot, so a future change to rounding, precision limits, or fill selection would modify the behavior of orders admitted under PV15 instead of preserving the old generation and selecting new behavior only for a newer protocol version. Add an order-math generation to the DPP token version tables, keep the current implementation as v0 selected by PV15, and pass the active version through DPP validation and Drive transformation.
In `packages/rs-dpp/src/tokens/token_order_side.rs`:
- [BLOCKING] packages/rs-dpp/src/tokens/token_order_side.rs:74-92: Version the consensus order-ID derivation through platform-version tables
(existing thread: https://github.com/dashpay/platform/pull/5529#discussion_r4371726120)
`TokenOrderSide::order_id` directly invokes `Document::generate_document_id_v0`, and `order_id_preimage_size` is likewise unversioned. The resulting identifier is the committed order-document key, escrow key, proof lookup key, and marketplace event identifier. Although the `_v0` call preserves the current bytes, it does not select that implementation from the active `PlatformVersion`; a future order-ID generation would require changing this helper and coordinating every caller manually, risking inconsistent storage and proof paths during replay. The repository's versioning conventions require shared consensus helpers to receive the active platform version and dispatch through a table-selected generation. Add an order-ID method slot to the DPP token versions, dispatch the current implementation as v0, and thread the active version through preimage sizing, fee calculation, Drive, proof, and event callers.
| fn total_at(filled: TokenAmount, price: Credits, unit: Option<u128>, round_up: bool) -> u128 { | ||
| // At most `i64::MAX * i64::MAX`, under `u128::MAX`. | ||
| let value = filled as u128 * price as u128; | ||
| match unit { | ||
| Some(unit) if round_up => value.div_ceil(unit), | ||
| Some(unit) => value / unit, | ||
| // A whole token is more base units than `u128` holds, so more than `value`: a nonzero | ||
| // total is under one credit, 1 rounded up and 0 rounded down. | ||
| None if round_up => u128::from(value > 0), | ||
| None => 0, | ||
| } | ||
| } | ||
|
|
||
| impl TokenOrder { | ||
| /// The amount filled so far. | ||
| pub fn filled(&self) -> TokenAmount { | ||
| self.amount.saturating_sub(self.remaining) | ||
| } | ||
|
|
||
| /// The amount a fill asking for between `min_amount` and `max_amount` takes, or `None` when | ||
| /// no amount in that range keeps the order's rules. | ||
| /// | ||
| /// The fill takes as much as it may: everything open, up to `max_amount`. When that would | ||
| /// leave less than `min_fill` open, it takes less, leaving exactly `min_fill`, if that still | ||
| /// takes at least `min_fill` and `min_amount`. A fill that does not take everything open takes | ||
| /// at least `min_fill`. | ||
| pub fn fill_amount( | ||
| &self, | ||
| min_amount: TokenAmount, | ||
| max_amount: TokenAmount, | ||
| ) -> Option<TokenAmount> { | ||
| let mut take = max_amount.min(self.remaining); | ||
| if take < self.remaining { | ||
| let left = self.remaining - take; | ||
| if left < self.min_fill { | ||
| take = self.remaining.checked_sub(self.min_fill)?; | ||
| } | ||
| if take < self.min_fill { | ||
| return None; | ||
| } | ||
| } | ||
| if take == 0 || take < min_amount { | ||
| return None; | ||
| } | ||
| Some(take) | ||
| } | ||
|
|
||
| /// The credits a fill of `fill_amount` moves: what a listing's buyer pays, rounded up, or | ||
| /// what an offer's seller receives, rounded down, as the change in the order's running total | ||
| /// at the order's own `decimals`. | ||
| pub fn fill_credits(&self, fill_amount: TokenAmount) -> Result<Credits, ProtocolError> { | ||
| let unit = unit(self.decimals); | ||
| let filled = self.filled(); | ||
| let filled_after = filled | ||
| .checked_add(fill_amount) | ||
| .ok_or(ProtocolError::Overflow( | ||
| "a fill takes more than an order holds", | ||
| ))?; | ||
| let round_up = matches!(self.side, TokenOrderSide::Ask); | ||
| let before = total_at(filled, self.price, unit, round_up); | ||
| let after = total_at(filled_after, self.price, unit, round_up); | ||
| Credits::try_from(after - before) | ||
| .map_err(|_| ProtocolError::Overflow("a fill moves more credits than fit in u64")) | ||
| } | ||
|
|
||
| /// What an order of `amount` base units at `price` credits per whole token is worth, rounded | ||
| /// up, for a token with `decimals` decimals: everything its buyers pay for a listing, and the | ||
| /// escrow an offer holds. `None` when it is more credits than an order holds (`i64::MAX`). | ||
| pub fn order_value(amount: TokenAmount, price: Credits, decimals: u8) -> Option<Credits> { | ||
| let value = total_at(amount, price, unit(decimals), true); | ||
| if value > i64::MAX as u128 { | ||
| None | ||
| } else { | ||
| Some(value as Credits) | ||
| } |
There was a problem hiding this comment.
🔴 Blocking: Version token order pricing and fill arithmetic through platform-version tables
total_at, fill_amount, fill_credits, and order_value determine consensus outcomes: they select the amount filled, the credits transferred, the rounding behavior, and whether signed order terms are admissible. DPP calls order_value during unpaid structural validation, while Drive calls fill_amount and fill_credits during stateful transformation. None accepts PlatformVersion or has a method-version slot, so a future change to rounding, precision limits, or fill selection would modify the behavior of orders admitted under PV15 instead of preserving the old generation and selecting new behavior only for a newer protocol version. Add an order-math generation to the DPP token version tables, keep the current implementation as v0 selected by PV15, and pass the active version through DPP validation and Drive transformation.
source: gpt-6.1-sol (phase2-reviewer: general)
thepastaclaw
left a comment
There was a problem hiding this comment.
Re-review — Final validation — Phase 1 + Phase 2
The marketplace implementation correctly gates listings at protocol version 15 and addresses the previously reported escrow, pricing-scale, proof, fee, and validation issues. However, the newly consensus-critical order arithmetic and persisted order-ID derivation bypass the platform-version dispatch tables, preventing future protocol generations from selecting new behavior while preserving PV15 replay semantics.
🔴 2 blocking
2 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: general); reviewer 9: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 10: gpt-6.1-sol (agent: phase2-reviewer, role: platform-versioning); reviewer 11: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 12: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)
- Triage:
criticalbygpt-6.1-sol(effort low) — This large, intricate diff changes consensus-critical token state transitions and directly moves escrowed tokens and credits through placement, fills, and cancellations in rs-dpp and rs-drive-abci. - Phase 1 reviewers:
muse-spark-1.3-contributor— general (completed, effort xhigh); agentphase1-reviewer,muse-spark-1.3-contributor— rust-quality (completed, effort xhigh); agentphase1-reviewer - Phase 1 model:
muse-spark-1.3-contributor— not quota-gated; passed overgemini-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; agentsol-verifier - Phase 2 reviewers:
gpt-6.1-sol— general (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— architecture-layering (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— platform-versioning (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— rust-quality (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— security-auditor (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— general (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— architecture-layering (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— platform-versioning (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— rust-quality (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— security-auditor (completed, effort xhigh); agentphase2-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-dpp/src/tokens/token_order.rs`:
- [BLOCKING] packages/rs-dpp/src/tokens/token_order.rs:67-141: Version token order pricing and fill arithmetic through platform-version tables
(existing thread: https://github.com/dashpay/platform/pull/5529#discussion_r4372881399)
The consensus-critical `total_at`, `fill_amount`, `fill_credits`, and `order_value` helpers are direct, unversioned implementations. `order_value` is used by DPP structural validation, while the fill helpers determine the tokens and credits moved by Drive execution. The PV15 marketplace gate prevents these paths from running before PV15, but that gate is not a method-generation selector: a later protocol version cannot change rounding, minimum-fill selection, or order-value semantics while preserving the current PV15 implementation without modifying these shared helpers and all of their callers. Add a DPP token-order method slot with an explicit v0 implementation selected by the platform-version tables, then thread the active `PlatformVersion` through validation and transformation.
In `packages/rs-dpp/src/tokens/token_order_side.rs`:
- [BLOCKING] packages/rs-dpp/src/tokens/token_order_side.rs:74-92: Version the consensus order-ID derivation through platform-version tables
(existing thread: https://github.com/dashpay/platform/pull/5529#discussion_r4371726120)
`TokenOrderSide::order_id` directly selects `Document::generate_document_id_v0` and accepts no `PlatformVersion`; `order_id_preimage_size` is also unversioned. This identifier is the committed order-document key, token-escrow key, fill/cancel lookup key, proof path, and marketplace identifier. Naming the current implementation through a `_v0` document-ID call does not provide runtime table selection. Add an order-ID method slot with a frozen v0 dispatcher in the DPP token-version tables, and thread the selected generation through order-ID derivation, fee accounting, Drive lookups, proof generation and verification, and event construction.
thepastaclaw
left a comment
There was a problem hiding this comment.
Re-review — Final validation — Phase 1 + Phase 2
The marketplace implementation preserves the reviewed escrow, pricing-scale, proof, fee, and validation fixes, and the supplied CI snapshot has no failing code check related to these changes. Two consensus-critical helpers remain outside PlatformVersion dispatch: order economics and persistent order-ID derivation, so future protocol generations cannot select compatible behavior without relying on every caller being updated consistently.
🔴 2 blocking
2 carried-forward finding(s) already raised on this PR; not re-posting as new inline comments.
Review provenance
Source: reviewer 1: muse-spark-1.3-contributor (agent: phase1-reviewer, role: general); reviewer 2: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 3: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 4: gpt-6.1-sol (agent: phase2-reviewer, role: ffi-engineer); reviewer 5: muse-spark-1.3-contributor (agent: phase1-reviewer, role: rust-quality); reviewer 6: gpt-6.1-sol (agent: phase2-reviewer, role: platform-versioning); reviewer 7: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 8: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)
- Triage:
criticalbygpt-6.1-sol(effort low) — This large diff changes consensus-critical token validation and execution, moves user funds and token balances through escrow, alters persistent order storage and proofs, and adds protocol-version-gated state-transition serialization. - Phase 1 reviewers:
muse-spark-1.3-contributor— general (completed, effort xhigh); agentphase1-reviewer,muse-spark-1.3-contributor— rust-quality (completed, effort xhigh); agentphase1-reviewer - Phase 1 model:
muse-spark-1.3-contributor— not quota-gated; passed overgemini-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 verifier:
gpt-6.1-sol— final-verifier; agentsol-verifier - Phase 2 reviewers:
gpt-6.1-sol— general (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— architecture-layering (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— ffi-engineer (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— platform-versioning (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— rust-quality (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— security-auditor (completed, effort xhigh); agentphase2-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-dpp/src/tokens/token_order.rs`:
- [BLOCKING] packages/rs-dpp/src/tokens/token_order.rs:67-141: Version token order pricing and fill arithmetic through platform-version tables
(existing thread: https://github.com/dashpay/platform/pull/5529#discussion_r4372881399)
The consensus-visible `fill_amount`, `fill_credits`, `total_at`, and `order_value` helpers determine how many tokens and credits a fill moves, how rounding works, and whether an order is representable. They accept no `PlatformVersion` and are not selected from the DPP token version tables. Although the current marketplace callers are PV15-gated v0 paths, that gate only controls whether the transition kind is admitted; it does not provide a version selector for these shared rules. A later protocol generation changing rounding, overflow handling, or fill selection would therefore require coordinated ad hoc changes across DPP validation and Drive transformation and could accidentally alter the behavior of the existing generation. Add an explicit token-order method version with a v0 implementation and dispatch it through the active platform version in every validation and transformation caller.
In `packages/rs-dpp/src/tokens/token_order_side.rs`:
- [BLOCKING] packages/rs-dpp/src/tokens/token_order_side.rs:68-92: Version the consensus order-ID derivation through platform-version tables
(existing thread: https://github.com/dashpay/platform/pull/5529#discussion_r4371726120)
`TokenOrderSide::order_id` directly calls `Document::generate_document_id_v0` without receiving or consulting `PlatformVersion`. This identifier is the committed order-document key, escrow key, proof lookup key, and marketplace identifier, so it is consensus-visible storage identity rather than a display-only helper. The `_v0` suffix names an implementation but does not select it from the protocol tables; callers in Drive transformation, proof paths, and event-related code continue to use the same direct method. Add an order-ID method slot and dispatcher with an explicit v0 implementation, then use the selected generation consistently for preimage sizing, fee calculation, Drive transformation, proof verification, and all other consumers.
…ion 15 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The token's trade mode carries the minimum fill (Tradeable { minFill }), which
every fill reads as the token has it then; a listing of less is refused. A
listing no longer signs minFill or decimals: it records the token's decimals
when placed, as the token market contract's generatedFrom declares. The value
check moves to the batch's advanced structure, which knows the token.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…igit number Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
thepastaclaw
left a comment
There was a problem hiding this comment.
Re-review — Final validation — Phase 1 + Phase 2
The marketplace implementation covers escrow conservation, paid failures, proof roots and guarantees, bounds, frozen-account behavior, and same-block fills. Four in-scope blocking issues remain: consensus-critical order arithmetic and order-ID derivation bypass PlatformVersion dispatch, and a signed placement can be repriced by a token-conventions update before inclusion; the approval summary also omits the effective pricing scale. Kotlin SDK checks and the book preview remain pending, with no failed code checks in the supplied CI snapshot.
🔴 4 blocking | 🟡 1 suggestion(s)
1 finding(s) not shown inline (the lines are not part of this PR's diff)
🟡 Suggestion: Preserve Tradeable minFill in mobile token models
packages/swift-sdk/Sources/SwiftDashSDK/Persistence/Types/TokenTypes.swift:247-257
The contract JSON now carries the economically relevant minFill, but the Swift model intentionally reduces Tradeable to the variant name, and the Kotlin entity/materializer make the same choice. A persisted mobile token therefore cannot report the minimum fill that governs marketplace operations; consumers may display incomplete rules or construct fills that are predictably rejected. Persist the minimum-fill value in the mobile models with the required schema migration, or provide a canonical API that exposes it without discarding it during materialization.
source: gpt-6.1-sol (phase2-reviewer: general, architecture-layering, platform-versioning, rust-quality, security-auditor)
4 carried-forward finding(s) already raised on this PR; not re-posting as new inline comments.
Review provenance
Source: reviewer 1: muse-spark-1.3-contributor (agent: phase1-reviewer, role: general); reviewer 2: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 3: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 4: gpt-6.1-sol (agent: phase2-reviewer, role: platform-versioning); reviewer 5: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 6: muse-spark-1.3-contributor (agent: phase1-reviewer, role: rust-quality); reviewer 7: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)
- Triage:
criticalbygpt-6.1-sol(effort low) — This large, intricate diff changes consensus-critical token state transitions and validation plus escrowed token and credit funds movement across rs-dpp, rs-drive-abci, and rs-drive. - Phase 1 reviewers:
muse-spark-1.3-contributor— general (completed, effort xhigh); agentphase1-reviewer,muse-spark-1.3-contributor— rust-quality (completed, effort xhigh); agentphase1-reviewer - Phase 1 model:
muse-spark-1.3-contributor— not quota-gated; passed overgemini-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 verifier:
gpt-6.1-sol— final-verifier; agentsol-verifier - Phase 2 reviewers:
gpt-6.1-sol— general (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— architecture-layering (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— platform-versioning (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— rust-quality (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— security-auditor (completed, effort xhigh); agentphase2-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/swift-sdk/Sources/SwiftDashSDK/Persistence/Types/TokenTypes.swift`:
- [SUGGESTION] packages/swift-sdk/Sources/SwiftDashSDK/Persistence/Types/TokenTypes.swift:247-257: Preserve Tradeable minFill in mobile token models
The contract JSON now carries the economically relevant `minFill`, but the Swift model intentionally reduces `Tradeable` to the variant name, and the Kotlin entity/materializer make the same choice. A persisted mobile token therefore cannot report the minimum fill that governs marketplace operations; consumers may display incomplete rules or construct fills that are predictably rejected. Persist the minimum-fill value in the mobile models with the required schema migration, or provide a canonical API that exposes it without discarding it during materialization.
In `packages/rs-dpp/src/tokens/token_order.rs`:
- [BLOCKING] packages/rs-dpp/src/tokens/token_order.rs:93-141: Version token order pricing and fill arithmetic through platform-version tables
(existing thread: https://github.com/dashpay/platform/pull/5529#discussion_r4372881399)
The new `fill_amount`, `fill_credits`, and `order_value` helpers determine consensus-visible fill quantities, rounding, credit transfers, and representable-value bounds, but they accept no `PlatformVersion` and are absent from the DPP method-version tables. They are called from validation and the Drive transformation path, so changing rounding, minimum-fill handling, or high-decimal behavior later would change the behavior of the already-selected protocol-15 implementation instead of selecting a new generation. Add a frozen v0 implementation and PlatformVersion-selected dispatch, then thread the active version through every validation, transformation, fee, and proof caller.
In `packages/rs-dpp/src/tokens/token_order_side.rs`:
- [BLOCKING] packages/rs-dpp/src/tokens/token_order_side.rs:74-92: Version the consensus order-ID derivation through platform-version tables
(existing thread: https://github.com/dashpay/platform/pull/5529#discussion_r4371726120)
`TokenOrderSide::order_id` directly selects `Document::generate_document_id_v0`, and `order_id_preimage_size` fixes the associated hash-cost calculation without accepting a platform version or consulting a version table. The result is the persisted order-document key, escrow key, proof path, and marketplace identifier. A future change to the preimage or derivation would therefore require editing the behavior in place and could make nodes derive different storage locations for the same placement. Add a PlatformVersion-selected order-ID generation with an explicit v0 implementation and thread it through preimage sizing, fee derivation, placement, proof, query, and event paths.
In `packages/rs-drive/src/state_transition_action/batch/batched_transition/token_transition/token_place_ask_transition_action/v0/transformer.rs`:
- [BLOCKING] packages/rs-drive/src/state_transition_action/batch/batched_transition/token_transition/token_place_ask_transition_action/v0/transformer.rs:95-120: Bind the pricing scale into the seller's signed placement
(existing thread: https://github.com/dashpay/platform/pull/5529#discussion_r4347888980)
The placement transition signs only `amount` and `price`, while the transformer reads `decimals` from mutable token conventions immediately before creating the order document. A seller can sign while the token has 8 decimals, then an authorized conventions update can change it to 16 before inclusion; the same signed price is then interpreted at the new scale, changing the seller's economics without another signature. For example, 1,000 base units at 1,000,000,000 credits per whole token changes from 10,000 credits at 8 decimals to 1 credit at 16 decimals. Carry the intended scale in the signed transition and reject a mismatch before escrow creation, or otherwise make convention changes invalidate pending placements.
In `packages/rs-platform-wallet/src/wallet/identity/network/state_transition_summary.rs`:
- [BLOCKING] packages/rs-platform-wallet/src/wallet/identity/network/state_transition_summary.rs:600-604: Include the signed pricing scale in listing approval summaries
(existing thread: https://github.com/dashpay/platform/pull/5529#discussion_r4352824559)
The approval summary renders only `price: <value> credits per whole token`, while the current transition carries no signed scale and placement resolves the scale from mutable token state. Consequently, otherwise identical signing requests can result in materially different listing economics after a conventions update, with no approval-screen detail exposing that ambiguity. Bind the scale into the signed placement and render it in the summary, or show the exact trusted placement-time scale together with an execution-time agreement check.
Basic explanation
What this does: Lets holders of a tradeable token sell it to each other on Platform. A seller lists an amount at a price (
TokenPlaceAsk): the tokens leave their balance and wait in escrow. A buyer takes part or all of the listing (TokenFillAsk): the tokens go to the buyer and the credits go to the seller in the same block, so neither side has to trust the other. The seller can withdraw what is left at any time (TokenCancelOrder).Value: A token can now have a real secondary market without an off-chain exchange or a trusted middleman. Order books are readable with the document queries and proofs every SDK already has, because each listing is an
askdocument of the token market contract.Risks: Medium. Three new token transitions from protocol version 15. Before 15 each is refused unpaid at the batch's
is_allowedgate, as software that cannot decode them refuses them, so no earlier version changes. From 15 the tests check that token supply stays balanced through listings, partial fills, sell-outs and cancels, and that a fill can never move more than the listing holds or cost the buyer more than their limit.Issue being fixed or feature implemented
The third PR of the token marketplace stack (PV15), on top of #5526 (escrow trees and the token market contract). This one adds the sell side: listings. Offers (bids) come next.
What was done?
Transitions (
TokenTransitionvariants 18 to 20, appended)TokenPlaceAskamount,priceaskdocument createdTokenFillAskorderId,minAmount,maxAmount,maxTotalPriceaskrewritten or deletedTokenCancelOrderorderId,sidepriceis credits per whole token. The order id is derived from the token, the seller and the nonce, so a client knows it before placing.The minimum fill and the decimals are the token's. A listing signs neither:
TokenTradeMode::TradeablebecomesTradeable { min_fill }(feat(platform)!: tradeable tokens (PV15) #5524's variant, unreleased, changed in place). The minimum fill is from 1 toi64::MAX(InvalidTokenOrderFillRangeError10463 on contract create and update, and on aMarketplaceTradeModeconfiguration update).TokenConfiguration::min_fill()reads it.TokenOrder::fill_amounttakes it), so changing it applies to open listings too; a listing left under a raised minimum is still sold whole. A listing of less than it is refused (10463, the minimum and the amount).asktype declares themgeneratedFromsys.tokens.decimals(feat(platform)!: generatedFrom sys.tokens.decimals generates a token's decimals (PV15) #5604, feat(platform)!: token marketplace escrow trees and token market contract (PV15) #5526), and the placement transformer writes the token's current ones. A conventions update while a signed listing waits places it at the new decimals.Rounding: each fill pays the change in the listing's running total, rounded up, so splitting never changes the total:
Validation
validate_token_order_terms, checks a new order's terms, so an offer shares it;validate_token_order_terms_for_tokenchecks what it needs of the token (below). New basic errorsInvalidTokenOrderPriceError(10462),InvalidTokenOrderFillRangeError(10463) andInvalidTokenOrderValueError(10465); 10464 is unused (orders have no expiry).token_configuration_or_position_error, so a listing of a token its contract does not define bumps the nonce withContractHasNoTokensErrororInvalidTokenPositionErrorinstead of failing as an internal error, and a fill checks the same before reading the listing; the token base structure validation now reads the same helper.ask.decimals, from feat(platform)!: token marketplace escrow trees and token market contract (PV15) #5526) and every fill prices it at them, so an authorized conventions update cannot reprice an open listing. A fill reads the token's minimum fill, not its decimals.advanced_structure2 from feat(platform)!: tradeable tokens (PV15) #5524, per-action structure validations 0): the rules that read nothing beyond the contract and the block run here, so check_tx refuses them too. A listing needs a tradeable token, an amount of at least the token's minimum fill (10463) and a worth of at mosti64::MAXcredits at the token's decimals (10465), all paid; a fill a token that is still tradeable.TokenNotTradeableError(40750),TokenOrderNotFoundError(40751),TokenOrderFillAmountError(40753),TokenOrderPriceLimitError(40754),TokenOrderNotOwnedError(40755),TokenOrderSelfFillError(40756). 40752 is unused (orders have no expiry), and so is 40757 (the value limit is a structure check).TokenOrderDecimalsMismatchErroris defined but unused: orders name no decimals. The placement proof reads the decimals the order recorded.Execution (drive): two new token ops,
TokenPlaceInOrderEscrowandTokenReleaseFromOrderEscrow, over the escrow primitives of #5526; theaskdocument carries the seller's storage flags.Gate:
TokenTransition::is_marketplace_order(), an exhaustive classifier, drives the PV15is_allowedrefusal, so an order kind added later is gated without being listed.Proofs:
TokenPlaceAskproves theaskdocument and checks its terms. The order id follows from the token, the signer and the nonce, so only this transition creates a document with it: one holding its terms proves the placement after partial fills too, withremainingfrom the amount down to 1, so a client asking for the proof after a fill still confirms it (a listing filled entirely is proved through its history record from #5531);TokenFillAskthe buyer's token balance;TokenCancelOrderthe order's absence. Drive'sverifyfeature now enablesdpp/token-market-contract, so a verify-only build (wasm-drive-verify) has the contract the order proofs read:cargo tree -p wasm-drive-verify -e normal,features -i data-contractslistsdata-contracts feature "token-market"with the change and not without it.Events:
TokenEvent::OrderPlaced,OrderFilled,OrderCancelled(appended, with JSON shapes). History documents come in a later PR, so these transitions do not write token history yet.Bindings: wasm-dpp2
TokenPlaceAskTransition,TokenFillAskTransition,TokenCancelOrderTransitionand the three event variants (every integer setter takes a BigInt throughtry_to_u64and leaves the transition unchanged when it is out of range); wasm-dpp reports the kinds and points to wasm-dpp2; the platform wallet's signing summary shows each.Docs: book
data-model/token-marketplace.md(listings, rounding, rules, storage, proofs) and v15 note 7.How Has This Been Tested?
cargo test -p drive-abci --lib -- batch::tests::token::marketplace(16 tests, PV15 blocks):askholds the terms, the proof verifies;cargo test -p drive-abci --lib -- token_marketplace_gate: each order kind refused before 15 under its name, allowed from 15.Review round 2, in
batch::tests::token::marketplace:InvalidTokenPositionErrorandContractHasNoTokensError;UnpaidConsensusError(IdentityInsufficientBalanceError), with their credits, nonce, token balance and the listing unchanged.yarn workspace @dashevo/wasm-dpp2 exec mocha tests/unit/TokensTransitions.spec.ts(81 passing): each listing setter sets a value in range and throws on2^64and-1, keeping the previous value.cargo test -p drive-abci --lib -- batch::tests::token(regressions in the existing token tests). Round 2:batch::tests::token --skip shielded(264 passed),batch::tests::token::marketplace(20).cargo test -p dpp --features all_features --lib -- token_order consensus::codes state_error basic_error token_event marketplace_rules.cargo test -p drive --lib -- state_transition_action::batch order_escrow verify::state_transition.cargo test -p wasm-dpp2 --lib -- token_event(documented event fields match the serializer).cargo clippyon dpp, drive, drive-abci, wasm-dpp, wasm-dpp2, platform-wallet, platform-version.Review round 2, in
batch::tests::token::marketplace:should_refuse_a_listing_signed_at_other_decimals: a holder other than the conventions controller signs a listing at 8 decimals, the contract owner changes the token to 16, and the original listing is refused by check_tx and in a block withTokenOrderDecimalsMismatchError(40758), nothing escrowed; without the check it would execute;should_refuse_filling_a_frozen_makers_listing_and_let_the_maker_cancel: a fill of a frozen maker's listing is refused with the maker'sIdentityTokenAccountFrozenError, balances and escrow unchanged, and the frozen maker cancels and gets the escrow back;should_refuse_a_frozen_buyers_fill_unless_the_token_allows_transfers_to_frozen_balances: refused with the buyer'sIdentityTokenAccountFrozenErrorwhen the token disallows it, executed into the frozen balance when it allows it;ProofError::IncorrectProof;decimalsis required, settable within a byte, and refused outside it.Review round 4 (the decimals part superseded in round 5): the wallet's listing row names the signed decimals (two listings differing only in them render differently); a listing of two whole tokens at
i64::MAXcredits at 0 decimals is refused unpaid withInvalidTokenOrderValueError(10465) by check_tx and in a block (dpp unit test at the boundary too); every marketplace proof round trip compares its root with the committed Drive root; a fill whose rounded running total does not change (0.5 credits per base unit, the second of three units) pays 0 credits, takes its unit, advances the nonce and charges its fee.Round 5 (minimum fill and decimals from the token):
should_refuse_a_listing_of_less_than_the_tokens_minimum_fill: 99 at a minimum of 100 is refused with 10463 by check_tx and in a block; 100 is placed.should_fill_open_listings_by_the_tokens_current_minimum_fill: the owner raises the minimum from 1 to 600 after a listing of 1,000; a fill of 500 is refused (TokenOrderFillAmountErrornaming 600), one of everything executes.i64::MAX + 1refused),fill_amountwith the token's minimum.cargo test -p drive-abci --lib -- tests::token::marketplace is_allowed contract_structure_error tests::token::non_transferable(71 passed),cargo test -p drive --lib -- order_escrow verify token(978),cargo test -p dpp --all-features --lib -- token_order token_place_ask token_configuration token_marketplace_rules token_configuration_item(227),cargo test -p platform-wallet --lib state_transition_summary.TokenMaterializerTradeModeTestpasses; the SwiftDataContractParserTradeModeTestswere updated but not run (no xcframework in this worktree).In-place changes to shipped generations
TokenTradeMode::Tradeablegains itsmin_fillfield in place. The variant is feat(platform)!: tradeable tokens (PV15) #5524's, admitted only from protocol version 15 (refused as not active before), so no stored contract or configuration update holds it;NotTradeablekeeps its tag and its JSON form.The token base transition action's advanced structure validation (
structure_v0, every protocol version) now reads its rule throughTokenBaseTransitionActionAccessorsV0::token_configuration_or_position_error. The helper holds the same condition and builds the same two errors with the same fields, so every result is unchanged; it moved so the listing transformer can refuse an unknown token with the same error.The state transition proof paths:
verify_state_transition_was_executed_with_proofv0, which its v1 dispatcher generation also calls, the execution path queries a node proves a transition with, and the proof guarantee classifier gain arms forTokenPlaceAsk,TokenFillAskandTokenCancelOrder. Every arm for an existing kind is unchanged, so a proof of any transition an earlier protocol version admits verifies, or is refused, exactly as before. The new arms are reached only by the marketplace kinds, which the batch'sis_allowedgate refuses before protocol version 15, so no earlier version can execute one for a client to verify. These paths are also not consensus: they build and check proofs for clients, and no node's state or result depends on them. A new verifier generation would only duplicate the dispatcher without changing what any version verifies.The batch's other shipped v0 paths gain arms for
TokenPlaceAsk,TokenFillAskandTokenCancelOrderonly:BatchTransition::validate_base_structure_v0(each kind's structure validation), the batch transformer v0 (each kind's transformation), the batch state validation v0 (each kind's state validation),outputs_only_bundle_tag_v0in the shielded-proof processor and the accepted token pool nullifiers of the state validation v0 (both list the three kinds among those that touch no shielded pool). No arm of an existing kind changes. The batch'sis_allowedgate refuses the three kinds, unpaid, before protocol version 15, so on protocol versions 1 through 14 none of the new arms is reached and every block replays as before.Breaking Changes
Consensus, from protocol version 15: three new token transitions, new consensus errors and new token events. Earlier protocol versions are unchanged: the new kinds are refused there before anything else runs.
Wire and API, all unreleased (protocol version 15):
TokenTradeMode::TradeableisTradeable { min_fill }; its JSON form is{ "Tradeable": { "minFill": n } }, no longer the string"Tradeable". A downstreammatchneedsTradeable { .. }. wasm-dpp2'sTokenTradeMode.Tradeable(minFill)takes the minimum and has aminFillgetter.TokenOrderlosesmin_fill;fill_amounttakes the token's minimum fill.Checklist:
structure.rs, regeneratedgrovedb-structure.json, and checked the structure viewer link posted on this pull requestFor repository code-owners and collaborators only
🤖 Generated with Claude Code