Separate out the logic we use to write labels so that it can be used
in dropwtxmgr to re-add labels if we want to keep them when we drop the
rest of the db.
Code is duplicated across these two functions, so update
rangeBlockTransactions to use minedTxDetails. When iterating through
a block, we do have the block metadata already (which is looked up by
minedTxDetails). This change can be further optimized to split
minedTxDetails into minedTxDetails and minedTxDetailsWithoutBlock to
reduce this redundancy.
Add and test functions which can be used to write optional transaction
labels to disk in their own bucket. These labels are keyed by txid and
write the labels to disk using-length value encoding scheme. Although
the length field is not required at present, it is added to allow future
extensibility without a migration.
This approach is chosen over adding this information to txRecords,
Because a migration would be required to add a field after the variable
Length serialized tx.
The put label function will overwrite existing labels if called more
than once for the same txid. User side validation of whether we want
to override this label should be performed by calling code. Labels must
be > 0 characters and <= 500 characters (an arbitrarily chosen limit).
Previously, inserting a transaction as unconfirmed into the store and
later confirming it would leave a lingering unconfirmed input record.
This was discovered as part of
https://github.com/btcsuite/btcwallet/pull/655. This issue would only
affect the wallet if it tracked spent transaction outputs, which it
doesn't. We aim to resolve it in any case for the sake of internal
consistency.
This test ensures that there aren't any lingering unconfirmed records
for a transaction that existed within the store as unconfirmed before
becoming confirmed. At the moment, this is currently failing due to a
gap when moving a transaction from unconfirmed to confirmed within the
store. This will be resolved in a subsequent commit.
This commit ensures the wallet won't enter an inconsitent state
by checking tx confirmation before adding credit.
Without this fix, In the case the existing transaction is already
confirmed on-chain the flow updates the bucketUnminedCredits but
without adding en entry also to the bucketUnmined resulting in
inconsistent state.
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 follow up our previous migration to reset our synced
block to our birthday block with another migration to drop our
transaction history. We'll need to do this to ensure that the
transaction store matches the exact state of our outputs on-chain to
prevent inadvertently crafting any invalid transactions.
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 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 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 remove the duplicate test case from TestStoreQueries
as we'll no longer allow storing a transaction as unconfirmed if it's
already confirmed.
In this commit, we resolve a lingering bug within the wallet where it's
possible that an output is added as unconfirmed credit after the fact
that it has already confirmed. This would lead to duplicate entries for
the same output within the wallet causing double spend transactions to
be crafted.
In this commit, we add a new test case to the wtxmgr store to ensure
that duplicate outputs don't exists within the store. It's possible for
this to happen if an output is marked as unconfirmed credit, then marked
as confirmed once it confirms, and once again marked as unconfirmed. It
can be marked as unconfirmed again due to the backend notifying the
client about this transaction. Ideally this should not happen, but the
root cause is much more involved. As a stop gap, we'll ensure that
outputs can be marked as unconfirmed credits more than once whatsoever.
As is, the test case fails, which proves that this is an issue. A later
commit will resolve this and the test case should pass.
In this commit, we modify the way we store spending transaction hashes
for unconfirmed spends. Now, rather than only keeping track of one
possible unconfirmed spend, we track multiple in order to ensure we
properly handle transaction replacements, like in the case of RBF,
double spends, etc. With this in, the double spent tests recently added
should now pass.
In this commit, we slightly refactor the existing moveMinedTx method
and split it into two: deleteUnminedTx and updateMinedBalance. We do
this as moveMinedTx is no longer moving the transaction from the unmined
bucket to the mined, instead it just removes it from the unmined bucket.
In this commit, we remove most of the common code between insertMinedTx
and moveMinedTx. Now, all the common logic is handled within
insertMinedTx, and moveMinedTx only contains its unique logic.
In this commit, we add a set of double spend tests to ensure that we can
properly detect and handle them. At this point, we do not do this, but a
follow up commit will address this.
In this commit, we add a new method to the Store object that allows
callers to *manually* remove any conflicting transactions. At times,
it’s the case that while we were offline another transaction was
broadcast that double spends our own, or with the existence of RBF,
another replacement transaction was generated. In this case, when we
come back online, the tx will be rejected. Currently, we have no way of
removing such transaction sot avoid the retransmit-then-reject-dance.
This commit fixes that by adding RemoveUnminedTx.
While making these tests compile and pass, we ended up tripping on the
broken bolt cursor usage painfully discovered in dcrwallet, so i've
ported that fix over as well. Would have learned about that a whole
lot sooner if those tests were never disabled..
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 fixes a bug introduced by the new segwit behavior upstream in
btcd. Previously it was a nice optimization to skip the extra
serialization, and compute the txid manually from the serialized
transaction.
However, with segwit, doing that will generate an invalid txid, since
txid’s should ignore any witness data. Therefore, we now fallback to
using msgTx.TxSha() which serializes the transaction without witness
data before calculating the txid.
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 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 changes the wtxmgr.Store.UnminedTxs method to sort transactions,
using the Kahn topological sort algorithm, before returning
transactions to the caller. This is possible because transactions
form a sort of directed acyclic graph (DAG) where transactions
reference other transactions to spend their outputs (multiple
referenced outputs from a single transaction spent by the same
transaction count as a single graph edge).
This prevents the possibility of orphan rejection errors when sending
unmined transactions to a full node at startup. As these transactions
are sent using the sendrawtransaction RPC, which does not permit
orphans, this topological sort is required.
Fixes#156.
