When debugging another issue, I found a hang-during-startup race condition due to
LoadWallet calling SetMinVersion (via LoadCryptedKey).
Writing to the file that you're in the process of reading is a bad idea.
This fixes test_bitcoin failures on openbsd reported by dhill on IRC.
On some systems rand() is a simple LCG over 2^31 and so it produces
an even-odd sequence. ApproximateBestSubset was only using the least
significant bit and so every run of the iterative solver would be the
same for some inputs, resulting in some pretty dumb decisions.
Using something other than the least significant bit would paper over
the issue but who knows what other way a system's rand() might get us
here. Instead we use an internal RNG with a period of something like
2^60 which is well behaved. This also makes it possible to make the
selection deterministic for the tests, if we wanted to implement that.
Extremely large transactions with lots of inputs can cost the network
almost as much to process as they cost the sender in fees.
We would never create transactions larger than 100K big; this change
makes transactions larger than 100K non-standard, so they are not
relayed/mined by default. This is most important for miners that might
create blocks larger than 250K big, who could be vulnerable to a
make-your-blocks-so-expensive-to-verify-they-get-orphaned attack.
Two changes:
Use IsConfirmed() instead of IsFinal(), so 'getbalance "*" 0' uses the same
'is this output spendable' criteria as 'getbalance'. Fixes issue #172.
And a tiny refactor to CWallet::GetBalance() (redundant call to IsFinal -- IsConfirmed
calls IsFinal).
getbalance with no arguments and 'getbalance "*" 0' could return different different results,
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.
If the user was really after the fastest possible confirmation times
they would be manually setting a fee. In cases where the wallet builds
a transaction with a priority that is too low to qualify as free until
the next block, go ahead without a fee. Confirmation frequently takes
multiple blocks even when a minimum fee is provided.
This avoids a potential crash when trying to read the scrippubkeys on
transactions where the first input IsMine but some of the rest are not
when running listaddressgroupings.
The original test (checking whether the transaction occurs in the
txindex) is not usable anymore, as it will miss anything already
fully spent. However, as merkle transactions (and by extension,
wallet transactions) track which block they were last seen being
included in, we can use that to determine the need for
rebroadcasting.
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.
Corrupt wallets used to cause a DB_RUNRECOVERY uncaught exception and a
crash. This commit does three things:
1) Runs a BDB verify early in the startup process, and if there is a
low-level problem with the database:
+ Moves the bad wallet.dat to wallet.timestamp.bak
+ Runs a 'salvage' operation to get key/value pairs, and
writes them to a new wallet.dat
+ Continues with startup.
2) Much more tolerant of serialization errors. All errors in deserialization
are reported by tolerated EXCEPT for errors related to reading keypairs
or master key records-- those are reported and then shut down, so the user
can get help (or recover from a backup).
3) Adds a new -salvagewallet option, which:
+ Moves the wallet.dat to wallet.timestamp.bak
+ extracts ONLY keypairs and master keys into a new wallet.dat
+ soft-sets -rescan, to recreate transaction history
This was tested by randomly corrupting testnet wallets using a little
python script I wrote (https://gist.github.com/3812689)
OrderedTxItems returns a multimap of pointers, but needs a place to store the actual CAccountingEntries it points to.
It had been using a stack item, which was clobbered as soon as it returned, resulting in undefined behaviour.
This fixes at least bug #1768.
- Show address receiving the generation, and include it in the correct "account"
- Multiple entries in listtransactions output if the coinbase has multiple outputs to us
Logic:
- If sending a transaction, assign its timestamp to the current time.
- If receiving a transaction outside a block, assign its timestamp to the current time.
- If receiving a block with a future timestamp, assign all its (not already known) transactions' timestamps to the current time.
- If receiving a block with a past timestamp, before the most recent known transaction (that we care about), assign all its (not already known) transactions' timestamps to the same timestamp as that most-recent-known transaction.
- If receiving a block with a past timestamp, but after the most recent known transaction, assign all its (not already known) transactions' timestamps to the block time.
For backward compatibility, new accounting data is stored after a \0 in the comment string.
This way, old versions and third-party software should load and store them, but all actual use (listtransactions, for example) ignores it.
- 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>"
Implement listunspent / getrawtransaction / createrawtransaction /
signrawtransaction, to support creation and
signing-on-multiple-device multisignature transactions.
AvailableCoins() makes a vector of available outputs which is then passed to SelectCoinsMinConf(). This allows unit tests to test the coin selection algorithm without having the whole blockchain available.
This adds a field labelled 'Immature' in the overview section under the 'unconfirmed' field, which shows mined
income that has not yet matured (which is currently not displayed anywhere, even though the transactions
exist in the transaction list). To do that I added a 'GetImmatureBalance' method to the wallet, and connected
that through to the GUI as per the 'GetBalance' and 'GetUnconfirmedBalance' methods. I did a small 'no-op'
change to make the code in adjacent functions a little more readable (imo); it was a change I had made in my
repo earlier...but I thought it wouldn't hurt so left it in. Immature balance comes from mined income that is
at least two blocks deep in the chain (same logic as displayed transactions).
My reasoning is:
- as a miner, it's a critical stat I want to see
- as a miner, and taking into account the label 'immature', the uncertainty is pretty clearly implied
- those numbers are already displayed in the transaction list
- this makes the overview numbers add up to what's in the transaction list
- it's not displayed if the immature balance is 0, so won't bother non-miners
I also 'cleaned' the overview UI a little, moving code to the XML and removing HTML.
This introduces internal types:
* CKeyID: reference (hash160) of a key
* CScriptID: reference (hash160) of a script
* CTxDestination: a boost::variant of the former two
CBitcoinAddress is retrofitted to be a Base58 encoding of a
CTxDestination. This allows all internal code to only use the
internal types, and only have RPC and GUI depend on the base58 code.
Furthermore, the header dependencies are a lot saner now. base58.h is
at the top (right below rpc and gui) instead of at the bottom. For the
rest: wallet -> script -> keystore -> key. Only keystore still requires
a forward declaration of CScript. Solving that would require splitting
script into two layers.
- 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.
C++ STL ::size() generally returns unsigned, which implies that "int idx"
style of loop variable will generate a signed-vs-unsigned comparison warning
when testing the loop exit condition "idx < blah.size()"
Update areas of the bitcoin code where loop variables may be more properly and
correctly defined as unsigned.
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.
- 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
- Overall, this is better design
- This fixes problems with the address book UI not updating when the address book is changed through RPC
- Move Statusbar change detection responsibility to ClientModel
Do not automatically change the wallet format unless the user takes an
explicit action that implies an upgrade (encrypting, for now), or uses
-walletupgrade.
-walletupgrade optionally takes an integer argument: the client version
up to which upgrading is allowed. Without an argument, it is upgraded
to latest supported version. If an argument to -walletupgrade is
provided at the time the wallet is created, the new wallet will initially
not use features beyond that version.
Third, the current wallet version number is reported in getinfo.
so it takes a flag for how to interpret OP_EVAL.
Also increased IsStandard size of scriptSigs to 500 bytes, so
a 3-of-3 multisig transaction IsStandard.
OP_EVAL is a new opcode that evaluates an item on the stack as a script.
It enables a new type of bitcoin address that needs an arbitrarily
complex script to redeem.
Introduces two new RPC calls:
* dumpprivkey: retrieve the private key corresponding to an address
* importprivkey: add a private key to your wallet
The private key format is analoguous to the address format. It is
a 51-character base58-encoded string, that includes a version number
and a checksum.
Includes patch by mhanne:
* add optional account parameter for importprivkey, if omitted use default
SecureString is identical to std::string except with secure_allocator
substituting for std::allocator. This makes casting between them
impossible, so converting between the two at API boundaries requires
calling ::c_str() for now.
Also changed semantics of CWalletTx::GetTxTime(); now always returns the time the transaction was received by this node, not the average block time.
And added information about -DDEBUG_LOCKORDER to coding.txt.
Collapsed multiple wallet mutexes to a single cs_wallet, to avoid deadlocks with wallet methods that acquired locks in different order.
Also change master RPC call handler to acquire cs_main and cs_wallet locks before executing RPC calls; requiring each RPC call to acquire the right set of locks in the right order was too error-prone.
With the separation of CENT and MIN_TX_FEE, it is now reasonable
to create change outputs between 0.01 and 0.0005, as these are
spendable according to the policy, even though they require a fee
to be paid.
Also, when enough fee was already present, everything can go into
a change output, without further increasing the fee.
Instead of conversion functions between pubkey/uint160/address in
base58.h, have a fully fledged class CBitcoinAddress (CAddress was
already taken) to represent addresses.
This commit adds support for ckeys, or enCrypted private keys, to the wallet.
All keys are stored in memory in their encrypted form and thus the passphrase
is required from the user to spend coins, or to create new addresses.
Keys are encrypted with AES-256-CBC using OpenSSL's EVP library. The key is
calculated via EVP_BytesToKey using SHA512 with (by default) 25000 rounds and
a random salt.
By default, the user's wallet remains unencrypted until they call the RPC
command encryptwallet <passphrase> or, from the GUI menu, Options->
Encrypt Wallet.
When the user is attempting to call RPC functions which require the password
to unlock the wallet, an error will be returned unless they call
walletpassphrase <passphrase> <time to keep key in memory> first.
A keypoolrefill command has been added which tops up the users keypool
(requiring the passphrase via walletpassphrase first).
keypoolsize has been added to the output of getinfo to show the user the
number of keys left before they need to specify their passphrase (and call
keypoolrefill).
Note that walletpassphrase will automatically fill keypool in a separate
thread which it spawns when the passphrase is set. This could cause some
delays in other threads waiting for locks on the wallet passphrase, including
one which could cause the passphrase to be stored longer than expected,
however it will not allow the passphrase to be used longer than expected as
ThreadCleanWalletPassphrase will attempt to get a lock on the key as soon
as the specified lock time has arrived.
When the keypool runs out (and wallet is locked) GetOrReuseKeyFromPool
returns vchDefaultKey, meaning miners may start to generate many blocks to
vchDefaultKey instead of a new key each time.
A walletpassphrasechange <oldpassphrase> <newpassphrase> has been added to
allow the user to change their password via RPC.
Whenever keying material (unencrypted private keys, the user's passphrase,
the wallet's AES key) is stored unencrypted in memory, any reasonable attempt
is made to mlock/VirtualLock that memory before storing the keying material.
This is not true in several (commented) cases where mlock/VirtualLocking the
memory is not possible.
Although encryption of private keys in memory can be very useful on desktop
systems (as some small amount of protection against stupid viruses), on an
RPC server, the password is entered fairly insecurely. Thus, the only main
advantage encryption has for RPC servers is for RPC servers that do not spend
coins, except in rare cases, eg. a webserver of a merchant which only receives
payment except for cases of manual intervention.
Thanks to jgarzik for the original patch and sipa, gmaxwell and many others
for all their input.
Conflicts:
src/wallet.cpp
