Encrypted descriptor by pythcoiner · Pull Request #1846 · wizardsardine/liana

pythcoiner · 2025-09-08T08:04:30Z

This is a lightweight version of #1807, where we do not encrypt for a backup export.
(I just dropped the commit that enable encryption on backup export)

jp1ac4 · 2025-09-09T11:18:37Z

Should the encrypted descriptor option be listed here?

nondiremanuel · 2025-09-09T13:40:57Z

Should the encrypted descriptor option be listed here?

I inserted it only in the "Wallet" section of the settings to distinguish between the concept of backup and import/exports material. So my idea is that it shouldn't be added here, but open to discuss. At the same time we have the plain text descriptor file among the exports, which seems to contradict this concept, but I don't think it should be in the wallet section since we should incentivize to use the encrypted version.

pythcoiner · 2025-09-10T09:49:08Z

I inserted it only in the "Wallet" section of the settings to distinguish between the concept of backup and import/exports material. So my idea is that it shouldn't be added here, but open to discuss. At the same time we have the plain text descriptor file among the exports, which seems to contradict this concept, but I don't think it should be in the wallet section since we should incentivize to use the encrypted version.

the plaintext descriptor option must be kept for register descriptor on airgap devices

jp1ac4

I've successfully tested importing an encrypted backup using a signing device, a manually pasted xpub and the corresponding mnemonic.

liana-gui/src/backup.rs

liana-gui/src/decrypt.rs

liana-gui/src/app/view/settings/mod.rs

liana-gui/src/export.rs

jp1ac4 · 2025-09-11T08:58:52Z

liana-gui/src/decrypt.rs

+            modal: None,
+        }
+    }
+    pub fn update(&mut self, msg: Decrypt) -> Task<installer::Message> {


Would it be useful to return something more general that doesn't depend on installer here?

for now only uses cases are with installer::Message, if needed we can later make it more generic

jp1ac4 · 2025-09-11T09:00:55Z

liana-gui/src/installer/prompt.rs

-pub const BACKUP_DESCRIPTOR_MESSAGE: &str = "A backup of your wallet configuration is necessary to recover your funds. Please make sure to store your Wallet backup file (or alternatively to copy and paste the descriptor string) in one or more secure and accessible locations. You still need to back up your seed phrases too, since they are not included in the file.";
-pub const BACKUP_DESCRIPTOR_HELP: &str = "In Bitcoin, the coins are locked using a Script (related to the 'address'). In order to recover your funds you need to know both the Scripts you have participated in (your 'addresses'), and be able to sign a transaction that spends from those. For the ability to sign, you back up your private key, this is your mnemonic ('seed words'). For finding the coins that belong to you, you back up a template of your Script (your 'addresses'), this is your descriptor, included in your wallet backup file. However, note the descriptor need not be stored as securely as the private key. A thief who steals your descriptor but not your private key cannot steal your funds.";
+pub const BACKUP_DESCRIPTOR_MESSAGE: &str = "This backup is required to recover your funds.
+Click “Back Up Descriptor” to download an encrypted file of your wallet configuration and store it in safe, accessible places.


Should this be changed to "encrypted file of your wallet descriptor"?

@nondiremanuel

pythcoiner · 2025-09-12T14:25:40Z

bump encrypted_backup to last changes & rebased

pythcoiner · 2025-09-16T12:22:07Z

added sanity checks before write encrypted descriptor to disk

liana/src/descriptors/mod.rs

jp1ac4 · 2025-09-16T13:18:35Z

liana/src/descriptors/mod.rs

+            if let DescriptorPublicKey::MultiXPub(multixkey) = k {
+                let key = DescriptorPublicKey::XPub(DescriptorXKey {
+                    origin: multixkey.origin.clone(),
+                    xkey: multixkey.xkey,


Rather than adding dummy values, which could misleadingly suggest these are the values present in the descriptor itself, wouldn't it be clearer to return the multikey.xkey and then let the caller add these dummy values as required?

those are not dummies values, It's just about having a DescriptorPublicKey that contains Xpub + Origin, Xpub is not enough as it does not contains the origin and I expect we use the result of this function to list xpubs of the Bip388 Policy

this way I think we can directly call .to_string()

DescriptorPublicKey::Single is not suitable as it will not output a xpub

DescriptorPublicKey::MultiXpub is not suitable as DerivPaths cannot be empty

I am fine with it, maybe add a comment to say that is a vessel for the xkey.

well it's not a vessel, it's litteraly how it's intended to be used:

this

let key_str = "[abcdef01/0/0/0]xpub6Eze7yAT3Y1wGrnzedCNVYDXUqa9NmHVWck5emBaTbXtURbe1NWZbK9bsz1TiVE7Cz341PMTfYgFw1KdLWdzcM1UMFTcdQfCYhhXZ2HJvTW"; let key = DescriptorPublicKey::from_str(key_str).unwrap();

will yield

XPub( DescriptorXKey { origin: Some( ( 0xabcdef01, 0/0/0, ), ), xkey: Xpub { network: Main, depth: 4, parent_fingerprint: 0xc6c45ae6, child_number: Normal { index: 0, }, public_key: PublicKey( 4fcda97320d37e42ddd2c9adf5c5c5edf7ff9decb550c50ad2ace5ae3a16abb3b780ec63e8766dbaa3e323148cc0e6ee760e3a41b5a0b1229f5d69b59ce3a0a3, ), chain_code: 0xf98f1f14305e50df1ff23883210e9020e212bc62a790eff034e9f5036af844a1, }, derivation_path: , wildcard: None, }, )

I'm fine to add a comment, but from my POV make it more confusing

Sorry I understand better now. I think a name like definite_keys_without_deriv_path() would make it clearer what is being returned (rust-miniscript uses DefiniteDescriptorKey for a DescriptorPublicKey without wildcard).

Also, since we require an origin for all user-provided keys when creating a LianaDescriptor (as enforced in the constructors), an alternative check would be to exclude any descriptor keys that don't have an origin. Or simply to return all keys and let the caller check if it has an origin.

an alternative check would be to exclude any descriptor keys that don't have an origin

in which other case than the unspendable key we can have a key w/o origin?

(rust-miniscript uses DefiniteDescriptorKey for a DescriptorPublicKey without wildcard)

No in our case it's not a DefiniteDescriptorKey, my understanding of "definite" in rust-miniscript context is that the key is not deriveable anymore, while the key we generating here is the "root" key for derivation

in which other case than the unspendable key we can have a key w/o origin?

I meant it would be an equivalent check, since we expect a key not to have an origin if and only if it's unspendable, but I suppose it's more reliable to check for the NUMS directly.

but I suppose it's more reliable to check for the NUMS directly.

yes especially if in the future we allow importing key w/o origin (watchonly?)

liana-gui/src/installer/mod.rs

liana-gui/src/export.rs

import_descriptor.rs

…dable_key()

jp1ac4

tACK b278ef2.

jp1ac4

ACK bbe75f7.

pythcoiner mentioned this pull request Sep 8, 2025

[WIP] Encrypted backup #1807

Draft

7 tasks

pythcoiner requested review from edouardparis and jp1ac4 September 8, 2025 08:40

nondiremanuel assigned pythcoiner Sep 8, 2025

nondiremanuel added this to Liana General Sep 8, 2025

nondiremanuel moved this to In Review in Liana General Sep 8, 2025

nondiremanuel added this to the v13 Liana - Encrypted backup milestone Sep 8, 2025

jp1ac4 reviewed Sep 11, 2025

View reviewed changes

pythcoiner force-pushed the encrypted_descriptor branch 6 times, most recently from 6c49810 to af661b5 Compare September 12, 2025 14:20

pythcoiner force-pushed the encrypted_descriptor branch 2 times, most recently from 44bee47 to 8ca0b13 Compare September 16, 2025 12:19

jp1ac4 reviewed Sep 16, 2025

View reviewed changes

edouardparis reviewed Sep 16, 2025

View reviewed changes

liana-gui/src/installer/mod.rs Outdated Show resolved Hide resolved

pythcoiner force-pushed the encrypted_descriptor branch from da49af8 to 3f11fe3 Compare September 17, 2025 07:01

edouardparis reviewed Sep 17, 2025

View reviewed changes

liana-gui/src/export.rs Show resolved Hide resolved

pythcoiner force-pushed the encrypted_descriptor branch 5 times, most recently from b733b01 to 3f005eb Compare September 17, 2025 11:58

pythcoiner added 11 commits September 17, 2025 16:08

liana-sdk: add a getter for LianaDescriptor.multi_desc

298615d

gui: add dependency for encrypted backup

b866bfe

liana-gui: move default_derivation_path() to utils

d5e8aa5

installer: export encrypted descriptor instead plaintext backup

f728377

gui: wording + add encrypted descriptor export

d700719

installer: implement import encrypted descriptor

c754316

export: remove unused variant of ImportExportType

b68cdff

installer: move ImportDescriptorModal with some logic to

b3788d5

import_descriptor.rs

installer: impl import encrypted backup/descriptor for liana-connect

4c4fc03

liana-sdk: implement LianaDescriptor.spendable_keys()/.process_unspen…

438820e

…dable_key()

export: sanitize encrypted backup before writing to disk

b278ef2

pythcoiner force-pushed the encrypted_descriptor branch from 3f005eb to b278ef2 Compare September 17, 2025 14:12

jp1ac4 approved these changes Sep 17, 2025

View reviewed changes

decrypt: move to installer

5e27112

pythcoiner force-pushed the encrypted_descriptor branch from b3574fd to 5e27112 Compare September 17, 2025 17:32

export: add test for decrypt descriptor from file

bbe75f7

jp1ac4 approved these changes Sep 18, 2025

View reviewed changes

edouardparis merged commit 1932ada into wizardsardine:master Sep 18, 2025
13 checks passed

nondiremanuel moved this from In Review to Done in Liana General Sep 23, 2025

This was referenced Sep 23, 2025

Allow the user to back up an encrypted version of the descriptor #1803

Closed

Allow the user to restore a Liana wallet using an encrypted backup #1804

Closed

Conversation

pythcoiner commented Sep 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jp1ac4 commented Sep 9, 2025

Uh oh!

nondiremanuel commented Sep 9, 2025

Uh oh!

pythcoiner commented Sep 10, 2025

Uh oh!

jp1ac4 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pythcoiner commented Sep 12, 2025

Uh oh!

pythcoiner commented Sep 16, 2025

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

jp1ac4 left a comment

Choose a reason for hiding this comment

Uh oh!

jp1ac4 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

pythcoiner commented Sep 8, 2025 •

edited

Loading