Fixes issue #2178 : attacker could penny-flood with invalid-signature
transactions to deduce which addresses belonged to your node.
I'm committing this early for code review; I still need to write up
a test plan.
Executive summary of fix: check all transactions received from the network
for penny-flood rate-limiting before adding to the memory pool. But do NOT
ratelimit transactions added to the memory pool:
- because of blockchain reorgs
- stored in the wallet and added at startup
- sent from the GUI or one of the send* RPC commands (CWallet::CommitTransaction)
The limit-free-transactions code really should be a method on CNode, with
counters per-peer. But that is a bigger change for another day.
Client (SPV) mode never got implemented entirely, and whatever part was already
working, is likely not been tested (or even executed at all) for the past two
years. This removes it entirely.
If we want an SPV implementation, I think we should first get the block chain
data structures to be encapsulated in a class implementing a standard interface,
and then writing an alternate implementation with SPV semantics.
Since block validation happens in parallel, multiple threads may be
accessing the signature cache simultaneously. To prevent contention:
* Turn the signature cache lock into a shared mutex
* Make reading from the cache only acquire a shared lock
* Let block validations not store their results in the cache
* During block verification (when parallelism is requested), script
check actions are stored instead of being executed immediately.
* After every processed transactions, its signature actions are
pushed to a CScriptCheckQueue, which maintains a queue and some
synchronization mechanism.
* Two or more threads (if enabled) start processing elements from
this queue,
* When the block connection code is finished processing transactions,
it joins the worker pool until the queue is empty.
As cs_main is held the entire time, and all verification must be
finished before the block continues processing, this does not reach
the best possible performance. It is a less drastic change than
some more advanced mechanisms (like doing verification out-of-band
entirely, and rolling back blocks when a failure is detected).
The -par=N flag controls the number of threads (1-16). 0 means auto,
and is the default.
- some users reported it as weird, that the estimated block count could be
lower than our own nodes block number (which is indeed true and not good)
- this pull adds a new default behaviour, which displays our own block
number as estimated block number, if own >= est. block count
- the pull raises space for nodes block counts in cPeerBlockCounts to 8 to
be more accurate
- also removes a reduntant setNumBlocks() call in RPCConsole and moves
initialisation of numBlocksAtStartup in ClientModel, where it belongs
-checklevel gets a new meaning:
0: verify blocks can be read from disk (like before)
1: verify (contextless) block validity (like before)
2: verify undo files can be read and have good checksums
3: verify coin database is consistent with the last few blocks
(close to level 6 before)
4: verify all validity rules of the last few blocks
Level 3 is the new default, as it's reasonably fast. As level 3 and
4 are implemented using an in-memory rollback of the database, they
are limited to as many blocks as possible without exceeding the
limits set by -dbcache. The default of -dbcache=25 allows for some
150-200 blocks to be rolled back.
In case an error is found, the application quits with a message
instructing the user to restart with -reindex. Better instructions,
and automatic recovery (when possible) or automatic reindexing are
left as future work.
When the coin database is out of date with the block database, the
best block in it is automatically switched to. This reconnection
process can take time, so allow it to be interrupted.
This also stops block connection as soon as shutdown is requested,
leading to a faster shutdown.
This problem is like earth (mostly harmless). After/during a
-reindex, it means the statistics about the last block file
reported in debug.log are always of blk00000.dat instead of the
last file. Apart from that, it means a few more database entries
need to be read when finding a file to append to the first time.
- even if we are allowed to fail pre-allocating, it's better to check
for sufficient space before calling AllocateFileRange() and if we
are out of disk space return with error()
- the above change allows us to remove the CheckDiskSpace() check
in CBlock::AcceptBlock()
In case a reorganisation fails, the internal state could become
inconsistent (memory only). Previously, a cache per block connect
or disconnect action was used, so blocks could not be applied in
a partial way. Extend this to a cache for the entire reorganisation,
making it atomic entirely. This also simplifies the code a bit.
- fix ThreadSafeMessageBox always displays error icon
- allow to specify MSG_ERROR / MSG_WARNING or MSG_INFORMATION without a
custom caption / title
- allow to specify CClientUIInterface::ICON_ERROR / ICON_WARNING and
ICON_INFORMATION (which is default) as message box icon
- remove CClientUIInterface::OK from ThreadSafeMessageBox-calls, as
the OK button will be set as default, if none is specified
- prepend "Bitcoin - " to used captions
- rename BitcoinGUI::error() -> BitcoinGUI::message() and add function
documentation
- change all style parameters and enum flags to unsigned
- update code to use that new API
- update Client- and WalletModel to use new BitcoinGUI::message() and
rename the classes error() method into message()
- include the possibility to supply the wanted icon for messages from
Client- and WalletModel via "style" parameter
When a transaction A is in the memory pool, while a transaction B
(which shares an input with A) gets accepted into a block, A was
kept forever in the memory pool.
This commit adds a CTxMemPool::removeConflicts method, which
removes transactions that conflict with a given transaction, and
all their children.
This results in less transactions in the memory pool, and faster
construction of new blocks.
These flags select features to be enabled/disabled during script
evaluation/checking, instead of several booleans passed along.
Currently these flags are defined:
* SCRIPT_VERIFY_P2SH: enable BIP16-style subscript evaluation
* SCRIPT_VERIFY_STRICTENC: enforce strict adherence to pubkey/sig encoding standards.
- remove an unwanted ";" at the end of the ~CCoinsView() destructor
- in FindBlockPos() and FindUndoPos() only call fclose(), is file is open
- fix an error string in the CBlockUndo class
Flushes the blktree/ and coins/ databases, and reindexes the
block chain files, as if their contents was loaded via -loadblock.
Based on earlier work by Jeff Garzik.
Implements #1948
- Add macro `CLIENT_VERSION_IS_RELEASE` to clientversion.h
- When running a prerelease (the above macro is `false`):
- In UI, show an orange warning bar at the top. This will be used for other
warnings (and alerts) as well, instead of the status bar.
- For `bitcoind`, show the warning in the "errors" field in `getinfo`
response.
CreateNewBlock was reading pindexBest at the start before taking the lock
so it was possible to have the the block content not match the prevheader
and this can also trigger a newly added assert in ConnectBlock.
I noticed this during a code review after twobitcoins reported that ab91bf39
(BIP30 for all blocks) could cause a null dereference on a modified node
that mined during the IBD, or on testnet when it reached heights 91842 and
91880 due to CreateNewBlock calling ConnectBlock with pindex->phashBlock NULL.
- remove uiInterface.InitMessage() calls from ThreadImport(), as Qt
doesn't like them getting called out of it's main thread and because the
thread will continue to run after the GUI was loaded
Split off CBlockTreeDB and CCoinsViewDB into txdb-*.{cpp,h} files,
implemented by either LevelDB or BDB.
Based on code from earlier commits by Mike Hearn in his leveldb
branch.
To prevent excessive copying of CCoins in and out of the CCoinsView
implementations, introduce a GetCoins() function in CCoinsViewCache
with returns a direct reference. The block validation and connection
logic is updated to require caching CCoinsViews, and exploits the
GetCoins() function heavily.
Use CBlock's vMerkleTree to cache transaction hashes, and pass them
along as argument in more function calls. During initial block download,
this results in every transaction's hash to be only computed once.
During the initial block download (or -loadblock), delay connection
of new blocks a bit, and perform them in a single action. This reduces
the load on the database engine, as subsequent blocks often update an
earlier block's transaction already.
This switches bitcoin's transaction/block verification logic to use a
"coin database", which contains all unredeemed transaction output scripts,
amounts and heights.
The name ultraprune comes from the fact that instead of a full transaction
index, we only (need to) keep an index with unspent outputs. For now, the
blocks themselves are kept as usual, although they are only necessary for
serving, rescanning and reorganizing.
The basic datastructures are CCoins (representing the coins of a single
transaction), and CCoinsView (representing a state of the coins database).
There are several implementations for CCoinsView. A dummy, one backed by
the coins database (coins.dat), one backed by the memory pool, and one
that adds a cache on top of it. FetchInputs, ConnectInputs, ConnectBlock,
DisconnectBlock, ... now operate on a generic CCoinsView.
The block switching logic now builds a single cached CCoinsView with
changes to be committed to the database before any changes are made.
This means no uncommitted changes are ever read from the database, and
should ease the transition to another database layer which does not
support transactions (but does support atomic writes), like LevelDB.
For the getrawtransaction() RPC call, access to a txid-to-disk index
would be preferable. As this index is not necessary or even useful
for any other part of the implementation, it is not provided. Instead,
getrawtransaction() uses the coin database to find the block height,
and then scans that block to find the requested transaction. This is
slow, but should suffice for debug purposes.
Introduce a AllocateFileRange() function in util, which wipes or
at least allocates a given range of a file. It can be overriden
by more efficient OS-dependent versions if necessary.
Block and undo files are now allocated in chunks of 16 and 1 MiB,
respectively.
Change the block storage layer again, this time with multiple files
per block, but tracked by txindex.dat database entries. The file
format is exactly the same as the earlier blk00001.dat, but with
smaller files (128 MiB for now).
The database entries track how many bytes each block file already
uses, how many blocks are in it, which range of heights is present
and which range of dates.
Special serializer/deserializer for amount values. It is optimized for
values which have few non-zero digits in decimal representation. Most
amounts currently in the txout set take only 1 or 2 bytes to
represent.
These command are a leftover from send-to-IP transactions, which have been
removed a long time ago.
Also removes CNode::mapRequests and CNode::PushRequests, as these were
only used for the mentioned commands.
Matt pointed out some time ago that there existed a minor DOS
attack where a node in its initial block download could be wedged
by an overwrite attack in a fork created between checkpoints before
a time where BIP30 was enforced. Now that the BIP30 timestamp
is irreversibly past the check can be more aggressive and apply to
all blocks except the two historic violations.
Hard-code a special nId=max int alert, to be broadcast if the
alert key is ever compromised. It applies to all versions, never
expires, cancels all previous alerts, and has a fixed message:
URGENT: Alert key compromised, upgrade required
Variations are not allowed (ignored), so an attacker with
the private key cannot broadcast empty-message nId=max alerts.
This fixes two alert system vulnerabilities found by
Sergio Lerner; you could send peers unlimited numbers
of invalid alert message to try to either fill up their
debug.log with messages and/or keep their CPU busy
checking signatures.
Fixed by disconnecting/banning peers if they send 10 or more
bad (invalid/expired/cancelled) alerts.
If 950 of the last 1,000 blocks are nVersion=2, reject nVersion=1
(or zero, but no bitcoin release has created block.nVersion=0) blocks
-- 75 of last 100 on testnet3.
This rule is being put in place now so that we don't have to go
through another "express support" process to get what we really
want, which is for every single new block to include the block height
in the coinbase.
"Version 2" blocks are blocks that have nVersion=2 and
have the block height as the first item in their coinbase.
Block-height-in-the-coinbase is strictly enforced when
version=2 blocks are a supermajority in the block chain
(750 of the last 1,000 blocks on main net, 51 of 100 for
testnet). This does not affect old clients/miners at all,
which will continue producing nVersion=1 blocks, and
which will continue to be valid.
- If the height is in the first half, start at the genesis block and go up, rather than at the top
- Cache the last lookup and use it as a reference point if it's close to the next request, to make linear lookups always fast
- ensure warnings always start with "Warning:" and that the first
character after ":" is written uppercase
- ensure the first sentence in warnings ends with an "!"
- remove unneeded spaces from Warning-strings
- add missing Warning-string translation
- remove a "\n" and replace with untranslatable "<br><br>"
The new bytes are based on "11" to appeal to Gavin's 11 fetish.
This breaks existing testnet3 nodes as the blockchain files
are also versioned. To upgrade a node delete everything
except wallet.dat from your .bitcoin/testnet3 folder.
Modify CreateNewBlock so that instead of processing all transactions
in priority order, process the first 27K of transactions in
priority order and then process the rest in fee-per-kilobyte
order.
This is the first, minimal step towards better a better fee-handling
system for both miners and end-users; this patch should be easy
to backport to the old versions of Bitcoin, and accomplishes the
most important goal-- allow users to "buy their way in" to blocks
using transaction fees.
* Fix wrong thread name for wallet *relocking* thread
- Was named the unlocking thread
* Use consistent naming
Signed-off-by: Giel van Schijndel <me@mortis.eu>
NOTE: These thread names are visible in gdb when using 'info threads'.
Additionally both 'top' and 'ps' show these names *unless* told to
display the command-line instead of task name.
Signed-off-by: Giel van Schijndel <me@mortis.eu>
Adds CBlock::CURRENT_VERSION and CTransaction::CURRENT_VERSION
constants, and makes non-CURRENT_VERSION transactions nonstandard.
This will help make future upgrades smoother.
Prior to this change, each TX typically generated 3+ debug messages,
askfor tx 8644cc97480ba1537214 0
sending getdata: tx 8644cc97480ba1537214
askfor tx 8644cc97480ba1537214 1339640761000000
askfor tx 8644cc97480ba1537214 1339640881000000
CTxMemPool::accept() : accepted 8644cc9748 (poolsz 6857)
After this change, there is only one message for each valid TX received
CTxMemPool::accept() : accepted 22a73c5d8c (poolsz 42)
and two messages for each orphan tx received
ERROR: FetchInputs() : 673dc195aa mempool Tx prev not found 1e439346fc
stored orphan tx 673dc195aa (mapsz 19)
The -debugnet option, or its superset -debug, will restore the full debug
output.
- Signals now go directly from the core to WalletModel/ClientModel.
- WalletModel subscribes to signals on CWallet: Prepares for multi-wallet support, by no longer assuming an implicit global wallet.
- Gets rid of noui.cpp, the few lines that were left are merged into init.cpp
- Rename wxXXX message flags to MF_XXX, to make them UI indifferent.
- ThreadSafeMessageBox no longer returns the value `4` which was never used, converted to void.
Gets rid of `MainFrameRepaint` in favor of specific update functions that tell the UI exactly what changed.
This improves the efficiency of various handlers. Also fixes problems with mined transactions not showing up until restart.
The following notifications were added:
- `NotifyBlocksChanged`: Block chain changed
- `NotifyKeyStoreStatusChanged`: Wallet status (encrypted, locked) changed.
- `NotifyAddressBookChanged`: Address book entry changed.
- `NotifyTransactionChanged`: Wallet transaction added, removed or updated.
- `NotifyNumConnectionsChanged`: Number of connections changed.
- `NotifyAlertChanged`: New, updated or cancelled alert. As this finally makes it possible for the UI to know when a new alert arrived, it can be shown as OS notification.
These notifications could also be useful for RPC clients. However, currently, they are ignored in bitcoind (in noui.cpp).
Also brings back polling with timer for numBlocks in ClientModel. This value updates so frequently during initial download that the number of signals clogs the UI thread and causes heavy CPU usage. And after initial block download, the value changes so rarely that a delay of half a second until the UI updates is unnoticable.
If Reorganize() fails, then its caller, CBlock::SetBestChain(),
will call TxnAbort().
Redundant TxnAbort() calls are harmless. The second will return an
error return value, with no other side effects. TxnAbort() return
values are generally never checked. The impact is nil.
Loop over all inputs doing inexpensive validity checks first,
and then loop over them a second time doing expensive signature
checks. This helps prevent possible CPU exhaustion attacks
where an attacker tries to make a victim waste time checking
signatures for invalid transactions.
Remove orphan transactions from memory once
all of their parent transactions are received
and they're still not valid.
Thanks to Sergio Demian Lerner for suggesting this fix.
Old log message:
storing orphan tx df2244f6bc
New log message:
storing orphan tx df2244f6bc (mapsz 51)
Also, trim a few trailing whitespace in main.cpp.
Immediately issue a "getblocks", instead of a "getdata" (which will
trigger the relevant "inv" to be sent anyway), and only do so when
the previous set of invs led us into a known and attached part of
the block tree.
This prevents an undefined operation in main.cpp, when shifting the hash value
left by 32 bits.
Shifting a signed int left into the sign bit is undefined in C++11.
Introduce a boolean variable for each "network" (ipv4, ipv6, tor, i2p),
and track whether we are likely to able to connect to it. Addresses in
"addr" messages outside of our network get limited relaying and are not
stored in addrman.
This will make bitcoin relay valid routable IPv6 addresses, and when
USE_IPV6 is enabled, listen on IPv6 interfaces and attempt connections
to IPv6 addresses.
FetchInputs already logs failures internally. This commit makes the logging
more consistent with other FetchInputs callsites also.
Prior to this commit, two log lines were logged for one condition:
ERROR: FetchInputs() : de15fde415 mempool Tx prev not found a2c75da227
ERROR: CTxMemPool::accept() : FetchInputs failed de15fde415
After this commit, only one line is logged:
ERROR: FetchInputs() : e0507ab2c7 mempool Tx prev not found 9a620262cd
Previously, a single TX would trigger two log lines in quick succession,
addUnchecked(): size 152
CTxMemPool::accept() : accepted c4cfdd48b7
After this change, only one log line is used:
CTxMemPool::accept() : accepted 98885e65db (poolsz 26)
Pull request #948 introduced a fix for nodes stuck on a long side branch
of the main chain. The fix was non-functional however, as the additional
getdata request was created in a first step of processing, but dropped
in a second step as it was considered redundant. This commits fixes it
by sending the request directly.
Works for wallet transactions, memory-pool transaction and block chain
transactions.
Available for all:
* txid
* version
* locktime
* size
* coinbase/inputs/outputs
* confirmations
Available only for wallet transactions:
* amount
* fee
* details
* blockindex
Available for wallet transactions and block chain transactions:
* blockhash
* time
This commit removes the dependency of serialize.h on PROTOCOL_VERSION,
and makes this parameter required instead of implicit. This is much saner,
as it makes the places where changing a version number can have an
influence obvious.
Conflict:
* cs_main in ProcessMessages() (before calling ProcessMessages)
* cs_vSend in CNode::BeginMessage
versus:
* cs_vSend in ThreadMessageHandler2 (before calling SendMessages)
* cs_main in SendMessages
Even though cs_vSend is a try_lock, if it succeeds simultaneously with
the locking of cs_main in ProcessMessages(), it could cause a deadlock.
Open database once per "tx" message, rather than multiple times,
in the case of orphan transaction presence.
As a side effect, a now-unused CTransaction::AcceptToMemoryPool()
variant is removed.
Reference miner exists for testnet-in-a-box type situations, and as a
reference. We don't care enough about highly optimized internal
mining to keep workarounds like this.
Add a pong message that is sent in reply to a ping. It echoes back a nonce
field that is now added to the ping message. Send a nonce of zero in ping
messages.
Original author: Mike Hearn @ Google
Modified Mike's change to introduce a mild form of protocol documentation in
version.h.
Where possible, use boost::filesystem::path instead of std::string or
char* for filenames. This avoids a lot of manual string tinkering, in
favor of path::operator/.
GetDataDir is also reworked significantly, it now only keeps two cached
directory names (the network-specific data dir, and the root data dir),
which are decided through a parameter instead of pre-initialized global
variables.
Finally, remove the "upgrade from 0.1.5" case where a debug.log in the
current directory has to be removed.
All client version information is moved to version.cpp, which optionally
(-DHAVE_BUILD_INFO) includes build.h. build.h is automatically generated
on supporting platforms via contrib/genbuild.sh, using git describe.
The git export-subst attribute is used to put the commit id statically
in version.cpp inside generated archives, and this value is used if no
build.h is present.
The gitian descriptors are modified to use git archive instead of a
copy, to create the src/ directory in the output. This way,
src/src/version.cpp will contain the static commit id. To prevent
gitian builds from getting the "-dirty" marker in their git-describe
generated identifiers, no touching of files or running sed on the
makefile is performed anymore. This does not seem to influence
determinism.
- rename wxMessageBox, remove redundant arguments to noui/qtui calls
- also, add flag to force blocking, modal dialog box for disk space warning etc
- clarify function naming
- no more special MessageBox needed from AppInit2, as window object is created before calling AppInit2
In cases of very large reorganisations (hundreds of blocks), a situation
may appear where an 'inv' is sent as response to a 'getblocks', but the
last block mentioned in the inv is already known to the receiver node.
However, the supplying node uses a request for this last block as a
trigger to send the rest of the inv blocks. If it never comes, the block
chain download is stuck.
This commit makes the receiver node always request the last inv'ed block,
even if it is already known, to prevent this problem.