Skip to content

Commit

Permalink
WIP: fix tests?
Browse files Browse the repository at this point in the history
  • Loading branch information
danielabrozzoni committed Nov 17, 2021
1 parent 0c26b28 commit 1199102
Show file tree
Hide file tree
Showing 10 changed files with 45 additions and 37 deletions.
24 changes: 17 additions & 7 deletions src/daemon/bitcoind/poller.rs
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,9 @@ fn maybe_broadcast_spend_transactions(
// be part of the CPFP wallet work.
db_mark_broadcasted_spend(&db_path, &txid)?;

if db_transaction_has_priority(&db_path, &txid)? {
if revaultd.read().unwrap().is_manager()
&& db_transaction_has_priority(&db_path, &txid)?
{
let broadcasted_blockheight = bitcoind.get_tip()?.height;
needs_cpfp_cache.insert(
txid,
Expand Down Expand Up @@ -554,8 +556,10 @@ fn new_tip_event(
mark_confirmed_emers(revaultd, bitcoind, unvaults_cache)?;
mark_confirmed_unemers(revaultd, bitcoind, unvaults_cache)?;

// Do our transactions need CPFP?
maybe_cpfp_txs(revaultd, bitcoind, needs_cpfp_cache)?;
if revaultd.read().unwrap().is_manager() {
// Do our transactions need CPFP?
maybe_cpfp_txs(revaultd, bitcoind, needs_cpfp_cache)?;
}
Ok(())
}

Expand Down Expand Up @@ -924,7 +928,9 @@ fn comprehensive_rescan(
&unvault_tx,
)?;

if db_transaction_has_priority_dbtx(db_tx, &unvault_txid)? {
if revaultd.read().unwrap().is_manager()
&& db_transaction_has_priority_dbtx(db_tx, &unvault_txid)?
{
let broadcasted_blockheight = bitcoind.get_tip()?.height;
needs_cpfp_cache.insert(
unvault_txid,
Expand Down Expand Up @@ -985,7 +991,9 @@ fn comprehensive_rescan(
spend_txid
);

if db_transaction_has_priority_dbtx(db_tx, &spend_txid)? {
if revaultd.read().unwrap().is_manager()
&& db_transaction_has_priority_dbtx(db_tx, &spend_txid)?
{
let broadcasted_blockheight = bitcoind.get_tip()?.height;
let tx = match db_spend_transaction_dbtx(&db_tx, &spend_txid)? {
Some(s) => CpfpTx::Spend(s.psbt),
Expand Down Expand Up @@ -1489,7 +1497,9 @@ fn handle_spent_deposit(
// Was it spent by an Unvault tx? No worry if the Unvault txo was spent too, it'll be
// noticed when we poll them next.
if bitcoind.is_current(&unvault_outpoint.txid)? {
if db_transaction_has_priority(db_path, &unvault_outpoint.txid)? {
if revaultd.read().unwrap().is_manager()
&& db_transaction_has_priority(db_path, &unvault_outpoint.txid)?
{
// I really need the UnvaultTransaction here :sob: This same call
// was done in the unvault_txin_from_deposit fn and I'm doing it again
// here. So sad.
Expand Down Expand Up @@ -1792,7 +1802,7 @@ fn maybe_create_wallet(revaultd: &mut RevaultD, bitcoind: &BitcoinD) -> Result<(
log::trace!("Importing unvault descriptors '{:?}'", &addresses);
bitcoind.startup_import_unvault_descriptors(addresses, wallet.timestamp, fresh_wallet)?;

if revaultd.our_man_xpub.is_some() {
if revaultd.is_manager() {
// Importing the CPFP addresses as well
let mut addresses = revaultd.all_cpfp_addresses();
for i in 0..addresses.len() {
Expand Down
3 changes: 2 additions & 1 deletion src/daemon/database/actions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -756,7 +756,7 @@ pub fn db_insert_spend(

for unvault_tx in unvault_txs.into_iter() {
db_tx.execute(
"INSERT INTO spend_inputs (unvault_id, spend_id, has_priority) VALUES (?1, ?2, NULL)",
"INSERT INTO spend_inputs (unvault_id, spend_id, has_priority) VALUES (?1, ?2, false)",
params![unvault_tx.id, spend_id],
)?;
}
Expand All @@ -770,6 +770,7 @@ pub fn db_update_spend(
spend_tx: &SpendTransaction,
has_priority: Option<bool>,
) -> Result<(), DatabaseError> {
let has_priority = has_priority.unwrap_or(false);
let spend_txid = spend_tx.txid();
let spend_psbt = spend_tx.as_psbt_serialized();

Expand Down
3 changes: 3 additions & 0 deletions src/daemon/database/interface.rs
Original file line number Diff line number Diff line change
Expand Up @@ -873,6 +873,9 @@ pub fn db_vaults_from_spend(
Ok(db_vaults)
}

/// Returns a boolean representing the transaction priority. The transaction should be
/// an unvault or a spend, otherwise false will be returned. If the transaction is not
/// found in the database, false will be returned.
pub fn db_transaction_has_priority(db_path: &Path, txid: &Txid) -> Result<bool, DatabaseError> {
// We don't know if this tx is an unvault or a spend...
db_query(
Expand Down
2 changes: 1 addition & 1 deletion src/daemon/database/schema.rs
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ CREATE TABLE spend_inputs (
id INTEGER PRIMARY KEY NOT NULL,
unvault_id INTEGER NOT NULL,
spend_id INTEGER NOT NULL,
has_priority BOOLEAN CHECK (has_priority IN (0,1, NULL)),
has_priority BOOLEAN NOT NULL CHECK (has_priority IN (0,1)) DEFAULT 0,
FOREIGN KEY (unvault_id) REFERENCES presigned_transactions (id)
ON UPDATE RESTRICT
ON DELETE RESTRICT,
Expand Down
5 changes: 3 additions & 2 deletions src/daemon/jsonrpc/api/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,7 @@ pub trait RpcApi {
&self,
meta: Self::Metadata,
spend_txid: Txid,
priority: bool,
priority: Option<bool>,
) -> jsonrpc_core::Result<serde_json::Value>;

#[rpc(meta, name = "revault")]
Expand Down Expand Up @@ -1320,10 +1320,11 @@ impl RpcApi for RpcImpl {
&self,
meta: Self::Metadata,
spend_txid: Txid,
priority: bool,
priority: Option<bool>,
) -> jsonrpc_core::Result<serde_json::Value> {
manager_only!(meta);

let priority = priority.unwrap_or(false);
let revaultd = meta.rpc_utils.revaultd.read().unwrap();
let db_path = revaultd.db_file();

Expand Down
8 changes: 5 additions & 3 deletions src/daemon/revaultd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -299,8 +299,9 @@ pub struct RevaultD {
pub secp_ctx: secp256k1::Secp256k1<secp256k1::VerifyOnly>,
/// The locktime to use on all created transaction. Always 0 for now.
pub lock_time: u32,
/// CPFP private key to fee-bump transactions. Managers have it if they create a cpfp_secret
/// file containing the key. If you don't have this key, you obviously can't cpfp.
/// CPFP private key to fee-bump Unvault and Spend transactions.
/// In the absence of the key file in the data directory, the automated CPFP mechanism
/// is silently disabled.
pub cpfp_key: Option<ExtendedPrivKey>,

// Network stuff
Expand Down Expand Up @@ -408,7 +409,8 @@ impl RevaultD {
.ok_or(CpfpKeyError::KeyNotInDescriptor)?;
} else {
log::warn!(
"CPFP key not found, consider creating a cpfp_secret file in the datadir"
"CPFP key not found, consider creating a cpfp_secret file in the datadir.\
Automated CPFP won't be available."
);
}
key
Expand Down
18 changes: 5 additions & 13 deletions tests/test_framework/revault_network.py
Original file line number Diff line number Diff line change
Expand Up @@ -353,7 +353,7 @@ def deploy(
stk_config,
man_config,
wt_process=miradord if with_watchtowers else None,
stkman_cpfp_privs[i].get_xpriv_bytes() if with_cpfp else None,
cpfp_priv=stkman_cpfp_privs[i].get_xpriv_bytes() if with_cpfp else None,
)
start_jobs.append(self.executor.submit(revaultd.start))
self.stkman_wallets.append(revaultd)
Expand Down Expand Up @@ -564,14 +564,14 @@ def broadcast_unvaults(self, vaults, destinations, feerate, priority=False):
spend_psbt.deserialize(spend_tx)
spend_psbt.tx.calc_sha256()
man.rpc.setspendtx(spend_psbt.tx.hash, priority)
return deposits, spend_psbt
return spend_psbt

def unvault_vaults(self, vaults, destinations, feerate, priority=False):
"""
Unvault these {vaults}, advertizing a Spend tx spending to these {destinations}
(mapping of addresses to amounts)
"""
self.broadcast_unvaults(vaults, destinations, feerate)
spend_psbt = self.broadcast_unvaults(vaults, destinations, feerate, priority)
deposits = [f"{v['txid']}:{v['vout']}" for v in vaults]
self.bitcoind.generate_block(1, wait_for_mempool=len(deposits))
for w in self.participants():
Expand Down Expand Up @@ -658,21 +658,13 @@ def unvault_vaults_anyhow(self, vaults, priority=False):
destinations, feerate = self._any_spend_data(vaults)
return self.unvault_vaults(vaults, destinations, feerate, priority)

def unvault_vaults_anyhow_unconfirmed(self, vaults, priority=False):
"""
Unvault these vaults with a random Spend transaction for a maximum amount and a
fixed feerate.
"""
destinations, feerate = self._any_spend_data(vaults)
return self.unvault_vaults_unconfirmed(vaults, destinations, feerate, priority)

def broadcast_unvaults_anyhow(self, vaults):
def broadcast_unvaults_anyhow(self, vaults, priority=False):
"""
Broadcast the Unvault transactions for these vaults with a random Spend
transaction for a maximum amount and a fixed feerate.
"""
destinations, feerate = self._any_spend_data(vaults)
return self.broadcast_unvaults(vaults, destinations, feerate)
return self.broadcast_unvaults(vaults, destinations, feerate, priority)

def spend_vaults_anyhow(self, vaults):
"""Spend these vaults to a random address for a maximum amount for a fixed feerate"""
Expand Down
9 changes: 5 additions & 4 deletions tests/test_framework/revaultd.py
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,7 @@ def __init__(
bitcoind,
stk_config=None,
man_config=man_config,
wt_process=None,
cpfp_priv=cpfp_priv,
)
assert self.man_keychain is not None
Expand Down Expand Up @@ -258,8 +259,8 @@ def __init__(
coordinator_noise_key,
coordinator_port,
bitcoind,
stk_config,
man_config,
wt_process,
cpfp_priv,
stk_config=stk_config,
man_config=man_config,
wt_process=wt_process,
cpfp_priv=cpfp_priv,
)
6 changes: 2 additions & 4 deletions tests/test_misc.py
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,7 @@ def test_no_cosig_server(revault_network):
@pytest.mark.skipif(not POSTGRES_IS_SETUP, reason="Needs Postgres for servers db")
def test_cpfp_transaction(revault_network, bitcoind):
CSV = 12
revault_network.deploy(2, 1, n_stkmanagers=1, csv=CSV)
revault_network.deploy(2, 1, n_stkmanagers=1, csv=CSV, with_watchtowers=False)
man = revault_network.mans()[1]
stks = revault_network.stks()
amount = 0.24
Expand All @@ -241,9 +241,7 @@ def test_cpfp_transaction(revault_network, bitcoind):

revault_network.secure_vault(vault)
revault_network.activate_vault(vault)
_, spend_psbt = revault_network.unvault_vaults_anyhow_unconfirmed(
[vault], priority=True
)
spend_psbt = revault_network.broadcast_unvaults_anyhow([vault], priority=True)

unvault_psbt = serializations.PSBT()
unvault_b64 = stks[0].rpc.getunvaulttx(deposit)["unvault_tx"]
Expand Down
4 changes: 2 additions & 2 deletions tests/test_rpc.py
Original file line number Diff line number Diff line change
Expand Up @@ -1148,7 +1148,7 @@ def test_getserverstatus(revault_network, bitcoind):
@pytest.mark.skipif(not POSTGRES_IS_SETUP, reason="Needs Postgres for servers db")
def test_setspendtx_cpfp_not_enabled(revault_network, bitcoind):
CSV = 12
revault_network.deploy(2, 1, n_stkmanagers=1, csv=CSV, cpfp_enabled=False)
revault_network.deploy(2, 1, n_stkmanagers=1, csv=CSV, with_cpfp=False)
man = revault_network.mans()[1]
stks = revault_network.stks()
amount = 0.24
Expand All @@ -1161,4 +1161,4 @@ def test_setspendtx_cpfp_not_enabled(revault_network, bitcoind):
RpcError,
match="Can't read the cpfp key",
):
revault_network.unvault_vaults_anyhow_unconfirmed([vault], priority=True)
revault_network.broadcast_unvaults_anyhow([vault], priority=True)

0 comments on commit 1199102

Please sign in to comment.