diff --git a/lightning/src/chain/chainmonitor.rs b/lightning/src/chain/chainmonitor.rs index b3b69096997..fffe9668159 100644 --- a/lightning/src/chain/chainmonitor.rs +++ b/lightning/src/chain/chainmonitor.rs @@ -986,6 +986,12 @@ where let random_bytes = self._entropy_source.get_secure_random_bytes(); const MAX_PEER_STORAGE_SIZE: usize = 65531; + // The overhead between the sum of `serialized_length()` for the included monitors and + // the final `PeerStorage{}.data` wire size. This accounts for: + // - 2 bytes: CollectionLength vec length prefix from `encode()` + // - 16 bytes: ChaCha20Poly1305 AEAD tag + // - 32 bytes: random_bytes nonce seed appended during encryption + const PEER_STORAGE_OVERHEAD: usize = 2 + 16 + 32; const USIZE_LEN: usize = core::mem::size_of::(); let mut random_bytes_cycle_iter = random_bytes.iter().cycle(); @@ -1042,7 +1048,7 @@ where let serialized_length = peer_storage_monitor.serialized_length(); - if current_size + serialized_length > MAX_PEER_STORAGE_SIZE { + if current_size + serialized_length > MAX_PEER_STORAGE_SIZE - PEER_STORAGE_OVERHEAD { continue; } else { current_size += serialized_length; @@ -2215,4 +2221,85 @@ mod tests { assert_eq!(chain_monitor_a.pending_operation_count(), 0); assert_eq!(chain_monitor_b.pending_operation_count(), 0); } + + /// Regression test for peer-storage blob size accounting. + /// + /// `send_peer_storage` caps the *plaintext* payload at `MAX_PEER_STORAGE_SIZE` (65531), but the + /// message put on the wire is the *encrypted* blob, which is ~50 bytes larger: a 2-byte + /// CollectionLength vec prefix, a 16-byte ChaCha20Poly1305 AEAD tag, and a 32-byte nonce seed. + /// Without accounting for this overhead, the emitted `PeerStorage` message can exceed BOLT #1's + /// 65531-byte limit (and even BOLT #8's 65535-byte transport limit, which would panic the node + /// in `peer_channel_encryptor`). + #[test] + #[cfg(peer_storage)] + fn test_peer_storage_blob_within_size_limit() { + const MAX_PEER_STORAGE_SIZE: usize = 65531; + + let broadcaster = TestBroadcaster::new(Network::Testnet); + let fee_est = TestFeeEstimator::new(253); + let logger = TestLogger::new(); + let persister = TestPersister::new(); + let chain_source = TestChainSource::new(Network::Testnet); + let keys = TestKeysInterface::new(&[0; 32], Network::Testnet); + + let chain_monitor = ChainMonitor::new( + Some(&chain_source), + &broadcaster, + &logger, + &fee_est, + &persister, + &keys, + keys.get_peer_storage_key(), + false, + ); + + // Create enough monitors to stress the peer storage size limit such that the total + // plaintext serialized_length() exceeds MAX_PEER_STORAGE_SIZE, ensuring the selection + // loop actually hits the cap. 600 monitors at ~300 bytes each = ~180K, well above the + // 65531 max, so the encrypted output will be forced to the boundary regardless of + // the random selection order. This guarantees the test catches the difference when + // PEER_STORAGE_OVERHEAD is not accounted for. + let num_monitors = 600; + let mut counterparty_id = None; + for i in 0..num_monitors { + let mut chan_id = [0u8; 32]; + chan_id[0] = (i / 256) as u8; + chan_id[1] = (i % 256) as u8; + let chan = ChannelId::from_bytes(chan_id); + let monitor = crate::chain::channelmonitor::dummy_monitor(chan, |keys| { + TestChannelSigner::new(DynSigner::new(keys)) + }); + if i == 0 { + counterparty_id = Some(monitor.get_counterparty_node_id()); + } + assert!(Watch::watch_channel(&chain_monitor, chan, monitor).is_ok()); + } + + // Trigger send_peer_storage for the first monitor's counterparty. This internally + // iterates over monitors and should cap the total encrypted size. + let node_id = counterparty_id.unwrap(); + chain_monitor.send_peer_storage(node_id); + + let msg_events = chain_monitor.get_and_clear_pending_msg_events(); + let mut saw_peer_storage = false; + for ev in msg_events { + if let MessageSendEvent::SendPeerStorage { msg, .. } = ev { + saw_peer_storage = true; + assert!( + msg.data.len() <= MAX_PEER_STORAGE_SIZE, + "peer_storage blob length {} exceeds BOLT #1 limit of {}", + msg.data.len(), + MAX_PEER_STORAGE_SIZE, + ); + assert!( + msg.data.len() > MAX_PEER_STORAGE_SIZE - 500, + "peer_storage blob length {} is too far from the {} byte limit, \ + boundary was not meaningfully tested", + msg.data.len(), + MAX_PEER_STORAGE_SIZE, + ); + } + } + assert!(saw_peer_storage, "expected a SendPeerStorage message"); + } } diff --git a/lightning/src/chain/channelmonitor.rs b/lightning/src/chain/channelmonitor.rs index a2412bbaf5e..28d9888fd26 100644 --- a/lightning/src/chain/channelmonitor.rs +++ b/lightning/src/chain/channelmonitor.rs @@ -7017,7 +7017,7 @@ pub(super) fn dummy_monitor( let shutdown_script = crate::ln::script::ShutdownScript::new_p2wpkh_from_pubkey(dummy_key); let best_block = BlockLocator::from_network(Network::Testnet); let signer = wrap_signer(keys); - ChannelMonitor::new( + let monitor = ChannelMonitor::new( secp_ctx, signer, Some(shutdown_script.into_inner()), @@ -7031,7 +7031,13 @@ pub(super) fn dummy_monitor( dummy_key, channel_id, false, - ) + ); + // The production constructor sets current_counterparty_commitment_number to 1 << 48 as a + // sentinel meaning "counterparty commitment not yet received". This value (2^48) exceeds the + // 48-bit field in be48_to_array and causes a panic on serialization. Since dummy monitors + // may be serialized in tests, reset it to a valid value. + monitor.inner.lock().unwrap().current_counterparty_commitment_number = 0; + monitor } #[cfg(test)]