fork rust dlc crates#106
Conversation
bennyhodl
force-pushed
the
fork-rust-dlc-crates
branch
from
1cdefca to
729ac7a
Compare
matthewjablack
left a comment
matthewjablack
left a comment
There was a problem hiding this comment.
Do we need a calculateMaxCollateral from utxos?
| let max_witness_len = if fund_input.dlc_input.is_some() { | ||
| 220 | ||
| } else { | ||
| 107 |
There was a problem hiding this comment.
Do we need to worried about this being 108? I think this is set to 108 in node-dlc
There was a problem hiding this comment.
rust-dlc has it at 107, i just continued that4
|
|
||
| let key_id = match contract { | ||
| Contract::Confirmed(c) => Ok(c.accepted_contract.offered_contract.keys_id), | ||
| _ => Err(Error::InvalidState( |
There was a problem hiding this comment.
Might be good to change this to Signed (or have a state PreConfirmed) because if you initially created a single funding transaction with no change, then you have no way to fee bump, other than splice in more collateral as a CPFP
There was a problem hiding this comment.
On second thought, this seems unnecessary unless it becomes a large problem down the road
bennyhodl
force-pushed
the
fork-rust-dlc-crates
branch
2 times, most recently
from
1dfde3a to
c93dfda
Compare
bennyhodl
force-pushed
the
fork-rust-dlc-crates
branch
3 times, most recently
from
41421dc to
fd9a283
Compare
bennyhodl
force-pushed
the
fork-rust-dlc-crates
branch
from
fd9a283 to
ed5744b
Compare
matthewjablack
left a comment
matthewjablack
left a comment
There was a problem hiding this comment.
Looking solid, just a couple edge cases for tests that could be added
|
|
||
| self.store | ||
| .update_contract(&Contract::PreClosed(preclosed_contract.clone())) | ||
| .await?; |
There was a problem hiding this comment.
What happens if splice transaction ends up being RBF'd?
I'm assuming this doesn't block it from changing to Close somewhere else with an execution transaction
| pub fund: Transaction, | ||
| /// The contract execution transactions for closing the contract on a | ||
| /// certain outcome | ||
| pub cets: Vec<Transaction>, |
There was a problem hiding this comment.
To confirm, a splice tx is just added to cets correct (in the DlcTransactions object)?
| .send_splice_offer( | ||
| &test_params.contract_input, | ||
| spliced_input.bob_public_key, | ||
| &spliced_input.contract_id, |
There was a problem hiding this comment.
Edge case, what if we try to splice the same contract twice?
There was a problem hiding this comment.
Also would be good to have a test that references a contract ID that doesn't exist
i.e. fake_contract_id
There was a problem hiding this comment.
And maybe a test with collateral below dust as well (to make sure that fails)
bennyhodl
force-pushed
the
fork-rust-dlc-crates
branch
from
25c5c08 to
20295bd
Compare
2 participants
This PR pulls in
rust-dlc/{dlc, dlc-messages, dlc-trie}to dlcdevkitThe decision was made after slow release times and some related features to improve
dlcdevkitAlong with pulling in the crates it includes new features that are ported from my fork of rust-dlc
single funded
splicing