From 4be92a9e0805f2b191314de4208a9a19986e43bf Mon Sep 17 00:00:00 2001 From: Peter Neuroth Date: Wed, 20 Nov 2024 13:45:33 +0100 Subject: [PATCH 1/4] plugin: Make onion fields optional As reported by JssDWt in https://github.com/Blockstream/greenlight/issues/541, the fields of the onion passed to the htlc_accepted_hook might not be set. To avoid any unnecessary panics we make them optional and handle an unset field in the lsp plugin. Signed-off-by: Peter Neuroth --- libs/gl-plugin/src/lsp.rs | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/libs/gl-plugin/src/lsp.rs b/libs/gl-plugin/src/lsp.rs index 19579e9f0..ca8549915 100644 --- a/libs/gl-plugin/src/lsp.rs +++ b/libs/gl-plugin/src/lsp.rs @@ -15,8 +15,8 @@ use serde_json::Value; struct Onion { payload: tlv::SerializedTlvStream, short_channel_id: Option, - forward_msat: Amount, - outgoing_cltv_value: u32, + forward_msat: Option, + outgoing_cltv_value: Option, #[serde(deserialize_with = "from_hex")] shared_secret: Vec, #[serde(deserialize_with = "from_hex")] @@ -59,7 +59,18 @@ pub async fn on_htlc_accepted(plugin: Plugin, v: Value) -> Result a, + None => { + // An onion without an `amt_to_forward` is unorthodox and can not + // be processed by this plugin. Skip it. + return Ok(serde_json::to_value(HtlcAcceptedResponse { + result: "continue".to_string(), + ..Default::default() + }) + .unwrap()); + } + }; let res = if htlc_amt.msat() < onion_amt.msat() { log::info!( From 09fc51fb59fdc950fe8554aa41c4c7e6bd2ace14 Mon Sep 17 00:00:00 2001 From: Peter Neuroth Date: Thu, 21 Nov 2024 09:30:29 +0100 Subject: [PATCH 2/4] tests: Check that we handle missing fields Adds a check that we return "continue" on the htlc_accepted_hook if we don't understand an onion payload in the lsp client plugin because of missing fields. Signed-off-by: Peter Neuroth --- libs/gl-plugin/src/lsp.rs | 8 +- libs/gl-plugin/src/tlv.rs | 7 + libs/gl-testing/tests/test_node.py | 378 ++++++++++++++++------------- 3 files changed, 227 insertions(+), 166 deletions(-) diff --git a/libs/gl-plugin/src/lsp.rs b/libs/gl-plugin/src/lsp.rs index ca8549915..c7adeb440 100644 --- a/libs/gl-plugin/src/lsp.rs +++ b/libs/gl-plugin/src/lsp.rs @@ -62,8 +62,12 @@ pub async fn on_htlc_accepted(plugin: Plugin, v: Value) -> Result a, None => { - // An onion without an `amt_to_forward` is unorthodox and can not - // be processed by this plugin. Skip it. + // An onion payload without an `amt_to_forward` is unorthodox and + // can not be processed by this plugin. Skip it. + log::debug!( + "lsp-plugin: got an onion payload={} without an amt forward_msat.", + req.onion.payload + ); return Ok(serde_json::to_value(HtlcAcceptedResponse { result: "continue".to_string(), ..Default::default() diff --git a/libs/gl-plugin/src/tlv.rs b/libs/gl-plugin/src/tlv.rs index ce4b0ef3c..7a5fb8dee 100644 --- a/libs/gl-plugin/src/tlv.rs +++ b/libs/gl-plugin/src/tlv.rs @@ -191,6 +191,13 @@ impl<'de> Deserialize<'de> for SerializedTlvStream { } } +impl std::fmt::Display for SerializedTlvStream { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + let s = hex::encode(SerializedTlvStream::to_bytes(self.clone())); + write!(f, "{s}") + } +} + #[cfg(test)] mod tests { use super::*; diff --git a/libs/gl-testing/tests/test_node.py b/libs/gl-testing/tests/test_node.py index 4bead92bf..d3689db75 100644 --- a/libs/gl-testing/tests/test_node.py +++ b/libs/gl-testing/tests/test_node.py @@ -22,8 +22,7 @@ def test_node_start(scheduler, clients): def test_node_connect(scheduler, clients, bitcoind): - """Register and schedule a node, then connect to it. - """ + """Register and schedule a node, then connect to it.""" c = clients.new() c.register(configure=True) n = c.node() @@ -32,8 +31,7 @@ def test_node_connect(scheduler, clients, bitcoind): def test_node_signer(clients, executor): - """Ensure we can attach a signer to the node and sign an invoice. - """ + """Ensure we can attach a signer to the node and sign an invoice.""" c = clients.new() c.register(configure=True) n = c.node() @@ -42,12 +40,9 @@ def test_node_signer(clients, executor): # it'll block until the signer connects. fi = executor.submit( n.invoice, - label='test', - amount_msat=clnpb.AmountOrAny( - amount=clnpb.Amount(msat=42000) - ), + label="test", + amount_msat=clnpb.AmountOrAny(amount=clnpb.Amount(msat=42000)), description="desc", - ) # Now attach the signer and the above call should return @@ -73,22 +68,22 @@ def test_node_network(node_factory, clients, bitcoind): # Handshake needs signer for ECDH of Noise_XK exchange s = c.signer().run_in_thread() - gl1.connect_peer(l2.info['id'], f'127.0.0.1:{l2.daemon.port}') + gl1.connect_peer(l2.info["id"], f"127.0.0.1:{l2.daemon.port}") # Now open a channel from l2 -> gl1 - l2.fundwallet(sats=2*10**6) - l2.rpc.fundchannel(c.node_id.hex(), 'all') + l2.fundwallet(sats=2 * 10**6) + l2.rpc.fundchannel(c.node_id.hex(), "all") bitcoind.generate_block(6, wait_for_mempool=1) # Now wait for the channel to confirm wait_for(lambda: len(gl1.list_peer_channels().channels) > 0) - wait_for(lambda: gl1.list_peer_channels().channels[0].state == 2) # CHANNELD_NORMAL - wait_for(lambda: len(l1.rpc.listchannels()['channels']) == 2) + wait_for(lambda: gl1.list_peer_channels().channels[0].state == 2) # CHANNELD_NORMAL + wait_for(lambda: len(l1.rpc.listchannels()["channels"]) == 2) inv = gl1.invoice( amount_msat=clnpb.AmountOrAny(amount=clnpb.Amount(msat=10000)), description="desc", - label="lbl" + label="lbl", ).bolt11 decoded = l1.rpc.decodepay(inv) @@ -99,21 +94,18 @@ def test_node_network(node_factory, clients, bitcoind): def test_node_invoice_preimage(clients): - """Test that we can create an invoice with a specific preimage - """ + """Test that we can create an invoice with a specific preimage""" c = clients.new() c.register(configure=True) s = c.signer().run_in_thread() gl1 = c.node() preimage = bytes.fromhex("00" * 32) - expected = '66687aadf862bd776c8fc18b8e9f8e20089714856ee233b3902a591d0d5f2925' + expected = "66687aadf862bd776c8fc18b8e9f8e20089714856ee233b3902a591d0d5f2925" i = gl1.invoice( - label='lbl', - amount_msat=clnpb.AmountOrAny( - amount=clnpb.Amount(msat=21000000) - ), + label="lbl", + amount_msat=clnpb.AmountOrAny(amount=clnpb.Amount(msat=21000000)), description="desc", preimage=preimage, ) @@ -154,76 +146,75 @@ def test_node_invoice_amountless(bitcoind, node_factory, clients): s = c.signer().run_in_thread() # Now open a channel from l2 <- gl1 (this could be easier...) - gl1.connect_peer(l1.info['id'], f'127.0.0.1:{l1.daemon.port}') + gl1.connect_peer(l1.info["id"], f"127.0.0.1:{l1.daemon.port}") addr = gl1.new_address().bech32 txid = bitcoind.rpc.sendtoaddress(addr, 1) bitcoind.generate_block(1, wait_for_mempool=[txid]) wait_for(lambda: len(gl1.list_funds().outputs) == 1) gl1.fund_channel( - id=bytes.fromhex(l1.info['id']), - amount=clnpb.AmountOrAll(amount=clnpb.Amount(msat=10**9)) + id=bytes.fromhex(l1.info["id"]), + amount=clnpb.AmountOrAll(amount=clnpb.Amount(msat=10**9)), ) bitcoind.generate_block(6, wait_for_mempool=1) - + # the channels array is optional wait_for(lambda: len(gl1.list_peer_channels().channels) > 0) - wait_for(lambda: gl1.list_peer_channels().channels[0].state == 2) # CHANNELD_NORMAL + wait_for(lambda: gl1.list_peer_channels().channels[0].state == 2) # CHANNELD_NORMAL # Generate an invoice without amount: - inv = l1.rpc.call('invoice', payload={ - 'label': 'test', - 'amount_msat': 'any', - 'description': 'desc' - })['bolt11'] + inv = l1.rpc.call( + "invoice", + payload={"label": "test", "amount_msat": "any", "description": "desc"}, + )["bolt11"] print(inv) print(l1.rpc.decodepay(inv)) - p = gl1.pay( - inv, - clnpb.Amount(msat=31337) - ) - invs = l1.rpc.listinvoices()['invoices'] + p = gl1.pay(inv, clnpb.Amount(msat=31337)) + invs = l1.rpc.listinvoices()["invoices"] - assert(len(invs) == 1) - assert(invs[0]['status'] == 'paid') + assert len(invs) == 1 + assert invs[0]["status"] == "paid" def test_node_listpays_preimage(clients, node_factory, bitcoind): - """Test that GL nodes correctly return incoming payment details. - """ + """Test that GL nodes correctly return incoming payment details.""" c = clients.new() c.register(configure=True) s = c.signer().run_in_thread() gl1 = c.node() l1 = node_factory.get_node() - gl1.connect_peer(l1.info['id'], f'127.0.0.1:{l1.daemon.port}') + gl1.connect_peer(l1.info["id"], f"127.0.0.1:{l1.daemon.port}") addr = gl1.new_address().bech32 txid = bitcoind.rpc.sendtoaddress(addr, 1) bitcoind.generate_block(1, wait_for_mempool=[txid]) wait_for(lambda: len(gl1.list_funds().outputs) == 1) gl1.fund_channel( - id=bytes.fromhex(l1.info['id']), - amount=clnpb.AmountOrAll(amount=clnpb.Amount(msat=10**9)) + id=bytes.fromhex(l1.info["id"]), + amount=clnpb.AmountOrAll(amount=clnpb.Amount(msat=10**9)), ) bitcoind.generate_block(6, wait_for_mempool=1) # the channels array is optional wait_for(lambda: len(gl1.list_peer_channels().channels) > 0) - wait_for(lambda: gl1.list_peer_channels().channels[0].state == 2) # CHANNELD_NORMAL + wait_for(lambda: gl1.list_peer_channels().channels[0].state == 2) # CHANNELD_NORMAL - preimage = "00"*32 + preimage = "00" * 32 - i = l1.rpc.call("invoice", { - 'amount_msat': '2100sat', - 'label': 'lbl', - 'description': 'desc', - 'preimage': preimage, - }) + i = l1.rpc.call( + "invoice", + { + "amount_msat": "2100sat", + "label": "lbl", + "description": "desc", + "preimage": preimage, + }, + ) from rich.rule import Rule from rich.console import Console + console = Console() console.rule("[bold red]") - gl1.pay(i['bolt11']) + gl1.pay(i["bolt11"]) console.rule("[bold red]") pay = gl1.listpays() @@ -247,30 +238,33 @@ def test_lsp_jit_fee(clients, node_factory, bitcoind): We test multiple parts and overpay slightly to verify that even that works out ok. + We also check that we can handle unorthodox onion payloads that + don't carry fields that we expect. + """ c = clients.new() c.register(configure=True) s = c.signer().run_in_thread() gl1 = c.node() l1 = node_factory.get_node() - gl1.connect_peer(l1.info['id'], f'127.0.0.1:{l1.daemon.port}') + gl1.connect_peer(l1.info["id"], f"127.0.0.1:{l1.daemon.port}") l1.fundwallet(10**6) - wait_for(lambda: len(l1.rpc.listfunds()['outputs']) > 0) - l1.rpc.fundchannel(c.node_id.hex(), 'all') + wait_for(lambda: len(l1.rpc.listfunds()["outputs"]) > 0) + l1.rpc.fundchannel(c.node_id.hex(), "all") bitcoind.generate_block(6, wait_for_mempool=1) - wait_for(lambda: l1.rpc.listpeerchannels()['channels'][0]['state'] == 'CHANNELD_NORMAL') + wait_for( + lambda: l1.rpc.listpeerchannels()["channels"][0]["state"] == "CHANNELD_NORMAL" + ) # Create an invoice for 10k - preimage = '00' * 32 - payment_hash = '66687aadf862bd776c8fc18b8e9f8e20089714856ee233b3902a591d0d5f2925' + preimage = "00" * 32 + payment_hash = "66687aadf862bd776c8fc18b8e9f8e20089714856ee233b3902a591d0d5f2925" parts = 2 p1, p2 = 300000, 700000 # The two parts we're going to use fee = 100000 # Fee leverage on each part inv = gl1.invoice( - label='lbl', - amount_msat=clnpb.AmountOrAny( - amount=clnpb.Amount(msat=p1 + p2 - parts * fee) - ), + label="lbl", + amount_msat=clnpb.AmountOrAny(amount=clnpb.Amount(msat=p1 + p2 - parts * fee)), description="desc", preimage=bytes.fromhex(preimage), ).bolt11 @@ -278,78 +272,131 @@ def test_lsp_jit_fee(clients, node_factory, bitcoind): decoded = l1.rpc.decodepay(inv) # So we have an invoice for 100k, now send it in two parts: - o1 = l1.rpc.createonion(hops=[{ - "pubkey": c.node_id.hex(), - "payload": ( - "30" + - "0203" + "0493e0" + # amt_to_forward: 30k - "04016e" + # 110 blocks CLTV - "0823" + decoded['payment_secret'] + "0f4240" + # Payment_secret + total_msat - "FB0142" # Typ 251 payload 0x42 (testing we don't lose TLVs) - ) - }], assocdata=payment_hash) - - o2 = l1.rpc.createonion(hops=[{ - "pubkey": c.node_id.hex(), - "payload": ( - "30" + - "0203" + "0aae60" + # amt_to_forward: 70k - "04016e" + # 110 blocks CLTV - "0823" + decoded['payment_secret'] + "0f4240" + # Payment_secret + total_msat - "FB0142" # Typ 251 payload 0x42 (testing we don't lose TLVs) - ) - }], assocdata=payment_hash) - - l1.rpc.call('sendonion', { - 'onion': o1['onion'], - 'first_hop': { - "id": c.node_id.hex(), - "amount_msat": f"{p1 - fee}msat", - "delay": 21, + o1 = l1.rpc.createonion( + hops=[ + { + "pubkey": c.node_id.hex(), + "payload": ( + "30" + + "0203" + + "0493e0" # amt_to_forward: 30k + + "04016e" # 110 blocks CLTV + + "0823" + + decoded["payment_secret"] + + "0f4240" # Payment_secret + total_msat + + "FB0142" # Typ 251 payload 0x42 (testing we don't lose TLVs) + ), + } + ], + assocdata=payment_hash, + ) + + o2 = l1.rpc.createonion( + hops=[ + { + "pubkey": c.node_id.hex(), + "payload": ( + "30" + + "0203" + + "0aae60" # amt_to_forward: 70k + + "04016e" # 110 blocks CLTV + + "0823" + + decoded["payment_secret"] + + "0f4240" # Payment_secret + total_msat + + "FB0142" # Typ 251 payload 0x42 (testing we don't lose TLVs) + ), + } + ], + assocdata=payment_hash, + ) + + l1.rpc.call( + "sendonion", + { + "onion": o1["onion"], + "first_hop": { + "id": c.node_id.hex(), + "amount_msat": f"{p1 - fee}msat", + "delay": 21, + }, + "payment_hash": payment_hash, + "partid": 1, + "groupid": 1, + "shared_secrets": o1["shared_secrets"], }, - 'payment_hash': payment_hash, - 'partid': 1, - 'groupid': 1, - 'shared_secrets': o1['shared_secrets'], - }) - l1.rpc.call('sendonion', { - 'onion': o2['onion'], - 'first_hop': { - "id": c.node_id.hex(), - "amount_msat": f"{p2 - fee}msat", - "delay": 21, + ) + l1.rpc.call( + "sendonion", + { + "onion": o2["onion"], + "first_hop": { + "id": c.node_id.hex(), + "amount_msat": f"{p2 - fee}msat", + "delay": 21, + }, + "payment_hash": payment_hash, + "partid": 2, + "groupid": 1, + "shared_secrets": o1["shared_secrets"], }, - 'payment_hash': payment_hash, - 'partid': 2, - 'groupid': 1, - 'shared_secrets': o1['shared_secrets'], - }) + ) # Check that custom payloads are preserved. See the type=251 field # at the end of the onion-construction above. - c.find_node().process.wait_for_log(r'Serialized payload: .*fb0142') + c.find_node().process.wait_for_log(r"Serialized payload: .*fb0142") + + l1.rpc.waitsendpay(payment_hash=payment_hash, partid=1, timeout=10) + l1.rpc.waitsendpay(payment_hash=payment_hash, partid=2, timeout=10) + + # Check that custom payloads that we do not understand are skipped. The + # following onion payload does not contain `amt_to_forward`, see Bolt4. + o3 = l1.rpc.createonion( + hops=[ + { + "pubkey": c.node_id.hex(), + "payload": ( + "26" + + "04016e" # 110 blocks CLTV + + "0821" + + "0000000000000000000000000000000000000000000000000000000000000000" + + "00" # payment_data with dummy values + ), + } + ], + assocdata="0000000000000000000000000000000000000000000000000000000000000000", + ) - l1.rpc.waitsendpay( - payment_hash=payment_hash, - partid=1, - timeout=10 + l1.rpc.call( + "sendonion", + { + "onion": o3["onion"], + "first_hop": { + "id": c.node_id.hex(), + "amount_msat": "1msat", + "delay": 21, + }, + "payment_hash": "0000000000000000000000000000000000000000000000000000000000000000", + "partid": 0, + "groupid": 1, + "shared_secrets": o3["shared_secrets"], + }, ) - l1.rpc.waitsendpay( - payment_hash=payment_hash, - partid=2, - timeout=10 + + # The htlc should be passed on to the next consumer. + c.find_node().process.wait_for_log( + r"lsp-plugin: got an onion payload=.* without an amt forward_msat.", + timeout=10, ) def test_custommsg(clients, node_factory, bitcoind, executor): - """Connect a GL node and a CLN node and have them talk. - """ + """Connect a GL node and a CLN node and have them talk.""" c = clients.new() c.register(configure=True) s = c.signer().run_in_thread() gl1 = c.node() l1 = node_factory.get_node() - gl1.connect_peer(l1.info['id'], f'127.0.0.1:{l1.daemon.port}') + gl1.connect_peer(l1.info["id"], f"127.0.0.1:{l1.daemon.port}") # Part 1: CLN -> GL m = gl1.stream_custommsg() @@ -358,32 +405,34 @@ def test_custommsg(clients, node_factory, bitcoind, executor): # Give the executor time to actually register itself with the # notification import time + time.sleep(1) l1.rpc.sendcustommsg(c.node_id.hex(), "FFFFDEADBEEF") res = f.result(1) - assert res.payload == b'\xff\xff\xde\xad\xbe\xef' - assert res.peer_id.hex() == l1.info['id'] + assert res.payload == b"\xff\xff\xde\xad\xbe\xef" + assert res.peer_id.hex() == l1.info["id"] # Part 2: GL -> CLN - gl1.send_custommsg(bytes.fromhex(l1.info['id']), b"\xff\xffhello") + gl1.send_custommsg(bytes.fromhex(l1.info["id"]), b"\xff\xffhello") - l1.daemon.wait_for_logs([ - r'connectd: peer_in INVALID 65535', - r'Calling custommsg hook of plugin chanbackup', - ]) + l1.daemon.wait_for_logs( + [ + r"connectd: peer_in INVALID 65535", + r"Calling custommsg hook of plugin chanbackup", + ] + ) def test_node_reconnect(clients, scheduler, node_factory, bitcoind): - """Connect from GL to a peer, then restart and we should reconnect. - """ + """Connect from GL to a peer, then restart and we should reconnect.""" c = clients.new() c.register(configure=True) s = c.signer().run_in_thread() gl1 = c.node() l1 = node_factory.get_node() - gl1.connect_peer(l1.info['id'], f'127.0.0.1:{l1.daemon.port}') + gl1.connect_peer(l1.info["id"], f"127.0.0.1:{l1.daemon.port}") time.sleep(1) node = scheduler.nodes[0] @@ -393,33 +442,33 @@ def test_node_reconnect(clients, scheduler, node_factory, bitcoind): gl1 = c.node() rpc = scheduler.nodes[0].rpc() - wait_for(lambda: rpc.listpeers()['peers'] != []) - peer = rpc.listpeers()['peers'][0] - assert peer['connected'] - assert peer['id'] == l1.info['id'] + wait_for(lambda: rpc.listpeers()["peers"] != []) + peer = rpc.listpeers()["peers"][0] + assert peer["connected"] + assert peer["id"] == l1.info["id"] def test_vls_crash_repro( - clients: Clients, - scheduler: Scheduler, - node_factory, - bitcoind) -> None: - """Reproduce an overflow panic in VLS v0.10.0. """ - l1, = node_factory.line_graph(1, opts={'experimental-anchors': None}) - assert(l1.rpc.getinfo()['version'] == 'v24.02gl1') + clients: Clients, scheduler: Scheduler, node_factory, bitcoind +) -> None: + """Reproduce an overflow panic in VLS v0.10.0.""" + (l1,) = node_factory.line_graph(1, opts={"experimental-anchors": None}) + assert l1.rpc.getinfo()["version"] == "v24.02gl1" c = clients.new() c.register(configure=True) s = c.signer().run_in_thread() gl1 = c.node() - gl1.connect_peer(l1.info['id'], f'127.0.0.1:{l1.daemon.port}') + gl1.connect_peer(l1.info["id"], f"127.0.0.1:{l1.daemon.port}") l1.fundwallet(10**7) - l1.rpc.fundchannel(c.node_id.hex(), 'all') + l1.rpc.fundchannel(c.node_id.hex(), "all") bitcoind.generate_block(1, wait_for_mempool=1) - wait_for(lambda: l1.rpc.listpeerchannels()['channels'][0]['state'] == 'CHANNELD_NORMAL') + wait_for( + lambda: l1.rpc.listpeerchannels()["channels"][0]["state"] == "CHANNELD_NORMAL" + ) # Roei reports that the issue can be triggered by sending n from # l1 to n1 and then (n-1)msat back to l1 @@ -427,7 +476,7 @@ def test_vls_crash_repro( inv = gl1.invoice( amount_msat=clnpb.AmountOrAny(amount=clnpb.Amount(msat=2500000)), description="desc", - label="lbl" + label="lbl", ).bolt11 l1.rpc.pay(inv) @@ -435,10 +484,7 @@ def test_vls_crash_repro( def test_sendpay_signer( - clients: Clients, - scheduler: Scheduler, - node_factory, - bitcoind + clients: Clients, scheduler: Scheduler, node_factory, bitcoind ) -> None: """Ensure that `sendpay` works with the signer. @@ -455,35 +501,39 @@ def test_sendpay_signer( s = c.signer().run_in_thread() gl1 = c.node() - gl1.connect_peer(l1.info['id'], f'127.0.0.1:{l1.daemon.port}') + gl1.connect_peer(l1.info["id"], f"127.0.0.1:{l1.daemon.port}") addr = gl1.new_address().bech32 txid = bitcoind.rpc.sendtoaddress(addr, 1) bitcoind.generate_block(1, wait_for_mempool=[txid]) wait_for(lambda: len(gl1.list_funds().outputs) == 1) gl1.fund_channel( - id=bytes.fromhex(l1.info['id']), - amount=clnpb.AmountOrAll(amount=clnpb.Amount(msat=10**10)) + id=bytes.fromhex(l1.info["id"]), + amount=clnpb.AmountOrAll(amount=clnpb.Amount(msat=10**10)), ) bitcoind.generate_block(1, wait_for_mempool=1) - wait_for(lambda: l1.rpc.listpeerchannels()['channels'][0]['state'] == 'CHANNELD_NORMAL') + wait_for( + lambda: l1.rpc.listpeerchannels()["channels"][0]["state"] == "CHANNELD_NORMAL" + ) amount = 10**9 - chan = l1.rpc.listpeerchannels()['channels'][0] - inv = l1.rpc.invoice(amount, 'lbl', 'desc') - payment_hash = bytes.fromhex(inv['payment_hash']) - payment_secret = bytes.fromhex(inv['payment_secret']) - - route = [clnpb.SendpayRoute( - id=bytes.fromhex(l1.info['id']), - channel=chan['short_channel_id'], - amount_msat=clnpb.Amount(msat=amount), - delay=18, - ),] + chan = l1.rpc.listpeerchannels()["channels"][0] + inv = l1.rpc.invoice(amount, "lbl", "desc") + payment_hash = bytes.fromhex(inv["payment_hash"]) + payment_secret = bytes.fromhex(inv["payment_secret"]) + + route = [ + clnpb.SendpayRoute( + id=bytes.fromhex(l1.info["id"]), + channel=chan["short_channel_id"], + amount_msat=clnpb.Amount(msat=amount), + delay=18, + ), + ] req = clnpb.SendpayRequest( route=route, payment_hash=payment_hash, - bolt11=inv['bolt11'], + bolt11=inv["bolt11"], payment_secret=payment_secret, ) From 9b66b16c048fd1bbe2d057201725221299af82ae Mon Sep 17 00:00:00 2001 From: Peter Neuroth Date: Thu, 21 Nov 2024 14:50:27 +0100 Subject: [PATCH 3/4] plugin: Add macro to shortcut the hook flow Adds a macro that allows us to replace `unwrap`s in order to prevent panics. The macro shortcuts further execution and returns from the htlc_accepted_hook. Signed-off-by: Peter Neuroth --- libs/gl-plugin/src/lsp.rs | 66 ++++++++++++++++-------------- libs/gl-testing/tests/test_node.py | 4 +- 2 files changed, 38 insertions(+), 32 deletions(-) diff --git a/libs/gl-plugin/src/lsp.rs b/libs/gl-plugin/src/lsp.rs index c7adeb440..509211c98 100644 --- a/libs/gl-plugin/src/lsp.rs +++ b/libs/gl-plugin/src/lsp.rs @@ -54,27 +54,35 @@ struct HtlcAcceptedResponse { const TLV_FORWARD_AMT: u64 = 2; const TLV_PAYMENT_SECRET: u64 = 8; +/// A macro to break out of the current hook flow and return a `continue` +/// signal to core-lightning. This is to be used when we don't know how to +/// handle a given payload or as a shortcut in case we could identify that the +/// incoming htlc is not part of a LSP jit channel opening. +macro_rules! unwrap_or_continue { + ($res:expr) => { + match $res { + Ok(x) => x, + Err(e) => { + log::debug!("Lsp-plugin continue, reason: {}", e.to_string()); + return Ok(serde_json::to_value(HtlcAcceptedResponse { + result: "continue".to_string(), + ..Default::default() + }) + .expect("Could not serialize json value")); + } + } + }; +} + pub async fn on_htlc_accepted(plugin: Plugin, v: Value) -> Result { - let req: HtlcAcceptedRequest = serde_json::from_value(v).unwrap(); + let req: HtlcAcceptedRequest = unwrap_or_continue!(serde_json::from_value(v)); log::debug!("Decoded {:?}", &req); let htlc_amt = req.htlc.amount_msat; - let onion_amt = match req.onion.forward_msat { - Some(a) => a, - None => { - // An onion payload without an `amt_to_forward` is unorthodox and - // can not be processed by this plugin. Skip it. - log::debug!( - "lsp-plugin: got an onion payload={} without an amt forward_msat.", - req.onion.payload - ); - return Ok(serde_json::to_value(HtlcAcceptedResponse { - result: "continue".to_string(), - ..Default::default() - }) - .unwrap()); - } - }; + let onion_amt = unwrap_or_continue!(req.onion.forward_msat.ok_or(format!( + "payload={} is missing forward_msat", + &req.onion.payload + ))); let res = if htlc_amt.msat() < onion_amt.msat() { log::info!( @@ -85,7 +93,10 @@ pub async fn on_htlc_accepted(plugin: Plugin, v: Value) -> Result Result Date: Thu, 21 Nov 2024 15:10:39 +0100 Subject: [PATCH 4/4] plugin: Remove linter errors from lsp Remove clippy errors on lsp.rs: `the borrowed expression implements the required traits` Signed-off-by: Peter Neuroth --- libs/gl-plugin/src/lsp.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libs/gl-plugin/src/lsp.rs b/libs/gl-plugin/src/lsp.rs index 509211c98..2d778f011 100644 --- a/libs/gl-plugin/src/lsp.rs +++ b/libs/gl-plugin/src/lsp.rs @@ -161,7 +161,7 @@ where { match buffer { None => serializer.serialize_none(), - Some(buffer) => serializer.serialize_str(&hex::encode(&buffer.as_ref())), + Some(buffer) => serializer.serialize_str(&hex::encode(buffer)), } } @@ -172,5 +172,5 @@ where { use serde::de::Error; String::deserialize(deserializer) - .and_then(|string| Vec::from_hex(&string).map_err(|err| Error::custom(err.to_string()))) + .and_then(|string| Vec::from_hex(string).map_err(|err| Error::custom(err.to_string()))) }