Browse Source

WalletService: clean up and fix callback handling

* Migrate callbacks registered under provisional (tx-output tuples) keys
  as soon as their txids are known. Leave a txid breadcrumb in place so
  register_callbacks and check_callback_called can find it and the
  migrated list of callbacks.

* Invoke callbacks via list comprehensions, retaining only the callbacks
  that return False. The old code was buggy, as it was removing elements
  from the callback lists while iterating over them, which would cause
  callbacks to be skipped.

* The existing code would fail to call any "confirmed" callbacks for a
  remove-only transaction if no "unconfirmed" callbacks had been
  registered for that transaction, and it would discontinue monitoring
  of a transaction after just one "confirmed" callback had returned
  True, even if other "confirmed" callbacks returned False to remain
  registered. This commit overhauls the logic to fix all of these bugs.

* Delete emptied callback lists from the dicts to be tidy.
master
Matt Whitlock 4 years ago
parent
commit
8fe5b7b712
  1. 94
      jmclient/jmclient/wallet_service.py

94
jmclient/jmclient/wallet_service.py

@ -75,10 +75,11 @@ class WalletService(Service):
# Dicts of registered callbacks, by type # Dicts of registered callbacks, by type
# and then by txinfo, for events # and then by txinfo, for events
# on transactions. # on transactions.
self.callbacks = {} self.callbacks = {
self.callbacks["all"] = [] "all": [], # note: list, not dict
self.callbacks["unconfirmed"] = {} "unconfirmed": {},
self.callbacks["confirmed"] = {} "confirmed": {},
}
self.restart_callback = None self.restart_callback = None
@ -201,9 +202,12 @@ class WalletService(Service):
# note that in this case, txid is ignored. # note that in this case, txid is ignored.
self.callbacks["all"].extend(callbacks) self.callbacks["all"].extend(callbacks)
elif cb_type in ["unconfirmed", "confirmed"]: elif cb_type in ["unconfirmed", "confirmed"]:
if txinfo not in self.callbacks[cb_type]: if callbacks:
self.callbacks[cb_type][txinfo] = [] reg = self.callbacks[cb_type].setdefault(txinfo, [])
self.callbacks[cb_type][txinfo].extend(callbacks) if isinstance(reg, str):
# found a txid breadcrumb for this txinfo
reg = self.callbacks[cb_type].setdefault(reg, [])
reg.extend(callbacks)
else: else:
assert False, "Invalid argument: " + cb_type assert False, "Invalid argument: " + cb_type
@ -371,10 +375,16 @@ class WalletService(Service):
# remove these from the list # remove these from the list
f(txd, txid) f(txd, txid)
# The tuple given as the second possible key for the dict # txid is not always available at the time of callback registration.
# is such because txid is not always available # Migrate any callbacks registered under the provisional key, and
# at the time of callback registration). # leave a txid breadcrumb so check_callback_called can find it.
possible_keys = [txid, tuple((x.scriptPubKey, x.nValue) for x in txd.vout)] txos = tuple((x.scriptPubKey, x.nValue) for x in txd.vout)
for cb_type in ["unconfirmed", "confirmed"]:
callbacks = self.callbacks[cb_type]
reg = callbacks.get(txos)
if isinstance(reg, list):
callbacks.setdefault(txid, [])[:0] = reg
callbacks[txos] = txid
# note that len(added_utxos) > 0 is not a sufficient condition for # note that len(added_utxos) > 0 is not a sufficient condition for
# the tx being new, since wallet.add_new_utxos will happily re-add # the tx being new, since wallet.add_new_utxos will happily re-add
@ -389,22 +399,26 @@ class WalletService(Service):
if len(added_utxos) > 0 or len(removed_utxos) > 0 \ if len(added_utxos) > 0 or len(removed_utxos) > 0 \
or txid in self.active_txs: or txid in self.active_txs:
if confs == 0: if confs == 0:
for k in possible_keys: callbacks = [f for f in
if k in self.callbacks["unconfirmed"]: self.callbacks["unconfirmed"].pop(txid, [])
for f in self.callbacks["unconfirmed"][k]: if not f(txd, txid)]
# True implies success, implies removal: if callbacks:
if f(txd, txid): self.callbacks["unconfirmed"][txid] = callbacks
self.callbacks["unconfirmed"][k].remove(f) else:
# keep monitoring for conf > 0: self.callbacks["unconfirmed"].pop(txos, None)
self.active_txs[txid] = txd if self.callbacks["confirmed"].get(txid):
# keep monitoring for conf > 0:
self.active_txs[txid] = txd
elif confs > 0: elif confs > 0:
for k in possible_keys: callbacks = [f for f in
if k in self.callbacks["confirmed"]: self.callbacks["confirmed"].pop(txid, [])
for f in self.callbacks["confirmed"][k]: if not f(txd, txid, confs)]
if f(txd, txid, confs): if callbacks:
self.callbacks["confirmed"][k].remove(f) self.callbacks["confirmed"][txid] = callbacks
if txid in self.active_txids: else:
self.active_txs.pop(txid, None) self.callbacks["confirmed"].pop(txos, None)
# no more callbacks registered; stop monitoring tx
self.active_txs.pop(txid, None)
def check_callback_called(self, txinfo, callback, cbtype, msg): def check_callback_called(self, txinfo, callback, cbtype, msg):
""" Intended to be a deferred Task to be scheduled some """ Intended to be a deferred Task to be scheduled some
@ -413,15 +427,27 @@ class WalletService(Service):
If the callback was previously called, return True, otherwise False. If the callback was previously called, return True, otherwise False.
""" """
assert cbtype in ["unconfirmed", "confirmed"] assert cbtype in ["unconfirmed", "confirmed"]
if txinfo in self.callbacks[cbtype]: callbacks = self.callbacks[cbtype]
if callback in self.callbacks[cbtype][txinfo]: if isinstance(txinfo, str):
txid = txinfo
reg = callbacks.get(txid)
else:
txid = None
reg = callbacks.get(txinfo)
if isinstance(reg, str):
# found a txid breadcrumb for this txinfo
txid = reg
reg = callbacks.get(txid)
if reg:
if callback in reg:
# the callback was not called, drop it and warn # the callback was not called, drop it and warn
self.callbacks[cbtype][txinfo].remove(callback) reg.remove(callback)
# TODO - dangling txids in self.active_txs will if not reg:
# be caused by this, but could also happen for del callbacks[txinfo]
# other reasons; possibly add logic to ensure that if txid:
# this never occurs, although their presence should callbacks.pop(txid, None)
# not cause a functional error. # no more callbacks registered; stop monitoring tx
self.active_txs.pop(txid, None)
jlog.info("Timed out: " + msg) jlog.info("Timed out: " + msg)
return False return False
# if callback is not in the list, it was already # if callback is not in the list, it was already

Loading…
Cancel
Save