-
Notifications
You must be signed in to change notification settings - Fork 417
Peer Storage (Part 3): Identifying Lost Channel States #3897
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
base: main
Are you sure you want to change the base?
Changes from all commits
a6f1785
9a0ef54
8d82fd3
4c9f3c3
006a5d0
27303ec
676afbc
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -29,15 +29,17 @@ use bitcoin::hash_types::{BlockHash, Txid}; | |
use crate::chain; | ||
use crate::chain::chaininterface::{BroadcasterInterface, FeeEstimator}; | ||
use crate::chain::channelmonitor::{ | ||
Balance, ChannelMonitor, ChannelMonitorUpdate, MonitorEvent, TransactionOutputs, | ||
write_util, Balance, ChannelMonitor, ChannelMonitorUpdate, MonitorEvent, TransactionOutputs, | ||
WithChannelMonitor, | ||
}; | ||
use crate::chain::transaction::{OutPoint, TransactionData}; | ||
use crate::chain::{ChannelMonitorUpdateStatus, Filter, WatchedOutput}; | ||
use crate::events::{self, Event, EventHandler, ReplayEvent}; | ||
use crate::ln::channel_state::ChannelDetails; | ||
use crate::ln::msgs::{self, BaseMessageHandler, Init, MessageSendEvent}; | ||
use crate::ln::our_peer_storage::DecryptedOurPeerStorage; | ||
use crate::ln::our_peer_storage::{ | ||
DecryptedOurPeerStorage, PeerStorageMonitorHolder, PeerStorageMonitorHolderList, | ||
}; | ||
use crate::ln::types::ChannelId; | ||
use crate::prelude::*; | ||
use crate::sign::ecdsa::EcdsaChannelSigner; | ||
|
@@ -47,6 +49,7 @@ use crate::types::features::{InitFeatures, NodeFeatures}; | |
use crate::util::errors::APIError; | ||
use crate::util::logger::{Logger, WithContext}; | ||
use crate::util::persist::MonitorName; | ||
use crate::util::ser::{VecWriter, Writeable}; | ||
use crate::util::wakers::{Future, Notifier}; | ||
use bitcoin::secp256k1::PublicKey; | ||
use core::ops::Deref; | ||
|
@@ -810,10 +813,53 @@ where | |
} | ||
|
||
fn send_peer_storage(&self, their_node_id: PublicKey) { | ||
// TODO: Serialize `ChannelMonitor`s inside `our_peer_storage`. | ||
|
||
static MAX_PEER_STORAGE_SIZE: usize = 65000; | ||
let random_bytes = self.entropy_source.get_secure_random_bytes(); | ||
let serialised_channels = Vec::new(); | ||
let random_usize = usize::from_le_bytes(random_bytes[0..8].try_into().unwrap()); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Depending on the platform, a
and use that instead of |
||
|
||
let monitors = self.monitors.read().unwrap(); | ||
let mut monitors_list = PeerStorageMonitorHolderList { monitors: Vec::new() }; | ||
let mut curr_size = 0; | ||
|
||
// Randomising Keys in the HashMap to fetch monitors without repetition. | ||
let mut keys: Vec<&ChannelId> = monitors.keys().collect(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we make this a bit cleaner by using the proposed iterator |
||
for i in (1..keys.len()).rev() { | ||
let j = random_usize % (i + 1); | ||
keys.swap(i, j); | ||
} | ||
|
||
for chan_id in keys { | ||
let mon = &monitors[chan_id]; | ||
let mut ser_chan = VecWriter(Vec::new()); | ||
let min_seen_secret = mon.monitor.get_min_seen_secret(); | ||
let counterparty_node_id = mon.monitor.get_counterparty_node_id(); | ||
|
||
match write_util(&mon.monitor.inner.lock().unwrap(), true, &mut ser_chan) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: Please move taking the lock out into a dedicated variable. This would also make it easier to spot the scoping of the lock, IMO. |
||
Ok(_) => { | ||
let mut ser_channel = Vec::new(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think instead of creating a new But I'm currently confused what we use |
||
let peer_storage_monitor = PeerStorageMonitorHolder { | ||
channel_id: *chan_id, | ||
min_seen_secret, | ||
counterparty_node_id, | ||
monitor_bytes: ser_chan.0, | ||
}; | ||
peer_storage_monitor.write(&mut ser_channel).unwrap(); | ||
|
||
curr_size += ser_channel.len(); | ||
if curr_size > MAX_PEER_STORAGE_SIZE { | ||
break; | ||
} | ||
|
||
monitors_list.monitors.push(peer_storage_monitor); | ||
}, | ||
Err(_) => { | ||
panic!("Can not write monitor for {}", mon.monitor.channel_id()) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Really, please avoid these explicit panics in any of this code. |
||
}, | ||
} | ||
} | ||
|
||
let mut serialised_channels = Vec::new(); | ||
monitors_list.write(&mut serialised_channels).unwrap(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same here, just use |
||
let our_peer_storage = DecryptedOurPeerStorage::new(serialised_channels); | ||
let cipher = our_peer_storage.encrypt(&self.our_peerstorage_encryption_key, &random_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.
This should be a
const
rather thanstatic
, I think? Also, would probably make sense to add this add the module level, with some docs.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.
Also isn't the max size 64 KiB, not 65K?