Skip to content

prune_batches leaves PrunedUpTo pointing past batches it did not actually remove #189

Description

@mallison031

Repo context. accensa-contracts holds the on-chain half of Accensa: ReceiptAnchor
(Merkle batch anchoring so an agent can verify it was charged correctly without trusting
the seller's API) and RefundVault (policy-bounded refunds without the merchant becoming
a custodian). Both are deployed on testnet. soroban-sdk 27.0.4, MIT. Read README.md,
docs/SECURITY_MODEL.md and DEPLOYMENTS.md before starting.

Problem

The prune loop advances past a missing batch:

} else {
    // If it's not present, it might have been manually deleted or we skipped it.
    // We should just increment and continue.
    pruned_up_to += 1;
}

A batch can be absent because it was pruned — or because its persistent entry hit its TTL and was
archived while the instance entry survived. The two are indistinguishable at this point, and the
loop treats both as "keep going".

That produces a wrong PrunedUpTo. Consider batches 1..10 where 5 has been archived by TTL
expiry and 6 is still live and newer than before_ledger. The loop prunes 1–4, hits the gap at
5, increments past it, reaches 6, and stops because 6 is too new. PrunedUpTo is now 6. Batch 5
was never pruned — it was archived, and archived entries can be restored. If it is restored
later, it sits below PrunedUpTo, so every consumer that trusts PrunedUpTo as "everything
below this is gone" is wrong about it.

The comment acknowledges the ambiguity ("might have been manually deleted or we skipped it") and
resolves it by guessing.

What to do

  • Decide what PrunedUpTo actually guarantees, and write it down. "The lowest batch id that has
    not been deliberately pruned" and "the lowest batch id that is readable" are different
    invariants and the code currently satisfies neither reliably.
  • Stop advancing over gaps silently. Either halt at the first missing entry and let the caller
    retry after restoring, or track skipped ids explicitly so a restore cannot land below the
    watermark.
  • Emit something observable when a gap is encountered — the current behaviour is silent.

Note on scope

#149 covers the instruction-budget exhaustion in this same loop and #151 covers get_batch not
distinguishing pruned from never-existed. All three touch prune_batches; coordinate before
starting, and consider taking them as one branch.

Acceptance criteria

  • The PrunedUpTo invariant is stated in a doc comment and in docs/SECURITY_MODEL.md.
  • A test reproduces the archived-gap scenario above and asserts the new behaviour.
  • A restored batch can never end up below PrunedUpTo.

Metadata

Metadata

Assignees

Labels

Stellar WaveDrips Wave Programarea: contractsWork in contractsbugSomething isn't workingcomplexity: high200 pts — integrations or architectural work

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions