[c-lightning] Feature freeze in preparation for 0.6?

[ZmnSCPxj]

Good morning all,

I also did a quick review of the BOLT spec and here are a few items we do not implement on master yet (there may be more that went beyond my understanding).  Might be good to review and consider if we should also consider covering the BOLT spec better for 0.6, or defer better spec coverage for later.

1.  `push_msat`.  We always set this to 0.  My instinct is to ignore this; I do not see anyone asking for it and I do not personally find any use for it, so maybe ignoring this part of the spec is OK.
2.  `option_data_loss_protect`.  Fairly good way to protect against inadvertent recovery of stale DB backups.  However possibly complicated to implement, due to its optionality (we should at least initially not require it of peers, but use it (and provide it) if available).
3.  BOLT11 `r`.  I have a PR for *payment* of `r` fields, but the *receiving* via `r` private routes is not really done well yet.  I will have to do some archeology on our codebase to recover the old "send `channel_update` on lockin" behavior, keep track of fees of unpublished channels, and add a query to gossipd from `lightningd/invoice` to fetch fees if available.
4.  `option_upfront_shutdown_script`.  Seems like a useful way to protect against some kinds of hacks, but probably not enough of them to be all that useful in our case.  Our implementation is so incomplete we seem to be missing a field for them (`shutdown_scriptpubkey`) in `gen_peer_wire_csv`, unlike `option_data_loss_protect` which has all fields present in the `gen_peer_wire_csv`.

Regards,
ZmnSCPxj

​Sent with ProtonMail Secure Email.​

‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐

On April 30, 2018 8:53 PM, Christian Decker <decker.christian at gmail.com> wrote:

> Hi all,
> 
> thanks Rusty for taking the lead on this, we definitely should do a 0.6
> 
> release as soon as possible. I took the opportunity and went through the
> 
> issue list, marking some as stale and closing others that are fixed or
> 
> can't be reproduced with today's code. So here are the things that I
> 
> think need to be part of the feature freeze, on top of what Rusty and
> 
> ZmnSCPxj already mentioned:
> 
> -   I'd like to bump some of our default parameters to at least make them
>     
>     compatible with the default params that lnd and eclair use. The following
>     
>     need to be adjusted:
>     
>     -   Bump the `our_unilateral_close/to_us` maximum value to 2016. This'll hurt a
>         
>         bit since most closes are currently uncooperative and waiting 2 weeks
>         
>         instead of 1 day is bad, but Roasbeef assured me that they will slowly
>         
>         decrease this as the system stabilizes and watch-tower designs are
>         
>         implemented. This would sort of address #1110, in exchange for angry users
>         
>         with locked in funds...
>         
>     -   Bump the default `cltv_delta` to 8, since eclair is enforcing that as a
>         
>         minimum for the last hop currently. They mentioned removing that
>         
>         enforcement on the sender side, but current deployed wallets seem to still
>         
>         have it.
>         
> -   To reduce the number of unilateral closes we should accept a fee update that
>     
>     is outside of our range, but then close the channel collaboratively instead
>     
>     of slamming the door (paying the on-chain fees and having to wait an
>     
>     eternity). I just filed this as #1443.
>     
> -   We sometimes get into a situation in which the remote end doesn't send its
>     
>     `announcement_signature` upon reconnecting. Not sure if this is our fault or
>     
>     an incompatibility with another implementation. I'm tempted to just send it
>     
>     on every reconnect if we have reached `announce_depth`.
>     
> -   #551 seems important but unlikely to cause much trouble at first, if we can
>     
>     fix it good.
>     
> -   We need to change the DEVELOPER=1 to DEVELOPER=0 by default. `dev-xyz` RPC
>     
>     calls and options should only ever be used in a debugging scenario.
>     
> -   Of course all fixes that address a crash are definitely in-scope, but
>     
>     shouldn't be blocking the release if they're "rare"
>     
>     And now for the list of things we should not try to squeeze into the 0.6
>     
>     release:
>     
> -   Fee related fixes are probably out of scope and will need some additional
>     
>     work (#613, #666, #750, #1219, #1233)
>     
> -   Pruning support. This has gotten a lot better since we no longer rescan from
>     
>     the wallet birth, but if the sync horizon of c-lightning is being pruned,
>     
>     e.g., prolonged downtime, it'll still get stuck and we need an alternative
>     
>     source for the pruned blocks (maybe the Bitcoin P2P network?).
>     
>     Most of these are rather small changes, but especially bumping the default
>     
>     params may be controversial, which is why I'm not opening a PR just yet. Of
>     
>     course all of these are up for debate :-)
>     
>     Cheers,
>     
>     Christian
>     
>     --
>     
>     c-lightning mailing list
>     
>     c-lightning at lists.ozlabs.org
>     
>     https://lists.ozlabs.org/listinfo/c-lightning
>