Anchors not experimental

[rustyrussell]

Based on #6780 Rebased on master.

[vincenzopalazzo]

I did not look much inside the test, but looks like that this change cause a force close

lightningd-2 2024-01-23T07:26:48.120Z DEBUG   0266e4598d1d3c415f572a8488830b60f7e744ed9235eb0b1ba93283b315c03518-connectd: Subd did not close, forcing close
lightningd-2 2024-01-23T07:26:48.121Z **BROKEN** 0266e4598d1d3c415f572a8488830b60f7e744ed9235eb0b1ba93283b315c03518-connectd: Peer did not close, forcing close
lightningd-1 2024-01-23T07:26:48.121Z DEBUG   connectd: drain_peer

[ddustin]

Looks good to me but I did have a question. On normal commits we send the anchor info for each commitment to lightningd: https://github.com/rustyrussell/lightning/blob/df42490d2100a971eba07d5c5d7bcf61693ab260/channeld/channeld.c#L1768

On reestablish however, we discard the anchor info: https://github.com/rustyrussell/lightning/blob/df42490d2100a971eba07d5c5d7bcf61693ab260/channeld/channeld.c#L4734

We also discard the anchor info during splice: https://github.com/rustyrussell/lightning/blob/df42490d2100a971eba07d5c5d7bcf61693ab260/channeld/channeld.c#L2957

Is the anchor info meant to be discarded in those situations?

[cdecker]

Rebased on top of master. There were a couple of conflicts around the line_graph calls gaining more options. I will fix these up if I broke them, so this should make it through soon.

[rustyrussell]

Looks good to me but I did have a question. On normal commits we send the anchor info for each commitment to lightningd: https://github.com/rustyrussell/lightning/blob/df42490d2100a971eba07d5c5d7bcf61693ab260/channeld/channeld.c#L1768

On reestablish however, we discard the anchor info: https://github.com/rustyrussell/lightning/blob/df42490d2100a971eba07d5c5d7bcf61693ab260/channeld/channeld.c#L4734

Yes, this is a retransmission, so we already have given it.

We also discard the anchor info during splice: https://github.com/rustyrussell/lightning/blob/df42490d2100a971eba07d5c5d7bcf61693ab260/channeld/channeld.c#L2957

Is the anchor info meant to be discarded in those situations?

Hmm, this is a good question! We need to add a new anchor_info at this point. I'll create a test and fix...

[cdecker]

Rebased on top of master. This will get merged as soon as it passes CI.

[rustyrussell]

Rebase did some accidental reversions, fixed, and removed a flake...

[cdecker]

Rebase did some accidental reversions, fixed, and removed a flake...

Sorry about that, restarting CI to remove one more flake.

[cdecker]

We still have some test failures that I'm not exactly sure are flakes. @rustyrussell can you confirm that these are flakes?

[gw1] linux -- Python 3.8.18 /home/runner/.cache/pypoetry/virtualenvs/cln-meta-project-AqJ9wMix-py3.8/bin/python

node_factory = <pyln.testing.utils.NodeFactory object at 0x7f7357896e20>
bitcoind = <pyln.testing.utils.ElementsD object at 0x7f7357896940>
executor = <concurrent.futures.thread.ThreadPoolExecutor object at 0x7f7357896f70>

    def test_htlc_sig_persistence(node_factory, bitcoind, executor):
        """Interrupt a payment between two peers, then fail and recover funds using the HTLC sig.
        """
        # Feerates identical so we don't get gratuitous commit to update them
        l1 = node_factory.get_node(options={'dev-no-reconnect': None},
                                   feerates=(7500, 7500, 7500, 7500))
        l2 = node_factory.get_node(disconnect=['+WIRE_COMMITMENT_SIGNED'])
    
        l1.rpc.connect(l2.info['id'], 'localhost', l2.port)
        l1.fundchannel(l2, 10**6)
        f = executor.submit(l1.pay, l2, 31337000)
        l1.daemon.wait_for_log(r'HTLC out 0 RCVD_ADD_ACK_COMMIT->SENT_ADD_ACK_REVOCATION')
        l1.stop()
    
        # `pay` call is lost
        with pytest.raises(RpcError):
            f.result()
    
        # We should have the HTLC sig
        assert(len(l1.db_query("SELECT * FROM htlc_sigs;")) == 1)
    
        # This should reload the htlc_sig
        l2.rpc.dev_fail(l1.info['id'])
        # Make sure it broadcasts to chain.
        l2.wait_for_channel_onchain(l1.info['id'])
        l2.stop()
        bitcoind.generate_block(1)
        l1.start()
    
        assert l1.daemon.is_in_log(r'Loaded 1 HTLC signatures from DB')
    
        # Could happen in either order!
        l1.daemon.wait_for_log(r'Peer permanent failure in CHANNELD_NORMAL: Funding transaction spent')
    
        _, txid, blocks = l1.wait_for_onchaind_tx('OUR_HTLC_TIMEOUT_TO_US',
                                                  'THEIR_UNILATERAL/OUR_HTLC')
        assert blocks == 5
    
        bitcoind.generate_block(5)
        bitcoind.generate_block(1, wait_for_mempool=txid)
        l1.daemon.wait_for_logs([
            r'Owning output . (\d+)sat .SEGWIT. txid',
        ])
    
        # We should now have 1) the unilateral to us, and b) the HTLC respend to us
>       assert len(l1.rpc.listfunds()['outputs']) == 2
E       AssertionError: assert 3 == 2
E        +  where 3 = len([{'address': 'ert1qctx2k9cu9fd7nk449mzphqjcvvpyc4rx0wj5yx', 'amount_msat': 991908000, 'blockheight': 103, 'output': 1, ...}, {'address': 'ert1qguppdpqjn792rs94h97un9s87hcg2z653qf6tk35dldzy5gjs36s344afy', 'amount_msat': 951598000, 'blockheight': 104, 'output': 3, ...}, {'address': 'ert1qquwynjkj7ss08jq9l8mtnzjhy6wtzs2szrarsv', 'amount_msat': 23642000, 'blockheight': 110, 'output': 0, ...}])

tests/test_misc.py:306: AssertionError

(I will collect more as I find them running the CI).

[rustyrussell]

OK, I've not enabled anchors by default for elements. It needs more work for that, and it's unnecessary at the moment.

Tests have been re-reworked to allow either...