smite: extend accept_channel oracle with sanity and key reuse checks

smite-scenarios/src/executor.rs

@@ -251,6 +251,9 @@ pub struct Executor<C, B, R> {
     /// `temporary_channel_id`, so the funding flow can build commitments from
     /// the parameters actually sent on the wire.
     negotiations: HashMap<TemporaryChannelId, PendingChannel>,
+    /// Per-commitment points revealed by either us or the target, used to
+    /// detect points revealed more than once by the target.
+    per_commitment_points: HashSet<PublicKey>,
     /// Transactions stored outside Bitcoin Core's mempool, typically because they
     /// were rejected by mempool policy, to be included in the next `MineBlocks`
     /// operation. Each is stored as `(txid, raw_hex)`: re-signing the same
@@ -277,6 +280,7 @@ impl<C: Connection, B: BitcoinRpc, R: TargetRpc> Executor<C, B, R> {
             context,
             channel_states: HashMap::new(),
             negotiations: HashMap::new(),
+            per_commitment_points: HashSet::new(),
             private_mempool: Vec::new(),
             unmined_txids: HashSet::new(),
             mined_txids: HashSet::new(),
@@ -434,7 +438,11 @@ impl<C: Connection, B: BitcoinRpc, R: TargetRpc> Executor<C, B, R> {
 
                 Operation::SendOpenChannel => {
                     let oc = resolve_open_channel_message(&variables, instr.inputs[0]);
-                    record_send_open_channel(&mut self.negotiations, oc);
+                    record_send_open_channel(
+                        &mut self.negotiations,
+                        &mut self.per_commitment_points,
+                        oc,
+                    );
                     let encoded = Message::OpenChannel(oc.clone()).encode();
                     log::debug!(
                         "[{:?}] SendOpenChannel: {} bytes",
@@ -469,6 +477,7 @@ impl<C: Connection, B: BitcoinRpc, R: TargetRpc> Executor<C, B, R> {
                         &instr.inputs,
                         *include_alias,
                         &mut self.channel_states,
+                        &mut self.per_commitment_points,
                     );
                     let encoded = Message::ChannelReady(cr).encode();
                     log::debug!(
@@ -505,8 +514,13 @@ impl<C: Connection, B: BitcoinRpc, R: TargetRpc> Executor<C, B, R> {
                         accept_channel: &ac,
                         negotiation: self.negotiations.get(&ac.temporary_channel_id),
                         negotiated_features: &self.context.negotiated_features,
+                        per_commitment_points: &self.per_commitment_points,
                     })?;
-                    record_recv_accept_channel(&mut self.negotiations, &ac);
+                    record_recv_accept_channel(
+                        &mut self.negotiations,
+                        &mut self.per_commitment_points,
+                        &ac,
+                    );
                     Some(Variable::AcceptChannel(ac))
                 }
 
@@ -526,7 +540,11 @@ impl<C: Connection, B: BitcoinRpc, R: TargetRpc> Executor<C, B, R> {
                 Operation::RecvChannelReady => {
                     if is_channel_ready_expected(&self.channel_states, &mut self.bitcoin_cli) {
                         log::debug!("[{:?}] RecvChannelReady: waiting", start.elapsed());
-                        recv_channel_ready(&mut self.conn, &mut self.channel_states)?;
+                        recv_channel_ready(
+                            &mut self.conn,
+                            &mut self.channel_states,
+                            &mut self.per_commitment_points,
+                        )?;
                         log::debug!("[{:?}] RecvChannelReady: received", start.elapsed());
                     }
                     None
@@ -905,6 +923,7 @@ fn build_channel_ready(
     inputs: &[usize],
     include_alias: bool,
     channel_states: &mut HashMap<ChannelId, ChannelState>,
+    per_commitment_points: &mut HashSet<PublicKey>,
 ) -> ChannelReady {
     let channel_id = resolve_channel_id(variables, inputs[0]);
     let second_per_commitment_point = resolve_pubkey(variables, inputs[1]);
@@ -916,12 +935,15 @@ fn build_channel_ready(
     // yet recorded: `channel_ready` may be resent, but BOLT peers ignore
     // redundant ones, so recording a resend would leave us with the wrong point
     // and make us reject a valid received commitment signature as invalid.
+    //
+    // The same point is added to `per_commitment_points` as revealed by us.
     if let Some(state) = channel_states.get_mut(&channel_id)
         && state.commitment.commitment_number == 0
     {
         let next_point = state.next_holder_per_commitment_point_mut();
         if next_point.is_none() {
             *next_point = Some(second_per_commitment_point);
+            per_commitment_points.insert(second_per_commitment_point);
         }
     }
 
@@ -1189,7 +1211,8 @@ fn recv_bolt<M: FromMessage>(
 /// Receives and decodes a `channel_ready` message.
 ///
 /// The `second_per_commitment_point` is recorded as the counterparty's next
-/// per-commitment point on the channel it identifies.
+/// per-commitment point on the channel it identifies, and added to
+/// `per_commitment_points` as revealed by the target.
 ///
 /// # Errors
 ///
@@ -1199,6 +1222,7 @@ fn recv_bolt<M: FromMessage>(
 fn recv_channel_ready(
     conn: &mut impl Connection,
     channel_states: &mut HashMap<ChannelId, ChannelState>,
+    per_commitment_points: &mut HashSet<PublicKey>,
 ) -> Result<(), ExecuteError> {
     let cr: ChannelReady = recv_bolt(conn, RECV_CHANNEL_READY_TIMEOUT)?;
 
@@ -1207,6 +1231,8 @@ fn recv_channel_ready(
         .ok_or(Violation::UnknownChannel(cr.channel_id))?;
     *state.next_counterparty_per_commitment_point_mut() = Some(cr.second_per_commitment_point);
 
+    per_commitment_points.insert(cr.second_per_commitment_point);
+
     Ok(())
 }
 
@@ -1262,8 +1288,12 @@ fn verify_funding_signed(
 /// it is left untouched, preserving the first `open_channel`. Once a
 /// `funding_created` has been built, it is overwritten, allowing the
 /// `temporary_channel_id` to be reused for a new negotiation.
+///
+/// Whenever a negotiation is recorded, its `first_per_commitment_point` is
+/// added to `per_commitment_points` as revealed by us.
 fn record_send_open_channel(
     negotiations: &mut HashMap<TemporaryChannelId, PendingChannel>,
+    per_commitment_points: &mut HashSet<PublicKey>,
     open_channel: &OpenChannel,
 ) {
     if negotiations
@@ -1281,23 +1311,27 @@ fn record_send_open_channel(
             funding_built: false,
         },
     );
+    per_commitment_points.insert(open_channel.first_per_commitment_point);
 }
 
 /// Pairs a received `accept_channel` with the recorded `open_channel` of the
-/// same `temporary_channel_id`.
+/// same `temporary_channel_id`, and adds its `first_per_commitment_point` to
+/// `per_commitment_points` as revealed by the target.
 ///
 /// # Panics
 ///
 /// Panics if no matching `open_channel` exists. This should be unreachable, as
 /// `AcceptChannelOracle` reports such messages as a [`Violation`].
 fn record_recv_accept_channel(
     negotiations: &mut HashMap<TemporaryChannelId, PendingChannel>,
+    per_commitment_points: &mut HashSet<PublicKey>,
     accept_channel: &AcceptChannel,
 ) {
     negotiations
         .get_mut(&accept_channel.temporary_channel_id)
         .expect("AcceptChannelOracle guaranteed this temporary_channel_id exists")
         .accept_channel = Some(accept_channel.clone());
+    per_commitment_points.insert(accept_channel.first_per_commitment_point);
 }
 
 /// Extracts a field from a parsed `accept_channel` message.

smite-scenarios/src/executor/tests.rs

@@ -422,6 +422,13 @@ fn execute_records_negotiation_for_open_and_accept() {
     let accept_channel = pending.accept_channel.as_ref().unwrap();
     assert_eq!(accept_channel.clone(), sample_accept_channel());
     assert!(!pending.funding_built);
+    assert_eq!(
+        *fx.per_commitment_points(),
+        HashSet::from([
+            pending.open_channel.first_per_commitment_point,
+            accept_channel.first_per_commitment_point,
+        ])
+    );
 }
 
 #[test]
@@ -484,9 +491,18 @@ fn execute_recv_accept_channel_rejects_reuse_before_funding() {
     );
     b.append(Operation::RecvAcceptChannel, &[resent]);
 
+    // Use a fresh `first_per_commitment_point` so the resent `accept_channel`
+    // is otherwise valid, with only its `temporary_channel_id` reused before
+    // funding_created.
+    let accept_channel = sample_accept_channel();
+    let resent_accept_channel = AcceptChannel {
+        first_per_commitment_point: sample_pubkey(8),
+        ..sample_accept_channel()
+    };
+
     let err = Fixture::new()
-        .queue(&Message::AcceptChannel(sample_accept_channel()))
-        .queue(&Message::AcceptChannel(sample_accept_channel()))
+        .queue(&Message::AcceptChannel(accept_channel))
+        .queue(&Message::AcceptChannel(resent_accept_channel))
         .run_err(&b.build());
 
     let ExecuteError::Violation(Violation::InvalidAcceptChannel(id, reason)) = &err else {
@@ -498,18 +514,57 @@ fn execute_recv_accept_channel_rejects_reuse_before_funding() {
     ));
 }
 
+#[test]
+fn execute_recv_accept_channel_rejects_reused_per_commitment_point() {
+    let temporary_channel_id = TemporaryChannelId::new([0xcc; 32]);
+
+    // Negotiate a channel, then negotiate a second one on a different
+    // `temporary_channel_id`.
+    let mut b = ProgramBuilder::new();
+    negotiate_channel(&mut b, &announced_open_channel());
+    let mut second_open_channel = announced_open_channel();
+    second_open_channel.message.temporary_channel_id = temporary_channel_id;
+    negotiate_channel(&mut b, &second_open_channel);
+
+    // Use the first `accept_channel`'s `first_per_commitment_point` so the second
+    // `accept_channel` is otherwise valid, with only its
+    // `first_per_commitment_point` reused.
+    let earlier_point = sample_accept_channel().first_per_commitment_point;
+    let second_accept_channel = AcceptChannel {
+        temporary_channel_id,
+        ..sample_accept_channel()
+    };
+
+    let err = Fixture::new()
+        .queue(&Message::AcceptChannel(sample_accept_channel()))
+        .queue(&Message::AcceptChannel(second_accept_channel))
+        .run_err(&b.build());
+
+    let ExecuteError::Violation(Violation::InvalidAcceptChannel(id, reason)) = &err else {
+        panic!("unexpected error: {err:?}");
+    };
+    assert_eq!(*id, temporary_channel_id);
+    assert!(reason.contains(&format!(
+        "first_per_commitment_point {earlier_point} was reused from an earlier negotiation"
+    )));
+}
+
 #[test]
 fn execute_records_only_first_open_channel_for_duplicate_id_before_funding() {
     let temporary_channel_id = TemporaryChannelId::new([0xbb; 32]);
 
     // First open_channel: funding_satoshis = 100_000.
-    // Second open_channel: same temporary_channel_id, funding_satoshis = 200_000.
+    // Second open_channel: same temporary_channel_id, funding_satoshis = 200_000,
+    // and a fresh first_per_commitment_point.
     let mut b = ProgramBuilder::new();
     let first = send_open_channel(&mut b, &announced_open_channel());
 
-    // Override only funding_satoshis; reuse the first open_channel's other 19 inputs.
+    // Override only funding_satoshis and first_per_commitment_point; reuse the
+    // first open_channel's other 18 inputs.
     let mut second = first.vars;
     second.funding_satoshis = b.append(Operation::LoadAmount(200_000), &[]);
+    let sk = b.append(Operation::LoadPrivateKey([0x11; 32]), &[]);
+    second.first_per_commitment_point = b.append(Operation::DerivePoint, &[sk]);
     second.built = b.append(Operation::BuildOpenChannel, &second.build_inputs());
     b.append(Operation::SendOpenChannel, &[second.built]);
 
@@ -523,6 +578,18 @@ fn execute_records_only_first_open_channel_for_duplicate_id_before_funding() {
     assert_eq!(fx.sent::<OpenChannel>(1).funding_satoshis, 200_000);
     let pending = fx.negotiation(&temporary_channel_id);
     assert_eq!(pending.open_channel.funding_satoshis, 100_000);
+
+    // The two `open_channel`s went out with different
+    // `first_per_commitment_point`s, but only the recorded negotiation's point
+    // counts as revealed by us.
+    assert_ne!(
+        fx.sent::<OpenChannel>(0).first_per_commitment_point,
+        fx.sent::<OpenChannel>(1).first_per_commitment_point,
+    );
+    assert_eq!(
+        *fx.per_commitment_points(),
+        HashSet::from([fx.sent::<OpenChannel>(0).first_per_commitment_point])
+    );
 }
 
 #[test]
@@ -542,6 +609,12 @@ fn execute_records_open_channel_for_duplicate_id_after_funding() {
     assert_eq!(pending.open_channel.funding_satoshis, 100_000);
     assert!(pending.accept_channel.is_none());
     assert!(!pending.funding_built);
+    // The earlier negotiation was seeded rather than executed, so only the
+    // new `open_channel`'s point is recorded.
+    assert_eq!(
+        *fx.per_commitment_points(),
+        HashSet::from([pending.open_channel.first_per_commitment_point])
+    );
 }
 
 // -- Panic path tests --
@@ -1089,6 +1162,7 @@ fn execute_send_channel_ready() {
         *state.next_holder_per_commitment_point(),
         Some(expected_pcp1)
     );
+    assert_eq!(*fx.per_commitment_points(), HashSet::from([expected_pcp1]));
 }
 
 #[test]
@@ -1161,6 +1235,7 @@ fn execute_recv_channel_ready_invalid_funding_outpoint_is_noop() {
     let state = fx.channel_state(&funding_channel_id());
     assert!(state.next_counterparty_per_commitment_point().is_none());
     assert_eq!(fx.queued_len(), 1);
+    assert!(fx.per_commitment_points().is_empty());
 }
 
 #[test]
@@ -1180,6 +1255,7 @@ fn execute_recv_channel_ready_below_minimum_depth_is_noop() {
     let state = fx.channel_state(&funding_channel_id());
     assert!(state.next_counterparty_per_commitment_point().is_none());
     assert_eq!(fx.queued_len(), 1);
+    assert!(fx.per_commitment_points().is_empty());
 }
 
 #[test]
@@ -1201,6 +1277,7 @@ fn execute_recv_channel_ready_at_minimum_depth_records_point() {
         Some(target_pcp)
     );
     assert_eq!(fx.queued_len(), 0);
+    assert_eq!(*fx.per_commitment_points(), HashSet::from([target_pcp]));
 }
 
 #[test]
@@ -1228,6 +1305,7 @@ fn execute_recv_channel_ready_funding_mined_prematurely_is_noop() {
     assert!(state.was_funding_mined_prematurely);
     assert!(state.next_counterparty_per_commitment_point().is_none());
     assert_eq!(fx.queued_len(), 1);
+    assert!(fx.per_commitment_points().is_empty());
 }
 
 // -- extract_field tests --
@@ -1286,7 +1364,7 @@ fn extract_pubkeys() {
     let ac = sample_accept_channel();
     assert_eq!(
         extract_field(&ac, AcceptChannelField::FundingPubkey),
-        Variable::Point(sample_pubkey(1))
+        Variable::Point(sample_pubkey(7))
     );
     assert_eq!(
         extract_field(&ac, AcceptChannelField::RevocationBasepoint),

smite-scenarios/src/executor/tests/harness.rs

@@ -2,7 +2,7 @@
 
 use crate::executor::*;
 use bitcoin::{Amount, Transaction};
-use smite::bolt::{AcceptChannelTlvs, ChannelTypeVariant, FromMessage};
+use smite::bolt::{AcceptChannelTlvs, ChannelTypeVariant, FromMessage, REGTEST_CHAIN_HASH};
 use std::collections::VecDeque;
 use std::str::FromStr;
 
@@ -215,6 +215,11 @@ impl Fixture {
         &self.executor.rpc
     }
 
+    /// Returns the per-commitment points revealed by either us or the target.
+    pub fn per_commitment_points(&self) -> &HashSet<PublicKey> {
+        &self.executor.per_commitment_points
+    }
+
     /// Returns the transactions held outside Bitcoin Core's mempool.
     pub fn private_mempool(&self) -> &[(Txid, String)] {
         &self.executor.private_mempool
@@ -254,7 +259,7 @@ pub fn sample_pubkey(byte: u8) -> PublicKey {
 pub fn sample_context() -> ProgramContext {
     ProgramContext {
         target_pubkey: sample_pubkey(1),
-        chain_hash: [0xcc; 32],
+        chain_hash: REGTEST_CHAIN_HASH,
         block_height: 800_000,
         negotiated_features: Features::from_bits(&[
             Features::OPTION_STATIC_REMOTEKEY,
@@ -296,7 +301,7 @@ pub fn sample_accept_channel() -> AcceptChannel {
         minimum_depth: 6,
         to_self_delay: 144,
         max_accepted_htlcs: 483,
-        funding_pubkey: sample_pubkey(1),
+        funding_pubkey: sample_pubkey(7),
         revocation_basepoint: sample_pubkey(2),
         payment_basepoint: sample_pubkey(3),
         delayed_payment_basepoint: sample_pubkey(4),
@@ -467,7 +472,7 @@ pub fn sample_funding_negotiation() -> PendingChannel {
 
     PendingChannel {
         open_channel: OpenChannel {
-            chain_hash: [0xcc; 32],
+            chain_hash: REGTEST_CHAIN_HASH,
             temporary_channel_id: TemporaryChannelId::new([0xbb; 32]),
             funding_satoshis: 10_000_000,
             push_msat: 3_000_000_000,

smite-scenarios/src/scenarios.rs

@@ -10,7 +10,7 @@ pub use encrypted_bytes::EncryptedBytesScenario;
 pub use init::InitScenario;
 pub use ir::IrScenario;
 pub use noise::NoiseScenario;
-pub use setup::{PostInitSetup, REGTEST_CHAIN_HASH, SnapshotSetup};
+pub use setup::{PostInitSetup, SnapshotSetup};
 use smite::scenarios::ScenarioError;
 
 use std::time::Duration;