Fix potentially misleading test failures
This is in response to https://github.com/zcash/zcash/pull/3590#discussion_r224677644
Currently statements such as:
```assert_equal("Embarassment", "Embarrassment", "Misspelling detected")```
Will produce essentially the following error:
```
Assertion failed: Misspelling detected; expected: <Embarassment> but was: <Embarrassment>
File "/home/eirik/Development/zcash/qa/rpc-tests/test_framework/test_framework.py", line 121, in main
self.run_test()
File "/home/eirik/Development/zcash/qa/rpc-tests/test_spelling.py", line 13, in run_test
```
Which is misleading because the item on the left is not what is actually expected.
This PR changes the assertion failure to be displayed as:
```
Assertion failed: (left == right); Misspelling detected
left: <Embarassment>
right: <Embarrassment>
File "/home/eirik/Development/zcash/qa/rpc-tests/test_framework/test_framework.py", line 121, in main
self.run_test()
File "/home/eirik/Development/zcash/qa/rpc-tests/test_spelling.py", line 13, in run_test
```
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.
Make it clear that CWallet::GenerateNewZKey is Sprout specific
Closes https://github.com/zcash/zcash/issues/3577
When adding sapling support we had considered making the method `GenerateNewZKey` generic, but it ended up making more sense to add a second method `GenerateNewSaplingZKey` for sapling support.
This PR changes the name of `GenerateNewZKey` to `GenerateNewSproutZKey` and changes the return type from `libzcash::PaymentAddress` to `libzcash::SproutPaymentAddress` to make it more clear that this is what the method does.
Clarify in sendmany/z_sendmany rpc docs that amounts are not FP
Fixes#3539
I clarified in the docs that only 8 digits of precision are allowed. For example, if you try 9 digits you will get:
```
zcash-cli z_sendmany $TADDR "[{\"address\":\"$ZADDR\",\"amount\":0.123456789}]"
error code: -3
error message:
Invalid amount
```
Better error message when sending to both sprout and sapling
When trying to send to both Sprout and Sapling (not currently supported with z_sendmany) we were getting the following error in our operation result: `general exception: boost::bad_get: failed value get using boost::get`.
This PR changes this to fail with a better error message before the async operation begins:
```
error code: -8
error message:
Cannot send to both Sprout and Sapling addresses using z_sendmany
```
Resolves Sapling nullifier persistence issue when importing a key.
During a rescan, a CWalletTx was persisted to disk before it had its
note data set. This meant that upon restart, the CWalletTx would
potentially be missing its nullifiers causing the balance to include
notes which had already been spent.
The resolution is to force a CWalletTx to be persisted after it has had
its nullifiers set correctly, before the note witnesses are updated.
Allow minimum-difficulty blocks on testnet
This is a consensus rule change on testnet that will result in a chain split (leaving the stuck chain, as desired).
Reverts #2766 and part of #1338.
Closes#3552.
Replace Sapling support-related asserts with JSON errors
This helps prevent users from triggering an assert if they pass in Sapling addresses to RPC calls which do not fully support Sapling yet: z_mergetoaddress, z_importviewingkey and z_exportviewingkey.
Part of #3533.
Fix signing raw transactions with unsynced offline nodes
This PR address the issue in two different ways:
- In `signrawtransaction` we determine the consensus branch ID (which we then later use to construct the transaction) using the chain height. We now also consider the `APPROX_RELEASE_HEIGHT` as this is a better estimation than 0 for the height of the chain if we are unsynced. (This in and of itself solves the Overwinter signing issue).
- We have added an additional parameter to `signrawtransaction` to allow manually overriding the consensus branch ID that zcashd determines we are on. This allows users to work around corner cases where the first strategy is still insufficient.
Closes#3327.
Test peer banning logic in both pre- and post-initial block download states
The DoS ban scores are different for each, and it's nice to test both.
Follow-on from #3410.
Rpc test cleanup
The main purpose of this PR was to inline the method 'wait_for_async_operation_id' from util.py in tests where we were doing this manually. To make it work in all cases, I had to change the method to return the result in the case where the call failed. I also did a bit of general clean up (removing semicolons) in the files which I changed.
Fix assertion failure in circuit.merkle_tree_gadget_weirdness test on Windows
zcash-gtest.exe fails
```
[ RUN ] circuit.merkle_tree_gadget_weirdness
Assertion failed!
Program: C:\zcash-gtest.exe
File: ./snark/libsnark/gadgetlib1/gadgets/basic_gadgets.tcc, Line 50
Expression: this->pb.lc_val(packed).as_bigint().num_bits() <= bits.size()
```
The argument type of `__builtin_clzl()` function is unsigned long.
So, we need to replace `__builtin_clzl` with `__builtin_clzll`. (The argument type of `__builtin_clzll()` is unsigned long long (64bit)).
Refactor ContextualCheckBlock tests
This cleans up and condenses our quadratically-scaling transaction format / consensus rule tests. They still technically scale quadratically, but should be much more readable and manageable now - all that remains in the individual test cases is the specific version flag tweaking we're trying to test.
Closes#3187.
Use ZIP 32 for all Sapling spending keys
The wallet now only stores Sapling extended spending keys, and thus can
only be used with keys generated from an HDSeed via ZIP 32. This means
that all Sapling keys and addresses generated by users can be recovered
as long as they have a backup that includes the seed.
Depends on zcash/librustzcash#29
Closes#3380.
[ci-workers] Add support for MacOSX
Also alters the `unix.yml` script to install all Python modules in a virtualenv, and adds curl and cmake dependencies.
Closes#2469.
Add Sapling support to z_getbalance and z_gettotalbalance
Also includes preparatory changes for various other RPCs that depend on `GetFilteredNotes` etc.
Closes#3214.
Revise help output for z_sendmany
The previous version of the usage message for z_sendmany used the word "Change"
in an ambiguous way.
The intent was to describe "Change" the noun that indicates the portion of a
transaction's funds that are returned to the payer, however the context did not
provide an immediate cue to differentiate from the verb form meaning
"to alter". Indeed, in other help strings in the same file that meaning of the
word is correct, i.e. the verb form is used. Moreover a typo in the form of a
missing "a" later in the same sentence exacerbated the problem, by decreasing
readability precisely where context could reduce confusion.
generalize mininode.py protocol versioning
Fixes#2982 -- replace `overwintered` boolean argument to NodeConn constructor with a protocol version integer, so that adding Sapling (and beyond) will be easier.
Track Sapling notes and nullifiers in the wallet (in-memory only, no persistence to disk)
Part of #3061. Add in-memory tracking of Sapling notes and nullifiers to the wallet.
Set Sapling mainnet activation height
In mainnet, block #419200 is expected to appear on October 28, 2018, Zcash's birthday.
In testnet, block #280000 is expected about a week from now. (Current height is ~275249, plus 4032 blocks for a week, plus a little more just to make the number nice.)
These are the activation heights for Sapling in mainnet and testnet, respectively. Protocol version is also changed.
Always write the empty root down as the best root, since we may roll back
In [`3577de83`](https://github.com/zcash/zcash/commit/3577de83aa126fec4db8bb022dbeecf9a77a3af8) we started not writing the Sapling empty root down as the "best" anchor because we had changed the encodings and didn't want users who compiled from master to have inconsistent coindb's in the future if the encoding changed again for some reason.
However, if we don't write the empty root down then during rollbacks to Sapling activation we leave the best anchor on disk different from what's in the cache, which will trigger an assertion.
This reverts the change from `3577de83` since we've settled on the encodings.
ZIP 32 preparations
Includes Makefile changes cherry-picked from the following upstream PRs:
- bitcoin/bitcoin#7689
- bitcoin/bitcoin#10849
Part of #3380.
Update CCryptoKeyStore with Sapling support
Sapling crypter overrides for various `CCryptoKeyStore` functions such as:
- `HaveSaplingSpendingKey()`
- `GetSaplingSpendingKey()`
Also includes some changes to prepare for diversified addresses and ZIP 32.
Closes#3389
TransactionBuilder for Sapling and transparent transactions
This PR includes zcash/librustzcash#24, and will create a testnet chain split once merged into master (and once a Sapling transaction is mined).
Ensure sum of valueBalance and all vpub_new's does not exceed MAX_MONEY
HT to @daira for noticing that we don't check this in `CheckTransactionWithoutProofVerification`. We actually do check it in `GetShieldedValueIn` (by throwing an exception) but these exceptions are not handled by `CheckTxInputs`, which may cause a remote DoS.
Not exploitable assuming our proving system is secure, but definitely worthy of a fix.
don't ban peers when loading pre-overwinter blocks
Addresses #3399. If we see an invalid transaction (from our peers who are up to date, post-overwinter) during initial block download, don't invoke DoS (logging messages, blocking these peers). Instead, simply ignore the transaction.
Remove extra slash from lockfile path
Issue: https://github.com/zcash/zcash/issues/3415
After this change I can successfully run `fetch-params` in an msys2 environment.
Cache Sapling witnesses in the wallet
Closes#3062
I have not update the tests in test_wallet.cpp. Also, there are several other methods in the wallet that have to do with witnesses and note data which will need to be updated, but this PR focuses on IncrementNoteWitnesses and DecrementNoteWitnesses.
Sapling note plaintext (encryption and decryption)
Add encryption and decryption of SaplingNotePlaintext and SaplingOutgoingPlaintext classes.
This is part of #3061 to add Sapling note functionality.
Add call to sync_all() after (z_sendmany, wait)
Closes#2929. Add missing call to `self.sync_all()` after sending coins (and waiting for the transaction to complete). This matches what is done in other python test scripts.
add extra help how to enable experimental features
Closes#2728. When the user attempts to execute a disabled experimental RPC, print some nice text explaining how to enable the RPC (either command-line or adding lines to zcash.conf).
Add -Wl,-pie linker option for macOS and use it instead of -pie
fixes linker error (see below) due to "-pie" usage, replaced it with "-Wl,-pie" for macOS platform
```
clang: error: argument unused during compilation: '-pie' [-Werror,-Wunused-command-line-argument]
```
Update README.md
Added default options to build zcash with as many parallel compilation units as there are cpu cores.
This makes compiling zcash multiple times faster.
Fix absurd fee bug reported in #3281, with tests
This was ported from the Hush `absurd_fee_bug` PR: https://github.com/MyHush/hush/pull/136
Tests executed with `./qa/pull-tester/rpc-tests.sh wallet` and pass on Ubuntu 16.04. Some tests were added for previously untested behavior as well.
Closes#3281.
Add change indicator
This adds to the json returned when calling z_listreceivedbyaddress and z_listuspent an additional field entitiled 'change' which will be either true or false depending on whether or not the note was change from another transaction.
Extend JoinSplit tests to Groth
As we now have both PGHR and Groth proofs for Sprout - make the joinsplit tests check the Groth proofs as well.
See https://github.com/zcash/zcash/issues/3266
(rpc-test) accurately account for fee without rounding error
Fix for #2807, this test compares balances after doing key exports and imports, and expects these balances to be equal. But they are not exactly equal due to transaction fees, so the test makes them "equal" by rounding a value that has had fees taken out up to the nearest hundredth of a unit (which is much more than the default fee). This obviously is somewhat sloppy. It also converts a balance to float, which really should never be used due to loss of precision (use Decimal instead).
This change makes the test accurately account for the fee using precise comparisons, and removes the use of float. This test doesn't depend on the default fee (0.0001) but instead sets the fee (to that value). This way, if the default fee changes in the future, this test will continue to run. While testing these changes, I set the fee to various values (up to the max, 0.0190), and the test still passes.
Closes#3328. Send alert to put non-Overwinter nodes into safe mode.
The alert targets nodes running protocol version <= 170004.
Overwinter compatible nodes run protocol version >= 170005.
Sapling address encodings
This PR enables Sapling keys and addresses to be passed in anywhere Sprout keys
and addresses are used. Doing so will cause crashes until those places are updated
with Sapling support.
Includes code cherry-picked from the following upstream PRs:
- bitcoin/bitcoin#11167
- Only the `ConvertBits()` function.
- bitcoin/bitcoin#11630
Closes#3058.
Homu