chore(dlc_protocol): Track rollovers in dlc protocol table #2125

holzeis · 2024-02-29T15:26:25Z

Adds a dlc_protocol_type to the dlc_protocols table, allowing us to show what action triggered the dlc protocol. I opted to adding this enum, as it is helpful in deciding post processing actions, and the renew protocol is used for rollover and opening positions, so it would be ambiguous picking that information from the dlc_messages.

orderbook=# select protocol_type, previous_protocol_id, protocol_id, contract_id, channel_id, protocol_state from dlc_protocols where trader_pubkey='02e66db0280277b35f88b1a1fcf58f8b2208568b6c7ea9eb071d894836865bef63';
 protocol_type |         previous_protocol_id         |             protocol_id              |                           contract_id                            |                            channel_id                            | protocol_state 
---------------+--------------------------------------+--------------------------------------+------------------------------------------------------------------+------------------------------------------------------------------+----------------
 open          |                                      | 6d6a8ded-997e-4ed1-a390-d626f271eee0 | a02000f42e1bee780c81f4e13f4623c0db6b047a9e59b70cf079eedc45b20801 | 990afbeff87f7ef9cd68b7982f35174a2283005d301130c621e4ed15feb2a5e5 | Success
 settle        | 6d6a8ded-997e-4ed1-a390-d626f271eee0 | 47c03bc0-eb46-4edc-96ff-e993d9f382be | a02000f42e1bee780c81f4e13f4623c0db6b047a9e59b70cf079eedc45b20801 | 990afbeff87f7ef9cd68b7982f35174a2283005d301130c621e4ed15feb2a5e5 | Success
 renew         | 47c03bc0-eb46-4edc-96ff-e993d9f382be | 2fd477dd-9a1b-4bd9-a86a-91eda2624415 | f2ff62b546ed3b43cb0031be56c512e3f5b6e4fd8cae89311ecfd4c6f21a6daa | 990afbeff87f7ef9cd68b7982f35174a2283005d301130c621e4ed15feb2a5e5 | Success
 rollover      | 2fd477dd-9a1b-4bd9-a86a-91eda2624415 | ad913883-bda0-4ffd-934e-df25610c08ae | 3336fd8a8b07d88dd1e5ab39f7b356f32db8d17f78d65f0f7214fd73caafad8f | 990afbeff87f7ef9cd68b7982f35174a2283005d301130c621e4ed15feb2a5e5 | Success
 settle        | ad913883-bda0-4ffd-934e-df25610c08ae | c22adca5-cc5b-4e1d-9ef9-33bf5e4aeee5 | 3336fd8a8b07d88dd1e5ab39f7b356f32db8d17f78d65f0f7214fd73caafad8f | 990afbeff87f7ef9cd68b7982f35174a2283005d301130c621e4ed15feb2a5e5 | Success

luckysori · 2024-03-01T01:15:13Z

@holzeis: Can we close #2107 with this PR? It's not the same solution, but I think what you are doing here is what I suggested originally anyway.

holzeis · 2024-03-01T08:29:29Z

@holzeis: Can we close #2107 with this PR? It's not the same solution, but I think what you are doing here is what I suggested originally anyway.

Yes, although we might want to keep it as i think it could be useful to have a link from the dlc_protocol to the dlc_messages. But its definitely less of a priority with this now.

luckysori

I think this makes sense overall, but I would like to read your responses before approving explicitly.

coordinator/src/node.rs

crates/ln-dlc-node/src/node/dlc_channel.rs

coordinator/src/dlc_protocol.rs

coordinator/src/db/positions.rs

coordinator/src/dlc_protocol.rs

coordinator/src/node.rs

luckysori · 2024-03-04T12:01:59Z

coordinator/src/node.rs


                        let protocol_executor =
                            dlc_protocol::DlcProtocolExecutor::new(self.pool.clone());
                        protocol_executor.finish_dlc_protocol(
                            protocol_id,
                            &node_id,
-                            &contract_id,
+                            channel.get_contract_id(),


🔧 Maybe we could just add a comment (and a named variable) for all these.

Not sure what you mean. Do you want a separate variable for the contract_id?

luckysori · 2024-03-04T12:05:32Z

crates/tests-e2e/tests/e2e_close_position.rs

@@ -89,6 +90,34 @@ async fn can_open_close_open_close_position() {
        .unwrap();
    tracing::info!(%app_off_chain_balance, "Opened second position");

+    // rolling over before closing the second position reproduces https://github.com/get10101/10101/issues/2126


🙃 Probably controversial, but could we add a dedicated test? I suppose the problem would happen with the first position too. It's a bit of an arbitrary spot to do this.

Also, I think the test is no longer reproducing the issue because the issue is already fixed by now. Can you adapt the comment? I was a bit confused with the wording.

luckysori

I think this is fine after thinking a bit more about this thread: https://github.com/get10101/10101/pull/2125/files#r1511020380.

This allows us to track easily in the database what action triggered the dlc protocol.

chore(settle): Disregard additional check if contract is actually closed. This is the responsibility of `rust-dlc`. Removing this check as it adds unnecessary complexity. We can assume that if `rust-dlc` successfully processed the `ChannelMessage::SettleFinalize` that the contract got closed.

Verifies that #2126 is fixed

holzeis requested review from bonomat and luckysori February 29, 2024 15:26

holzeis self-assigned this Feb 29, 2024

holzeis force-pushed the chore/add-rollover-to-protocols-table branch from a5fbfb1 to 8e79dac Compare February 29, 2024 15:27

holzeis linked an issue Feb 29, 2024 that may be closed by this pull request

Add rollover to protocols table #2103

Closed

holzeis mentioned this pull request Feb 29, 2024

Failed at closing position (foreign key constraint violation) #2126

Closed

holzeis force-pushed the chore/add-rollover-to-protocols-table branch from 4e5e736 to f3d91d6 Compare March 1, 2024 08:24

holzeis linked an issue Mar 1, 2024 that may be closed by this pull request

Failed at closing position (foreign key constraint violation) #2126

Closed

holzeis force-pushed the chore/add-rollover-to-protocols-table branch from e30a9f8 to 9dc885d Compare March 1, 2024 11:24

holzeis linked an issue Mar 4, 2024 that may be closed by this pull request

Closed position remains in Closing #2142

Closed

holzeis force-pushed the chore/add-rollover-to-protocols-table branch from 9dc885d to 744a63d Compare March 4, 2024 08:15

luckysori reviewed Mar 4, 2024

View reviewed changes

luckysori approved these changes Mar 4, 2024

View reviewed changes

holzeis force-pushed the chore/add-rollover-to-protocols-table branch from 744a63d to 1d5388a Compare March 4, 2024 14:28

holzeis added 8 commits March 4, 2024 15:29

chore: Rename contract executor to protocol executor

02b19e9

chore: Pass contract and channel id by reference

c7be71c

chore(dlc_channel): Use contract id type as return argument

9ddb759

chore(dlc_channel): Fix typo

c37314a

feat(dlc_protocol): Add dlc protocol type

c81cf5c

This allows us to track easily in the database what action triggered the dlc protocol.

feat(dlc_protocol): Add dlc protocol entry for rollovers

cc34d88

chore(test): Ensure dlc channel is settled in close position test

b2b0e3f

Verifies that #2126 is fixed

holzeis force-pushed the chore/add-rollover-to-protocols-table branch from 1d5388a to b2b0e3f Compare March 4, 2024 14:29

holzeis added this pull request to the merge queue Mar 4, 2024

Merged via the queue into main with commit 87c8c0f Mar 4, 2024
20 checks passed

holzeis deleted the chore/add-rollover-to-protocols-table branch March 4, 2024 14:45

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore(dlc_protocol): Track rollovers in dlc protocol table #2125

chore(dlc_protocol): Track rollovers in dlc protocol table #2125

holzeis commented Feb 29, 2024

luckysori commented Mar 1, 2024 •

edited

Loading

holzeis commented Mar 1, 2024 •

edited

Loading

luckysori left a comment

luckysori Mar 4, 2024

holzeis Mar 4, 2024

luckysori Mar 4, 2024

luckysori Mar 4, 2024

luckysori left a comment

chore(dlc_protocol): Track rollovers in dlc protocol table #2125

chore(dlc_protocol): Track rollovers in dlc protocol table #2125

Conversation

holzeis commented Feb 29, 2024

luckysori commented Mar 1, 2024 • edited Loading

holzeis commented Mar 1, 2024 • edited Loading

luckysori left a comment

Choose a reason for hiding this comment

luckysori Mar 4, 2024

Choose a reason for hiding this comment

holzeis Mar 4, 2024

Choose a reason for hiding this comment

luckysori Mar 4, 2024

Choose a reason for hiding this comment

luckysori Mar 4, 2024

Choose a reason for hiding this comment

luckysori left a comment

Choose a reason for hiding this comment

luckysori commented Mar 1, 2024 •

edited

Loading

holzeis commented Mar 1, 2024 •

edited

Loading