Merge #214: descriptors: rule-out 0-value timelocks

b74a6ffc16740c1a8c0b93a8b4aa34bc1a1b819a qa: unflake test_rescan_edge_cases (Antoine Poinsot)
f6db9c6677ca43c51afbacbf4036cfe55f5539b1 descriptors: rule out null timelocks (Antoine Poinsot)

Pull request description:

  Fixes #208.

ACKs for top commit:
  darosior:
    self-ACK b74a6ffc16740c1a8c0b93a8b4aa34bc1a1b819a

Tree-SHA512: 3d4ed44f3b376e897bd28d11b9e1ce367d6cdf28f70472f39b3939d2cf25580302a81542131086aa1b9a028161f0eabe084efdd54a55703d0a2d4150eac84ae9
This commit is contained in:
Antoine Poinsot 2022-12-13 17:47:01 +01:00
commit 1e56c58166
No known key found for this signature in database
GPG Key ID: E13FC145CD3F4304
2 changed files with 16 additions and 5 deletions

View File

@ -39,7 +39,9 @@ pub enum DescCreationError {
impl std::fmt::Display for DescCreationError {
fn fmt(&self, f: &mut fmt::Formatter) -> std::fmt::Result {
match self {
Self::InsaneTimelock(tl) => write!(f, "Timelock value '{}' isn't safe to use", tl),
Self::InsaneTimelock(tl) => {
write!(f, "Timelock value '{}' isn't valid or safe to use", tl)
}
Self::InvalidKey(key) => {
write!(
f,
@ -179,9 +181,10 @@ impl ToPublicKey for DerivedPublicKey {
// All this is achieved simply through asking for a 16-bit integer, since all the
// above are signaled in leftmost bits.
fn csv_check(csv_value: u32) -> Result<(), DescCreationError> {
u16::try_from(csv_value)
.map(|_| ())
.map_err(|_| DescCreationError::InsaneTimelock(csv_value))
if csv_value > 0 && u16::try_from(csv_value).is_ok() {
return Ok(());
}
Err(DescCreationError::InsaneTimelock(csv_value))
}
// We require the descriptor key to:
@ -340,8 +343,12 @@ impl MultipathDescriptor {
// - not be disabled
// - be in number of blocks
// - be 'clean' / minimal, ie all bits without consensus meaning should be 0
// - be positive (Miniscript requires it not to be 0)
//
// All this is achieved through asking for a 16-bit integer.
if timelock == 0 {
return Err(DescCreationError::InsaneTimelock(timelock as u32));
}
let timelock = Sequence::from_height(timelock);
if let Some(key) = vec![&owner_key, &heir_key]
@ -601,7 +608,7 @@ mod tests {
let owner_key = descriptor::DescriptorPublicKey::from_str("xpub6Eze7yAT3Y1wGrnzedCNVYDXUqa9NmHVWck5emBaTbXtURbe1NWZbK9bsz1TiVE7Cz341PMTfYgFw1KdLWdzcM1UMFTcdQfCYhhXZ2HJvTW/<0;1>/*").unwrap();
let heir_key = descriptor::DescriptorPublicKey::from_str("xpub688Hn4wScQAAiYJLPg9yH27hUpfZAUnmJejRQBCiwfP5PEDzjWMNW1wChcninxr5gyavFqbbDjdV1aK5USJz8NDVjUy7FRQaaqqXHh5SbXe/<0;1>/*").unwrap();
let timelock = 52560;
assert_eq!(MultipathDescriptor::new(owner_key, heir_key, timelock).unwrap().to_string(), "wsh(or_d(pk(xpub6Eze7yAT3Y1wGrnzedCNVYDXUqa9NmHVWck5emBaTbXtURbe1NWZbK9bsz1TiVE7Cz341PMTfYgFw1KdLWdzcM1UMFTcdQfCYhhXZ2HJvTW/<0;1>/*),and_v(v:pkh(xpub688Hn4wScQAAiYJLPg9yH27hUpfZAUnmJejRQBCiwfP5PEDzjWMNW1wChcninxr5gyavFqbbDjdV1aK5USJz8NDVjUy7FRQaaqqXHh5SbXe/<0;1>/*),older(52560))))#8n2ydpkt");
assert_eq!(MultipathDescriptor::new(owner_key.clone(), heir_key.clone(), timelock).unwrap().to_string(), "wsh(or_d(pk(xpub6Eze7yAT3Y1wGrnzedCNVYDXUqa9NmHVWck5emBaTbXtURbe1NWZbK9bsz1TiVE7Cz341PMTfYgFw1KdLWdzcM1UMFTcdQfCYhhXZ2HJvTW/<0;1>/*),and_v(v:pkh(xpub688Hn4wScQAAiYJLPg9yH27hUpfZAUnmJejRQBCiwfP5PEDzjWMNW1wChcninxr5gyavFqbbDjdV1aK5USJz8NDVjUy7FRQaaqqXHh5SbXe/<0;1>/*),older(52560))))#8n2ydpkt");
// We prevent footguns with timelocks by requiring a u16. Note how the following wouldn't
// compile:
@ -609,6 +616,9 @@ mod tests {
//MultipathDescriptor::new(owner_key.clone(), heir_key.clone(), (1 << 31) + 1).unwrap_err();
//MultipathDescriptor::new(owner_key, heir_key, (1 << 22) + 1).unwrap_err();
// You can't use a null timelock in Miniscript.
MultipathDescriptor::new(owner_key, heir_key, 0).unwrap_err();
let owner_key = descriptor::DescriptorPublicKey::from_str("[aabb0011/10/4893]xpub661MyMwAqRbcFG59fiikD8UV762quhruT8K8bdjqy6N2o3LG7yohoCdLg1m2HAY1W6rfBrtauHkBhbfA4AQ3iazaJj5wVPhwgaRCHBW2DBg/<0;1>/*").unwrap();
let heir_key = descriptor::DescriptorPublicKey::from_str("xpub661MyMwAqRbcFfxf71L4Dx4w5TmyNXrBicTEAM7vLzumxangwATWWgdJPb6xH1JHcJH9S3jNZx3fCnkkB1WyqrqGgavj1rehHcbythmruvZ/24/32/<0;1>/*").unwrap();
let timelock = 57600;

View File

@ -222,6 +222,7 @@ def test_rescan_edge_cases(lianad, bitcoind):
lianad.stop()
lianad.start()
wait_for(lambda: lianad.rpc.getinfo()["rescan_progress"] is None)
wait_synced()
assert coins_before == sorted_coins()
# Lose our state again