Skip to content

Conversation

rustyrussell
Copy link
Contributor

This contains a new magic string for CI to complain about, then steal's Matt's commit which reduces severity.

Closes #8466

@rustyrussell rustyrussell added this to the v25.09 milestone Aug 19, 2025
@rustyrussell rustyrussell requested a review from cdecker as a code owner August 19, 2025 04:19
@madelinevibes madelinevibes added the Type::Bug An error, flaw, or fault that produces an incorrect or unexpected result label Aug 19, 2025
Copy link
Collaborator

@endothermicdev endothermicdev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK 2362d67

rustyrussell and others added 2 commits August 21, 2025 16:13
This message is logged when connectd tries to shut down a peer
connection but the transmit buffer remains full for too long, maybe
because the peer has crashed or has lost connectivity. Logging this
message at the BROKEN level is inappropriate because BROKEN is intended
to flag logic errors that imply incorrect code in CLN. The error in
question here is actually a runtime error, which does not imply
incorrect code (at least on our side), so demote the log message to the
UNUSUAL level. (Even this is still probably too severe, as this message
is logged rather more frequently than "unusual" would suggest.)

Changelog-None
Closes: ElementsProject#5678
…d_chans_leases

```
 FAILED tests/test_bookkeeper.py::test_bookkeeping_missed_chans_leases - AssertionError: assert [{'tag': 'channel_open', 'credit_msat': 506268000, 'debit_msat': 0}, {'tag': 'lease_fee', 'credit_msat': 0, 'debit_msat': 6268000}, {'tag': 'invoice', 'credit_msat': 0, 'debit_msat': 11000000}, {'tag': 'onchain_fee', 'credit_msat': 1314000, 'debit_msat': 0}] == [{'tag': 'channel_open', 'credit_msat': 506268000, 'debit_msat': 0}, {'tag': 'lease_fee', 'credit_msat': 0, 'debit_msat': 6268000}, {'tag': 'onchain_fee', 'credit_msat': 1314000, 'debit_msat': 0}, {'tag': 'invoice', 'credit_msat': 0, 'debit_msat': 11000000}]
  
  At index 2 diff: {'tag': 'invoice', 'credit_msat': 0, 'debit_msat': 11000000} != {'tag': 'onchain_fee', 'credit_msat': 1314000, 'debit_msat': 0}
  
  Full diff:
    [
        {
            'credit_msat': 506268000,
            'debit_msat': 0,
            'tag': 'channel_open',
        },
        {
            'credit_msat': 0,
            'debit_msat': 6268000,
            'tag': 'lease_fee',
        },
        {
  +         'credit_msat': 0,
  +         'debit_msat': 11000000,
  +         'tag': 'invoice',
  +     },
  +     {
            'credit_msat': 1314000,
            'debit_msat': 0,
            'tag': 'onchain_fee',
        },
  -     {
  -         'credit_msat': 0,
  -         'debit_msat': 11000000,
  -         'tag': 'invoice',
  -     },
    ]
```

Signed-off-by: Rusty Russell <[email protected]>
Don't test installing a plugin under valgrind.  There's no way to
increase reckless' (completely reasonable) 15 seconds timeout, and that
can happen under valgrind & CI:

```
 def test_reckless_uv_install(node_factory):
        node = get_reckless_node(node_factory)
        node.start()
        r = reckless([f"--network={NETWORK}", "-v", "install", "testpluguv"],
                     dir=node.lightning_dir)
>       assert r.returncode == 0
E       assert 1 == 0
E        +  where 1 = self.returncode, self.stdout, self.stderr.returncode

tests/test_reckless.py:359: AssertionError
...
***RECKLESS STDERR***
config file not found: /tmp/ltests-tui1vmrg/test_reckless_uv_install_1/lightning-1/regtest/config
press [Y] to create one now.
config file not found: /tmp/ltests-tui1vmrg/test_reckless_uv_install_1/lightning-1/reckless/regtest-reckless.conf
config file not found: /tmp/ltests-tui1vmrg/test_reckless_uv_install_1/lightning-1/reckless/.sources
Traceback (most recent call last):
  File "/home/runner/work/lightning/lightning/tools/reckless", line 2091, in <module>
    log.add_result(args.func(target))
  File "/home/runner/work/lightning/lightning/tools/reckless", line 1524, in install
    return _enable_installed(installed, plugin_name)
  File "/home/runner/work/lightning/lightning/tools/reckless", line 1476, in _enable_installed
    if enable(installed.name):
  File "/home/runner/work/lightning/lightning/tools/reckless", line 1647, in enable
    lightning_cli('plugin', 'start', path)
  File "/home/runner/work/lightning/lightning/tools/reckless", line 1613, in lightning_cli
    clncli = run(cmd, stdout=PIPE, stderr=PIPE, check=False, timeout=timeout)
  File "/opt/hostedtoolcache/Python/3.10.18/x64/lib/python3.10/subprocess.py", line 505, in run
    stdout, stderr = process.communicate(input, timeout=timeout)
  File "/opt/hostedtoolcache/Python/3.10.18/x64/lib/python3.10/subprocess.py", line 1154, in communicate
    stdout, stderr = self._communicate(input, endtime, timeout)
  File "/opt/hostedtoolcache/Python/3.10.18/x64/lib/python3.10/subprocess.py", line 2022, in _communicate
    self._check_timeout(endtime, orig_timeout, stdout, stderr)
  File "/opt/hostedtoolcache/Python/3.10.18/x64/lib/python3.10/subprocess.py", line 1198, in _check_timeout
    raise TimeoutExpired(
subprocess.TimeoutExpired: Command '['/home/runner/work/lightning/lightning/cli/lightning-cli', '--network=regtest', '--lightning-dir=/tmp/ltests-tui1vmrg/test_reckless_uv_install_1/lightning-1', 'plugin', 'start', '/tmp/ltests-tui1vmrg/test_reckless_uv_install_1/lightning-1/reckless/testpluguv/testpluguv.py']' timed out after 15 seconds
```

Signed-off-by: Rusty Russell <[email protected]>
DER sigs!  Normally, the commitment weight is:

```
Anchorspend for local commit tx fee 9751sat (w=722), commit_tx fee 4866sat (w=1284): package feerate 7286 perkw
Creating anchor spend for local commit tx 6a0816ca60d499edc70bfb786ebd164fb7a55d234c84d926102f5bd35087fd45: we're paying fee 9751sat
```

But if we're "lucky" the commitment tx is shorter:

```
Anchorspend for local commit tx fee 9744sat (w=722), commit_tx fee 4866sat (w=1283): package feerate 7286 perkw
Creating anchor spend for local commit tx acf78532a9448dd62a4e6319a3d2712189a88b6e59abc637260067d60df70782: we're paying fee 9744sat
```

The resulting failure:

```
2025-08-21T02:30:34.1906751Z >       assert moves == expected
...
...
2025-08-21T02:30:34.1965346Z E               {
2025-08-21T02:30:34.1965529Z E                   'account_id': 'wallet',
2025-08-21T02:30:34.1965767Z E                   'blockheight': 104,
2025-08-21T02:30:34.1965997Z E                   'created_index': 6,
2025-08-21T02:30:34.1966229Z E         -         'credit_msat': 15579000,
2025-08-21T02:30:34.1966467Z E         ?                           ^^
2025-08-21T02:30:34.1966698Z E         +         'credit_msat': 15586000,
2025-08-21T02:30:34.1966927Z E         ?                           ^^
2025-08-21T02:30:34.1967150Z E                   'debit_msat': 0,
2025-08-21T02:30:34.1967376Z E                   'extra_tags': [],
2025-08-21T02:30:34.1967599Z E         -         'output_msat': 15579000,
2025-08-21T02:30:34.1967832Z E         ?                           ^^
2025-08-21T02:30:34.1968061Z E         +         'output_msat': 15586000,
2025-08-21T02:30:34.1968294Z E         ?                           ^^
2025-08-21T02:30:34.1968540Z E                   'primary_tag': 'deposit',
2025-08-21T02:30:34.1968908Z E                   'utxo': 'acf78532a9448dd62a4e6319a3d2712189a88b6e59abc637260067d60df70782:0',
2025-08-21T02:30:34.1969366Z E               },
```

Signed-off-by: Rusty Russell <[email protected]>
1. Establish a channel with l3; we already have one with l2.
2. Don't bother generating 6 more blocks (fundchannel ensures it's mined).
3. Allow htlcs to be empty: Whitslack reported that happens for him
4. Use only_one() to access where we insist there is only one element in the list.
5. Tighten tests to assert the exact contents, not just test some.

Signed-off-by: Rusty Russell <[email protected]>
Fixes: ElementsProject#8497
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type::Bug An error, flaw, or fault that produces an incorrect or unexpected result
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants