Previously, the wallet would determine the key scope to use for change
addresses by locating the one compatible with P2WPKH addresses, but this
wasn't always safe like in the case when multiple key scopes that
supported these addresses existed within the address manager, leading
the change address to be created outside of the intended key scope.
In this commit, we address a slight regression within the wallet
that was introduced in a previous commit. When attempting to send coins
on-chain, we would never ask the chain backend to notify us of the
transaction upon confirmation. This, along with the rebroadcast of
unconfirmed transactions logic, would result in the wallet becoming out
of sync with the chain.
Below is an example of how this could have happened:
1. Send funds on-chain.
2. Wallet doesn't ask to be notified of the confirmation.
3. Since the wallet is not notified of the confirmation, the
transaction remains in the unconfirmed bucket, even though it might
have already confirmed on-chain.
4. Restart and trigger the rebroadcast of unconfirmed transactions.
5. The unconfirmed transaction is removed from the unconfirmed bucket
due to it already existing on-chain, without it being moved to the
confirmed bucket. Moving to the confirmed bucket would require the
block at which it confirmed, which we don't have at this point.
In this commit, we do away with the internal relayFee all together.
Instead, we’ll pass in the fee rate when we’re crafting any
transactions. This allows the caller to manually dictate their desired
fee rate.
This changes the database access APIs and each of the "manager"
packages (waddrmgr/wstakemgr) so that transactions are opened (only)
by the wallet package and the namespace buckets that each manager
expects to operate on are passed in as parameters.
This helps improve the atomicity situation as it means that many
calls to these APIs can be grouped together into a single
database transaction.
This change does not attempt to completely fix the "half-processed"
block problem. Mined transactions are still added to the wallet
database under their own database transaction as this is how they are
notified by the consensus JSON-RPC server (as loose transactions,
without the rest of the block that contains them). It will make
updating to a fixed notification model significantly easier, as the
same "manager" APIs can still be used, but grouped into a single
atomic transaction.
When spending a segwit output, the wallet also needs the input value of
the previous output script. Therefore when selecting outputs the input
value is now returned. Additionally when validating newly signed
outputs the input value as also passed into `txscript.Engine`
Since the coinbase maturity is now allowed to be defined per chain and
the old blockchain.CoinbaseMaturity constant has been removed, this
updates the code accordingly.
Also, update glide.lock to use the required version of btcd.
This began as a change to improve the fee calculation code and evolved
into a much larger refactor which improves the readability and
modularity of all of the transaction creation code.
Transaction fee calculations have been switched from full increments
of the relay fee to a proportion based on the transaction size. This
means that for a relay fee of 1e3 satoshis/kB, a 500 byte transaction
is only required to pay a 5e2 satoshi fee and a 1500 byte transaction
only need pay a 1.5e3 fee. The previous code would end up estimating
these fees to be 1e3 and 2e3 respectively.
Because the previous code would add more fee than needed in almost
every case, the transaction size estimations were optimistic
(best/smallest case) and signing was done in a loop where the fee was
incremented by the relay fee again each time the actual size of the
signed transaction rendered the fee too low. This has switched to
using worst case transaction size estimates rather than best case, and
signing is only performed once.
Transaction input signature creation has switched from using
txscript.SignatureScript to txscript.SignTxOutput. The new API is
able to redeem outputs other than just P2PKH, so the previous
restrictions about P2SH outputs being unspendable (except through the
signrawtransaction RPC) no longer hold.
Several new public packages have been added:
wallet/txauthor - transaction authoring and signing
wallet/txfees - fee estimations and change output inclusion
wallet/txrules - simple consensus and mempool policy rule checks
Along with some internal packages:
wallet/internal/txsizes - transaction size estimation
internal/helpers - context free convenience functions
The txsizes package is internal as the estimations it provides are
specific for the algorithms used by these new packages.
Previously, when creating a change address during the process of
creating a new transaction an error case would be hit in the waddrmgr
triggered by attempting to derive a new internal address from under a
waddrmgr.ImportedAddrAccount. To remedy this error, we now use the
default account for change when spending outputs from an imported
key. This approach allows funds under the control of imported
private keys to be protected under the wallet's seed as soon as
they've been partially spent.
This is a rather monolithic commit that moves the old RPC server to
its own package (rpc/legacyrpc), introduces a new RPC server using
gRPC (rpc/rpcserver), and provides the ability to defer wallet loading
until request at a later time by an RPC (--noinitialload).
The legacy RPC server remains the default for now while the new gRPC
server is not enabled by default. Enabling the new server requires
setting a listen address (--experimenalrpclisten). This experimental
flag is used to effectively feature gate the server until it is ready
to use as a default. Both RPC servers can be run at the same time,
but require binding to different listen addresses.
In theory, with the legacy RPC server now living in its own package it
should become much easier to unit test the handlers. This will be
useful for any future changes to the package, as compatibility with
Core's wallet is still desired.
Type safety has also been improved in the legacy RPC server. Multiple
handler types are now used for methods that do and do not require the
RPC client as a dependency. This can statically help prevent nil
pointer dereferences, and was very useful for catching bugs during
refactoring.
To synchronize the wallet loading process between the main package
(the default) and through the gRPC WalletLoader service (with the
--noinitialload option), as well as increasing the loose coupling of
packages, a new wallet.Loader type has been added. All creating and
loading of existing wallets is done through a single Loader instance,
and callbacks can be attached to the instance to run after the wallet
has been opened. This is how the legacy RPC server is associated with
a loaded wallet, even after the wallet is loaded by a gRPC method in a
completely unrelated package.
Documentation for the new RPC server has been added to the
rpc/documentation directory. The documentation includes a
specification for the new RPC API, addresses how to make changes to
the server implementation, and provides short example clients in
several different languages.
Some of the new RPC methods are not implementated exactly as described
by the specification. These are considered bugs with the
implementation, not the spec. Known bugs are commented as such.
This a refactor of the btcwallet main package to create a new wallet
package.
The main feature of this package is the integration of all the other
wallet components (waddrmgr, txstore, and chain) and the Wallet type is
'runnable', so it will be continuously updating itself against changes
notified by the remote btcd instance.
It also includes several methods which provide access to information
necessary to run a wallet RPC server.
