Previously, addresses that belong to a watch-only account would have a
derivation path using the internal account number used to identify
accounts within the databse, rather than the actual account number based
on the account's master public key child index. This wasn't an issue
before as only one account would exist within the wallet, the 0 account,
which is also the default. To ensure users of the DerivationPath struct
can arrive at addresses correctly, we introduce a new field
InternalAccount to denote the internal account number and repurpose the
existing Account field to its actual meaning.
We would previously request spend notifications for all transaction
outputs, leading to irrelevant transactions being found in the wallet's
transaction store.
There's no need to retrieve the full block as we're only interesting in
retrieve its corresponding height, which can be done with
GetBlockHeaderVerbose.
All label parameter to PublishTransaction. Pass in an empty string
in rpc call as a placeholder for follow up PR which will add a label
parameter to the PublishTransaction request.
This PR allows the creation of managers and accounts that are watch-only. The state of the database after creation would be identical to the state after calling
Manager.ConvertToWatchingOnly, assuming accounts with the right xpubs were created in the former case.
Co-authored-by: Ken Sedgwick <ken@bonsai.com>
In similar fashion to the previous commit, due to a no longer existing
bug within the wallet, it was possible for change addresses to be
created outside of their intended key scope (the default), so wallets
affected by this now need to ensure upon recovery that they scan the
chain for _all_ existing key scopes, rather than just the default ones,
to reflect their proper balance. Through manual testing, it was shown
that the impact of recovering the additional key scopes is negligible in
most cases for both full nodes and light clients.
Due to a no longer existing bug within the wallet, it was possible for
change addresses to be created outside of their intended key scope (the
default), so wallets affected by this now need to ensure they scan the
chain for all addresses within the default key scopes (as expected), and
all _internal_ addresses (branch used for change addresses) within any
other registered key scopes to reflect their proper balance.
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.
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.
Normally the wallet doesn't wait for the chain backend to be synced
on regtest/simnet because there we cannot be certain if we are at
the chain tip, as there are no other nodes to compare to. For
Neutrino, this is a bit different because we rely on the cfheader
server to tell us what it thinks the chain tip is. For a wallet
recovery on Neutrino we therefore need to make sure we are at least
synced up to what the server thinks is the tip.
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.
A nil txid could've been returned from publishTransaction even if it was
successful. This was due to the underlying SendRawTransaction call
"failing", e.g., when the transaction being broadcast has already
confirmed, but publishTranasction interpreting such failure as a
success.
In this commit, we speed up creating a fresh wallet when using the btcd
backend. Before this commit, we would need to rescan 10k or so blocks
when creating a new wallet with the btcd backend. btcd will actually
fully scan all the blocks even though we have zero addresses or UTXOs to
look for. As a result, this can take quite some time.
In this commit we modify the starting height of the initial rescan to
start at the birthday height, and only modify it if it's unset, or the
best height of the chain is before this birthday height. As a result, we
won't always have the full 10k block re org safety horizon on disk, but
will tend to this level after we begin to sync forward.
Previously, the wallet would attempt to store the same block it
checkpointed during its initial sync when performing a recovery. This
would cause the previous block existence validation check to be in
place, which would ultimately fail because the previous block was not
stored intentionally.
To address this, we always start/resume our recovery from the wallet's
best height. This also ensures that we do not rescan the same block
again when resuming a recovery after a shutdown.
In this commit we fix a lingering bug in our output sanity checks that
would only show up during time periods of persistently higher fees.
Before this commit we would incorrectly use the fee rate instead of the
min relay fee when checking an output for dust. This would cause us to
mistakenly reject a transaction for having a dust output.
We fix this by falling back to using the current min-relayfee.
Currently, wallet rescans start from its known tip of the chain. Since
we no longer store blocks all the way from genesis to the tip of the
chain, performing a rescan would cause us to scan blocks all the way
from genesis, which we want to avoid. To prevent this, we set the
wallet's tip to be the current reorg safe height. This ensures that
we're unable to scan any blocks before it, and that we maintain
MaxReorgDepth blocks stored.
This commit serves as another building point to allow the wallet to not
store blocks all the way from genesis to the tip of chain. We modify the
wallet's recovery logic to now start from either its birthday block, or
the current reorg safe height if it's before the birthday, to ensure the
wallet properly only stores MaxReorgDepth blocks.
We also refactor things a bit in hopes of making the logic a bit more
readable.
We do this as the wallet will no longer store blocks all the way from
genesis to the tip of the chain. Instead, in order to find a reasonable
birthday block, we resort to performing a binary search for a block
timestamp that's within +/-2 hours of the birthday timestamp.
This serves as groundwork for only storing up to MaxReorgDepth blocks
upon initial sync. To do so, we want to make sure the chain backend
considers itself current so that we can only fetch the latest
MaxReorgDepth blocks from it.
This ensures the wallet can properly do an initial sync, a recovery, or
detect if it's on a stale branch before attempting to process new blocks
at tip.
Since the rescan will be triggered synchronously as well, we'll need to
catch the wallet's quit chan when handling rescan batches in order to
allow for clean shutdowns.
This unifies the logic of receiving an error when broadcasting a
confirmed transaction through btcd's/bitcoind's RPC interface. The btcd
dependency update is required in order for it to match bitcoind's
behavior. For older nodes that have yet to update, the confirmed
transaction will still be caught by the "transaction already exists"
case. This is not needed for bitcoind however, because its been sending
the same RPC error code for several major releases now.
In this commit, we address an issue with chains that are not current,
like in the often case of regtest and simnet chains. Syncing the wallet
would fail due to the chain not being current and not finding a suitable
birthday block. We fix this by just using the last synced block as the
birthday block to ensure we can properly sync to the chain.
In this commit, we fix a regression in the wallet when attempting to
sync new developer test chains such as regtest and simnet. The wallet
would block syncing until a block was mined, but in order to mine a
block, an address must be generated by the wallet first. This address
generation would block as the syncing logic was already holding the
database's mutex.
In this commit, we fix an issue with the wallet's initial sync logic
where we'd miss processing all of the blocks in the chain. This can
happen if the backend is considered current while we're still catching
up. To address this, we make sure we update our best height to process
those missed blocks.
Co-authored-by: Roei Erez <roeierez@gmail.com>
By doing this, we defer all error string-matching to happen within
publishTransaction, which allows us to simplify some of the existing
logic and maintain consistency.
In this commit, we rework how publishTransaction works in order to
correctly handle removing invalid transactions from the wallet's
unconfirmed transaction store. This is crucial as otherwise, invalid
transactions can remain within the wallet and be used for further
transactions, causing a chain of inaccurate transactions.
publishTransaction will now only return an error if the transaction
fails to be broadcast and it has not been previously seen in the
mempool/chain. This is intended in order to provide an easier API to
callers. Any other errors when broadcasting the transaction will cause
it to be removed from the wallet's unconfirmed transaction store to
ensure it maintains an accurate view of the chain.
We do this in order to be able to reuse the new publishTransaction
method within other parts of the wallet in order to consolidate all
error string-matching within one place.
This is done to avoid the birthday rescan to fail if the chain backend
reports a bestheight of 0.
Earlier it could happen that we attempted to sync to the birthday, but
since only the genesis block was available, which would be rejected as
birthday block because of the timestamp, it would fail to find a block
and the sync would fail.
In this commit, we consolidate the existing rollback logic to carry out
its duties under one database transaction.
Co-authored-by: Roei Erez <roeierez@gmail.com>
In this commit, we refactor the wallet's syncing logic with
syncWithChain to use the newer, simpler methods: syncToBirthday and
recovery. Along the way, we also fix a bug within the wallet where it
was possible to sync past the birthday, but not sync to tip completely
and restart, which would lead to us starting a rescan from the latest
synced height, rather than from the birthday stamp.
This commit slightly changes the wallet's syncing behavior to the
following:
1. Ensure the wallet is synced to its birthday.
2. Perform a recovery if requested.
3. Check for chain reorgs.
4. Dispatch a rescan from the current synced height.
Co-authored-by: Roei Erez <roeierez@gmail.com>
