Previously, the createencryptedwallet and stop requests did not check
that the client had successfully authenticated to the server. This
change moves the check outside of the select statement for these
special cased handlers (previously run from the default case) so an
auth check will occur even if a request method does not match either
of these two.
The stack trace did not make it evidant which request had caused this
issue, so add extra logging for the request that caused it. Sanitize
this request if it may contain any secrets.
Additionally, in this situation, begin shutting down the wallet rather
than simply dropping the response. This will help to catch the issue
since it's easier to notice all requests failing, rather than just one
that was dropped. If shutdown takes an unreasonably long time, panic.
Previously, registerations for wallet notifications (new txs, changed
account balances) were only passed up to websocket clients if the
wallet was loaded off disk (SetWallet was called with a non-nil
wallet), and not for the case when the RPC server would create the
wallet (if it wasn't created yet, and the user manually created it
with createencryptedwallet). This change fixes that by registering
for these notifications when this code path is taken.
The notified chain server connection state was being passed through
the wallet and then notified to the RPC server, which prevented this
notification from ever firing if a wallet didn't exist yet. Instead,
make the RPC server register for these notifications directly from the
chain server RPC client.
I'm not happy with this notification and how it's handled in the code,
but to not break existing clients this change is being made. Fixing
the notifiation mess and modifying existing clients to use a new
notification API will need to be done sometime later.
This prevents a hang when attempting to set the wallet (and register
for wallet notifications) when the process is interrupted and the rpc
server begins shutting down.
If a websocket client was already connected and the wallet and/or
chain server is loaded into the rpc server (enabling the handlers
specific to those components), the btcdconnected notifications were
not being sent, and this could break clients that expected the
notification. I'm not happy with this change, but since this is how
notifications are currently done (unsolicited), and to not break
compatibility yet, I'm adding these back in for now.
Eventually, this notification will require explicit registration
before it is received by a client. See issue #84.
Closes#115.
Addresses do no have balances. In situations where a payment is
required and just a single address was provided, it is better to track
the unspent outputs themselves, rather than watching some artificial
measure of payment.
This commit is the result of several big changes being made to the
wallet. In particular, the "handshake" (initial sync to the chain
server) was quite racy and required proper synchronization. To make
fixing this race easier, several other changes were made to the
internal wallet data structures and much of the RPC server ended up
being rewritten.
First, all account support has been removed. The previous Account
struct has been replaced with a Wallet structure, which includes a
keystore for saving keys, and a txstore for storing relevant
transactions. This decision has been made since it is the opinion of
myself and other developers that bitcoind accounts are fundamentally
broken (as accounts implemented by bitcoind support both arbitrary
address groupings as well as moving balances between accounts -- these
are fundamentally incompatible features), and since a BIP0032 keystore
is soon planned to be implemented (at which point, "accounts" can
return as HD extended keys). With the keystore handling the grouping
of related keys, there is no reason have many different Account
structs, and the AccountManager has been removed as well. All RPC
handlers that take an account option will only work with "" (the
default account) or "*" if the RPC allows specifying all accounts.
Second, much of the RPC server has been cleaned up. The global
variables for the RPC server and chain server client have been moved
to part of the rpcServer struct, and the handlers for each RPC method
that are looked up change depending on which components have been set.
Passthrough requests are also no longer handled specially, but when
the chain server is set, a handler to perform the passthrough will be
returned if the method is not otherwise a wallet RPC. The
notification system for websocket clients has also been rewritten so
wallet components can send notifications through channels, rather than
requiring direct access to the RPC server itself, or worse still,
sending directly to a websocket client's send channel. In the future,
this will enable proper registration of notifications, rather than
unsolicited broadcasts to every connected websocket client (see
issue #84).
Finally, and the main reason why much of this cleanup was necessary,
the races during intial sync with the chain server have been fixed.
Previously, when the 'Handshake' was run, a rescan would occur which
would perform modifications to Account data structures as
notifications were received. Synchronization was provided with a
single binary semaphore which serialized all access to wallet and
account data. However, the Handshake itself was not able to run with
this lock (or else notifications would block), and many data races
would occur as both notifications were being handled. If GOMAXPROCS
was ever increased beyond 1, btcwallet would always immediately crash
due to invalid addresses caused by the data races on startup. To fix
this, the single lock for all wallet access has been replaced with
mutexes for both the keystore and txstore. Handling of btcd
notifications and client requests may now occur simultaneously.
GOMAXPROCS has also been set to the number of logical CPUs at the
beginning of main, since with the data races fixed, there's no reason
to prevent the extra parallelism gained by increasing it.
Closes#78.
Closes#101.
Closes#110.
The responses chan for a websocket client was being closed by one of
the websocket goroutines, but it was not the only sender to this
channel. There was also the notification handler, run by the server
to handle notifications to all websocket clients. It was possible to
hit cases where sends to this channel would still occur (the select
statement doesn't guarantee that the picked channel operation won't
panic, even if there's another that won't). To fix this, wait on the
client being removed from the notification group, or if the server is
already shutting down, wait on the notification handler completely
closing, to ensure that no more sends to the channel will occur,
before closing the channel.
Fixes#110.
This package is used solely for the storage of private and public
keys, and the addresses they represent. Since "wallet" is an
overloaded term and a working wallet requires transaction history as
well, rename this package and its data structures to more clearly
reflect what it is for.
When a BIP0032 wallet is implemented and multiple address chains can
be supported by a single keystore, the Account structure will
represent a single wallet (and be renamed to reflect that change),
rather than keeping the collection of Account structs as currently
managed by the AccountManager. In preperation for this, and to remove
a global variable, move the fee increment for created transactions to
this structure. When setting the fee, look it up from the default
account.
Pass the RPC client to the notification handlers. Update the last
seen block for blockconnected notifications in the client structure
directly, protecting access with a mutex.
When the addmultisigaddress RPC was called, the wallet with the
imported address was not being written to disk, and if no more writes
were scheduled, the address could be lost. This change immediately
writes the updated keystore to disk before the RPC returns.
Closes#98.
This change fixes the asynchronous deferred locking that used to be
performed after some timeout after a call to walletpassphrase by
managing the locked state of each account in a new account manager
goroutine. The timeouts for new unlock requests replace any running
timeouts for older requests, rather than allowing previous timeouts to
expire before the most recent one.
Fixes#105.
This commit is the result of inspecting the results of both cpu and
memory profiling, to improve areas where wallet can be more efficient
on transaction inserts.
One problem that's very evident by profiling is how much waiting there
is for file (txstore, wallet) writes. This commit does not attempt to
fix this yet, but focuses on the easier-to-fix memory allocation
issues which can slow down the rest of wallet due to excessive garbage
collection scanning.
While here, fix a race where a closure run as a goroutine was closing
over a range iterator.
The Credit and Debits structures are simple wrappers around an
embedded *txstore.TxRecord, as well as an output index in the case of
Credit. This means that a Credit is at most two words, while a Debits
struct is just one. To avoid the unnecessary garbage of creating
Credit and Debits structures on the heap (where the underlying
TxRecord likely already is), simply pass around everywhere as
non-pointer types, and modify the receivers for all Credit and Debits
methods to non-pointer receivers since none of them ever modify the
value.
This was only necessary for a very old version of the transaction
store. The current implementation stores both sent (debit) and
received (credit) records for individual transactions.
This change rewrites much of the error handling for the RPC server
components to match a more idiomatic Go error handling style as well as
fix several issues regarding error equality checks.
Closes#94.
The gettransaction handler was attempting to lookup the "sent-to"
address of an outgoing transaction from the transaction store (as a
wallet credit). This is the incorrect address when sending to an
address controlled by another wallet, and panics when there are no
credits (for example, sending to another wallet without any change
address). Instead, use the first non-change output address is used as
the address of the "send" result.
This fixes the panic reported when debugging issue #91.
While here, fix the category strings used for wallet credits to
support immature and generate (the categories for coinbase outputs).
If an unexpected error is encounted when creating the encrypted
wallet, rather than using btcjson.ErrInternal, wrap the error message
using btcjson.ErrWallet.Code.
This change is the result of using the errcheck tool
(https://github.com/kisielk/errcheck) to find all unchecked errors,
both unassigned and those assigned to the blank identifier.
Every returned error is now handled in some manner. These include:
- Logging errors that would otherwise be missed
- Returning errors to the caller for further processing
- Checking error values to determine what to do next
- Panicking for truely exceptional "impossible" errors
On the subject of panics, they are a sharp tool and should be used
sparingly. That being said, I have added them to check errors that
were previously explicitly ignored, because they were expected to
always return without failure. This could be due to fake error paths
(i.e. writing to a bytes.Buffer panics for OOM and should never return
an error) or previous logic asserts that an error case is impossible.
Rather than leaving these unhandled and letting code fail later,
either with incorrect results or a nil pointer dereference, it now
produces a stack trace at the error emit site, which I find far more
useful when debugging.
While here, a bunch of dead code was removed, including code to move
pre-0.1.1 uxto and transaction history account files to the new
directory (as they would be unreadable anyways) and a big chunk of
commented out rpcclient code.
So (SigHashAll & SigHashSingle)!= 0, which is not the intention here. fix up
that check to only match SigHashSingle.
Found by drahn, debugged together, fix by me.
This is an intial pass at converting the btcwallet and deps codebases
to pass a network by their parameters, rather than by a magic number
to identify the network. The parameters in params.go have been
updated to embed a *btcnet.Params, and all previous uses of cfg.Net()
have been replaced with activeNet.{Params,Net} (where activeNet is
the global var for the active network).
Although dependancy packages have not yet been updated from using
btcwire.BitcoinNet to btcnet.Params, the parameters are now accessible
at all callsites, and individual packages can be updated to use btcnet
without requiring updates in each external btc* package at once.
While here, the exported API for btcwallet internal library packages
(txstore and wallet) have been updated to pass full network parameters
rather than the btcwire definition of a network.
The category for a received coinbase output should be "generate" for a
mature coinbase (one that has reached btcchain.CoinbaseMaturity
confirmations), or "immature" if the required number of confirmations
has not been reached yet. New Confirmed and Confirmations methods
have been added to the transaction store's TxRecord type to check if
the required number of confirmations have been met for coinbase
outputs.
While here, update the main package to use the new TxRecord methods,
rather than duplicating the confirmation checking code in two places.
The last transaction store was a great example of how not to write
scalable software. For a variety of reasons, it was very slow at
processing transaction inserts. Among them:
1) Every single transaction record being saved in a linked list
(container/list), and inserting into this list would be an O(n)
operation so that records could be ordered by receive date.
2) Every single transaction in the above mentioned list was iterated
over in order to find double spends which must be removed. It is
silly to do this check for mined transactions, which already have
been checked for this by btcd. Worse yet, if double spends were
found, the list would be iterated a second (or third, or fourth)
time for each removed transaction.
3) All spend tracking for signed-by-wallet transactions was found on
each transaction insert, even if the now spent previous transaction
outputs were known by the caller.
This list could keep going on, but you get the idea. It was bad.
To resolve these issues a new transaction store had to be implemented.
The new implementation:
1) Tracks mined and unmined transactions in different data structures.
Mined transactions are cheap to track because the required double
spend checks have already been performed by the chain server, and
double spend checks are only required to be performed on
newly-inserted mined transactions which may conflict with previous
unmined transactions.
2) Saves mined transactions grouped by block first, and then by their
transaction index. Lookup keys for mined transactions are simply
the block height (in the best chain, that's all we save) and index
of the transaction in the block. This makes looking up any
arbitrary transaction almost an O(1) operation (almost, because
block height and block indexes are mapped to their slice indexes
with a Go map).
3) Saves records in each transaction for whether the outputs are
wallet credits (spendable by wallet) and for whether inputs debit
from previous credits. Both structures point back to the source
or spender (credits point to the transaction that spends them, or
nil for unspent credits, and debits include keys to lookup the
transaction credits they spent. While complicated to keep track
of, this greatly simplifies the spent tracking for transactions
across rollbacks and transaction removals.
4) Implements double spend checking as an almost O(1) operation. A
Go map is used to map each previous outpoint for all unconfirmed
transactions to the unconfirmed tx record itself. Checking for
double spends on confirmed transaction inserts only involves
looking up each previous outpoint of the inserted tx in this map.
If a double spend is found, removal is simplified by only
removing the transaction and its spend chain from store maps,
rather than iterating a linked list several times over to remove
each dead transaction in the spend chain.
5) Allows the caller to specify the previous credits which are spent
by a debiting transaction. When a transaction is created by
wallet, the previous outputs are already known, and by passing
their record types to the AddDebits method, lookups for each
previously unspent credit are omitted.
6) Bookkeeps all blocks with transactions with unspent credits, and
bookkeeps the transaction indexes of all transactions with unspent
outputs for a single block. For the case where the caller adding a
debit record does not know what credits a transaction debits from,
these bookkeeping structures allow the store to only consider known
unspent transactions, rather than searching through both spent and
unspents.
7) Saves amount deltas for the entire balance as a result of each
block, due to transactions within that block. This improves the
performance of calculating the full balance by not needing to
iterate over every transaction, and then every credit, to determine
if a credit is spent or unspent. When transactions are moved from
unconfirmed to a block structure, the amount deltas are incremented
by the amount of all transaction credits (both spent and unspent)
and debited by the total amount the transaction spends from
previous wallet credits. For the common case of calculating a
balance with just one confirmation, the only involves iterating
over each block structure and adding the (possibly negative)
amount delta. Coinbase rewards are saved similarly, but with a
different amount variable so they can be seperatly included or
excluded.
Due to all of the changes in how the store internally works, the
serialization format has changed. To simplify the serialization
logic, support for reading the last store file version has been
removed. Past this change, a rescan (run automatically) will be
required to rebuild the transaction history.
This commit modifies all code paths which work with transaction result
objects to use the concrete ListTransactionsResult provided by the btcjson
package. This provides nicer marshalling and unmarshalling as well as
access to properly typed fields.
- Instead of returning a special constructed type whenever queries for an
address. Return the internal object with an immutable external
interface.
- Make the private key gettable from PubKeyAddress to prevent having to look up
multiple times to get information from the same structure
- Enforce addresses always have public keys.
This change takes advantage of the RawMessage type in the
encoding/json package to defer unmarshaling of all JSON-RPC values
until absolutely necessary.
This is particularly important for request passthrough when btcwallet
must ask btcd to handle a chain request for a wallet client. In the
previous code, during the marshal and unmarshal dance to set the
original client's request id in the btcd response, large JSON numbers
were being mangled to use (scientific) E notation even when they could
be represented as a integer without any loss of precision.
- Move the MarkAddresForAccount and LookupAccountByAddress functionality
into account maanger.
- Move the wallet opeing logic into account manager (the only place that calls
it) and unexport.
- Move accountHandler to using a single channel for commands. Many of
the commands have ordering restraints (add account, list all accounts,
remove account, access account, mark account for address) which are very
much undefined with the multi-channel model.
- Rework all callers of LookupAccountByAddress to get the account structure
directly.
This change adds a notification handler for the new rescanprogress
notification and takes advantage of the recent rescan manager and
partial syncing support to mark addresses as partially synced. If the
network connection to btcd is lost or wallet is restarted during a
rescan, a new rescan will start at the earliest block height for any
wallet address, taking partial syncs into consideration.
The private key import codepath (called when handling the
importprivkey RPC method) was not triggering rescans for the imported
address. This change begins a new rescan for each import and adds
additional logic to the wallet file to keep track of unsynced imported
addresses. After a rescan on an imported address completes, the
address is marked as in sync with the rest of wallet and future
handshake rescans will start from the last seen block, rather than the
import height of the unsynced address.
While here, improve the logging for not just import rescans, but
rescanning on btcd connect (part of the handshake) as well.
Fixes#74.
Shortly we will add new types of address, so make AddressInfo an
interface, with concrete types providing address-specific information.
Adapt existing code to this new status quo.
Notifications ariving from btcd were being reordered (each handled by
its own goroutine, rather then being always sent in the order they
originated). This was breaking the new transaction store by inserting
transaction records in an 'impossible' manner, that is, inserting txs
without block info after the store already held records of the same tx
with block info, without first performing a rollback.
This is handled by the transaction store insert methods by checking
for identical transactions (double spends with the same tx sha), but
where the block heights mismatch and the new record does not have a
block set. The error is returned all the way up to the goroutine
running each rpc request/notification handler, and if hit, the btcd
connection is closed and all accounts are reopened from disk. This is
not optimal, but it allows us to use the connect logic to correctly
catch us up to the best chain with the last good state of all accounts
while only rescanning a few blocks.
Fixes#72.
This change replaces the old transaction store file format and
implementation. The most important change is how the full backing
transactions for any received or sent transaction are now saved,
rather than simply saving parsed-out details of the tx (tx shas, block
height/hash, pkScripts, etc.).
To support the change, notifications for received transaction outputs
and txs spending watched outpoints have been updated to use the new
redeemingtx and recvtx notifications as these contain the full tx,
which is deserializead and inserted into the store.
The old transaction store serialization code is completely removed, as
updating to the new format automatically cannot be done. Old wallets
first running past this change will error reading the file and start a
full rescan to rebuild the data. Unlike previous rescan code,
transactions spending outpoint managed by wallet are also included.
This results in recovering not just received history, but history for
sent transactions as well.
The disk syncer now maintains its own countdown timer, creating a new
timer only when necessary (when there is no timer running, and
something is scheduled to be written). When the timer expires, the
select loop begins selecting on a grab of the account manager's binary
semaphore, and if read, performs the sync and nils the select channel
to prevent a future grab until a new timer has expired.
Tested with a race-enabled build on Windows. No lockups or races
related to the disk syncing experienced with constant client requests
and incoming btcd notifications, and scheduled writes run as expected
once the countdown timer expires, locking out all server request and
notifiation handling.
