Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

0.8.0 #19

Merged
merged 307 commits into from
Dec 17, 2019
Merged

0.8.0 #19

merged 307 commits into from
Dec 17, 2019

Conversation

gruve-p
Copy link
Member

@gruve-p gruve-p commented Dec 17, 2019

No description provided.

cdecker and others added 30 commits November 12, 2019 21:23
`DEVELOPER=1` assumes that the binary has been compiled with developer set to
true, which might not be the case for plugin developers. Setting this to 0 by
default has no effect in c-lightning since we always at least set it in
`config.vars` but may prevent some issues outside.
We were relying heavily on NodeFactory to do some magic before instantiating
the Node with rpc and daemon initialized, that meant that we'd have to replace
all 3 classes when customizing the node to our needs. Moving that
initialization into the node itself means that the LightningNode class now can
be swapped out and customized, without having to wire everything else through.
Quite a few of the things in the LightningNode class are tailored to their use
in the c-lightning tests, so I decided to split those customizations out into
a sub-class, and adding one more fixture that just serves the class. This
allows us to override the LightningNode implementation in our own tests, while
still having sane defaults for other users.
Needs to initialize global now.

Signed-off-by: Rusty Russell <[email protected]>
Previously, returned null if a scriptpubkey was not Segwit; now
handles encoding to Base58 for other types.
If a 'upfront_shutdown_script' was specified, show the address +
scriptpubky in `listpeers`

Changelog-added: JSON API: `listpeers` channels now include `close_to` and `close_to_addr` iff a `close_to` address was specified at channel open
--dev-force-tmp-channel-id flag takes a 64-character hex string
to use as the temporary channel id. Useful for spec tests

[ Fixed crash in non-DEVELOPER mode --RR ]
Changelog-None
Travis adds the correct PYTHONPATH, but "make check" doesn't.

Signed-off-by: Rusty Russell <[email protected]>
This highlights the various places we need to change.

Signed-off-by: Rusty Russell <[email protected]>
We add routines to decode the expected fields from both legacy and tlv
hop formats.

Signed-off-by: Rusty Russell <[email protected]>
Signed-off-by: Rusty Russell <[email protected]>
Changelog-changed: JSON API: `htlc_accepted` hook has `type` (currently `legacy` or `tlv`) and other fields directly inside `onion`.
Changelog-deprecated: JSON API: `htlc_accepted` hook `per_hop_v0` object deprecated, as is `short_channel_id` for the final hop.
We keep the feature bitmap on disk, so we cache this in the struct
explicitly.

Signed-off-by: Rusty Russell <[email protected]>
Default is legacy.  If we have future styles, new strings can be defined,
but for now it's "tlv" or "legacy".

Signed-off-by: Rusty Russell <[email protected]>
For legacy, they were the same, but for TLV we care whether it's the
final hop or not.

Signed-off-by: Rusty Russell <[email protected]>
This means we can make sphinx_add_v0_hop static, too.

Signed-off-by: Rusty Russell <[email protected]>
This is required for the protocol tests, which can be slow.

Signed-off-by: Rusty Russell <[email protected]>
It really, really doesn't matter.  But we were dramatically reducing
our view of the network:

In my gossip_store (mainnet):
  channel_announcement: 30349
  channel_update: 55119
  node_announcment: 1783

Changelog-Fixed: No longer discard most node_announcements (fixes ElementsProject#3194)
Signed-off-by: Rusty Russell <[email protected]>
rustyrussell and others added 28 commits December 12, 2019 15:03
…bolt11.

The pay plugin has been supplying the bolt11 string since 0.7.0, so only
ancient "pay" commands would be omitted by this change.

You can create a no-bolt11 "sendpay" manually, but then you'll find it
in 'listsendpays'.

Changelog-Removed: JSON: `listpays` won't shown payments made via sendpay without a bolt11 string, or before 0.7.0.
Signed-off-by: Rusty Russell <[email protected]>
This won't usually be visible to the end-user, since the pay plugin doesn't
do multi-part yet (and mpp requires EXPERIMENTAL_FEATURES), but we're ready
once it does.

Signed-off-by: Rusty Russell <[email protected]>
It makes sense, and it's been proposed for addition to the spec to
broad agreement:

	lightning/bolts#712

Signed-off-by: Rusty Russell <[email protected]>
Bastien TEINTURIER <[email protected]> writes:
> It looks like the split on c-lightning side is quite limited at the moment:
> the only option is to split a payment in exactly its two halves,
> otherwise I get rejected because of the rule of overpaying more than
> twice the amount?

We only tested exactly two equal-size payments; indeed, our finalhop
test was backwards.  We only complain if the final hop pays more than
twice msat (technically, this test is still too loose for mpp: the
spec says we should sum to the exact amount).

Reported-by: @t-bast
Signed-off-by: Rusty Russell <[email protected]>
Bastien TEINTURIER <[email protected]> writes:
> One thing I noticed but didn't investigate much: after sending the two
> payments, I tried using `waitsendpay` and it reported an error *208*
> (*"Never attempted payment for
> '98ee736d29d860948e436546a88b0cc84f267de8818531b0fdbe6ce3d080f22a'"*).
> 
> I was expecting the result to be something like: "payment succeeded for
> that payment hash" (the HTLCs were correctly settled).

Indeed, if you waitsendpay without specifying a partid, you are waiting
for 0, which may not exist.  Clarify the error msg.

Reported-by: @t-bast
Signed-off-by: Rusty Russell <[email protected]>
This uses the same state machine as HTLCs, but they're only
ever added, not removed.  Since we can only have one in each
state, we use a simple array; mostly NULL.

We could make this more space-efficient by folding everything into the
first 5 states, but that would be more complex than just using the
identical state machine.

One subtlety: we don't send uncommitted fee_states over the wire.

Signed-off-by: Rusty Russell <[email protected]>
And also move it to initial_channel, so we can use it there.

This saves churn in the next patch.

Signed-off-by: Rusty Russell <[email protected]>
…mitment.

The `channel_got_commitsig` we send the lightningd also implies we sent
the revoke_and_ack, as an optimization.  It doesn't currently matter,
since channel_sending_revoke_and_ack doesn't do anything important to the
state, but that changes once we start uploading the entire fee_states.

So now we move our state machine *before* sending to lightningd, in
preparation for sending fee_states too.

Unfortunately, we need to marshall the info to send before we
increment the state, as lightningd expects that.

Signed-off-by: Rusty Russell <[email protected]>
This is an intermediary step: we still don't save it to the database,
but we do use the fee_states struct to track it internally.

Signed-off-by: Rusty Russell <[email protected]>
These used to be necessary as we could have feerate changes which
we couldn't track: now we do, we don't need these flags.

Signed-off-by: Rusty Russell <[email protected]>
The upgrade here is a bit tricky: we map the two values into the
feerate_state.  This is trivial if they're both the same, but if
they're different we don't know exactly what state they're in (this
being the source of the bug!).

So, we assume that the have received the update and not acked it,
as that would be the normal case.

Signed-off-by: Rusty Russell <[email protected]>
This is the final step: we pass the complete fee_states to and from
channeld.

Changelog-Fixed: "Bad commitment signature" closing channels when we sent back-to-back update_fee messages across multiple reconnects.
Signed-off-by: Rusty Russell <[email protected]>
The norm for channels is a 1% reserve.

Signed-off-by: Rusty Russell <[email protected]>
Otherwise tests for hold_invoice fail on Travis (they use 180 / 2 as
the timeout, and we free it after 70 seconds).

Signed-off-by: Rusty Russell <[email protected]>
Thanks to @t-bast, who made this possible by interop testing with Eclair!

Changelog-Added: Protocol: can now send and receive TLV-style onion messages.
Changelog-Added: Protocol: can now send and receive BOLT11 payment_secrets.
Changelog-Added: Protocol: can now receive basic multi-part payments.
Changelog-Added: RPC: low-level commands sendpay and waitsendpay can now be used to manually send multi-part payments.

Signed-off-by: Rusty Russell <[email protected]>
We still close the channel if we *send* an error, but we seem to have hit
another case where LND sends an error which seems transient, so this will
make a best-effort attempt to preserve our channel in that case.

Some test have to be modified, since they don't terminate as they did
previously :(

Changelog-Changed: quirks: We'll now reconnect and retry if we get an error on an established channel. This works around lnd sending error messages that may be non-fatal.

Signed-off-by: Rusty Russell <[email protected]>
This matters now we have an array in tlv_init_tlvs!  We were overallocating
in fromwire by 32x!

Signed-off-by: Rusty Russell <[email protected]>
The sqlite database location has been changed in aab83e7.

Changelog-None
@gruve-p gruve-p merged commit 6c0ddf2 into master Dec 17, 2019
@gruve-p gruve-p deleted the 0.8.0 branch December 17, 2019 13:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.