-
Notifications
You must be signed in to change notification settings - Fork 922
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
Peer storage enable #8140
Draft
rustyrussell
wants to merge
40
commits into
ElementsProject:master
Choose a base branch
from
rustyrussell:peer-storage-enable
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Peer storage enable #8140
rustyrussell
wants to merge
40
commits into
ElementsProject:master
from
rustyrussell:peer-storage-enable
+2,021
−4,093
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
_in_ versions complain if used, _out_ don't (since we don't know if they are going to use it). This was the wrong choice, and thus causes a BROKEN log message when we update CLN_NEXT_VERSION. Signed-off-by: Rusty Russell <[email protected]>
…do_stress any more. In d18f564 "pytest: stop using deprecated commando_rune commands." we stopped using deprecated commands in these tests, but we didn't remove the 'allow-deprecated-apis' flag. Signed-off-by: Rusty Russell <[email protected]>
…utput. Somehow we missed this when we added libplugin support. Signed-off-by: Rusty Russell <[email protected]>
When we update the CLN_NEXT_VERSION, these will only be available with --i-promise-to-fix-broken-api-user. Signed-off-by: Rusty Russell <[email protected]>
We now have to explicitly enable various deprecated commando commands, and now when deprecations are disabled, we honour missing MPP option in bolt12 invoices. Signed-off-by: Rusty Russell <[email protected]>
We haven't printed this since v24.08. Signed-off-by: Rusty Russell <[email protected]> Changelog-Removed: listconfigs raw listing (deprecated v23.08, disabled by default in v24.11).
Signed-off-by: Rusty Russell <[email protected]> Changelog-Removed: connection/disconnection/block_added notification raw fields (deprecated v23.08, disabled by default in v24.11).
Changelog-Removed: accept-htlc-tlv-types (deprecated v23.08, disabled by default in v24.11). Signed-off-by: Rusty Russell <[email protected]>
… sockets. Changelog-Removed: --bind-addr and --addr on onion addresses and local sockets (deprecated v23.08, disabled by default in v24.11). Signed-off-by: Rusty Russell <[email protected]>
Signed-off-by: Rusty Russell <[email protected]> Changelog-Removed: Config: `announce-addr-dns` (deprecated v23.08, disabled in v24.11).
Signed-off-by: Rusty Russell <[email protected]> Changelog-Removed: allowing 0/1 instead of false/true for plugin options (deprecated v23.08, disabled by default in v24.11).
…t values. Signed-off-by: Rusty Russell <[email protected]> Changelog-Removed: default settings for flag options in plugins which aren't "false".
Signed-off-by: Rusty Russell <[email protected]> Changelog-Removed: plugins which didn't accept string JSON RPC fields (deprecated v23.08, disabled by default in v24.11).
Signed-off-by: Rusty Russell <[email protected]> Changelog-Removed: RPC `listchannels` no longer includes private local channels (deprecated v23.08, disabled by default in v24.11).
…fore sending announcement_signatures. We have a replacement quote which is suitable here, but it comes in a later BOLT commit. Signed-off-by: Rusty Russell <[email protected]>
…ices. Signed-off-by: Rusty Russell <[email protected]>
Unfortunately a spec typo means the data fields are missing (PR pending), so we still patch those in. The message "your_peer_storage" got renamed to "peer_storage_retrieval", and the option "want_peer_backup_storage" was removed. Signed-off-by: Rusty Russell <[email protected]>
…ound `channel_announcement` handling (ElementsProject#1220)" Signed-off-by: Rusty Russell <[email protected]>
Signed-off-by: Rusty Russell <[email protected]>
Since we included the spec for it, this is a good time to implement it. I also asked chatgpt to write some unit tests. I had to mangle them a bit, but it probably saved me a few minutes. Signed-off-by: Rusty Russell <[email protected]>
Calling the funding tx an anchor is pre-spec terminology, which is now confusing; let's rename the variable. Signed-off-by: Rusty Russell <[email protected]>
In this case a "are we closed onchain". Signed-off-by: Rusty Russell <[email protected]>
We want a more fine-grained approach, so we now have: 1. update_channel_update() - returns true if it changed. 2. channel_should_enable() - should this channel be marked enabled. 3. arm_refresh_timer() - start a refresh timer for the channel_update. Signed-off-by: Rusty Russell <[email protected]>
This message was too verbose (even for trace!) Signed-off-by: Rusty Russell <[email protected]>
They can all call get_block_height(); the extra argument confused me and I thought they were called before the block height was actually updated. Signed-off-by: Rusty Russell <[email protected]>
… wait until 6 deep. The spec used to say you had to wait for channel to be ready, *and* 6 depth before exchanging signatures. Now the 6 depth requirement is only on the actual announcing of the channel: you can send sigs any time. This means our state machine goes from: NOT_USABLE -> NOT_DEEP_ENOUGH -> NEED_PEER_SIGS -> ANNOUNCED to: NOT_USABLE -> NEED_PEER_SIGS -> NOT_DEEP_ENOUGH -> ANNOUNCED However, this revealed that our state machine is insufficient, so rework it to be more general and understandable. In particular, check for unexpected state transitions, and thus document them. Signed-off-by: Rusty Russell <[email protected]> Changelog-Changed: Protocol: We now exchange `announcement_signatures` as soon as we're ready, rather than waiting for 6 blocks (as per recent BOLT update)
This is the only way to track spec changes in future. Signed-off-by: Rusty Russell <[email protected]>
after_listpeers() doesn't use the argument, so we don't need to allocate anything. Signed-off-by: Rusty Russell <[email protected]>
We should not send a message on unknown custom message receipt (we expect this!). And don't LOG_INFORM every time we send a peer storage. Signed-off-by: Rusty Russell <[email protected]>
This avoids globals, which is a bit neater. Signed-off-by: Rusty Russell <[email protected]>
This seems fair. Signed-off-by: Rusty Russell <[email protected]>
…ing with NULL cb. This should complete the command, otherwise we get an assertion on line 1090. Signed-off-by: Rusty Russell <[email protected]>
By keeping a local hash table, we won't have to look up every time. We still write to the datastore when it changes, and we need to initialize it at plugin start. Signed-off-by: Rusty Russell <[email protected]>
Now actually use our cache. Signed-off-by: Rusty Russell <[email protected]>
We already get the connected hook, so in there we can add to a hash table of suitable peers. Rather than subscribe to disconnection, we simply remove the peer if a sendcustommsg fails. This does make after_send_scb_single() a bit more complex, since it needs this specific node_id: we use a `struct info` per node and a pointer to a shared "idx" reference counter. Signed-off-by: Rusty Russell <[email protected]>
This is much better for very large nodes with lots of channel activity. Signed-off-by: Rusty Russell <[email protected]>
This, too, is vital for large nodes, which don't want a significant traffic increase. Signed-off-by: Rusty Russell <[email protected]>
They will in fact get truncated, and never restore. Large nodes should be using some real backup strategy! For this reason, we split "peer_backup" support into send vs store support, so we can turn off send of our own without disabling storing/sending theirs. Signed-off-by: Rusty Russell <[email protected]>
Now we've make it only on existing channels, and not have to call listdatastore every time, that means we can safely turn it on by default. Changelog-Added: Protocol: we now offer peer storage to any peers who create a channel. Changelog-Deprecated: Config: `--experimental-peer-storage` (it's now the default). Signed-off-by: Rusty Russell <[email protected]>
We picked one node and iterated. This means we would only sent to 1, not 2 nodes if we picked the last (common if there were only a few peers). But it also means we would generally send to the same pair of nodes, which isn't great for redundancy. Rework to be more random. Signed-off-by: Rusty Russell <[email protected]>
e89b21c
to
36b6df1
Compare
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 #8136
This gets chanbackup to the point that we can enable it by default. If we did that now it would have too much penalty for large nodes with many channels, so we fix that up, and finally enable it!