[ZIP-233] Network Sustainability Mechanism: Burning

[giddie]

This mostly consists of the addition of a burn_amount transaction field, introduced in the ZFuture network upgrade. Enabling cfg(zcash_unstable = "zfuture") meant that a lot of TZE code was unnecessarily enabled, so we switched some of those sections to zcash_unstable = "tze" as well as introducing zcash_unstable = "nsm" to keep things separate. The zfuture cfg is now used just to enable the ZFuture network upgrade.

The code for the generated test vectors is available in this PR.

[codecov]

Codecov Report

Attention: Patch coverage is 24.65753% with 55 lines in your changes missing coverage. Please review.

Project coverage is 57.90%. Comparing base (0d388d8) to head (3ee2de4).

Files with missing lines	Patch %	Lines
zcash_primitives/src/transaction/mod.rs	16.32%	41 Missing ⚠️
zcash_primitives/src/transaction/builder.rs	10.00%	9 Missing ⚠️
pczt/src/roles/tx_extractor/mod.rs	0.00%	2 Missing ⚠️
zcash_client_sqlite/src/wallet.rs	0.00%	2 Missing ⚠️
zcash_primitives/src/transaction/sighash.rs	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1567      +/-   ##
==========================================
- Coverage   58.01%   57.90%   -0.12%     
==========================================
  Files         181      181              
  Lines       21976    22046      +70     
==========================================
+ Hits        12750    12766      +16     
- Misses       9226     9280      +54     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[daira]

Blocking on review by @str4d and @nuttycom, since this requires a design decision about the future of ZFuture (and the interaction of zcash_unstable flags).

[aphelionz]

Can somebody re-run the clippy test when it's convenient? 🙏

[nuttycom]

@aphelionz there's an underlying clippy CI issue being fixed in #1785 - you might want to rebase once that merges to clear this issue.

[aphelionz]

@mariopil are you able to quickly rebase one more time now that the clippy fixes are in?

[nuttycom]

A few high-level blocking comments thus far.

[mariopil]

@aphelionz I was on vacation last two weeks - I've rebased now vs latest main and addressed @nuttycom comments.

[daira]

Changes needed.

[mariopil]

Thank you @daira for the review. I've done the fixes. I've added read_v6_header_fragment and write_v6_header, and moved the reading and writing of zip233_amount there.

[daira]

Does not compile.

[daira]

MarioP wrote in #zebra on the R&D Discord:

I've noticed this digest header change

librustzcash/zcash_primitives/src/transaction/txid.rs

Line 240 in 2630263

h.write_u64_le((*zip233_amount).into()).unwrap();

in the ZIP-233 librustzcash PR breaks a few tests in Zebra (merkle_root_is_valid, v5_with_sapling_spends, v4_with_sapling_spends, block_test_vectors, binding_signatures) that test processing old blocks from testnet and mainnet. Shouldn't the writing zip233Amount in the header be conditioned e.g. based on the Tx version?

Yes, probably, although I haven't had chance to look in detail. Please check that this does not break Zebra tests before merging.

[mariopil]

Thank you, @daira. Yes, I've tested this with Zebra, and all tests are passing.

[aphelionz]

@daira @nuttycom friendly reminder to review again 🙏

[arya2]

This is looking good, there are a handful of outstanding lints and compilation issues that need fixing.

[daira]

Suggested change

	Zatoshis::ZERO,
	Zatoshis::ZERO, // TODO: change when PCZTs add v6 transaction support

[nuttycom]

#1879 now builds atop this branch, merging main and addressing comments from review.