Skip to content

Make all peer messages constant message size#8893

Merged
rustyrussell merged 13 commits intoElementsProject:masterfrom
rustyrussell:guilt/constant-message-size
Feb 18, 2026
Merged

Make all peer messages constant message size#8893
rustyrussell merged 13 commits intoElementsProject:masterfrom
rustyrussell:guilt/constant-message-size

Conversation

@rustyrussell
Copy link
Contributor

Not sufficient, but definitely necessary for avoiding trivial observation.

@rustyrussell rustyrussell added this to the v26.04 milestone Feb 10, 2026
@rustyrussell rustyrussell force-pushed the guilt/constant-message-size branch 12 times, most recently from 5132083 to efbcb8f Compare February 15, 2026 23:33
l3 doesn't just need to know about l2 (which it can get from the
channel_announcement), but needs to see the node_announcement.

Otherwise:

```
        l1, l2 = node_factory.line_graph(2, wait_for_announce=True,
                                         # No onion_message support in l1
                                         opts=[{'dev-force-features': -39},
                                               {'dev-allow-localhost': None}])
    
        l3 = node_factory.get_node()
        l3.rpc.connect(l1.info['id'], 'localhost', l1.port)
        wait_for(lambda: l3.rpc.listnodes(l2.info['id'])['nodes'] != [])
    
        offer = l2.rpc.call('offer', {'amount': '2msat',
                                      'description': 'simple test'})
>       l3.rpc.call('fetchinvoice', {'offer': offer['bolt12']})

tests/test_pay.py:4804: 
...	
>           raise RpcError(method, payload, resp['error'])
E           pyln.client.lightning.RpcError: RPC call failed: method: fetchinvoice, payload: {'offer': 'lno1qgsqvgnwgcg35z6ee2h3yczraddm72xrfua9uve2rlrm9deu7xyfzrcgqypq5zmnd9khqmr9yp6x2um5zcssxwz9sqkjtd8qwnx06lxckvu6g8w8t0ue0zsrfqqygj636s4sw7v6'}, error: {'code': 1003, 'message': 'Failed: could not route or connect directly to 033845802d25b4e074ccfd7cd8b339a41dc75bf9978a034800444b51d42b07799a: {"code":400,"message":"Unable to connect, no address known for peer"}'}
```

Signed-off-by: Rusty Russell <[email protected]>
Commit 888745b (dev_disconnect:
remove @ marker.) in v0.11 in April 2022) removed the '@' marker from
our dev_disconnect code, but one test still uses it.

Refactoring this code made it crash on invalid input.  The test
triggered a db issue which has been long fixed, so I'm simply removing
it.

Signed-off-by: Rusty Russell <[email protected]>
This requires access to dumpcap.  On Ubuntu, at least, this means you
need to be in the "wireshark" group.

We may also need:
	sudo ethtool -K lo gro off gso off tso off

Signed-off-by: Rusty Russell <[email protected]>
Give us a single "next message" function to call.  This will be useful
when we want to write more than one at a time.

Signed-off-by: Rusty Russell <[email protected]>
Do all the special treatment of the message type first.

Signed-off-by: Rusty Russell <[email protected]>
This gives us finer control over write sizes: for now we just cap
the write size.

Signed-off-by: Rusty Russell <[email protected]>
We're doing our own buffering now.

We leave the is_urgent() function for two commits in the future though.

Signed-off-by: Rusty Russell <[email protected]>
… count.

We are about to use them to make our packet size constant, and this
will upset the tests.

Signed-off-by: Rusty Russell <[email protected]>
Messages are now constant.

Signed-off-by: Rusty Russell <[email protected]>
Changelog-Added: Protocol: we now pad all peer messages to make them the same length.
@rustyrussell rustyrussell force-pushed the guilt/constant-message-size branch from efbcb8f to ab251ba Compare February 16, 2026 01:01
This replaces our previous nagle-based toggling.

Signed-off-by: Rusty Russell <[email protected]>
…ges.

Since we delay the others quite a lot (up to 1 second), it's better to consider
most messages "urgent" and worth immediately transmitting.

Signed-off-by: Rusty Russell <[email protected]>
This is exactly what membuf is for: it handles expansion much more
neatly.

Signed-off-by: Rusty Russell <[email protected]>
@rustyrussell rustyrussell force-pushed the guilt/constant-message-size branch from ab251ba to d20c18a Compare February 16, 2026 03:36
@Lagrang3
Copy link
Collaborator

The test added on commit "pytest: add fixture for checking packet sizes." won't fail as expected.
I think it is because assert_constant_payload() is only called after the test is completed during cleanup phase.

@rustyrussell
Copy link
Contributor Author

The test added on commit "pytest: add fixture for checking packet sizes." won't fail as expected. I think it is because assert_constant_payload() is only called after the test is completed during cleanup phase.

No, it definitely works (I hit it during debugging!) and I reverted and took off xfail:

tests/test_connection.py::test_constant_packet_size PASSED               [100%]
tests/test_connection.py::test_constant_packet_size ERROR                [100%]

==================================== ERRORS ====================================
________________ ERROR at teardown of test_constant_packet_size ________________

have_pcap_tools = None
tmp_path = PosixPath('/tmp/pytest-of-rusty/pytest-0/test_constant_packet_size0')

    @pytest.fixture
    def tcp_capture(have_pcap_tools, tmp_path):
        # You will need permissions.  Most distributions have a group which has
        # permissions to use dumpcap:
        #     $ ls -l /usr/bin/dumpcap
        #     -rwxr-xr-- 1 root wireshark 229112 Apr 16  2024 /usr/bin/dumpcap
        #     $ getcap /usr/bin/dumpcap
        #     /usr/bin/dumpcap cap_net_admin,cap_net_raw=eip
        # So you just need to be in the wireshark group.
        cap = TcpCapture(tmp_path)
        yield cap
        cap.stop()
>       cap.assert_constant_payload()

tests/fixtures.py:193: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

self = <fixtures.TcpCapture object at 0x731d573af230>

    def assert_constant_payload(self):
        tshark_cmd = [
            "tshark",
            "-r", str(self.pcap),
            "-Y", "tcp.len > 0",
            "-T", "fields",
            "-e", "tcp.len",
        ]
    
        out = subprocess.check_output(tshark_cmd, text=True)
        lengths = [int(x) for x in out.splitlines() if x.strip()]
    
        assert lengths, f"No TCP payload packets captured on port {self.port}"
    
        uniq = set(lengths)
>       assert len(uniq) == 1, (
               ^^^^^^^^^^^^^^

@rustyrussell rustyrussell merged commit 963b353 into ElementsProject:master Feb 18, 2026
119 of 127 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants