Skip to content

A stated refusal from the second push destination frees inputs the first may have admitted #460

Description

@MichaelTaylor3d

Task

is_definitive_rejection frees a bundle's inputs on any stated refusal, but chia-query's push_tx transmits to up to three destinations — so a stated refusal from destination B can free the inputs of a bundle destination A already admitted.

The path, concretely

chia-query router.rs:793 push_tx calls peer_then_coinset(peer, peer_retry, coinset), and router.rs:139-167 will try all three:

Peer A admits the bundle to its mempool, but the ack times out (peer/mod.rs:1074, "request timed out") → peer A is ejected and the bundle is re-transmitted to peer B → peer B has already seen it gossiped and answers with a stated reason → is_definitive_rejection is truereserve_pushed_bundle is skipped → the inputs of a bundle sitting in the public mempool return to the selectable set → a second send inside the 10-minute window reselects them.

That is the double-select window dig-node#348 exists to close, reached by a different route.

This is NOT a regression, and it must not be read as one

Before dig-node#453, rejection was Some(..) on every non-admitted answer, so this path freed the coins along with all the others. #453 strictly narrowed it. What changed is that this is now the only remaining free path, which is what makes it worth naming.

The question to settle

Should the guard key on the CLASS of stated reason rather than merely on its presence?

A reason like BAD_AGGREGATE_SIGNATURE is a property of the bundle — it will be refused everywhere, so freeing the inputs is correct. A reason like DOUBLE_SPEND or a mempool-full condition is a property of that peer's view, and may mean another peer is holding the very bundle whose inputs are about to be freed.

Two cautions for whoever takes it:

  • The reason string is supplied by an untrusted peer (§13 / NC-12). A classifier keyed on attacker-controlled text is itself a decision: a peer that wants its refusal to stick can write whatever text makes that happen. Consider what a hostile string can achieve in each direction before keying on content.
  • Do not shorten the TTL to compensate. RESERVATION_TTL_MS = 600_000 and the code's own doc states the reasoning: that trades a double-select for a lockout, and a lockout is the worse failure — measured on dig-account as available=4000000 selectable=0, renewable indefinitely.

Evidence

Found by the security re-gate of #453 while verifying that the #348 hold actually fires. It correctly declined to gate on this, since #453 improves the situation rather than causing it. Traced by direct read; not fired against a live multi-peer push.

Acceptance

A stated refusal that is a property of one peer's view, arriving after another destination may have admitted the bundle, does not return the inputs to selection — proven by a test that distinguishes the two reason classes, not merely that some reason is present.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions