a473eff [bench] Replace 0.00(000)1 with MICRO/MILLI #defines in validation.cpp. (Karl-Johan Alm)
5f850b0 [bench] Include ms/blk stats in Connect* benchmarks. (Karl-Johan Alm)
Pull request description:
Display the average per block runtime for the various benchmarked times in the block connect functions to give an overview of long(er) term time distribution statistics.
Tree-SHA512: 3d6f24f6b9e3dbb448a647e2cda8e7b90ad6a16d4821f49f426a8e1ebc3ce5a0cf0a8cde82213e293affba441615702dfe50822c8c818e282af03bfe383d83e0
Instead of passing a StartShutdown reference all the way up from
txdb, give ShowProgress a "resumeable" boolean, which is used to
inform the user if the action will be resumed, but cancel is always
allowed by just calling StartShutdown().
90d4d89 scripted-diff: Use the C++11 keyword nullptr to denote the pointer literal instead of the macro NULL (practicalswift)
Pull request description:
Since C++11 the macro `NULL` may be:
* an integer literal with value zero, or
* a prvalue of type `std::nullptr_t`
By using the C++11 keyword `nullptr` we are guaranteed a prvalue of type `std::nullptr_t`.
For a more thorough discussion, see "A name for the null pointer: nullptr" (Sutter &
Stroustrup), http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2007/n2431.pdf
With this patch applied there are no `NULL` macro usages left in the repo:
```
$ git grep NULL -- "*.cpp" "*.h" | egrep -v '(/univalue/|/secp256k1/|/leveldb/|_NULL|NULLDUMMY|torcontrol.*NULL|NULL cert)' | wc -l
0
```
The road towards `nullptr` (C++11) is split into two PRs:
* `NULL` → `nullptr` is handled in PR #10483 (scripted, this PR)
* `0` → `nullptr` is handled in PR #10645 (manual)
Tree-SHA512: 3c395d66f2ad724a8e6fed74b93634de8bfc0c0eafac94e64e5194c939499fefd6e68f047de3083ad0b4eff37df9a8a3a76349aa17d55eabbd8e0412f140a297
85c82b5 Avoid masking of difficulty adjustment errors by checkpoints (Pieter Wuille)
Pull request description:
Currently difficulty adjustment violations are not reported for chains that branch off before the last checkpoint. Change this by moving the checkpoint check after the difficulty check.
Tree-SHA512: 33666f2c3459151b28c42041a463779e6df18f61d3dd5b1879a0af4e5b199ef74d1e33e06af68bebfdfb211569ad5fb56556bfebe9d63b5688d910ea211b839a
d34957e [wallet] [tests] Add keypool topup functional test (Jonas Schnelli)
095142d [wallet] keypool mark-used and topup (John Newbery)
c25d90f [wallet] Add HasUnusedKeys() helper (John Newbery)
f2123e3 [wallet] Cache keyid -> keypool id mappings (John Newbery)
83f1ec3 [wallet] Don't hold cs_LastBlockFile while calling setBestChain (John Newbery)
2376bfc [wallet] [moveonly] Move LoadKeyPool to cpp (Matt Corallo)
cab8557 [wallet] [moveonly] Move CAffectedKeysVisitor (Jonas Schnelli)
Pull request description:
This PR contains the first part of #10882 :
- if a key from the keypool is used, mark all keys up to that key as used, and then try to top up the keypool
- top up the keypool on startup
Notably, it does not stop the node or prevent the best block from advancing if the keypool drops below a threshold (which means that transactions may be missed and funds lost if restoring from an old HD wallet backup).
Tree-SHA512: ac681fefeaf7ec2aab2fa1da93d12273ea80bd05eb48d7b3b551ea6e5d975dd97ba7de52b7fba52993823280ac4079cc36cf78a27dac708107ebf8fb6326142b
Currently difficulty adjustment violations are not reported for
chains that branch off before the last checkpoint. Change this
by moving the checkpoint check after the difficulty check.
c0025d0 Fix segfault when shutting down before fully loading (Matt Corallo)
1385697 Order chainstate init more logically. (Matt Corallo)
ff3a219 Call RewindBlockIndex even if we're about to run -reindex-chainstate (Matt Corallo)
b0f3249 More user-friendly error message if UTXO DB runs ahead of block DB (Matt Corallo)
eda888e Fix some LoadChainTip-related init-order bugs. (Matt Corallo)
Pull request description:
This does a number of things to clean up chainstate init order,
fixing some issues as it goes:
* Order chainstate init more logically - first all of the
blocktree-related loading, then coinsdb, then
pcoinsTip/chainActive. Only create objects as needed.
* More clearly document exactly what is and isn't called in
-reindex and -reindex-chainstate both with comments noting
calls as no-ops and by adding if guards.
* Move the writing of fTxIndex to LoadBlockIndex - this fixes a
bug introduced in d6af06d68a where
InitBlockIndex was writing to fTxIndex which had not yet been
checked (because LoadChainTip hadn't yet initialized the
chainActive, which would otherwise have resulted in
InitBlockIndex being a NOP), allowing you to modify -txindex
without reindex, potentially corrupting your chainstate!
* Rename InitBlockIndex to LoadGenesisBlock, which is now a more
natural name for it. Also check mapBlockIndex instead of
chainActive, fixing a bug where we'd write the genesis block out
on every start.
* Move LoadGenesisBlock further down in init. This is a more logical
location for it, as it is after all of the blockindex-related
loading and checking, but before any of the UTXO-related loading
and checking.
* Give LoadChainTip a return value - allowing it to indicate that
the UTXO DB ran ahead of the block DB. This just provides a nicer
error message instead of the previous mysterious
assert(!setBlockIndexCandidates.empty()) error.
* Calls ActivateBestChain in case we just loaded the genesis
block in LoadChainTip, avoiding relying on the ActivateBestChain
in ThreadImport before continuing init process.
* Move all of the VerifyDB()-related stuff into a -reindex +
-reindex-chainstate if guard. It couldn't do anything useful
as chainActive.Tip() would be null at this point anyway.
Tree-SHA512: 3c96ee7ed44f4130bee3479a40c5cd99a619fda5e309c26d60b54feab9f6ec60fabab8cf47a049c9cf15e88999b2edb7f16cbe6819e97273560b201a89d90762
RewindBlockIndex works over both chainActive - disconnecting blocks
from the tip that need witness verification - and mapBlockIndex -
requiring redownload of blocks missing witness data.
It should never have been the case that the second half is skipped
if we're about to run -reindex-chainstate.
This gives LoadChainTip a return value - allowing it to indicate that
the UTXO DB ran ahead of the block DB. This just provides a nicer
error message instead of the previous mysterious
assert(!setBlockIndexCandidates.empty()) error.
This also calls ActivateBestChain in case we just loaded the genesis
block in LoadChainTip, avoiding relying on the ActivateBestChain
in ThreadImport before continuing init process.
* Move the writing of fTxIndex to LoadBlockIndex - this fixes a
bug introduced in d6af06d68a where
InitBlockIndex was writing to fTxIndex which had not yet been
checked (because LoadChainTip hadn't yet initialized the
chainActive, which would otherwise have resulted in
InitBlockIndex being a NOP), allowing you to modify -txindex
without reindex, potentially corrupting your chainstate!
* Rename InitBlockIndex to LoadGenesisBlock, which is now a more
natural name for it. Also check mapBlockIndex instead of
chainActive, fixing a bug where we'd write the genesis block out
on every start.
095b917 Avoid using sizes on non-fixed-width types to derive protocol constants. (Gregory Maxwell)
Pull request description:
Thanks to awemany for pointing this out.
This replaces #10172 which appears to be abandoned, but uses the constants as requested on that PR.
Tree-SHA512: 032c0d75b3aaf807a7d0c7fb8ff5515acc45ad58bd00fe81413f900fe02bad900534a970403b9bb568e132c9eddea6043e958daf625e8acc84375bd41ee2e2ef
d9d1bd3 nCheckDepth chain height fix (romanornr)
Pull request description:
````
if (nCheckDepth <= 0)
nCheckDepth = 1000000000; // suffices until the year 19000
if (nCheckDepth > chainActive.Height())
nCheckDepth = chainActive.Height();
````
These lines confuse me.
Correct me if I am wrong, but we can't check any more blocks than we have right?
If someone requests <= 0 it get set it into some huge number and then immediately limit it to the chain height in the following statement.
````
if (nCheckDepth > chainActive.Height())
nCheckDepth = chainActive.Height();
````
when using ````--checkblocks=Z```` When Z is ````0```` or any other negative number, it will check all blocks.
I think it should be changed to this maybe.
````
if (nCheckDepth <= 0 || nCheckDepth > chainActive.Height())
nCheckDepth = chainActive.Height();
````
Which gets rid of that huge number which is confusing for any other altcoins that have a different block time.
Tree-SHA512: 8ee0ae5f33b399fa74dc16926709694ccfe1fc8a043cba2f5d00884220ac1b9b13f2df4588041f4133be634e5c7b14f4eebe24294028dafe91581a97dbe627f3
Some people keep thinking that MAX_BLOCK_BASE_SIZE is a separate
size limit from the weight limit when it fact it is superfluous,
and used in early tests before the witness data has been
validated or just to compute worst case sizes. The size checks
that use it would not behave any differently consensus wise
if they were eliminated completely.
Its correct value is not independently settable but is a function
of the weight limit and weight formula.
This patch just eliminates it and uses the scale factor as
required to compute the worse case constants.
It also moves the weight factor out of primitives into consensus,
which is a more logical place for it.
18bacec6c Make check to distinguish between orphan txs and old txs more efficient. (Alex Morcos)
Tree-SHA512: b6b4bad89aa561975dce7b68b2fdad5623af5ebcb9c38fd6a72b5f6d0544ed441df4865591ac018f7ae0df9b5c60820cb4d9e55664f5667c9268458df70fd554
69a4339ef missing white space in function arg (Lawrence Nahum)
Pull request description:
Tree-SHA512: 94d832eca0b5429cf48c7c1d4489942b53182c5b7ad9f24264867e7631301173c23a48f7b2e8fd22ff1b3137bafacb3f9d4320f4df235668c9096aaefff7afe8
176c021 [qa] Test non-atomic chainstate writes (Suhas Daftuar)
d6af06d Dont create pcoinsTip until after ReplayBlocks. (Matt Corallo)
eaca1b7 Random db flush crash simulator (Pieter Wuille)
0580ee0 Adapt memory usage estimation for flushing (Pieter Wuille)
013a56a Non-atomic flushing using the blockchain as replay journal (Pieter Wuille)
b3a279c [MOVEONLY] Move LastCommonAncestor to chain (Pieter Wuille)
Tree-SHA512: 47ccc62303f9075c44d2a914be75bd6969ff881a857a2ff1227f05ec7def6f4c71c46680c5a28cb150c814999526797dc05cf2701fde1369c06169f46eccddee
Check that cached script execution results are only valid for the same
script flags; that script execution checks are returned for non-cached
transactions; and that cached results are only valid for transactions
with the same witness hash.
Checking for the existence in the CCoinsViewCache of the outputs of a new tx
will result in a disk hit for every output since they will not be found. On the
other hand if those outputs exist already, then the inputs must also have been
missing, so we can move this check inside the input existence check so in the
common case of a new tx it doesn't need to run.
The purpose of the check is to avoid spamming the orphanMap with slightly old
txs which we have already seen in a block, but it is already only optimistic
(depending on the outputs not being spent), so make it even more efficient by
only checking the cache and not the entire pcoinsTip.
This requires that we not access pcoinsTip in InitBlockIndex's
FlushStateToDisk (so we just skip it until later in AppInitMain)
and the LoadChainTip in LoadBlockIndex (which there is already one
later in AppinitMain, after ReplayBlocks, so skipping it there is
fine).
Includes some simplifications by Suhas Daftuar and Pieter Wuille.
5a9b508 [trivial] Add end of namespace comments (practicalswift)
Tree-SHA512: 92b0fcae4d1d3f4da9e97569ae84ef2d6e09625a5815cd0e5f0eb6dd2ecba9852fa85c184c5ae9de5117050330ce995e9867b451fa8cd5512169025990541a2b
This wraps CheckInputs in ATMP's cache-inputs call to check that
each scriptPubKey the CCoinsViewCache provides is the one which
was committed to by the input's transaction hash.
A few "a->an" and "an->a".
"Shows, if the supplied default SOCKS5 proxy" -> "Shows if the supplied default SOCKS5 proxy". Change made on 3 occurrences.
"without fully understanding the ramification of a command" -> "without fully understanding the ramifications of a command".
Removed duplicate words such as "the the".
9417d7a33 Be much more agressive in AccessCoin docs. (Matt Corallo)
f58349ca8 Restore some assert semantics in sigop cost calculations (Matt Corallo)
3533fb4d3 Return a bool in SpendCoin to restore pre-per-utxo assert semantics (Matt Corallo)
ec1271f2b Remove useless mapNextTx lookup in CTxMemPool::TrimToSize. (Matt Corallo)
Tree-SHA512: 158a4bce063eac93e1d50709500a10a7cb1fb3271f10ed445d701852fce713e2bf0da3456088e530ab005f194ef4a2adf0c7cb23226b160cecb37a79561f29ca
d9bec888f Use REJECT_DUPLICATE for already known and conflicted txn (Pieter Wuille)
Tree-SHA512: adc6dc5caed731c3fd5c8784e8820a074e320360cdb5579c5b9299f9799dd99de60b7382d336ab1909dab8b23e744456d78aa0c3b1c8dd1af3d1b779314cf8fa
3ff1fa8 Use override keyword on CCoinsView overrides (Russell Yanofsky)
24e44c3 Don't return stale data from CCoinsViewCache::Cursor() (Russell Yanofsky)
Tree-SHA512: 08699dae0925ffb9c018f02612ac6b7eaf73ec331e2f4f934f1fe25a2ce120735fa38596926e924897c203f7470e99f0a99cf70d2ce31ff428b105e16583a861
CCoinsViewCache doesn't actually support cursor iteration returning the
current contents of the cache, so raise an error when the cursor method is
called instead of returning a cursor that iterates over stale data.
Also update the gettxoutsetinfo RPC which was relying on the old behavior to be
explicit about which view it is returning data about.
This adds a new CuckooCache in validation, caching whether all of a
transaction's scripts were valid with a given set of script flags.
Unlike previous attempts at caching an entire transaction's
validity, which have nearly universally introduced consensus
failures, this only caches the validity of a transaction's
scriptSigs. As these are pure functions of the transaction and
data it commits to, this should be much safer.
This is somewhat duplicative with the sigcache, as entries in the
new cache will also have several entries in the sigcache. However,
the sigcache is kept both as ATMP relies on it and because it
prevents malleability-based DoS attacks on the new higher-level
cache. Instead, the -sigcachesize option is re-used - cutting the
sigcache size in half and using the newly freed memory for the
script execution cache.
Transactions which match the script execution cache never even have
entries in the script check thread's workqueue created.
Note that the cache is indexed only on the script execution flags
and the transaction's witness hash. While this is sufficient to
make the CScriptCheck() calls pure functions, this introduces
dependancies on the mempool calculating things such as the
PrecomputedTransactionData object, filling the CCoinsViewCache, etc
in the exact same way as ConnectBlock. I belive this is a reasonable
assumption, but should be noted carefully.
In a rather naive benchmark (reindex-chainstate up to block 284k
with cuckoocache always returning true for contains(),
-assumevalid=0 and a very large dbcache), this connected blocks
~1.7x faster.
589827975 scripted-diff: various renames for per-utxo consistency (Pieter Wuille)
a5e02bc7f Increase travis unit test timeout (Pieter Wuille)
73de2c1ff Rename CCoinsCacheEntry::coins to coin (Pieter Wuille)
119e552f7 Merge CCoinsViewCache's GetOutputFor and AccessCoin (Pieter Wuille)
580b02309 [MOVEONLY] Move old CCoins class to txdb.cpp (Pieter Wuille)
8b25d2c0c Upgrade from per-tx database to per-txout (Pieter Wuille)
b2af357f3 Reduce reserved memory space for flushing (Pieter Wuille)
41aa5b79a Pack Coin more tightly (Pieter Wuille)
97072d668 Remove unused CCoins methods (Pieter Wuille)
ce23efaa5 Extend coins_tests (Pieter Wuille)
508307968 Switch CCoinsView and chainstate db from per-txid to per-txout (Pieter Wuille)
4ec0d9e79 Refactor GetUTXOStats in preparation for per-COutPoint iteration (Pieter Wuille)
13870b56f Replace CCoins-based CTxMemPool::pruneSpent with isSpent (Pieter Wuille)
05293f3cb Remove ModifyCoins/ModifyNewCoins (Pieter Wuille)
961e48397 Switch tests from ModifyCoins to AddCoin/SpendCoin (Pieter Wuille)
8b3868c1b Switch CScriptCheck to use Coin instead of CCoins (Pieter Wuille)
c87b957a3 Only pass things committed to by tx's witness hash to CScriptCheck (Matt Corallo)
f68cdfe92 Switch from per-tx to per-txout CCoinsViewCache methods in some places (Pieter Wuille)
000391132 Introduce new per-txout CCoinsViewCache functions (Pieter Wuille)
bd83111a0 Optimization: Coin&& to ApplyTxInUndo (Pieter Wuille)
cb2c7fdac Replace CTxInUndo with Coin (Pieter Wuille)
422634e2f Introduce Coin, a single unspent output (Pieter Wuille)
7d991b55d Store/allow tx metadata in all undo records (Pieter Wuille)
c3aa0c119 Report on-disk size in gettxoutsetinfo (Pieter Wuille)
d34242430 Remove/ignore tx version in utxo and undo (Pieter Wuille)
7e0032290 Add specialization of SipHash for 256 + 32 bit data (Pieter Wuille)
e484652fc Introduce CHashVerifier to hash read data (Pieter Wuille)
f54580e7e error() in disconnect for disk corruption, not inconsistency (Pieter Wuille)
e66dbde6d Add SizeEstimate to CDBBatch (Pieter Wuille)
Tree-SHA512: ce1fb1e40c77d38915cd02189fab7a8b125c7f44d425c85579d872c3bede3a437760997907c99d7b3017ced1c2de54b2ac7223d99d83a6658fe5ef61edef1de3
As the maximum amount of data that can be pulled into the cache due to
a block validation is much lower now (at most one CCoin entry per input
and per output), reduce the conservative estimate used to determine
flushing time.
This patch makes several related changes:
* Changes the CCoinsView virtual methods (GetCoins, HaveCoins, ...)
to be COutPoint/Coin-based rather than txid/CCoins-based.
* Changes the chainstate db to a new incompatible format that is also
COutPoint/Coin based.
* Implements reconstruction code for hash_serialized_2.
* Adapts the coins_tests unit tests (thanks to Russell Yanofsky).
A side effect of the new CCoinsView model is that we can no longer
use the (unreliable) test for transaction outputs in the UTXO set
to determine whether we already have a particular transaction.
This clarifies a bit more the ways in which the new script execution
cache could break consensus in the future if additional data from
the CCoins object were to be used as a part of script execution.
After this change, any such consensus breaks should be very visible
to reviewers, hopefully ensuring no such changes can be made.
The earlier CTxInUndo class now holds the same information as the Coin
class. Instead of duplicating functionality, replace CTxInUndo with a
serialization adapter for Coin.
Previously, transaction metadata (height, coinbase or not, and before
the previous commit also nVersion) was only stored for undo records
that correspond to the last output of a transaction being spent.
This only saves 2 bytes per undo record. Change this to storing this
information for every undo record, and stop complaining for having it
in non-last output spends. This means that undo dat written with
this patch won't be readable by older versions anymore.
This makes the following changes:
* In undo data and the chainstate database, the transaction nVersion
field is removed from the data structures, always written as 0, and
ignored when reading.
* The definition of hash_serialized in gettxoutsetinfo is changed to no
longer incude the nVersion field. It is renamed to hash_serialized_2
to avoid confusion. The new definition also includes transaction
height and coinbase information, as this information was missing
before.
This depends on having a CHashVerifier-based undo data checksum
verifier.
Apart from changing the definition of serialized_hash, downgrading
after using this patch is supported, as no release ever used the value
of nVersion field in UTXO entries.
This is necessary later, when we drop the nVersion field from the undo
data. At that point deserializing and reserializing the data won't
roundtrip anymore, and thus that approach can't be used to verify
checksums anymore.
With this CHashVerifier approach, we can deserialize while hashing the
exact serialized form that was used. This is both more efficient and
more correct in that case.
The error() function unconditionally reports an error. It should only
be used for actually exception situations, and not for the type of
inconsistencies that ApplyTxInUndo/DisconnectBlock can graciously deal
with.
This also makes a subtle semantics change: in ApplyTxInUndo, when a
record with metadata is encountered (indicating it is the last spend
from a tx), don't wipe the CCoins record if it wasn't empty at that
point. This makes sure that UTXO operations never affect any other
UTXOs (including those from the same tx).
211adc0 Use range-based for loops (C++11) when looping over vector elements (practicalswift)
Tree-SHA512: 0e007f20dcef99d3c7a1036265e00f689d69f42e02fd82dd8389f45b52d31947e5f9388de2610d3d9bd9f554915ce0d35ebce561e5ae3a9013956d0ee4937145
Rather than re-add disconnected block transactions back to the mempool
immediately, store them in a separate disconnectpool for later processing,
because we expect most such transactions to reappear in the chain that is
still to be connected (and thus we can avoid the work of reprocessing those
transactions through the mempool altogether).
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.
68af651 MOVEONLY: move TxConfirmStats to cpp (Alex Morcos)
2332f19 Initialize TxConfirmStats in constructor (Alex Morcos)
5ba81e5 Read and Write fee estimate file directly from CBlockPolicyEstimator (Alex Morcos)
14e10aa Call estimate(Smart)Fee directly from CBlockPolicyEstimator (Alex Morcos)
dbb9e36 Give CBlockPolicyEstimator it's own lock (Alex Morcos)
f6187d6 Make processBlockTx private. (Alex Morcos)
ae7327b Make feeEstimator its own global instance of CBlockPolicyEstimator (Alex Morcos)
Tree-SHA512: dbf3bd2b30822e609a35f3da519b62d23f8a50e564750695ddebd08553b4c01874ae3e07d792c6cc78cc377d2db33b951ffedc46ac7edaf5793f9ebb931713af
This removes another callback from block connection logic, making it
easier to reason about the wallet-RPCs-returns-stale-info issue.
UpdatedTransaction was previously used by the GUI to display
coinbase transactions only after they have a block built on top of
them. This worked fine for in most cases, but only worked due to a
corner case if the user received a coinbase payout in a block
immediately prior to restart. In that case, the normal process of
caching the most recent coinbase transaction's hash would not work,
and instead it would only work because of the on-load -checkblocks
calling DisconnectBlock and ConnectBlock on the current tip.
In order to make this more robust, a full mapWallet loop after the
first block which is connected after restart was added.
