Skip to content
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 Feature – Part 2 #3623

Open
wants to merge 23 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
b133e80
Add method to derive Peer Storage encryption key
Jan 17, 2025
3778b81
Add OurPeerStorage for serialized Peer Storage backups
adi2011 Feb 26, 2025
28d60ec
Enable ChainMonitor to distribute PeerStorage
Jan 18, 2025
ebefcd1
Distribute PeerStorage from ChainMonitor
Jan 19, 2025
65aee4d
Handle PeerStorageRetrieval in ChannelManager
Jan 19, 2025
35863ce
test: update test_peer_storage to validate latest changes
adi2011 Mar 7, 2025
777f6b1
fixup: Add OurPeerStorage for serialized Peer Storage backups
adi2011 Mar 10, 2025
c3a3859
fixup: Add method to derive Peer Storage encryption key
adi2011 Mar 10, 2025
ef2b8a6
fixup: Add to Enable ChainMonitor to distribute PeerStorage
adi2011 Mar 10, 2025
162b6c4
fixup: Add method to derive Peer Storage encryption key
adi2011 Mar 10, 2025
5a881a3
fixup: Add OurPeerStorage for serialized Peer Storage backups
adi2011 Mar 11, 2025
30d9068
fixup: Add method to derive Peer Storage encryption key
adi2011 Mar 11, 2025
5deb6ad
fixup: Distribute PeerStorage from ChainMonitor
adi2011 Mar 11, 2025
569caa4
fixup: Enable ChainMonitor to distribute PeerStorage
adi2011 Mar 11, 2025
58affb5
fixup: Handle PeerStorageRetrieval in ChannelManager
adi2011 Mar 11, 2025
4730cdc
fixup: Enable ChainMonitor to distribute PeerStorage
adi2011 Mar 11, 2025
5ef58e9
fixup: Add method to derive Peer Storage encryption key
adi2011 Mar 11, 2025
f6d6204
fixup: Add method to derive Peer Storage encryption key
adi2011 Mar 11, 2025
57341a1
fixup: Add method to derive Peer Storage encryption key
adi2011 Mar 11, 2025
42af2ab
fixup: Add OurPeerStorage for serialized Peer Storage backups
adi2011 Mar 11, 2025
0fd40de
fixup: Distribute PeerStorage from ChainMonitor
adi2011 Mar 11, 2025
0c10bd5
fixup: Handle PeerStorageRetrieval in ChannelManager
adi2011 Mar 11, 2025
4adeeee
fixup: Add OurPeerStorage for serialized Peer Storage backups
adi2011 Mar 12, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions fuzz/src/chanmon_consistency.rs
Original file line number Diff line number Diff line change
Expand Up @@ -337,6 +337,10 @@ impl NodeSigner for KeyProvider {
unreachable!()
}

fn get_peer_storage_key(&self) -> [u8; 32] {
[0; 32]
}

fn sign_bolt12_invoice(
&self, _invoice: &UnsignedBolt12Invoice,
) -> Result<schnorr::Signature, ()> {
Expand Down
18 changes: 12 additions & 6 deletions fuzz/src/full_stack.rs
Original file line number Diff line number Diff line change
Expand Up @@ -420,6 +420,10 @@ impl NodeSigner for KeyProvider {
let secp_ctx = Secp256k1::signing_only();
Ok(secp_ctx.sign_ecdsa(&msg_hash, &self.node_secret))
}

fn get_peer_storage_key(&self) -> [u8; 32] {
[0; 32]
}
}

impl SignerProvider for KeyProvider {
Expand Down Expand Up @@ -608,6 +612,14 @@ pub fn do_test(mut data: &[u8], logger: &Arc<dyn Logger>) {
];

let broadcast = Arc::new(TestBroadcaster { txn_broadcasted: Mutex::new(Vec::new()) });

let keys_manager = Arc::new(KeyProvider {
node_secret: our_network_key.clone(),
inbound_payment_key: ExpandedKey::new(inbound_payment_key),
counter: AtomicU64::new(0),
signer_state: RefCell::new(new_hash_map()),
});

let monitor = Arc::new(chainmonitor::ChainMonitor::new(
None,
broadcast.clone(),
Expand All @@ -616,12 +628,6 @@ pub fn do_test(mut data: &[u8], logger: &Arc<dyn Logger>) {
Arc::new(TestPersister { update_ret: Mutex::new(ChannelMonitorUpdateStatus::Completed) }),
));

let keys_manager = Arc::new(KeyProvider {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not quite sure why this code move is necessary?

Copy link
Author

@adi2011 adi2011 Mar 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So that we can use keys_manager.get_peer_storage_key() inside ChainMonitor::new().

node_secret: our_network_key.clone(),
inbound_payment_key: ExpandedKey::new(inbound_payment_key),
counter: AtomicU64::new(0),
signer_state: RefCell::new(new_hash_map()),
});
let network = Network::Bitcoin;
let best_block_timestamp = genesis_block(network).header.time;
let params = ChainParameters { network, best_block: BestBlock::from_network(network) };
Expand Down
4 changes: 4 additions & 0 deletions fuzz/src/onion_message.rs
Original file line number Diff line number Diff line change
Expand Up @@ -246,6 +246,10 @@ impl NodeSigner for KeyProvider {
) -> Result<bitcoin::secp256k1::ecdsa::Signature, ()> {
unreachable!()
}

fn get_peer_storage_key(&self) -> [u8; 32] {
unreachable!()
}
}

impl SignerProvider for KeyProvider {
Expand Down
1 change: 1 addition & 0 deletions lightning/src/ln/blinded_payment_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1609,6 +1609,7 @@ fn route_blinding_spec_test_vector() {
fn sign_invoice(
&self, _invoice: &RawBolt11Invoice, _recipient: Recipient,
) -> Result<RecoverableSignature, ()> { unreachable!() }
fn get_peer_storage_key(&self) -> [u8;32] { unreachable!() }
fn sign_bolt12_invoice(
&self, _invoice: &UnsignedBolt12Invoice,
) -> Result<schnorr::Signature, ()> { unreachable!() }
Expand Down
4 changes: 2 additions & 2 deletions lightning/src/ln/channelmanager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16386,19 +16386,19 @@ pub mod bench {
config.channel_config.max_dust_htlc_exposure = MaxDustHTLCExposure::FeeRateMultiplier(5_000_000 / 253);
config.channel_handshake_config.minimum_depth = 1;

let chain_monitor_a = ChainMonitor::new(None, &tx_broadcaster, &logger_a, &fee_estimator, &persister_a);
let seed_a = [1u8; 32];
let keys_manager_a = KeysManager::new(&seed_a, 42, 42);
let chain_monitor_a = ChainMonitor::new(None, &tx_broadcaster, &logger_a, &fee_estimator, &persister_a, keys_manager_a.get_peer_storage_key());
let node_a = ChannelManager::new(&fee_estimator, &chain_monitor_a, &tx_broadcaster, &router, &message_router, &logger_a, &keys_manager_a, &keys_manager_a, &keys_manager_a, config.clone(), ChainParameters {
network,
best_block: BestBlock::from_network(network),
}, genesis_block.header.time);
let node_a_holder = ANodeHolder { node: &node_a };

let logger_b = test_utils::TestLogger::with_id("node a".to_owned());
let chain_monitor_b = ChainMonitor::new(None, &tx_broadcaster, &logger_a, &fee_estimator, &persister_b);
let seed_b = [2u8; 32];
let keys_manager_b = KeysManager::new(&seed_b, 42, 42);
let chain_monitor_b = ChainMonitor::new(None, &tx_broadcaster, &logger_a, &fee_estimator, &persister_b, keys_manager_b.get_peer_storage_key());
let node_b = ChannelManager::new(&fee_estimator, &chain_monitor_b, &tx_broadcaster, &router, &message_router, &logger_b, &keys_manager_b, &keys_manager_b, &keys_manager_b, config.clone(), ChainParameters {
network,
best_block: BestBlock::from_network(network),
Expand Down
39 changes: 39 additions & 0 deletions lightning/src/sign/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -829,6 +829,30 @@ pub trait NodeSigner {
/// [phantom node payments]: PhantomKeysManager
fn get_inbound_payment_key(&self) -> ExpandedKey;

/// Defines a method to derive a 32-byte encryption key for peer storage.
///
/// Implementations of this method must derive a secure encryption key using a
/// cryptographically strong key derivation function (e.g., HKDF or a hardened
/// child key derivation). The derived key is used to encrypt or decrypt peer
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could link BIP32 here.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, Thanks!

/// storage data, ensuring confidentiality and integrity.
///
/// # Implementation Details
///
/// - The key must be derived from a node-specific secret to ensure uniqueness.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure what a "node-specific secret" means. Honestly, I'm not sure any of the sections below the first paragraph are needed.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, on second thought, I agree it’s unnecessary.

/// - The derived key must be exactly **32 bytes** and suitable for symmetric
/// encryption algorithms.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is given by the return type, we don't have to specify it.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I wrote it just as a note to implementations. But, yeah, the return type specifies it. Removing it.

///
/// # Returns
///
/// A **32-byte array** representing the encryption key for peer storage.
///
/// # Usage
///
/// This method is invoked when encrypting or decrypting peer storage data.
/// It must return the same key every time it is called, ensuring consistency
/// for encryption and decryption operations.
fn get_peer_storage_key(&self) -> [u8; 32];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does this return a [u8; 32] rather than simply a SecretKey?

Copy link
Author

@adi2011 adi2011 Mar 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, fixing this...


/// Get node id based on the provided [`Recipient`].
///
/// This method must return the same value each time it is called with a given [`Recipient`]
Expand Down Expand Up @@ -1778,6 +1802,7 @@ pub struct KeysManager {
shutdown_pubkey: PublicKey,
channel_master_key: Xpriv,
channel_child_index: AtomicUsize,
peer_storage_key: SecretKey,

#[cfg(test)]
pub(crate) entropy_source: RandomBytes,
Expand Down Expand Up @@ -1846,6 +1871,10 @@ impl KeysManager {
.private_key;
let mut inbound_pmt_key_bytes = [0; 32];
inbound_pmt_key_bytes.copy_from_slice(&inbound_payment_key[..]);
let peer_storage_key: SecretKey = master_key
.derive_priv(&secp_ctx, &ChildNumber::from_hardened_idx(6).unwrap())
.expect("Your RNG is busted")
.private_key;

let mut rand_bytes_engine = Sha256::engine();
rand_bytes_engine.input(&starting_time_secs.to_be_bytes());
Expand All @@ -1861,6 +1890,8 @@ impl KeysManager {
node_id,
inbound_payment_key: ExpandedKey::new(inbound_pmt_key_bytes),

peer_storage_key,

destination_script,
shutdown_pubkey,

Expand Down Expand Up @@ -2086,6 +2117,10 @@ impl NodeSigner for KeysManager {
self.inbound_payment_key.clone()
}

fn get_peer_storage_key(&self) -> [u8; 32] {
self.peer_storage_key.secret_bytes()
}

fn sign_invoice(
&self, invoice: &RawBolt11Invoice, recipient: Recipient,
) -> Result<RecoverableSignature, ()> {
Expand Down Expand Up @@ -2247,6 +2282,10 @@ impl NodeSigner for PhantomKeysManager {
self.inbound_payment_key.clone()
}

fn get_peer_storage_key(&self) -> [u8; 32] {
self.inner.peer_storage_key.secret_bytes()
}

fn sign_invoice(
&self, invoice: &RawBolt11Invoice, recipient: Recipient,
) -> Result<RecoverableSignature, ()> {
Expand Down
6 changes: 4 additions & 2 deletions lightning/src/util/dyn_signer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,8 @@ inner,
fn sign_bolt12_invoice(,
invoice: &crate::offers::invoice::UnsignedBolt12Invoice
) -> Result<secp256k1::schnorr::Signature, ()>,
fn get_inbound_payment_key(,) -> ExpandedKey
fn get_inbound_payment_key(,) -> ExpandedKey,
fn get_peer_storage_key(,) -> [u8; 32]
);

delegate!(DynKeysInterface, SignerProvider,
Expand Down Expand Up @@ -288,7 +289,8 @@ delegate!(DynPhantomKeysInterface, NodeSigner,
fn sign_invoice(, invoice: &RawBolt11Invoice, recipient: Recipient) -> Result<RecoverableSignature, ()>,
fn sign_bolt12_invoice(, invoice: &crate::offers::invoice::UnsignedBolt12Invoice
) -> Result<secp256k1::schnorr::Signature, ()>,
fn get_inbound_payment_key(,) -> ExpandedKey
fn get_inbound_payment_key(,) -> ExpandedKey,
fn get_peer_storage_key(,) -> [u8; 32]
);

impl SignerProvider for DynPhantomKeysInterface {
Expand Down
8 changes: 8 additions & 0 deletions lightning/src/util/test_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1451,6 +1451,10 @@ impl NodeSigner for TestNodeSigner {
unreachable!()
}

fn get_peer_storage_key(&self) -> [u8; 32] {
unreachable!()
}

fn get_node_id(&self, recipient: Recipient) -> Result<PublicKey, ()> {
let node_secret = match recipient {
Recipient::Node => Ok(&self.node_secret),
Expand Down Expand Up @@ -1529,6 +1533,10 @@ impl NodeSigner for TestKeysInterface {
self.backing.sign_invoice(invoice, recipient)
}

fn get_peer_storage_key(&self) -> [u8; 32] {
self.backing.get_peer_storage_key()
}

fn sign_bolt12_invoice(
&self, invoice: &UnsignedBolt12Invoice,
) -> Result<schnorr::Signature, ()> {
Expand Down