search

zcash / zips

Zcash Improvement Proposals
https://zips.z.cash
MIT License
274 stars 156 forks source link

[protocol spec] [ZIP 212] zcashd enforces the 0x02 lead byte for coinbase outputs only after end of the original grace period #630

Open daira opened 2 years ago

daira commented 2 years ago

@defuse and I found a consensus bug that could have caused a chain fork but didn't (and no longer can).

For coinbase transactions with a shielded Sapling output, the spec requires the lead byte of the note plaintext to be 0x02 even in the grace period after Canopy. zcash_primitives/src/sapling/note_encryption.rs does not implement this correctly; it only enforces it after the end of the grace period. This would have been just a relatively harmless spec-implementation divergence, but Aditya also extended the grace period in the librustzcash used for the embedded zcashd in ZecWallet fullnode. So, during that grace period extension, a coinbase transaction with a shielded Sapling output having plaintext version 0x01 would have split ZecWallet's embedded zcashd from the rest of the network.

(That change to adityapk00/librustzcash was never merged to zcash/librustzcash.)

daira commented 2 years ago

I filed this here rather than in zcash/zcash because the easiest fix is to change the protocol spec to match zcashd. Alternatively zcashd could be changed to match the protocol spec, provided that there are no 0x01 note plaintexts in coinbase Sapling coinbase outputs during the original grace period on mainnet and testnet. (This would not change the behaviour for 0x01 note plaintexts in non-coinbase Sapling outputs, which were intentionally allowed during the grace period.)

daira commented 2 years ago

ZIP 212 also needs to be changed if the protocol spec is.