Skip to content

feat(platform)!: token marketplace offers (PV15) - #5530

Open
QuantumExplorer wants to merge 55 commits into
claude/token-market-listingsfrom
claude/token-market-offers
Open

QuantumExplorer wants to merge 55 commits into
claude/token-market-listingsfrom
claude/token-market-offers

Conversation

@QuantumExplorer

@QuantumExplorer QuantumExplorer commented Oct 21, 2026 •

Copy link
Copy Markdown
Member

Basic explanation

What this does: Lets anyone make an offer for a tradeable token on Platform. A buyer offers a price for an amount (TokenPlaceBid): what the offer is worth leaves their credit balance and waits in escrow. A holder sells into it (TokenFillBid): the tokens go to the buyer and the seller is paid out of the escrow in the same block. The buyer can withdraw the offer at any time (TokenCancelOrder with the bid side) and gets back what is left.

Value: Completes the order book: holders can list (the previous PR) and buyers can bid, so a token can be priced from both sides without trusting anyone. Offers are bid documents of the token market contract, readable with the queries and proofs SDKs already have, and the depth at each price is one proven sum of what the bids hold in escrow.

Risks: Medium. Two new token transitions and a new use of the credit escrow, from protocol version 15 only; before 15 both are refused unpaid at the batch's is_allowed gate. The tests check each credit movement exactly (the escrow, each payment, the rounding left over and the fees) and that a sale can never pay more than the offer holds.

Issue being fixed or feature implemented

The fourth PR of the token marketplace stack (PV15), on top of #5529 (listings). Listings let holders sell at their price; this adds the other side, where buyers name the price.

What was done?

Transitions (TokenTransition variants 21 and 22, appended)

Transition Fields Effect
TokenPlaceBid amount, price, decimals, minFill Buyer's credits to escrow (what the offer is worth, rounded up), bid document created
TokenFillBid orderId, minAmount, maxAmount, minTotalReceived Seller's tokens to the buyer, escrow credits to the seller, bid rewritten or deleted
TokenCancelOrder (bid side) orderId, side Escrow back to the buyer, offer deleted
Before: a buyer cannot make an offer for a token on Platform; TokenCancelOrder of a bid finds none.

After, an offer for 1,000 base units at 10 credits each:
  place                          buyer -10,000 credits (plus fee), 10,000 in escrow
  sell 400, minimum 4,000        seller -400 tokens, +4,000 credits; buyer +400 tokens; escrow 6,000
  sell up to 10,000, minimum 1   takes the 600 still wanted; escrow empty, the offer closes and the
                                 buyer gets its storage back

Rounding: an offer holds its value rounded up, each sale is paid the change in its running total rounded down, and what is left goes back to the buyer when it closes:

1.5 credits per base unit, an offer for 7 holds 11:  sales of 3, 3, 1 are paid 4 + 5 + 1, and 1 credit returns

Validation

  • Structure (dpp): the same validate_token_order_terms as a listing (feat(platform)!: token marketplace listings (PV15) #5529); a sale's range like a listing fill. An offer has no expiry: it stays open until it is filled entirely or cancelled.
  • Transformer (drive): an offer's escrow is worked out here at the decimals it signs. One worth more than i64::MAX credits never reaches it: the shared validate_token_order_terms refuses it with the transition's structure, unpaid and in check_tx, with InvalidTokenOrderValueError (10465). The configuration is read through token_configuration_or_position_error (feat(platform)!: token marketplace listings (PV15) #5529), so an offer naming a token its contract does not define bumps the nonce with ContractHasNoTokensError or InvalidTokenPositionError; a sale checks the same before reading the offer.
  • Signed decimals: TokenPlaceBidTransitionV0 signs the decimals its per-whole-token price is at, as a listing does in feat(platform)!: token marketplace listings (PV15) #5529. 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: otherwise whoever controls the token's conventions could change them between signing and execution and make the buyer escrow far 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 placement proof compares them with the order's.
  • Decimals: an offer records the token's decimals when it is placed (bid.decimals), and its escrow and every sale into it are worked out at them. A conventions update after placement cannot reprice an open offer or drain its escrow early; a sale reads no token configuration. A sale reads the offer and refuses not found, self fill, fill range and a payment below minTotalReceived (TokenOrderPriceLimitError, whose doc already covers a sale's minimum).
  • Advanced structure (batch advanced_structure 2): a tradeable token for an offer, a still tradeable token for a sale.
  • State: not paused; for an offer the buyer not frozen unless the token allows transfers to frozen balances (a sale moves tokens to them); for a sale the seller not frozen and holding the amount, the buyer not frozen unless allowed.
  • The offer's escrow counts in the batch's used balances, so fee validation checks the buyer can pay it.

Execution (drive): the credits move through identity balance operations, so the batch's balance merging sees them, and two new token ops hold and set the offer's escrow (TokenHoldCreditsInOrderEscrow, TokenSetCreditsInOrderEscrow); the seller's tokens move with the existing TokenTransfer op. TokenBalanceKey::CreditOrderEscrow joins the repeated-write guard.

Proofs: TokenPlaceBid proves the bid document and checks its terms, after sales too: remaining from the amount down to 1 and an escrow equal to what the offer is worth less what was paid for what is filled (TokenOrder::escrow_after_fills), so a client asking for the proof after a sale still confirms the placement (an offer filled entirely is proved through its history record from #5531); TokenFillBid the seller's token balance; the cancel arm already proves the absence for either side.

Bindings and docs: wasm-dpp2 TokenPlaceBidTransition, TokenFillBidTransition (every integer setter takes a BigInt through try_to_u64 and leaves the transition unchanged when it is out of range); wasm-dpp kinds; wallet signing summary rows; book "Offers" section; v15 note 8. Structure validation unit tests for all four order kinds.

How Has This Been Tested?

  • cargo test -p drive-abci --lib -- batch::tests::token::marketplace (36 tests, 16 for offers):
    • an offer holds its value in escrow, the buyer pays exactly escrow plus fee, the bid holds the terms, the proof verifies;
    • a partial sale then the rest: exact seller payments (payment minus the reported fee), the buyer unchanged by sales, escrow and document follow, supply balanced, the sale proof verifies; the buyer gets exactly the storage refund on close;
    • running-total rounding: payments 4, 5, 1 and the 1 credit left over returned;
    • cancel returns exactly what the escrow held, the absence proof verifies;
    • refusals: not tradeable and value too large (unpaid, check_tx too), buyer cannot pay, selling into one's own offer, below the seller's minimum, more than the seller holds, not the owner, paused (cancel still works).
  • cargo test -p drive-abci --lib -- token_marketplace_gate: both new kinds refused before 15 under their names, allowed from 15.
  • Review round 1, in batch::tests::token::marketplace::offers:
    • placement at 8 decimals, a sale of 100, a conventions update to 2, a second sale of 100: paid 1,000 credits at the offer's 8 decimals (at 2 it would take 1,000,000,000 out of an escrow of 9,000), then the remaining 800 sell for exactly the 8,000 left; an offer placed after the update records 2 and holds 100,000,000 for 10 base units;
    • an offer naming position 1 of a one-token contract or DPNS (no tokens), and a sale naming position 1, are refused paid with InvalidTokenPositionError / ContractHasNoTokensError by check_tx and in a block, the escrow untouched;
    • a buyer short of the fee and a buyer short of the escrow are each refused with exactly one UnpaidConsensusError(IdentityInsufficientBalanceError), credits and nonce unchanged.
  • offers::should_refuse_an_offer_signed_at_other_decimals: the buyer signs at 8 decimals, the contract owner changes the token to 4, and the offer is refused by check_tx and in a block with TokenOrderDecimalsMismatchError (40758), no escrow; without the check it executes and escrows 100,000,000 credits.
  • yarn workspace @dashevo/wasm-dpp2 exec mocha tests/unit/TokensTransitions.spec.ts: each offer and sale setter sets a value in range and throws on 2^64 and -1, keeping the previous value.
  • cargo test -p drive-abci --lib -- batch::tests::token (regressions).
  • cargo test -p dpp --features all_features --lib -- validate_structure::v0::tests token_order.
  • cargo test -p drive --lib -- state_transition_action::batch order_escrow verify::state_transition drive_op_batch; cargo test -p wasm-dpp2 --lib.
  • cargo clippy on dpp, drive, drive-abci, wasm-dpp, wasm-dpp2, platform-wallet, platform-version.

In-place changes to shipped generations

Every change below adds arms for TokenPlaceBid and TokenFillBid only. No arm of an existing transition kind changes, and the batch's is_allowed gate refuses both 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.

  • Batch validation and execution, v0 paths selected at every protocol version: 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) and its accepted token pool nullifiers, and outputs_only_bundle_tag_v0 in the shielded-proof processor (both list the two kinds among those that touch no shielded pool).
  • BatchTransitionAction v0's used balances: a TokenPlaceBid reports the escrow it moves out of the buyer's balance, so the fee check counts it as principal. Only a placed offer reaches the arm.
  • Version tables: DRIVE_ABCI_VALIDATION_VERSIONS_V1 to V10 gain token_place_bid_transition_* and token_fill_bid_transition_* structure and state validation fields, and DRIVE_STATE_TRANSITION_METHOD_VERSIONS_V1 to V4 gain token_place_bid_transition and token_fill_bid_transition, all 0. Only the new arms read them, so before protocol version 15 they are never read.
  • Proof paths: verify_state_transition_was_executed_with_proof v0 (which its v1 dispatcher generation also calls), the execution path queries a node proves a transition with, and the proof guarantee classifier gain arms for the two kinds. A proof of any transition an earlier protocol version admits verifies, or is refused, exactly as before. These paths are not consensus: they build and check proofs for clients, and no node's state or result depends on them.
  • The batch's advanced structure validation 2, which also gains the two kinds, is selected only by protocol version 15, so it is not a shipped generation.

Breaking Changes

Consensus, from protocol version 15: two new token transitions, and TokenCancelOrder now withdraws offers. Earlier protocol versions are unchanged: the new kinds are refused there before anything else runs.

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

Buyers can offer credits for an amount of a tradeable token (TokenPlaceBid),
holders can sell into an open offer (TokenFillBid), and TokenCancelOrder now
withdraws offers too. An offer holds what it is worth, rounded up, in the
prefunded specialized balances escrow; each sale is paid the change in the
offer's running total rounded down, and whatever the rounding leaves goes back
to the buyer when the offer closes.

TokenTransition variants 21-22, refused unpaid before protocol version 15 by
the is_allowed gate; the tradeable and expiry rules run with batch advanced
structure 2 (check_tx too), the offer's value in its transformer, paused,
frozen and seller balance in state validation. The escrow counts as a
purchase in the batch's used balances. Proofs, wasm-dpp2 bindings, wallet
summary rows, the book and v15 note 8, plus structure validation unit tests
for every order kind.

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

coderabbitai Bot commented Oct 21, 2026 •

Copy link
Copy Markdown
Contributor

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

Run configuration

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

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

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

❤️ Share

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

@thepastaclaw

thepastaclaw commented Oct 21, 2026 •

Copy link
Copy Markdown
Collaborator

⛔ Final review complete — 1 blocking finding(s) (commit 9b7f60f) · triage: critical

@github-actions

github-actions Bot commented Oct 21, 2026 •

Copy link
Copy Markdown
Contributor

📖 Book Preview built successfully.

Download the preview from the workflow artifacts.
To view locally: download the artifact, unzip, and open index.html.

Updated at 2026-10-27T00:04:55.342Z

QuantumExplorer and others added 2 commits October 22, 2026 00:05
…offers

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

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.

Final validation — Phase 1 + Phase 2

The bid implementation is broadly integrated with the existing versioned validation, escrow, proof, and operation pipelines. However, the reviewed range contains four blocking correctness issues: fills use mutable token decimals instead of the placement-time denomination, malformed expiries can surface as internal errors, missing token positions are dereferenced before consensus validation, and WASM setters silently narrow out-of-range bigint values. The provided CI checks pass, but none cover the changed Rust marketplace paths or WASM tests.

🔴 4 blocking | 🟡 1 suggestion(s)

Review provenance

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

  • Triage: critical by gpt-6.1-sol (effort low) — This large cross-cutting diff changes consensus-critical token state transitions and credit escrow funds movement across DPP, Drive, and rs-drive-abci, including protocol-version dispatch and proof execution paths.
  • 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 (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; 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 — ffi-engineer (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — platform-versioning (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — rust-quality (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — security-auditor (completed, effort xhigh); agent phase2-reviewer, gpt-6-astra — architecture-layering (completed, effort xhigh); agent phase2-reviewer, gpt-6-astra — platform-versioning (completed, effort xhigh); agent phase2-reviewer, gpt-6-astra — rust-quality (completed, effort xhigh); agent phase2-reviewer
  • 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/state_transition_action/batch/batched_transition/token_transition/token_fill_bid_transition_action/v0/transformer.rs`:
- [BLOCKING] packages/rs-drive/src/state_transition_action/batch/batched_transition/token_transition/token_fill_bid_transition_action/v0/transformer.rs:159-175: Do not reprice an open bid from mutable token decimals
  PlaceBid calculates and persists the escrow using the token's decimals at placement, but FillBid reads the current decimals at line 159 for every payout. Token conventions are mutable through an authorized `TokenConfigurationChangeItem::Conventions` update: the DPP update validator allows this when the change-control rules authorize it, and applying the item replaces the conventions object. If a bid for 1,000 base units at 100 credits per whole token is placed with two decimals, it holds 1,000 credits; changing the token to one decimal makes a 100-base-unit fill demand 1,000 credits instead of 100. A partial fill can therefore drain the entire escrow while leaving the order open, and a later fill can fail with the checked subtraction overflow. Persist the placement-time denomination or reject convention changes while bids are open, and exercise placement, partial fill, convention update, and subsequent fill in execution tests.

In `packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/batched_transition/token_place_bid_transition/validate_structure/v0/mod.rs`:
- [BLOCKING] packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/batched_transition/token_place_bid_transition/validate_structure/v0/mod.rs:36-41: Reject expiries that cannot be represented in the order document
  `expires_at` is an unrestricted `u64`, but `TokenOrder::properties()` stores it as a signed `i64`. This validator checks amount, price, and minimum fill but not the expiry range, so a valid-looking bid with `expires_at = Some(i64::MAX as u64 + 1)` reaches `TokenOrder::new_document()` and returns `ProtocolError::Overflow` before the later block-time expiry validation runs. That turns malformed user input into an internal processing error instead of a normal consensus rejection and nonce-bump result. Add the representability check in the DPP order-field validation, preferably shared with the ask validator, while retaining the separate block-relative expiry check; cover `i64::MAX` and `i64::MAX + 1` through check_tx and block processing.

In `packages/rs-drive/src/state_transition_action/batch/batched_transition/token_transition/token_place_bid_transition_action/v0/transformer.rs`:
- [BLOCKING] packages/rs-drive/src/state_transition_action/batch/batched_transition/token_transition/token_place_bid_transition_action/v0/transformer.rs:102-103: Validate token existence before dereferencing its configuration
  The base action construction copies `token_contract_position` without proving that the position exists. The bid transformer then calls `base_action.token_configuration()?` before the existing token-base consensus validator can return `ContractHasNoTokensError` or `InvalidTokenPositionError`. A bid naming a missing position therefore produces `ProtocolError::CorruptedCodeExecution` rather than the expected paid consensus rejection and nonce bump; the FillBid transformer has the same dereference at line 159. Move token-position validation ahead of configuration-dependent escrow and pricing calculations, or use a shared helper that returns the existing typed consensus errors, and cover both a tokenless contract and an invalid position in check_tx and block processing.

In `packages/wasm-dpp2/src/state_transitions/batch/token_transitions/token_fill_bid.rs`:
- [BLOCKING] packages/wasm-dpp2/src/state_transitions/batch/token_transitions/token_fill_bid.rs:139-145: Reject lossy bigint conversion in bid transition setters
  The constructors use `try_to_u64`, but the new setters accept raw `TokenAmount` and `Credits` aliases, which are `u64` values. At the JavaScript/WASM boundary an out-of-range bigint can therefore be narrowed modulo 2^64 instead of being rejected. Assigning `1n << 64n` to `minTotalReceived`, for example, silently stores zero and removes the seller's minimum-payout protection; the same issue applies to `minAmount` and `maxAmount` here and to `amount`, `price`, `minFill`, and `expiresAt` in `TokenPlaceBidTransitionWasm`. Match the existing checked setter pattern used by `TokenTransferTransitionWasm`: accept a `&js_sys::BigInt` (and the checked optional conversion for `expiresAt`), call `try_to_u64`, return `WasmDppResult<()>`, and leave the previous value unchanged on conversion failure.

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:1235-1242: Assert the specific insufficient-funds rejection
  This test only asserts that execution was not successful. It would also pass for an internal error, an unrelated consensus error, or an empty result list, so it does not prove that the bid escrow was included in the used-balance pre-check. Match the expected insufficient-balance consensus error and paid/unpaid classification, assert exactly one execution result, and check the relevant balance and nonce effects. The current fixture gives the buyer exactly the escrow principal, so add a case below the principal as well as the existing principal-plus-fee shortfall.
Out-of-scope follow-up suggestions (1)

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

  • Existing listing fills also use mutable token decimals — The pre-existing listing-side transformer at packages/rs-drive/src/state_transition_action/batch/batched_transition/token_transition/token_fill_ask_transition_action/v0/transformer.rs:159-160 also prices fills using live conventions. A conventions authority can therefore reprice an open ask after placement. This is the same underlying denomination issue but predates this bid PR.
    • Follow-up: Track a coordinated fix for ask and bid order denomination persistence, or address the listing-side exposure in a separate marketplace follow-up.

QuantumExplorer and others added 4 commits October 22, 2026 09:32
…offers

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- An offer records the token's decimals when it is placed; its escrow and
  every sale into it are worked out at them, so a conventions update cannot
  reprice an open offer or drain its escrow early. Sales read no token
  configuration.
- TokenPlaceBid validates its terms with the shared
  validate_token_order_terms, refusing an expiresAt above i64::MAX with
  InvalidTokenOrderExpiryError (10464).
- An offer or a sale naming a token its contract does not define bumps the
  nonce with ContractHasNoTokensError or InvalidTokenPositionError.
- The PlaceBid proof check also requires remaining == amount and an escrow
  equal to the offer's value at its decimals.
- wasm-dpp2 offer and sale 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 escrow.

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 306c99f, all five prior findings are fixed. One blocking issue remains: a successful bid placement cannot be confirmed with the current-state proof after a subsequent partial or complete fill. This was a static verification; current PR checks pass, but they do not provide dedicated Rust/WASM test or clippy results.

🔴 1 blocking

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: muse-spark-1.3-contributor (agent: phase1-reviewer, role: general); 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: critical by gpt-6.1-sol (effort low) — The large, cross-layer diff directly changes consensus-controlled funds movement in packages/rs-drive/src/state_transition_action/action_convert_to_operations/batch/token/token_fill_bid_transition.rs and the bid transformers, including escrow funding, settlement, rounding, and refunds.
  • 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 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 — ffi-engineer (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — platform-versioning (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — rust-quality (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — security-auditor (completed, effort xhigh); agent phase2-reviewer
🤖 Prompt for all review comments with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.

In `packages/rs-drive/src/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:1408-1416: Preserve placement confirmation after an offer is filled
  Placement verification requires the live offer to remain untouched, but confirmation does not retrieve a placement-time snapshot. DAPI's wait handler fetches the proof when servicing the request, `query_proofs_v0` calls `prove_state_transition(..., None, ...)`, and the order query requests the latest document without history. Place an offer for 1,000 units with 10,000 credits in escrow, sell 400 units into it, then wait for the placement result after reconnecting: the authentic document now has `remaining = 600` and `escrow = 6,000`, so these checks reject the successful placement. A complete fill deletes the document and instead fails the existence check above. The buyer has already funded the offer and received tokens, yet proof-backed confirmation returns an error. Preserve authenticated placement evidence through subsequent fills and closure, using durable execution evidence or an authenticated historical proof rather than requiring an untouched live order; absence alone must not establish placement. Add placement→partial-fill→placement-proof and placement→complete-fill→placement-proof regressions, and check the matching PlaceAsk mechanism when fixing the shared lifecycle assumption.

QuantumExplorer and others added 4 commits October 22, 2026 11:05
The PlaceBid proof accepted only an untouched bid. Like a listing, an
offer's order id is the transition's alone, so a bid holding its terms
proves the placement after sales: remaining from the amount down to 1, and
an escrow equal to what the offer is worth less what was paid for what is
filled (TokenOrder::escrow_after_fills).

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 findings and all six prior findings against a3ed2d0. Five prior findings are fixed; placement confirmation still fails after an offer closes, leaving one blocking issue that the linked history work does not resolve for history-disabled tokens. This was a static review: current GitHub checks pass, but no dedicated Rust test or clippy result is listed.

🔴 1 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: muse-spark-1.3-contributor (agent: phase1-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: gpt-6.1-sol (agent: phase2-reviewer, role: platform-versioning); reviewer 6: muse-spark-1.3-contributor (agent: phase1-reviewer, role: rust-quality); 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: critical by gpt-6.1-sol (effort low) — The large, intricate diff directly changes funds movement and consensus validation, notably in token_fill_bid_transition_action/v0/transformer.rs and action_convert_to_operations/batch/token/token_fill_bid_transition.rs, which calculate escrow payouts and apply token and credit transfers.
  • 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 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 — ffi-engineer (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — platform-versioning (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — rust-quality (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — security-auditor (completed, effort xhigh); agent phase2-reviewer
🤖 Prompt for all review comments with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.

In `packages/rs-drive/src/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:1403-1407: Preserve placement confirmation after an offer is filled
  (existing thread: https://github.com/dashpay/platform/pull/5530#discussion_r4345904240)
  The partial-fill fix works while the bid remains open, but this existence check still rejects placement confirmation after a complete fill or cancellation deletes the document. The regression at marketplace/mod.rs:1452–1454 explicitly expects that failure. DAPI obtains the proof when servicing the wait request, and query_proofs_v0 proves current state rather than a placement-time snapshot. Consequently, the buyer can successfully fund an offer and receive tokens yet be unable to confirm its placement; placement followed by a complete fill in the same block leaves no committed live-order state to retrieve even with an immediate request. PR #5531 is still open on top of this PR, and its described fallback only works when transfer history is enabled, leaving valid history-disabled bids affected. This is a defect in the newly added confirmation path, not an unrelated follow-up. Preserve authenticated placement evidence independently of optional transfer history, or provide an authenticated historical proof, and require successful confirmation in complete-fill and cancellation regressions. Absence alone must remain insufficient evidence of placement.

QuantumExplorer and others added 6 commits October 22, 2026 12:51
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>
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

Two blocking issues remain at the reviewed head. Placement confirmation still fails after a bid is completely filled because the live order document is deleted and no durable placement evidence is queried, while the wallet approval summary omits the signed denomination and resulting escrow commitment. The available CI snapshot reports successful Kotlin and book checks, but no Rust marketplace test result.

🔴 2 blocking

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

Review provenance

Source: reviewer 1: muse-spark-1.3-contributor (agent: phase1-reviewer, role: general); reviewer 2: 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: critical by gpt-6.1-sol (effort low) — This large, intricate diff changes consensus-critical token state transitions, credit escrow and settlement funds movement, signed transition serialization, validation gates, storage operations, and proof verification in files such as token_place_bid_transition, token_fill_bid_transition, and their rs-drive-abci validators.
  • 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 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 — ffi-engineer (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — platform-versioning (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — rust-quality (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — security-auditor (completed, effort xhigh); agent phase2-reviewer
🤖 Prompt for all review comments with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.

In `packages/rs-platform-wallet/src/wallet/identity/network/state_transition_summary.rs`:
- [BLOCKING] packages/rs-platform-wallet/src/wallet/identity/network/state_transition_summary.rs:624-630: Show the signed denomination and credit exposure before approving a bid
  The PlaceBid approval projection shows price, minimum fill, expiry, and the requested token count, but it omits the signed `decimals` field and leaves the credit amount unset. The denomination materially changes the escrow commitment: 1,000 base units at a price of 1,000,000,000 credits per whole token require 10,000 credits at 8 decimals but 100,000,000 credits at 4 decimals. Those requests otherwise render the same approval details, so a user cannot distinguish the credit exposure they are authorizing. The summary should display the signed decimals and the escrow commitment calculated with the canonical DPP `TokenOrder::order_value` logic; changing only decimals should have a regression showing a different approval summary.

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:1404-1409: Preserve placement confirmation after an offer is filled
  (existing thread: https://github.com/dashpay/platform/pull/5530#discussion_r4345904240)
  The partial-fill path now validates the live bid correctly, but a complete fill still removes the only placement evidence this verifier accepts. In the sequence PlaceBid → full FillBid → placement confirmation, `verify_token_order_document` returns `None` after the bid document is deleted and this branch returns `IncorrectProof`, even though the placement and sale succeeded. The marketplace regression at lines 1720–1722 explicitly asserts this failure. The proof-backed confirmation path must retain authenticated placement evidence through closure, such as an authenticated historical execution record; absence alone cannot prove that the placement occurred. The referenced #5531 is not part of this reviewed head.

QuantumExplorer and others added 2 commits October 22, 2026 19:23
…credits it escrows

The PlaceBid approval row left its credit amount unset and did not show the
signed decimals, so two offers committing 10,000 or 100,000,000 credits
(1,000 base units at 1,000,000,000 credits per whole token at 8 or 4
decimals) looked the same. The row's amount is now the escrow, worked out
with TokenOrder::order_value, with a detail line when Platform would refuse
it as too large, and the price line names the decimals (listings too).

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 findings against head 328b6ba: six prior findings are fixed, but placement confirmation after closure remains broken, and placement proofs also accept forbidden group metadata as execution-proved. One non-blocking coverage suggestion remains for bid-side frozen-account roles. The currently reported GitHub checks pass; this verification was static, with no local builds or tests run.

🔴 2 blocking | 🟡 1 suggestion(s)

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

Review provenance

Source: reviewer 1: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 2: muse-spark-1.3-contributor (agent: phase1-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: 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: critical by gpt-6.1-sol (effort low) — This large, intricate diff changes consensus-critical token transition rules and directly implements credit escrow, payment, refund, fee, and signature handling across DPP, Drive, and ABCI validation code.
  • 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 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 — ffi-engineer (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — platform-versioning (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — rust-quality (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — security-auditor (completed, effort xhigh); agent phase2-reviewer
🤖 Prompt for all review comments with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.

In `packages/rs-drive/src/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:1391-1403: Reject forbidden group metadata before proving a bid placement
  This arm does not reject base.using_group_info(). Adding group metadata to an otherwise matching placement leaves its derived order ID, document-term comparisons, and owner-balance query unchanged, so the authentic placement proof still passes. However, DPP's batch basic-structure validator rejects that request with GroupActionNotAllowedOnTransitionError because PlaceBid cannot calculate a group action ID. The classifier nevertheless labels PlaceBid as ExecutionProved unconditionally, and the SDK proof path performs no intervening structural validation, allowing a strict wait to accept terminal success for a request that could never execute. Reject prohibited group metadata at the Drive verification boundary and check the matching PlaceAsk path. Add a regression that verifies an authentic placement proof against the same transition with group metadata added. The guard present in the child #5531 is absent from this reviewed head.
- [BLOCKING] packages/rs-drive/src/verify/state_transition/verify_state_transition_was_executed_with_proof/v0/mod.rs:1404-1408: Preserve placement confirmation after an offer is filled
  (existing thread: https://github.com/dashpay/platform/pull/5530#discussion_r4345904240)
  Partial fills now preserve placement confirmation, but complete fills and cancellation delete the only evidence this verifier accepts. DAPI obtains the proof when servicing the wait request, and query_proofs_v0 proves current state rather than a placement-time snapshot. A buyer reconnecting after closure therefore cannot obtain proof-backed confirmation of a placement that successfully committed their credits; the new marketplace regression explicitly expects this failure after the final fill at line 1722. The linked #5531 is an open child PR, not part of this head, and its fallback explicitly rejects closed offers when transfer history is disabled, a configuration this PR supports. This is a failure introduced by the new placement path, so the tracked follow-up does not make it safely deferrable. Preserve authenticated placement evidence for every supported offer, or supply an authenticated execution-time proof that remains usable after closure, and cover both complete fill and cancellation with transfer history disabled.

In `packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/action_validation/token/token_fill_bid_transition_action/state_v0/mod.rs`:
- [SUGGESTION] packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/action_validation/token/token_fill_bid_transition_action/state_v0/mod.rs:86-94: Exercise the bid-side frozen-account roles
  FillBid applies different rules to the signer, who sells tokens, and the order maker, who receives them: the seller must always be unfrozen, while a frozen buyer may receive tokens when the configuration allows it. The marketplace frozen-account regressions exercise FillAsk, not the new FillBid validator, so swapping the participant identities or applying the recipient exception to the seller would not be detected by the offer suite. Add execution cases that freeze the buyer after placement under both values of is_allowed_transfer_to_frozen_balance, and cases that reject a frozen seller under either value. Assert that the typed error names the correct participant, rejected fills preserve token balances and escrow, and the frozen buyer can still cancel and recover the remaining escrow.

QuantumExplorer and others added 2 commits October 22, 2026 20:22
…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>
QuantumExplorer and others added 5 commits October 23, 2026 14:20
TokenPlaceBid loses expiresAt, as TokenPlaceAsk did: an offer is
withdrawn by cancelling it, and a sale into an offer no longer checks a
time. The offer expiry tests go with it.

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

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

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

The offer implementation has two blocking lifecycle defects. Fee validation checks only the identity's pre-event balance after outgoing principals, so bid cancellation refunds and FillBid sale proceeds cannot fund the same event's fee; strict placement confirmation also still fails after a complete fill because the live bid document is deleted and this head has no historical fallback. The available CI snapshot contains no failed checks, but local builds and tests were not run under the static-review restriction.

🔴 2 blocking

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

🔴 Blocking: Let bid refunds and sale proceeds fund the signer fee
packages/rs-drive-abci/src/execution/platform_events/state_transition_processing/validate_fees_of_event/v1/mod.rs:348-359

Fee validation computes balance_after_principal_operation from the identity's pre-event balance and removed_balance, then compares that amount with the estimated gas. removed_balance comes from BatchAction::all_used_balances, which includes the outgoing PlaceBid escrow but does not include the positive AddToIdentityBalance operations emitted by bid cancellation or FillBid. Those positive operations are applied only after this precheck. Consequently, a buyer who placed an offer with all available credits cannot cancel it when its free balance is zero, and a token-only seller cannot fill an offer when the escrow proceeds would otherwise cover the fee; both transitions are rejected as insufficient balance before their refund or payment can occur. Account for deterministic same-event balance outputs in the fee precheck and add cancellation and FillBid tests that start with zero free identity balance.

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

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

Review provenance

Source: reviewer 1: muse-spark-1.3-contributor (agent: phase1-reviewer, role: general); reviewer 2: 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: critical by gpt-6.1-sol (effort low) — This large, intricate diff changes consensus-critical token state transitions, credit escrow and payment movements, cryptographic signing data, serialization, and persistent storage/proof execution paths.
  • 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 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 — ffi-engineer (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — platform-versioning (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — rust-quality (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — security-auditor (completed, effort xhigh); agent phase2-reviewer
🤖 Prompt for all review comments with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.

In `packages/rs-drive-abci/src/execution/platform_events/state_transition_processing/validate_fees_of_event/v1/mod.rs`:
- [BLOCKING] packages/rs-drive-abci/src/execution/platform_events/state_transition_processing/validate_fees_of_event/v1/mod.rs:348-359: Let bid refunds and sale proceeds fund the signer fee
  Fee validation computes `balance_after_principal_operation` from the identity's pre-event balance and `removed_balance`, then compares that amount with the estimated gas. `removed_balance` comes from `BatchAction::all_used_balances`, which includes the outgoing PlaceBid escrow but does not include the positive `AddToIdentityBalance` operations emitted by bid cancellation or FillBid. Those positive operations are applied only after this precheck. Consequently, a buyer who placed an offer with all available credits cannot cancel it when its free balance is zero, and a token-only seller cannot fill an offer when the escrow proceeds would otherwise cover the fee; both transitions are rejected as insufficient balance before their refund or payment can occur. Account for deterministic same-event balance outputs in the fee precheck and add cancellation and FillBid tests that start with zero free identity balance.

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:1431-1464: Preserve placement confirmation after an offer is filled
  (existing thread: https://github.com/dashpay/platform/pull/5530#discussion_r4345904240)
  The PlaceBid proof arm queries only the current `bid` document and requires it to be present. A complete FillBid deletes that document and its escrow entry, while the current marketplace execution-path query also requests only the live order and does not include a historical placement record. Therefore, after a valid placement is completely filled before a strict wait request obtains its proof, verification returns an incorrect-proof error instead of `ExecutionProved`; the buyer has funded the offer and received the tokens but cannot distinguish successful execution from broadcast failure through the proof-backed API. The author-referenced history work in dashpay/platform#5531 is not present in this head, so it does not resolve the current PR's normal complete-fill lifecycle. Persist and query an immutable placement artifact bound to the signed order terms, or otherwise make complete-fill confirmation available before enabling this strict proof path.

@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

The reviewed head still has two blocking correctness issues. Fee validation checks only the signer's pre-transition balance minus removed principal, so bid refunds and FillBid proceeds cannot fund the same transition's fee; additionally, valid marketplace orders for tokens with 39–255 decimals are rejected because the shared order-value helper requires 10^decimals to fit in u128. The tracked complete-fill proof limitation remains intentionally deferred to #5531.

🔴 2 blocking

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

🔴 Blocking: Let bid refunds and sale proceeds fund the signer fee
packages/rs-drive-abci/src/execution/platform_events/state_transition_processing/validate_fees_of_event/v1/mod.rs:348-359

Fee validation computes the signer's available balance as balance - principal, without including positive identity-balance operations emitted by the same event. TokenFillBid credits the seller with sale proceeds, and bid cancellation credits the maker with the remaining escrow; execution applies those operations before charging the gas fee. Consequently, a seller whose only credits are the sale proceeds can be rejected as insufficiently funded, and a buyer whose credits are entirely in bid escrow can be unable to cancel and recover that escrow when it is needed to pay the fee. The new bid fill and cancellation paths make this mismatch reachable; fee validation must account for validated same-event identity credits while retaining the existing principal and key-budget protections.

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

Review provenance

Source: reviewer 1: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 2: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 3: muse-spark-1.3-contributor (agent: phase1-reviewer, role: general); reviewer 4: gpt-6.1-sol (agent: phase2-reviewer, role: ffi-engineer); 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: critical by gpt-6.1-sol (effort low) — This large, intricate diff changes consensus-critical token state transitions and directly moves credits into escrow, pays sellers, refunds buyers, and validates cryptographic signatures and proofs.
  • 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 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 — ffi-engineer (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — platform-versioning (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — rust-quality (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — security-auditor (completed, effort xhigh); agent phase2-reviewer
🤖 Prompt for all review comments with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.

In `packages/rs-drive-abci/src/execution/platform_events/state_transition_processing/validate_fees_of_event/v1/mod.rs`:
- [BLOCKING] packages/rs-drive-abci/src/execution/platform_events/state_transition_processing/validate_fees_of_event/v1/mod.rs:348-359: Let bid refunds and sale proceeds fund the signer fee
  Fee validation computes the signer's available balance as `balance - principal`, without including positive identity-balance operations emitted by the same event. `TokenFillBid` credits the seller with sale proceeds, and bid cancellation credits the maker with the remaining escrow; execution applies those operations before charging the gas fee. Consequently, a seller whose only credits are the sale proceeds can be rejected as insufficiently funded, and a buyer whose credits are entirely in bid escrow can be unable to cancel and recover that escrow when it is needed to pay the fee. The new bid fill and cancellation paths make this mismatch reachable; fee validation must account for validated same-event identity credits while retaining the existing principal and key-budget protections.

In `packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/batched_transition/token_place_bid_transition/validate_structure/v0/mod.rs`:
- [BLOCKING] packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/batched_transition/token_place_bid_transition/validate_structure/v0/mod.rs:14-19: Rejects valid bids for tokens with 39 or more decimals
  The bid structure validator routes otherwise valid terms through `TokenOrder::order_value`. That helper first computes `10u128.checked_pow(decimals)`, which fails at decimals 39 and above. The token-market contract accepts decimals through 255, and with positive amount and price terms each bounded by `i64::MAX`, the product is below 10^39; therefore the mathematically correct rounded-up order value at decimals 39–255 is representable (at most one credit), not an overflow. Such bids are nevertheless rejected as `InvalidTokenOrderValueError` before placement. The same shared arithmetic also affects the ask validator. Rework the rounded division so it does not require the full power of ten to fit in u128, and add coverage for decimals 39 and 255 for both order sides.

QuantumExplorer and others added 4 commits October 25, 2026 19:25
…refused

escrow_after_fills follows the u128-free unit of the listing fix, and
the offer structure validation accepts terms at their bounds at 39 and
255 decimals.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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

The offer implementation is otherwise well integrated: protocol gating, escrow accounting, decimal binding, proof guards, frozen-account rules, WASM conversions, wallet summaries, and regression coverage are present. One blocking proof-lifecycle defect remains: PlaceBid is classified as execution-proved but only accepts the live bid document, so a placement cannot be proof-confirmed after the offer is completely filled or cancelled. The fee-funding concern is withdrawn because the existing fee policy intentionally requires pre-transition funding and validation rejects the event before execution when that balance is absent.

🔴 1 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: muse-spark-1.3-contributor (agent: phase1-reviewer, role: general); reviewer 4: gpt-6.1-sol (agent: phase2-reviewer, role: ffi-engineer); 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: critical by gpt-6.1-sol (effort low) — This large, intricate diff changes consensus-critical token state transitions and credit escrow funds movement across DPP, Drive, ABCI validation, proofs, platform-version dispatch, and WASM bindings.
  • 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 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 — ffi-engineer (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — platform-versioning (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — rust-quality (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — security-auditor (completed, effort xhigh); agent phase2-reviewer
🤖 Prompt for all review comments with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.

In `packages/rs-drive/src/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:1413-1464: Preserve placement confirmation after an offer is filled
  (existing thread: https://github.com/dashpay/platform/pull/5530#discussion_r4345904240)
  The PlaceBid verifier is classified as `ExecutionProved`, but its evidence query only accepts the current `bid` document and requires that document to exist with matching terms. A complete `TokenFillBid` or bid cancellation deletes the document, and this head has no historical `orderPlaced` evidence in the query or verification path. Consequently, a client that waits for confirmation after an offer closes receives an absence/error even though the placement executed and the buyer either received tokens or recovered escrow. Preserve authenticated placement evidence through closure, or classify this transition fail-closed as affected state until durable evidence is available; the tracked history follow-up is not present at this exact head.

@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

The offer implementation is broadly consistent across escrow accounting, arithmetic, validation, proof binding, protocol gating, wallet summaries, and WASM conversions. One blocking lifecycle defect remains: a PlaceBid transition is classified as execution-proved, but its proof requires a live bid document that a normal complete fill deletes; the tracked history follow-up is not present at this head. CI checks in the supplied snapshot succeeded.

🔴 1 blocking | 🟡 1 suggestion(s)

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

Review provenance

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

  • Triage: critical by gpt-6.1-sol (effort low) — This large, cross-cutting change modifies consensus-critical token state transitions and credit escrow funds movement across DPP, Drive, ABCI validation, version dispatch, proofs, and WASM bindings.
  • 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 — ffi-engineer (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — platform-versioning (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — rust-quality (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — security-auditor (completed, effort xhigh); agent phase2-reviewer, gpt-6-astra — ffi-engineer (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 — ffi-engineer (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — platform-versioning (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — rust-quality (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — security-auditor (completed, effort xhigh); agent phase2-reviewer
  • Model comparison: every Phase-2 reviewer also ran on gpt-6-astra; the verifier weighed both sets without knowing which model wrote which
🤖 Prompt for all review comments with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.

In `packages/rs-drive/src/state_transition_action/batch/batched_transition/token_transition/token_place_bid_transition_action/v0/transformer.rs`:
- [SUGGESTION] packages/rs-drive/src/state_transition_action/batch/batched_transition/token_transition/token_place_bid_transition_action/v0/transformer.rs:99-117: Keep bid decimal compatibility validation in Drive-ABCI advanced structure
  The bid transformer performs the signed-decimals versus token-convention check and returns a bump before the action reaches Drive-ABCI advanced-structure validation. The corresponding PlaceAsk rule is implemented in `rs-drive-abci` advanced-structure validation, where the action already carries the resolved token configuration and the check requires no additional state read. This makes the two placement paths use different validation stages and causes the bid mismatch to be classified during transformation rather than by the shared advanced-structure pipeline. Calculate the bid escrow from the signed decimals in the transformer, then move `TokenOrderDecimalsMismatchError` validation into the bid `structure_v0` validator alongside the ask implementation.

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:1438-1464: Preserve placement confirmation after an offer is filled
  (existing thread: https://github.com/dashpay/platform/pull/5530#discussion_r4345904240)
  The PlaceBid verifier requires the current bid document to exist. The execution-path query requests only that live order document, while a complete TokenFillBid deletes it and the marketplace action path records no durable placement history (`keeps_history` is false for PlaceBid and FillBid). Therefore a normal place-bid → complete-fill → reconnect or delayed-proof sequence returns IncorrectProof even though the offer executed and its escrow was paid out. The partial-fill path is handled by the current term and escrow checks, but complete fills and cancellations still need authenticated durable placement evidence; add the orderPlaced history record and merged proof query from the tracked history work, or do not classify this placement as execution-proved until that evidence exists.

QuantumExplorer and others added 2 commits October 26, 2026 18:41
…advanced structure

As a listing's: the bid transformer works its escrow out at the signed
decimals, and the place-bid structure validation refuses the offer with
TokenOrderDecimalsMismatchError unless they are the token's, before
anything executes, in check_tx too.

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

The marketplace offer implementation is broadly integrated correctly, including escrow accounting, persisted decimals, token binding, frozen-account validation, checked WASM conversions, and protocol gating. One blocking fee-validation defect remains: sellers and cancelling bidders cannot use credits released by the same transition to fund the signer fee, even though execution credits those proceeds before debiting the fee. Complete-fill placement-proof durability is a tracked follow-up in #5531 and is intentionally deferred.

🔴 1 blocking | 🟡 1 suggestion(s)

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

🔴 Blocking: Let bid refunds and sale proceeds fund the signer fee
packages/rs-drive-abci/src/execution/platform_events/state_transition_processing/validate_fees_of_event/v1/mod.rs:348-359

Fee validation compares the signer's pre-transition balance after subtracting only the outgoing principal with the required fee. For bid cancellation, the action operations add the remaining escrow back to the maker; for FillBid, they add the sale proceeds to the seller, and a complete fill can also return the remaining escrow to the buyer. all_used_balances() only accounts for outgoing purchase amounts such as PlaceBid escrow, so these incoming identity-balance operations are absent from the affordability check. execute_event_v1 applies the high-level operations before applying the fee balance change, which means a seller with insufficient liquid credits or a bidder whose credits are entirely locked in escrow can be rejected before execution even though the same transition would provide enough credits to pay its fee. Account for the transition's incoming balance effects in fee validation, while preserving the existing versioned behavior for earlier protocol generations.

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

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: critical by gpt-6.1-sol (effort low) — This large cross-cutting change modifies consensus-critical token transition validation and execution, credit escrow and payout funds movement, signed transition serialization, and proof verification across DPP, Drive, and ABCI components.
  • 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 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 — ffi-engineer (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — platform-versioning (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — rust-quality (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — security-auditor (completed, effort xhigh); agent phase2-reviewer
🤖 Prompt for all review comments with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.

In `packages/rs-drive-abci/src/execution/platform_events/state_transition_processing/validate_fees_of_event/v1/mod.rs`:
- [BLOCKING] packages/rs-drive-abci/src/execution/platform_events/state_transition_processing/validate_fees_of_event/v1/mod.rs:348-359: Let bid refunds and sale proceeds fund the signer fee
  Fee validation compares the signer's pre-transition balance after subtracting only the outgoing principal with the required fee. For bid cancellation, the action operations add the remaining escrow back to the maker; for FillBid, they add the sale proceeds to the seller, and a complete fill can also return the remaining escrow to the buyer. `all_used_balances()` only accounts for outgoing purchase amounts such as PlaceBid escrow, so these incoming identity-balance operations are absent from the affordability check. `execute_event_v1` applies the high-level operations before applying the fee balance change, which means a seller with insufficient liquid credits or a bidder whose credits are entirely locked in escrow can be rejected before execution even though the same transition would provide enough credits to pay its fee. Account for the transition's incoming balance effects in fee validation, while preserving the existing versioned behavior for earlier protocol generations.

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:295-303: Document the inert edits to shipped v0 validation generations
  This change adds PlaceBid and FillBid arms to shipped v0 validation, transformation, state-validation, and proof generations that are selected by pre-PV15 version tables. The new arms are intended to be unreachable before PV15 because the marketplace `is_allowed` gate rejects these transition kinds first, and the v15 notes state that the new generations are zero in the tables. Add the required PR-description section titled `In-place changes to shipped generations`, enumerating the affected v0 dispatchers, the protocol versions that select them, and why the pre-PV15 gate makes the edits inert during replay.
Out-of-scope follow-up suggestions (1)

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

  • Preserve placement confirmation after an offer is filled — The current PlaceBid proof verifier still requires the live bid document, so a complete fill cannot be proven after the offer is deleted. The PR explicitly assigns durable placement evidence for completed orders to #5531; this is a concrete tracked follow-up and does not block this PR's partial-fill implementation.
    • Follow-up: Complete the authenticated history-based placement proof for closed offers in #5531, including the matching PlaceAsk lifecycle and complete-fill regressions.

@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

The offer implementation has three in-scope blocking correctness gaps: bid refunds and sale proceeds cannot fund the signer’s fee, completed offers cannot retain placement proof after their live document is deleted, and the advertised optional bid expiry is absent from the protocol model. The available CI snapshot shows successful Kotlin and book checks, but no Rust marketplace check is listed.

🔴 3 blocking | 🟡 2 suggestion(s)

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

🔴 Blocking: Let bid refunds and sale proceeds fund the signer fee
packages/rs-drive-abci/src/execution/platform_events/state_transition_processing/validate_fees_of_event/v1/mod.rs:348-359

Batch fee validation derives the signer’s available balance from the pre-transition identity balance after subtracting principal. BatchAction supplies added_to_balance_outputs: None, all_used_balances() accounts for PlaceBid escrow but not a bid-cancellation refund or the seller’s FillBid proceeds, and the check at these lines therefore rejects a signer whose only usable credits arrive from the same operation. A buyer whose balance is entirely held in bid escrow cannot cancel without an unrelated top-up, and a token holder with no liquid credits cannot sell into an offer even when the escrowed proceeds would cover the fee. The new bid flows should expose deterministic incoming identity credits to the shared fee-admission path, or otherwise provide an explicit non-circular fee-funding rule; the existing fee code even retains a TODO noting that refunds are not currently accounted for.

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

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: 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: critical by gpt-6.1-sol (effort low) — This is a large, intricate protocol change that modifies consensus-critical token state transitions, credit escrow and payout funds movement, signed transition serialization, and proof execution across DPP, Drive, and ABCI validation.
  • 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 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 — ffi-engineer (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — platform-versioning (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — rust-quality (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — security-auditor (completed, effort xhigh); agent phase2-reviewer
🤖 Prompt for all review comments with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.

In `packages/rs-drive-abci/src/execution/platform_events/state_transition_processing/validate_fees_of_event/v1/mod.rs`:
- [BLOCKING] packages/rs-drive-abci/src/execution/platform_events/state_transition_processing/validate_fees_of_event/v1/mod.rs:348-359: Let bid refunds and sale proceeds fund the signer fee
  Batch fee validation derives the signer’s available balance from the pre-transition identity balance after subtracting `principal`. `BatchAction` supplies `added_to_balance_outputs: None`, `all_used_balances()` accounts for PlaceBid escrow but not a bid-cancellation refund or the seller’s FillBid proceeds, and the check at these lines therefore rejects a signer whose only usable credits arrive from the same operation. A buyer whose balance is entirely held in bid escrow cannot cancel without an unrelated top-up, and a token holder with no liquid credits cannot sell into an offer even when the escrowed proceeds would cover the fee. The new bid flows should expose deterministic incoming identity credits to the shared fee-admission path, or otherwise provide an explicit non-circular fee-funding rule; the existing fee code even retains a TODO noting that refunds are not currently accounted for.

In `packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/batched_transition/token_place_bid_transition/v0/mod.rs`:
- [BLOCKING] packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/batched_transition/token_place_bid_transition/v0/mod.rs:25-40: Carry the advertised bid expiry through the protocol model
  The PR description advertises an optional `expiresAt`, expiry validation, and expired-offer behavior, but `TokenPlaceBidTransitionV0` contains only `amount`, `price`, `decimals`, and `min_fill`. `TokenOrder` has no expiry property, and the bid action, document conversion, and advanced-structure validation have no expiry to enforce. Consequently callers cannot create an expiring offer and every offer remains fillable until cancellation. Add the expiry to the owning DPP transition/order representation and carry it through Drive, validation, proofs, bindings, and the contract schema, or remove the claim and associated behavior from the feature description and tests.

In `book/src/data-model/token-marketplace.md`:
- [SUGGESTION] book/src/data-model/token-marketplace.md:215-218: Offer proof description still demands an untouched order
  The implementation intentionally accepts a live bid after partial fills when `remaining` is between 1 and the original amount and escrow equals `TokenOrder::escrow_after_fills()`. The book still says the proof checks untouched `remaining` and escrow, which contradicts the partial-fill proof behavior and the new regression test. Update the description to state the valid remaining range and escrow-after-fills relationship.

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:295-303: Document the inert edits to shipped v0 validation generations
  (existing thread: https://github.com/dashpay/platform/pull/5530#discussion_r4380481725)
  The PR description’s “In-place changes to shipped generations” section says every listed change adds only PlaceBid and FillBid arms, but this range also changes the existing TokenCancelOrder v0 path: the cancellation transformer no longer rejects bid orders, allowing it to refund bid escrow. The pre-PV15 gate makes that behavior unreachable for historical protocol versions, but the compatibility inventory must explicitly include the TokenCancelOrder transformer/converter changes and explain why the existing ask behavior remains unchanged.

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:1438-1464: Preserve placement confirmation after an offer is filled
  (existing thread: https://github.com/dashpay/platform/pull/5530#discussion_r4345904240)
  The PlaceBid proof branch requires the live `bid` document to be present. A complete FillBid deletes that document, and this head neither writes nor queries a durable marketplace placement record, so an authentic placement proof fetched after the final fill fails at the `let Some(document)` check even though the buyer funded the offer and received the tokens. The PR description points to `dashpay/platform#5531` for history evidence, but that follow-up is not part of this head; absence of the live order cannot be the sole evidence for a placement whose state is intentionally deleted.

Comment thread book/src/data-model/token-marketplace.md Outdated
The book said a TokenPlaceBid's proof checks an untouched remaining and
escrow. The verifier accepts an open offer after partial sales: remaining
between 1 and the amount, and the escrow equal to what the offer holds
after its sales (TokenOrder::escrow_after_fills).

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

The marketplace implementation addresses the prior arithmetic, validation, binding, wallet-summary, and partial-fill proof findings. One blocking lifecycle defect remains: placement proofs require a live bid document even after a successful complete fill deletes it, while the durable history evidence referenced by the PR is absent from this head. The fee-precheck concern is not retained because identity-paid fees intentionally require pre-existing signer funds.

🔴 1 blocking | 🟡 1 suggestion(s)

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

Review provenance

Source: reviewer 1: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 2: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 3: gpt-6.1-sol (agent: phase2-reviewer, role: 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: gpt-6-astra (agent: phase2-reviewer, role: general); reviewer 9: muse-spark-1.3-contributor (agent: phase1-reviewer, role: rust-quality); reviewer 10: gpt-6-astra (agent: phase2-reviewer, role: ffi-engineer); 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-critical token marketplace rules and moves credits through escrow, including validation, serialization, protocol-version gates, proof verification, and Rust/WASM boundaries.
  • 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 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 — ffi-engineer (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — platform-versioning (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — rust-quality (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — security-auditor (completed, effort xhigh); agent phase2-reviewer, gpt-6-astra — general (completed, effort xhigh); agent phase2-reviewer, gpt-6-astra — ffi-engineer (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/state_transition_action/batch/batched_transition/token_transition/token_place_bid_transition_action/v0/mod.rs`:
- [SUGGESTION] packages/rs-drive/src/state_transition_action/batch/batched_transition/token_transition/token_place_bid_transition_action/v0/mod.rs:24-25: Keep the bid action documentation aligned with the validation stage
  The field documentation says that the transformer refuses signed decimals that do not match the token. That check was moved to Drive-ABCI advanced-structure validation, while the transformer now only computes escrow using the signed decimals. The stale ownership statement can mislead maintainers about which validation stage produces TokenOrderDecimalsMismatchError.

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:1431-1454: Preserve placement confirmation after an offer is filled
  (existing thread: https://github.com/dashpay/platform/pull/5530#discussion_r4345904240)
  The PlaceBid proof branch queries only the latest live bid document and returns IncorrectProof when that document is absent. A complete FillBid, or a bid-side cancellation, deletes the bid document; the execution-path query also targets only the live order, and the action conversion shown in this head does not write an orderPlaced history record. Consequently, a successfully executed placement can no longer be confirmed after the offer is completely filled, even though the buyer funded it and may have received the tokens. The PR text references durable history support from #5531, but that implementation is not present in this exact head. Add authenticated placement evidence for closed offers and consume it in both proof generation and verification; apply the same lifecycle treatment to PlaceAsk.

…ot the transformer

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

The offer implementation has one remaining blocking funds-liveness defect: identity fee validation requires the signer to hold the fee before applying bid refunds or sale proceeds, so an all-in buyer cannot cancel and a zero-credit seller cannot fill an offer even when the incoming escrow covers the fee. Complete-fill placement proof remains intentionally deferred to #5531; no other in-scope blockers were confirmed. The Kotlin SDK check is still running, while the completed policy, book, title, and CodeRabbit checks passed.

🔴 1 blocking

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

🔴 Blocking: Let bid refunds and sale proceeds fund the signer fee
packages/rs-drive-abci/src/execution/platform_events/state_transition_processing/validate_fees_of_event/v1/mod.rs:348-359

The identity fee pre-check computes balance_after_principal_operation only from the signer's pre-transition balance and principal. A bid cancellation has no principal, while TokenCancelOrder adds the escrow refund only in the later Drive operations. Likewise, TokenFillBid adds the seller's proceeds through AddToIdentityBalance only after validation. The execution path applies those operations before charging the metered fee, but the validation event does not account for the incoming identity credits, so a buyer who committed all available credits to an offer cannot cancel it and a seller with no pre-existing credits cannot sell into it even when the escrowed amount covers the fee. These are valid user-state transitions that can strand escrow and make the offer flow non-live. Include same-event identity credits in the deterministic fee funding calculation, or explicitly reserve the fee from the escrow/proceeds while keeping estimation and execution consistent.

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

Review provenance

Source: reviewer 1: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 2: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 3: gpt-6.1-sol (agent: phase2-reviewer, role: platform-versioning); reviewer 4: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 5: muse-spark-1.3-contributor (agent: phase1-reviewer, role: general); reviewer 6: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); reviewer 7: muse-spark-1.3-contributor (agent: phase1-reviewer, role: rust-quality); 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-critical token state-transition validation and execution, including escrowed credit funds movement, token transfers, serialization, proofs, and protocol-version activation.
  • 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 verifier: gpt-6.1-sol — final-verifier; agent sol-verifier
  • Phase 2 reviewers: gpt-6.1-sol — general (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — architecture-layering (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — platform-versioning (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — rust-quality (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — security-auditor (completed, effort xhigh); agent phase2-reviewer
🤖 Prompt for all review comments with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.

In `packages/rs-drive-abci/src/execution/platform_events/state_transition_processing/validate_fees_of_event/v1/mod.rs`:
- [BLOCKING] packages/rs-drive-abci/src/execution/platform_events/state_transition_processing/validate_fees_of_event/v1/mod.rs:348-359: Let bid refunds and sale proceeds fund the signer fee
  The identity fee pre-check computes `balance_after_principal_operation` only from the signer's pre-transition balance and `principal`. A bid cancellation has no principal, while `TokenCancelOrder` adds the escrow refund only in the later Drive operations. Likewise, `TokenFillBid` adds the seller's proceeds through `AddToIdentityBalance` only after validation. The execution path applies those operations before charging the metered fee, but the validation event does not account for the incoming identity credits, so a buyer who committed all available credits to an offer cannot cancel it and a seller with no pre-existing credits cannot sell into it even when the escrowed amount covers the fee. These are valid user-state transitions that can strand escrow and make the offer flow non-live. Include same-event identity credits in the deterministic fee funding calculation, or explicitly reserve the fee from the escrow/proceeds while keeping estimation and execution consistent.

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

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants