feat: close_dlc msg and cooperative close logic by matthewjablack · Pull Request #255 · p2pderivatives/rust-dlc

matthewjablack · 2025-03-16T22:57:00Z

What

Add close_dlc msg type, and cooperative close logic

Why

It allows Alice or Bob to pass a message with the intention of cooperatively closing the DLC, allowing for early exit from a position.

The initiator provides an input, so they can cancel if the counterparty doesn't respond right away.

Yes this creates a free option problem, but the expectation is that the counterparty is a trusted party (for example a market maker).

This message type was discussed in DLC Specs: discreetlogcontracts/dlcspecs#161

And has been implemented in the Atomic Finance typescript libraries: AtomicFinance/bitcoin-abstraction-layer#86 and AtomicFinance/node-dlc#122

Tibo-lg

It's a good feature, though:

please add an integration test so we can be confident it's working, I don't think it does right now
The chain monitor is used for channels, I don't think what you implemented here will work
I think the best way to go to close the contract on the party offering the cooperative close would be to stay in the Confirmed state, but then maybe have a close_tx: Option<Transaction> field that you check within check_confirmed_contract so that you can mark the contract as PreClosed/Closed. It could also be a list if we assume that there could be multiple cooperative close offers (in case some are not accepted/processed).

matthewjablack · 2025-07-07T08:04:07Z

Thanks for that review @Tibo-lg

please add an integration test so we can be confident it's working, I don't think it does right now

Added to manager_execution_tests.rs

The chain monitor is used for channels, I don't think what you implemented here will work

Thanks for clarification, removed accordingly

I think the best way to go to close the contract on the party offering the cooperative close would be to stay in the Confirmed state, but then maybe have a close_tx: Option field that you check within check_confirmed_contract so that you can mark the contract as PreClosed/Closed. It could also be a list if we assume that there could be multiple cooperative close offers (in case some are not accepted/processed).

Not sure what you had in mind here. Were you thinking of having close_tx: Option<Transaction> be added to an existing struct? Which one did you have in mind?

I'm guessing you were thinking of adding it to DlcTransactions

- rm problematic CooperativeClose state that could persist indefinitely - fix state validation to expect Confirmed instead of Signed state - rm chain monitor usage (designed for channels, not standalone contracts) - add integration tests - rename verify_and_complete_cooperative_close to complete_cooperative_close - update storage and serialization to remove CooperativeClose variant Fixes issues identified in code review where CooperativeClose state could get stuck if counterparty never responds, and resolves transaction broadcast conflicts with chain monitor architecture.

- replace manual Ok/Err match with .ok() method - fixes CI failure on clippy:manual_ok_err lint

Tibo-lg · 2025-07-13T19:19:57Z

+        };
+
+        self.store
+            .update_contract(&Contract::Closed(closed_contract))?;


Thanks, fixed in 3356604

Tibo-lg · 2025-07-13T19:40:39Z

+                        .expect("Error accepting cooperative close");
+
+                    // Bob should now be in Closed state (he broadcast the transaction)
+                    assert_contract_state!(bob_manager_send, contract_id, Closed);


Should be PreClosed because the transaction is not confirmed.

Good catch, fixed in 3356604

Tibo-lg · 2025-07-13T19:42:32Z

+                    // In a real scenario, Alice would detect the close transaction and call on_counterparty_close
+                    // For the test, we'll verify the cooperative close functionality worked correctly


We definitely want to detect the close transaction, and we want to test it.

I've added check_pending_close_transactions function and included it in periodic_check https://github.com/p2pderivatives/rust-dlc/pull/255/files#diff-6901f71d060cd9a08d68e0e45ba8e40bcd714a041ddcdff6ea168eb4b35a1c4dR813-R863

Also updated tests https://github.com/p2pderivatives/rust-dlc/pull/255/files#diff-11a961d721d78a3c54a7b2a0b90b92b5a3768c0fbed6fef9c45014c0be8702e8R910-R936

Tibo-lg · 2025-07-13T19:50:10Z

Not sure what you had in mind here. Were you thinking of having close_tx: Option be added to an existing struct? Which one did you have in mind?

I'm guessing you were thinking of adding it to DlcTransactions

I don't have a strong opinion on where to put it, I guess DlcTransactions struct is fine.

- add `pending_close_txs` field to track cooperative close offers - add `check_pending_close_transactions()` for chain monitoring - fix cooperative close flow: validate vs accept separation - implement proper state transitions: Confirmed → PreClosed → Closed - update test coverage for cooperative close detection - support self-initiated and counterparty-initiated closes Resolves automatic detection of cooperative close transactions that are broadcast by counterparties, ensuring both parties transition to the correct final state

Regenerated test files after adding pending_close_txs field to Dlctransactions. These serialized files were used by the sled storage provider tests to verify correct serialization/deserialization.

matthewjablack · 2025-07-17T07:05:42Z

Thanks @Tibo-lg

I added pending_close_txs field to DlcTransactions to track cooperative close offers, check_pending_close_transactions() for chain. monitoring

Also fixed proper state transitions, with Confirmed → PreClosed → Closed

Tibo-lg · 2025-08-01T07:05:22Z

+    pub fund_input_serial_id: u64,
+    /// The funding inputs to use.
+    pub funding_inputs: Vec<FundingInput>,
+    /// The funding signatures.
+    pub funding_signatures: FundingSignatures,


I think these fields are never used?

These fields are necessary to ensure the offeror of the close contract can avoid a free option problem. They could offer a close to the counterparty, and the counterparty does not respond.

discreetlogcontracts/dlcspecs#161 (comment)

I've added an additional optional argument for create_collaborative_close_transaction to include additional inputs in a915378

Right now the way you set those fields does not make sense, you are setting them to the funding inputs and signatures, which matches with the names but not what you're describing here. If you want to be able to double spend the closing transaction then the inputs and signatures (actually a single one should be enough) should be set to utxos from the wallet. Honestly I feel the best is either you fully implement this (having it optional if you prefer), or just remove it.

Tibo-lg · 2025-08-01T07:05:56Z

+    /// The signature for the closing transaction.
+    pub close_signature: Signature,
+    /// The payout amount for the offer party in satoshis.
+    pub offer_payout: Amount,


You don't need to pass that it can be re-computed from total_collateral

Great point, updated in 5e06374

Tibo-lg · 2025-08-01T07:08:45Z

+        Ok((close_message, counter_party))
+    }
+
+    /// Accepts a cooperative close request by verifying the counter party's signature,


I don't think you are verifying the signature (not that you need to, since if it's incorrect the broadcast will fail)

Good catch, updated the comment in 9d84b29

Tibo-lg · 2025-08-01T07:11:58Z

+                    assert_contract_state!(alice_manager_send, contract_id, Closed);
+
+                    // Verify the close transaction was properly broadcast and confirmed
+                    let _close_txid = {


Remove this block if you don't need this

Good point, removed in 36c1e23

Tibo-lg · 2025-08-01T07:13:27Z


-                    periodic_check!(second, contract_id, Confirmed);
+                    match path {
+                        TestPath::Close => {


I would prefer that you don't refactor the rest of the tests to keep the PR focused, feel free to do another refactor PR.

Undid test refactor in 36c1e23

Tibo-lg · 2025-08-01T07:20:41Z

+            // Check if this is a cooperative close (no attestations) or a CET close (with attestations)
+            let (signed_cet, pnl) = if contract.attestations.is_none() {
+                // Cooperative close - no signed_cet in the final closed contract
+                let pnl = contract
+                    .signed_contract
+                    .accepted_contract
+                    .compute_pnl(&contract.signed_cet)?;
+                (None, pnl)
+            } else {
+                // CET close - include the signed_cet
+                let pnl = contract
+                    .signed_contract
+                    .accepted_contract
+                    .compute_pnl(&contract.signed_cet)?;
+                (Some(contract.signed_cet.clone()), pnl)
+            };
+


Why do you differentiate here?

Good point, no need to differentiate, fixed in 9d84b29

Tibo-lg · 2025-08-01T07:23:27Z

    }

+    /// Check for pending cooperative close transactions
+    fn check_pending_close_transactions(&self) -> Result<(), Error> {


This logic should be implemented within check_confirmed_contracts, unless there is a reason to have it separate? Right now that means having to go two times through the confirmed contracts.

Moved to check_confirmed_contracts in 9d84b29

Tibo-lg · 2025-08-01T07:24:29Z

+                    let pnl = contract.accepted_contract.compute_pnl(pending_close_tx)?;
+                    let closed_contract = ClosedContract {
+                        attestations: None, // Cooperative close has no attestations
+                        signed_cet: None,   // Cooperative close doesn't use a CET


Why set it to None, I think it's fine to save the closing transaction here, unless it breaks something?

You're right, signed_cet can be the closing tx, which is must more elegant. Updated in 9d84b29

- rm offer_payout field as it can be computed from total_collateral - accept_payout - add fee_rate_per_vb field to prevent free option problem - update serialization accordingly

- rm differentiation between cooperative close and CET close in check_preclosed_contracts - update accept_cooperative_close comment to be more accurate about what it does - move pending close transaction logic into check_confirmed_contracts - save closing transaction instead of setting to None

- add additional_inputs parameter to offer_collaborative_close functions - update create_collaborative_close_transaction to accept additional inputs - add documentation explaining the free option problem prevention - keep funding inputs in CloseDlc message to prevent free option problem

- add test for collaborative close with additional inputs - update existing tests to use new API with additional_inputs parameter - add test for free option problem prevention - update test structure to reflect simplified cooperative close logic

- update close_msg.json test file to match new CloseDlc structure - rm offerPayout field and add feeRatePerVb field - clean up extra blank lines in manager.rs - fixes test failures caused by structure changes

feat: close_dlc msg and cooperative close logic

5597d4a

Tibo-lg requested changes Jul 3, 2025

View reviewed changes

Comment thread dlc-manager/src/manager.rs Outdated

Comment thread dlc-manager/src/contract_updater.rs Outdated

Comment thread dlc-manager/src/contract/mod.rs Outdated

Comment thread dlc-manager/src/manager.rs Outdated

matthewjablack force-pushed the feat/close-dlc branch from 22c22c6 to 9a41fd9 Compare July 7, 2025 14:57

lint: apply rustfmt formatting fixes

cbddbc3

matthewjablack requested a review from Tibo-lg July 7, 2025 15:16

lint: clippy manual_ok_err warning in hex_utils

92cbf5c

- replace manual Ok/Err match with .ok() method - fixes CI failure on clippy:manual_ok_err lint

matthewjablack force-pushed the feat/close-dlc branch from 07d6b11 to 92cbf5c Compare July 7, 2025 17:50

Tibo-lg requested changes Jul 13, 2025

View reviewed changes

matthewjablack added 3 commits July 16, 2025 23:51

lint: clippy inneficient swaps w/ assignment

d52300f

test: update serialized files for pending_close_txs

7109c80

Regenerated test files after adding pending_close_txs field to Dlctransactions. These serialized files were used by the sled storage provider tests to verify correct serialization/deserialization.

bennyhodl suggested changes Jul 17, 2025

View reviewed changes

Comment thread dlc-manager/src/contract_updater.rs Outdated

refactor: use derive_contract_signer for close

ec6cb89

bennyhodl mentioned this pull request Jul 30, 2025

feat: cooperative close bennyhodl/dlcdevkit#107

Merged

Tibo-lg requested changes Aug 1, 2025

View reviewed changes

matthewjablack force-pushed the feat/close-dlc branch from 271662e to ec6cb89 Compare August 5, 2025 02:17

matthewjablack added 5 commits August 7, 2025 23:43

feat: remove offer_payout from CloseDlc message

5e06374

- rm offer_payout field as it can be computed from total_collateral - accept_payout - add fee_rate_per_vb field to prevent free option problem - update serialization accordingly

fix: update test files and clean up code

c5733ba

- update close_msg.json test file to match new CloseDlc structure - rm offerPayout field and add feeRatePerVb field - clean up extra blank lines in manager.rs - fixes test failures caused by structure changes

		// In a real scenario, Alice would detect the close transaction and call on_counterparty_close
		// For the test, we'll verify the cooperative close functionality worked correctly

Uh oh!

Conversation

matthewjablack commented Mar 16, 2025

What

Why

Uh oh!

Tibo-lg left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

matthewjablack commented Jul 7, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Tibo-lg commented Jul 13, 2025

Uh oh!

matthewjablack commented Jul 17, 2025

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

matthewjablack Aug 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Tibo-lg left a comment •

edited

Loading

matthewjablack Aug 8, 2025 •

edited

Loading