Skip to content

Commit

Permalink
Merge #772: feat: check policy validity to prevent duplicate check
Browse files Browse the repository at this point in the history
757daee feat: check policy validity in from_ast to prevent duplicate check (ChrisCho-H)

Pull request description:

  - `Miniscript::from_ast()` only executes `check_global_consensus_validity()` which does not include `check_global_policy_validity()`. Change to use `check_global_validity()` to check both consensus and policy validness.
  - `wrap_into_miniscript()` executes `check_global_validity()` internally after `Miniscript::from_ast()`. As `Miniscript::from_ast()` now executes `check_global_validity()`,  it doesn't have to check duplicately.

  I was originally thinking to change `check_global_validity()` inside `wrap_into_miniscript()` to `check_global_policy_validity()`(as consensus check is done by `from_ast()`). I think it's better to strengthen the validness check in `from_ast()`, and do nothing in `wrap_into_miniscript()`.

ACKs for top commit:
  apoelstra:
    ACK 757daee; successfully ran local tests

Tree-SHA512: e6afc44b8e5fa9310d6656f1d624f8f3b25c893c02e6e2936e924e5e78f0d4beba53c5114c49348a572d1ea5a2b3af4739bc6bcd8ea22c968b3cc47c6f521bd3
  • Loading branch information
apoelstra committed Nov 12, 2024
2 parents e35eb70 + 757daee commit 4114967
Showing 1 changed file with 1 addition and 3 deletions.
4 changes: 1 addition & 3 deletions src/miniscript/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ mod private {
if (res.ext.tree_height as u32) > MAX_RECURSION_DEPTH {
return Err(Error::MaxRecursiveDepthExceeded);
}
Ctx::check_global_consensus_validity(&res)?;
Ctx::check_global_validity(&res)?;
Ok(res)
}

Expand Down Expand Up @@ -742,7 +742,6 @@ where
for ch in frag_wrap.chars().rev() {
// Check whether the wrapper is valid under the current context
let ms = Miniscript::from_ast(unwrapped)?;
Ctx::check_global_validity(&ms)?;
match ch {
'a' => unwrapped = Terminal::Alt(Arc::new(ms)),
's' => unwrapped = Terminal::Swap(Arc::new(ms)),
Expand All @@ -759,7 +758,6 @@ where
}
// Check whether the unwrapped miniscript is valid under the current context
let ms = Miniscript::from_ast(unwrapped)?;
Ctx::check_global_validity(&ms)?;
Ok(ms)
}

Expand Down

0 comments on commit 4114967

Please sign in to comment.