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 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 add a sanity check for the wallet's birthday block
before syncing as a result of the migration that populated it for
existing wallets. This is done as the second part to the migration to
ensure we do not miss any relevant events throughout rescans.
The sanity check performs two main checks: whether the birthday block
timestamp reflects a time before the birthday timestamp and whether the
delta between these two timestamps is a reasonable amount. The birthday
block is then found as the first candidate that satisfies both of these
conditions.
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.
ImportPrivateKey
In this commit, we ensure that when an external private key is imported
into the wallet, that we do not overwrite our existing birthday with the
one provided. If this were to happen and we forced a wallet rescan using
the birthday as our starting point, then we'd miss detecting relevant
on-chain events that occurred between them.
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 remove the old upgrade/migration logic of the
transaction manager as it's been superseded by the new approach using
the migration.Manager interface.
In this commit, we modify the wallet to use the new migration logic
provided by the recently introduced migration package. Additionally,
we'll also perform all of our upgrades within the same database
transaction to guarantee fault-tolerance of the wallet.
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 transaction 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.
There are no existing migrations for the transaction manager, but since
the latest version was already defined as 1, we'll start from there to
be backwards-compatible.
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.
In this commit, we add a new sub-package to the walletdb package:
migration. In this package, we define a new interface, Manager, which
will expose all of the necessary functions required to abstract the
migration logic of different sub-services within the wallet, like the
address and transaction managers. The implementations of this interface
will then be able to use the migration logic within the Upgrade
function with no additional complexity.
In this commit, we relax the initial sync detection logic a bit. We do
this as right now, if a user creates an address during the sync point,
if they restart, then we'll fall back to performing a rescan from that
height as we'll detect that we aren't performing the initial sync, so
won't pick up the birthday timestamp.
To fix this, we now declare that if we have no UTXO's, then we're still
performing the initial sync. This solves this issue as when the user
restarts, we'll continue to wait for the backend to sync, and pick up
the proper birthday height before we attempt to scan forward for the
rescan. However, the one tradeoff is that we'll now always start the
rescan from the birthday height until the wallet has gained it's first
UTXO. I don't think this is too bad, as after all, the point of a wallet
is to manage utxos.
In this commit, we extend TestRemoveUnminedTx to also account for
checking the store's total balance (confirmed and unconfirmed). It
currently ensures that the UTXO state is correct, but as a sanity check,
we'll also ensure that balances are properly updated.
In this commit, we refactor the logic outside of PublishTransaction into
another unexported method. This will pave the road for unifying the
logic between SendOutputs and PublishTransaction.
In this commit, we simplify the logic when broadcasting transactions to
the greater network. Rather than special casing when running with a
Neutrino backend, we'll always add the transaction to the store as
relevant when attempting to broadcast it. This will properly insert it
into the store and update unconfirmed balances. In the event that the
transaction failed to broadcast, it can be removed from the store with
no side-effects, essentially acting as if the transaction was never
added to the store in the first place.
