Revert "Revert "Add rules--presently disabled--for using GetMedianTimePast as endpoint for lock-time calculations""
This reverts commit 40cd32e835.
After careful analysis it was determined that the change was, in fact, safe and several people were suffering
momentary confusion about locktime semantics.
This reverts commit 9d55050773.
As noted by Luke-Jr, under some conditions this will accept transactions which are invalid by the network
rules. This happens when the current block time is head of the median time past and a transaction's
locktime is in the middle.
This could be addressed by changing the rule to MAX(this_block_time, MTP+offset) but this solution and
the particular offset used deserve some consideration.
The lock-time code currently uses CBlock::nTime as the cutoff point for time based locked transactions. This has the unfortunate outcome of creating a perverse incentive for miners to lie about the time of a block in order to collect more fees by including transactions that by wall clock determination have not yet matured. By using CBlockIndex::GetMedianTimePast from the prior block instead, the self-interested miner no longer gains from generating blocks with fraudulent timestamps. Users can compensate for this change by simply adding an hour (3600 seconds) to their time-based lock times.
If enforced, this would be a soft-fork change. This commit only adds the functionality on an unexecuted code path, without changing the behaviour of Bitcoin Core.
Assume that when a wallet transaction has a valid block hash and transaction position
in it, the transaction is actually there. We're already trusting wallet data in a
much more fundamental way anyway.
To prevent backward compatibility issues, a new record is used for storing the
block locator in the wallet. Old wallets will see a wallet file synchronized up
to the genesis block, and rescan automatically.
Since the introduction of the ScriptForMining callback, the mining
functions (setgenerate and generate) crash with an assertion failure
(due to a NULL pointer script returned) if the keypool is empty. Fix
this by giving a proper error.
No longer relevant after #5957. This hack existed because of another
hack where the numthreads parameter, on regtest, doubled as how many
blocks to generate.
To determine the default for `-par`, the number of script verification
threads, use [boost:🧵:physical_concurrency()](http://www.boost.org/doc/libs/1_58_0/doc/html/thread/thread_management.html#thread.thread_management.thread.physical_concurrency)
which counts only physical cores, not virtual cores.
Virtual cores are roughly a set of cached registers to avoid context
switches while threading, they cannot actually perform work, so spawning
a verification thread for them could even reduce efficiency and will put
undue load on the system.
Should fix issue #6358, as well as some other reported system overload
issues, especially on Intel processors.
The function was only introduced in boost 1.56, so provide a utility
function `GetNumCores` to fall back for older Boost versions.
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.
Previously due to an off-by-one error the wallet ignored
nLockTime-by-height transactions that would be valid in the next block
even though they are accepted into the mempool. The transactions
wouldn't show up until confirmed, nor would they be included in the
unconfirmed balance. Similar to the mempool behavior fix in 665bdd3b,
the wallet code was calling IsFinalTx() directly without taking into
account the fact that doing so tells you if the transaction could have
been mined in the *current* block, rather than the next block.
To fix this we strip IsFinalTx() of non-consensus-critical
functionality, removing the default arguments, and add CheckFinalTx() to
check if a transaction will be final in the next block.
When the internal miner is enabled at the start of a new node, there
is an near instant assert in TestBlockValidity because its attempting
to mine a block before the top checkpoint.
Also avoids a data race around vNodes.
This adds a -regetest-only undocumented (for regression testing only)
command-line option -blockversion=N to set block.nVersion.
Adds to the "has the rest of the network upgraded to a
block.nVersion we don't understand" code so it calls
-alertnotify when 51 of the last 100 blocks are up-version.
But it only alerts once, not with every subsequent new, upversion
block.
And adds a forknotify.py regression test to make sure it works.
Tested using forknotify.py:
Before adding CAlert::Notify, get:
Assertion failed: -alertnotify did not warn of up-version blocks
Before adding code to only alert once:
Assertion failed: -alertnotify excessive warning of up-version blocks
After final code in this pull:
Tests successful
Gregory Maxwell