Skip to content

Commit

Permalink
Allow get_shutdown_scriptpubkey and get_destination_script to return …
Browse files Browse the repository at this point in the history
…an error
  • Loading branch information
benthecarman committed Apr 22, 2023
1 parent 5f96d13 commit 76b903b
Show file tree
Hide file tree
Showing 5 changed files with 50 additions and 28 deletions.
8 changes: 4 additions & 4 deletions fuzz/src/chanmon_consistency.rs
Original file line number Diff line number Diff line change
Expand Up @@ -259,18 +259,18 @@ impl SignerProvider for KeyProvider {
})
}

fn get_destination_script(&self) -> Script {
fn get_destination_script(&self) -> Result<Script, ()> {
let secp_ctx = Secp256k1::signing_only();
let channel_monitor_claim_key = SecretKey::from_slice(&[0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 2, self.node_secret[31]]).unwrap();
let our_channel_monitor_claim_key_hash = WPubkeyHash::hash(&PublicKey::from_secret_key(&secp_ctx, &channel_monitor_claim_key).serialize());
Builder::new().push_opcode(opcodes::all::OP_PUSHBYTES_0).push_slice(&our_channel_monitor_claim_key_hash[..]).into_script()
Ok(Builder::new().push_opcode(opcodes::all::OP_PUSHBYTES_0).push_slice(&our_channel_monitor_claim_key_hash[..]).into_script())
}

fn get_shutdown_scriptpubkey(&self) -> ShutdownScript {
fn get_shutdown_scriptpubkey(&self) -> Result<ShutdownScript, ()> {
let secp_ctx = Secp256k1::signing_only();
let secret_key = SecretKey::from_slice(&[0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 3, self.node_secret[31]]).unwrap();
let pubkey_hash = WPubkeyHash::hash(&PublicKey::from_secret_key(&secp_ctx, &secret_key).serialize());
ShutdownScript::new_p2wpkh(&pubkey_hash)
Ok(ShutdownScript::new_p2wpkh(&pubkey_hash))
}
}

Expand Down
16 changes: 8 additions & 8 deletions lightning/src/chain/keysinterface.rs
Original file line number Diff line number Diff line change
Expand Up @@ -545,13 +545,13 @@ pub trait SignerProvider {
///
/// This method should return a different value each time it is called, to avoid linking
/// on-chain funds across channels as controlled to the same user.
fn get_destination_script(&self) -> Script;
fn get_destination_script(&self) -> Result<Script, ()>;

/// Get a script pubkey which we will send funds to when closing a channel.
///
/// This method should return a different value each time it is called, to avoid linking
/// on-chain funds across channels as controlled to the same user.
fn get_shutdown_scriptpubkey(&self) -> ShutdownScript;
fn get_shutdown_scriptpubkey(&self) -> Result<ShutdownScript, ()>;
}

/// A simple implementation of [`WriteableEcdsaChannelSigner`] that just keeps the private keys in memory.
Expand Down Expand Up @@ -1366,12 +1366,12 @@ impl SignerProvider for KeysManager {
InMemorySigner::read(&mut io::Cursor::new(reader), self)
}

fn get_destination_script(&self) -> Script {
self.destination_script.clone()
fn get_destination_script(&self) -> Result<Script, ()> {
Ok(self.destination_script.clone())
}

fn get_shutdown_scriptpubkey(&self) -> ShutdownScript {
ShutdownScript::new_p2wpkh_from_pubkey(self.shutdown_pubkey.clone())
fn get_shutdown_scriptpubkey(&self) -> Result<ShutdownScript, ()> {
Ok(ShutdownScript::new_p2wpkh_from_pubkey(self.shutdown_pubkey.clone()))
}
}

Expand Down Expand Up @@ -1461,11 +1461,11 @@ impl SignerProvider for PhantomKeysManager {
self.inner.read_chan_signer(reader)
}

fn get_destination_script(&self) -> Script {
fn get_destination_script(&self) -> Result<Script, ()> {
self.inner.get_destination_script()
}

fn get_shutdown_scriptpubkey(&self) -> ShutdownScript {
fn get_shutdown_scriptpubkey(&self) -> Result<ShutdownScript, ()> {
self.inner.get_shutdown_scriptpubkey()
}
}
Expand Down
42 changes: 32 additions & 10 deletions lightning/src/ln/channel.rs
Original file line number Diff line number Diff line change
Expand Up @@ -986,7 +986,10 @@ impl<Signer: WriteableEcdsaChannelSigner> Channel<Signer> {
secp_ctx.seeded_randomize(&entropy_source.get_secure_random_bytes());

let shutdown_scriptpubkey = if config.channel_handshake_config.commit_upfront_shutdown_pubkey {
Some(signer_provider.get_shutdown_scriptpubkey())
match signer_provider.get_shutdown_scriptpubkey() {
Ok(scriptpubkey) => Some(scriptpubkey),
Err(_) => return Err(APIError::ChannelUnavailable { err: "Failed to get shutdown scriptpubkey".to_owned()}),
}
} else { None };

if let Some(shutdown_scriptpubkey) = &shutdown_scriptpubkey {
Expand All @@ -995,6 +998,11 @@ impl<Signer: WriteableEcdsaChannelSigner> Channel<Signer> {
}
}

let destination_script = match signer_provider.get_destination_script() {
Ok(script) => script,
Err(_) => return Err(APIError::ChannelUnavailable { err: "Failed to get destination script".to_owned()}),
};

let temporary_channel_id = entropy_source.get_secure_random_bytes();

Ok(Channel {
Expand All @@ -1021,7 +1029,7 @@ impl<Signer: WriteableEcdsaChannelSigner> Channel<Signer> {

holder_signer,
shutdown_scriptpubkey,
destination_script: signer_provider.get_destination_script(),
destination_script,

cur_holder_commitment_transaction_number: INITIAL_COMMITMENT_NUMBER,
cur_counterparty_commitment_transaction_number: INITIAL_COMMITMENT_NUMBER,
Expand Down Expand Up @@ -1333,7 +1341,10 @@ impl<Signer: WriteableEcdsaChannelSigner> Channel<Signer> {
} else { None };

let shutdown_scriptpubkey = if config.channel_handshake_config.commit_upfront_shutdown_pubkey {
Some(signer_provider.get_shutdown_scriptpubkey())
match signer_provider.get_shutdown_scriptpubkey() {
Ok(scriptpubkey) => Some(scriptpubkey),
Err(_) => return Err(ChannelError::Close("Failed to get upfront shutdown scriptpubkey".to_owned())),
}
} else { None };

if let Some(shutdown_scriptpubkey) = &shutdown_scriptpubkey {
Expand All @@ -1342,6 +1353,11 @@ impl<Signer: WriteableEcdsaChannelSigner> Channel<Signer> {
}
}

let destination_script = match signer_provider.get_destination_script() {
Ok(script) => script,
Err(_) => return Err(ChannelError::Close("Failed to get destination script".to_owned())),
};

let mut secp_ctx = Secp256k1::new();
secp_ctx.seeded_randomize(&entropy_source.get_secure_random_bytes());

Expand All @@ -1368,7 +1384,7 @@ impl<Signer: WriteableEcdsaChannelSigner> Channel<Signer> {

holder_signer,
shutdown_scriptpubkey,
destination_script: signer_provider.get_destination_script(),
destination_script,

cur_holder_commitment_transaction_number: INITIAL_COMMITMENT_NUMBER,
cur_counterparty_commitment_transaction_number: INITIAL_COMMITMENT_NUMBER,
Expand Down Expand Up @@ -4355,7 +4371,10 @@ impl<Signer: WriteableEcdsaChannelSigner> Channel<Signer> {
Some(_) => false,
None => {
assert!(send_shutdown);
let shutdown_scriptpubkey = signer_provider.get_shutdown_scriptpubkey();
let shutdown_scriptpubkey = match signer_provider.get_shutdown_scriptpubkey() {
Ok(scriptpubkey) => scriptpubkey,
Err(_) => return Err(ChannelError::Close("Failed to get upfront shutdown scriptpubkey".to_owned())),
};
if !shutdown_scriptpubkey.is_compatible(their_features) {
return Err(ChannelError::Close(format!("Provided a scriptpubkey format not accepted by peer: {}", shutdown_scriptpubkey)));
}
Expand Down Expand Up @@ -6062,7 +6081,10 @@ impl<Signer: WriteableEcdsaChannelSigner> Channel<Signer> {
let update_shutdown_script = match self.shutdown_scriptpubkey {
Some(_) => false,
None if !chan_closed => {
let shutdown_scriptpubkey = signer_provider.get_shutdown_scriptpubkey();
let shutdown_scriptpubkey = match signer_provider.get_shutdown_scriptpubkey() {
Ok(scriptpubkey) => scriptpubkey,
Err(_) => return Err(APIError::ChannelUnavailable { err: "Failed to get upfront shutdown scriptpubkey".to_owned() }),
};
if !shutdown_scriptpubkey.is_compatible(their_features) {
return Err(APIError::IncompatibleShutdownScript { script: shutdown_scriptpubkey.clone() });
}
Expand Down Expand Up @@ -7086,17 +7108,17 @@ mod tests {

fn read_chan_signer(&self, _data: &[u8]) -> Result<Self::Signer, DecodeError> { panic!(); }

fn get_destination_script(&self) -> Script {
fn get_destination_script(&self) -> Result<Script, ()> {
let secp_ctx = Secp256k1::signing_only();
let channel_monitor_claim_key = SecretKey::from_slice(&hex::decode("0fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff").unwrap()[..]).unwrap();
let channel_monitor_claim_key_hash = WPubkeyHash::hash(&PublicKey::from_secret_key(&secp_ctx, &channel_monitor_claim_key).serialize());
Builder::new().push_opcode(opcodes::all::OP_PUSHBYTES_0).push_slice(&channel_monitor_claim_key_hash[..]).into_script()
Ok(Builder::new().push_opcode(opcodes::all::OP_PUSHBYTES_0).push_slice(&channel_monitor_claim_key_hash[..]).into_script())
}

fn get_shutdown_scriptpubkey(&self) -> ShutdownScript {
fn get_shutdown_scriptpubkey(&self) -> Result<ShutdownScript, ()> {
let secp_ctx = Secp256k1::signing_only();
let channel_close_key = SecretKey::from_slice(&hex::decode("0fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff").unwrap()[..]).unwrap();
ShutdownScript::new_p2wpkh_from_pubkey(PublicKey::from_secret_key(&secp_ctx, &channel_close_key))
Ok(ShutdownScript::new_p2wpkh_from_pubkey(PublicKey::from_secret_key(&secp_ctx, &channel_close_key)))
}
}

Expand Down
2 changes: 1 addition & 1 deletion lightning/src/ln/shutdown_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -669,7 +669,7 @@ fn test_unsupported_anysegwit_shutdown_script() {
let nodes = create_network(3, &node_cfgs, &node_chanmgrs);

// Check that using an unsupported shutdown script fails and a supported one succeeds.
let supported_shutdown_script = chanmon_cfgs[1].keys_manager.get_shutdown_scriptpubkey();
let supported_shutdown_script = chanmon_cfgs[1].keys_manager.get_shutdown_scriptpubkey().unwrap();
let unsupported_shutdown_script =
ShutdownScript::new_witness_program(WitnessVersion::V16, &[0, 40]).unwrap();
chanmon_cfgs[1].keys_manager
Expand Down
10 changes: 5 additions & 5 deletions lightning/src/util/test_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -171,8 +171,8 @@ impl SignerProvider for OnlyReadsKeysInterface {
))
}

fn get_destination_script(&self) -> Script { unreachable!(); }
fn get_shutdown_scriptpubkey(&self) -> ShutdownScript { unreachable!(); }
fn get_destination_script(&self) -> Result<Script, ()> { Err(()) }
fn get_shutdown_scriptpubkey(&self) -> Result<ShutdownScript, ()> { Err(()) }
}

pub struct TestChainMonitor<'a> {
Expand Down Expand Up @@ -793,14 +793,14 @@ impl SignerProvider for TestKeysInterface {
))
}

fn get_destination_script(&self) -> Script { self.backing.get_destination_script() }
fn get_destination_script(&self) -> Result<Script, ()> { self.backing.get_destination_script() }

fn get_shutdown_scriptpubkey(&self) -> ShutdownScript {
fn get_shutdown_scriptpubkey(&self) -> Result<ShutdownScript, ()> {
match &mut *self.expectations.lock().unwrap() {
None => self.backing.get_shutdown_scriptpubkey(),
Some(expectations) => match expectations.pop_front() {
None => panic!("Unexpected get_shutdown_scriptpubkey"),
Some(expectation) => expectation.returns,
Some(expectation) => Ok(expectation.returns),
},
}
}
Expand Down

0 comments on commit 76b903b

Please sign in to comment.