diff --git a/dlc-manager/src/channel/ser.rs b/dlc-manager/src/channel/ser.rs index d339481b..d6ad81a3 100644 --- a/dlc-manager/src/channel/ser.rs +++ b/dlc-manager/src/channel/ser.rs @@ -60,7 +60,7 @@ impl_dlc_writeable_enum!( (6, RenewOffered, {(offered_contract_id, writeable), (counter_payout, writeable), (is_offer, writeable), (offer_next_per_update_point, writeable), (timeout, writeable)}), (7, RenewAccepted, {(contract_id, writeable), (offer_per_update_point, writeable), (accept_per_update_point, writeable), (buffer_transaction, writeable), (buffer_script_pubkey, writeable), (timeout, writeable), (own_payout, writeable)}), (8, RenewConfirmed, {(contract_id, writeable), (offer_per_update_point, writeable), (accept_per_update_point, writeable), (buffer_transaction, writeable), (buffer_script_pubkey, writeable), (offer_buffer_adaptor_signature, {cb_writeable, write_ecdsa_adaptor_signature, read_ecdsa_adaptor_signature}), (timeout, writeable), (own_payout, writeable), (total_collateral, writeable)}), - (10, RenewFinalized, {(contract_id, writeable), (offer_per_update_point, writeable), (accept_per_update_point, writeable), (buffer_transaction, writeable), (buffer_script_pubkey, writeable), (offer_buffer_adaptor_signature, {cb_writeable, write_ecdsa_adaptor_signature, read_ecdsa_adaptor_signature}), (accept_buffer_adaptor_signature, {cb_writeable, write_ecdsa_adaptor_signature, read_ecdsa_adaptor_signature}), (timeout, writeable), (own_payout, writeable), (total_collateral, writeable)}), + (10, RenewFinalized, {(contract_id, writeable), (prev_offer_per_update_point, writeable), (buffer_transaction, writeable), (buffer_script_pubkey, writeable), (offer_buffer_adaptor_signature, {cb_writeable, write_ecdsa_adaptor_signature, read_ecdsa_adaptor_signature}), (accept_buffer_adaptor_signature, {cb_writeable, write_ecdsa_adaptor_signature, read_ecdsa_adaptor_signature}), (timeout, writeable), (own_payout, writeable), (total_collateral, writeable)}), (9, Closing, {(buffer_transaction, writeable), (contract_id, writeable), (is_initiator, writeable)}), (11, CollaborativeCloseOffered, { (counter_payout, writeable), (offer_signature, writeable), (close_tx, writeable), (timeout, writeable) }) ;; diff --git a/dlc-manager/src/channel/signed_channel.rs b/dlc-manager/src/channel/signed_channel.rs index 0951c080..e6b8c7dc 100644 --- a/dlc-manager/src/channel/signed_channel.rs +++ b/dlc-manager/src/channel/signed_channel.rs @@ -264,12 +264,9 @@ typed_enum!( RenewFinalized { /// The [`crate::ContractId`] of the offered contract. contract_id: ContractId, - /// The per update point to be used by the offer party for the setup - /// of the next channel state. - offer_per_update_point: PublicKey, - /// The per update point to be used by the accept party for the setup - /// of the next channel state. - accept_per_update_point: PublicKey, + /// The previous per update point that was used by the offer party for the previous + /// state of the channel. + prev_offer_per_update_point: PublicKey, /// The buffer transaction. buffer_transaction: Transaction, /// The buffer transaction script pubkey. diff --git a/dlc-manager/src/channel_updater.rs b/dlc-manager/src/channel_updater.rs index 5b8088f6..d8c33cd3 100644 --- a/dlc-manager/src/channel_updater.rs +++ b/dlc-manager/src/channel_updater.rs @@ -1920,6 +1920,7 @@ where Some(signed_channel.channel_id), )?; + let prev_offer_per_update_point = signed_channel.counter_per_update_point; signed_channel.counter_per_update_point = offer_per_update_point; signed_channel.own_per_update_point = accept_per_update_point; @@ -1966,8 +1967,7 @@ where signed_channel.state = SignedChannelState::RenewFinalized { contract_id: signed_contract.accepted_contract.get_contract_id(), - offer_per_update_point, - accept_per_update_point, + prev_offer_per_update_point, buffer_transaction: buffer_transaction.clone(), buffer_script_pubkey: buffer_script_pubkey.clone(), offer_buffer_adaptor_signature: renew_confirm.buffer_adaptor_signature, @@ -2048,6 +2048,14 @@ where total_collateral: *total_collateral, }; + if PublicKey::from_secret_key(secp, &renew_finalize.per_update_secret) + != signed_channel.counter_per_update_point + { + return Err(Error::InvalidParameters( + "Invalid per update secret in channel renew finalize".to_string(), + )); + } + signed_channel .counter_party_commitment_secrets .provide_secret( @@ -2080,12 +2088,14 @@ where /// Verify the given [`RenewRevoke`] and update the state of the channel. pub fn renew_channel_on_revoke( + secp: &Secp256k1, signed_channel: &mut SignedChannel, renew_revoke: &RenewRevoke, ) -> Result<(), Error> { let ( contract_id, total_collateral, + prev_offer_per_update_point, offer_buffer_adaptor_signature, accept_buffer_adaptor_signature, buffer_transaction, @@ -2094,10 +2104,19 @@ pub fn renew_channel_on_revoke( RenewFinalized, contract_id, total_collateral, + prev_offer_per_update_point, offer_buffer_adaptor_signature, accept_buffer_adaptor_signature | buffer_transaction )?; + if PublicKey::from_secret_key(secp, &renew_revoke.per_update_secret) + != *prev_offer_per_update_point + { + return Err(Error::InvalidParameters( + "Invalid per update secret in channel renew revoke".to_string(), + )); + } + signed_channel .counter_party_commitment_secrets .provide_secret( diff --git a/dlc-manager/src/manager.rs b/dlc-manager/src/manager.rs index d7b0b794..78883c6c 100644 --- a/dlc-manager/src/manager.rs +++ b/dlc-manager/src/manager.rs @@ -1888,7 +1888,11 @@ where let mut signed_channel = get_channel_in_state!(self, &renew_revoke.channel_id, Signed, Some(*peer_id))?; - crate::channel_updater::renew_channel_on_revoke(&mut signed_channel, renew_revoke)?; + crate::channel_updater::renew_channel_on_revoke( + &self.secp, + &mut signed_channel, + renew_revoke, + )?; self.store .upsert_channel(Channel::Signed(signed_channel), None) diff --git a/dlc-manager/src/sub_channel_manager.rs b/dlc-manager/src/sub_channel_manager.rs index aac0eeec..7cc9ef47 100644 --- a/dlc-manager/src/sub_channel_manager.rs +++ b/dlc-manager/src/sub_channel_manager.rs @@ -2472,6 +2472,17 @@ where .dlc_channel_manager .get_closed_sub_dlc_channel(dlc_channel_id, state.own_balance)?; + if PublicKey::from_secret_key( + self.dlc_channel_manager.get_secp(), + &confirm.split_revocation_secret, + ) != state.signed_subchannel.counter_per_split_point + { + return Err(Error::InvalidParameters( + "Invalid per update secret in subchannel close confirm".to_string(), + ) + .into()); + } + sub_channel .counter_party_secrets .provide_secret( @@ -2482,14 +2493,6 @@ where Error::InvalidParameters("Invalid split revocation secret".to_string()) })?; - debug_assert_eq!( - PublicKey::from_secret_key( - self.dlc_channel_manager.get_secp(), - &confirm.split_revocation_secret - ), - state.signed_subchannel.counter_per_split_point - ); - let raa = RevokeAndACK { channel_id: confirm.channel_id, per_commitment_secret: *confirm.commit_revocation_secret.as_ref(), @@ -2588,6 +2591,17 @@ where Some(*counter_party) )?; + if PublicKey::from_secret_key( + self.dlc_channel_manager.get_secp(), + &finalize.split_revocation_secret, + ) != state.signed_subchannel.counter_per_split_point + { + return Err(Error::InvalidParameters( + "Invalid per update secret in subchannel close finalize".to_string(), + ) + .into()); + } + sub_channel .counter_party_secrets .provide_secret( @@ -3136,7 +3150,7 @@ where None => true, Some(state) if state == ReestablishFlag::OffChainClosed as u8 => true, Some(state) if state == ReestablishFlag::CloseConfirmed as u8 => { - let finalize = self.get_reconnect_close_finalize(&channel)?; + let finalize = self.get_reconnect_close_finalize(&channel, true)?; self.actions .lock() @@ -3280,7 +3294,7 @@ where temporary_channel_id: dlc_channel.temporary_channel_id, party_points: dlc_channel.own_points, per_update_point: dlc_channel.own_per_update_point, - offer_per_update_seed: channel.per_split_seed, + offer_per_update_seed: Some(dlc_channel.own_per_update_seed), is_offer_party: true, counter_party: dlc_channel.counter_party, // TODO(tibo): use value from original offer @@ -3471,7 +3485,7 @@ where } SubChannelState::OffChainClosed => { if let Some(counter_state) = peer_state { - let finalize = self.get_reconnect_close_finalize(&channel)?; + let finalize = self.get_reconnect_close_finalize(&channel, false)?; if counter_state == ReestablishFlag::CloseConfirmed as u8 { self.actions .lock() @@ -3508,6 +3522,9 @@ where fn get_reconnect_close_finalize( &self, channel: &SubChannel, + // when the channel has been offered again already, the offer party will have decreased its + // update index, so we need to restore it to provide a proper split secret. + restore_index: bool, ) -> Result { let per_split_seed = self .dlc_channel_manager @@ -3516,9 +3533,14 @@ where &channel.per_split_seed.expect("to have a per split seed"), )?; + let update_index = if restore_index { + channel.update_idx + 1 + } else { + channel.update_idx + }; let per_split_secret = SecretKey::from_slice(&build_commitment_secret( per_split_seed.as_ref(), - channel.update_idx, + update_index, )) .map_err(|e| APIError::ExternalError { err: e.to_string() })?; diff --git a/dlc-manager/tests/ln_dlc_channel_execution_tests.rs b/dlc-manager/tests/ln_dlc_channel_execution_tests.rs index 801f600b..e35a83e2 100644 --- a/dlc-manager/tests/ln_dlc_channel_execution_tests.rs +++ b/dlc-manager/tests/ln_dlc_channel_execution_tests.rs @@ -592,6 +592,70 @@ fn ln_dlc_renewed_close() { force_close_stable(&mut test_params); } +#[test] +#[ignore] +fn ln_dlc_open_disconnect_renewed_close() { + let mut test_params = test_init(); + + make_ln_payment(&test_params.alice_node, &test_params.bob_node, 900000); + + offer_sub_channel_with_reconnect(&test_params); + let sub_channel = test_params + .alice_node + .dlc_manager + .get_store() + .get_sub_channel(test_params.channel_id) + .unwrap() + .unwrap(); + let dlc_channel_id = sub_channel.get_dlc_channel_id(0).unwrap(); + let contract_id = assert_channel_contract_state!( + test_params.alice_node.dlc_manager, + dlc_channel_id, + Confirmed + ); + + renew(&test_params, &dlc_channel_id); + + assert_contract_state_unlocked!(test_params.alice_node.dlc_manager, contract_id, Closed); + assert_contract_state_unlocked!(test_params.bob_node.dlc_manager, contract_id, Closed); + + mocks::mock_time::set_time(EVENT_MATURITY as u64); + + force_close_stable(&mut test_params); +} + +#[test] +#[ignore] +fn ln_dlc_open_disconnect_settled_close() { + let mut test_params = test_init(); + + make_ln_payment(&test_params.alice_node, &test_params.bob_node, 900000); + + offer_sub_channel_with_reconnect(&test_params); + let sub_channel = test_params + .alice_node + .dlc_manager + .get_store() + .get_sub_channel(test_params.channel_id) + .unwrap() + .unwrap(); + let dlc_channel_id = sub_channel.get_dlc_channel_id(0).unwrap(); + let contract_id = assert_channel_contract_state!( + test_params.alice_node.dlc_manager, + dlc_channel_id, + Confirmed + ); + + settle(&test_params, &dlc_channel_id); + + assert_contract_state_unlocked!(test_params.alice_node.dlc_manager, contract_id, Closed); + assert_contract_state_unlocked!(test_params.bob_node.dlc_manager, contract_id, Closed); + + mocks::mock_time::set_time(EVENT_MATURITY as u64); + + force_close_stable(&mut test_params); +} + #[test] #[ignore] fn ln_dlc_settled_close() { @@ -2187,7 +2251,7 @@ fn renew(test_params: &LnDlcTestParams, dlc_channel_id: &ChannelId) { ) .unwrap(); - let (accept, bob_key) = test_params + let (accept, _) = test_params .bob_node .dlc_manager .accept_renew_offer(dlc_channel_id) @@ -2198,7 +2262,7 @@ fn renew(test_params: &LnDlcTestParams, dlc_channel_id: &ChannelId) { .dlc_manager .on_dlc_message( &Message::Channel(ChannelMessage::RenewAccept(accept)), - test_params.bob_node.channel_manager.get_our_node_id(), + test_params.bob_node_id, ) .unwrap() .unwrap(); @@ -2206,14 +2270,21 @@ fn renew(test_params: &LnDlcTestParams, dlc_channel_id: &ChannelId) { let msg = test_params .bob_node .dlc_manager - .on_dlc_message(&msg, bob_key) + .on_dlc_message(&msg, test_params.alice_node_id) .unwrap() .unwrap(); - test_params + let msg = test_params .alice_node .dlc_manager - .on_dlc_message(&msg, test_params.bob_node.channel_manager.get_our_node_id()) + .on_dlc_message(&msg, test_params.bob_node_id) + .unwrap() + .unwrap(); + + test_params + .bob_node + .dlc_manager + .on_dlc_message(&msg, test_params.alice_node_id) .unwrap(); }