Remove --enable-experimental-features, use all runtime flags. #6209
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
rustyrussell
force-pushed
the
guilt/no-experimental-build
branch
from
30802f9 to
cbff53e
Compare
rustyrussell
force-pushed
the
guilt/no-experimental-build
branch
2 times, most recently
from
5fb1d1a to
a56bee8
Compare
rustyrussell
force-pushed
the
guilt/no-experimental-build
branch
5 times, most recently
from
1e96146 to
11af8e5
Compare
rustyrussell
force-pushed
the
guilt/no-experimental-build
branch
2 times, most recently
from
33b7f41 to
90f8579
Compare
Signed-off-by: Rusty Russell <[email protected]>
There are no feature-dependent fields left in the spec. (I've also made a PR for the spec to remove the tooling for it there). Signed-off-by: Rusty Russell <[email protected]>
The modern style is to assert that all messages have tlvs, but many are currently empty. In particular, c4c5a8e5fb30b1b99fa5bb0aba7d0b6b4c831ee5 added "update_add_htlc_tlvs" without adding an explicit field of that type to "update_add_htlc". Signed-off-by: Rusty Russell <[email protected]>
Since this was merged, `make extract-peer-csv` was broken! But the field names changed: 1. `tlv_update_add_tlvs` -> `tlv_update_add_htlc_tlvs` 2. `blinding` -> `blinding_point`. Signed-off-by: Rusty Russell <[email protected]>
Changelog-EXPERIMENTAL: Config: `--experimental-quiesce` enables queiescence, for testing. Signed-off-by: Rusty Russell <[email protected]>
…MENTAL_FEATURES And no longer insist on opt_quiesce. Changelog-EXPERIMENTAL: Config: `--experimental-upgrade-protocol` enables simple channel upgrades. Signed-off-by: Rusty Russell <[email protected]>
Signed-off-by: Rusty Russell <[email protected]> xs
We no longer have any experimental-only wire definitions. Signed-off-by: Rusty Russell <[email protected]>
…_FEATURES. I don't know if anyone was using them, they seem half-hearted. Signed-off-by: Rusty Russell <[email protected]>
This currently means anchors tests are disabled, awaiting the PR which implements zero-fee-htlc anchors to reenable them. Signed-off-by: Rusty Russell <[email protected]>
Changelog-EXPERIMENTAL: Build: all experimental features are now runtime-enabled; no more ./configure --enable-experimental-features Signed-off-by: Rusty Russell <[email protected]>
rustyrussell
force-pushed
the
guilt/no-experimental-build
branch
from
90f8579 to
6efc1c5
Compare
vincenzopalazzo
approved these changes
May 22, 2023
There was a problem hiding this comment.
ACK 6efc1c5
Wow this is really good work, and it includes a lot of code simplification
Comment on lines
-40
to
-47
| - **type** (string, optional): the purpose of this input (*EXPERIMENTAL\_FEATURES* only) (one of "theirs", "deposit", "withdraw", "channel\_funding", "channel\_mutual\_close", "channel\_unilateral\_close", "channel\_sweep", "channel\_htlc\_success", "channel\_htlc\_timeout", "channel\_penalty", "channel\_unilateral\_cheat") | ||
| - **channel** (short\_channel\_id, optional): the channel this input is associated with (*EXPERIMENTAL\_FEATURES* only) | ||
| - **outputs** (array of objects): Each output, in order: | ||
| - **index** (u32): the 0-based output number | ||
| - **amount\_msat** (msat): the amount of the output | ||
| - **scriptPubKey** (hex): the scriptPubKey | ||
| - **type** (string, optional): the purpose of this output (*EXPERIMENTAL\_FEATURES* only) (one of "theirs", "deposit", "withdraw", "channel\_funding", "channel\_mutual\_close", "channel\_unilateral\_close", "channel\_sweep", "channel\_htlc\_success", "channel\_htlc\_timeout", "channel\_penalty", "channel\_unilateral\_cheat") | ||
| - **channel** (short\_channel\_id, optional): the channel this output is associated with (*EXPERIMENTAL\_FEATURES* only) |
There was a problem hiding this comment.
Can this be a breaking change for cln-grpc?
Also if people are not using it, the grpc could complain about different model, but for now it is also easy to upgrade
So I just put my though here
There was a problem hiding this comment.
No, because it was experimental only, the fields didn't exist for most people anyway.
There was a problem hiding this comment.
I do not think that the code generation is smart enough to catch the experimental field or these changes in the rust model are unrelated? https://github.com/ElementsProject/lightning/pull/6209/files#diff-af0e7f481b7fd8fc471ab4d5a808c9b4512e0d347bfceff633effc80acdcf5fe
But maybe the proto-buff do not complain when there is the optional fields
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.
Based on #6263 to fix CI.Merged!Most experimental features are already runtime-enabled, this completes them, and removes the build concept entirely.
It will clash a little with the zero-fee anchors work, but that's OK.