We do not need to be able to calculate multiple SignatureHash versions for a
single transaction format; instead, we use the transaction format to determine
the SigVersion.
The consensus branch ID *does* need to be passed in from the outside, as only
the caller knows the context in which the SignatureHash is being calculated
(ie. mempool acceptance vs. block validation).
JoinSplit signature verification has been moved into ContextualCheckTransaction,
where the consensus branch ID can be obtained.
The argument to the sign command for zcash-tx has been modified to take a height
in addition to the optional sigtype flags.
Mempool improvements, branch ID awareness
Whenever the local chain tip is updated, transactions in the mempool which commit to an
unmineable branch ID (for example, just before a network upgrade activates, where the
next block will have a different branch ID) will be removed.
Includes commits cherry-picked from the following upstream PRs:
- bitcoin/bitcoin#6654
- Only the mempool index change.
- bitcoin/bitcoin#6776
- bitcoin/bitcoin#7020
- bitcoin/bitcoin#6915
Part of #2074.
This will prevent users from starting their nodes if they switch between
software versions that implement different network upgrades. It will also
prevent users from using the testnet if they have more than MAX_REORG_LIMIT
post-upgrade blocks, and the upgrade point is shifted in a newer software
version.
Use JoinSplitTestingSetup for Boost sighash tests
Symptom: When running all tests, the test suite passed. But when running the
sighash tests on their own, the test suite segfaulted.
Cause: The sighash tests depend on the proving parameters being accessible, but
BasicTestingSetup doesn't load them.
Add getdeprecationinfo RPC method to return deprecation block height
Closes#2828
Returns:
```
{
"version": xxxxx, (numeric) the server version
"subversion": "/MagicBean:x.y.z[-v]/", (string) the server subversion string
"deprecationheight": xxxxx, (numeric) the deprecation block height
}
```
Symptom: When running all tests, the test suite passed. But when running the
sighash tests on their own, the test suite segfaulted.
Cause: The sighash tests depend on the proving parameters being accessible, but
BasicTestingSetup doesn't load them.
Bitcoin 0.12 chainparams cleanups
Cherry-picked from the following upstream PRs:
- bitcoin/bitcoin#6222
- bitcoin/bitcoin#6381
- bitcoin/bitcoin#6473
- bitcoin/bitcoin#6242
Part of #2074.
Remove redundant service flag NODE_GETUTXO meant for Bitcoin XT.
For #2885. This frees up a service bit. Zcash has never used this flag
and upstream Bitcoin Core does not support this flag, although
it did reserve it for compatibility with Bitcoin XT.
Clean up the code in chainparams a bit more after the recent
refactorings. In particular, make sure the structure of the "RegTest"
params matches the structure of the other classes. This makes the code
clearer to read.
Also remove redundant values of the genesis block in always-specified
optional arguments and mark variable/argument as "const".
If/when CTransaction::CURRENT_VERSION is incremented, this will break CChainParams and the miner tests. This fix sets the transaction version explicitly where we depend on the hash value (genesis block, proof of work checks).
This frees up a service bit. Zcash has never used this flag
and upstream Bitcoin Core does not support this flag, although
it did reserve it for compatibility with Bitcoin XT.
Fix libsnark dependency build.
This changes libsnark to build in-place, instead of copying first to
a build directory. Previously, modifications made to the original
sources wouldn't get rebuilt without a 'make clean' because users
would be pointing to the copies.
This closes#2689.
The "zcviewingkey" field has never been documented before, and the method itself
is deprecated; this just ensures it is consistent with the rest of the RPC.
The wallet code previously assumed that an unlocked wallet would always
have a spending key associated with a note decryptor. Viewing keys break
this assumption.
This changes libsnark to build in-place, instead of copying first to
a build directory. Previously, modifications made to the original
sources wouldn't get rebuilt without a 'make clean' because users
would be pointing to the copies.
This closes#2689.
Track net value entering and exiting the Sprout circuit
Delta values will be stored for new blocks; old blocks can be filled in by
re-indexing. The net value currently in the Sprout circuit is only calculated
when delta values for all previous blocks are present.
Part of #2351.
Block indices are flushed to disk when they are marked as dirty, and this
happens via enough distinct pathways that it would be sufficiently complex to
update nSproutValue via all of them. Thus it is necessary to be able to
serialize "no value" for writes by upgraded clients.
Fix buffer overflow in libsnark
Applies `-fstack-protector-all` to libsnark, and fixes an underlying bug. This bug was not triggerable in `zcashd` because the function in question was being inlined by the compiler.
Closes#2788.
Build system improvements
Includes commits cherry-picked from the following upstream PRs:
- bitcoin/bitcoin#6978
- Only the first commit (second is for QT)
- bitcoin/bitcoin#7059
- bitcoin/bitcoin#7603
- Only the first commit (without the `BITCOIN_QT_BIN` variable; the rest are for QT)
- bitcoin/bitcoin#7954
- bitcoin/bitcoin#8314
- Only the second commit (first is for QT)
- bitcoin/bitcoin#8504
- Only the first commit (second was undoing something we didn't have)
- bitcoin/bitcoin#8520
- bitcoin/bitcoin#8563
- bitcoin/bitcoin#8249
- bitcoin/bitcoin#9156
- bitcoin/bitcoin#9831
- bitcoin/bitcoin#9789
- bitcoin/bitcoin#10766
Part of #2074.
Delta values will be stored for new blocks; old blocks can be filled in by
re-indexing. The net value currently in the Sprout circuit is only calculated
when delta values for all previous blocks are present.
Add examples for z_rpc calls
Adds missing cli and json-rpc examples for `z_` rpc calls
Closes#2723
(Accidentally named the branch being merged in with wrong issue number)
noexcept is default for destructors as of c++11. By throwing in reverselock's
destructor if it's lock has been tampered with, the likely result is
std::terminate being called. Indeed that happened before this change.
Once reverselock has taken another lock (its ctor didn't throw), it makes no
sense to try to grab or lock the parent lock. That is be broken/undefined
behavior depending on the parent lock's implementation, but it shouldn't cause
the reverselock to fail to re-lock when destroyed.
To avoid those problems, simply swap the parent lock's contents with a dummy
for the duration of the lock. That will ensure that any undefined behavior is
caught at the call-site rather than the reverse lock's destruction.
Barring a failed mutex unlock which would be indicative of a larger problem,
the destructor should now never throw.
Get rid of underscore prefixes for include guards.
This is because they may conflict with reserved names. Also took
this opportunity to make the guards in the zcash subdir a bit
more uniform by having them all conform to ZC_<...>_H_
This closes#1272
Remove Boost conditional compilation.
Boost is currently at version 106200 in the depends subsystem
so it is safe to remove code the refers to earlier versions.
This closes#2445
This is because they may conflict with reserved names. Also took
this opportunity to make the guards in the zcash subdir a bit
more uniform by having them all conform to ZC_<...>_H_
This closes#1272
Previously libsnark would try to build libgtest if /usr/src/gtest
exists on the build machine. This caused issues because the
version of libgtest in /usr/src isn't necessarily the same as
the one that the rest of zcash's gtests were built with.
Get the sec-hard tests to run correctly.
This fixes the way arguments were passed to security-check, and also
a typo in how BIND_NOW was being searched for in a list.
Also fix how symbol-check is invoked although that script isn't
currently used.
This fixes the way arguments were passed to security-check, and also
a typo in how BIND_NOW was being searched for in a list.
Also fix how symbol-check is invoked although that script isn't
currently used.
The new parameter is to satisfy the principle of least astonishment
by providing a sensible default for the maximum number of transparent
inputs to shield. If users do not configure -mempooltxinputlimit
it is possible for them to create transactions with hundreds of
inputs which suffer from mining delay, due to the current state of
the network where some miners have configured -mempooltxinputlimit
as a way to deal with the problem of quadratic hashing.
Fix various thread assertion errors caused during shutdown.
Cherry-picked from the following upstream PRs:
- bitcoin/bitcoin#6719
- bitcoin/bitcoin#6990
- bitcoin/bitcoin#8421
- Second commit only in this PR
- bitcoin/bitcoin#11006
I've cherry-picked the relevant commits, along with a note in each commit referring to the original Bitcoin commit ID (and the Zcash issue numbers where applicable). I've tested each issue with/without these patches applied.
Closes#2214, #2334, and #2554.
Move libsnark in-repo as a git subtree
This PR pulls in the libsnark subtree at the exact commit that we currently fetch via the depends system. To verify:
```
$ ./contrib/devtools/git-subtree-check.sh src/snark
src/snark in HEAD was last updated to upstream commit 9ada3f84ab484c57b2247c2f41091fd6a0916573 (tree c10a38c759)
src/snark in HEAD currently refers to tree 34e916d3f6
:100644 100644 427f4f4ce913e54da68b M Makefile
:040000 040000 42f29e42d1dd73536163 M src
FAIL: subtree directory tree doesn't match subtree commit tree
```
This shows that there are changes relative to what we currently use, due to the later commits in the PR. If we exclude them, we see that the code is identical:
```
$ git checkout 26a8f68ea8
$ ./contrib/devtools/git-subtree-check.sh src/snark
src/snark in HEAD was last updated to upstream commit 9ada3f84ab484c57b2247c2f41091fd6a0916573 (tree c10a38c759)
src/snark in HEAD currently refers to tree c10a38c759
GOOD
```
Closes#820.
Importprivkey shows address
Scratching an itch: make `importprivkey` output the corresponding address.
Without this PR, `importprivkey` shows no output. Because we're moving towards an "address-based" RPC interface, rather than "account-based", there's a gap when using `importprivkey` because there's no way to assign it to a specific account, but also no easy way to determine the address. This change fixes that wart.
Fixes for gcc 7
This fixes a few issues when using a newer compiler (in my case gcc 7.1.1) available in for example Arch Linux.
Solves for example this issue: https://github.com/zcash/zcash/issues/2304
The first thing is an error when checking for boost_system and is solved by disabling that warning (wich is treated as an error and, hence, stops the build):
```
configure:22242: checking for exit in -lboost_system-mt
configure:22267: g++ -m64 -o conftest -std=c++11 -pipe -fPIC -O1 -fwrapv -fno-strict-aliasing -Werror -g -Wformat -Wformat-security -Wstack-protector -fstack-protector-all -fPIE ..<snip>... /x86_64-unknown-linux-gnu/share/../lib conftest.cpp -lboost_system-mt -lanl >&5
conftest.cpp:70:6: error: declaration of 'char exit()' conflicts with built-in declaration 'void exit(int)' [-Werror=builtin-declaration-mismatch]
char exit ();
^~~~
cc1plus: all warnings being treated as errors
```
The second thing was to clean some code that is deprecated in C++11 which also lead to a warning treated as error. It could also be fixed with `-Wno-deprecated` but better to fix the issue in my opinion.
Previously various user-facing strings have used inconsistent currency units "BTC",
"btc" and "bitcoins". This adds a single constant and uses it for each reference to
the currency unit.
Also adds a description of the unit for --maxtxfee, and adds the missing "amount"
field description to the (deprecated) move RPC command.
When using std::thread in place of boost::thread, letting the threads destruct
results in a std::terminate. According to the docs, the same thing should be
be happening in later boost versions:
http://www.boost.org/doc/libs/1_55_0/doc/html/thread/thread_management.html#thread.thread_management.thread.destructor
I'm unsure why this hasn't blown up already, but explicitly detaching can't
hurt.
Zcash: cherry-picked from commit d3773ca9aeb0d2f12dc0c5a0726778050c8cb455
This fixes#2554 (zcash-cli stop during getblocktemplate long poll
causes 'Assertion `!pthread_mutex_unlock(&m)' failed.')
This continues/fixes #6719.
`event_base_loopbreak` was not doing what I expected it to, at least in
libevent 2.0.21.
What I expected was that it sets a timeout, given that no other pending
events it would exit in N seconds. However, what it does was delay the
event loop exit with 10 seconds, even if nothing is pending.
Solve it in a different way: give the event loop thread time to exit
out of itself, and if it doesn't, send loopbreak.
This speeds up the RPC tests a lot, each exit incurred a 10 second
overhead, with this change there should be no shutdown overhead in the
common case and up to two seconds if the event loop is blocking.
As a bonus this breaks dependency on boost::thread_group, as the HTTP
server minds its own offspring.
Zcash: cherry-picked from commit a264c32e3321ae909ca59cb8ce8bf5d812dbc4e1
This makes sure that the event loop eventually terminates, even if an
event (like an open timeout, or a hanging connection) happens to be
holding it up.
Zcash: cherry-picked from commit ec908d5f7aa9ad7e3487018e06a24cb6449cc58b
Add a WaitExit() call to http's WorkQueue to make it delete the work
queue only when all worker threads stopped.
This fixes a problem that was reproducable by pressing Ctrl-C during
AppInit2:
```
/usr/include/boost/thread/pthread/condition_variable_fwd.hpp:81: boost::condition_variable::~condition_variable(): Assertion `!ret' failed.
/usr/include/boost/thread/pthread/mutex.hpp:108: boost::mutex::~mutex(): Assertion `!posix::pthread_mutex_destroy(&m)' failed.
```
I was assuming that `threadGroup->join_all();` would always have been
called when entering the Shutdown(). However this is not the case in
bitcoind's AppInit2-non-zero-exit case "was left out intentionally
here".
Zcash: cherry-picked from commit de9de2de361ab1355b976f17371d73e36fe3bf56
Fixes#2334 and #2214.
Shutting down the HTTP server currently breaks off all current requests.
This can create a race condition with RPC `stop` command, where the calling
process never receives confirmation.
This change removes the listening sockets on shutdown so that no new
requests can come in, but no longer breaks off requests in progress.
Meant to fix bitcoin/#6717.
Zcash: cherry-picked from commit 5e0c22135600fe36811da3b78216efc61ba765fb
Corrections are to the median block times, which were generated by subtracting
CBlockIndex::nMedianTimeSpan / 2 from the block height and then multiplying by
the target spacing. GetMedianTimePast() takes an array sorted by std::sort() and
returns element CBlockIndex::nMedianTimeSpan / 2, meaning that if
CBlockIndex::nMedianTimeSpan is odd (which it is), there is an out-by-one error
in the subtraction.
Fixes#2480 where missing map entry would cause a segfault.
`wtxHeight = mapBlockIndex[wtx.hashBlock]->nHeight;` results in undefined behaviour when the block hash is not present in the map, returning a null value which is dereferenced via `->nHeight`. This error is triggered by a zero-conf wallet transaction which has not been mined yet. As discussed in #2480, on some systems there is a segfault whilst on others there is a silent exit. This makes it difficult to write a test, but the fix has been tested empirically to confirm an exception is thrown. This PR fixes the segfault and complements #2525 which prevents a user from sending from a zaddr with minconf 0.
Ariel
Jack Grigg
Pieter Wuille
Nicolas DORIER
Jorge Timón
Jonas Nick
21E14
Homu
Suhas Daftuar
Matt Corallo
Simon
Alex Morcos
Ashley Holman
jc
Jay Graber
Pavel Vasin
Cory Fields
Daniel Kraft
Mark Friedenbach
syd
kozyilmaz
Ariel Gabizon
Sean Bowe
João Barbosa
Jason Davies
Bruno Arueira
Nathan Wilcox
Ross Nicoll
Wladimir J. van der Laan