The commit being reverted resulted in the discovery of a bug in which
change addresses could at times be created outside of the default key
scopes, causing us to not properly determine their spends.
It was discovered that the wallet can scan the chain for unnecessary
additional addresses that are derived by higher-level applications using
custom key scopes. This isn't much of an issue for full nodes, but it
can cause light clients to scan more than what's required, triggering
more false positive matches which lead to block retrieval.
Now, we'll only scan the chain for addresses that exist within the
default key scopes, as those are the only ones the wallet should be
concerned about.
We update the dropwtxmgr utility tool to take into account that the
wallet only stores MaxReorgDepth blocks, which introduced an additional
constraint when updating the wallet's synced state. The constraint
ensures that the previous block exists when updating the wallet's synced
state, but this does not hold for the birthday block since it's the
first block we'll store.
In this commit, we add a migration that will be used by existing wallets
to ensure they can adhere to the new requirement of storing up to
MaxReorgDepth entries within the block hash index.
In this commit, we modify the wallet's block hash index to only store up
to MaxReorgDepth blocks. This allows us to reduce consumed storage, as
we'd be mostly storing duplicate data. We choose to store up to
MaxReorgDepth to ensure we can recover from a potential long reorg.
This commit makes nextAddresses add a function to the transactions
OnCommit handler used to update the cache on successful database
transaction commit. Before this we would risk the cache and database of
get out of sync if the database transaction failed or was aborted after
the cache was updated.
In this commit, we modify the dropwtxmgr tool to force a rescan upon
restart from the wallet's birthday block, rather than the chain's
genesis block. We can safely do this as we expect that no on-chain
events relevant to the wallet should happen before this block. For
older wallets which do not have their birthday block set, the rescan
should start from the genesis block.
In this commit, we add a new key/value pair to the waddrmgr's sync
bucket to store the verification status of the birthday block. This
verification status determines whether the wallet has verified the
correctness of its birthday block through its sanity check on startup.
In this commit, we add a migration to force a rescan of users' wallets
starting from their birthday block to ensure that their balance is
reflected correctly as it is on-chain. This was inspired by the recent
bug discovered where the wallet would not watch for the confirmation of
a relevant transaction.
In this commit, we add a new migration to the waddrmgr to populate the
birthday block for existing wallets. This will deem useful when
performing rescans for whatever reason, as we'll now be able to start
from this point rather than the genesis block, incurring a longer
rescan.
The migration is not as reliable since we do not store block timestamps,
so we'll need to estimate our height by looking at the genesis timestamp
and assuming a block occurs every 10 minutes. This can be unsafe, and
cause us to actually miss on-chain events, so a sanity check will be
added before the wallet attempts to sync itself in a later commit.
In this commit, we add a new key/value pair within the waddrmgr's
syncBucket that will represent the birthday block of the wallet. This
can then be used to force rescans from this point, rather than from the
genesis block.
In this commit, we convert our unit tests to have package-level access.
We do this as an effort to reduce test code duplication when we
introduce migration tests which require access to specific unexported
functions/methods.
In this commit, we remove the old upgrade/migration logic of the address
manager as it's been superseded by the new approach using the
migration.Manager interface.
In this commit, we can remove the LatestVersion constant as it's no
longer needed. Instead, we'll now define the latest version as the last
entry in the slice of versions previously defined.
In this commit, we add an implementation of the recently introduced
migration.Manager interface for the address manager. With this, we'll
now be able to only expose the things required for the migration to
happen, but have the actual migration logic live at a much higher level.
The existing versions defined are set up in the same way as the existing
upgrade/migration logic, which will end up being superseded by this and
removed in a later commit.
This commit places a mutex around calls to newSecretKey,
since the inner function needs to be swapped out
during testing. Prior to this change, the race
detector would panic since the mutation was
unprotected.
In this commit, we add the new DerivationInfo method to the current
default implementation of the ManagedPubKeyAddress interface. In doing
this, we replace the account field with the derivationPath, as we can
obtain the account field from the derivationPath itself.
In this commit, we add a new method DerivationInfo to the
ManagedPubKeyAddress interface. This method is meant to provide callers
with the information necessary to independently derive each key returned
by the various methods provided to derive addresses.
This commit resolves a deadlock observed when attempting
to generate addresses. There were a few cases, particularly
in chainAddressRowToManaged and loadAccountInfo, which accessed
the public IsLocked() method of the Manager, even though the
shared mutex had already been acquired.
The solution is to create an internal isLocked() method, which
can be safely called assuming the manager's mutex has already been
acquired. As the comments above both of the methods in question
specify, we can assume the Manager's mutex *is* already acquired.
This commit also reduces some unnecessary code duplication, since
the recent changes left both a Locked() and IsLocked() method that
perform the same functionality. IsLocked() was favored as it more
clearly indicates that the returned value is a boolean.
In this commit, we fix a deadlock bug that was introduced recently.
This can happen when ForEachActiveAccountAddress or
ForEachActiveAddress is called, as these internally need to grab the
mutex of the manager (within the scoped manager) in order to check if
the manager is locked or not.
In this commit, we add a database migration from version 4 to version 5.
We also take this opportunity to clean up the old migration code. This
is no longer needed as wallets very old can simply go back in the prior
git history to migrate to version 4, then go from there to version 5.
In this commit, we remove many of the methods in the Manager struct as
they’ve now be been replicated within a scoped format for each of the
ScopedKeyManagers.
A major change is that we’ll now actually store the master HD private
and public keys. This required as in order to create new scopes, we
need access to the master HD private key as hardened derivation is
required in accordance with BIP43.
The initial creation of the manager namespaces has also been extended
to create the namespaces and keys for the set of default key scopes.
Finally, a series of utility method has been added to allow callers to
create ScopedKeyManagers for arbitrary sets of scopes.
In this commit, we create a new struct that houses the key derivation,
address management, and account management for a particular scope. A
scope consists of a (purpose, cointype) tuple. Additionally, each
ScopedKeyManager is able to generate internal/external addresses for a
specific *address type*. This make rescans easier as for each scope, we
know what type of output to look for within the chain.
The ScopedKeyManagers have two new primary methods that weren’t
previously present within the regular Manager:
* DeriveFromKeyPath
* NewRawAccount
These two methods allow callers a greater degree of control over the
way that accounted are created and addressed derived.
In this commit, we create new key spaces to allow users to store the
encrypted master priv/pub keys. This is required as in order to create
new scopes, we must do hardened derivation from the root key.
In this commit, we make a fundamental modification bucket structure
within the database. Most buckets are no under an additional layer of
nesting: the scope. The scope encapsulates which (purpose, coin type)
pair the address, accounts, and coin type keys belong to.
In this commit, we remove all direct references to BIP 44 as upcoming
changes will shift to a model that is no longer directly dependent on
BIP 44 in favor of restoring a layer of abstraction and allowing users
to manage multiple (purpose, coin type) scopes within the same
database.
In this commit, we introduce the concept of scopes for individual key
managers. Each scope will lock down a key manager to a particular
purpose and coin type within the BIP0043 hierarchy. Each scope will
also have a set address type schema. This schema will be consulted when
creating addresses for a particular scoped key manager.
Finally, we introduce 3 new default scopes:
* BIP 44
* BIP 84
* BIP 49++ (BIP49 but uses p2wkh for change addresses)
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.
This commit introduces two new address types to the waddrmgr. The first
address type is the native p2wkh (pay-to-witness-key-hash) output type
introduced as part of BIP0141 and the segwit soft-fork. The second
address type is a p2wkh output nested *within* a regular p2sh output.
This second address allows older wallets which are not yet aware of the
new segwit output types to transparently pay to a wallet which does
support them. Additionally, using this nested p2wkh output the wallet
gains both the space+transaction fee savings, as well as the
malleability fixes.
Both address types have been implemented as special cases of the
ManagedPubKeyAddress since they share several traits, only
differentiating in the signing mechanism needed, and the concrete
implementation of btcutil.Address returned by the address.
Two new `addressType` constants have been added to waddrmgr’s db in
order to properly serialize and deserialize the new address types.
This updates all code to make use of the new chainhash package since the
old wire.ShaHash type and related functions have been removed in favor
of the abstracted package.
Also, while here, rename all variables that included sha in their name
to include hash instead.
Finally, update glide.lock to use the required version of btcd, btcutil,
and btcrpcclient.
This commit corrects various things found by the static checkers
(comments, unkeyed fields, return after some if/else).
Add generated files and legacy files to the whitelist to be ignored.
Catch .travis.yml up with btcd so goclean can be run.
This changes the wallet.Open function signature to remove the database
namespace parameters. This is done so that the wallet package itself
is responsible for the location and opening of these namespaces from
the database, rather than requiring the caller to open these ahead of
time.
A new wallet.Create function has also been added. This function
initializes a new wallet in an empty database, using the same
namespaces as wallet.Open will eventually use. This relieves the
caller from needing to manage wallet database namespaces explicitly.
Fixes#397.
This change only prevents creating new accounts with the empty name or
renaming an existing account to one. Any accounts in the DB that are
already named the empty string are left untouched (and should be
renamed to something meaningful by the user).
Fixes#369.
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.
The behaviour of function Address() in waddrmgr has been updated such that
it now displays the correct behaviour as described in the comments. That is,
when a public key address is given as a btcutil.Address, the key is converted
to a public key hash address so that serializing with ScriptAddress() yields
the corresponding public key hash. This allows the address manager to find
the corresponding private key, and fixes the signing of multisignature
transactions.
Rather than the main package being responsible for opening the address
and transaction managers, the namespaces of these components are
passed as parameters to the wallet.Open function.
Additionally, the address manager Options struct has been split into
two: ScryptOptions which holds the scrypt parameters needed during
passphrase key derivation, and OpenCallbacks which is only passed to
the Open function to allow the caller to provide additional details
during upgrades.
These changes are being done in preparation for a notification server
in the wallet package, with callbacks passed to the Open and Create
functions in waddrmgr and wtxmgr. Before this could happen, the
wallet package had to be responsible for actually opening the managers
from their namespaces.
If the account number to name index mapped the default account name to
an alias, the upgrade would not succeed and the upgrade would be
aborted (and rolled back).
This became a problem for upgrading old (pre-v3) wallets since the v3
upgrade did not rename the previous "" account to "default", but
instead just created an alias.
Fix tested by @dajohi, who ran into this issue with a wallet upgrade
from an older keystore version.
Rather than disallowing the default account to be renamed as was
proposed in #245 (and implemented in #246), the default account name
is no longer considered a reserved name by the address manager.
Instead, it is simply the initial name used for the first initial
account.
A database upgrade removes any additional aliases for the default
account in the database. This prevents a lookup for some name which
is not an account name from mapping to the default account
unexpectedly (potentially preventing incorrect account usage from the
RPC server due to bad iteraction with default parameters).
All unset account names in a JSON-RPC request are expected to be set
nil by btcjson. This behavior depends on btcsuite/btcd#399.
Additionally, the manager no longer considers the wildcard * to be a
reserved account name. Due to poor API decisions, the RPC server
overloads the meaning of account fields to optionally allow referring
to all accounts at a time, or a single account. This is not a address
manager responsibility, though, as a future cleaner API should not use
multiple differet meanings for the same field across multiple
requests. Therefore, don't burden down future APIs with this quirk
and prevent incorrect wildcard usage from the RPC server.
Closes#245.
This introduce a new internal package to deal with the explicit
clearing of data (such as private keys) in byte slices, byte arrays
(32 and 64-bytes long), and multi-precision "big" integers.
Benchmarks from a xeon e3 (Xor is the zeroing funcion which Bytes
replaces):
BenchmarkXor32 30000000 52.1 ns/op
BenchmarkXor64 20000000 91.5 ns/op
BenchmarkRange32 50000000 31.8 ns/op
BenchmarkRange64 30000000 49.5 ns/op
BenchmarkBytes32 200000000 10.1 ns/op
BenchmarkBytes64 100000000 15.4 ns/op
BenchmarkBytea32 1000000000 2.24 ns/op
BenchmarkBytea64 300000000 4.46 ns/op
Removes an XXX from the votingpool package.
This commit makes the creation and updating of the address manager more
explicit so it's easier to upgrade in the future.
In particular, rather than treating the initial creation as an upgrade by
relying on creating the initial buckets on the fly on each load, the code
now explicitly provides distinct create and upgrade paths that are invoked
from the Create and Open functions, respectively.
It also adds some commented out sample code to illustrate how upgrades
should be done and a check to ensure bumping the version number without
writing upgrade code results in a new error, ErrUpgrade, being returned.
Finally, a test has been added for the new functionality.
This commit converts the wallet to use the new secure hierarchical
deterministic wallet address manager package as well as the walletdb
package.
The following is an overview of modified functionality:
- The wallet must now be created before starting the executable
- A new flag --create has been added to create the new wallet using wizard
style question and answer prompts
- Starting the process without an existing wallet will instruct now
display a message to run it with --create
- Providing the --create flag with an existing wallet will simply show an
error and return
In addition the snacl package has been modified to return the memory after
performing scrypt operations to the OS.
Previously a runtime.GC was being invoked which forced it to release the
memory as far as the garbage collector is concerned, but the memory was
not released back to the OS immediatley. This modification allows the
memory to be released immedately since it won't be needed again until the
next wallet unlock.
