From 5ef3115ae31427f70a36756eeab8cb3f7a126d7f Mon Sep 17 00:00:00 2001 From: seun Date: Tue, 1 Oct 2024 16:59:03 -0700 Subject: [PATCH 1/4] feat: impl check_error_condition --- packages/engine/src/errors.cairo | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/engine/src/errors.cairo b/packages/engine/src/errors.cairo index 447347c6..f7f5d05d 100644 --- a/packages/engine/src/errors.cairo +++ b/packages/engine/src/errors.cairo @@ -26,6 +26,8 @@ pub mod Error { pub const DISCOURAGE_UPGRADABLE_WITNESS_PROGRAM: felt252 = 'Upgradable witness program'; pub const WITNESS_PROGRAM_INVALID: felt252 = 'Invalid witness program'; pub const SCRIPT_TOO_LARGE: felt252 = 'Script is too large'; + + pub const SCRIPT_UNFINISHED: felt252 = 'Script unfinished'; } pub fn byte_array_err(err: felt252) -> ByteArray { From 3b1b92b07b814b4ef5bcd870865dc37afc0b7261 Mon Sep 17 00:00:00 2001 From: seun Date: Mon, 7 Oct 2024 15:14:31 +0100 Subject: [PATCH 2/4] update impl chek-error --- packages/engine/src/engine.cairo | 77 +++++++++++++++++++------------- 1 file changed, 47 insertions(+), 30 deletions(-) diff --git a/packages/engine/src/engine.cairo b/packages/engine/src/engine.cairo index c319563d..77c02945 100644 --- a/packages/engine/src/engine.cairo +++ b/packages/engine/src/engine.cairo @@ -147,6 +147,8 @@ pub trait EngineExtrasTrait { fn is_witness_active(ref self: Engine, version: i64) -> bool; // Return the script since last OP_CODESEPARATOR fn sub_script(ref self: Engine) -> ByteArray; + // Check if the script has failed and return an error if it has + fn check_error_condition(ref self: Engine, final_script: bool) -> Result<(), felt252>; } pub impl EngineExtrasImpl> of EngineExtrasTrait { @@ -203,6 +205,43 @@ pub impl EngineExtrasImpl> of EngineExtrasTrait { }; return sub_script; } + + fn check_error_condition(ref self: Engine, final_script: bool) -> Result<(), felt252> { + // Check if execution is actually done + if self.script_idx < self.scripts.len() { + return Result::Err(Error::SCRIPT_UNFINISHED); + } + + // Check if witness stack is clean + if final_script + && self.is_witness_active(0) + && self.dstack.len() != 1 { // TODO: Hardcoded 0 + return Result::Err(Error::SCRIPT_NON_CLEAN_STACK); + } + if final_script + && self.has_flag(ScriptFlags::ScriptVerifyCleanStack) + && self.dstack.len() != 1 { + return Result::Err(Error::SCRIPT_NON_CLEAN_STACK); + } + + // Check if stack has at least one item + if self.dstack.len() < 1 { + return Result::Err(Error::SCRIPT_EMPTY_STACK); + } + + // Check the final stack value + match self.dstack.pop_bool() { + Result::Ok(v) => { + if !v { + // TODO: Implement logging of failed scripts if needed + return Result::Err(Error::SCRIPT_FAILED); + } + }, + Result::Err(e) => { return Result::Err(e); }, + } + + Result::Ok(()) + } } pub trait EngineInternalTrait { @@ -640,36 +679,14 @@ pub impl EngineInternalImpl of EngineInternalTrait { return Result::Err(err); } - // TODO: CheckErrorCondition - if self.is_witness_active(0) && self.dstack.len() != 1 { // TODO: Hardcoded 0 - return Result::Err(Error::SCRIPT_NON_CLEAN_STACK); - } - if self.has_flag(ScriptFlags::ScriptVerifyCleanStack) && self.dstack.len() != 1 { - return Result::Err(Error::SCRIPT_NON_CLEAN_STACK); - } - - if self.dstack.len() < 1 { - return Result::Err(Error::SCRIPT_EMPTY_STACK); - } else { - // TODO: pop bool? - let top_stack = self.dstack.peek_byte_array(0)?; - let ret_val = top_stack.clone(); - let mut is_ok = false; - let mut i = 0; - while i != top_stack.len() { - if top_stack[i] != 0 { - is_ok = true; - break; - } - i += 1; - }; - if is_ok { - return Result::Ok(ret_val); - } else { - return Result::Err(Error::SCRIPT_FAILED); - } - } - } + match self.check_error_condition(true) { + Result::Ok(_) => { + // Script executed successfully, return top of stack + self.dstack.peek_byte_array(0) + }, + Result::Err(e) => Result::Err(e), + } + } fn verify_witness( ref self: Engine, witness: Span From bf242d065980ab8c5ac70dd41b5eea0e3ee23166 Mon Sep 17 00:00:00 2001 From: Brandon Roberts Date: Tue, 8 Oct 2024 09:35:14 -0400 Subject: [PATCH 3/4] Cleanup failing tests --- packages/engine/src/engine.cairo | 39 +++++++++++--------------------- 1 file changed, 13 insertions(+), 26 deletions(-) diff --git a/packages/engine/src/engine.cairo b/packages/engine/src/engine.cairo index 77c02945..6b6054fd 100644 --- a/packages/engine/src/engine.cairo +++ b/packages/engine/src/engine.cairo @@ -148,7 +148,7 @@ pub trait EngineExtrasTrait { // Return the script since last OP_CODESEPARATOR fn sub_script(ref self: Engine) -> ByteArray; // Check if the script has failed and return an error if it has - fn check_error_condition(ref self: Engine, final_script: bool) -> Result<(), felt252>; + fn check_error_condition(ref self: Engine) -> Result; } pub impl EngineExtrasImpl> of EngineExtrasTrait { @@ -206,20 +206,18 @@ pub impl EngineExtrasImpl> of EngineExtrasTrait { return sub_script; } - fn check_error_condition(ref self: Engine, final_script: bool) -> Result<(), felt252> { + fn check_error_condition(ref self: Engine) -> Result { // Check if execution is actually done if self.script_idx < self.scripts.len() { return Result::Err(Error::SCRIPT_UNFINISHED); } // Check if witness stack is clean - if final_script - && self.is_witness_active(0) + if self.is_witness_active(0) && self.dstack.len() != 1 { // TODO: Hardcoded 0 return Result::Err(Error::SCRIPT_NON_CLEAN_STACK); } - if final_script - && self.has_flag(ScriptFlags::ScriptVerifyCleanStack) + if self.has_flag(ScriptFlags::ScriptVerifyCleanStack) && self.dstack.len() != 1 { return Result::Err(Error::SCRIPT_NON_CLEAN_STACK); } @@ -227,20 +225,15 @@ pub impl EngineExtrasImpl> of EngineExtrasTrait { // Check if stack has at least one item if self.dstack.len() < 1 { return Result::Err(Error::SCRIPT_EMPTY_STACK); + } else { + // Check the final stack value + let is_ok = self.dstack.peek_bool(0)?; + if is_ok { + return Result::Ok(self.dstack.peek_byte_array(0)?); + } else { + return Result::Err(Error::SCRIPT_FAILED); + } } - - // Check the final stack value - match self.dstack.pop_bool() { - Result::Ok(v) => { - if !v { - // TODO: Implement logging of failed scripts if needed - return Result::Err(Error::SCRIPT_FAILED); - } - }, - Result::Err(e) => { return Result::Err(e); }, - } - - Result::Ok(()) } } @@ -679,13 +672,7 @@ pub impl EngineInternalImpl of EngineInternalTrait { return Result::Err(err); } - match self.check_error_condition(true) { - Result::Ok(_) => { - // Script executed successfully, return top of stack - self.dstack.peek_byte_array(0) - }, - Result::Err(e) => Result::Err(e), - } + return self.check_error_condition(); } fn verify_witness( From 9d7d887694aa15a1597db7549ed71e2b1bcb84ef Mon Sep 17 00:00:00 2001 From: Brandon Roberts Date: Tue, 8 Oct 2024 09:39:50 -0400 Subject: [PATCH 4/4] Scarb fmt --- packages/engine/src/engine.cairo | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/packages/engine/src/engine.cairo b/packages/engine/src/engine.cairo index 10b21bac..f71d8b4f 100644 --- a/packages/engine/src/engine.cairo +++ b/packages/engine/src/engine.cairo @@ -411,8 +411,8 @@ pub impl EngineImpl< return Result::Err(err); } - return self.check_error_condition(); - } + return self.check_error_condition(); + } } // TODO: Remove functions that can be locally used only @@ -721,12 +721,10 @@ pub impl EngineInternalImpl< } // Check if witness stack is clean - if self.is_witness_active(0) - && self.dstack.len() != 1 { // TODO: Hardcoded 0 + if self.is_witness_active(0) && self.dstack.len() != 1 { // TODO: Hardcoded 0 return Result::Err(Error::SCRIPT_NON_CLEAN_STACK); } - if self.has_flag(ScriptFlags::ScriptVerifyCleanStack) - && self.dstack.len() != 1 { + if self.has_flag(ScriptFlags::ScriptVerifyCleanStack) && self.dstack.len() != 1 { return Result::Err(Error::SCRIPT_NON_CLEAN_STACK); }