adb.get_tx_height: return "LOCAL" height if missing full signed tx
get_tx_height previously did not consider whether the walletDB has the full tx for the corresponding txid, and could consider a tx even "mined" and spv-ed, even if we were missing the raw tx. Now if the full tx is missing or the tx in the db is partial, get_tx_height considers it to be LOCAL. In particular the txbatcher, in `run_iteration`, - first saves a tx as local *unsigned* (to reserve UTXOs), - then signs it and tries to broadcast it. The history tx will later transition to local and signed, after we get back the broadcasted tx via the Synchronizer dance. In the meantime there is a race where we have an unsigned tx in the history, but the txid could already transition to mempool or even to mined, before we download the full raw tx from the server. During that time window, it makes the adb state more consistent to just consider the history tx to be local, as done here.
This commit is contained in:
@@ -289,7 +289,7 @@ class AddressSynchronizer(Logger, EventListener):
|
||||
# of add_transaction tx, we might learn of more-and-more inputs of
|
||||
# being is_mine, as we roll the gap_limit forward
|
||||
is_coinbase = tx.inputs()[0].is_coinbase_input()
|
||||
tx_height = self.get_tx_height(tx_hash).height
|
||||
tx_height = self.get_tx_height(tx_hash, force_local_if_missing_tx=False).height
|
||||
if not allow_unrelated:
|
||||
# note that during sync, if the transactions are not properly sorted,
|
||||
# it could happen that we think tx is unrelated but actually one of the inputs is is_mine.
|
||||
@@ -695,10 +695,24 @@ class AddressSynchronizer(Logger, EventListener):
|
||||
if old_height != wanted_height:
|
||||
util.trigger_callback('adb_set_future_tx', self, txid)
|
||||
|
||||
def get_tx_height(self, tx_hash: str) -> TxMinedInfo:
|
||||
def get_tx_height(
|
||||
self,
|
||||
tx_hash: str,
|
||||
*,
|
||||
force_local_if_missing_tx: bool = True,
|
||||
) -> TxMinedInfo:
|
||||
if tx_hash is None: # ugly backwards compat...
|
||||
return TxMinedInfo(height=TX_HEIGHT_LOCAL, conf=0)
|
||||
with self.lock:
|
||||
if force_local_if_missing_tx:
|
||||
# It can happen for a txid in any state (unconf/unverified/verified) that we
|
||||
# don't have the raw tx yet, simply due to network timing.
|
||||
# Having only a partial tx is another variant of this.
|
||||
# FIXME in fact even if we have a complete tx saved, the server might have
|
||||
# a different tx if only the witness differs. We should compare wtxids.
|
||||
tx = self.db.get_transaction(tx_hash)
|
||||
if tx is None or isinstance(tx, PartialTransaction):
|
||||
return TxMinedInfo(height=TX_HEIGHT_LOCAL, conf=0)
|
||||
verified_tx_mined_info = self.db.get_verified_tx(tx_hash)
|
||||
if verified_tx_mined_info:
|
||||
conf = max(self.get_local_height() - verified_tx_mined_info.height + 1, 0)
|
||||
|
||||
@@ -221,7 +221,7 @@ class TxBatcher(Logger):
|
||||
|
||||
class TxBatch(Logger):
|
||||
|
||||
def __init__(self, wallet, storage: StoredDict):
|
||||
def __init__(self, wallet: 'Abstract_Wallet', storage: StoredDict):
|
||||
Logger.__init__(self)
|
||||
self.wallet = wallet
|
||||
self.storage = storage
|
||||
@@ -404,6 +404,9 @@ class TxBatch(Logger):
|
||||
return
|
||||
|
||||
# add tx to wallet, in order to reserve utxos
|
||||
# note: This saves the tx as local *unsigned*.
|
||||
# It will transition to local and signed, after we broadcast
|
||||
# the signed tx and get it back via the Synchronizer dance.
|
||||
self.wallet.adb.add_transaction(tx)
|
||||
# await password
|
||||
if not await self.sign_transaction(tx):
|
||||
|
||||
Reference in New Issue
Block a user