Skip to content

Commit

Permalink
Merge #760: feature: improve context err
Browse files Browse the repository at this point in the history 
faf16fd feat: unify err msg to contain helpful info (ChrisCho-H)
919bc4b fix: use ImpossibleSatisfaction when op_count is None (ChrisCho-H)

Pull request description:

  919bc4b
  - use `ImpossibleSatisfaction` when op_count is None instead of `MaxOpCountExceeded`.

  51a50ae
  - unify error message for satisfaction-kind error and scriptPubKey-kind error so that these error can contain max(limit) with what it got(actual).

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

Tree-SHA512: bd2b88c297af6a333c6155ef9eb9103c303e8b6e7df62f78b5c337721c0deefc601a307027fafe50b95bcbcde73594b5a822a0d89a9e869720755dba4f9fdeaf
  • Loading branch information
@apoelstra
apoelstra committed Oct 21, 2024
2 parents 0f03df0 + faf16fd commit aa3691d
Show file tree
Hide file tree
Showing 2 changed files with 62 additions and 36 deletions.
94 changes: 60 additions & 34 deletions src/miniscript/context.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,19 +45,19 @@ pub enum ScriptContextError {
MaxWitnessItemsExceeded { actual: usize, limit: usize },
/// At least one satisfaction path in the Miniscript fragment contains more
/// than `MAX_OPS_PER_SCRIPT`(201) opcodes.
MaxOpCountExceeded,
MaxOpCountExceeded { actual: usize, limit: usize },
/// The Miniscript(under segwit context) corresponding
/// Script would be larger than `MAX_STANDARD_P2WSH_SCRIPT_SIZE`,
/// `MAX_SCRIPT_SIZE` or `MAX_BLOCK`(`Tap`) bytes.
MaxWitnessScriptSizeExceeded { max: usize, got: usize },
/// The Miniscript (under p2sh context) corresponding Script would be
/// larger than `MAX_SCRIPT_ELEMENT_SIZE` bytes.
MaxRedeemScriptSizeExceeded,
MaxRedeemScriptSizeExceeded { max: usize, got: usize },
/// The Miniscript(under bare context) corresponding
/// Script would be larger than `MAX_SCRIPT_SIZE` bytes.
MaxBareScriptSizeExceeded,
MaxBareScriptSizeExceeded { max: usize, got: usize },
/// The policy rules of bitcoin core only permit Script size upto 1650 bytes
MaxScriptSigSizeExceeded,
MaxScriptSigSizeExceeded { actual: usize, limit: usize },
/// Impossible to satisfy the miniscript under the current context
ImpossibleSatisfaction,
/// No Multi Node in Taproot context
Expand All @@ -81,11 +81,11 @@ impl error::Error for ScriptContextError {
| XOnlyKeysNotAllowed(_, _)
| UncompressedKeysNotAllowed
| MaxWitnessItemsExceeded { .. }
| MaxOpCountExceeded
| MaxOpCountExceeded { .. }
| MaxWitnessScriptSizeExceeded { .. }
| MaxRedeemScriptSizeExceeded
| MaxBareScriptSizeExceeded
| MaxScriptSigSizeExceeded
| MaxRedeemScriptSizeExceeded { .. }
| MaxBareScriptSizeExceeded { .. }
| MaxScriptSigSizeExceeded { .. }
| ImpossibleSatisfaction
| TaprootMultiDisabled
| StackSizeLimitExceeded { .. }
Expand Down Expand Up @@ -113,35 +113,39 @@ impl fmt::Display for ScriptContextError {
}
ScriptContextError::MaxWitnessItemsExceeded { actual, limit } => write!(
f,
"At least one spending path in the Miniscript fragment has {} more \
witness items than limit {}.",
"At least one satisfaction path in the Miniscript fragment has {} witness items \
(limit: {}).",
actual, limit
),
ScriptContextError::MaxOpCountExceeded => write!(
ScriptContextError::MaxOpCountExceeded { actual, limit } => write!(
f,
"At least one satisfaction path in the Miniscript fragment contains \
more than MAX_OPS_PER_SCRIPT opcodes."
"At least one satisfaction path in the Miniscript fragment contains {} opcodes \
(limit: {}).",
actual, limit
),
ScriptContextError::MaxWitnessScriptSizeExceeded { max, got } => write!(
f,
"The Miniscript corresponding Script cannot be larger than \
{} bytes, but got {} bytes.",
{} bytes, but got {} bytes.",
max, got
),
ScriptContextError::MaxRedeemScriptSizeExceeded => write!(
ScriptContextError::MaxRedeemScriptSizeExceeded { max, got } => write!(
f,
"The Miniscript corresponding Script would be larger than \
MAX_SCRIPT_ELEMENT_SIZE bytes."
"The Miniscript corresponding Script cannot be larger than \
{} bytes, but got {} bytes.",
max, got
),
ScriptContextError::MaxBareScriptSizeExceeded => write!(
ScriptContextError::MaxBareScriptSizeExceeded { max, got } => write!(
f,
"The Miniscript corresponding Script would be larger than \
MAX_SCRIPT_SIZE bytes."
"The Miniscript corresponding Script cannot be larger than \
{} bytes, but got {} bytes.",
max, got
),
ScriptContextError::MaxScriptSigSizeExceeded => write!(
ScriptContextError::MaxScriptSigSizeExceeded { actual, limit } => write!(
f,
"At least one satisfaction in Miniscript would be larger than \
MAX_SCRIPTSIG_SIZE scriptsig"
"At least one satisfaction path in the Miniscript fragment has {} bytes \
(limit: {}).",
actual, limit
),
ScriptContextError::ImpossibleSatisfaction => {
write!(f, "Impossible to satisfy Miniscript under the current context")
Expand Down Expand Up @@ -396,8 +400,12 @@ impl ScriptContext for Legacy {
fn check_witness(witness: &[Vec<u8>]) -> Result<(), ScriptContextError> {
// In future, we could avoid by having a function to count only
// len of script instead of converting it.
if witness_to_scriptsig(witness).len() > MAX_SCRIPTSIG_SIZE {
return Err(ScriptContextError::MaxScriptSigSizeExceeded);
let script_sig = witness_to_scriptsig(witness);
if script_sig.len() > MAX_SCRIPTSIG_SIZE {
return Err(ScriptContextError::MaxScriptSigSizeExceeded {
actual: script_sig.len(),
limit: MAX_SCRIPTSIG_SIZE,
});
}
Ok(())
}
Expand All @@ -421,7 +429,10 @@ impl ScriptContext for Legacy {
match node_checked {
Ok(_) => {
if ms.ext.pk_cost > MAX_SCRIPT_ELEMENT_SIZE {
Err(ScriptContextError::MaxRedeemScriptSizeExceeded)
Err(ScriptContextError::MaxRedeemScriptSizeExceeded {
max: MAX_SCRIPT_ELEMENT_SIZE,
got: ms.ext.pk_cost,
})
} else {
Ok(())
}
Expand All @@ -434,9 +445,12 @@ impl ScriptContext for Legacy {
ms: &Miniscript<Pk, Self>,
) -> Result<(), ScriptContextError> {
match ms.ext.ops.op_count() {
None => Err(ScriptContextError::MaxOpCountExceeded),
None => Err(ScriptContextError::ImpossibleSatisfaction),
Some(op_count) if op_count > MAX_OPS_PER_SCRIPT => {
Err(ScriptContextError::MaxOpCountExceeded)
Err(ScriptContextError::MaxOpCountExceeded {
actual: op_count,
limit: MAX_OPS_PER_SCRIPT,
})
}
_ => Ok(()),
}
Expand All @@ -451,7 +465,10 @@ impl ScriptContext for Legacy {
match ms.max_satisfaction_size() {
Err(_e) => Err(ScriptContextError::ImpossibleSatisfaction),
Ok(size) if size > MAX_SCRIPTSIG_SIZE => {
Err(ScriptContextError::MaxScriptSigSizeExceeded)
Err(ScriptContextError::MaxScriptSigSizeExceeded {
actual: size,
limit: MAX_SCRIPTSIG_SIZE,
})
}
_ => Ok(()),
}
Expand Down Expand Up @@ -543,9 +560,12 @@ impl ScriptContext for Segwitv0 {
ms: &Miniscript<Pk, Self>,
) -> Result<(), ScriptContextError> {
match ms.ext.ops.op_count() {
None => Err(ScriptContextError::MaxOpCountExceeded),
None => Err(ScriptContextError::ImpossibleSatisfaction),
Some(op_count) if op_count > MAX_OPS_PER_SCRIPT => {
Err(ScriptContextError::MaxOpCountExceeded)
Err(ScriptContextError::MaxOpCountExceeded {
actual: op_count,
limit: MAX_OPS_PER_SCRIPT,
})
}
_ => Ok(()),
}
Expand Down Expand Up @@ -760,7 +780,10 @@ impl ScriptContext for BareCtx {
match node_checked {
Ok(_) => {
if ms.ext.pk_cost > MAX_SCRIPT_SIZE {
Err(ScriptContextError::MaxBareScriptSizeExceeded)
Err(ScriptContextError::MaxBareScriptSizeExceeded {
max: MAX_SCRIPT_SIZE,
got: ms.ext.pk_cost,
})
} else {
Ok(())
}
Expand All @@ -773,9 +796,12 @@ impl ScriptContext for BareCtx {
ms: &Miniscript<Pk, Self>,
) -> Result<(), ScriptContextError> {
match ms.ext.ops.op_count() {
None => Err(ScriptContextError::MaxOpCountExceeded),
None => Err(ScriptContextError::ImpossibleSatisfaction),
Some(op_count) if op_count > MAX_OPS_PER_SCRIPT => {
Err(ScriptContextError::MaxOpCountExceeded)
Err(ScriptContextError::MaxOpCountExceeded {
actual: op_count,
limit: MAX_OPS_PER_SCRIPT,
})
}
_ => Ok(()),
}
Expand Down
4 changes: 2 additions & 2 deletions src/miniscript/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1686,15 +1686,15 @@ mod tests {
// Should panic for exceeding the max consensus size, as multi properly used
assert_eq!(
legacy_multi_ms.unwrap_err().to_string(),
"The Miniscript corresponding Script would be larger than MAX_SCRIPT_ELEMENT_SIZE bytes."
"The Miniscript corresponding Script cannot be larger than 520 bytes, but got 685 bytes."
);
assert_eq!(
segwit_multi_ms.unwrap_err().to_string(),
"The Miniscript corresponding Script cannot be larger than 3600 bytes, but got 4110 bytes."
);
assert_eq!(
bare_multi_ms.unwrap_err().to_string(),
"The Miniscript corresponding Script would be larger than MAX_SCRIPT_SIZE bytes."
"The Miniscript corresponding Script cannot be larger than 10000 bytes, but got 10275 bytes."
);
}
}
Expand Down

0 comments on commit aa3691d

Please sign in to comment.