Skip to content

Commit 9df7a16

Browse files
sanket1729sanket1729
committed
Policy fixes
1 parent 9c1b3fa commit 9df7a16

File tree

5 files changed

+79
-51
lines changed

5 files changed

+79
-51
lines changed

‎src/lib.rs‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -310,6 +310,8 @@ pub enum Error {
310310
#[cfg(feature = "compiler")]
311311
///Compiler related errors
312312
CompilerError(policy::compiler::CompilerError),
313+
///Errors related to policy
314+
PolicyError(policy::concrete::PolicyError),
313315
///Interpreter related errors
314316
InterpreterError(descriptor::InterpreterError),
315317
/// Bad Script Sig. As per standardness rules, only pushes are allowed in
@@ -401,6 +403,7 @@ impl fmt::Display for Error {
401403
Error::InterpreterError(ref e) => fmt::Display::fmt(e, f),
402404
#[cfg(feature = "compiler")]
403405
Error::CompilerError(ref e) => fmt::Display::fmt(e, f),
406+
Error::PolicyError(ref e) => fmt::Display::fmt(e, f),
404407
Error::BadScriptSig => f.write_str("Script sig must only consist of pushes"),
405408
Error::NonEmptyWitness => f.write_str("Non empty witness for Pk/Pkh"),
406409
Error::NonEmptyScriptSig => f.write_str("Non empty script sig for segwit spend"),
@@ -429,6 +432,13 @@ impl From<policy::compiler::CompilerError> for Error {
429432
}
430433
}
431434

435+
#[doc(hidden)]
436+
impl From<policy::concrete::PolicyError> for Error {
437+
fn from(e: policy::concrete::PolicyError) -> Error {
438+
Error::PolicyError(e)
439+
}
440+
}
441+
432442
/// The size of an encoding of a number in Script
433443
pub fn script_num_size(n: usize) -> usize {
434444
match n {

‎src/policy/compiler.rs‎

Lines changed: 2 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ impl fmt::Display for CompilerError {
8989
}
9090

9191
#[doc(hidden)]
92-
impl From<policy::concrete::PolicyError> for CompilerError {
92+
impl std::convert::From<policy::concrete::PolicyError> for CompilerError {
9393
fn from(e: policy::concrete::PolicyError) -> CompilerError {
9494
CompilerError::PolicyError(e)
9595
}
@@ -1136,9 +1136,6 @@ mod tests {
11361136
use std::str::FromStr;
11371137

11381138
use miniscript::satisfy;
1139-
use policy::concrete::PolicyError::{
1140-
IncorrectThresh, NonBinaryArgAnd, NonBinaryArgOr, TimeTooFar, ZeroTime,
1141-
};
11421139
use policy::Liftable;
11431140
use BitcoinSig;
11441141
use DummyKey;
@@ -1173,40 +1170,13 @@ mod tests {
11731170
}
11741171

11751172
fn policy_compile_lift_check(s: &str) -> Result<(), CompilerError> {
1176-
let policy = DummyPolicy::from_str(s).expect("parse");
1173+
let policy = DummyPolicy::from_str(s).unwrap();
11771174
let miniscript = policy.compile()?;
11781175

11791176
assert_eq!(policy.lift().sorted(), miniscript.lift().sorted());
11801177
Ok(())
11811178
}
11821179

1183-
#[test]
1184-
fn compile_invalid() {
1185-
assert_eq!(
1186-
policy_compile_lift_check("thresh(2,pk(),thresh(0))"),
1187-
Err(CompilerError::PolicyError(IncorrectThresh))
1188-
);
1189-
assert_eq!(
1190-
policy_compile_lift_check("thresh(2,pk(),thresh(0,pk()))"),
1191-
Err(CompilerError::PolicyError(IncorrectThresh))
1192-
);
1193-
assert_eq!(
1194-
policy_compile_lift_check("and(pk())"),
1195-
Err(CompilerError::PolicyError(NonBinaryArgAnd))
1196-
);
1197-
assert_eq!(
1198-
policy_compile_lift_check("or(pk())"),
1199-
Err(CompilerError::PolicyError(NonBinaryArgOr))
1200-
);
1201-
assert_eq!(
1202-
policy_compile_lift_check("thresh(3,after(0),pk(),pk())"),
1203-
Err(CompilerError::PolicyError(ZeroTime))
1204-
);
1205-
assert_eq!(
1206-
policy_compile_lift_check("thresh(2,older(2147483650),pk(),pk())"),
1207-
Err(CompilerError::PolicyError(TimeTooFar))
1208-
);
1209-
}
12101180
#[test]
12111181
fn compile_basic() {
12121182
assert!(policy_compile_lift_check("pk()").is_ok());

‎src/policy/concrete.rs‎

Lines changed: 25 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ impl fmt::Display for PolicyError {
8989
}
9090
PolicyError::NonBinaryArgOr => f.write_str("Or policy fragment must take 2 arguments"),
9191
PolicyError::IncorrectThresh => {
92-
f.write_str("Threshold k must be greater than 0 and less than n")
92+
f.write_str("Threshold k must be greater than 0 and less than or equal to n 0<k<=n")
9393
}
9494
PolicyError::TimeTooFar => {
9595
f.write_str("Relative/Absolute time must be less than 2^31; n < 2^31")
@@ -382,12 +382,24 @@ where
382382
}
383383
match (frag_name, top.args.len() as u32) {
384384
("pk", 1) => expression::terminal(&top.args[0], |pk| Pk::from_str(pk).map(Policy::Key)),
385-
("after", 1) => expression::terminal(&top.args[0], |x| {
386-
expression::parse_num(x).map(Policy::After)
387-
}),
388-
("older", 1) => expression::terminal(&top.args[0], |x| {
389-
expression::parse_num(x).map(Policy::Older)
390-
}),
385+
("after", 1) => {
386+
let num = expression::terminal(&top.args[0], |x| expression::parse_num(x))?;
387+
if num > 2u32.pow(31) {
388+
return Err(Error::PolicyError(PolicyError::TimeTooFar));
389+
} else if num == 0 {
390+
return Err(Error::PolicyError(PolicyError::ZeroTime));
391+
}
392+
Ok(Policy::After(num))
393+
}
394+
("older", 1) => {
395+
let num = expression::terminal(&top.args[0], |x| expression::parse_num(x))?;
396+
if num > 2u32.pow(31) {
397+
return Err(Error::PolicyError(PolicyError::TimeTooFar));
398+
} else if num == 0 {
399+
return Err(Error::PolicyError(PolicyError::ZeroTime));
400+
}
401+
Ok(Policy::Older(num))
402+
}
391403
("sha256", 1) => expression::terminal(&top.args[0], |x| {
392404
sha256::Hash::from_hex(x).map(Policy::Sha256)
393405
}),
@@ -402,7 +414,7 @@ where
402414
}),
403415
("and", _) => {
404416
if top.args.len() != 2 {
405-
return Err(errstr("and fragment must have exactly two children"));
417+
return Err(Error::PolicyError(PolicyError::NonBinaryArgAnd));
406418
}
407419
let mut subs = Vec::with_capacity(top.args.len());
408420
for arg in &top.args {
@@ -412,7 +424,7 @@ where
412424
}
413425
("or", _) => {
414426
if top.args.len() != 2 {
415-
return Err(errstr("or fragment must have exactly two children"));
427+
return Err(Error::PolicyError(PolicyError::NonBinaryArgOr));
416428
}
417429
let mut subs = Vec::with_capacity(top.args.len());
418430
for arg in &top.args {
@@ -421,16 +433,13 @@ where
421433
Ok(Policy::Or(subs))
422434
}
423435
("thresh", nsubs) => {
424-
if top.args.is_empty() {
425-
return Err(errstr("thresh without args"));
426-
}
427-
if !top.args[0].args.is_empty() {
428-
return Err(errstr(top.args[0].args[0].name));
436+
if top.args.is_empty() || !top.args[0].args.is_empty() {
437+
return Err(Error::PolicyError(PolicyError::IncorrectThresh));
429438
}
430439

431440
let thresh = expression::parse_num(top.args[0].name)?;
432-
if thresh >= nsubs {
433-
return Err(errstr(top.args[0].name));
441+
if thresh >= nsubs || thresh <= 0 {
442+
return Err(Error::PolicyError(PolicyError::IncorrectThresh));
434443
}
435444

436445
let mut subs = Vec::with_capacity(top.args.len() - 1);

‎src/policy/mod.rs‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -178,4 +178,43 @@ mod tests {
178178
assert!(ConcretePol::from_str("thresh()").is_err());
179179
assert!(SemanticPol::from_str("thresh()").is_err());
180180
}
181+
182+
#[test]
183+
fn compile_invalid() {
184+
// Since the root Error does not support Eq type, we hvae to
185+
// compare the string representations of the error
186+
assert_eq!(
187+
ConcretePol::from_str("thresh(2,pk(),thresh(0))")
188+
.unwrap_err()
189+
.to_string(),
190+
"Threshold k must be greater than 0 and less than or equal to n 0<k<=n"
191+
);
192+
assert_eq!(
193+
ConcretePol::from_str("thresh(2,pk(),thresh(0,pk()))")
194+
.unwrap_err()
195+
.to_string(),
196+
"Threshold k must be greater than 0 and less than or equal to n 0<k<=n"
197+
);
198+
assert_eq!(
199+
ConcretePol::from_str("and(pk())").unwrap_err().to_string(),
200+
"And policy fragment must take 2 arguments"
201+
);
202+
assert_eq!(
203+
ConcretePol::from_str("or(pk())").unwrap_err().to_string(),
204+
"Or policy fragment must take 2 arguments"
205+
);
206+
assert_eq!(
207+
ConcretePol::from_str("thresh(3,after(0),pk(),pk())")
208+
.unwrap_err()
209+
.to_string(),
210+
"Time must be greater than 0; n > 0"
211+
);
212+
213+
assert_eq!(
214+
ConcretePol::from_str("thresh(2,older(2147483650),pk(),pk())")
215+
.unwrap_err()
216+
.to_string(),
217+
"Relative/Absolute time must be less than 2^31; n < 2^31"
218+
);
219+
}
181220
}

‎src/policy/semantic.rs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,11 +18,11 @@ use bitcoin::hashes::hex::FromHex;
1818
use bitcoin::hashes::{hash160, ripemd160, sha256, sha256d};
1919
use std::{fmt, str};
2020

21+
use super::concrete::PolicyError;
2122
use errstr;
2223
use std::str::FromStr;
2324
use Error;
2425
use {expression, MiniscriptKey};
25-
2626
/// Abstract policy which corresponds to the semantics of a Miniscript
2727
/// and which allows complex forms of analysis, e.g. filtering and
2828
/// normalization.
@@ -235,7 +235,7 @@ where
235235
}),
236236
("and", _) => {
237237
if top.args.len() != 2 {
238-
return Err(errstr("and fragment must have exactly two children"));
238+
return Err(Error::PolicyError(PolicyError::NonBinaryArgAnd));
239239
}
240240
let mut subs = Vec::with_capacity(top.args.len());
241241
for arg in &top.args {
@@ -245,7 +245,7 @@ where
245245
}
246246
("or", _) => {
247247
if top.args.len() != 2 {
248-
return Err(errstr("or fragment must have exactly two children"));
248+
return Err(Error::PolicyError(PolicyError::NonBinaryArgOr));
249249
}
250250
let mut subs = Vec::with_capacity(top.args.len());
251251
for arg in &top.args {

0 commit comments

Comments
 (0)