Test init networks TLV #13
Open
Conversation
This file contains 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
When the `p` multiplier is used, the amount MUST be divisible by 10 since the resolution used internally is millisatoshi. This addresses but does not close lightning#692.
Tabs or spaces ? Spaces seems to largely beat tabs in this files (and more globally in the repo).
We simply specify, in each case, where they will appear ("Context").
Because `globalfeatures` is already in use, we fold that into the
renamed `localfeatures` field to unify them (now called `features`),
but dissuade further use.
Note also: we REQUIRE minimal `features` field in
channel_announcement, since otherwise both sides of channel will not
agree and not be able to create their signatures!
Consider these theoretical future features:
`opt_dlog_chan`: a new channel type which uses a new discrete log HTLC
type, but can't support traditional HTLC:
* `init`: presents as odd (optional) or even (if traditional channels
not supported)
* `node_announcement`: the same as above, so you can seek suitable peers.
* `channel_announcement`: presents as even (compulsory), since users need
to use the new HTLCs.
`opt_wumbochan`: a node which allows channels > 2^24 satoshis:
* `init`: presents as odd (optional), or maybe even (if you only want
giant channels)
* `node_announcement`: the same as above, so you can seek suitable peers.
* `channel_announcement`: not present, since size of channel indicates
capacity.
`opt_wumbohtlc`: a channel which allows HTLCs > 2^32 millisatoshis:
* `init`: presents as odd (optional), or even (compulsory)
* `node_announcement`: the same as above, so you can seek suitable peers.
* `channel_announcement`: odd (optional) since you can use the channel
without understanding what this option means.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Co-Authored-By: Bastien Teinturier <31281497+t-bast@users.noreply.github.com>
The feature fields refer to the properties of the channel/node, not the message itself, so we can still propagate them (and should, to avoid splitting the network). If we want to make an incompatible announcement message, we'll use a different type, or insert an even TLV type. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
A bit less dense, but avoids a separate feature space. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
I recently made a cut & paste bug with the protocol tests, and paid an HTLC of amount 100M msat, but with only a 1M msat `amt_to_forward` in the hop_data. To my surprise, it was accepted. This is because we allow overpaying the routing fee (considered 0 for the final hop). This doesn't make sense for the final hop: anything but exact equality implies a bug, or that the previous node took the wrong amount from the payment. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We also define what the basic_mpp feature means in an invoice, by reference to the next commit. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This also defines the TLV format for payment_secret; the two are intertwined. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This means the BOLT11 invoice must offer it (we already say it must set the field if it offers it), and that the receiving node must require it (again, we already say it must check it if it requires it). Without the payment_secret, MPP payments are especially vulnerable to probing attacks: unlike normal payments (with amounts) they can be detected with 1msat payment probes. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
*Require* payment_secret for multi-part payments
BOLT 4 explicitly indicates var_onion_optin may appear in a BOLT 11 invoice, however, BOLT 9 only indicates it is available in init and node_announcement contextx. Resolve this conflict in favor of BOLT 4 as there doesn't seem to be much reason to *not* allow it in BOLT 11 invoices.
This appears to have been an oversight in the flat features spec, and is somewhat implicitly relied on for several new feature bits - if var_onion_optin is set on a node_announcement (its not allowed on a channel_announcement), then trying to route through that node using the pre-tlv formt is somewhat nonsensical, and should be forbidden.
Resolve two spec oddities regarding new features.
Update a requirement that was missed in lightning#711
…lightning#722) As reading of commit 6729755 shows, `final_expiry_too_soon` was 17, not PERM|17. Note that because we folded a previously non-permanent failure into the now-permanent PERM|15 failure code, modifications to payment algorithms may now be needed to specificalyl detect this case, otherwise payment algorithms may give up in some edge cases where blocks are mined while payments are in-transit between sender and receiver.
This commit: - Adds a new Dependencies column to the BOLT 9 feature table populated with existing feature dependencies. - Requires that all valid feature vectors set transitive dependencies. - Requires checking transitive dependencies when validating init messages and payment request. - Removes transitive feature requiremetns from the BOLT 11 writer, now that they are implicit by needing to comply with the BOLT 9 origin requirements.
As a final step, we now can remove several of the BOLT 11 writer's requirements now that it builds on BOLT 9's, particularly: - setting the even bit if a feature is required. - only setting a feature if the node supports a given feature. The lone requirement that remains pertains to setting the `s` value if and only if the `payment_secret` feature is set.
You can still override on cmdline. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Feature bit is now 13 (0x80000), and we include the my_current_per_commitment_point with option_static_remotekey; we just ignore it. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
They worked for about two weeks after I wrote them! Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Will be used later for fundchannel process
Allow for the test to specify the feerate for a fundchannel open
Build out the command for fundchannel, originates open_channel from node under test
If we exit before fundchannel has finished, it shows errors; let's swallow the errors if they're happening on/after shutdown
We need to make sure that any fundchannel process is killed by stop, so that we can run multiple fundchannel attempts in the same go Also removes 'shutdown(wait=False)' since it doesn't do anything, really.
Simple 'open channel' check from the opener's perspective. Uses the 'new' fundchannel impl for c-lightning.
Moving c-lightning over to 'ground' sigs means that it'd be more flexible if the protocol test framework allowed for dynamically generating or verifying sigs based on a hash + private key. Uses ':' notation as a separator so that `htlc_signature` list parsing will still work without modification
missing openchannel, will follow with cleanup
As changed in commit aab83e729b93d6ce2e2b4702681aaba71462bec8. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
…e used. Add another 16 bits. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
NameError: name 'peers' is not defined Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This has been discussed for forever, but now we have TLVs the correct encoding seems obvious. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
darosior
force-pushed
the
test_init_networks
branch
from
0915ea7
to
66f2f6d
Compare
|
Rebased now that lightningnetwork#682 was merged |
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.
This is based upon #12.
This is rebased upon
the non-merged-yetlightning#682.This (succesfully) tests ElementsProject/lightning#3371.