We determine whether they are allowed or not based on the hook return
value of `mindepth`. To do so we need to pass that value down to
`openingd` and verify that the `channel_type` and our permissions
match up.
There's a fascinating bug report which suggests this happens on local channels,
implying spendable_msat is wrong?
See-also: #6567
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
1. announce-addr-discovered-port takes a port option.
2. accept-htlc-tlv-types was deprecated in favor of multiple accept-htlc-tlv-type.
3. Document clnrest.py options.
4. Don't list --version twice in lightningd --help (initial_config_opts calls
opt_register_version() already).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
It didn't handle the case where an undocumented option was
was a flag (i.e. didn't end in =), so rework it to be
a simple list and use grep.
Add some more options we don't document, too.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Our formats changed, so this didn't work any more!
Add a sanity check that in future if we get no options from a command,
we complain.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Doesn't happen on x86, but struct gossmap_chan defines:
```
u32 private: 1;
u32 plus_scid_off: 31;
```
And complains when we initialize plus_scid_off and access it later:
```
VALGRIND=1 valgrind -q --error-exitcode=7 --track-origins=yes --leak-check=full --show-reachable=yes --errors-for-leak-kinds=all plugins/renepay/test/run-mcf > /dev/null
==186886== Conditional jump or move depends on uninitialised value(s)
==186886== at 0x10076388: chan_iter (gossmap.c:1098)
==186886== by 0x100797F3: gossmap_next_chan (gossmap.c:1112)
==186886== by 0x1008C5AF: main (run-mcf.c:309)
==186886== Uninitialised value was created by a heap allocation
==186886== at 0x40F0A44: malloc (vg_replace_malloc.c:431)
==186886== by 0x10072BAF: allocate (tal.c:256)
==186886== by 0x100737A7: tal_alloc_ (tal.c:463)
==186886== by 0x100738DF: tal_alloc_arr_ (tal.c:506)
==186886== by 0x10079507: load_gossip_store (gossmap.c:690)
==186886== by 0x10079667: gossmap_load (gossmap.c:978)
==186886== by 0x1008C4AF: main (run-mcf.c:295)
```
Reported-by: @grubles
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Fixes: #6557
As side-effect, getroute(0) is special too.
Reported-by: MiddleW4y in Discord
Fixes: #6577
Changelog-Fixed: `pay` will still use an invoice routehint if path to it doesn't take 1-msat payments.
Reported-by: @niftynei
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Fixed: Plugins: we clean up properly if a plugin fails to start, and we don't kill all processes if it's from `plugin startdir`.
Don’t send the funding spend to onchaind if we detect it in inflights (aka. a splice). While we already prevented onchaind_funding_spent from being called directly, the call to wallet_channeltxs_add meant onchaind_funding_spent would be called *anyway* on restart. This is now fixed.
Additionally there was a potential for a race problem depending on the firing order of the channel depth and and funding spent events.
Instead of requiring these events fire in a specific order, we make a special “memory only” inflight object to prevent the race regardless of firing order.
Changelog-Fixed: Splice: bugfix for restart related race condition interacting with adversarial close detection.
This should provide the default help message and exit, but was
resulting in a segmentation fault from freeing pointers passed to
the default config.
Changelog-Fixed: lightning-cli properly returns help without argument
We have a report that LND said our (unannounced) channel was disabled, so we didn't
use it for routehints. We're better off ignoring that in this case (if the peer is
actually not connected, the routehint code will check that and ignore anyway).
Fixes: #6555
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Changed: pay: use channels in routehints even if peer says they're "disabled" (LND compat)
This is actually a valid complaint (though this is a sanity check for
things we make ourselves, still!).
```
In file included from common/test/run-blindedpath_onion.c:9:
common/test/../sphinx.c: In function ‘sphinx_add_hop_has_length’:
common/test/../sphinx.c:117:12: error: ‘prepended_len’ may be used uninitialized [-Werror=maybe-uninitialized]
117 | if (lenlen + prepended_len != tal_bytelen(payload))
| ^
common/test/../sphinx.c:109:27: note: ‘prepended_len’ was declared here
109 | bigsize_t lenlen, prepended_len;
| ^~~~~~~~~~~~~
cc1: all warnings being treated as errors
```
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Compiler can't tell that we always set have_state[PAY_FLOW_FAILED_FINAL]
when we set this:
```
plugins/renepay/payment.c: In function ‘payment_reconsider’:
plugins/renepay/payment.c:287:25: error: ‘final_error’ may be used uninitialized [-Werror=maybe-uninitialized]
287 | payment_fail(payment, final_error, "%s", final_msg);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
plugins/renepay/payment.c:194:30: note: ‘final_error’ was declared here
194 | enum jsonrpc_errcode final_error, ecode;
| ^~~~~~~~~~~
plugins/renepay/payment.c:287:25: error: ‘final_msg’ may be used uninitialized [-Werror=maybe-uninitialized]
287 | payment_fail(payment, final_error, "%s", final_msg);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
plugins/renepay/payment.c:195:21: note: ‘final_msg’ was declared here
195 | const char *final_msg;
| ^~~~~~~~~
```
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Indeed, we can fall through this if it's not a valid enum value.
gcc-12 (Ubuntu 12.2.0-17ubuntu1) 12.2.0
```
In file included from plugins/commando.c:10:
ccan/ccan/tal/str/str.h: In function ‘rune_altern_to_english’:
ccan/ccan/tal/str/str.h:43:9: error: ‘cond_str’ may be used uninitialized [-Werror=maybe-uninitialized]
43 | tal_fmt_(ctx, TAL_LABEL(char, "[]"), __VA_ARGS__)
| ^~~~~~~~
plugins/commando.c:97:21: note: ‘cond_str’ was declared here
97 | const char *cond_str;
| ^~~~~~~~
cc1: all warnings being treated as errors
```
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
```
In function ‘peer_reconnect’,
inlined from ‘init_channel’ at channeld/channeld.c:5890:3,
inlined from ‘main’ at channeld/channeld.c:5951:2:
channeld/channeld.c:5028:21: error: ‘next_matches_inflight’ may be used uninitialized [-Werror=maybe-uninitialized]
5027 | if (remote_next_funding && !next_matches_current
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
5028 | && !next_matches_inflight) {
| ^~~~~~~~~~~~~~~~~~~~~~~~~
channeld/channeld.c: In function ‘main’:
channeld/channeld.c:4595:36: note: ‘next_matches_inflight’ was declared here
4595 | bool next_matches_current, next_matches_inflight;
| ^~~~~~~~~~~~~~~~~~~~~
channeld/channeld.c:5042:57: error: ‘inflight’ may be used uninitialized [-Werror=maybe-uninitialized]
5042 | &inflight->outpoint.txid),
| ^
channeld/channeld.c:4594:26: note: ‘inflight’ was declared here
4594 | struct inflight *inflight;
| ^~~~~~~~
cc1: all warnings being treated as errors
make: *** [Makefile:300: channeld/channeld.o] Error 1
```
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We cannot carry pointers into the gossmap across localmod addition
or removal.
We didn't notice because the map->chan_arr is not normally resized,
but if we change gossmap.c line 689 to only allocate 1 to start, we see this:
```
VALGRIND=1 valgrind -q --error-exitcode=7 --track-origins=yes --leak-check=full --show-reachable=yes --errors-for-leak-kinds=all plugins/renepay/test/run-mcf > /dev/null
==2349744== Invalid read of size 4
==2349744== at 0x1788C2: gossmap_chan_scid (gossmap.c:558)
==2349744== by 0x1872A2: get_chan_extra_half_by_chan (flow.c:346)
==2349744== by 0x187797: remove_completed_flow (flow.c:488)
==2349744== by 0x187927: remove_completed_flow_set (flow.c:518)
==2349744== by 0x18DF4D: main (run-mcf.c:393)
==2349744== Address 0x4b80f38 is 88 bytes inside a block of size 136 free'd
==2349744== at 0x4848C63: realloc (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==2349744== by 0x173D71: tal_resize_ (tal.c:744)
==2349744== by 0x177E36: next_free_chan (gossmap.c:336)
==2349744== by 0x177ED3: new_channel (gossmap.c:351)
==2349744== by 0x178441: add_channel (gossmap.c:458)
==2349744== by 0x1798D4: gossmap_apply_localmods (gossmap.c:904)
==2349744== by 0x18DEDB: main (run-mcf.c:388)
==2349744== Block was alloc'd at
==2349744== at 0x4848C63: realloc (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==2349744== by 0x173D71: tal_resize_ (tal.c:744)
==2349744== by 0x177E36: next_free_chan (gossmap.c:336)
==2349744== by 0x177ED3: new_channel (gossmap.c:351)
==2349744== by 0x178441: add_channel (gossmap.c:458)
==2349744== by 0x178B6D: map_catchup (gossmap.c:635)
==2349744== by 0x178F45: load_gossip_store (gossmap.c:697)
==2349744== by 0x179D71: gossmap_load (gossmap.c:978)
==2349744== by 0x18D22F: main (run-mcf.c:295)
```
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
It's not there if it's a local error:
```
{
"code": 202,
"message": "Parsing '{message:%,data:{erring_index:%,failcode:%,raw_message:': object does not have member raw_message"
}
```
Reported-by: https://github.com/daywalker90Fixes: #6553
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
In spec commit 498f104fd399488c77f449d05cb21c0b604636a2 (August 2021),
Bastien Teinturier removed the requirement that the mutual close fee be
less than or equal the final commitment tx.
We adopted that change in v0.10.2, but we made sure to never offer a fee
under the final commitment tx's fee, so we didn't break older nodes.
However, the closing tx can actually be larger than the final commitment tx!
The final commit tx has a 22-byte P2WKH output and a 34-byte P2WSH output;
the closing can have two 34-byte outputs, making it 4*8 = 32 Sipa heavier.
Previously this would only happen if both sides asked for P2WSH outputs,
but now it happens with P2TR, which we now do.
The result is that we create a tx which is below the finally commitment
tx fee, and may be below minrelayfee (as it was in regtest).
So it's time to remove that backwards-compatibility hack.
Changelog-Fixed: Protocol: We may propose mutual close transaction which has a slightly higher fee than the final commitment tx (depending on the outputs, e.g. two taproot outputs).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Fixes: #6545
Unifies the pay_flow resolve functions, and moves remove_htlc_payflow
and commit_htlc_payflow to the top.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We want to make sure that on every path, we terminate the flow. The simplest
way to do this is encourage the pattern "return pay_flow_xxx(flow)".
Indeed, this caught a few places I missed!
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
