Improve UX of experimental options/features and other small fixes #5047
Conversation
SimonVrouwe
force-pushed
the
2022-02-20_TODOs_ready
branch
from
5ccb568
to
b70b842
Compare
|
That's odd, Which are both there: The CI test failure:
Which is here in lightning/contrib/pyln-testing/pyln/testing/utils.py Lines 301 to 302 in d0c7e18 These lines are at the end of the log. Maybe process died quicker then its (log) tail ? |
|
Maybe I got lucky, but the Will push another fixup for another test round, if that also passes will squash. |
This fix doesn't do anything. If there is more in the logs, we don't check if it's dead; only once we've exhausted the logs do we see if it's dead. The real problem is that wait_for_log only reads forward. Here's what happens when I run locally (with valgrind, it's too fast to trigger otherwise): Now, we Switching to a single wait_for_logs() on all three fixes this: diff --git a/tests/test_plugin.py b/tests/test_plugin.py
index 5963a437f..ad69ead61 100644
--- a/tests/test_plugin.py
+++ b/tests/test_plugin.py
@@ -2513,6 +2513,6 @@ def test_plugin_shutdown(node_factory):
# Now, should also shutdown or timeout on finish, RPC calls then fail with error code -5
l1.rpc.plugin_start(p, dont_shutdown=True)
l1.rpc.stop()
- l1.daemon.wait_for_log(r'test_libplugin: shutdown called')
- l1.daemon.wait_for_logs(['misc_notifications.py: via lightningd shutdown, datastore failed',
+ l1.daemon.wait_for_logs(['test_libplugin: shutdown called',
+ 'misc_notifications.py: via lightningd shutdown, datastore failed',
'test_libplugin: failed to self-terminate in time, killing.']) |
SimonVrouwe
force-pushed
the
2022-02-20_TODOs_ready
branch
from
414de25
to
a0c0499
Compare
|
|
Trying to debug the failing test: lightning/tests/test_connection.py Lines 3697 to 3706 in 86b83e4 this is in the log where instead of l1, l2 logs the awaited-for line. But now I think it doesn't matter which one logs that line, see explanation below. I can reproduce locally after few runs with valgrind=1 lightning/connectd/multiplex.c Lines 354 to 366 in 86b83e4 then run the test with What is happening in above (fail) case: Solution is to check for desired log-line in both l1 and l2. |
|
Just a last point with respect to would become: I think the latter is more readible and would be more compact when bits jump higher. as comma-separated string: However it would not be consistent with "features" field returned by connect, listchannels and listnodes, these all return hex-string. |
|
Nice, I'm folding fixups now... |
Let's go better and add "featuremap"? {0: "option_data_loss_protect/even", 1023: "option_unknown_odd_1022/odd" etc...?) Separate patch though IMHO |
…ent with tlvs Terminal prompt got messed up because missing newline in case of empty fields. printwire_addresses expected it to be the last field, which is not the case of a node_announcement with tlv
All build flags and (experimental) options make it hard to find out what features are supported or enabled. And the undocumented `--list-features-only`, does not account for all our featurebits, for example bit 55 (keysend). Changelog-Added: JSON-RPC: `getinfo` result now includes `our_features` (bits) for various Bolt ElementsProject#9 contexts
It did not pick-up the last (bolt11) bits set.
…` field returned by listnodes, listchannels and connect.
…z-percent Changelog-Experimental: option `--lease-fee-base-msat` renamed to `--lease-fee-base-sat` Changelog-Experimental: option `--lease-fee-base-msat` deprecated and will be removed next release
…o works with dualfund and some cleanup, fundchannel has been returning tx, txid and outnum for years
…re reliable it now waits for 'alias' in node_announcement, not just block confirms. more cleanup
[ Folded 2 fixups --RR ]
rustyrussell
force-pushed
the
2022-02-20_TODOs_ready
branch
from
4895631
to
3dd54c9
Compare
|
Ack 3dd54c9 |
So I was trying to connect the dots to understand dual-fund and leases and then give it a shot.
Did my first dual-fund on mainnet a couple of days ago and then a lease, it all worked out, very cool.
I needed to run custom pytest's, looked up Bolt pull-requests, to get some confidence in the meaning of various options and parameters. And came across a few issues during my journey.
For which I propose fixes in this PR:
configurationhas these also and a plugin can be used to show them also