Skip to content

fix(pivx): compute correct txid for Sapling transactions#1606

Open
pragmaxim wants to merge 1 commit into
trezor:masterfrom
pragmaxim:fix/pivx-sapling-txid
Open

fix(pivx): compute correct txid for Sapling transactions#1606
pragmaxim wants to merge 1 commit into
trezor:masterfrom
pragmaxim:fix/pivx-sapling-txid

Conversation

@pragmaxim

Copy link
Copy Markdown
Contributor

Fixes #1522

Problem

PIVX Sapling transactions (version >= 3) include shielded fields (valueBalance, vShieldedSpend, vShieldedOutput, bindingSig) after the transparent part. wire.MsgTx.TxHash() only hashes the transparent fields via SerializeNoWitness, producing a phantom txid that gets written to the address index. Subsequent API queries for that txid return HTTP 500.

Also, ParseTx used WitnessEncoding via Deserialize(), which can misinterpret a Sapling tx with 0 inputs as a segwit transaction (bytes 0x00 0x01 are read as the segwit marker+flags).

Fix

For Sapling transactions (version >= 3), compute the txid as sha256d(full raw transaction bytes), matching PIVX's consensus definition (SerializeHash in src/primitives/transaction.cpp). In ParseTx, read the version first and use BaseEncoding for version >= 3, matching PivxDecodeTransactions.

Changes

bchain/coins/pivx/pivxparser.go:

  • PivxDecodeTransactions: track per-tx raw byte offsets in the block buffer, return raw tx data
  • ParseBlock: override txid for Sapling txs with chainhash.DoubleHashH(rawTx)
  • ParseTx: fix encoding selection (BaseEncoding for v>=3), override txid with chainhash.DoubleHashH(rawBytes)

Testing

  • All 58 unit tests pass (including PIVX pre-Sapling test data)
  • All 19 connectivity tests pass
  • Integration tests: ethereum, bcash, bsc, avalanche all pass
  • No pre-Sapling test data is affected (version gate >= 3)

Deployment

Existing databases with Sapling-era blocks must be re-indexed (delete data dir and re-sync). The fix only affects newly parsed data.

PIVX Sapling transactions (version >= 3) include shielded fields
(valueBalance, vShieldedSpend, vShieldedOutput, bindingSig) after
the transparent part. wire.MsgTx.TxHash() only hashes the transparent
fields, producing a phantom txid that is stored in the address index.

Fix by computing the txid as sha256d of the full raw transaction bytes
for Sapling transactions, matching PIVX's consensus definition.

Also fix a pre-existing encoding bug in ParseTx: it used WitnessEncoding
via Deserialize(), which can misinterpret a Sapling tx with 0 inputs
as a segwit transaction (0x00 0x01 marker). Now reads the version first
and uses BaseEncoding for version >= 3, matching PivxDecodeTransactions.

Fixes trezor#1522

@cranycrane cranycrane left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

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.

PIVX: Sapling (v≥3) transactions get a wrong txid in the address index

2 participants