Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 7 additions & 3 deletions doc/API.md
Original file line number Diff line number Diff line change
Expand Up @@ -471,9 +471,13 @@ cover the requested feerate.

#### Response

| Field | Type | Description |
| -------------- | --------- | ---------------------------------------------------- |
| `psbt` | string | PSBT of the recovery transaction, encoded as base64. |
| Field | Type | Description |
| -------------- | -------------- | ----------------------------------------------------- |
| `psbt` | string | PSBT of the recovery transaction, encoded as base64. |
| `warnings` | list of string | Warnings, if any, generated during recovery creation. |

A warning will be included in the `warnings` response field if the sweep address
is known to belong to this wallet, since recovered funds would be locked under the same descriptor again.

### `updatelabels`

Expand Down
8 changes: 5 additions & 3 deletions liana-gui/src/app/state/spend/step.rs
Original file line number Diff line number Diff line change
Expand Up @@ -437,9 +437,9 @@ impl DefineSpend {
.create_recovery(max_address.clone(), &outpoints, feerate_vb, Some(reco_tl))
.await
// Map the PSBT to `CreateSpendResult` result. We only need the PSBT below.
.map(|psbt| CreateSpendResult::Success {
.map(|(psbt, warnings)| CreateSpendResult::Success {
psbt,
warnings: vec![],
warnings: warnings.iter().map(|w| w.to_string()).collect(),
})
} else {
daemon
Expand Down Expand Up @@ -738,7 +738,9 @@ impl Step for DefineSpend {
)
.await
.map_err(|e| e.into())
.map(|psbt| (psbt, vec![]))
.map(|(psbt, warnings)| {
(psbt, warnings.iter().map(|w| w.to_string()).collect())
})
},
Message::Psbt,
);
Expand Down
6 changes: 3 additions & 3 deletions liana-gui/src/daemon/client/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use std::iter::FromIterator;

use async_trait::async_trait;
use lianad::bip329::Labels;
use lianad::commands::{GetLabelsBip329Result, UpdateDerivIndexesResult};
use lianad::commands::{CreateRecoveryWarning, GetLabelsBip329Result, UpdateDerivIndexesResult};
use serde::de::DeserializeOwned;
use serde::{Deserialize, Serialize};
use serde_json::json;
Expand Down Expand Up @@ -209,7 +209,7 @@ impl<C: Client + Send + Sync + Debug> Daemon for Lianad<C> {
coins_outpoints: &[OutPoint],
feerate_vb: u64,
sequence: Option<u16>,
) -> Result<Psbt, DaemonError> {
) -> Result<(Psbt, Vec<CreateRecoveryWarning>), DaemonError> {
let mut params = serde_json::Map::new();
params.insert("address".to_string(), json!(address));
params.insert("outpoints".to_string(), json!(coins_outpoints));
Expand All @@ -218,7 +218,7 @@ impl<C: Client + Send + Sync + Debug> Daemon for Lianad<C> {
params.insert("timelock".to_string(), json!(sequence));
}
let res: CreateRecoveryResult = self.call("createrecovery", Some(params))?;
Ok(res.psbt)
Ok((res.psbt, res.warnings))
}

async fn get_labels(
Expand Down
10 changes: 6 additions & 4 deletions liana-gui/src/daemon/embedded.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
use lianad::bip329::Labels;
use lianad::commands::UpdateDerivIndexesResult;
use lianad::{
bip329::Labels,
commands::{CreateRecoveryWarning, UpdateDerivIndexesResult},
};
use std::collections::{HashMap, HashSet};
use tokio::sync::Mutex;

Expand Down Expand Up @@ -236,11 +238,11 @@ impl Daemon for EmbeddedDaemon {
coins_outpoints: &[OutPoint],
feerate_vb: u64,
sequence: Option<u16>,
) -> Result<Psbt, DaemonError> {
) -> Result<(Psbt, Vec<CreateRecoveryWarning>), DaemonError> {
self.command(|daemon| {
daemon
.create_recovery(address, coins_outpoints, feerate_vb, sequence)
.map(|res| res.psbt)
.map(|res| (res.psbt, res.warnings))
.map_err(|e| DaemonError::Unexpected(e.to_string()))
})
.await
Expand Down
4 changes: 2 additions & 2 deletions liana-gui/src/daemon/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ use liana::miniscript::bitcoin::{
secp256k1, Address, Network, OutPoint, Txid,
};
use lianad::bip329::Labels;
use lianad::commands::UpdateDerivIndexesResult;
use lianad::commands::{CreateRecoveryWarning, UpdateDerivIndexesResult};
use lianad::{
commands::{CoinStatus, LabelItem, TransactionInfo},
config::Config,
Expand Down Expand Up @@ -182,7 +182,7 @@ pub trait Daemon: Debug {
coins_outpoints: &[OutPoint],
feerate_vb: u64,
sequence: Option<u16>,
) -> Result<Psbt, DaemonError>;
) -> Result<(Psbt, Vec<CreateRecoveryWarning>), DaemonError>;
async fn list_txs(&self, txid: &[Txid]) -> Result<model::ListTransactionsResult, DaemonError>;
async fn get_labels(
&self,
Expand Down
10 changes: 7 additions & 3 deletions liana-gui/src/services/connect/client/backend/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,10 @@ use liana::{
};
use lianad::{
bip329::Labels,
commands::{CoinStatus, GetInfoDescriptors, LCSpendInfo, LabelItem, UpdateDerivIndexesResult},
commands::{
CoinStatus, CreateRecoveryWarning, GetInfoDescriptors, LCSpendInfo, LabelItem,
UpdateDerivIndexesResult,
},
config::Config,
};
use reqwest::{Error, IntoUrl, Method, RequestBuilder};
Expand Down Expand Up @@ -936,7 +939,7 @@ impl Daemon for BackendWalletClient {
coins_outpoints: &[OutPoint],
feerate_vb: u64,
sequence: Option<u16>,
) -> Result<Psbt, DaemonError> {
) -> Result<(Psbt, Vec<CreateRecoveryWarning>), DaemonError> {
let timelock = sequence.ok_or(DaemonError::Unexpected("Missing sequence".to_string()))?;
let res: api::DraftPsbt = self
.inner
Expand All @@ -955,7 +958,8 @@ impl Daemon for BackendWalletClient {
)
.await?;

Ok(res.raw)
// the connect API does not know `CreateRecoveryWarning` and gives warnings as raw text.
Ok((res.raw, vec![]))
}

async fn get_labels(
Expand Down
53 changes: 51 additions & 2 deletions lianad/src/commands/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1251,6 +1251,10 @@ impl DaemonControl {
/// otherwise not currently recoverable using the given recovery path.
///
/// Note that not all coins may be spendable through a single recovery path at the same time.
///
/// A warning will be included in the result's `warnings` field if the sweep address is
/// known to belong to this same wallet, since recovered funds would be locked under the
/// same descriptor again.
pub fn create_recovery(
&self,
address: bitcoin::Address<address::NetworkUnchecked>,
Expand Down Expand Up @@ -1311,7 +1315,14 @@ impl DaemonControl {
return Err(CommandError::RecoveryNotAvailable);
}

// If DB knows (as derived address) about the provided address, it means
// the sweep address belongs to this wallet.
let sweep_addr_info = sweep_addr.info;
let mut warnings = Vec::new();
if sweep_addr_info.is_some() {
warnings.push(CreateRecoveryWarning::ToOwnAddress);
}

Comment thread
qlrd marked this conversation as resolved.
let locktime = self.anti_fee_sniping_locktime();
let CreateSpendRes {
psbt, has_change, ..
Expand All @@ -1329,7 +1340,7 @@ impl DaemonControl {
self.maybe_increase_last_deriv_index(&mut db_conn, &sweep_addr_info);
}

Ok(CreateRecoveryResult { psbt })
Ok(CreateRecoveryResult { psbt, warnings })
}
}

Expand Down Expand Up @@ -1523,6 +1534,29 @@ pub struct TransactionInfo {
pub struct CreateRecoveryResult {
#[serde(serialize_with = "ser_to_string", deserialize_with = "deser_fromstr")]
pub psbt: Psbt,
#[serde(default)]
pub warnings: Vec<CreateRecoveryWarning>,
}

/// Warnings to be included in the `warnings` response field.
#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)]
#[serde(rename_all = "snake_case")]
pub enum CreateRecoveryWarning {
/// A warning if the sweep address is known to belong to user's wallet.
ToOwnAddress,
}

impl fmt::Display for CreateRecoveryWarning {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
match self {
CreateRecoveryWarning::ToOwnAddress => write!(
f,
"Recovery address belongs to the same wallet. If you can no \
longer spend with the primary path, use an address from a \
different wallet instead."
),
}
}
}

#[cfg(test)]
Expand Down Expand Up @@ -3136,7 +3170,9 @@ mod tests {
};
let dummy_txid = dummy_tx.compute_txid();
let dummy_op = bitcoin::OutPoint::new(dummy_txid, 0);
let ms = DummyLiana::new_timelock(DummyBitcoind::new(), DummyDatabase::new(), 10);
let db = DummyDatabase::new();
let mut db_handle = db.clone();
let ms = DummyLiana::new_timelock(DummyBitcoind::new(), db, 10);
let control = &ms.control();
let mut db_conn = control.db().lock().unwrap().connection();
db_conn.new_txs(&[dummy_tx]);
Expand Down Expand Up @@ -3242,6 +3278,19 @@ mod tests {
Err(CommandError::OutpointNotRecoverable(dummy_op, 11)),
);

// allow to recover with own address with a warning attached.
let own_addr = control.get_new_address();
db_handle.insert_derived_address(
own_addr.address.clone(),
own_addr.derivation_index,
false,
);
let own_unchecked_addr = own_addr.address.as_unchecked().clone();
let res = control
.create_recovery(own_unchecked_addr, &[], 1, None)
.unwrap();
assert_eq!(res.warnings, vec![CreateRecoveryWarning::ToOwnAddress]);

// If the coin is spending, it is no longer recoverable.
db_conn.spend_coins(&[(
dummy_op,
Expand Down
20 changes: 18 additions & 2 deletions lianad/src/testutils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -156,8 +156,10 @@ struct DummyDbState {
timestamp: u32,
rescan_timestamp: Option<u32>,
last_poll_timestamp: Option<u32>,
derived_addresses: HashMap<bitcoin::Address, (bip32::ChildNumber, bool)>,
}

#[derive(Clone)]
pub struct DummyDatabase {
db: sync::Arc<sync::RwLock<DummyDbState>>,
}
Expand Down Expand Up @@ -191,6 +193,7 @@ impl DummyDatabase {
timestamp: now,
rescan_timestamp: None,
last_poll_timestamp: None,
derived_addresses: HashMap::new(),
})),
}
}
Expand All @@ -200,6 +203,19 @@ impl DummyDatabase {
self.db.write().unwrap().coins.insert(coin.outpoint, coin);
}
}

pub fn insert_derived_address(
&mut self,
addr: bitcoin::Address,
index: bip32::ChildNumber,
is_change: bool,
) {
self.db
.write()
.unwrap()
.derived_addresses
.insert(addr, (index, is_change));
}
}

impl DatabaseConnection for DummyDatabase {
Expand Down Expand Up @@ -363,9 +379,9 @@ impl DatabaseConnection for DummyDatabase {

fn derivation_index_by_address(
&mut self,
_: &bitcoin::Address,
addr: &bitcoin::Address,
) -> Option<(bip32::ChildNumber, bool)> {
None
self.db.read().unwrap().derived_addresses.get(addr).copied()
}

fn coins_by_outpoints(
Expand Down
8 changes: 8 additions & 0 deletions tests/test_rpc.py
Original file line number Diff line number Diff line change
Expand Up @@ -1298,8 +1298,16 @@ def test_create_recovery(lianad, bitcoind):
][0]
reco_address = bitcoind.rpc.getnewaddress()
res = lianad.rpc.createrecovery(reco_address, 18)

# No warnings because it swept to an external address
assert len(res["warnings"]) == 0
reco_psbt = PSBT.from_base64(res["psbt"])

# Recover to own address but warn user about re-lock behaviour
own_address = lianad.rpc.getnewaddress()["address"]
res_own = lianad.rpc.createrecovery(address=own_address, feerate=18)
assert res_own["warnings"] == ["to_own_address"]

# Do the same passing all three coins explicitly:
res_op = lianad.rpc.createrecovery(reco_address, 18, 10, first_outpoints)
reco_psbt_op = PSBT.from_base64(res_op["psbt"])
Expand Down
Loading