From bc447b2af07714d63164ef814ccc7ce208d9fe41 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Mon, 13 Nov 2023 13:38:01 +1030 Subject: [PATCH] pyln-client: don't automatically turn JSON into Millisatoshi class. Now _msat fields are all integers (last conversion 23.08) we can simply leave them alone, rather than trying to convert them. And for turning Millisatoshi into JSON, we simply globally replace the default encoding function to try ".to_json()" on items, which allows anything to be marshalled. The global replacement was interfering with other uses of JSON, such as the clnrest plugin. Signed-off-by: Rusty Russell Changelog-Changed: pyln-client: no longer autoconverts _msat field to Millisatoshi class (leaves as ints). --- contrib/pyln-client/pyln/client/lightning.py | 86 +++---------------- contrib/pyln-client/pyln/client/plugin.py | 1 - contrib/pyln-testing/pyln/testing/fixtures.py | 4 +- contrib/pyln-testing/pyln/testing/utils.py | 3 +- tests/test_closing.py | 4 +- 5 files changed, 18 insertions(+), 80 deletions(-) diff --git a/contrib/pyln-client/pyln/client/lightning.py b/contrib/pyln-client/pyln/client/lightning.py index 70b693b514b7..a2c104889179 100644 --- a/contrib/pyln-client/pyln/client/lightning.py +++ b/contrib/pyln-client/pyln/client/lightning.py @@ -10,18 +10,15 @@ from typing import Optional, Union -def _patched_default(self, obj): - return getattr(obj.__class__, "to_json", _patched_default.default)(obj) - +def to_json_default(self, obj): + """ + Try to use .to_json() if available, otherwise use the normal JSON default method. + """ + return getattr(obj.__class__, "to_json", old_json_default)(obj) -def monkey_patch_json(patch=True): - is_patched = JSONEncoder.default == _patched_default - if patch and not is_patched: - _patched_default.default = JSONEncoder.default # Save unmodified - JSONEncoder.default = _patched_default # Replace it. - elif not patch and is_patched: - JSONEncoder.default = _patched_default.default +old_json_default = JSONEncoder.default +JSONEncoder.default = to_json_default class RpcError(ValueError): @@ -41,8 +38,7 @@ class Millisatoshi: """ A subtype to represent thousandths of a satoshi. - Many JSON API fields are expressed in millisatoshis: these automatically - get turned into Millisatoshi types. Converts to and from int. + If you put this in an object, converting to JSON automatically makes it an "...msat" string, so you can safely hand it even to our APIs which treat raw numbers as satoshis. Converts to and from int. """ def __init__(self, v: Union[int, str, Decimal]): """ @@ -286,10 +282,8 @@ def __del__(self) -> None: class UnixDomainSocketRpc(object): - def __init__(self, socket_path, executor=None, logger=logging, encoder_cls=json.JSONEncoder, decoder=json.JSONDecoder(), caller_name=None): + def __init__(self, socket_path, executor=None, logger=logging, caller_name=None): self.socket_path = socket_path - self.encoder_cls = encoder_cls - self.decoder = decoder self.executor = executor self.logger = logger self._notify = None @@ -303,7 +297,7 @@ def __init__(self, socket_path, executor=None, logger=logging, encoder_cls=json. self.next_id = 1 def _writeobj(self, sock, obj): - s = json.dumps(obj, ensure_ascii=False, cls=self.encoder_cls) + s = json.dumps(obj, ensure_ascii=False) sock.sendall(bytearray(s, 'UTF-8')) def _readobj(self, sock, buff=b''): @@ -318,7 +312,7 @@ def _readobj(self, sock, buff=b''): return {'error': 'Connection to RPC server lost.'}, buff else: buff = parts[1] - obj, _ = self.decoder.raw_decode(parts[0].decode("UTF-8")) + obj, _ = json.JSONDecoder().raw_decode(parts[0].decode("UTF-8")) return obj, buff def __getattr__(self, name): @@ -480,67 +474,13 @@ class LightningRpc(UnixDomainSocketRpc): between calls, but it does not (yet) support concurrent calls. """ - class LightningJSONEncoder(json.JSONEncoder): - def default(self, o): - try: - return o.to_json() - except NameError: - pass - return json.JSONEncoder.default(self, o) - - class LightningJSONDecoder(json.JSONDecoder): - def __init__(self, *, object_hook=None, parse_float=None, - parse_int=None, parse_constant=None, - strict=True, object_pairs_hook=None, - patch_json=True): - self.object_hook_next = object_hook - super().__init__(object_hook=self.millisatoshi_hook, parse_float=parse_float, parse_int=parse_int, parse_constant=parse_constant, strict=strict, object_pairs_hook=object_pairs_hook) - - @staticmethod - def replace_amounts(obj): - """ - Recursively replace _msat fields with appropriate values with Millisatoshi. - """ - if isinstance(obj, dict): - for k, v in obj.items(): - # Objects ending in msat are not treated specially! - if k.endswith('msat') and not isinstance(v, dict): - if isinstance(v, list): - obj[k] = [Millisatoshi(e) for e in v] - # FIXME: Deprecated "listconfigs" gives two 'null' fields: - # "lease-fee-base-msat": null, - # "channel-fee-max-base-msat": null, - # FIXME: Removed for v23.08, delete this code in 24.08? - elif v is None: - obj[k] = None - else: - obj[k] = Millisatoshi(v) - else: - obj[k] = LightningRpc.LightningJSONDecoder.replace_amounts(v) - elif isinstance(obj, list): - obj = [LightningRpc.LightningJSONDecoder.replace_amounts(e) for e in obj] - - return obj - - def millisatoshi_hook(self, obj): - obj = LightningRpc.LightningJSONDecoder.replace_amounts(obj) - if self.object_hook_next: - obj = self.object_hook_next(obj) - return obj - - def __init__(self, socket_path, executor=None, logger=logging, - patch_json=True): + def __init__(self, socket_path, executor=None, logger=logging): super().__init__( socket_path, executor, - logger, - self.LightningJSONEncoder, - self.LightningJSONDecoder() + logger ) - if patch_json: - monkey_patch_json(patch=True) - def addgossip(self, message): """ Inject this (hex-encoded) gossip message. diff --git a/contrib/pyln-client/pyln/client/plugin.py b/contrib/pyln-client/pyln/client/plugin.py index 59d99bf7886a..6df2c6a5d7c9 100644 --- a/contrib/pyln-client/pyln/client/plugin.py +++ b/contrib/pyln-client/pyln/client/plugin.py @@ -679,7 +679,6 @@ def _write_locked(self, obj: JSONType) -> None: # then utf8 ourselves. s = bytes(json.dumps( obj, - cls=LightningRpc.LightningJSONEncoder, ensure_ascii=False ) + "\n\n", encoding='utf-8') with self.write_lock: diff --git a/contrib/pyln-testing/pyln/testing/fixtures.py b/contrib/pyln-testing/pyln/testing/fixtures.py index 4bfce3578d1d..c0b0f9cfb134 100644 --- a/contrib/pyln-testing/pyln/testing/fixtures.py +++ b/contrib/pyln-testing/pyln/testing/fixtures.py @@ -348,8 +348,8 @@ def is_msat_request(checker, instance): return False def is_msat_response(checker, instance): - """An integer, but we convert to Millisatoshi in JSON parsing""" - return type(instance) is Millisatoshi + """A positive integer""" + return type(instance) is int and instance >= 0 def is_txid(checker, instance): """Bitcoin transaction ID""" diff --git a/contrib/pyln-testing/pyln/testing/utils.py b/contrib/pyln-testing/pyln/testing/utils.py index 7f39637d2c76..49d75cb8bfe5 100644 --- a/contrib/pyln-testing/pyln/testing/utils.py +++ b/contrib/pyln-testing/pyln/testing/utils.py @@ -684,12 +684,11 @@ class PrettyPrintingLightningRpc(LightningRpc): Also validates (optional) schemas for us. """ def __init__(self, socket_path, executor=None, logger=logging, - patch_json=True, jsonschemas={}): + jsonschemas={}): super().__init__( socket_path, executor, logger, - patch_json, ) self.jsonschemas = jsonschemas self.check_request_schemas = True diff --git a/tests/test_closing.py b/tests/test_closing.py index 28e40fac0944..c1c354da75dd 100644 --- a/tests/test_closing.py +++ b/tests/test_closing.py @@ -3716,7 +3716,7 @@ def test_closing_anchorspend_htlc_tx_rbf(node_factory, bitcoind): assert 'anchors_zero_fee_htlc_tx/even' in only_one(l1.rpc.listpeerchannels()['channels'])['channel_type']['names'] # We reduce l1's UTXOs so it's forced to use more than one UTXO to push. - fundsats = int(only_one(l1.rpc.listfunds()['outputs'])['amount_msat'].to_satoshi()) + fundsats = int(Millisatoshi(only_one(l1.rpc.listfunds()['outputs'])['amount_msat']).to_satoshi()) psbt = l1.rpc.fundpsbt("all", "1000perkw", 1000)['psbt'] # Pay 5k sats in fees, send most to l2 psbt = l1.rpc.addpsbtoutput(fundsats - 20000 - 5000, psbt, destination=l2.rpc.newaddr()['bech32'])['psbt'] @@ -3909,7 +3909,7 @@ def test_peer_anchor_push(node_factory, bitcoind, executor, chainparams): wait_for_announce=True) # We splinter l2's funds so it's forced to use more than one UTXO to push. - fundsats = int(only_one(l2.rpc.listfunds()['outputs'])['amount_msat'].to_satoshi()) + fundsats = int(Millisatoshi(only_one(l2.rpc.listfunds()['outputs'])['amount_msat']).to_satoshi()) OUTPUT_SAT = 10000 NUM_OUTPUTS = 10 psbt = l2.rpc.fundpsbt("all", "1000perkw", 1000)['psbt']