Skip to content

Handle sizeable push msat (fix #195) + handle two first per_commitment_point + keys interface tests #230

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.

Sign up for GitHub

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

Merged
merged 13 commits into from
Nov 20, 2018
Merged

Handle sizeable push msat (fix #195) + handle two first per_commitment_point + keys interface tests #230

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
04e6be9
Implement spendable output to_local output on local commitment tx
Oct 31, 2018
8c638ff
Track local_commitment_tx at funding_created
Oct 31, 2018
f88f826
Add TxOut in DynamicOutput
Nov 11, 2018
f053169
Add test_claim_sizeable_push_msat
Oct 31, 2018
1b33064
Split provide_their_next_commitment_point from provide_secret
Nov 1, 2018
3518f1f
Add test_static_spendable_outputs_preimage_tx
Nov 1, 2018
1255885
Add check_spend_closing_transaction ChannelMonitor
Nov 17, 2018
e22220d
Add test_claim_on_remote_sizeable_push_msat
Nov 5, 2018
73415a8
Add test_static_spendable_outputs_justice_tx_revoked_commitment_tx
Nov 5, 2018
b6b5dec
Add test_static_spendable_outputs_justice_tx_revoked_htlc*
Nov 6, 2018
394b11c
Add key_storage selection in ChannelMonitor insert_combine
Nov 6, 2018
890d176
Add test_dynamic_spendable_outputs_local_htlc_*
Nov 6, 2018
3a7b40e
Add test_static_output_closing_tx
Nov 7, 2018
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
27 changes: 21 additions & 6 deletions src/chain/keysinterface.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,19 +37,34 @@ pub enum SpendableOutputDescriptor {
/// The output which is referenced by the given outpoint
output: TxOut,
},
/// Outpoint commits to a P2WSH, should be spend by the following witness :
/// Outpoint commits to a P2WSH
/// P2WSH should be spend by the following witness :
/// <local_delayedsig> 0 <witnessScript>
/// With input nSequence set to_self_delay.
/// Outputs from a HTLC-Success/Timeout tx
DynamicOutput {
/// Outputs from a HTLC-Success/Timeout tx/commitment tx
DynamicOutputP2WSH {
/// Outpoint spendable by user wallet
outpoint: OutPoint,
/// local_delayedkey = delayed_payment_basepoint_secret + SHA256(per_commitment_point || delayed_payment_basepoint)
local_delayedkey: SecretKey,
/// witness redeemScript encumbering output
/// local_delayedkey = delayed_payment_basepoint_secret + SHA256(per_commitment_point || delayed_payment_basepoint) OR
key: SecretKey,
/// witness redeemScript encumbering output.
witness_script: Script,
/// nSequence input must commit to self_delay to satisfy script's OP_CSV
to_self_delay: u16,
/// The output which is referenced by the given outpoint
output: TxOut,
},
/// Outpoint commits to a P2WPKH
/// P2WPKH should be spend by the following witness :
/// <local_sig> <local_pubkey>
/// Outputs to_remote from a commitment tx
DynamicOutputP2WPKH {
/// Outpoint spendable by user wallet
outpoint: OutPoint,
/// localkey = payment_basepoint_secret + SHA256(per_commitment_point || payment_basepoint
key: SecretKey,
/// The output which is reference by the given outpoint
output: TxOut,
}
}

Expand Down
25 changes: 17 additions & 8 deletions src/ln/channel.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -436,7 +436,7 @@ impl Channel {

let secp_ctx = Secp256k1::new();
let channel_monitor = ChannelMonitor::new(&chan_keys.revocation_base_key, &chan_keys.delayed_payment_base_key,
&chan_keys.htlc_base_key, BREAKDOWN_TIMEOUT,
&chan_keys.htlc_base_key, &chan_keys.payment_base_key, &keys_provider.get_shutdown_pubkey(), BREAKDOWN_TIMEOUT,
keys_provider.get_destination_script(), logger.clone());

Ok(Channel {
Expand Down Expand Up @@ -624,9 +624,10 @@ impl Channel {

let secp_ctx = Secp256k1::new();
let mut channel_monitor = ChannelMonitor::new(&chan_keys.revocation_base_key, &chan_keys.delayed_payment_base_key,
&chan_keys.htlc_base_key, BREAKDOWN_TIMEOUT,
&chan_keys.htlc_base_key, &chan_keys.payment_base_key, &keys_provider.get_shutdown_pubkey(), BREAKDOWN_TIMEOUT,
keys_provider.get_destination_script(), logger.clone());
channel_monitor.set_their_base_keys(&msg.htlc_basepoint, &msg.delayed_payment_basepoint);
channel_monitor.provide_their_next_revocation_point(Some((INITIAL_COMMITMENT_NUMBER, msg.first_per_commitment_point)));
channel_monitor.set_their_to_self_delay(msg.to_self_delay);

let mut chan = Channel {
Expand Down Expand Up @@ -1347,6 +1348,7 @@ impl Channel {
}

self.channel_monitor.set_their_base_keys(&msg.htlc_basepoint, &msg.delayed_payment_basepoint);
self.channel_monitor.provide_their_next_revocation_point(Some((INITIAL_COMMITMENT_NUMBER, msg.first_per_commitment_point)));

self.their_dust_limit_satoshis = msg.dust_limit_satoshis;
self.their_max_htlc_value_in_flight_msat = cmp::min(msg.max_htlc_value_in_flight_msat, self.channel_value_satoshis * 1000);
Expand All @@ -1371,22 +1373,25 @@ impl Channel {
Ok(())
}

fn funding_created_signature(&mut self, sig: &Signature) -> Result<(Transaction, Signature), HandleError> {
fn funding_created_signature(&mut self, sig: &Signature) -> Result<(Transaction, Transaction, Signature, TxCreationKeys), HandleError> {
let funding_script = self.get_funding_redeemscript();

let local_keys = self.build_local_transaction_keys(self.cur_local_commitment_transaction_number)?;
let local_initial_commitment_tx = self.build_commitment_transaction(self.cur_local_commitment_transaction_number, &local_keys, true, false, self.feerate_per_kw).0;
let mut local_initial_commitment_tx = self.build_commitment_transaction(self.cur_local_commitment_transaction_number, &local_keys, true, false, self.feerate_per_kw).0;
let local_sighash = Message::from_slice(&bip143::SighashComponents::new(&local_initial_commitment_tx).sighash_all(&local_initial_commitment_tx.input[0], &funding_script, self.channel_value_satoshis)[..]).unwrap();

// They sign the "local" commitment transaction, allowing us to broadcast the tx if we wish.
// They sign the "local" commitment transaction...
secp_call!(self.secp_ctx.verify(&local_sighash, &sig, &self.their_funding_pubkey.unwrap()), "Invalid funding_created signature from peer", self.channel_id());

// ...and we sign it, allowing us to broadcast the tx if we wish
self.sign_commitment_transaction(&mut local_initial_commitment_tx, sig);

let remote_keys = self.build_remote_transaction_keys()?;
let remote_initial_commitment_tx = self.build_commitment_transaction(self.cur_remote_commitment_transaction_number, &remote_keys, false, false, self.feerate_per_kw).0;
let remote_sighash = Message::from_slice(&bip143::SighashComponents::new(&remote_initial_commitment_tx).sighash_all(&remote_initial_commitment_tx.input[0], &funding_script, self.channel_value_satoshis)[..]).unwrap();

// We sign the "remote" commitment transaction, allowing them to broadcast the tx if they wish.
Ok((remote_initial_commitment_tx, self.secp_ctx.sign(&remote_sighash, &self.local_keys.funding_key)))
Ok((remote_initial_commitment_tx, local_initial_commitment_tx, self.secp_ctx.sign(&remote_sighash, &self.local_keys.funding_key), local_keys))
}

pub fn funding_created(&mut self, msg: &msgs::FundingCreated) -> Result<(msgs::FundingSigned, ChannelMonitor), HandleError> {
Expand All @@ -1409,7 +1414,7 @@ impl Channel {
let funding_txo_script = self.get_funding_redeemscript().to_v0_p2wsh();
self.channel_monitor.set_funding_info((funding_txo, funding_txo_script));

let (remote_initial_commitment_tx, our_signature) = match self.funding_created_signature(&msg.signature) {
let (remote_initial_commitment_tx, local_initial_commitment_tx, our_signature, local_keys) = match self.funding_created_signature(&msg.signature) {
Ok(res) => res,
Err(e) => {
self.channel_monitor.unset_funding_info();
Expand All @@ -1420,6 +1425,8 @@ impl Channel {
// Now that we're past error-generating stuff, update our local state:

self.channel_monitor.provide_latest_remote_commitment_tx_info(&remote_initial_commitment_tx, Vec::new(), self.cur_remote_commitment_transaction_number);
self.last_local_commitment_txn = vec![local_initial_commitment_tx.clone()];
self.channel_monitor.provide_latest_local_commitment_tx_info(local_initial_commitment_tx, local_keys, self.feerate_per_kw, Vec::new());
self.channel_state = ChannelState::FundingSent as u32;
self.channel_id = funding_txo.to_channel_id();
self.cur_remote_commitment_transaction_number -= 1;
Expand Down Expand Up @@ -1489,6 +1496,7 @@ impl Channel {
return Err(ChannelError::Close("Peer sent a funding_locked at a strange time"));
}

self.channel_monitor.provide_their_next_revocation_point(Some((INITIAL_COMMITMENT_NUMBER - 1 , msg.next_per_commitment_point)));
Copy link
Collaborator

Choose a reason for hiding this comment

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

Hmm, I think this split may be simpler by just moving the their_next_revocation_point tracking into provide_latest_remote_commitment_tx_info. We only ever use the revocation point in case we have remote_tx_info available corresponding to it, so it should be way simpler to just move it there.

Copy link
Author

Choose a reason for hiding this comment

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

Not sure for this one... firstly we get per_commitment_point from remote at time we don't/didn't have generate remote commitment tx (accept_channel/new_from_req/funding_locked) and secondly we may use revocation point to generate pubkeys without per_commitment_option (htlcs_outputs) in case of sizeable push_msat?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Hmm? You mean you intend to add some use for their_cur_revocation_points beyond what its used for now, right now its only ever used in conjunction with remote_claimable_outpoints data.

Copy link
Author

Choose a reason for hiding this comment

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

Yes exactly, see deacb3f, in case of to_remote ouput you don't need remote_claimable_outpoints but you still need per_commitment_point to derive your remoteprivatekey. If I get derivation scheme right, but please review it thoroughly, that's easy to mess this part

self.their_prev_commitment_point = self.their_cur_commitment_point;
self.their_cur_commitment_point = Some(msg.next_per_commitment_point);
Ok(())
Expand Down Expand Up @@ -1868,7 +1876,8 @@ impl Channel {
return Err(HandleError{err: "Got a revoke commitment secret which didn't correspond to their current pubkey", action: None});
}
}
self.channel_monitor.provide_secret(self.cur_remote_commitment_transaction_number + 1, msg.per_commitment_secret, Some((self.cur_remote_commitment_transaction_number - 1, msg.next_per_commitment_point)))?;
self.channel_monitor.provide_secret(self.cur_remote_commitment_transaction_number + 1, msg.per_commitment_secret)?;
self.channel_monitor.provide_their_next_revocation_point(Some((self.cur_remote_commitment_transaction_number - 1, msg.next_per_commitment_point)));

// Update state now that we've passed all the can-fail calls...
// (note that we may still fail to generate the new commitment_signed message, but that's
Expand Down
Loading