skip policy compilation when parsing descriptor

This allows to create a new `LianaPolicy` without checking that
it compiles into miniscript. This compilation check is used as
a sanity check on the policy in addition to other Liana-specific
checks, but this compiled policy is not otherwise used.

When parsing an existing descriptor, we will assume that the
policy can be compiled. Otherwise, the method `LianaPolicy::from_multipath_descriptor`
ends up calling `LianaPolicy::into_multipath_descriptor_fallible`.

A `LianaPolicy` is created from a descriptor each time we
call `LianaDescriptor::from_str`, which is done often, e.g.
when updating derivation indices in the DB. The sanity check
of the descriptor previously performed during compilation of
the policy will now be done explicitly as part of the
`LianaDescriptor::from_str` method.

Removing the compilation check won't affect the `LianaDescriptor`
that we end up with when parsing a string as we already use the
`Descriptor` obtained from parsing the string rather than that
obtained by compilation of the policy.

This change is particularly important with the upcoming upgrade
to miniscript 12.0, which has been found to increase Liana policy
compilation times significantly.

An additional option would be to reduce the number of times we
call `LianaDescriptor::from_str`, e.g. by passing the descriptor
as a parameter rather than reading from DB, but this current
change should address the main performance issues.
This commit is contained in:
Michael Mallan 2025-01-21 11:07:10 +00:00
parent c16237d354
commit a95cfad6d2
No known key found for this signature in database
GPG Key ID: 5177CDCEDB0EABEB
2 changed files with 31 additions and 4 deletions

View File

@ -464,10 +464,14 @@ pub struct LianaPolicy {
impl LianaPolicy {
/// Create a new Liana policy from a given configuration.
///
/// `compile` controls whether to check the policy compiles
/// to miniscript before returning.
fn _new(
primary_path: PathInfo,
recovery_paths: BTreeMap<u16, PathInfo>,
is_taproot: bool,
compile: bool,
) -> Result<LianaPolicy, LianaPolicyError> {
if recovery_paths.is_empty() {
return Err(LianaPolicyError::MissingRecoveryPath);
@ -523,7 +527,9 @@ impl LianaPolicy {
recovery_paths,
is_taproot,
};
policy.clone().into_multipath_descriptor_fallible()?;
if compile {
policy.clone().into_multipath_descriptor_fallible()?;
}
Ok(policy)
}
@ -532,7 +538,12 @@ impl LianaPolicy {
primary_path: PathInfo,
recovery_paths: BTreeMap<u16, PathInfo>,
) -> Result<LianaPolicy, LianaPolicyError> {
Self::_new(primary_path, recovery_paths, /* is_taproot = */ true)
Self::_new(
primary_path,
recovery_paths,
/* is_taproot = */ true,
/* compile = */ true,
)
}
/// Create a new Liana policy for use under a P2WSH context.
@ -540,7 +551,12 @@ impl LianaPolicy {
primary_path: PathInfo,
recovery_paths: BTreeMap<u16, PathInfo>,
) -> Result<LianaPolicy, LianaPolicyError> {
Self::_new(primary_path, recovery_paths, /* is_taproot = */ false)
Self::_new(
primary_path,
recovery_paths,
/* is_taproot = */ false,
/* compile = */ true,
)
}
/// Create a Liana policy from a descriptor. This will check the descriptor is correctly formed
@ -631,7 +647,15 @@ impl LianaPolicy {
// Use the constructor for sanity checking the keys and the Miniscript policy. Note this
// makes sure the recovery paths mapping isn't empty, too.
let prim_path = primary_path.ok_or(LianaPolicyError::IncompatibleDesc)?;
LianaPolicy::_new(prim_path, recovery_paths, is_taproot)
// We don't compile the policy as we assume it compiles given we started with a descriptor.
// This will still perform all other checks to make sure the descriptor conforms to
// a Liana policy.
LianaPolicy::_new(
prim_path,
recovery_paths,
is_taproot,
/* compile = */ false,
)
}
pub fn primary_path(&self) -> &PathInfo {

View File

@ -115,7 +115,10 @@ impl str::FromStr for LianaDescriptor {
fn from_str(s: &str) -> Result<LianaDescriptor, Self::Err> {
// Parse a descriptor and check it is a multipath descriptor corresponding to a valid Liana
// spending policy.
// Sanity checks are not always performed when calling `Descriptor::from_str`, so we perform
// them explicitly. See https://github.com/rust-bitcoin/rust-miniscript/issues/734.
let desc = descriptor::Descriptor::<descriptor::DescriptorPublicKey>::from_str(s)
.and_then(|desc| desc.sanity_check().map(|_| desc))
.map_err(LianaDescError::Miniscript)?;
LianaPolicy::from_multipath_descriptor(&desc)?;