Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
179 changes: 124 additions & 55 deletions smite-scenarios/src/executor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,9 @@ use smite::channel_tx::{
build_funding_transaction,
};
use smite::noise::{ConnectionError, NoiseConnection};
use smite::oracles::{AcceptChannelContext, AcceptChannelOracle, Oracle};
use smite::oracles::{
AcceptChannelContext, AcceptChannelOracle, FundingSignedContext, FundingSignedOracle, Oracle,
};
use smite::pending_channel::PendingChannel;
use smite::violation::Violation;
use smite_ir::operation::AcceptChannelField;
Expand Down Expand Up @@ -499,7 +501,10 @@ impl<C: Connection, B: BitcoinRpc> Executor<C, B> {
log::debug!("[{:?}] RecvFundingSigned: waiting", start.elapsed());
let fs = recv_funding_signed(&mut self.conn)?;
log::debug!("[{:?}] RecvFundingSigned: received", start.elapsed());
verify_funding_signed(&fs, &self.channel_states)?;
FundingSignedOracle.evaluate(&FundingSignedContext {
funding_signed: &fs,
channel: self.channel_states.get(&fs.channel_id),
})?;
Some(Variable::ChannelId(fs.channel_id))
}

Expand Down Expand Up @@ -958,17 +963,6 @@ fn build_funding_created(
};
let signature = config.sign_counterparty_commitment(&state, &holder);

let channel_id = ChannelId::v1_from_funding_outpoint(config.funding_outpoint);

// Check whether the funding outpoint is valid and contains the negotiated
// amount and funding script. If not, there is a good chance the target will
// neither complete the funding flow nor send an error message.
let is_funding_outpoint_valid = funding_tx.matches_funding_output(
&open_channel.funding_pubkey,
&accept_channel.funding_pubkey,
open_channel.funding_satoshis,
);

// Only track a new channel when this negotiation has not built a
// `funding_created` yet. If it has, we are likely resending one for the
// same `temporary_channel_id` with a different outpoint, which the target
Expand All @@ -978,13 +972,30 @@ fn build_funding_created(
// This also means that building the same message again must not clobber a
// channel whose state has already been established (and possibly advanced).
if !pending.funding_built {
let channel_id = ChannelId::v1_from_funding_outpoint(config.funding_outpoint);

// Check whether the funding outpoint is valid and contains the
// negotiated amount and funding script. If not, there is a good chance
// the target will neither complete the funding flow nor send an error
// message.
let is_funding_outpoint_valid = funding_tx.matches_funding_output(
&open_channel.funding_pubkey,
&accept_channel.funding_pubkey,
open_channel.funding_satoshis,
);
// TODO: Once we support sending malformed signatures, update this state
// when constructing one so that the peer's acceptance can be detected
// as a violation.
let sent_invalid_signature = opener_funding_pubkey != open_channel.funding_pubkey;

channel_states.entry(channel_id).or_insert_with(|| {
ChannelState::new(
config,
holder,
state,
is_funding_outpoint_valid,
mined_txids.contains(&funding_outpoint.txid),
sent_invalid_signature,
)
});
}
Expand Down Expand Up @@ -1315,9 +1326,9 @@ fn recv_channel_ready(
/// A `channel_ready` is expected when a tracked channel is still at commitment
/// number 0, the counterparty's next per-commitment point is unknown, the
/// advertised funding outpoint pays the negotiated funding output, the funding
/// transaction was mined only after we sent `funding_created`, and it has at
/// least `minimum_depth` confirmations (as specified in the received
/// `accept_channel`).
/// transaction was mined only after we sent `funding_created`, we have not sent
/// a signature the peer is required to reject, and it has at least
/// `minimum_depth` confirmations (as specified in the received `accept_channel`).
fn is_channel_ready_expected(
channel_states: &HashMap<ChannelId, ChannelState>,
bitcoin_cli: &mut impl BitcoinRpc,
Expand All @@ -1327,34 +1338,12 @@ fn is_channel_ready_expected(
&& state.next_counterparty_per_commitment_point().is_none()
&& state.is_funding_outpoint_valid
&& !state.was_funding_mined_prematurely
&& !state.sent_invalid_signature
&& bitcoin_cli.get_transaction_confirmations(state.config.funding_outpoint.txid)
>= state.config.minimum_depth
})
}

/// Verifies the counterparty's signature from a `funding_signed` message using
/// the channel state associated with the message's `channel_id`.
///
/// # Errors
///
/// Returns [`Violation::UnknownChannel`] if no channel state exists for the
/// given `channel_id`, or [`Violation::InvalidCounterpartySignature`] if the
/// signature is invalid for the holder's initial commitment transaction.
fn verify_funding_signed(
fs: &FundingSigned,
channel_states: &HashMap<ChannelId, ChannelState>,
) -> Result<(), Violation> {
let state = channel_states
.get(&fs.channel_id)
.ok_or(Violation::UnknownChannel(fs.channel_id))?;

state
.config
.verify_counterparty_signature(&state.commitment, &state.holder, &fs.signature)
.then_some(())
.ok_or(Violation::InvalidCounterpartySignature(fs.channel_id))
}

/// Records a sent `open_channel`, keyed by `temporary_channel_id`, so the
/// funding flow can build commitments from the values actually put on the wire.
///
Expand Down Expand Up @@ -3661,10 +3650,13 @@ mod tests {
std::time::Instant::now(),
)
.unwrap_err();
assert!(matches!(
err,
ExecuteError::Violation(Violation::UnknownChannel(id)) if id == channel_id
));
let ExecuteError::Violation(Violation::InvalidFundingSigned(id, reason)) = &err else {
panic!("unexpected error: {err:?}");
};
assert_eq!(*id, channel_id);
assert!(
reason.contains("unknown channel_id: no funding_created was sent for this channel")
);
}

#[test]
Expand Down Expand Up @@ -3701,10 +3693,11 @@ mod tests {
std::time::Instant::now(),
)
.unwrap_err();
assert!(matches!(
err,
ExecuteError::Violation(Violation::InvalidCounterpartySignature(id)) if id == channel_id
));
let ExecuteError::Violation(Violation::InvalidFundingSigned(id, reason)) = &err else {
panic!("unexpected error: {err:?}");
};
assert_eq!(*id, channel_id);
assert!(reason.contains("invalid funding_signed: signature is not valid"));
}

#[test]
Expand Down Expand Up @@ -3930,17 +3923,41 @@ mod tests {

#[test]
fn execute_recv_channel_ready_invalid_funding_outpoint_is_noop() {
let (mut executor, channel_id, _) = recv_channel_ready_executor();
let channel_id = ChannelId::v1_from_funding_outpoint(OutPoint {
txid: "b86cbf63856be414fb9636160d729a29935fb998524410475878295272021727"
.parse()
.unwrap(),
vout: 0,
});

// Corrupt the negotiated opener funding pubkey so the broadcast funding
// transaction's output no longer pays the negotiated 2-of-2 script,
// marking the funding outpoint invalid.
// The expected signature here was computed using LDK as the source of
// truth.
let fs_bytes = Message::FundingSigned(FundingSigned {
channel_id,
signature: "30440220096b230642dcafeb2271e80b66eb7e7d7b5750af43a253d103bda600d27c10d302206b535c31d1c2a810f87950d17ed368cca017cdce6a26b23608da2fb62e9d8015".parse().unwrap(),
})
.encode();
let cr_bytes = Message::ChannelReady(ChannelReady {
channel_id,
second_per_commitment_point: sample_pubkey(1),
tlvs: ChannelReadyTlvs::default(),
})
.encode();

let mut executor = Executor::new(
MockConnection::new(),
MockBitcoinCli {
utxos: vec![sample_utxo()],
change_spk: sample_change_spk(),
..Default::default()
},
sample_context(),
);
executor.conn.queue_recv(fs_bytes);
executor.conn.queue_recv(cr_bytes);
executor
.negotiations
.get_mut(&ChannelId::new([0xbb; 32]))
.unwrap()
.open_channel
.funding_pubkey = sample_pubkey(1);
.insert(ChannelId::new([0xbb; 32]), sample_funding_negotiation());

let mut instrs = send_funding_created_and_recv_funding_signed_instructions();
instrs.extend([
Expand All @@ -3954,6 +3971,11 @@ mod tests {
},
]);

// We will create a transaction with the wrong acceptor funding pubkey,
// so the broadcast funding transaction's output no longer pays the
// negotiated 2-of-2 script, making the funding outpoint invalid.
instrs[3].inputs = vec![0];

// With invalid funding outpoint the target does not owe us a
// `channel_ready`, so `RecvChannelReady` must be a no-op.
executor
Expand Down Expand Up @@ -4100,6 +4122,53 @@ mod tests {
assert_eq!(executor.conn.recv_queue.len(), 1);
}

#[test]
fn execute_recv_channel_ready_invalid_signature_is_noop() {
let (mut executor, channel_id, _) = recv_channel_ready_executor();

let mut instrs = send_funding_created_and_recv_funding_signed_instructions();
// A `funding_signed` answering a `funding_created` we signed with the
// wrong key is itself a violation, which `FundingSignedOracle` reports.
// Stop before receiving one so `RecvChannelReady` is reached.
instrs.pop(); // Drop the trailing `RecvFundingSigned` instruction.

// We will sign the commitment with the acceptor's private key instead
// of the opener's, so the signature does not match the `funding_pubkey`
// negotiated in `open_channel`.
instrs[9].inputs = vec![6, 2, 8];

instrs.extend([
Instruction {
// Mine past the negotiated `minimum_depth`.
operation: Operation::MineBlocks(8),
inputs: vec![],
},
Instruction {
operation: Operation::RecvChannelReady,
inputs: vec![],
},
]);

// Having signed with the wrong key, the target does not owe us a
// `channel_ready`, so `RecvChannelReady` must be a no-op even though the
// confirmation count is sufficient.
executor
.execute(
&Program {
instructions: instrs,
},
std::time::Instant::now(),
)
.unwrap();

// The target's next per-commitment point is still unknown and the queued
// `funding_signed` and `channel_ready` remain untouched.
let state = executor.channel_states.get_mut(&channel_id).unwrap();
assert!(state.sent_invalid_signature);
assert!(state.next_counterparty_per_commitment_point().is_none());
assert_eq!(executor.conn.recv_queue.len(), 2);
}

// -- extract_field tests --

// TODO: Once we can actually construct and send accept_channel messages, it
Expand Down
8 changes: 8 additions & 0 deletions smite/src/channel_tx/commitment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,12 @@ pub struct ChannelState {
/// after the block height at which they receive `funding_created`, so they
/// may never observe it and never send `channel_ready`.
pub was_funding_mined_prematurely: bool,
/// Whether we have ever sent a signature the peer was required to reject,
/// such as a `funding_created`, `commitment_signed`, or HTLC signature etc.
/// it cannot verify. Set on the first occurrence and never cleared, since
/// BOLT 2 requires the peer to fail the channel or disconnect in response,
/// any subsequent positive response is therefore a violation.
pub sent_invalid_signature: bool,
}

impl Side {
Expand Down Expand Up @@ -179,6 +185,7 @@ impl ChannelState {
commitment: CommitmentState,
is_funding_outpoint_valid: bool,
was_funding_mined_prematurely: bool,
sent_invalid_signature: bool,
) -> Self {
Self {
config,
Expand All @@ -188,6 +195,7 @@ impl ChannelState {
acceptor_next_per_commitment_point: None,
is_funding_outpoint_valid,
was_funding_mined_prematurely,
sent_invalid_signature,
}
}

Expand Down
2 changes: 2 additions & 0 deletions smite/src/oracles.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,11 @@
//! Oracles evaluate conditions beyond simple crashes.

mod accept_channel;
mod funding_signed;

use super::violation::Violation;
pub use accept_channel::{AcceptChannelContext, AcceptChannelOracle};
pub use funding_signed::{FundingSignedContext, FundingSignedOracle};

/// `Oracle` evaluates a condition against some context
pub trait Oracle<C> {
Expand Down
Loading