The previous code did not mine enough blocks to have sufficient matured
funds for the tests it needed to perform, taking into account the slow
start. So, we now mine 200 instead of 110 blocks.
Signed-off-by: Daira Emma Hopwood <daira@jacaranda.org>
* used "fee" to mean "fee rate", "kB" to mean 1000 bytes, "satoshis"
to mean zatoshis, or that incorrectly used "BTC" in place of "ZEC";
* used obsolete concepts such as "zero fee" or "free transaction"; or
* did not accurately document their applicability.
Uses of "satoshis" as a JSON key are not altered.
Signed-off-by: Daira Emma Hopwood <daira@jacaranda.org>
(We express it that way rather than 300 zats/1000 bytes, because the
threshold is always rounded to an integer and then multiplied by 3.)
Bitcoin Core added the concept of "dust" in bitcoin/bitcoin#2577.
At that point the dust threshold was tied to three times the
minRelayTxFee rate, with the motivation that if you'd pay more than
a third of the minimum relay fee to spend something, it should be
considered dust. This was implemented as a standard rule rejecting
dust outputs.
This motivation will not apply after ZIP 317 block construction
is implemented: at that point the ZIP 317 marginal fee will be
5000 zats per logical action, but the dust threshold rate will
still be three times 100 zats per 1000 bytes. Those costs would
only coincide if the marginal size per logical action were
5000/300 * 1000 ~= 16667 bytes, and in practice the marginal size
for any kind of input is much smaller than that.
However, to avoid interoperability problems (older wallets creating
transactions that newer nodes will reject because they view the
outputs as dust), we will have to coordinate any increase in the
dust threshold carefully.
More history: in Zcash the minRelayTxFee rate was 5000 zats/1000 bytes
at launch, changed to 1000 zats/1000 bytes in zcashd v1.0.3 and to
100 zats/1000 bytes in zcashd v1.0.7-1 (#2141). The relaying problem
for shielded transactions (#1969) that prompted the latter change was
fixed more thoroughly by the addition of `CFeeRate::GetFeeForRelay`
in #4916, ensuring that a transaction paying `DEFAULT_FEE` can always
be relayed. At the same time the default fee was set to 1000 zats,
per ZIP 313.
An earlier commit in this PR changed relaying policy to be more strict
about enforcing minRelayTxFee. The commit just before this one also
allowed `-minrelaytxfee=0`, which we are going to use to avoid some test
breakage. But if the dust threshold rate were still set to three times
the minRelayTxFee rate, then setting `-minrelaytxfee=0` would have the
side effect of setting the dust threshold to zero, which is not intended.
Bitcoin Core took a different approach to disentangling the dust
threshold from the relay threshold, adding a `-dustrelayfee` option
(bitcoin/bitcoin#9380). We don't want to do that because it is likely
that we will change the dust policy again, and adding a user-visible
config option might conflict with that. Also, it isn't a good idea for
the dust threshold rate to be configurable per node; it's a standard
rule parameter and should only be changed with network-wide coordination
(if it is increased then wallets have to change before nodes, and vice
versa if it is decreased). So for now we set it to a constant that
matches the behaviour before this PR.
Since we can no longer modify the dust threshold, we remove a check
from transaction_tests.cpp that relied on doing so.
This change also indirectly fixes a false-positive assertion error that
would occur in `SpendableInputs::LimitToAmount` if we allowed the dust
threshold to be zero.
Signed-off-by: Daira Emma Hopwood <daira@jacaranda.org>
Remove GetPriority and ComputePriority. Remove internal machinery for tracking priority in CTxMemPoolEntry.
(cherry picked from commit bitcoin/bitcoin@359e8a03d1)
Zcash:
* We don't have `src/bench/mempool_eviction.cpp`.
* We don't have `-walletrejectlongchains`.
* Now we can remove `MAX_PRIORITY`.
* Fix a comment in `coins.h` while we're changing code next to it.
* Update the `Mempool/PriorityStatsDoNotCrash` regression test.
Signed-off-by: Daira Emma Hopwood <daira@jacaranda.org>
Remove all coin age priority functionality from unit tests and RPC tests.
(cherry picked from commit bitcoin/bitcoin@0315888d0d)
Zcash:
* We cannot remove the `pool` parameter from the `CTxMemPool` constructor
because we do not have bitcoin/bitcoin#9138. (Backporting that PR is
unnecessary and would be a distraction from the purpose of this one;
the changes made by it are orthgonal.)
* We don't have `prioritise_transaction.py`, `MempoolAncestorIndexingTest`,
`MempoolSizeLimitTest`, or the `estimateSmartFee` functionality, so omit
the changes for those.
Signed-off-by: Daira Emma Hopwood <daira@jacaranda.org>
0bd581a add release notes for removal of priority estimation (Alex Morcos)
b2322e0 Remove priority estimation (Alex Morcos)
(cherry picked from commit bitcoin/bitcoin@3c03dc2cfc)
Zcash:
* MAX_PRIORITY is still needed because it's used for the priority of
shielded transactions.
* Changes relating to "smart priority" are omitted since we do not have
that.
* Change the new minimum client version for `fee_estimates.dat` to
FEE_ESTIMATES_WITHOUT_PRIORITY_VERSION == 5050000. Immediately make
sure we write at least that version and accept it for reading.
Signed-off-by: Daira Emma Hopwood <daira@jacaranda.org>
When a `zcash-cli` command fails, it attempts to print the help message for the command. However,
making the `help` call can also fail, and there was a bug in this check, so that we tried to display
the help message when the `help` call failed, and tried to display the error when the `help` call
succeeded – both leading to an assertion failure.
This also makes some minor changes to the output formatting.
Fixes#6561
Instead of storing the indices of args to convert from string, store two
`vector<bool>` (per operation), the first containing an entry for each required
parameter (`true` if we should convert it), and the second containing an entry
for each optional parameter.
This allows us to check a few more things on the client side:
- does the operation exist
- have enough arguments been passed
- have too many arguments been passed
This is ostensibly a fix for `zcash-cli` to be able to use `asOfHeight` where
available, but it also caught a few bugs in the old implementation:
- `submitblock` didn’t convert its optional (but ignored) second arg;
- `z_getpaymentdisclosure` docs claimed all the args were strings, but two are
actually ints;`
- `listreceivedbyaddress` didn’t convert the optional `includeImmatureCodebase`;
- `listsinceblock` didn’t convert the optional `includeRemoved` and
`includeChange`;`
- `gettransaction` didn’t convert `verbose`;
- `listunspent` didn’t convert `includeUnsafe` or `queryOptions`;
- `z_getbalanceforviewingkey` didn’t convert minconf; and
- a minor non-bug – `z_getbalanceforaddress` had a handler even though the
operation has been removed.
`getblockdeltas` also incorrectly tries to convert its required string argument,
but correcting that would be a breaking API change. Instead, it is deferred to
Fixes#6429.
zcash/zcash#5987 added a bridge to `orchard::Bundle<Authorized, Amount>`
for `getrawtransaction`. This commit expands it to also cover the
consensus rules, by migrating over missing functionality from the
hand-written FFI methods, and exposing the Orchard `BatchValidator` type
directly (as with Sapling) instead of via the C++ `AuthValidator`
intermediary.
Part of zcash/zcash#6397.
Notable changes
===============
This hotfix remediates memory exhaustion vulnerabilities that zcashd inherited
as a fork of bitcoind. These bugs could allow an attacker to use peer-to-peer
messages to fill the memory of a node, resulting in a crash.
-----BEGIN PGP SIGNATURE-----
iQGzBAABCgAdFiEEX8Nd8pnYcf0pobEL9FXpuSAjoYsFAmP1CD8ACgkQ9FXpuSAj
oYvGXQwAqaa1l5qC3VY/60jkH4xmWoSUEzeCOPUG7lYJrIEyzgXj8Ko0Cjr308jm
ISXDDTOxKb2hfnCbeqbZqRyFbzGzG5L6AkjHAvmQMiZwx2JlbH2k+jd5fggOZSMv
shL5KbxWN2YRftc8r+fDraJMbGULbKBWQooKaFyQmupT+bVsRf1Nh+lFIVG4FUwA
oWZot36wB6Y99Y57wlyN2m22+j1glyk5mKv2ttXYbdwSRTFB5W5L1US6Z8uxXQPb
Qa7sVO90QtzkHr+GPtMiTn513VuLFr+KArGn+qidU/PvblJI/vXuBf54g7JZw4Ot
gkKziatgaN3pO30I4rTij78LCgKJZ/WImLE7nwTl/bG2Ki1WfyolNLjS+1pXpcPL
xlXso5ioKlSIGhnlPouXwoxlaqTpDwRKLp0azNJl5hG/tXEHupToK2M61woi9LlP
4RB+a75OptFi9NMp/Sx0T8zNRn7OB8iP+3BgRP0+mzcLC1AHfjJk/IRo9CQh5jRZ
MMe98OrC
=6QYn
-----END PGP SIGNATURE-----
Merge tag 'v5.3.3' into hotfix-v5.4.2
zcashd release v5.3.3
Notable changes
===============
This hotfix remediates memory exhaustion vulnerabilities that zcashd inherited
as a fork of bitcoind. These bugs could allow an attacker to use peer-to-peer
messages to fill the memory of a node, resulting in a crash.
Use the score index on the mempool to only add sorted txs in order. Remove much of the validation while building the block, relying on mempool to be consistent and only contain txs that can be mined.
The mempool is assumed to be consistent as far as not containing txs which spend non-existent outputs or double spends, and scripts are valid. Finality of txs is still checked (except not coinbase maturity, assumed in mempool).
Still TestBlockValidity in case mempool consistency breaks and return error state if an invalid block was created.
Unit tests are modified to realize that invalid blocks can now be constructed if the mempool breaks its consistency assumptions and also updated to have the right fees, since the cached value is now used for block construction.
Conflicts:
src/miner.cpp
(cherry picked from commit bitcoin/bitcoin@553cad94e2)
Zcash: Merged in our changes.
Under some circumstances it is possible for there to be a significant,
discontinuous jump in a node's clock value. On mining nodes, this can
result in block templates which are no longer valid due to time-based
nLockTime constraints. UpdateTime() is modified so that it will never
decrease a block's nLockTime, thereby preventing such invalidations.
(cherry picked from commit bitcoin/bitcoin@ef8dfe41d1)
Zcash: Updated CreateNewBlock_validity test and wallet_1941 RPC test to
ensure we satisfy the future timestamp soft fork rule.
Associate with each CTxMemPoolEntry all the size/fees of descendant
mempool transactions. Sort mempool by max(feerate of entry, feerate
of descendants). Update statistics on-the-fly as transactions enter
or leave the mempool.
Also add ancestor and descendant limiting, so that transactions can
be rejected if the number or size of unconfirmed ancestors exceeds
a target, or if adding a transaction would cause some other mempool
entry to have too many (or too large) a set of unconfirmed in-
mempool descendants.
(cherry picked from commit bitcoin/bitcoin@5add7a74a6)
Zcash:
- Mempool methods were adapted to our mempool changes.
- Default ancestor and descendant size limits were double to account for
our larger block size.
- The mempool_packages RPC test fee was adapted to account for our
emissions curve (which results in a smaller per-block reward that
needs to be split into smaller shards for sequential transactions.
- Includes some modifications to account for us backporting
bitcoin/bitcoin@f3fe83673e early in
zcash/zcash#5269.
Zcash: Also includes some minor changes from scripted diffs that
occurred between backported PRs.
(cherry picked from commit bitcoin/bitcoin@4defdfab94)
Zcash: Commented out second BENCHMARK argument as we haven't backported
the benchmark refactor that adds it.
(cherry picked from commit bitcoin/bitcoin@d0c9632883)
-BEGIN VERIFY SCRIPT-
sed -i 's/insecure_randbits(1)/insecure_randbool()/g' src/test/*_tests.cpp
sed -i 's/insecure_randrange(2)/insecure_randbool()/g' src/test/*_tests.cpp
sed -i 's/insecure_randrange(2)/insecure_randbool()/g' src/gtest/test_*.cpp
sed -i 's/insecure_randrange(4)/insecure_randbits(2)/g' src/test/*_tests.cpp
sed -i 's/insecure_randrange(32)/insecure_randbits(5)/g' src/test/*_tests.cpp
sed -i 's/insecure_randrange(256)/insecure_randbits(8)/g' src/test/*_tests.cpp
-END VERIFY SCRIPT-
Zcash: Added gtests to script.
(cherry picked from commit bitcoin/bitcoin@2fcd9cc86b)
Zcash: Excludes changes to files we don't have, and removes the
duplicate definitions in `src/gtest/utils.h`.
(cherry picked from commit bitcoin/bitcoin@124d13a58c)
This change improves clock management for zcashd by ensuring
that all clock methods (obtaining seconds, milliseconds, and
microseconds since the epoch) agree under testing conditions
using `-mocktime`, and also adds a feature that allows tests
to specify an offset to the system clock; this is useful to
allow comprehensive testing of the "timejacking attack mitigation"
consensus rules.
This adds two new CuckooCaches in validation, each caching whether all
of a transaction bundle's proofs and signatures were valid.
Bundles which match the validation cache never have proofs or signatures
added to the batch validators. For blocks where all transactions have
been previously observed in the mempool, the final validation of the
batches should be a no-op.
Part of zcash/zcash#6049.
Daira Emma Hopwood
Alex Morcos
Wladimir J. van der Laan
Greg Pfeil
Kris Nuttycombe
Jack Grigg
Jack Grigg
Charlie O'Keefe
instagibbs
Suhas Daftuar
Mark Friedenbach
Suhas Daftuar
Dimitris Apostolou
DeckerSU
Pieter Wuille