Merge branch 'master' into fix-flake-test_no_delay-9218

Author: ScuttoZ

.github/workflows/repro.yml

@@ -92,7 +92,7 @@ jobs:
         uses: dawidd6/action-send-mail@v17
         with:
           server_address: smtp.gmail.com
-          server_port: 587
+          server_port: 465
           secure: true
           username: ${{ secrets.EMAIL_USERNAME }}
           password: ${{ secrets.EMAIL_PASSWORD }}

Cargo.lock

@@ -0,0 +1,15 @@
+- Build with `--configure --enable-debugbuild` and `uv run make`.  Much faster with `uv run make RUST=0`.
+- Env vars for tests: `RUST=0` (if RUST plugins were not built), `VALGRIND=0` (much faster), `TIMEOUT=10` to speed failures (or 100 if VALGRIND=1).
+- *gen.[ch] files are generated by the Makefile.
+- ccan/ is imported from ccan repo: PRs should go there, then `make update-ccan`.
+- Insert BOLT quotes (current version is checked out in .tmp.lightningrfc/) in source when dealing with protocol work, for `make check-source-bolt`.  This ensures we keep up with spec changes.  `...` means skip some, but not over a section boundary. `...` at the start means "continues directly from previous quote".
+- `make check-source` does formatting checks.
+- assert-based unit tests can "#include ../file.c", OR add objects to Makefile targets: `make update-mocks` will regenerate mocks for functions from link errors.
+- Commits should be reviewable, bisectable, and include tests.  Pattern: one commit adds a python test with @pytest.mark.xfail(strict=True), next commit fixes the problem and removes that line.
+- Commits which fix crashes or bug MUST quote the bug for later searches: do not rely on being able to find the bug report in future!
+- Commits which create signficant user (not developer!) visible changes should have Changelog-(Added|Deprecated|Changed|Fixed|EXPERIMENTAL) for assembling CHANGELOG.md at release time.
+- Deprecations must use the deprecation infrastructure, and append to the table in doc/developers-guide/deprecated-features.md
+- Adding a new JSON-RPC command requires the most careful design: start with doc/schemas/NAME.json and add doc/NAME.json to MARKDOWNPAGES in doc/Makefile.  `make doc-all` will regen the rest.
+- In pytest integration tests, name nodes l1, l2, etc in creation order.  This matches the log prefixes they use (`lightningd-1` etc) and on test failure the logs and other ephemera will in /tmp/ltests-*/TESTNAME*/lightning-1/.
+- In pytest, never assert that a command raises an exception without specifying *exactly what* it raises!  Use `with pytest.raises(RpcError, match='xxx'):`
+- tal_bytelen / tal_count of NULL are defined to be zero, and you may assume this.

DEV.md

@@ -193,13 +193,7 @@ def bitcoind(request, directory, teardown_checks):
 
     yield bitcoind
 
-    try:
-        bitcoind.stop()
-    except Exception:
-        bitcoind.proc.kill()
-    bitcoind.proc.wait()
-
-    bitcoind.cleanup_files()
+    bitcoind.kill()
 
 
 class TeardownErrors(object):

contrib/pyln-testing/pyln/testing/fixtures.py

@@ -0,0 +1,25 @@
+#!/usr/bin/env python3
+"""Generic inline plugin shim: bridges lightningd stdio <-> inline-plugin.sock in cwd.
+Used by inline_plugin() in pyln/testing/utils.py."""
+import os
+import socket
+import sys
+import threading
+
+
+def _stdin_to_sock(conn):
+    while chunk := sys.stdin.buffer.read1(4096):
+        conn.sendall(chunk)
+    # Stdin closed means lightningd is done with us: exit immediately so the
+    # OS closes the socket and the serve thread can accept the next connection.
+    os._exit(0)
+
+
+conn = socket.socket(socket.AF_UNIX)
+conn.connect('inline-plugin.sock')
+
+threading.Thread(target=_stdin_to_sock, args=(conn,), daemon=True).start()
+
+while chunk := conn.recv(4096):
+    sys.stdout.buffer.write(chunk)
+    sys.stdout.buffer.flush()

contrib/pyln-testing/pyln/testing/inline-plugin.py

@@ -11,8 +11,10 @@
 from pyln.client import LightningRpc
 from pyln.client import Millisatoshi
 from pyln.client import NodeVersion
+from pyln.client import Plugin
 
 import ephemeral_port_reserve  # type: ignore
+import tempfile
 import json
 import logging
 import lzma
@@ -22,13 +24,15 @@
 import random
 import re
 import shutil
+import socket
 import sqlite3
 import string
 import struct
 import subprocess
 import sys
 import threading
 import time
+import types
 import warnings
 
 BITCOIND_CONFIG = {
@@ -78,6 +82,8 @@ def env(name, default=None):
 VALGRIND = env("VALGRIND") == "1"
 TEST_NETWORK = env("TEST_NETWORK", 'regtest')
 TEST_DEBUG = env("TEST_DEBUG", "0") == "1"
+
+INLINE_PLUGIN_PATH = os.path.join(os.path.dirname(__file__), 'inline-plugin.py')
 SLOW_MACHINE = env("SLOW_MACHINE", "0") == "1"
 DEPRECATED_APIS = env("DEPRECATED_APIS", "0") == "1"
 TIMEOUT = int(env("TIMEOUT", 180 if SLOW_MACHINE else 60))
@@ -166,25 +172,46 @@ def get_tx_p2wsh_outnum(bitcoind, tx, amount):
     return None
 
 
-unused_port_lock = threading.Lock()
-unused_port_set = set()
+_PORT_LOCK_DIR = Path(tempfile.gettempdir()) / "pyln-testing-ports"
+_PORT_LOCK_DIR.mkdir(exist_ok=True)
 
 
 def reserve_unused_port():
     """Get an unused port: avoids handing out the same port unless it's been
     returned"""
-    with unused_port_lock:
-        while True:
-            port = ephemeral_port_reserve.reserve()
-            if port not in unused_port_set:
-                break
-        unused_port_set.add(port)
+    while True:
+        port = ephemeral_port_reserve.reserve()
 
-    return port
+        lock_path = _PORT_LOCK_DIR / f"{port}.lock"
+        try:
+            fd = os.open(str(lock_path), os.O_CREAT | os.O_EXCL | os.O_WRONLY)
+            os.write(fd, str(os.getpid()).encode())
+            os.close(fd)
+            return port
+        except FileExistsError:
+            continue
 
 
 def drop_unused_port(port):
-    unused_port_set.remove(port)
+    if port:
+        lock_path = _PORT_LOCK_DIR / f"{port}.lock"
+        lock_path.unlink(missing_ok=True)
+
+
+def cleanup_stale_port_locks():
+    """Remove lockfiles whose owning process no longer exists."""
+    try:
+        for lock_path in _PORT_LOCK_DIR.glob("*.lock"):
+            try:
+                pid = int(lock_path.read_text())
+                try:
+                    os.kill(pid, 0)  # signal 0 = existence check, no actual signal
+                except ProcessLookupError:
+                    lock_path.unlink(missing_ok=True)
+            except (ValueError, PermissionError, FileNotFoundError):
+                pass
+    except Exception:
+        pass  # best-effort, never crash the test run over cleanup
 
 
 class TailableProc(object):
@@ -285,6 +312,8 @@ def kill(self):
 
     def cleanup_files(self):
         """Ensure files are closed."""
+        cleanup_stale_port_locks()
+
         for f in ["stdout_write", "stderr_write", "stdout_read", "stderr_read"]:
             try:
                 getattr(self, f).close()
@@ -454,10 +483,7 @@ def __init__(self, bitcoin_dir="/tmp/bitcoind-test", rpcport=None):
         TailableProc.__init__(self, bitcoin_dir, verbose=False)
 
         if rpcport is None:
-            self.reserved_rpcport = reserve_unused_port()
-            rpcport = self.reserved_rpcport
-        else:
-            self.reserved_rpcport = None
+            rpcport = reserve_unused_port()
 
         self.bitcoin_dir = bitcoin_dir
         self.rpcport = rpcport
@@ -494,9 +520,15 @@ def __init__(self, bitcoin_dir="/tmp/bitcoind-test", rpcport=None):
         self.rpc = SimpleBitcoinProxy(btc_conf_file=self.conf_file)
         self.proxies = []
 
-    def __del__(self):
-        if self.reserved_rpcport is not None:
-            drop_unused_port(self.reserved_rpcport)
+    def kill(self):
+        try:
+            self.stop()
+        except Exception:
+            self.proc.kill()
+        self.proc.wait()
+
+        self.cleanup_files()
+        drop_unused_port(self.rpcport)
 
     def start(self, wallet_file=None):
         TailableProc.start(self)
@@ -1069,6 +1101,11 @@ def grpc(self):
             creds,
             options=(('grpc.ssl_target_name_override', 'cln'),)
         )
+
+        # Force the connect+handshake to finish now, instead of lazily on
+        # the first RPC the caller happens to make.
+        grpc.channel_ready_future(channel).result(timeout=10)
+
         from pyln import grpc as clnpb
         return clnpb.NodeStub(channel)
 
@@ -1786,7 +1823,8 @@ def get_nodes(self, num_nodes, opts=None):
     def get_node(self, node_id=None, options=None, dbfile=None,
                  bkpr_dbfile=None, feerates=(15000, 11000, 7500, 3750),
                  start=True, wait_for_bitcoind_sync=True, may_fail=False,
-                 expect_fail=False, cleandir=True, gossip_store_file=None, unused_grpc_port=True, **kwargs):
+                 expect_fail=False, cleandir=True, gossip_store_file=None, unused_grpc_port=True,
+                 inline_plugin=None, **kwargs):
         node_id = self.get_node_id() if not node_id else node_id
         port = reserve_unused_port()
         grpc_port = self.get_unused_port() if unused_grpc_port else None
@@ -1830,6 +1868,11 @@ def get_node(self, node_id=None, options=None, dbfile=None,
             shutil.copy(gossip_store_file, os.path.join(node.daemon.lightning_dir, TEST_NETWORK,
                                                         'gossip_store'))
 
+        if inline_plugin is not None:
+            if 'plugin' not in node.daemon.opts:
+                node.daemon.opts['plugin'] = INLINE_PLUGIN_PATH
+            _inline_plugin(node, inline_plugin)
+
         if start:
             try:
                 node.start(wait_for_bitcoind_sync)
@@ -1944,3 +1987,65 @@ def killall(self, expected_successes):
             drop_unused_port(p)
 
         return not unexpected_fail, err_msgs
+
+
+def _inline_plugin(node, setup_fn):
+    """Set up an inline plugin serve thread for a not-yet-started node.
+
+    Normally called via get_node(inline_plugin=setup_fn).  The plugin's cwd
+    (set by lightningd) is node.daemon.lightning_dir/TEST_NETWORK/, which is
+    where the shim looks for inline-plugin.sock.
+
+    Example::
+
+        def setup(plugin):
+            @plugin.method('greet')
+            def greet(name, plugin):
+                return {'message': f'hello {name}'}
+
+        l1 = node_factory.get_node(inline_plugin=setup)
+        assert l1.rpc.greet('world') == {'message': 'hello world'}
+    """
+    sock_path = os.path.join(node.daemon.lightning_dir, TEST_NETWORK, 'inline-plugin.sock')
+    srv = socket.socket(socket.AF_UNIX)
+    srv.bind(sock_path)
+    srv.listen(1)
+
+    plugin = Plugin(autopatch=False)
+    setup_fn(plugin)
+
+    def serve():
+        while True:
+            conn, _ = srv.accept()
+
+            class _SockWriter:
+                def write(self, data):
+                    try:
+                        conn.sendall(data)
+                    except OSError:
+                        pass
+
+                def flush(self):
+                    pass
+
+            writer = _SockWriter()
+            plugin.stdout = types.SimpleNamespace(buffer=writer, flush=writer.flush)
+
+            partial = b""
+            while True:
+                try:
+                    chunk = conn.recv(4096)
+                except OSError:
+                    break
+                if not chunk:
+                    break
+                partial += chunk
+                msgs = partial.split(b'\n\n')
+                if len(msgs) < 2:
+                    continue
+                try:
+                    partial = plugin._multi_dispatch(msgs)
+                except Exception:
+                    break
+
+    threading.Thread(target=serve, daemon=True).start()

contrib/pyln-testing/pyln/testing/utils.py

@@ -46,7 +46,7 @@ Here's a checklist for the release process.
 
 1. Update CHANGELOG.md by changing rc(N-1) to rcN. Update the changelog list with information from newly merged PRs also.
 2. Update the package versions: `uv run make update-versions NEW_VERSION=v<VERSION>rcN`
-3. Add a PR with the rcN.
+3. Add a PR with the rcN, and merge it.
 4. Tag it `git pull && git tag -s v<VERSION>rcN && git push origin v<VERSION>rcN`.
 5. Pushing the tag automatically starts the "Release 🚀" CI job, creating a draft pre-release and uploading reproducible builds with their `SHA256SUMS` files signed by the project key.
 6. Set up the reproducible build environment by running the script `contrib/cl-repro.sh` to generate the necessary builder images.

doc/contribute-to-core-lightning/release-checklist.md

@@ -12,8 +12,7 @@ We have a 3 month release cycle, and the last two versions are supported.
 
 ## Reporting a Vulnerability
 
-To report security vulnerabilities, please send an email to one of the following addresses:
-- `rusty@rustcorp.com.au`
+To report security vulnerabilities, please send an email to:
 - `security@blockstream.com`
 
 Note: These email addresses are exclusively for vulnerability reporting.

doc/contribute-to-core-lightning/security-policy.md

@@ -35,3 +35,4 @@ tracing = { version = "^0.1", features = ["async-await", "log"] }
 [dev-dependencies]
 tokio = { version = "1", features = ["macros", "rt-multi-thread"] }
 cln-grpc = { workspace = true }
+cln-rpc = { workspace = true }

plugins/Cargo.toml

@@ -943,6 +943,11 @@ static struct command_result *json_getroutes(struct command *cmd,
 				    maxdelay_allowed);
 	}
 
+	if (node_id_eq(source, dest)) {
+		return command_fail(cmd, JSONRPC2_INVALID_PARAMS,
+				    "source and destination must be different");
+	}
+
 	if (command_check_only(cmd))
 		return command_check_done(cmd);