In zcash/zcash/pull/5544 we added an FFI method specific to the Orchard
part of the transaction builder, to enable passing the Orchard bundle on
the Rust side directly instead of using a partial serialization. We
passed the remainder of the transaction to the Rust side by serializing,
as we similarly do in `SignatureHash`. We then decomposed the parsed
transaction on the Rust side to insert the Orchard bundle. However, we
then used the `TxDigests` that was derived from the parsed transaction,
not the reconstructed one, which meant we produced an invalid sighash
when building Orchard-containing transactions.
The addition of `OrchardRawAddress` to `RecipientAddress` drives most of
the changes in this commit, which enable `z_sendmany` to send funds to
addresses in the Orchard pool once NU5 activates.
- We added `OrchardRawAddress` to the `Receiver` variant, which prevents
it from having a default constructor.
- We added `ReceiverType::Orchard`, which Clang warns is unused when we
use `ReceiverType` in a `switch` statement.
We also add Orchard-specific information to several RPCs in order for
tests to pass:
- `z_listunifiedreceivers`
- `z_getbalanceforaccount`
- `z_getbalanceforviewingkey`
We add the Orchard change address mapping on account creation, as we
only ever generate a single change address internally. External Orchard
receivers are added to the map when `z_getaddressforaccount` is called.
This commit also fixes two bugs in the handling of Orchard internal IVKs.
When adding a full viewing key to the Orchard (Rust) wallet, we need to
derive both the internal and external IVKs (and map them to the external
FVK so we effectively never expose the internal FVK). We were only
deriving the external IVK, meaning that trying to add the Orchard change
address to the wallet should have failed. However, in a separate bug the
C++ internal IVK derivation method was calling the Rust external IVK
derivation function, meaning that we were effectively using the external
Orchard address at index 0 _as_ the change Orchard address.
This ensures we have the same semantics for detecting Orchard and
Sapling receivers in UAs, even if the UA they are part of was derived
outside of the zcashd wallet.
The only place we were using the returned Unified Spending Key was in a
test, where we immediately converted it to a UFVK.
Co-authored-by: Kris Nuttycombe <kris@nutty.land>
In cases where we want to be able to decode a string that
is known to be a unified address, it doesn't make sense to have
to route through KeyIO::DecodePaymentAddress and then return
an error depending upon the result type, when it's possible to
provide unified address parsing more directly.
KeyIO::DecodePaymentAddress has been modified to delegate
to UnifiedAddress::Parse
This tests the following operations:
* add a spending key to the wallet
* add notes from the Orchard bundle component of a transaction to the
wallet
* detect notes in the wallet that correspond to the spending key
* verify that notes that are not ours are not mistakenly labeled
as ours
We implement this by throwing an exception in SignatureHash on violation
of the rules. For CHECKMULTISIG and CHECKSIG this results in `false`
being pushed onto the stack, while for CHECKMULTISIGVERIFY and
CHECKSIGVERIFY this results in an error.
The ZIP 244 changes mean that we're going to need to alter every
callsite to pass through all of the transparent `CTxOut`s being spent.
Given that we need to pass it over to Rust, it makes more sense to just
have `PrecomputedTransactionData` be the vehicle for conveying this data
across.
The ZIP 244 changes mean that shielded signatures will now require
access to any transparent inputs of the transaction, so we need to
validate the shielded signatures around the same point during block
connection or `AcceptToMemoryPool` as when we validate transparent
signatures.
This fixes a potential bug with importing the mnemonic into a third
party transparent wallet. Previously, if a user called `getnewaddress`,
made a bunch of transactions that generated at least 20 change
addresses, and then called `getnewaddress` again, the two external
addresses would be separated by a gap of more than 20. If this mnemonic
were imported into a third party transparent wallet, the wallet would
not detect any funds in the second (or subsequent) transparent addresses
because it would detect 20 unused addresses in a row (via the BIP 44
default gap limit).
Now, we track external and internal keys separately; repeated calls to
`getnewaddress` will return addresses for sequential keys. This has the
added benefit that the sequence of `getnewaddress` outputs will be the
same after restoring from a backup.
Now that the keypool is not used by any consumers of external
transparent keys, we switch it to only contain internal keys. To handle
the impedance mismatch, we clear out the keypool when a mnemonic seed is
generated for the wallet.
We also add a `type` field to the output objects (matching the field in
`z_viewtransaction`), now that the output type can't be distinguished
solely from the address encoding.
The use of `std::optional` for the return type of the
`ZcashdUnifiedFullViewingKey::Address` operation obscured
a potential bug where the requested receiver types for a
unified address could not be satisfied because the UFVK
from which the address was being derived did not contain
all of the requested key types.
Adding the default address to the wallet automatically made sense
prior to support of diversified addresses, but this is no longer
desirable behavior as we have separated key generation from address
derivation.
This operation was poorly named; the wallet already contained
the IVK, and this operation was simply adding the ability to
look up the IVK by a specific diversified address derived
from that IVK.
This also alters `TransactionBuilder::SendChangeTo` to take a
`libzcash::ChangeAddress` value and thus avoids the invalid
t-addr case of CTxDestination.
When generating a new account, we were setting up the map from account
ID to UFVK ID, but were not setting up the reverse map from UFVK ID to
account ID. The latter map is needed in order to verify that a note in
the wallet belongs to the account. We were correctly loading it from the
account metadata on node start, but between account generation and first
restart, z_getbalanceforaccount would always return zero.
We now initialize the UFVK address metadata map with the account ID when
the account is generated.
The ZTXOSelector type allows selection of previous Zcash
transaction outputs (both transparent outputs and shielded notes)
on the basis of either a (legacy) bare address, or for a
BIP-44 account.
We now use the empty array of pools to indicate that the default pools
should be used when providing a diversifier index parameter, instead of
needing a default sentinel value for the diversifier index parameter
(which was previously suggested as '*' which would have caused issues
for defining a consistent type for that parameter).
This adds an `allowRevealedAmounts` argument to z_sendmany. This
flag must be present to allow an amount-revealing cross-pool transfer
to be constructed.
This replaces the old implementation of asyncrpcoperation_sendmany
with one where all transaction construction is delegated to the
transaction builder. The capabilities of z_sendmany are somewhat
modified in the process:
* z_sendmany now permits sending funds from a Sprout address to
both transparent and Sapling addresses. PRIVACY NOTE: When
user sends a Sprout->Sapling transaction, the amount of the
transaction is publicly revealed.
* z_sendmany no longer supports transactions sending funds into
the Sprout pool, with the exception of change amounts when
sending from a Sprout address.
* When sending transparent coinbase funds to a set of shielded
addresses, the amount sent to recipients must fully consume
the input value and no change is permitted. This is a slightly
weaker constraint than was previously implemented; in the past,
only a single shielded recipient was allowed.
This adds a new `AddrSet` type for use in note retrieval
as a filter, in place of a heterogeneous list of `RawAddress`
values. `RawAddress` will complicate the handling of addresses
within the wallet after the addition of unified addresses,
because it does not contain transparent receiver types, and
if we retain this polymorphism it means a lot of invalid states
are represented in places we don't want them to be. It's better
to figure out what types of addresses we're working with as soon
as possible after parsing, and use monomorphic calls from there
on in.
The `spendingkey_` field previously used by asyncrpcoperation_sendmany
caused a situation where call sites would assume the type of the
spending key based upon boolean flags that were derived elsewhere.
In attempting to support unified addresses, it is desirable to remove
these boolean deductions and instead work primarily based on the
direct evidence of the data at hand. This commit therefore removes
the `spendingkey_` field that could potentially produce results
that would disagree with these boolean values, in favor of such
direct evidence, making these call sites a bit less error prone.
The addition of `UnifiedAddress` to the `PaymentAddress` type
introduced the need for methods that interact with payment addresses
to support transparent receivers as shielded ones, which is somewhat
inconsistent with previous uses of the `PaymentAddress` type.
This commit adds both `CKeyID` and `CScriptID` as new variants
of the `PaymentAddress` type. Following commits will shift encoding
and decoding to use the `PaymentAddress` type exclusively wherever
possible, rather than the current mix of `CDestination` and
`PaymentAddress` that complicates the wallet codebase.
The presence of this variant results in a situation where more
of the code than necessary needs to be aware of and handle
decoding failures. This change moves all handling of decoding
failures to the point of decoding.
We added support for the NU5 consensus rules in v4.5.0, which alters the
block header to contain a `hashBlockCommitments` value instead of the
chain history root. However, the output of `getblocktemplate` wasn't
returning this value; once NU5 activated, the `blockcommitmentshash`
field was being set to "null" (all-zeroes).
In v4.6.0 we added full NU5 support to `getblocktemplate`, by adding a
`defaultroots` field that gave default values for `hashBlockCommitments`
and the components required to derive it. However, in doing so we
introduced a regression in the (now-deprecated) legacy fields, where
prior to NU5 activation they contained nonsense.
This commit fixes the output of `getblocktemplate` to have the intended
semantics for all fields:
- The `blockcommitmentshash` and `authdataroot` fields in `defaultroots`
are now omitted from block templates for heights before NU5 activation.
- The legacy fields now always contain the default value to be placed
into the block header (regaining their previous semantics).
Co-authored-by: Larry Ruane <larry@z.cash>
This test currently fails with submitblock returning the error
"bad-heartwood-root-in-block".
Added authdigest to GBT coinbasetxn field because we can't obtain this
via getrawtransaction.
Co-authored-by: Jack Grigg <jack@z.cash>
ZcashdUnifiedFullViewingKey::FindAddress was previously not checking
that the transparent child index did not overflow in unified address
generation. GenerateUnifiedAddress has been modified to search from
the last known diversifier index if no diversifier is specified, and
boundary conditions for transparent addresses are now correctly handled.
This reworks the way that address metadata is added to the wallet
to make adding address metadata to the wallet's in-memory cache
the responsibility of a single method rather than distributed
across multiple places in the code.
All of the necessary relationships between transparent receivers
and UFVKs are set up by `AddUnifiedAddress`, so there's no
need to do so as part of transparent key management otherwise.
This changes ZcashdUnifiedSpendingKey::ToUnifiedFullViewingKey
to return a `UnifiedFullViewingKey` object. This makes the
derivation of ZcashdUnifiedFullViewingKey values more regular,
removing the need to be able to construct these values directly
from the spending key.
This change also modifies ZcashdUnifiedSpendingKey to remove
the `std::optional` wrapper from its member keys. This
optionality is spurious, because it is always possible to
derive a "complete" spending key, and we do not support
import of unified spending keys.
This fixes two issues with the management of unified full viewing
key metadata:
* Adds a reverse mapping from ufvkid to account id.
* Ensures that UFVK metadata is correctly initialized when USK
metadata is loaded from disk.
This will ensure that miners can use the values returned by
getblocktemplate (in particular, the block commitment hash)
to submit a valid block using the submitblock RPC.
zcash/zcash:
The `getmininginfo` RPC now omits `currentblockize` and `currentblocktx`
when a block was never assembled via RPC on this node during its current
process instantiation. The relevant RPCs are `generate` (regtest only) and
`getblocktemplate`. Blocks are also assembled when running the internal
miner (`zcashd -gen=1`), after the node mines its first block.
(cherry picked from commit bitcoin/bitcoin@fa178a6385)
We use the `zcash_address` crate to parse Unified Addresses (which we
currently do nothing with). That crate returns an `UnsupportedAddress`
error for unhandled address kinds, which we were previously logging.
However, we _do_ support the other address kinds; we just parse them in
the C++ code.
Closeszcash/zcash#5321.
The new RPCs aren't functional, only have argument parsing and sample
outputs, guarded by experimental -orchardwallet flag.
These changes used the tickets linked from
https://github.com/zcash/zcash/issues/5056 as a guide.
zip339_phrase_to_seed will return `false` if the string it is being
used to convert to a seed is not valid UTF-8, but this result was
previously unchecked.
Fixes#5399
This type is backed by the `zcash_address` implementaion of
unified full viewing keys. It is intended for serialization
and parsing of UFVKs, and provides conversion functions that
allow for construction to and from ZcashdUnifiedFullViewingKey
values.
Adds SaplingDiversifiableFullViewingKey as a superclass of
SaplingExtendedFullViewingKey, and reduces the Sapling component
of UnifiedFullViewingKey to the new intermediate type. This
permits deserialization from the data that will be present
in the serialized form of the Sapling component of a UFVK.
This test-only change allows python (rpc) tests to specify, for example,
that NU5 should be activated at height X, without having to specify all
the previous network upgrades. Previous upgrades can (and must) still be
specified if they activate at different block heights (than, in this
example, NU5). This makes tests easier to write (and read), especially
as the number of network upgrades increases over time.
Note that this change only affects zcashd behavior in regtest mode. For
the other network modes (testnet and mainnet), the activation heights
are hard-coded in chainparams.cpp.
This commit does not include functional changes; it merely
breaks up content previously included in `zcash/address/zip32.h`
into smaller and more focused components.
Remove IncrementalSinsemillaTree; this will be replaced by
a more full-featured OrchardWallet type which embeds the
incremental merkle tree used in wallet operations.
Having this be a submodule of `orchard_ffi` while following Rust 2018
module structure made it impossible to use the `include!` macro on
`orchard_ffi.rs`, which is exactly what the `zcash_script` crate does.
See this comment for details:
https://github.com/rust-lang/rust/issues/50132#issuecomment-969450868
To resolve this, we restructure the FFI library crate to only have FFI
methods in "leaf" module files.
Sean Bowe
Kris Nuttycombe
Larry Ruane
Pieter Wuille
sasha
Jack Grigg
Conrado Gouvea
Steven
Steven Smith
John Newbery
therealyingtong
Kris Nuttycombe
str4d
Daira Hopwood
Charlie O'Keefe
zancas
Steven Smith
Marius Kjærstad
MarcoFalke