Fix settle after disconnect during open bug

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) })
     ;;

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.

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<All>,
     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(

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)

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<SubChannelCloseFinalize, Error> {
         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() })?;
 

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,22 +2262,29 @@ 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();
 
     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();
 }