Orchard output ordering is no longer deterministic, as we now shuffle
spends and outputs during bundle building. But we _can_ still check that
the action index of a note being spent is correct.
Also fixes a bug in the `nuparams` helper, which would have caused MSB
zeroes in consensus branch IDs to not be rendered in the `-nuparams`
option. This hadn't been encountered because we haven't yet generated a
consensus branch ID with a zero MSB.
We need to bump the `zcashd` protocol version because the new rules are
not compatible with existing rules followed by 170015 nodes, but we
_also_ need to ensure we can still bump it again once we set the testnet
reactivation height (changing node network behaviour again). This commit
also enables RPC tests to run (because previously the nodes considered
each other to be too old for NU5 to be active, and were disconnecting).
Before merging 4.7.0-rc1 into the nu5-consensus branch, we were in a
split state:
- 4.7.0-rc1 included Orchard support in the transaction builder, which
required special handling of Orchard bundles when computing sighashes.
The `PrecomputedTransactionData` structure could be shared, because
its digests were only relevant to transparent signatures (as shielded
signatures signed the txid directly even in shielding transactions).
- nu5-consensus included the changes to ZIP 244, which required passing
around a `PrecomputedTransactionData` that contained the set of all
transparent inputs being spent, because shielding transactions now also
need to commit to transparent inputs.
In the merge commit, we incorrectly handled the resolution: we correctly
derived a fresh `PrecomputedTransactionData` when signing the Orchard
bundle, but we reused the `PrecomputedTransactionData` that was
previously derived before checking whether or not we even had an Orchard
bundle, for transparent inputs. This meant that its commitments didn't
commit to the Orchard bundle, and so transparent signatures on
transactions with Orchard bundles would fail to verify.
Incidentally, this is the exact inverse of a bug we encounted while
implementing the ZIP 244 changes on the nu5-consensus branch: we were
correctly computing the transparent sighash, but we were relying on the
initial `TxDigests` derived within `PrecomputedTransactionData` for the
Orchard sighash, even though we were actively rewriting the transaction
to include the Orchard bundle. The fix there was similarly to re-compute
the `TxDigests` before computing the sighash.
The merge commit includes changes to address direct merge conflicts.
This commit makes the remaining changes necessary to integrate the
Orchard wallet changes with the NU5 consensus changes.
This includes:
- `orchard =0.1.0-beta.3` which includes the final circuit changes.
- The new NU5 consensus branch ID.
- Updated ZIP 244 test vectors (which use the NU5 consensus branch ID).
`CWallet::FindUnifiedAddressByReceiver` is a wrapper around the visitor
`UnifiedAddressForReceiver`, which looks up the Unified Address (if any)
corresponding to the given receiver. However, this only returns external
UAs, and returns `std::nullopt` both if the receiver does not correspond
to a UFVK, and if it does but is derived from an internal FVK. By using
this method as a filter, `listaddresses` was not filtering out internal
Sapling receivers, and thus was leaking Sapling change addresses for
accounts (which we do not want revealed in the UI anywhere).
Instead, we now check for UFVK metadata directly, which verifies that
the Sapling receiver is derived from a UFVK without any extra filtering.
- Legacy transparent addresses derived from the mnemonic seed are no
longer duplicated in the `legacy_random` source.
- Legacy transparent change addresses derived from the mnemonic seed are
now shown under that source.
- Sapling addresses that aren't part of a UA are now identified
correctly when derived from the mnemonic seed, rather than being
assumed to be derived from the legacy HD seed.
- The legacy_random source should only contain Sprout addresses (as
zcashd now only derives transparent addresses from the mnemonic seed).
- Sapling addresses should appear in the mnemonic_seed source, not the
legacy_seed source (as zcashd no longer ever generates keys there).
The RPC test also now has several additional checks:
- `listaddresses` does not return duplicate addresses (an address can
only come from a single source).
- Address sets are now explicitly checked (to ensure that there are no
unexpected addresses under the checked sources).
This now returns whether or not the address corresponds to an
internal recipient, an external address for the wallet, or a
counterparty's address (an address not derived from any of
the wallet's keys) from GetPaymentAddressForRecipient.
Fixes#5708
Daira Hopwood
Jack Grigg
therealyingtong
Steven
str4d
sasha
Larry Ruane
Jack Grigg
Dimitris Apostolou
Charlie O'Keefe
Kris Nuttycombe