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.
One could argue that the behavior before this commit was incorrect, as
the ChainClient interface expects a call to NotifyBlocks before
notifying blocks at tip, so we decide to fix this.
Since we now wait for the chain backend to be considered "current"
before proceeding to sync the wallet with it, any blocks that were
processed while waiting would result in being notified and scanned
twice, once by processing it at tip, and another while rescanning the
wallet, which is not desirable.
We use the recently introduced locateBirthdayBlock function within
birthdaySanityCheck as it serves as a more optimized alternative that
achieves the same purpose.
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.
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 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.
Since defer will copy the function with the parameters evaluated at its
invocation, the RescanFinished notification would be dispatched with the
incorrect block. To fix this, we'll just send the notification at the
end ourselves manually.
In this commit, we address a bug within the wallet when running with
Neutrino where it wouldn't be able to mark it as synced with the chain
due to not receiving a RescanFinished notification if it was a block
behind the chain. This happened because of the order in which the
notifications are delivered within the underlying Neutrino ChainService,
FilteredBlockConnected gets dispatched before BlockConnected. This
doesn't always work however because there is an implicit dependency
between them in which BlockConnected must occur first.
To address it, rather than switching the order in which the
notifications are dispatched, we just check if we can dispatch the final
RescanFinished notification after dispatching BlockConnected.
Co-Authored-By: Roei Erez <roeierez@gmail.com>
In this commit, we address an issue with the wallet store where it'd be
possible for us to keep lingering unconfirmed transaction entries for an
output that has been spent by a different, confirmed transaction. This
was caused due to us removing all spending transaction entries for a
given output when removing conflicts. Since all of the entries would be
removed, we weren't able to retrieve the hashes of the remaining
spending transactions to remove their records as well. Instead, we
propose to only remove the entry for the specified spending transaction.
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.
