-
Notifications
You must be signed in to change notification settings - Fork 488
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 for nodes to distribute small encrypted blobs. #1110
base: master
Are you sure you want to change the base?
Conversation
01-messaging.md
Outdated
|
||
Upon receiving `peer_storage` nodes should immediately respond with the latest `your_peer_storage`, which could work as an ack to the peer. | ||
|
||
Nodes with `option_provide_storage` should also regularly send `your_peer_storage` to verify that they store the latest and correct form of `blob`. Nodes may want to ignore `peer_storage` messages with peers they don’t have a channel with to avoid spam. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This part is unclear to me. Based on my reading, there are two instances when a node (running option_provide_storage
) will send your_peer_storage
message:
- When it gets a
peer_storage
message and has stored the data (like an ack) - When it reconnects with the peer for which it has stored data (to ensure that peer has a chance to update stale data)
The point about "regularly" sending your_peer_storage
seem to be specified loosely.
- What's the right frequency to send this to your peer?
- Does it create a spam vector for your peer?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed. You don't need to specify this, since you already said in requirements that "MAY send peer_storage
whenever necessary (usually, everytime their configuration changes)".
And I made a suggestion for spam above...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, the two instances that you mentioned are the only time when a peer needs to send your_peer_storage
.
01-messaging.md
Outdated
|
||
Upon receiving `peer_storage` nodes should immediately respond with the latest `your_peer_storage`, which could work as an ack to the peer. | ||
|
||
Nodes with `option_provide_storage` should also regularly send `your_peer_storage` to verify that they store the latest and correct form of `blob`. Nodes may want to ignore `peer_storage` messages with peers they don’t have a channel with to avoid spam. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed. You don't need to specify this, since you already said in requirements that "MAY send peer_storage
whenever necessary (usually, everytime their configuration changes)".
And I made a suggestion for spam above...
a766bfc
to
4e945ee
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ack
As stated during the spec meeting, I think this should be moved to the bLIP repository: https://github.com/lightning/blips |
I think in order to make peer storage reliable, it should have maximum coverage across the network. This approach attempts to address the backup issue, which is essential for enhancing the usability of the Lightning Network. Many small node operators may be reluctant to invest in RAID systems to protect their nodes; thus, this could serve as one of the safety nets for the network. That is why we should consider adding it to the main spec imo. |
01-messaging.md
Outdated
- If it offered `option_provide_storage`: | ||
- if it has an open channel with the sender: | ||
- MUST store the message. | ||
- MAY store anyway (e.g. might get paid for the service). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(e.g. might get paid for the service)
Could provide a bit more context here w.r.t being paid for the purpose of storing blobs
Is this why if it has an open channel with the sender
is a requirement?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, this needs more explanation, I'll add to it when I get back to my laptop.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this why if it has an open channel with the sender is a requirement?
I'd imagine this also helps with spam prevention. Since in order to do a drive-by to store data you'd have to commit capital.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Open channel is not a requirement. Some nodes might provide this service in exchange for some sats that is what might get paid for the service
means.
The sender of `peer_storage`: | ||
- MUST offer `option_want_storage`. | ||
- MAY send `peer_storage` whenever necessary (usually, everytime their configuration changes) | ||
- MUST limit its `blob` to 65531 bytes. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could also consider padding so that it is always fixed to 65531 bytes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why would you want to pad?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the reasoning would be privacy related: ie so that receiver cant deduce anything about the peer sending the data based on size changes.
That being said: I think all of that detail doesnt need to be specced out on this layer. How the blob is structured is the responsibility of the layer above.
Especially if the layer above is a "storage for payment" layer where they are charging per byte (or byte bucket) then we really dont want to force them to use the full 65k here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We already handle this with the randomization of ping/pong responses. The actual messages themselves are encrypted as well. Padding the message out to 65531 actually negatively impacts privacy as best I can tell since now we can do partial inference and filter out all non-max-length messages from the anonymity set.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oops I misread this. Indeed the receiver may come to learn about the contents of the payload through size analysis. My commentary on ping/pong length randomization is about 3p privacy, not 2p privacy.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah my initial thinking was that certain data sizes can be associated with certain types of backups or data in general
That being said: I think all of that detail doesnt need to be specced out on this layer. How the blob is structured is the responsibility of the layer above.
Yes I see what you're saying here. In this case I assume the "layer above" lies within the node implementation itself? It's not like external software would have direct access to this field
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Regardless, is it the BOLT's responsibility to document the need for padding?
1. type: 7 (`peer_storage`) | ||
2. data: | ||
* [`u16`: `length`] | ||
* [`length*byte`:`blob`] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could also extend this in order to have key-values instead of one single storage blob?
* [`length*byte`:`blob`] | |
* [`u16`:`key`] | |
* [`length*byte`:`blob`] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, good idea, but I think that would be more useful in case we want to distribute multiple backups (or backup with >65kb data).
In that case, we'd also have to specify which key
would be considered ack
, and this could lead to spamming (i.e. Peer can send me many blobs that could fill up my storage) that is why the current scheme just replaces the old storage blob with a new one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could also extend this in order to have key-values instead of one single storage blob?
Under the hood it might be a key-value structure - but think that that is better left to the above layer.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Under the hood it might be a key-value structure - but think that that is better left to the above layer.
+1. Single Opaque Buffer is simplest to implement and all other data structures can be implemented atop this if we want. KV storage also introduces the question of what we are supposed to send back on reconnect.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, not a great idea to go kv
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I concur with @t-bast that this feature could be a BLIP.
Since this is not a one-size-fits-all solution, we can create different BLIPs for various backup use cases (if any others are proposed in the future).
If we want to add this to the BOLTs, we should address some concerns like this one ElementsProject/lightning#6652
01-messaging.md
Outdated
Requirements: | ||
|
||
The sender of `peer_storage`: | ||
- MUST offer `option_want_storage`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
dont think i fully understand why we need the option_want_storage
feature bit..?
isnt it enough that im connecting to a node with option_provide_storage
and sending peer_storage
to it? Or is there a case where someone would want to seek out a node advertising option_want_storage
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm inclined to agree. I think in practice it is very likely that people will implement this protocol wholesale or not at all (it's pretty simple). That said, I can imagine scenarios where you may specifically not want to provide this feature but still would like to consume it. At 65k per peer though, idk what the big deal would be with providing it anyway.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Big nodes with large number of peers might not be interested in giving this service to all of their peers.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you suggesting they will only want to offer it to some of their peers or rather in that situation they wouldn't want to offer it to any of them?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Big nodes with large number of peers might not be interested in giving this service to all of their peers.
I dont think having/not having option_want_storage
changes this though?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the only reason you would advertise option_want_storage
is so that nodes offering the service could seek you out. but that doesnt really make sense to me. Makes more sense that if you want storage - you would seek out nodes that can offer it to you.
|
||
Nodes ask their peers to store data using the `peer_storage` message & expect peers to return them latest data using `your_peer_storage` message: | ||
|
||
1. type: 7 (`peer_storage`) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we not use even types for these messages? since we are only sending these if the peer advertises option_provide_storage
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
does IOKTBO apply to message types?? I thought it was just TLVs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah it applies to messages too afaiu: https://github.com/lightning/bolts/blob/master/01-messaging.md#lightning-message-format
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this applies to the low message type numbers though afaict. If it did then things like accept_channel
would be treated the same right? I would support using even messages, no issue with that at all, but I think I'm not quite understanding the intricacies of IOKTBO
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah i think for some of the very first messages this was sort of overlooked but it wasnt a problem since nothing would work if a node didnt understand something fundamental like accept_channel
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IOKTBO definitely applies to all messages, its just in the very initial protocol there's no distinction cause everyone supports everything :). Its definitely nice for these to be odd because then nodes can send peer_storage
without bothering to check if their peer supports the feature at all.
01-messaging.md
Outdated
|
||
- If it does store the message: | ||
- MAY delay storage to ratelimit peer to no more than one update per second. | ||
- MUST respond with `your_peer_storage` containing `blob` after it is stored. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lets not respond with the full 64KiB blob every time data gets stored. That seems incredibly wasteful. If you want an ACK, lets include an ID.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we can also ACK with a sha256 of the data provided.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't make me hash the data, either. Honestly, I'm not really sure if we need to have an ACK at all....its useful if you trust the counterparty and want to use it as a robust data store (but, even then, I think only LDK has support for fully async persistence?, and I'm not convinced our users would prefer this over a traditional storage layer) but I'm not sure that's worth doing here when the whole design space is around "you may persist, if you want, but peers shouldn't rely on it". Maybe @t-bast has other thoughts?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree with @TheBlueMatt here, our proposal for this feature (#881) does not have an ACK because I didn't find that useful.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It serves as assurance to the sender, although not foolproof, that it won't be excessively wasteful. Additionally, updating peer_storage won't be a frequent occurrence.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The sole reason for ACK was to assure the sender, is their any other efficient method to do this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But does it really assure the sender of anything meaningful? If used to store every channel state change (which is what we've been doing for years), we don't want to pause the channel's operation waiting for an ACK, it adds unnecessary complexity and risk of force-closing. This is mostly being used by nodes who will regularly disconnect (e.g. mobile phones), who have an opportunity of checking the back-up returned on every connection reestablishment, which is IMO enough assurance that the storing node behaves correctly?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd like to remind us that we didn't need ACKs for every channel update either, and yet we are nudging ourselves towards a simplified update protocol. Granted, the purpose of this is far less consequential than ACK'ing the channel updates themselves, so we can afford to be like "🤷🏻♂️ who knows if they got it, we'll just assume so".
My take here is that at minimum it helps with debugging, and the cost of the ACK seems rather trivial so avoiding it seems silly. Yes, it doesn't provide much, we can always send back the packet, or the hash, or some ID, and then just yeet the data into oblivion, but does it really cost us much to just ACK it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The sole reason for ACK was to assure the sender, is their any other efficient method to do this?
To echo @t-bast's comment - to assure the sender of what? And what should the sender do with that assurance. If a given message is not actually handled by a peer/it doesn't change their behavior in some meaningful way, we should drop it.
I'd like to remind us that we didn't need ACKs for every channel update either,
Not sure what you're referring to - do you mean we don't need revoke_and_ack
? We absolutely do, the protocol doesn't work without it.
My take here is that at minimum it helps with debugging, and the cost of the ACK seems rather trivial so avoiding it seems silly. Yes, it doesn't provide much, we can always send back the packet, or the hash, or some ID, and then just yeet the data into oblivion, but does it really cost us much to just ACK it?
Yes, more protocol. We have a lot of protocol, anywhere we can have just a bit less protocol is great. Its not clear to me why an ACK will help with debugging - a peer can always give us an old blob, whether they ACK'ed it or not, and if you get an old state you need to go try to figure out if the state is old and which one it is. You can always check if there was a ping/pong after the sent storage message and use that as an ack/they received it indication.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed ACK as a requirement. Still mentioned in the rationale so that implementations can decide for themselves.
1007e24
to
e356ade
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK
01-messaging.md
Outdated
The sender of `peer_storage`: | ||
- MAY send `peer_storage` whenever necessary (usually, everytime their configuration changes) | ||
- MUST limit its `blob` to 65531 bytes. | ||
- MUST encrypt the data in a way that can be validated when received. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
encryption doesn't generally ever provide validation. Thus, this sentence is very confusing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
MUST encrypt the data in a manner that ensures its integrity upon receipt?
- MAY store anyway. | ||
|
||
- If it does store the message: | ||
- MAY delay storage to ratelimit peer to no more than one update per second. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a lot of data storage. Even with no overhead 64KiB/second will destroy almost any consumer SSD in a year. ISTM one update per second is much too much if you're not charging for it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the idea is to replace it? Is it the IOPS themselves that will destroy the SSD or are you saying it'll just overrun the storage capacity.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, the peer is supposed to replace the received data every time. Although what do you suggest would be a good ratelimit?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think storage here means persistent storage? I am thinking this should not be in the spec, how often an implementation chooses to store the backup data should be up to them? There is really no need for a consensus on it?
Edit:
I think this might not work because there is no guarantee that there would be a graceful shutdown.
The way this is set up, the peer is expected to replace the backup data with the latest one right? that means only the latest data is needed to be backed up. The data is not also retrieved throughout the connection with the peer only on reconnection. So that means there is really no need to persist this data until the peer connection is over, right? So no need for update/second, but then again this should be up to implementation?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a lot of data storage. Even with no overhead 64KiB/second will destroy almost any consumer SSD in a year. ISTM one update per second is much too much if you're not charging for it.
Agree and I believe directly charging is too involved for a low level feature like this. Could indirectly relate this rate to a cost, something like rate limiting to 1 update per htlc that has been routed over the channel.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it the IOPS themselves that will destroy the SSD
Yes, over one year 64KiB/second is 2 TB, which is more than most consumer SSD's write lifetimes.
rate limiting to 1 update per htlc
that's even worse :)
ISTM a rate-limit of one/minute makes a lot more sense.
01-messaging.md
Outdated
- If it does store the message: | ||
- MAY delay storage to ratelimit peer to no more than one update per second. | ||
- MUST replace the old `blob` with the latest received. | ||
- MUST send `peer_storage_retrieval` again after reconnection. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we clarify this somewhat to be "immediately after exchanging init message" or something like that? We don't want it to be after channel reestablish.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be mentioned in rationale?
01-messaging.md
Outdated
- MAY send a warning. | ||
|
||
Rationale: | ||
If a peer supports `option_provide_storage` and we have a channel with it, we can send a `peer_storage` message with any `blob` so that peer can store it locally in their node. Everytime a node receives a `peer_storage` message, It needs to update the `blob` for the sender. Even if we don't have an open channel, some nodes might provide this service in exchange for some sats so they may store `peer_storage`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The first two sentences here seem to add nothing. This should instead describe what the feature is useful for, rather than simply what it does.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rephrasing it. Thanks for pointing out.
6270078
to
f076f6c
Compare
01-messaging.md
Outdated
@@ -494,6 +494,76 @@ every message maximally). | |||
Finally, the usage of periodic `ping` messages serves to promote frequent key | |||
rotations as specified within [BOLT #8](08-transport.md). | |||
|
|||
### Peer storage |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the "Peer Storage" header should be larger that the sub-headers. Ie, main header should use "##" and subheaders "###".
Also - should add an entry to the table of contents at the top of the doc
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for pointing this out. :)
4fa753f
to
3ce41b2
Compare
The sender of `peer_storage`: | ||
- MUST offer `option_want_storage`. | ||
- MAY send `peer_storage` whenever necessary (usually, everytime their configuration changes) | ||
- MUST limit its `blob` to 65531 bytes. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah my initial thinking was that certain data sizes can be associated with certain types of backups or data in general
That being said: I think all of that detail doesnt need to be specced out on this layer. How the blob is structured is the responsibility of the layer above.
Yes I see what you're saying here. In this case I assume the "layer above" lies within the node implementation itself? It's not like external software would have direct access to this field
- MAY store anyway. | ||
|
||
- If it does store the message: | ||
- MAY delay storage to ratelimit peer to no more than one update per second. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a lot of data storage. Even with no overhead 64KiB/second will destroy almost any consumer SSD in a year. ISTM one update per second is much too much if you're not charging for it.
Agree and I believe directly charging is too involved for a low level feature like this. Could indirectly relate this rate to a cost, something like rate limiting to 1 update per htlc that has been routed over the channel.
The sender of `peer_storage_retrieval`: | ||
- MUST include the last `blob` it stored for that peer. | ||
- when all channels with that peer are closed: | ||
- SHOULD wait at least 2016 blocks before deleting the `blob`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what's the thinking here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To make it a bit more reliable. So that user might have a chance to get the PeerStorage back from recent old peers as well.
1. type: 7 (`peer_storage`) | ||
2. data: | ||
* [`u16`: `length`] | ||
* [`length*byte`:`blob`] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, not a great idea to go kv
The sender of `peer_storage`: | ||
- MUST offer `option_want_storage`. | ||
- MAY send `peer_storage` whenever necessary (usually, everytime their configuration changes) | ||
- MUST limit its `blob` to 65531 bytes. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Regardless, is it the BOLT's responsibility to document the need for padding?
might provide this service in exchange for some sats so they may store `peer_storage`. | ||
|
||
`peer_storage_retrieval` should not be sent after `channel_reestablish` because then the user | ||
wouldn't have an option to recover the node and update its state in case they lost data. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wouldn't have an option to recover the node and update its state in case they lost data.
can you elaborate a bit more here? don't really get why sending peer_storage_retrieval
hurts here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If data is lost, a node would need to see their storage before they can process the reestablish message.
their data at each reconnection, by comparing the contents of the | ||
retrieved data with the last one they sent. However, nodes should not expect |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If nodes can't rely on their peer having the latest data, presumably they shouldn't be checking against the latest :)
their data at each reconnection, by comparing the contents of the | |
retrieved data with the last one they sent. However, nodes should not expect | |
their data at each reconnection, by comparing the contents of the | |
retrieved data with one they sent. However, nodes should not expect |
|
||
Nodes ask their peers to store data using the `peer_storage` message & expect peers to return them latest data using `your_peer_storage` message: | ||
|
||
1. type: 7 (`peer_storage`) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IOKTBO definitely applies to all messages, its just in the very initial protocol there's no distinction cause everyone supports everything :). Its definitely nice for these to be odd because then nodes can send peer_storage
without bothering to check if their peer supports the feature at all.
- MAY store anyway. | ||
|
||
- If it does store the message: | ||
- MAY delay storage to ratelimit peer to no more than one update per second. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it the IOPS themselves that will destroy the SSD
Yes, over one year 64KiB/second is 2 TB, which is more than most consumer SSD's write lifetimes.
rate limiting to 1 update per htlc
that's even worse :)
ISTM a rate-limit of one/minute makes a lot more sense.
This pull request proposes peer storage, inspired by @t-bast's peer backup storage. We create two separate messages, which users can use to distribute encrypted 'blobs' of up to 64 KB to their peers.
Users can distribute a 'blob' and update it whenever their configuration changes by sending a 'peer_storage' message. In return, they will always receive a 'your_peer_storage' message, which will serve as an acknowledgment.
Nodes can also decide whether they want to provide this service in exchange for some sats.
You can check out the implementation in CLN here
Thank you so much, @rustyrussell, for all the help and guidance!