This happens if:
1. The peer sets a timestamp filter to non-zero, and
2. We have a channel_announcement without a channel_update.
The timestamp is 0 as a placeholder as part of the recent gossip rework
(we used to hold these channel_announcement in memory, which was complex).
But this means we won't send it in this case, and if we later send the
channel_update, CI will complain about 'Bad gossip order'.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Normally, channels are marked dying, the 12 blocks later, removed.
But for local channels, we can access any spliced channel already, so
we remove them immediately from our local gossip. This left a hole in
our logic, if that channel was the last one keeping a
node_announcement alive.
Solution is to unify with the "moved node_announcement" path.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We make sure a node_announcement is preceeded by at least one channel_announcement,
but dying ones don't count (as they are not broadcast!).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We accept node_announcements on dying channels, but make sure we
set the dying flag it channels are alll dying.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We can update dying channels, though it seems weird! We accept gossip about them,
we just don't propagate it.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This avoids us gossiping about nodes which don't have live channels.
Interstingly, we previously tested that we *did* gossip such node
announcements, and now we fix that test.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
It prints a message to stderr, but actually it's fine with this version:
```
dump-gossipstore: UNKNOWN GOSSIP minor VERSION 14 (expected 12)
```
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
If the peer isn’t required to send signatures first but does while we are awaiting the next user RPC action — we should be caching the message and using it later.
Before we would leave the message cached in the socket itself, but tx_abort semantics require us to check the socket more often.
Add checking for and sending tx_abort to channeld.
When receiving it we first ACK it back, send a request to restart to lightningd, and then shutdown channeld.
We also must update the splice tests that relied on reconnect checks for splice warnings (as some are now tx_aborts instead).
In the old version requesting the config-value of an option
was a little bit tricky.
Let's say we want to have a plugin which uses a default
port of 1234.
```rust
let value = plugin.option("plugin-port");
match value {
Some(Value::Integer(_)) => {},
Some(Value::String(_)) => {}, // Can never happen
Some(Value::Boolean(_)) => {}, // Can never happen
None => {}, // Can never happen
}
```
Many users of the `cln_plugin` crate are overly cautious
and handle all these error scenario's. Which is completely unneeded.
Core Lightning will complain if you put a `String` where an `Integer` is
expected and will never send the value to the plug-in.
This change makes the API much more ergonomical and actually motivates
some of the changes in previous commits.
```
const MY_OPTION : ConfigOption<i64> = ConfigOption::new_i64_with_default(
"plugin-port',
1235,
"Description");
let value : Result<i64> = plugin.option(MY_OPTION);
```
The result will provide a proper error-message.
It is also safe to `unwrap` the result because it will
only be triggered if the user neglected to provide the
option to the `Builder`.
This is the first part of two commits that attempts to simplify
the API that is used to retrieve configuration values.
Previously, we encouraged the following pattern to build a plugin.
```rust
let configured_plugin =
Builder::new(
tokio::io::stdin(),
tokio::io::stdout())
.option(ConfigOption::new_i64_with_default("some-option", 0, "Description"))
.configure();
let value = configured_plugion.option("some-option");
match value {
Some(Integer(i)) => {}, // Config provided
None => {}, // No config set
_ => {}, // This should never happened
}
```
This commit helps to move to the following pattern
```rust
const SOME_OPTION : ConfigOption<i64> = ConfigOption::new_i64_with_default(
"some-option",
"description");
async fn main() -> Result<()> {
let plugin = Builder::new(tokio::io::stdin(), tokio::io::stdoout())
.option(SOME_OPTION)
.configure()
.await?;
let value : i64 = plugin.option(SOME_OPTION)?;
}
```
Breaking changes here.
This improves the semantics of `ConfigOption` and `options::Value`
drastically.
We've been using `options::Value` for 2 purposes
1. To specify the type and default value of an option
2. Coummunicate how a user configured an option
We fall here in the pit-fall of being poor at both purposes.
I've edited the code to ensure
- `options::Value` -> To read or specify the value of an option
- `option::ValueType` -> To specify the type of an option
**Configure an option**
Let's create an string-typed option create an option named `"required-string-opt"` the
developer has to use the following code.
```rust
let option = ConfigOption::new("required-string", Value::OptString, "description");
```
The semantics of `OptString` might falsely suggest that it is optional to specify the config.
However, we use `OptString` to say we are not providing a default-value.
After this commit we can use instead
```rust
let option = ConfigOption::new_str_no_default("required-string", "description");
```
For reading a configured value the `option::Value` is somewhat
cumbersome. The old version of had 6 different types of value
```rust
let value = plugin.option("required-string");
match value {
String(s) => {}, // User has configured string value or default
Integer(i) => {}, // User has configured int value or default
Boolean(b) => {}, // User has configured bool value or default
OptString => {}, // User has not configured value and no default
OptInteger = {}, // User has not configured value and no default
OptBOolean => {}, // User has not configured value and no default
}
```
This has been changed to
```rust
let value = plugin.option("required-string");
match value {
Some(String(s)) => {},
Some(Integer(i)) => {},
Some(Boolean(b)) => {},
None => {},
}
```
There are quite some things we want to generate from the schema
definitions, both inside and outside of CLN itself. By bundling the
schemas into the library we can make use of the tooling without
running in the CLN source tree. This is in part used to generate some
of the interfaces in Greenlight.
Changelog-None
Commit dac8964093 set aliases for channels at
creation time, but neglected to convert channels in the database. Do that now!
Fixes: #7039
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
If we try to reuse the same UTXO for the HTLC as the anchor, it will clash.
One block later we can spend the anchor change, all good.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We still want to test non-anchor channels, as we still support them, but
we've made it non-experimental. To test non-anchor channels, we
use dev-force-features: -23.
Changelog-Added: Protocol: `option_anchors_zero_fee_htlc_tx` enabled, no longer experimental.
Changelog-Changed: Config: `experimental-anchors` now does nothing (it's enabled by default).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Header from folded patch 'fixup!_options__make_anchors_enabled_by_default,_ignore_experimental-anchors.patch':
fixup! options: make anchors enabled by default, ignore experimental-anchors.
This simplifies our tests which will want to turn off anchors,
even though they won't be set for elements.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We used to look for either other outputs which are sufficient for
reserve, *or* be able to create sufficient change to meet the
emergency reserve. Allow the sum of both to meet the requirements:
otherwise test_funder_contribution_limits can flake.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We tried to open a channel with feerate 0 in this case! Rework so it's
clear that we have two different feerates here.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
The `amount` field is intended to be either a native unit (`msat`,
`sat` or `btc`) or it can also be a non-native unit that needs to be
converted into native when creating the invoice. As such limiting the
`amount` to only be native is very restrictive.
