Latest upstream updates from ElementsProject #18
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The `funder` plugin, does the same stuff as this temporary test plugin, so we move over to using that instead.
So we should treat it the same as EXPERIMENTAL_FEATURES
Nicer syntaxtic sugar for marking pytests as 'developer required'
Tests that will only run when !EXPERIMENTAL_DUAL_FUND:
@pytest.marker.openchannel('v1')
def test_...()
Tests that will only run when EXPERIMENTAL_DUAL_FUND:
@pytest.marker.openchannel('v2')
def test_...()
By default, tests only run as v1 unless marked as v2. These tests we want to run as both v1+v2 Includes fixes to have tests pass
Let's run more tests with the v2 open protocol
Found on CI where DEVELOPER=0 EXPERIMENTAL_DUAL_FUND=1, as we turn off automatic reconnects when DEVELOPER=1 This test has been modified to make the error happen every time, and then fixed. lightningd-2: 2021-05-07T20:12:03.790Z DEBUG 0266e4598d1d3c415f572a8488830b60f7e744ed9235eb0b1ba93283b315c03518-chan#1: Peer has reconnected, state AWAITING_UNILATERAL lightningd-2: 2021-05-07T20:12:03.812Z **BROKEN** lightningd: FATAL SIGNAL 6 (version e8b3f78) lightningd-2: 2021-05-07T20:12:03.812Z **BROKEN** lightningd: backtrace: common/daemon.c:44 (send_backtrace) 0x56384ee072e9 lightningd-2: 2021-05-07T20:12:03.813Z **BROKEN** lightningd: backtrace: common/daemon.c:52 (crashdump) 0x56384ee0733b ----------------------------- Captured stderr call ----------------------------- lightningd: lightningd/peer_control.c:1100: peer_connected_hook_final: Assertion `channel->state == DUALOPEND_OPEN_INIT || channel->state == DUALOPEND_AWAITING_LOCKIN' failed. lightningd: FATAL SIGNAL 6 (version e8b3f78) 0x56384ee072a1 send_backtrace common/daemon.c:39 0x56384ee0733b crashdump common/daemon.c:52 0x7f88486a020f ??? ???:0 0x7f88486a018b ??? ???:0 0x7f884867f858 ??? ???:0 0x7f884867f728 ??? ???:0 0x7f8848690f35 ??? ???:0 0x56384eddc94e peer_connected_hook_final lightningd/peer_control.c:1100 0x56384edea2ed plugin_hook_call_ lightningd/plugin_hook.c:275 0x56384eddfeb8 plugin_hook_call_peer_connected lightningd/peer_control.c:1156 0x56384eddfeb8 peer_connected lightningd/peer_control.c:1209 0x56384edc30cd connectd_msg lightningd/connect_control.c:332 0x56384edebe6f sd_msg_read lightningd/subd.c:509 0x56384edebfb1 read_fds lightningd/subd.c:310 0x56384ee483b0 next_plan ccan/ccan/io/io.c:59 0x56384ee4885b do_plan ccan/ccan/io/io.c:407 0x56384ee488f8 io_ready ccan/ccan/io/io.c:417 0x56384ee4a23c io_loop ccan/ccan/io/poll.c:445 0x56384edcabda io_loop_with_timers lightningd/io_loop_with_timers.c:24 0x56384edce826 main lightningd/lightningd.c:1111 0x7f88486810b2 ??? ???:0 0x56384edb52ad ??? ???:0 0xffffffffffffffff ??? ???:0
We were freeing the payload, which is then subsequently freed by the plugin_hook caller. Whoops. Now we pass through to the callback function and just clean up neatly. ------------------------------- Valgrind errors -------------------------------- Valgrind error file: valgrind-errors.406602 ==406602== Invalid read of size 8 ==406602== at 0x12AC93: openchannel2_hook_cb (dual_open_control.c:669) ==406602== by 0x12AF0A: openchannel2_hook_deserialize (dual_open_control.c:721) ==406602== by 0x16EF0E: plugin_hook_callback (plugin_hook.c:186) ==406602== by 0x169746: plugin_response_handle (plugin.c:514) ==406602== by 0x169959: plugin_read_json_one (plugin.c:620) ==406602== by 0x169B23: plugin_read_json (plugin.c:665) ==406602== by 0x1F4076: next_plan (io.c:59) ==406602== by 0x1F4C5B: do_plan (io.c:407) ==406602== by 0x1F4C9D: io_ready (io.c:417) ==406602== by 0x1F6F35: io_loop (poll.c:445) ==406602== by 0x13D48D: io_loop_with_timers (io_loop_with_timers.c:24) ==406602== by 0x143388: main (lightningd.c:1111) ==406602== Address 0x75e7418 is 56 bytes inside a block of size 3,520 free'd ==406602== at 0x483CA3F: free (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so) ==406602== by 0x204FB0: del_tree (tal.c:421) ==406602== by 0x20527E: tal_free (tal.c:486) ==406602== by 0x122D68: delete_channel (channel.c:124) ==406602== by 0x129291: channel_disconnect (dual_open_control.c:63) ==406602== by 0x129364: channel_close_conn (dual_open_control.c:82) ==406602== by 0x131CF6: peer_please_disconnect (connect_control.c:304) ==406602== by 0x131DEB: connectd_msg (connect_control.c:326) ==406602== by 0x172023: sd_msg_read (subd.c:509) ==406602== by 0x1F4076: next_plan (io.c:59) ==406602== by 0x1F4C5B: do_plan (io.c:407) ==406602== by 0x1F4C9D: io_ready (io.c:417) ==406602== Block was alloc'd at ==406602== at 0x483B7F3: malloc (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so) ==406602== by 0x204A39: allocate (tal.c:250) ==406602== by 0x204FFA: tal_alloc_ (tal.c:428) ==406602== by 0x123165: new_unsaved_channel (channel.c:209) ==406602== by 0x130D34: peer_start_dualopend (dual_open_control.c:2985) ==406602== by 0x15BD2A: peer_connected_hook_final (peer_control.c:1105) ==406602== by 0x16F2E5: plugin_hook_call_ (plugin_hook.c:275) ==406602== by 0x15BF5C: plugin_hook_call_peer_connected (peer_control.c:1155) ==406602== by 0x15C16C: peer_connected (peer_control.c:1208) ==406602== by 0x131E3B: connectd_msg (connect_control.c:332) ==406602== by 0x172023: sd_msg_read (subd.c:509) ==406602== by 0x171842: read_fds (subd.c:310)
If dualopend dies, we shouldn't reference it
hangs with EXP_DF when developer=0
Trying to put all the disconnect logic into the same path was a dumb idea. If you asked to reconnect but passed in an 'unsaved' channel, we would not call the 'reconnect' code. Instead, we make a differentiation between "unsaved" channels (ones that we haven't received commitment tx for) and handle the disconnect for these separate from where we want to do a reconnect.
No idea how this slipped past the first time
Peer sends funding locked, we tell lightningd who saves it to disk. Then we restart/reconnect and they retransmit funding_locked. We were re-notifying lightningd about their lock-in, which was crashing/breaking things. Instead, we ignore duplicate lock-in messages from the peer. lightningd-1: 2021-05-11T18:00:12.844Z **BROKEN** 022d223620a359a47ff7f7ac447c85c46c923da53389221a0054c11c1e3ca31d59-chan#1: Internal error DUALOPEND_AWAITING_LOCKIN: channel_got_funding_locked twice
This would flake fairly regularly, what we really care about is asserting that the l2 node is in CHANNELD_NORMAL state, while the l1 node hasn't progressed that far yet.
1. We don't need to check for NULL before tal_count(NULL). 2. Use of json_for_each_arr iterator is probably better. 3. Weird indent fixed. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Added: libhsmd: Added python bindings for `libhsmd`
With swig we now have C files that are generated with tools that are not under our control, so provide an escape hatch for them.
If l2 didn't get FUNDING_LOCKED from l1 before it disconnected, it won't be in state CHANNELD_NORMAL: it will be in DUALOPEND_AWAITING_LOCKIN. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This suppresses some "may-be-uninitialized" warnings later. It makes gcc pickier about how we ignore the result though :( Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Temporarily rename old getroute to getrouteold (we will remove this). Changelog-Changed: JSON-RPC: `getroute` is now implemented in a plugin. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
It sometimes fails because the two race, and sometimes because there's randomness, but it generally works (and doesn't fail systemically). We remove this before the final merge. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
…active=false. This is blurring the lines a bit, but it's closer to what gossipd did. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
It sometimes fails because the two race, and sometimes because there's randomness, but it generally works (and doesn't fail systemically). We remove this before the final merge. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This was previously internal to gossipd and was called "read_addresses". Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
It sometimes fails because the two race, and sometimes because there's randomness, but it generally works (and doesn't fail systemically). We remove this before the final merge. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Invoices need to know what the peers' update contains, to create routehints. It also wants to know if a peer has no other public channels (so-called "dead end" peers) to eliminate them from routehint consideration. This was previously done by a special function to ask gossipd. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This turned into a more extensive cleanup than intended. The previous warnings were overlapping and confusing, especially now MPP is the norm. *warning_capacity* is now the "even under best circumstances, we don't have enough incoming capacity", which is really what warning_mpp_capacity was trying to say (no longer printed). *warning_offline* and *warning_deadends* are only given if adding such peers would have helped give capacity (i.e. not if *warning_capacity* is set). The new *warning_private_unused* tells you that we would have sufficient capacity, but we refused to expose private channels. The test cases have been enhanced to cover the new warnings. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au> Changelog-Added: JSON-RPC: `invoice` now gives `warning_private_unused` if unused unannounced channels could have provided sufficient capacity. Changelog-Changed: JSON-RPC: `invoice` warnings are now better defined, and `warning_mpp_capacity` is no longer included (since `warning_capacity` covers that).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
…istnodes. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We're about to remove routing from gossipd. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This involves removing some fields from the now-misnamed routing.h datastructures, and various internal messages. One non-obvious change is to our "keepalive" logic which refreshes channels every 13 days: instead of using the 'enabled' flag on the last channel broadcast to decide whether to refresh it, we use the local connected status directly. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
…ugins. This mainly helps our CI under valgrind, which starts a fresh instance and immediately calls the invoice command. This can cause the topology plugin to try to access the gossmap file before it's created. We can also move the gossmap reading in topology to init time. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
In case this was the issue (it wasn't)!. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We were accidentally using the port that the tor service was connecting to, not the /torport the user said to use. Fixes: ElementsProject#4597 Reported-by: @openoms Signed-off-by: Rusty Russell <rusty@rustcorp.com.au> Changelog-Fixed: Config: `addr` autotor and statictor /torport arguments now advertized correctly.
We didn't set this to false on non-regtest! ``` ==198363== Conditional jump or move depends on uninitialised value(s) ==198363== at 0x10EF88: estimatefees_parse_feerate (bcli.c:443) ==198363== by 0x10F3BF: estimatefees_second_step (bcli.c:550) ==198363== by 0x10E720: bcli_finished (bcli.c:258) ==198363== by 0x1438A7: destroy_conn (poll.c:244) ==198363== by 0x1438CB: destroy_conn_close_fd (poll.c:250) ==198363== by 0x151A7A: notify (tal.c:240) ==198363== by 0x151F91: del_tree (tal.c:402) ==198363== by 0x15232D: tal_free (tal.c:486) ==198363== by 0x141EB8: io_close (io.c:450) ==198363== by 0x14400B: io_loop (poll.c:449) ==198363== by 0x114BB0: plugin_main (libplugin.c:1414) ==198363== by 0x1105C4: main (bcli.c:973) ``` Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-None
I tried to wait_for_log() on "successfully" without success :/ Signed-off-by: Antoine Poinsot <darosior@protonmail.com>
Test that it roundtrips with the non-stdin way, in order to make sure we don't introduce a discrepancy between the two. Signed-off-by: Antoine Poinsot <darosior@protonmail.com>
Avoid duplicating it, to minimize the potential for divergence. Signed-off-by: Antoine Poinsot <darosior@protonmail.com>
We would fail even if a process exited cleanly after having the line we were looking for, because we checked if it died before reading the logs. Co-Authored-by: Daniela Brozzoni <daniela@revault.dev> Signed-off-by: Antoine Poinsot <darosior@protonmail.com>
lightning/bolts#877 talks about removing this restriction (only Electrum actually enforced it on receive), so start by allowing creation of giant invoices, though we mark them as requiring mpp. Changelog-Changed: JSON-RPC: `invoice` now allows creation of giant invoices (>= 2^32 msat) Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-None
upstream updates
upstream updates
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Had some conflicts with the
estimatefeecode, since it's logic is bit changed in updates. Resolved conflict, updated code logic to reflect the same expected output with this updated code and it works now.