Previously didn't make clear that the ContextualCheckBlock* functions
meant the block headers as context - not the UTXO set itself - and that
ConnectBlock() also did UTXO-related validity checks (in the future we
may split that functionality into a separate UTXO-specific contextual
check block function).
Also, reordered to put validity checks first for better readability.
1) Chainparams: Explicit CChainParams arg for main:
-AcceptBlock
-AcceptBlockHeader
-ActivateBestChain
-ConnectTip
-InitBlockIndex
-LoadExternalBlockFile
-VerifyDB parametric constructor
2) Also pickup more Params()\. in main.cpp
3) Pass nPruneAfterHeight explicitly to new FindFilesToPrune() in main.cpp
Don't allow Sprout-to-Sapling migration when syncing during IBD or after wake from sleep.
Prevent migration transactions from being created in response to incoming blocks when a node launches and syncs (the initial block download phase) and when a node wakes from sleep/hibernation and starts syncing old blocks rapidly.
The extra specifier meant that a runtime error would be thrown
during Sprout to Sapling migration if `zrpcunsafe` logging
was enabled:
"tinyformat: Too many conversion specifiers in format string"
Activate turnstile on mainnet
This PR enables [ZIP209](https://github.com/zcash/zips/blob/master/zip-0209.rst) support on mainnet, to mark blocks as invalid if they would lead to a turnstile violation in the Sprout or Sapling value pools.
To test this PR, I performed the following manual tests:
1. Used RPC call `getblock` to verify the fallback Sprout value.
2. Individually changed the fallback Sprout block hash, block height and chain value, recompiling and relaunching the node, verifying that each individual change resulted in an error. When the block hash and block height are incorrect, an error is logged to debug.log `FallbackSproutValuePoolBalance(): fallback block hash is incorrect`. An incorrect chain value results in node termination with error: `void FallbackSproutValuePoolBalance(CBlockIndex*, const CChainParams&): Assertion '*pindex->nChainSproutValue == chainparams.SproutValuePoolCheckpointBalance()' failed.`
3. Ran the `Smoke Testing` described in PR #3885, on mainnet.
4. Launched zcashd with experimental feature `-developersetpoolsizezero` to manually trigger a turnstile violation in both Sprout and Sapling shielded pools. The Sprout turnstile violation occurred after launch, due to chance, when the next incoming block 520786 contained a Sprout unshielding transaction. The Sapling turnstile violation was triggered after creating a Sapling unshielding transaction.
add SpentIndex changes needed for bitcore block explorer
Addresses #3708, follow-on to #3837, note there are no tests yet (will come later with RPC interfaces).
init: Fix new HD seed generation for previously-encrypted wallets
Closes#3607.
How to verify (with `zcashd` flags `-testnet -wallet=wallet.3607.dat -experimentalfeatures -developerencryptwallet`):
- Start `zcashd` 2.0.0, encrypt the wallet, and stop the node.
- Start `zcashd` 2.0.1+ (before this branch), and see that it crashes during startup.
- Start `zcashd` built from this branch, and see that it does not crash during startup. Unlock the wallet, then stop.
- Start `zcashd` 2.0.1+ (before this branch), and see that it no longer crashes during startup.
Test turnstile violation
Adds experimental feature -developersetpoolsizezero to enable developers to test Sprout and Sapling turnstile violations on testnet and in regtest mode.
depends: Support additional cross-compilation targets in Rust
This will make it easier for third parties to cross-compile `zcashd` for other platforms. The third commit in this PR shows how to add a new target to the Rust dependency builder.
The default Rust target during cross-compilation is the canonical host, which is derived from `HOST` using `depends/config.sub`. If the canonical host differs from the required Rust target, add the necessary mapping in addition to the target itself.
Also includes fixes for cross-compiling aarch64 targets.
fix enable-debug build DB_COINS undefined
Fixes#3860. The way `DB_COINS` is defined in this PR is the same as in `src/txdb.cpp` (static "global" instead of class member).
Add Sprout support to TransactionBuilder
The logic in `TransactionBuilder::CreateJSDescriptions()` is a combination of the logic used by `z_sendmany`, `z_mergetoaddress` and `z_shieldcoinbase`, in order to support arbitrary numbers of Sprout inputs or outputs.
Reject blocks that violate turnstile
This is an implementation of a consensus rule which marks blocks as invalid if they would lead to a turnstile violation in the Sprout or Shielded value pools. The motivations and deployment details can be found in the [accompanying ZIP draft](https://github.com/zcash/zips/pull/210).
**This PR only introduces the rule for testnet at the moment.**
We achieve the institution of this rule in three ways:
1. Nodes prior to #2795 did not record the "delta" in the Sprout value pool balance as part of the on-disk block index. This was a long time ago, though, and all nodes that are consensus-compatible with the network today have been recording this value for newer blocks. However, the value is absent from older block indexes unless the node has reindexed or synchronized from scratch in the time since. We shouldn't need to require nodes to reindex in order to enforce this consensus rule. We avoid this problem by falling back on a hardcoded Sprout shielded value pool balance in a very recent block.
2. If during `ConnectBlock` we observe that the resulting shielded value pool balance of Sprout or Sapling is negative, we reject the block.
3. During the miner's block assembly process the miner will skip over transactions if adding them to the assembled block might violate the turnstile, since the resulting block would be invalid. This means that theoretical transactions violating the turnstile would still be relayed in the network (and made available in users' memory pools) and so a turnstile violation would have some visibility outside of block relay.
## Smoke Testing
It's really tricky to test the behavior that automatically falls back to hardcoded shielded value pool balances in our architecture because it's very testnet-specific and node-version-specific. However, we can do some smoke tests to see that everything is working.
I modified the serialization of `CDiskBlockIndex` to serialize `boost::none` for `nSproutValue`
```
if ((s.GetType() & SER_DISK) && (nVersion >= SPROUT_VALUE_VERSION)) {
boost::optional<CAmount> nSproutValueFake = boost::none;
READWRITE(nSproutValueFake);
}
```
and then began a reindex of my node which I interruped around height 130k on testnet. I then restored the original serialization and resumed the reindex; I have thus _roughly_ simulated a older node "upgrading" to a newer node that records the deltas when processing new blocks. My node showed pool monitoring was disabled, as expected, for Sprout. I confirmed that some blocks following the reindex had nonzero Sprout `valueDelta` from `getblock`, as expected. I finished the reindex, restarted the node, and confirmed that the serialization worked for newer blocks but not older blocks by querying `getblock`, simply as a reassurance.
Finally, I introduced the code in this PR and reloaded the node. The desired behavior (that the chain began to be "monitored" again) worked, and the values were consistent with the hardcoded constant. I then made a payment to a Sprout z-addr from the transparent pool and the pool value increased as expected, as reported by `getblockchaininfo`. I reindexed the node again to exercise the remaining logic and check for turnstile violations throughout the history of testnet; there were none.
The code was already compiled as part of the wallet, but the tests were
not, meaning that the tests would fail to compile when the wallet was
disabled.
Simplify DisconnectBlock arguments/return value
DisconnectBlock currently has a complicated interface:
Situation Return value
pfClean != nullptr pfClean == nullptr
All good: true true
Failure: false false
Unclean rewind: true false
with *pfClean=false
Change this to return a tristate enum instead. As an added bonus,
remove the ValidationState& argument which was unused.
DisconnectBlock currently has a complicated interface:
Situation Return value
pfClean != nullptr pfClean == nullptr
All good: true true
Failure: false false
Unclean rewind: true false
with *pfClean=false
Change this to return a tristate enum instead. As an added bonus,
remove the ValidationState& argument which was unused.
Update zcbenchmarks to include sapling data
Closes#3395
This PR adds a benchmark named `trydecryptsaplingnotes` which is intended to be similar to `trydecryptnotes`. It also adds a benchmark `incnotewitnessessapling` which is similar to `incnotewitnesses`.
As a side note, while looking for examples to follow I ran in to a fair amount of setup, which I wanted to be able to reuse, repeated across several tests. I pulled some of that logic in to a utility functions and refactored the existing tests using that setup.
zmq: add flag to publish all checked blocks
This change adds a hook for the BlockChecked signal to the zmq publisher. This is useful for light wallet daemon initialization (see https://github.com/zcash/zcash/issues/3638 for context, and [lightwalletd](https://github.com/zcash-hackworks/lightwalletd) for implementation).
The new flag is `-zmqpubcheckedblock=address`, in keeping with the established style.
Return more information when building a transaction fails
This PR is intended to make it easier to diagnose what went wrong when building a transaction using `TransactionBuilder` fails.
On shutdown, wait for miner threads to exit (join them)
Closes#2926. Have `Shutdown()` running in the main thread wait for miner threads to exit before exiting.
Allow user to ask server to save the Sprout R1CS to a file during startup.
This adds an experimental feature `-savesproutr1cs` which can be used to save the file `r1cs` containing the constraint system used in the original launch of Zcash. The file is written to the parameters directory. This can be used to recover this file for verification of the Sprout MPC transcript and parameters.
initialize pCurrentParams in TransactionBuilder tests
In issue https://github.com/zcash/zcash/issues/3715 the failing tests were calling Params(), which includes an assertion that pCurrentParams is not null, without first calling SelectParams(), which will set that pointer. All the other tests in the same test case (which don't fail in the manner described in #3715 ) start out by calling `SelectParams(CBaseChainParams::REGTEST);`.
This change adds an identical call to the affected tests, getting past the failed assertion in Params() on pCurrentParams.
Here is a GitLab pipeline showing output from these tests run in isolation before this change:
https://gitlab.com/charlieok/zcash/pipelines/38304730
...and here is one with the same set of tests after this change:
https://gitlab.com/charlieok/zcash/pipelines/38307556
This change adds a hook for the BlockChecked signal to the zmq
publisher. This is useful for light wallet daemon initialization.
The new flag is `-zmqpubcheckedblock=address`
These three tests were failing when run in isolation, or sporadically
when test order is randomized, since Params() includes an assertion that
pCurrentParams is not null.
When responding to "mempool" message, do not include the txid of an
expiring soon transaction in the "inv" message reply.
When responding to "getdata" message, do not reply with a "tx" message
for a transaction which is expiring soon.
Don't accept transactions which are about to expire (next 3 blocks).
Don't set a ban score if a peer does propragate these transactions.
See ZEC-013 for more detail.
Load sapling chain value into memory
`CBlockIndex::nSaplingValue` has been correctly set and written to disk since before Sapling activated, meaning that all nodes now are correctly tracking the Sapling shielded pool value on-disk. However, on restart the per-block values are not being read into memory, and so the in-memory pool value appears to be zero on every restart. Setting `nSaplingValue` in-memory during block index loading fixes the problem.
Add a call to SetupNetworking in BasicTestingSetup
In issue https://github.com/zcash/zcash/issues/3668 , the error code returned by `getaddrinfo_a` (10093) is `WSANOTINITIALISED`:
> Successful WSAStartup not yet performed. Either the application has not called WSAStartup or WSAStartup failed. The application may be accessing a socket that the current active task does not own (that is, trying to share a socket between tasks), or WSACleanup has been called too many times.
https://docs.microsoft.com/en-us/windows/desktop/winsock/windows-sockets-error-codes-2
`WSAStartup` is called from the `SetupNetworking` function in `util.cpp`, but `SetupNetworking` was not being called during test setup. This pull request adds that call.
Here is a gitlab pipeline running tests in the `test_bitcoin.exe` executable before this change:
https://gitlab.com/charlieok/zcash/pipelines/36483478
...and here is the same set of tests run after this change:
https://gitlab.com/charlieok/zcash/pipelines/36485098
Update IncrementalMerkleTree test vectors to use valid commitments
The original commitments were SHA256 outputs, and some were outside the
scalar field. This didn't affect the Merkle hash, which drops the high
bit from each commitment, but it does affect the creation of the Merkle
path in Rust, which requires path nodes to be valid scalars.
Here, we explicitly drop the high bit of all test vector commitments,
as well as reducing the two that remain outside the field. The test
vectors still pass, and can now also be used in the Rust implementation.
3604 fix sapling default memo - 0xF6 plus zeros
Closes#3604. For Sapling outputs, set the default memo to `no_memo` instead of all zeros. (See section 5.5 of the Sapling protocol specification.)
Closes#3329. Send alert to put non-Sapling nodes into safe mode.
The alert targets nodes running protocol version <= 170006.
Sapling compatible nodes run protocol version >= 170007.
When processing a new transaction, in addition to spending the Coins of its txin's it creates a new Coins for its outputs. The existing ModifyCoins function will first make sure this Coins does not already exist. It can not exist due to BIP 30, but because of that the lookup can't be cached and always has to go to the database. Since we are creating the coins to match the new tx anyway, there is no point in checking if they exist first anyway.
Zcash: Modified to account for the fact that BIP 30 and BIP 34 have applied
from the beginning.
Closes#3467. Add benchmarks for Sapling spends and outputs.
Four new benchmarks are added to RPC zcbenchmark:
- createsaplingspend
- createsaplingoutput
- verifysaplingspend
- verifysaplingoutput
Remove GetUnspentFilteredNotes
Closes#3580
This PR generalized `GetFilteredNotes`, expanding its functionality to also do what we do in `GetUnspentFilteredNotes`. This enables us to remove the latter.
Jorge Timón
MarcoFalke
Peter Todd
Jack Grigg
Daniel Kraft
Eirik0
Homu
Simon
Daira Hopwood
Larry Ruane
str4d
Gareth Davies
Braydon Fuller
Sean Bowe
Marius Kjærstad
Jonas Schnelli
Pieter Wuille
mdr0id
George Tankersley
Charlie O'Keefe
Jonathan "Duke" Leto
arielgabizon
Alex Morcos