Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feat/periodic check returns affected contracts #121

Open
wants to merge 12 commits into
base: fix/fee-comtputation-issue
Choose a base branch
from
3 changes: 2 additions & 1 deletion dlc-manager/src/contract/offered_contract.rs
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,8 @@ impl OfferedContract {
}
}

pub(crate) fn try_from_offer_dlc(
/// Convert an [`OfferDlc`] message to an [`OfferedContract`].
pub fn try_from_offer_dlc(
offer_dlc: &OfferDlc,
counter_party: PublicKey,
) -> Result<OfferedContract, crate::conversion_utils::Error> {
Expand Down
37 changes: 37 additions & 0 deletions dlc-manager/src/contract_updater.rs
Original file line number Diff line number Diff line change
Expand Up @@ -734,3 +734,40 @@ where
)?;
Ok(refund)
}

#[cfg(test)]
mod tests {
use std::rc::Rc;

use mocks::dlc_manager::contract::offered_contract::OfferedContract;
use secp256k1_zkp::PublicKey;

#[test]
fn accept_contract_test() {
let offer_dlc =
serde_json::from_str(include_str!("../test_inputs/offer_contract.json")).unwrap();
let dummy_pubkey: PublicKey =
"02e6642fd69bd211f93f7f1f36ca51a26a5290eb2dd1b0d8279a87bb0d480c8443"
.parse()
.unwrap();
let offered_contract =
OfferedContract::try_from_offer_dlc(&offer_dlc, dummy_pubkey).unwrap();
let blockchain = Rc::new(mocks::mock_blockchain::MockBlockchain::new());
let fee_rate: u64 = offered_contract.fee_rate_per_vb;
let utxo_value: u64 = offered_contract.total_collateral
- offered_contract.offer_params.collateral
+ crate::utils::get_half_common_fee(fee_rate).unwrap();
let wallet = Rc::new(mocks::mock_wallet::MockWallet::new(
&blockchain,
&[utxo_value, 10000],
));

mocks::dlc_manager::contract_updater::accept_contract(
secp256k1_zkp::SECP256K1,
&offered_contract,
&wallet,
&blockchain,
)
.expect("Not to fail");
}
}
11 changes: 11 additions & 0 deletions dlc-manager/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -199,3 +199,14 @@ impl_dlc_writeable!(Utxo, {
(redeem_script, writeable),
(reserved, writeable)
});

/// The response from periodic_check fn which returns the ids of the contracts that were affected in each state.
#[derive(Clone, Debug)]
pub struct AffectedContractIDs {
sosaucily marked this conversation as resolved.
Show resolved Hide resolved
/// The list of contracts that were moved into the confirmed state.
pub confirmed_contracts: Vec<ContractId>,
/// The list of contracts that were moved into the pre-closed state.
pub pre_closed_contracts: Vec<ContractId>,
/// The list of contracts that were moved into the closed state.
pub closed_contracts: Vec<ContractId>,
}
122 changes: 88 additions & 34 deletions dlc-manager/src/manager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ use crate::contract::{
};
use crate::contract_updater::{accept_contract, verify_accepted_and_sign_contract};
use crate::error::Error;
use crate::Signer;
use crate::{AffectedContractIDs, Signer};
use crate::{ChannelId, ContractId};
use bitcoin::Address;
use bitcoin::Transaction;
Expand Down Expand Up @@ -330,13 +330,15 @@ where

/// Function to call to check the state of the currently executing DLCs and
/// update them if possible.
pub fn periodic_check(&mut self) -> Result<(), Error> {
self.check_signed_contracts()?;
self.check_confirmed_contracts()?;
self.check_preclosed_contracts()?;
pub fn periodic_check(&mut self) -> Result<AffectedContractIDs, Error> {
let affected_contracts = AffectedContractIDs {
confirmed_contracts: self.check_signed_contracts()?,
pre_closed_contracts: self.check_confirmed_contracts()?,
closed_contracts: self.check_preclosed_contracts()?,
};
self.channel_checks()?;

Ok(())
Ok(affected_contracts)
}

fn on_offer_message(
Expand Down Expand Up @@ -470,47 +472,59 @@ where
Err(e)
}

fn check_signed_contract(&mut self, contract: &SignedContract) -> Result<(), Error> {
fn check_signed_contract(
&mut self,
contract: &SignedContract,
) -> Result<Option<ContractId>, Error> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It feels like it'd be easier to just return a boolean here, indicating whether the contract was updated or not.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

smart!!!

let confirmations = self.blockchain.get_transaction_confirmations(
&contract.accepted_contract.dlc_transactions.fund.txid(),
)?;
if confirmations >= NB_CONFIRMATIONS {
self.store
.update_contract(&Contract::Confirmed(contract.clone()))?;
return Ok(Some(contract.accepted_contract.get_contract_id()));
}
Ok(())
Ok(None)
}

fn check_signed_contracts(&mut self) -> Result<(), Error> {
fn check_signed_contracts(&mut self) -> Result<Vec<ContractId>, Error> {
let mut contracts_to_confirm = Vec::new();
for c in self.store.get_signed_contracts()? {
if let Err(e) = self.check_signed_contract(&c) {
error!(
match self.check_signed_contract(&c) {
Ok(Some(contract_id)) => contracts_to_confirm.push(contract_id),
Ok(None) => (),
Err(e) => error!(
"Error checking confirmed contract {}: {}",
c.accepted_contract.get_contract_id_string(),
e
)
),
}
}

Ok(())
Ok(contracts_to_confirm)
}

fn check_confirmed_contracts(&mut self) -> Result<(), Error> {
fn check_confirmed_contracts(&mut self) -> Result<Vec<ContractId>, Error> {
let mut contracts_to_close = Vec::new();
for c in self.store.get_confirmed_contracts()? {
// Confirmed contracts from channel are processed in channel specific methods.
if c.channel_id.is_some() {
continue;
}
if let Err(e) = self.check_confirmed_contract(&c) {
error!(
"Error checking confirmed contract {}: {}",
c.accepted_contract.get_contract_id_string(),
e
)
match self.check_confirmed_contract(&c) {
Err(e) => {
error!(
"Error checking confirmed contract {}: {}",
c.accepted_contract.get_contract_id_string(),
e
)
}
Ok(Some(contract_id)) => contracts_to_close.push(contract_id),
Ok(None) => (),
}
}

Ok(())
Ok(contracts_to_close)
}

fn get_closable_contract_info<'a>(
Expand Down Expand Up @@ -549,7 +563,10 @@ where
None
}

fn check_confirmed_contract(&mut self, contract: &SignedContract) -> Result<(), Error> {
fn check_confirmed_contract(
&mut self,
contract: &SignedContract,
) -> Result<Option<ContractId>, Error> {
let closable_contract_info = self.get_closable_contract_info(contract);
if let Some((contract_info, adaptor_info, attestations)) = closable_contract_info {
let cet = crate::contract_updater::get_signed_cet(
Expand All @@ -567,7 +584,7 @@ where
) {
Ok(closed_contract) => {
self.store.update_contract(&closed_contract)?;
return Ok(());
return Ok(Some(closed_contract.get_id()));
}
Err(e) => {
warn!(
Expand All @@ -582,24 +599,30 @@ where

self.check_refund(contract)?;

Ok(())
Ok(None)
}

fn check_preclosed_contracts(&mut self) -> Result<(), Error> {
fn check_preclosed_contracts(&mut self) -> Result<Vec<ContractId>, Error> {
let mut contracts_to_close = Vec::new();
for c in self.store.get_preclosed_contracts()? {
if let Err(e) = self.check_preclosed_contract(&c) {
error!(
match self.check_preclosed_contract(&c) {
Ok(Some(contract_id)) => contracts_to_close.push(contract_id),
Ok(None) => (),
Err(e) => error!(
"Error checking pre-closed contract {}: {}",
c.signed_contract.accepted_contract.get_contract_id_string(),
e
)
),
}
}

Ok(())
Ok(contracts_to_close)
}

fn check_preclosed_contract(&mut self, contract: &PreClosedContract) -> Result<(), Error> {
fn check_preclosed_contract(
&mut self,
contract: &PreClosedContract,
) -> Result<Option<ContractId>, Error> {
let broadcasted_txid = contract.signed_cet.txid();
let confirmations = self
.blockchain
Expand All @@ -625,10 +648,11 @@ where
.compute_pnl(&contract.signed_cet),
};
self.store
.update_contract(&Contract::Closed(closed_contract))?;
.update_contract(&Contract::Closed(closed_contract.clone()))?;
return Ok(Some(closed_contract.contract_id));
}

Ok(())
Ok(None)
}

fn close_contract(
Expand Down Expand Up @@ -2199,6 +2223,8 @@ mod test {
use secp256k1_zkp::PublicKey;
use std::{collections::HashMap, rc::Rc};

use crate::AffectedContractIDs;

type TestManager = Manager<
Rc<MockWallet>,
Rc<MockBlockchain>,
Expand All @@ -2209,9 +2235,12 @@ mod test {
>;

fn get_manager() -> TestManager {
let blockchain = Rc::new(MockBlockchain {});
let blockchain = Rc::new(MockBlockchain::new());
let store = Rc::new(MemoryStorage::new());
let wallet = Rc::new(MockWallet::new(&blockchain, 100));
let wallet = Rc::new(MockWallet::new(
&blockchain,
&(0..100).map(|x| x as u64 * 1000000).collect::<Vec<_>>(),
));

let oracle_list = (0..5).map(|_| MockOracle::new()).collect::<Vec<_>>();
let oracles: HashMap<bitcoin::XOnlyPublicKey, _> = oracle_list
Expand Down Expand Up @@ -2264,4 +2293,29 @@ mod test {
.on_dlc_message(&offer_message, pubkey())
.expect_err("To reject the second offer message");
}

#[test]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that's not a great test, I think it'd be better to add some checks in the integrations tests.

fn periodic_check_should_return_affected_contracts() {
let mut manager = get_manager();

let offer_message = Message::Offer(
serde_json::from_str(include_str!("../test_inputs/offer_contract.json")).unwrap(),
);

manager
.on_dlc_message(&offer_message, pubkey())
.expect("To accept the first offer message");

let affected_contracts = manager.periodic_check().unwrap();

let expected_affected_contracts = AffectedContractIDs {
confirmed_contracts: vec![],
pre_closed_contracts: vec![],
closed_contracts: vec![],
};
assert_eq!(
expected_affected_contracts.confirmed_contracts,
affected_contracts.confirmed_contracts
);
}
}
18 changes: 7 additions & 11 deletions dlc-manager/src/utils.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
//! #Utils
use std::ops::Deref;

use bitcoin::{consensus::Encodable, Txid};
Expand All @@ -18,13 +19,6 @@ use crate::{
Blockchain, Wallet,
};

const APPROXIMATE_CET_VBYTES: u64 = 190;
const APPROXIMATE_CLOSING_VBYTES: u64 = 168;

pub fn get_common_fee(fee_rate: u64) -> u64 {
(APPROXIMATE_CET_VBYTES + APPROXIMATE_CLOSING_VBYTES) * fee_rate
}

#[cfg(not(feature = "fuzztarget"))]
pub(crate) fn get_new_serial_id() -> u64 {
thread_rng().next_u64()
Expand Down Expand Up @@ -86,7 +80,9 @@ where
let change_spk = change_addr.script_pubkey();
let change_serial_id = get_new_serial_id();

let appr_required_amount = own_collateral + get_half_common_fee(fee_rate);
// Add base cost of fund tx + CET / 2 and a CET output to the collateral.
let appr_required_amount =
own_collateral + get_half_common_fee(fee_rate)? + dlc::util::weight_to_fee(124, fee_rate)?;
let utxos = wallet.get_utxos_for_amount(appr_required_amount, Some(fee_rate), true)?;

let mut funding_inputs_info: Vec<FundingInputInfo> = Vec::new();
Expand Down Expand Up @@ -145,9 +141,9 @@ where
})
}

fn get_half_common_fee(fee_rate: u64) -> u64 {
let common_fee = get_common_fee(fee_rate);
(common_fee as f64 / 2_f64).ceil() as u64
pub(crate) fn get_half_common_fee(fee_rate: u64) -> Result<u64, Error> {
let common_fee = dlc::util::get_common_fee(fee_rate)?;
Ok((common_fee as f64 / 2_f64).ceil() as u64)
}

pub(crate) fn get_range_info_and_oracle_sigs(
Expand Down
2 changes: 1 addition & 1 deletion dlc-manager/test_inputs/offer_contract.json
Original file line number Diff line number Diff line change
@@ -1 +1 @@
{"protocolVersion":1,"contractFlags":0,"chainHash":"06226e46111a0b59caaf126043eb5bbf28c34f3a5e332a1fc7b2b73cf188910f","temporaryContractId":"f6a1b2841c93db06e94200b227bb4bdea83068efa557d68e14775237cbaab56a","contractInfo":{"singleContractInfo":{"totalCollateral":101000000,"contractInfo":{"contractDescriptor":{"numericOutcomeContractDescriptor":{"numDigits":10,"payoutFunction":{"payoutFunctionPieces":[{"endPoint":{"eventOutcome":0,"outcomePayout":0,"extraPrecision":0},"payoutCurvePiece":{"polynomialPayoutCurvePiece":{"payoutPoints":[{"eventOutcome":3,"outcomePayout":90000000,"extraPrecision":0}]}}},{"endPoint":{"eventOutcome":5,"outcomePayout":101000000,"extraPrecision":0},"payoutCurvePiece":{"polynomialPayoutCurvePiece":{"payoutPoints":[]}}}],"lastEndpoint":{"eventOutcome":1023,"outcomePayout":101000000,"extraPrecision":0}},"roundingIntervals":{"intervals":[{"beginInterval":0,"roundingMod":1}]}}},"oracleInfo":{"single":{"oracleAnnouncement":{"announcementSignature":"18e18de8b3547e210addd32589db9520286f55c0c18510c67bb6f8ea66b05154b84c6ec0075e3623f886b7e2bc623b7df25e1bc25d1cc87c622b28f0ae526664","oraclePublicKey":"1d524d2753a36ebe340af67370f78219b4dbb6f56d2f96b3b21eaabec6f4a114","oracleEvent":{"oracleNonces":["bc927a2c8bf43c9d208e679848ffaf95d178fdbd2e29d1c66668f21dd75149e8","9ed74e19c1d532f5127829b7d9f183e0738ad084485428b53a7fe0c50f2efe5e","f44733d1129d0cd9253124749f8cff2c7e7eecd79888a4a015d3e3ad153ef282","f4f39e5733bfc5ca18530eb444419b31d9dc0ec938502615c33f2b0b7c05ac71","930991374fbf6b9a49e5e16fa3c5c39638af58f5a4c55682a93b2b940502e7bf","e3af3b59907c349d627e3f4f20125bdc1e979cac41ee82ef0a184000c79e904b","0b95d4335713752329a1791b963d526c0a49873bbbfcad9e1c03881508b2a801","48776cc1e3b8f3ff7fd6226ea2df5607787913468a1c0faad4ff315b7cf3b41d","0b39b0e1a14f5f50cb05f0a6d8e7c082f75e9fe386006727af933ce4d273a76f","479a38e13c1622bfd53299ee67680d7a0edd3fed92223e3a878c8d010fcc1a2d"],"eventMaturityEpoch":1623133104,"eventDescriptor":{"digitDecompositionEvent":{"base":2,"isSigned":false,"unit":"sats/sec","precision":0,"nbDigits":10}},"eventId":"Test"}}}}}}},"fundingPubkey":"02556021f6abda2ae7a74d38a4e4a3b00c1dd648db96397dcd8642c3d0b0b139d1","payoutSpk":"0014430af74f2f9dc88729fd02eaeb946fc161e2be1e","payoutSerialId":8165863461276958928,"offerCollateral":90000000,"fundingInputs":[{"inputSerialId":11632658032743242199,"prevTx":"02000000000101e79f7a30bb35206060eb09a99b6956bcdc7a1767b310c8dfde3595c69246a60e0000000000feffffff0200c2eb0b000000001600142a416c1e5f5e78bc6c518294fd1dd86b40eed2d77caf953e000000001600148e56705661334df89b2c1c7c4e41da9cef9eb38e0247304402201491f05ebe196b333420cbab3e7e7f3e431bfe91a42730cef9c6e64b0e8ff62302202c5fc79abbdb0a1c8ad422dbb97a54693feedc580f0cb7a62bdadaecbfc4f9430121035f57172a38f35f29f4357dcc2d24ea8e72638cf43190e4fdcb3f0ace215cfd5602020000","prevTxVout":0,"sequence":4294967295,"maxWitnessLen":107,"redeemScript":""}],"changeSpk":"001441ca183be469eab996f34ed31197a96b57f6050e","changeSerialId":16919534260907952016,"fundOutputSerialId":5054305248376932341,"feeRatePerVb":2,"cetLocktime":1623133103,"refundLocktime":1623737904}
{"protocolVersion":1,"contractFlags":0,"chainHash":"06226e46111a0b59caaf126043eb5bbf28c34f3a5e332a1fc7b2b73cf188910f","temporaryContractId":"f6a1b2841c93db06e94200b227bb4bdea83068efa557d68e14775237cbaab56a","contractInfo":{"singleContractInfo":{"totalCollateral":101000000,"contractInfo":{"contractDescriptor":{"numericOutcomeContractDescriptor":{"numDigits":10,"payoutFunction":{"payoutFunctionPieces":[{"endPoint":{"eventOutcome":0,"outcomePayout":0,"extraPrecision":0},"payoutCurvePiece":{"polynomialPayoutCurvePiece":{"payoutPoints":[{"eventOutcome":3,"outcomePayout":90000000,"extraPrecision":0}]}}},{"endPoint":{"eventOutcome":5,"outcomePayout":101000000,"extraPrecision":0},"payoutCurvePiece":{"polynomialPayoutCurvePiece":{"payoutPoints":[]}}}],"lastEndpoint":{"eventOutcome":1023,"outcomePayout":101000000,"extraPrecision":0}},"roundingIntervals":{"intervals":[{"beginInterval":0,"roundingMod":1}]}}},"oracleInfo":{"single":{"oracleAnnouncement":{"announcementSignature":"18e18de8b3547e210addd32589db9520286f55c0c18510c67bb6f8ea66b05154b84c6ec0075e3623f886b7e2bc623b7df25e1bc25d1cc87c622b28f0ae526664","oraclePublicKey":"1d524d2753a36ebe340af67370f78219b4dbb6f56d2f96b3b21eaabec6f4a114","oracleEvent":{"oracleNonces":["bc927a2c8bf43c9d208e679848ffaf95d178fdbd2e29d1c66668f21dd75149e8","9ed74e19c1d532f5127829b7d9f183e0738ad084485428b53a7fe0c50f2efe5e","f44733d1129d0cd9253124749f8cff2c7e7eecd79888a4a015d3e3ad153ef282","f4f39e5733bfc5ca18530eb444419b31d9dc0ec938502615c33f2b0b7c05ac71","930991374fbf6b9a49e5e16fa3c5c39638af58f5a4c55682a93b2b940502e7bf","e3af3b59907c349d627e3f4f20125bdc1e979cac41ee82ef0a184000c79e904b","0b95d4335713752329a1791b963d526c0a49873bbbfcad9e1c03881508b2a801","48776cc1e3b8f3ff7fd6226ea2df5607787913468a1c0faad4ff315b7cf3b41d","0b39b0e1a14f5f50cb05f0a6d8e7c082f75e9fe386006727af933ce4d273a76f","479a38e13c1622bfd53299ee67680d7a0edd3fed92223e3a878c8d010fcc1a2d"],"eventMaturityEpoch":1623133104,"eventDescriptor":{"digitDecompositionEvent":{"base":2,"isSigned":false,"unit":"sats/sec","precision":0,"nbDigits":10}},"eventId":"Test"}}}}}}},"fundingPubkey":"02556021f6abda2ae7a74d38a4e4a3b00c1dd648db96397dcd8642c3d0b0b139d1","payoutSpk":"0014430af74f2f9dc88729fd02eaeb946fc161e2be1e","payoutSerialId":8165863461276958928,"offerCollateral":90000000,"fundingInputs":[{"inputSerialId":11632658032743242199,"prevTx":"02000000000101e79f7a30bb35206060eb09a99b6956bcdc7a1767b310c8dfde3595c69246a60e0000000000feffffff0200c2eb0b000000001600142a416c1e5f5e78bc6c518294fd1dd86b40eed2d77caf953e000000001600148e56705661334df89b2c1c7c4e41da9cef9eb38e0247304402201491f05ebe196b333420cbab3e7e7f3e431bfe91a42730cef9c6e64b0e8ff62302202c5fc79abbdb0a1c8ad422dbb97a54693feedc580f0cb7a62bdadaecbfc4f9430121035f57172a38f35f29f4357dcc2d24ea8e72638cf43190e4fdcb3f0ace215cfd5602020000","prevTxVout":0,"sequence":4294967295,"maxWitnessLen":107,"redeemScript":""}],"changeSpk":"001441ca183be469eab996f34ed31197a96b57f6050e","changeSerialId":16919534260907952016,"fundOutputSerialId":5054305248376932341,"feeRatePerVb":2,"cetLocktime":1623133103,"refundLocktime":1623737904}
2 changes: 1 addition & 1 deletion dlc-trie/src/digit_trie.rs
Original file line number Diff line number Diff line change
Expand Up @@ -259,7 +259,7 @@ impl<'a, T> Iterator for DigitTrieIter<'a, T> {
while cur_child < (self.trie.base as isize) {
self.index_stack.push((Some(cur_index), cur_child + 1));
self.index_stack
.push((cur_children[(cur_child as usize)], -1));
.push((cur_children[cur_child as usize], -1));
match self.next() {
None => {
self.index_stack.pop();
Expand Down
2 changes: 1 addition & 1 deletion dlc/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ version = "0.4.0"

[dependencies]
bitcoin = {version = "0.29.2"}
miniscript = "8.0.0"
miniscript = { version = "9.0", features = ["serde", "std"], default-features = false }
sosaucily marked this conversation as resolved.
Show resolved Hide resolved
secp256k1-sys = {version = "0.6.1" }
secp256k1-zkp = {version = "0.7.0", features = ["bitcoin_hashes", "rand-std"]}
serde = {version = "1.0", default-features = false, optional = true}
Expand Down
9 changes: 8 additions & 1 deletion dlc/src/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -94,12 +94,19 @@ pub fn get_sig_for_p2wpkh_input<C: Signing>(
)
}

pub(crate) fn weight_to_fee(weight: usize, fee_rate: u64) -> Result<u64, Error> {
/// Returns the fee for the given weight at given fee rate.
pub fn weight_to_fee(weight: usize, fee_rate: u64) -> Result<u64, Error> {
(f64::ceil((weight as f64) / 4.0) as u64)
.checked_mul(fee_rate)
.ok_or(Error::InvalidArgument)
}

/// Return the common base fee for a DLC for the given fee rate.
pub fn get_common_fee(fee_rate: u64) -> Result<u64, Error> {
let base_weight = crate::FUND_TX_BASE_WEIGHT + crate::CET_BASE_WEIGHT;
weight_to_fee(base_weight, fee_rate)
}

fn get_pkh_script_pubkey_from_sk<C: Signing>(secp: &Secp256k1<C>, sk: &SecretKey) -> Script {
use bitcoin::hashes::*;
let pk = bitcoin::PublicKey {
Expand Down
Loading