Skip to content

Fix transaction proofs with duplicate witness ids - #2469

Draft
a-shannon wants to merge 6 commits into
ergoplatform:masterfrom
a-shannon:fix/transaction-membership-proof
Draft

a-shannon wants to merge 6 commits into
ergoplatform:masterfrom
a-shannon:fix/transaction-membership-proof

Conversation

@a-shannon

@a-shannon a-shannon commented Aug 14, 2026 •

Copy link
Copy Markdown
Contributor

Current scope — 29 October 2026

This is now a legacy proposal, held in draft. The current default builds on master/stable v6.0.8 (c1a69f65), v6.0.9 (9f77ea63) and v6.0.10 (c4692c2) declare scrypto 3.1.1. Its upstream duplicate-leaf fix uses the real ordered leaf count, so the originally reported traversal defect no longer needs this node-local workaround on those defaults.

The generic fix does not provide this PR's extra transaction-only input guards. The mapped stock HTTP route requires 32-byte IDs, and mandatory mining derives IDs from transaction objects; malformed direct-helper inputs are outside those consumers. External direct library callers were not audited. This is source/dependency evidence, not a fresh runtime-classpath or published-JAR claim.

The implementation and tests below describe the old v6.0.6 branch. I am not porting the custom traversal to the current branches. Any remaining direct-helper input hardening should be assessed separately.


Summary

  • return transaction membership proofs for the requested transaction when witness ids repeat
  • traverse the existing Merkle tree with its real ordered leaf count instead of scrypto's deduplicated map length
  • cover duplicate witnesses, padding boundaries, absent ids, and the mandatory-mining consumer

Root cause

For block versions after v1, BlockTransactions commits to txIds ++ witnessIds.
The scrypto tree keeps every leaf in topNode, but its hash-to-index map collapses
duplicate leaf hashes. proofByElement then traverses the real tree using the map's
smaller size, so it can return a proof for another leaf that still validates against
the transaction root.

This change locates the requested transaction in txIds and traverses the existing
tree with n leaves for v1 or 2n leaves for later versions. It also fails closed if
the traversal does not terminate at the requested leaf.

Compatibility

The ordered leaves, Merkle-tree construction, transaction root, block serialization,
JSON schema, and consensus bytes are unchanged. Only membership-proof selection is
corrected. The transaction lookup is intentionally tx-only and uses a bounded linear
scan followed by logarithmic tree traversal.

The generic duplicate-leaf fix is already merged in scrypto #5. This v6.0.6 branch
still resolves scrypto 3.0.0, so the node-local fix remains necessary here.

Validation

  • regression reproduced on the current v6.0.6 base: the test-only BlockTransactionsSpec run passed 1 test and failed the 2 new proof-identity cases
  • sbt "ergoCore/Test/testOnly org.ergoplatform.modifiers.history.BlockTransactionsSpec" — 3 passed
  • sbt "ergoCore/Test/testOnly org.ergoplatform.serialization.SerializationCoreTests -- -z BlockTransactions" — 1 passed
  • sbt "Test/testOnly org.ergoplatform.mining.ErgoMinerSpec" — 5 passed
  • sbt "Test/testOnly org.ergoplatform.http.routes.BlocksApiRouteSpec" — 10 passed
  • GitHub CI — 8/8 jobs passed on 202f762cb38b6ac56c6cc9afdb9bf51aa5208dca
  • independent review of the exact 3-file patch — no findings

Related work

Fixes #2463.

@a-shannon

Copy link
Copy Markdown
Contributor Author

Added an explicit deterministic regression in 30d9c02. It creates three distinct transaction IDs with the same witness ID, first verifies that the proof is valid against the block root, and then verifies that the leaf is the requested transaction ID. On the parent commit the test fails only on the leaf-identity assertion; on this branch it passes. BlockTransactionsSpec is green (3/3).

kushti and others added 3 commits September 1, 2026 13:46
…nmodified-1614

Test that proofsForTransactions() does not modify the UTXO set tree along a growing chain
…ardening-v606

Require wallet-owned change addresses
@a-shannon
a-shannon force-pushed the fix/transaction-membership-proof branch from ca36ced to 202f762 Compare September 2, 2026 20:34
@a-shannon

Copy link
Copy Markdown
Contributor Author

@kushti #2469 is ready for maintainer review. The current head (202f762) is one commit on top of v6.0.6, mergeable, and CI is 8/8 green. It fixes #2463 on the release branch that still resolves scrypto 3.0.0; the ordered leaves, Merkle root, serialization, and consensus bytes are unchanged.

@kushti
kushti changed the base branch from v6.0.6 to master September 21, 2026 09:51
@a-shannon
a-shannon marked this pull request as draft October 29, 2026 21:10
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.

Incorrect txProof when providing a prioritized transaction

4 participants