diff --git a/artifacts/program_methods/amm.bin b/artifacts/program_methods/amm.bin index 4342d19a..48ca489e 100644 Binary files a/artifacts/program_methods/amm.bin and b/artifacts/program_methods/amm.bin differ diff --git a/artifacts/program_methods/associated_token_account.bin b/artifacts/program_methods/associated_token_account.bin index a5005075..05201e8a 100644 Binary files a/artifacts/program_methods/associated_token_account.bin and b/artifacts/program_methods/associated_token_account.bin differ diff --git a/artifacts/program_methods/authenticated_transfer.bin b/artifacts/program_methods/authenticated_transfer.bin index f379f165..ee02b905 100644 Binary files a/artifacts/program_methods/authenticated_transfer.bin and b/artifacts/program_methods/authenticated_transfer.bin differ diff --git a/artifacts/program_methods/clock.bin b/artifacts/program_methods/clock.bin index 65bde241..24bc3f1b 100644 Binary files a/artifacts/program_methods/clock.bin and b/artifacts/program_methods/clock.bin differ diff --git a/artifacts/program_methods/pinata.bin b/artifacts/program_methods/pinata.bin index a0b6243c..3ef46501 100644 Binary files a/artifacts/program_methods/pinata.bin and b/artifacts/program_methods/pinata.bin differ diff --git a/artifacts/program_methods/pinata_token.bin b/artifacts/program_methods/pinata_token.bin index 446f81cb..1a111269 100644 Binary files a/artifacts/program_methods/pinata_token.bin and b/artifacts/program_methods/pinata_token.bin differ diff --git a/artifacts/program_methods/privacy_preserving_circuit.bin b/artifacts/program_methods/privacy_preserving_circuit.bin index dc85e82b..85c3a073 100644 Binary files a/artifacts/program_methods/privacy_preserving_circuit.bin and b/artifacts/program_methods/privacy_preserving_circuit.bin differ diff --git a/artifacts/program_methods/token.bin b/artifacts/program_methods/token.bin index 4eacd928..3658fe9c 100644 Binary files a/artifacts/program_methods/token.bin and b/artifacts/program_methods/token.bin differ diff --git a/artifacts/test_program_methods/burner.bin b/artifacts/test_program_methods/burner.bin index 7f044bfc..716dfe1c 100644 Binary files a/artifacts/test_program_methods/burner.bin and b/artifacts/test_program_methods/burner.bin differ diff --git a/artifacts/test_program_methods/chain_caller.bin b/artifacts/test_program_methods/chain_caller.bin index 516d8f91..907b1d93 100644 Binary files a/artifacts/test_program_methods/chain_caller.bin and b/artifacts/test_program_methods/chain_caller.bin differ diff --git a/artifacts/test_program_methods/changer_claimer.bin b/artifacts/test_program_methods/changer_claimer.bin index 4e4fc64f..960ef2df 100644 Binary files a/artifacts/test_program_methods/changer_claimer.bin and b/artifacts/test_program_methods/changer_claimer.bin differ diff --git a/artifacts/test_program_methods/claimer.bin b/artifacts/test_program_methods/claimer.bin index 869b6539..384ee42d 100644 Binary files a/artifacts/test_program_methods/claimer.bin and b/artifacts/test_program_methods/claimer.bin differ diff --git a/artifacts/test_program_methods/clock_chain_caller.bin b/artifacts/test_program_methods/clock_chain_caller.bin index 142e8734..aaad5393 100644 Binary files a/artifacts/test_program_methods/clock_chain_caller.bin and b/artifacts/test_program_methods/clock_chain_caller.bin differ diff --git a/artifacts/test_program_methods/data_changer.bin b/artifacts/test_program_methods/data_changer.bin index d708875e..17bf2895 100644 Binary files a/artifacts/test_program_methods/data_changer.bin and b/artifacts/test_program_methods/data_changer.bin differ diff --git a/artifacts/test_program_methods/extra_output.bin b/artifacts/test_program_methods/extra_output.bin index a5dc467c..94e56f7b 100644 Binary files a/artifacts/test_program_methods/extra_output.bin and b/artifacts/test_program_methods/extra_output.bin differ diff --git a/artifacts/test_program_methods/flash_swap_callback.bin b/artifacts/test_program_methods/flash_swap_callback.bin index 78cd8519..2a307c46 100644 Binary files a/artifacts/test_program_methods/flash_swap_callback.bin and b/artifacts/test_program_methods/flash_swap_callback.bin differ diff --git a/artifacts/test_program_methods/flash_swap_initiator.bin b/artifacts/test_program_methods/flash_swap_initiator.bin index f71f73b0..e16ddef0 100644 Binary files a/artifacts/test_program_methods/flash_swap_initiator.bin and b/artifacts/test_program_methods/flash_swap_initiator.bin differ diff --git a/artifacts/test_program_methods/malicious_authorization_changer.bin b/artifacts/test_program_methods/malicious_authorization_changer.bin index 5ceb9171..3cf31f3a 100644 Binary files a/artifacts/test_program_methods/malicious_authorization_changer.bin and b/artifacts/test_program_methods/malicious_authorization_changer.bin differ diff --git a/artifacts/test_program_methods/malicious_caller_program_id.bin b/artifacts/test_program_methods/malicious_caller_program_id.bin index aca8b92c..86d3a8a3 100644 Binary files a/artifacts/test_program_methods/malicious_caller_program_id.bin and b/artifacts/test_program_methods/malicious_caller_program_id.bin differ diff --git a/artifacts/test_program_methods/malicious_self_program_id.bin b/artifacts/test_program_methods/malicious_self_program_id.bin index a0f246be..17309bc6 100644 Binary files a/artifacts/test_program_methods/malicious_self_program_id.bin and b/artifacts/test_program_methods/malicious_self_program_id.bin differ diff --git a/artifacts/test_program_methods/minter.bin b/artifacts/test_program_methods/minter.bin index 7cb6c2c4..912374ce 100644 Binary files a/artifacts/test_program_methods/minter.bin and b/artifacts/test_program_methods/minter.bin differ diff --git a/artifacts/test_program_methods/missing_output.bin b/artifacts/test_program_methods/missing_output.bin index d675af45..8960f975 100644 Binary files a/artifacts/test_program_methods/missing_output.bin and b/artifacts/test_program_methods/missing_output.bin differ diff --git a/artifacts/test_program_methods/modified_transfer.bin b/artifacts/test_program_methods/modified_transfer.bin index 6c507245..ac2fc0b4 100644 Binary files a/artifacts/test_program_methods/modified_transfer.bin and b/artifacts/test_program_methods/modified_transfer.bin differ diff --git a/artifacts/test_program_methods/nonce_changer.bin b/artifacts/test_program_methods/nonce_changer.bin index d2db370d..1a80ee0c 100644 Binary files a/artifacts/test_program_methods/nonce_changer.bin and b/artifacts/test_program_methods/nonce_changer.bin differ diff --git a/artifacts/test_program_methods/noop.bin b/artifacts/test_program_methods/noop.bin index 9697f574..90491f16 100644 Binary files a/artifacts/test_program_methods/noop.bin and b/artifacts/test_program_methods/noop.bin differ diff --git a/artifacts/test_program_methods/pda_claimer.bin b/artifacts/test_program_methods/pda_claimer.bin index e7ad9462..b3a438e8 100644 Binary files a/artifacts/test_program_methods/pda_claimer.bin and b/artifacts/test_program_methods/pda_claimer.bin differ diff --git a/artifacts/test_program_methods/pinata_cooldown.bin b/artifacts/test_program_methods/pinata_cooldown.bin index 8fc5f206..1e96f5a6 100644 Binary files a/artifacts/test_program_methods/pinata_cooldown.bin and b/artifacts/test_program_methods/pinata_cooldown.bin differ diff --git a/artifacts/test_program_methods/private_pda_claimer.bin b/artifacts/test_program_methods/private_pda_claimer.bin index 9e39ff39..d69eff5c 100644 Binary files a/artifacts/test_program_methods/private_pda_claimer.bin and b/artifacts/test_program_methods/private_pda_claimer.bin differ diff --git a/artifacts/test_program_methods/program_owner_changer.bin b/artifacts/test_program_methods/program_owner_changer.bin index dbc565e8..35e51a53 100644 Binary files a/artifacts/test_program_methods/program_owner_changer.bin and b/artifacts/test_program_methods/program_owner_changer.bin differ diff --git a/artifacts/test_program_methods/simple_balance_transfer.bin b/artifacts/test_program_methods/simple_balance_transfer.bin index 2e0ef136..886e787e 100644 Binary files a/artifacts/test_program_methods/simple_balance_transfer.bin and b/artifacts/test_program_methods/simple_balance_transfer.bin differ diff --git a/artifacts/test_program_methods/time_locked_transfer.bin b/artifacts/test_program_methods/time_locked_transfer.bin index cf956702..0e18c188 100644 Binary files a/artifacts/test_program_methods/time_locked_transfer.bin and b/artifacts/test_program_methods/time_locked_transfer.bin differ diff --git a/artifacts/test_program_methods/validity_window.bin b/artifacts/test_program_methods/validity_window.bin index 8b25e015..da4ba82e 100644 Binary files a/artifacts/test_program_methods/validity_window.bin and b/artifacts/test_program_methods/validity_window.bin differ diff --git a/artifacts/test_program_methods/validity_window_chain_caller.bin b/artifacts/test_program_methods/validity_window_chain_caller.bin index db86a71c..cddf4ac2 100644 Binary files a/artifacts/test_program_methods/validity_window_chain_caller.bin and b/artifacts/test_program_methods/validity_window_chain_caller.bin differ diff --git a/nssa/core/src/program.rs b/nssa/core/src/program.rs index 041cf134..2cb52ec7 100644 --- a/nssa/core/src/program.rs +++ b/nssa/core/src/program.rs @@ -407,8 +407,8 @@ impl ProgramOutput { } /// Representation of a number as `lo + hi * 2^128`. -#[derive(PartialEq, Eq)] -struct WrappedBalanceSum { +#[derive(Debug, PartialEq, Eq)] +pub struct WrappedBalanceSum { lo: u128, hi: u128, } @@ -418,7 +418,7 @@ impl WrappedBalanceSum { /// /// Returns [`None`] if balance sum overflows `lo + hi * 2^128` representation, which is not /// expected in practical scenarios. - fn from_balances(balances: impl Iterator) -> Option { + pub fn from_balances(balances: impl Iterator) -> Option { let mut wrapped = Self { lo: 0, hi: 0 }; for balance in balances { @@ -433,6 +433,75 @@ impl WrappedBalanceSum { } } +impl std::fmt::Display for WrappedBalanceSum { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + if self.hi == 0 { + write!(f, "{}", self.lo) + } else { + write!(f, "{} * 2^128 + {}", self.hi, self.lo) + } + } +} + +impl From for WrappedBalanceSum { + fn from(value: u128) -> Self { + Self { lo: value, hi: 0 } + } +} + +#[derive(thiserror::Error, Debug)] +pub enum ExecutionValidationError { + #[error("Pre-state account IDs are not unique")] + PreStateAccountIdsNotUnique, + + #[error( + "Pre-state and post-state lengths do not match: pre-state length {pre_state_length}, post-state length {post_state_length}" + )] + MismatchedPreStatePostStateLength { + pre_state_length: usize, + post_state_length: usize, + }, + + #[error("Unallowed modification of nonce for account {account_id}")] + ModifiedNonce { account_id: AccountId }, + + #[error("Unallowed modification of program owner for account {account_id}")] + ModifiedProgramOwner { account_id: AccountId }, + + #[error( + "Trying to decrease balance of account {account_id} owned by {owner_program_id:?} in a program {executing_program_id:?} which is not the owner" + )] + UnauthorizedBalanceDecrease { + account_id: AccountId, + owner_program_id: ProgramId, + executing_program_id: ProgramId, + }, + + #[error( + "Unauthorized modification of data for account {account_id} which is not default and not owned by executing program {executing_program_id:?}" + )] + UnauthorizedDataModification { + account_id: AccountId, + executing_program_id: ProgramId, + }, + + #[error( + "Post-state for account {account_id} has default program owner but pre-state was not default" + )] + NonDefaultAccountWithDefaultOwner { account_id: AccountId }, + + #[error("Total balance across accounts overflowed 2^256 - 1")] + BalanceSumOverflow, + + #[error( + "Total balance across accounts is not preserved: total balance in pre-states {total_balance_pre_states}, total balance in post-states {total_balance_post_states}" + )] + MismatchedTotalBalance { + total_balance_pre_states: WrappedBalanceSum, + total_balance_post_states: WrappedBalanceSum, + }, +} + /// Derives an [`AccountId`] for a private PDA from the program ID, seed, and nullifier public key. /// /// Unlike public PDAs (`AccountId::from((&ProgramId, &PdaSeed))`), this includes the `npk` in the @@ -509,31 +578,39 @@ pub fn read_nssa_inputs() -> (ProgramInput, InstructionD /// - `pre_states`: The list of input accounts, each annotated with authorization metadata. /// - `post_states`: The list of resulting accounts after executing the program logic. /// - `executing_program_id`: The identifier of the program that was executed. -#[must_use] pub fn validate_execution( pre_states: &[AccountWithMetadata], post_states: &[AccountPostState], executing_program_id: ProgramId, -) -> bool { +) -> Result<(), ExecutionValidationError> { // 1. Check account ids are all different if !validate_uniqueness_of_account_ids(pre_states) { - return false; + return Err(ExecutionValidationError::PreStateAccountIdsNotUnique); } // 2. Lengths must match if pre_states.len() != post_states.len() { - return false; + return Err( + ExecutionValidationError::MismatchedPreStatePostStateLength { + pre_state_length: pre_states.len(), + post_state_length: post_states.len(), + }, + ); } for (pre, post) in pre_states.iter().zip(post_states) { // 3. Nonce must remain unchanged if pre.account.nonce != post.account.nonce { - return false; + return Err(ExecutionValidationError::ModifiedNonce { + account_id: pre.account_id, + }); } // 4. Program ownership changes are not allowed if pre.account.program_owner != post.account.program_owner { - return false; + return Err(ExecutionValidationError::ModifiedProgramOwner { + account_id: pre.account_id, + }); } let account_program_owner = pre.account.program_owner; @@ -542,7 +619,11 @@ pub fn validate_execution( if post.account.balance < pre.account.balance && account_program_owner != executing_program_id { - return false; + return Err(ExecutionValidationError::UnauthorizedBalanceDecrease { + account_id: pre.account_id, + owner_program_id: account_program_owner, + executing_program_id, + }); } // 6. Data changes only allowed if owned by executing program or if account pre state has @@ -551,13 +632,20 @@ pub fn validate_execution( && pre.account != Account::default() && account_program_owner != executing_program_id { - return false; + return Err(ExecutionValidationError::UnauthorizedDataModification { + account_id: pre.account_id, + executing_program_id, + }); } // 7. If a post state has default program owner, the pre state must have been a default // account if post.account.program_owner == DEFAULT_PROGRAM_ID && pre.account != Account::default() { - return false; + return Err( + ExecutionValidationError::NonDefaultAccountWithDefaultOwner { + account_id: pre.account_id, + }, + ); } } @@ -566,20 +654,23 @@ pub fn validate_execution( let Some(total_balance_pre_states) = WrappedBalanceSum::from_balances(pre_states.iter().map(|pre| pre.account.balance)) else { - return false; + return Err(ExecutionValidationError::BalanceSumOverflow); }; let Some(total_balance_post_states) = WrappedBalanceSum::from_balances(post_states.iter().map(|post| post.account.balance)) else { - return false; + return Err(ExecutionValidationError::BalanceSumOverflow); }; if total_balance_pre_states != total_balance_post_states { - return false; + return Err(ExecutionValidationError::MismatchedTotalBalance { + total_balance_pre_states, + total_balance_post_states, + }); } - true + Ok(()) } fn validate_uniqueness_of_account_ids(pre_states: &[AccountWithMetadata]) -> bool { diff --git a/nssa/src/error.rs b/nssa/src/error.rs index 61966515..55a0623b 100644 --- a/nssa/src/error.rs +++ b/nssa/src/error.rs @@ -1,12 +1,16 @@ use std::io; +use nssa_core::{ + account::{Account, AccountId}, + program::ProgramId, +}; use thiserror::Error; #[macro_export] macro_rules! ensure { ($cond:expr, $err:expr) => { if !$cond { - return Err($err); + return Err($err.into()); } }; } @@ -17,7 +21,7 @@ pub enum NssaError { InvalidInput(String), #[error("Program violated execution rules")] - InvalidProgramBehavior, + InvalidProgramBehavior(#[from] InvalidProgramBehaviorError), #[error("Serialization error: {0}")] InstructionSerializationError(String), @@ -32,15 +36,15 @@ pub enum NssaError { InvalidPublicKey(#[source] k256::schnorr::Error), #[error("Invalid hex for public key")] - InvalidHexPublicKey(hex::FromHexError), + InvalidHexPublicKey(#[source] hex::FromHexError), - #[error("Risc0 error: {0}")] + #[error("Failed to write program input: {0}")] ProgramWriteInputFailed(String), - #[error("Risc0 error: {0}")] + #[error("Failed to execute program: {0}")] ProgramExecutionFailed(String), - #[error("Risc0 error: {0}")] + #[error("Failed to prove program: {0}")] ProgramProveFailed(String), #[error("Invalid transaction: {0}")] @@ -77,6 +81,67 @@ pub enum NssaError { OutOfValidityWindow, } +#[derive(Error, Debug)] +pub enum InvalidProgramBehaviorError { + #[error( + "Inconsistent pre-state for account {account_id} : expected {expected:?}, actual {actual:?}" + )] + InconsistentAccountPreState { + account_id: AccountId, + // Boxed to reduce the size of the error type + expected: Box, + actual: Box, + }, + + #[error( + "Inconsistent authorization for account {account_id} : expected {expected_authorization}, actual {actual_authorization}" + )] + InconsistentAccountAuthorization { + account_id: AccountId, + expected_authorization: bool, + actual_authorization: bool, + }, + + #[error("Program ID mismatch: expected {expected:?}, actual {actual:?}")] + MismatchedProgramId { + expected: ProgramId, + actual: ProgramId, + }, + + #[error("Caller program ID mismatch: expected {expected:?}, actual {actual:?}")] + MismatchedCallerProgramId { + expected: Option, + actual: Option, + }, + + #[error(transparent)] + ExecutionValidationFailed(#[from] nssa_core::program::ExecutionValidationError), + + #[error("Trying to claim account {account_id} which is not default")] + ClaimedNonDefaultAccount { account_id: AccountId }, + + #[error("Trying to claim account {account_id} which is not authorized")] + ClaimedUnauthorizedAccount { account_id: AccountId }, + + #[error("PDA claim mismatch: expected {expected:?}, actual {actual:?}")] + MismatchedPdaClaim { + expected: AccountId, + actual: AccountId, + }, + + #[error("Private PDA claim mismatch: expected {expected:?}, actual {actual:?}")] + MismatchedPrivatePdaClaim { + expected: AccountId, + actual: AccountId, + }, + + #[error("Default account {account_id} was modified without being claimed")] + DefaultAccountModifiedWithoutClaim { account_id: AccountId }, + + #[error("Called program {program_id:?} which is not listed in dependencies")] + UndeclaredProgramDependency { program_id: ProgramId }, +} + #[cfg(test)] mod tests { diff --git a/nssa/src/privacy_preserving_transaction/circuit.rs b/nssa/src/privacy_preserving_transaction/circuit.rs index 6a327335..049de9b9 100644 --- a/nssa/src/privacy_preserving_transaction/circuit.rs +++ b/nssa/src/privacy_preserving_transaction/circuit.rs @@ -10,7 +10,7 @@ use nssa_core::{ use risc0_zkvm::{ExecutorEnv, InnerReceipt, ProverOpts, Receipt, default_prover}; use crate::{ - error::NssaError, + error::{InvalidProgramBehaviorError, NssaError}, program::Program, program_methods::{PRIVACY_PRESERVING_CIRCUIT_ELF, PRIVACY_PRESERVING_CIRCUIT_ID}, state::MAX_NUMBER_CHAINED_CALLS, @@ -114,9 +114,11 @@ pub fn execute_and_prove( env_builder.add_assumption(inner_receipt); for new_call in program_output.chained_calls.into_iter().rev() { - let next_program = dependencies - .get(&new_call.program_id) - .ok_or(NssaError::InvalidProgramBehavior)?; + let next_program = dependencies.get(&new_call.program_id).ok_or( + InvalidProgramBehaviorError::UndeclaredProgramDependency { + program_id: new_call.program_id, + }, + )?; chained_calls.push_front((new_call, next_program, Some(chained_call.program_id))); } diff --git a/nssa/src/state.rs b/nssa/src/state.rs index b19decd2..dc11b567 100644 --- a/nssa/src/state.rs +++ b/nssa/src/state.rs @@ -367,14 +367,14 @@ pub mod tests { account::{Account, AccountId, AccountWithMetadata, Nonce, data::Data}, encryption::{EphemeralPublicKey, Scalar, ViewingPublicKey}, program::{ - BlockValidityWindow, PdaSeed, ProgramId, TimestampValidityWindow, - private_pda_account_id, + BlockValidityWindow, ExecutionValidationError, PdaSeed, ProgramId, + TimestampValidityWindow, WrappedBalanceSum, private_pda_account_id, }, }; use crate::{ PublicKey, PublicTransaction, V03State, - error::NssaError, + error::{InvalidProgramBehaviorError, NssaError}, execute_and_prove, privacy_preserving_transaction::{ PrivacyPreservingTransaction, @@ -936,10 +936,11 @@ pub mod tests { #[test] fn program_should_fail_if_modifies_nonces() { - let initial_data = [(AccountId::new([1; 32]), 100)]; + let account_id = AccountId::new([1; 32]); + let initial_data = [(account_id, 100)]; let mut state = V03State::new_with_genesis_accounts(&initial_data, vec![], 0).with_test_programs(); - let account_ids = vec![AccountId::new([1; 32])]; + let account_ids = vec![account_id]; let program_id = Program::nonce_changer_program().id(); let message = public_transaction::Message::try_new(program_id, account_ids, vec![], ()).unwrap(); @@ -948,7 +949,14 @@ pub mod tests { let result = state.transition_from_public_transaction(&tx, 1, 0); - assert!(matches!(result, Err(NssaError::InvalidProgramBehavior))); + assert!(matches!( + result, + Err(NssaError::InvalidProgramBehavior( + InvalidProgramBehaviorError::ExecutionValidationFailed( + ExecutionValidationError::ModifiedNonce { account_id: err_account_id } + ) + )) if err_account_id == account_id + )); } #[test] @@ -965,7 +973,17 @@ pub mod tests { let result = state.transition_from_public_transaction(&tx, 1, 0); - assert!(matches!(result, Err(NssaError::InvalidProgramBehavior))); + assert!(matches!( + result, + Err(NssaError::InvalidProgramBehavior( + InvalidProgramBehaviorError::ExecutionValidationFailed( + ExecutionValidationError::MismatchedPreStatePostStateLength { + pre_state_length, + post_state_length + } + ) + )) if pre_state_length == 1 && post_state_length == 2 + )); } #[test] @@ -982,7 +1000,17 @@ pub mod tests { let result = state.transition_from_public_transaction(&tx, 1, 0); - assert!(matches!(result, Err(NssaError::InvalidProgramBehavior))); + assert!(matches!( + result, + Err(NssaError::InvalidProgramBehavior( + InvalidProgramBehaviorError::ExecutionValidationFailed( + ExecutionValidationError::MismatchedPreStatePostStateLength { + pre_state_length, + post_state_length + } + ) + )) if pre_state_length == 2 && post_state_length == 1 + )); } #[test] @@ -1006,7 +1034,12 @@ pub mod tests { let result = state.transition_from_public_transaction(&tx, 1, 0); - assert!(matches!(result, Err(NssaError::InvalidProgramBehavior))); + assert!(matches!( + result, + Err(NssaError::InvalidProgramBehavior(InvalidProgramBehaviorError::ExecutionValidationFailed( + ExecutionValidationError::ModifiedProgramOwner { account_id: err_account_id } + ))) if err_account_id == account_id + )); } #[test] @@ -1030,7 +1063,12 @@ pub mod tests { let result = state.transition_from_public_transaction(&tx, 1, 0); - assert!(matches!(result, Err(NssaError::InvalidProgramBehavior))); + assert!(matches!( + result, + Err(NssaError::InvalidProgramBehavior(InvalidProgramBehaviorError::ExecutionValidationFailed( + ExecutionValidationError::ModifiedProgramOwner { account_id: err_account_id } + ))) if err_account_id == account_id + )); } #[test] @@ -1054,7 +1092,12 @@ pub mod tests { let result = state.transition_from_public_transaction(&tx, 1, 0); - assert!(matches!(result, Err(NssaError::InvalidProgramBehavior))); + assert!(matches!( + result, + Err(NssaError::InvalidProgramBehavior(InvalidProgramBehaviorError::ExecutionValidationFailed( + ExecutionValidationError::ModifiedProgramOwner { account_id: err_account_id } + ))) if err_account_id == account_id + )); } #[test] @@ -1078,16 +1121,21 @@ pub mod tests { let result = state.transition_from_public_transaction(&tx, 1, 0); - assert!(matches!(result, Err(NssaError::InvalidProgramBehavior))); + assert!(matches!( + result, + Err(NssaError::InvalidProgramBehavior(InvalidProgramBehaviorError::ExecutionValidationFailed( + ExecutionValidationError::ModifiedProgramOwner { account_id: err_account_id } + ))) if err_account_id == account_id + )); } #[test] fn program_should_fail_if_transfers_balance_from_non_owned_account() { - let initial_data = [(AccountId::new([1; 32]), 100)]; - let mut state = - V03State::new_with_genesis_accounts(&initial_data, vec![], 0).with_test_programs(); let sender_account_id = AccountId::new([1; 32]); let receiver_account_id = AccountId::new([2; 32]); + let initial_data = [(sender_account_id, 100)]; + let mut state = + V03State::new_with_genesis_accounts(&initial_data, vec![], 0).with_test_programs(); let balance_to_move: u128 = 1; let program_id = Program::simple_balance_transfer().id(); assert_ne!( @@ -1106,7 +1154,12 @@ pub mod tests { let result = state.transition_from_public_transaction(&tx, 1, 0); - assert!(matches!(result, Err(NssaError::InvalidProgramBehavior))); + assert!(matches!( + result, + Err(NssaError::InvalidProgramBehavior(InvalidProgramBehaviorError::ExecutionValidationFailed( + ExecutionValidationError::UnauthorizedBalanceDecrease { account_id: err_account_id, owner_program_id, executing_program_id } + ))) if err_account_id == sender_account_id && owner_program_id != program_id && executing_program_id == program_id + )); } #[test] @@ -1131,7 +1184,12 @@ pub mod tests { let result = state.transition_from_public_transaction(&tx, 1, 0); - assert!(matches!(result, Err(NssaError::InvalidProgramBehavior))); + assert!(matches!( + result, + Err(NssaError::InvalidProgramBehavior(InvalidProgramBehaviorError::ExecutionValidationFailed( + ExecutionValidationError::UnauthorizedDataModification { account_id: err_account_id, executing_program_id } + ))) if err_account_id == account_id && executing_program_id == program_id + )); } #[test] @@ -1149,7 +1207,12 @@ pub mod tests { let result = state.transition_from_public_transaction(&tx, 1, 0); - assert!(matches!(result, Err(NssaError::InvalidProgramBehavior))); + assert!(matches!( + result, + Err(NssaError::InvalidProgramBehavior(InvalidProgramBehaviorError::ExecutionValidationFailed( + ExecutionValidationError::MismatchedTotalBalance { total_balance_pre_states, total_balance_post_states } + ))) if total_balance_pre_states == 0.into() && total_balance_post_states == 1.into() + )); } #[test] @@ -1178,7 +1241,12 @@ pub mod tests { let tx = PublicTransaction::new(message, witness_set); let result = state.transition_from_public_transaction(&tx, 1, 0); - assert!(matches!(result, Err(NssaError::InvalidProgramBehavior))); + assert!(matches!( + result, + Err(NssaError::InvalidProgramBehavior(InvalidProgramBehaviorError::ExecutionValidationFailed( + ExecutionValidationError::MismatchedTotalBalance { total_balance_pre_states, total_balance_post_states } + ))) if total_balance_pre_states == 100.into() && total_balance_post_states == 99.into() + )); } fn test_public_account_keys_1() -> TestPublicKeys { @@ -3209,7 +3277,12 @@ pub mod tests { let result = state.transition_from_public_transaction(&tx, 1, 0); - assert!(matches!(result, Err(NssaError::InvalidProgramBehavior))); + assert!(matches!( + result, + Err(NssaError::InvalidProgramBehavior( + InvalidProgramBehaviorError::ClaimedNonDefaultAccount { account_id: err_account_id } + )) if err_account_id == account_id + )); } /// This test ensures that even if a malicious program tries to perform overflow of balances @@ -3255,7 +3328,22 @@ pub mod tests { let witness_set = public_transaction::WitnessSet::for_message(&message, &[&sender_key]); let tx = PublicTransaction::new(message, witness_set); let res = state.transition_from_public_transaction(&tx, 1, 0); - assert!(matches!(res, Err(NssaError::InvalidProgramBehavior))); + let expected_total_balance_pre_states = WrappedBalanceSum::from_balances( + [sender_init_balance, recipient_init_balance].into_iter(), + ) + .unwrap(); + let expected_total_balance_post_states = WrappedBalanceSum::from_balances( + [sender_init_balance, recipient_init_balance, u128::MAX, 1].into_iter(), + ) + .unwrap(); + assert!(matches!( + res, + Err(NssaError::InvalidProgramBehavior( + InvalidProgramBehaviorError::ExecutionValidationFailed( + ExecutionValidationError::MismatchedTotalBalance { total_balance_pre_states, total_balance_post_states } + ) + )) if total_balance_pre_states == expected_total_balance_pre_states && total_balance_post_states == expected_total_balance_post_states + )); let sender_post = state.get_account_by_id(sender_id); let recipient_post = state.get_account_by_id(recipient_id); @@ -3500,7 +3588,14 @@ pub mod tests { let result = state.transition_from_public_transaction(&tx, 1, 0); // Should fail - cannot modify data without claiming the account - assert!(matches!(result, Err(NssaError::InvalidProgramBehavior))); + assert!(matches!( + result, + Err(NssaError::InvalidProgramBehavior( + InvalidProgramBehaviorError::DefaultAccountModifiedWithoutClaim { + account_id: err_account_id + } + )) if err_account_id == account_id + )); } #[test] diff --git a/nssa/src/validated_state_diff.rs b/nssa/src/validated_state_diff.rs index 71b7e5e5..744402ee 100644 --- a/nssa/src/validated_state_diff.rs +++ b/nssa/src/validated_state_diff.rs @@ -15,7 +15,7 @@ use nssa_core::{ use crate::{ V03State, ensure, - error::NssaError, + error::{InvalidProgramBehaviorError, NssaError}, privacy_preserving_transaction::{ PrivacyPreservingTransaction, circuit::Proof, message::Message, }, @@ -150,39 +150,52 @@ impl ValidatedStateDiff { .unwrap_or_else(|| state.get_account_by_id(account_id)); ensure!( pre.account == expected_pre, - NssaError::InvalidProgramBehavior + InvalidProgramBehaviorError::InconsistentAccountPreState { + account_id, + expected: Box::new(expected_pre), + actual: Box::new(pre.account.clone()) + } ); // Check that authorization flags are consistent with the provided ones or // authorized by program through the PDA mechanism + let expected_is_authorized = is_authorized(&account_id); ensure!( - pre.is_authorized == is_authorized(&account_id), - NssaError::InvalidProgramBehavior + pre.is_authorized == expected_is_authorized, + InvalidProgramBehaviorError::InconsistentAccountAuthorization { + account_id, + expected_authorization: expected_is_authorized, + actual_authorization: pre.is_authorized + } ); } // Verify that the program output's self_program_id matches the expected program ID. ensure!( program_output.self_program_id == chained_call.program_id, - NssaError::InvalidProgramBehavior + InvalidProgramBehaviorError::MismatchedProgramId { + expected: chained_call.program_id, + actual: program_output.self_program_id + } ); // Verify that the program output's caller_program_id matches the actual caller. ensure!( program_output.caller_program_id == caller_program_id, - NssaError::InvalidProgramBehavior + InvalidProgramBehaviorError::MismatchedCallerProgramId { + expected: caller_program_id, + actual: program_output.caller_program_id, + } ); // Verify execution corresponds to a well-behaved program. // See the # Programs section for the definition of the `validate_execution` method. - ensure!( - validate_execution( - &program_output.pre_states, - &program_output.post_states, - chained_call.program_id, - ), - NssaError::InvalidProgramBehavior - ); + validate_execution( + &program_output.pre_states, + &program_output.post_states, + chained_call.program_id, + ) + .map_err(InvalidProgramBehaviorError::ExecutionValidationFailed)?; // Verify validity window ensure!( @@ -197,31 +210,43 @@ impl ValidatedStateDiff { let Some(claim) = post.required_claim() else { continue; }; + let account_id = program_output.pre_states[i].account_id; + // The invoked program can only claim accounts with default program id. ensure!( post.account().program_owner == DEFAULT_PROGRAM_ID, - NssaError::InvalidProgramBehavior + InvalidProgramBehaviorError::ClaimedNonDefaultAccount { account_id } ); - let account_id = program_output.pre_states[i].account_id; - match claim { Claim::Authorized => { // The program can only claim accounts that were authorized by the signer. ensure!( is_authorized(&account_id), - NssaError::InvalidProgramBehavior + InvalidProgramBehaviorError::ClaimedUnauthorizedAccount { account_id } ); } Claim::Pda(seed) => { // The program can only claim accounts that correspond to the PDAs it is // authorized to claim. let pda = AccountId::from((&chained_call.program_id, &seed)); - ensure!(account_id == pda, NssaError::InvalidProgramBehavior); + ensure!( + account_id == pda, + InvalidProgramBehaviorError::MismatchedPdaClaim { + expected: pda, + actual: account_id + } + ); } Claim::PrivatePda { seed, npk } => { let pda = private_pda_account_id(&chained_call.program_id, &seed, &npk); - ensure!(account_id == pda, NssaError::InvalidProgramBehavior); + ensure!( + account_id == pda, + InvalidProgramBehaviorError::MismatchedPrivatePdaClaim { + expected: pda, + actual: account_id + } + ); } } @@ -247,7 +272,7 @@ impl ValidatedStateDiff { } // Check that all modified uninitialized accounts where claimed - for post in state_diff.iter().filter_map(|(account_id, post)| { + for (account_id, post) in state_diff.iter().filter_map(|(account_id, post)| { let pre = state.get_account_by_id(*account_id); if pre.program_owner != DEFAULT_PROGRAM_ID { return None; @@ -255,11 +280,11 @@ impl ValidatedStateDiff { if pre == *post { return None; } - Some(post) + Some((*account_id, post)) }) { ensure!( post.program_owner != DEFAULT_PROGRAM_ID, - NssaError::InvalidProgramBehavior + InvalidProgramBehaviorError::DefaultAccountModifiedWithoutClaim { account_id } ); } diff --git a/program_methods/guest/src/bin/privacy_preserving_circuit.rs b/program_methods/guest/src/bin/privacy_preserving_circuit.rs index dd97ab09..0b046d25 100644 --- a/program_methods/guest/src/bin/privacy_preserving_circuit.rs +++ b/program_methods/guest/src/bin/privacy_preserving_circuit.rs @@ -133,12 +133,17 @@ impl ExecutionState { // Check that the program is well behaved. // See the # Programs section for the definition of the `validate_execution` method. - let execution_valid = validate_execution( + let validated_execution = validate_execution( &program_output.pre_states, &program_output.post_states, chained_call.program_id, ); - assert!(execution_valid, "Bad behaved program"); + if let Err(err) = validated_execution { + panic!( + "Invalid program behavior in program {:?}: {err}", + chained_call.program_id + ); + } // Collect private-PDA bindings from this program_output's proven data. Each // `private_pda_seeds` entry in an outgoing chained call attests that the caller