openchannel: test new hook chainable mechanics

tests/plugins/accepter_close_to.py

@@ -1,35 +0,0 @@
-#!/usr/bin/env python3
-"""Simple plugin to test the openchannel_hook's
-   'close_to' address functionality.
-
-   If the funding amount is:
-      - a multiple of 11: we send back a valid address (regtest)
-      - a multiple of 7: we send back an empty address
-      - a multiple of 5: we send back an address for the wrong chain (mainnet)
-      - otherwise: we don't include the close_to
-"""
-
-from pyln.client import Plugin, Millisatoshi
-
-plugin = Plugin()
-
-
-@plugin.hook('openchannel')
-def on_openchannel(openchannel, plugin, **kwargs):
-    # - a multiple of 11: we send back a valid address (regtest)
-    if Millisatoshi(openchannel['funding_satoshis']).to_satoshi() % 11 == 0:
-        return {'result': 'continue', 'close_to': 'bcrt1q7gtnxmlaly9vklvmfj06amfdef3rtnrdazdsvw'}
-
-    # - a multiple of 7: we send back an empty address
-    if Millisatoshi(openchannel['funding_satoshis']).to_satoshi() % 7 == 0:
-        return {'result': 'continue', 'close_to': ''}
-
-    # - a multiple of 5: we send back an address for the wrong chain (mainnet)
-    if Millisatoshi(openchannel['funding_satoshis']).to_satoshi() % 5 == 0:
-        return {'result': 'continue', 'close_to': 'bc1qlq8srqnz64wgklmqvurv7qnr4rvtq2u96hhfg2'}
-
-    # - otherwise: we don't include the close_to
-    return {'result': 'continue'}
-
-
-plugin.run()

tests/plugins/openchannel_hook_accept.py

@@ -0,0 +1,19 @@
+#!/usr/bin/env python3
+"""Plugin to test openchannel_hook
+
+Will simply accept any channel. Useful fot testing chained hook.
+"""
+
+from pyln.client import Plugin
+
+plugin = Plugin()
+
+
+@plugin.hook('openchannel')
+def on_openchannel(openchannel, plugin, **kwargs):
+    msg = "accept on principle"
+    plugin.log(msg)
+    return {'result': 'continue'}
+
+
+plugin.run()

tests/plugins/openchannel_hook_accepter.py

@@ -0,0 +1,51 @@
+#!/usr/bin/env python3
+"""Simple plugin to test the openchannel_hook's
+   'close_to' address functionality.
+
+   If the funding amount is:
+      - 100005sat: we reject correctly w/o close_to
+      - 100004sat: we reject invalid by setting a close_to
+      - 100003sat: we send back a valid address (regtest)
+      - 100002sat: we send back an empty address
+      - 100001sat: we send back an address for the wrong chain (mainnet)
+      - otherwise: we don't include the close_to
+"""
+
+from pyln.client import Plugin, Millisatoshi
+
+plugin = Plugin()
+
+
+@plugin.hook('openchannel')
+def on_openchannel(openchannel, plugin, **kwargs):
+    # - 100005sat: we reject correctly w/o close_to
+    if Millisatoshi(openchannel['funding_satoshis']).to_satoshi() == 100005:
+        msg = "reject for a reason"
+        plugin.log(msg)
+        return {'result': 'reject', 'error_message': msg}
+
+    # - 100004sat: we reject invalid by setting a close_to
+    if Millisatoshi(openchannel['funding_satoshis']).to_satoshi() == 100004:
+        msg = "I am a broken plugin"
+        plugin.log(msg)
+        return {'result': 'reject', 'error_message': msg,
+                'close_to': "bcrt1q7gtnxmlaly9vklvmfj06amfdef3rtnrdazdsvw"}
+
+    # - 100003sat: we send back a valid address (regtest)
+    if Millisatoshi(openchannel['funding_satoshis']).to_satoshi() == 100003:
+        return {'result': 'continue', 'close_to': 'bcrt1q7gtnxmlaly9vklvmfj06amfdef3rtnrdazdsvw'}
+
+    # - 100002sat: we send back an empty address
+    if Millisatoshi(openchannel['funding_satoshis']).to_satoshi() == 100002:
+        return {'result': 'continue', 'close_to': ''}
+
+    # - 100001sat: we send back an address for the wrong chain (mainnet)
+    if Millisatoshi(openchannel['funding_satoshis']).to_satoshi() == 100001:
+        return {'result': 'continue', 'close_to': 'bc1qlq8srqnz64wgklmqvurv7qnr4rvtq2u96hhfg2'}
+
+    # - otherwise: accept and don't include the close_to
+    plugin.log("accept by design")
+    return {'result': 'continue'}
+
+
+plugin.run()

tests/plugins/openchannel_hook_reject.py

@@ -0,0 +1,20 @@
+#!/usr/bin/env python3
+"""Plugin to test openchannel_hook
+
+Will simply reject any channel with message "reject on principle".
+Useful fot testing chained hook.
+"""
+
+from pyln.client import Plugin
+
+plugin = Plugin()
+
+
+@plugin.hook('openchannel')
+def on_openchannel(openchannel, plugin, **kwargs):
+    msg = "reject on principle"
+    plugin.log(msg)
+    return {'result': 'reject', 'error_message': msg}
+
+
+plugin.run()

tests/test_connection.py

@@ -1050,13 +1050,13 @@ def test_funding_cancel_race(node_factory, bitcoind, executor):
 def test_funding_close_upfront(node_factory, bitcoind):
     l1 = node_factory.get_node()
 
-    opts = {'plugin': os.path.join(os.getcwd(), 'tests/plugins/accepter_close_to.py')}
+    opts = {'plugin': os.path.join(os.getcwd(), 'tests/plugins/openchannel_hook_accepter.py')}
     l2 = node_factory.get_node(options=opts)
 
     # The 'accepter_close_to' plugin uses the channel funding amount to determine
     # whether or not to include a 'close_to' address
-    amt_normal = 100007     # continues without returning a close_to
+    amt_normal = 100000     # continues without returning a close_to
-    amt_addr = 100001       # returns valid regtest address
+    amt_addr = 100003       # returns valid regtest address
 
     remote_valid_addr = 'bcrt1q7gtnxmlaly9vklvmfj06amfdef3rtnrdazdsvw'
 

tests/test_plugin.py

@@ -538,13 +538,7 @@ def test_openchannel_hook(node_factory, bitcoind):
     """
     opts = [{}, {'plugin': os.path.join(os.getcwd(), 'tests/plugins/reject_odd_funding_amounts.py')}]
     l1, l2 = node_factory.line_graph(2, fundchannel=False, opts=opts)
-
+    l1.fundwallet(10**6)
-    # Get some funds.
-    addr = l1.rpc.newaddr()['bech32']
-    txid = bitcoind.rpc.sendtoaddress(addr, 10)
-    numfunds = len(l1.rpc.listfunds()['outputs'])
-    bitcoind.generate_block(1, txid)
-    wait_for(lambda: len(l1.rpc.listfunds()['outputs']) > numfunds)
 
     # Even amount: works.
     l1.rpc.fundchannel(l2.info['id'], 100000)
@@ -574,6 +568,70 @@ def test_openchannel_hook(node_factory, bitcoind):
         l1.rpc.fundchannel(l2.info['id'], 100001)
 
 
+def test_openchannel_hook_error_handling(node_factory, bitcoind):
+    """ l2 uses a plugin that should fatal() crash the node.
+
+    This is because the plugin rejects a channel while
+    also setting a close_to address which isn't allowed.
+    """
+    opts = {'plugin': os.path.join(os.getcwd(), 'tests/plugins/openchannel_hook_accepter.py')}
+    # openchannel_reject_but_set_close_to.py')}
+    l1 = node_factory.get_node()
+    l2 = node_factory.get_node(options=opts,
+                               expect_fail=True,
+                               may_fail=True,
+                               allow_broken_log=True)
+    l1.connect(l2)
+    l1.fundwallet(10**6)
+
+    # next fundchannel should fail fatal() for l2
+    with pytest.raises(RpcError, match=r'Owning subdaemon openingd died'):
+        l1.rpc.fundchannel(l2.info['id'], 100004)
+    assert l2.daemon.is_in_log("Plugin rejected openchannel but also set close_to")
+
+
+def test_openchannel_hook_chaining(node_factory, bitcoind):
+    """ l2 uses a set of plugin that all use the openchannel_hook.
+
+    We test that chaining works by using multiple plugins in a way
+    that we check for the first plugin that rejects prevents from evaluating
+    further plugin responses down the chain.
+
+    """
+    opts = [{}, {'plugin': [
+        os.path.join(os.path.dirname(__file__), '..', 'tests/plugins/openchannel_hook_accept.py'),
+        os.path.join(os.path.dirname(__file__), '..', 'tests/plugins/openchannel_hook_accepter.py'),
+        os.path.join(os.path.dirname(__file__), '..', 'tests/plugins/openchannel_hook_reject.py')
+    ]}]
+    l1, l2 = node_factory.line_graph(2, fundchannel=False, opts=opts)
+    l1.fundwallet(10**6)
+
+    hook_msg = "openchannel_hook rejects and says '"
+    # 100005sat fundchannel should fail fatal() for l2
+    # because hook_accepter.py rejects on that amount 'for a reason'
+    with pytest.raises(RpcError, match=r'They sent error channel'):
+        l1.rpc.fundchannel(l2.info['id'], 100005)
+
+    # Note: hook chain order is currently undefined, because hooks are registerd
+    #       as a result of the getmanifest call, which may take some random time.
+    #       We need to workaround that fact, so test can be stable
+    correct_order = l2.daemon.is_in_log(hook_msg + "reject for a reason")
+    if correct_order:
+        assert l2.daemon.wait_for_log(hook_msg + "reject for a reason")
+        # the other plugin must not be called
+        assert not l2.daemon.is_in_log("reject on principle")
+    else:
+        assert l2.daemon.wait_for_log(hook_msg + "reject on principle")
+        # the other plugin must not be called
+        assert not l2.daemon.is_in_log("reject for a reason")
+
+    # 100000sat is good for hook_accepter, so it should fail 'on principle'
+    # at third hook openchannel_reject.py
+    with pytest.raises(RpcError, match=r'They sent error channel'):
+        l1.rpc.fundchannel(l2.info['id'], 100000)
+    assert l2.daemon.wait_for_log(hook_msg + "reject on principle")
+
+
 @unittest.skipIf(not DEVELOPER, "without DEVELOPER=1, gossip v slow")
 def test_htlc_accepted_hook_fail(node_factory):
     """Send payments from l1 to l2, but l2 just declines everything.