diff --git a/contracts/examples/plugin-policy-example-reverts/src/lib.rs b/contracts/examples/plugin-policy-example-reverts/src/lib.rs index a058176..e53af08 100644 --- a/contracts/examples/plugin-policy-example-reverts/src/lib.rs +++ b/contracts/examples/plugin-policy-example-reverts/src/lib.rs @@ -1,5 +1,5 @@ #![no_std] -use smart_account_interfaces::{SmartAccountPlugin, SmartAccountPolicy}; +use smart_account_interfaces::{PluginRejection, SmartAccountPlugin, SmartAccountPolicy}; use soroban_sdk::{ auth::{Context, ContractContext}, contract, contractimpl, contracttype, symbol_short, Address, Env, Symbol, TryFromVal, Vec, @@ -41,7 +41,7 @@ impl SmartAccountPlugin for PluginPolicyContractReverts { source.require_auth(); } - fn on_auth(env: &Env, source: Address, contexts: Vec) { + fn on_auth(env: &Env, source: Address, contexts: Vec) -> Result<(), PluginRejection> { source.require_auth(); // Increment the internal counter let current_counter: u32 = env.storage().instance().get(&AUTH_COUNTER_KEY).unwrap_or(0); @@ -60,6 +60,8 @@ impl SmartAccountPlugin for PluginPolicyContractReverts { counter: new_counter, }, ); + + Ok(()) } } diff --git a/contracts/examples/plugin-policy-example/src/lib.rs b/contracts/examples/plugin-policy-example/src/lib.rs index 4926334..5172ab1 100644 --- a/contracts/examples/plugin-policy-example/src/lib.rs +++ b/contracts/examples/plugin-policy-example/src/lib.rs @@ -1,5 +1,5 @@ #![no_std] -use smart_account_interfaces::{SmartAccountPlugin, SmartAccountPolicy}; +use smart_account_interfaces::{PluginRejection, SmartAccountPlugin, SmartAccountPolicy}; use soroban_sdk::{ auth::{Context, ContractContext}, contract, contractimpl, contracttype, symbol_short, Address, Env, Symbol, TryFromVal, Vec, @@ -36,7 +36,7 @@ impl SmartAccountPlugin for PluginPolicyContract { source.require_auth(); } - fn on_auth(env: &Env, source: Address, contexts: Vec) { + fn on_auth(env: &Env, source: Address, contexts: Vec) -> Result<(), PluginRejection> { source.require_auth(); // Increment the internal counter let current_counter: u32 = env.storage().instance().get(&AUTH_COUNTER_KEY).unwrap_or(0); @@ -55,6 +55,8 @@ impl SmartAccountPlugin for PluginPolicyContract { counter: new_counter, }, ); + + Ok(()) } } diff --git a/contracts/smart-account-interfaces/src/error.rs b/contracts/smart-account-interfaces/src/error.rs index d7b6f92..abe8028 100644 --- a/contracts/smart-account-interfaces/src/error.rs +++ b/contracts/smart-account-interfaces/src/error.rs @@ -1,5 +1,16 @@ use soroban_sdk::contracterror; +/// Dedicated rejection signal for the plugin `on_auth` interface. +/// +/// Plugins return `Err(PluginRejection::Rejected)` to intentionally block +/// authorization. Any panic is treated as a technical failure and skipped. +#[contracterror] +#[derive(Copy, Clone, Debug, PartialEq)] +#[repr(u32)] +pub enum PluginRejection { + Rejected = 1, +} + #[contracterror] #[derive(Copy, Clone, Debug, PartialEq)] #[repr(u32)] diff --git a/contracts/smart-account-interfaces/src/lib.rs b/contracts/smart-account-interfaces/src/lib.rs index 3e65a97..ee2af62 100644 --- a/contracts/smart-account-interfaces/src/lib.rs +++ b/contracts/smart-account-interfaces/src/lib.rs @@ -12,5 +12,5 @@ pub use auth::types::{ SignerKey, SignerPolicy, SignerRole, SpendTrackerKey, SpendingTracker, TokenTransferPolicy, WebauthnSigner, }; -pub use error::SmartAccountError; +pub use error::{PluginRejection, SmartAccountError}; pub use plugin::{SmartAccountPlugin, SmartAccountPluginClient}; diff --git a/contracts/smart-account-interfaces/src/plugin.rs b/contracts/smart-account-interfaces/src/plugin.rs index 3e7c866..4471ca7 100644 --- a/contracts/smart-account-interfaces/src/plugin.rs +++ b/contracts/smart-account-interfaces/src/plugin.rs @@ -1,8 +1,9 @@ +use crate::error::PluginRejection; use soroban_sdk::{auth::Context, contractclient, Address, Env, Vec}; #[contractclient(name = "SmartAccountPluginClient")] pub trait SmartAccountPlugin { fn on_install(env: &Env, source: Address); fn on_uninstall(env: &Env, source: Address); - fn on_auth(env: &Env, source: Address, contexts: Vec); + fn on_auth(env: &Env, source: Address, contexts: Vec) -> Result<(), PluginRejection>; } diff --git a/contracts/smart-account/src/auth/core/authorizer.rs b/contracts/smart-account/src/auth/core/authorizer.rs index f82ff0d..596a242 100644 --- a/contracts/smart-account/src/auth/core/authorizer.rs +++ b/contracts/smart-account/src/auth/core/authorizer.rs @@ -10,8 +10,8 @@ use crate::error::Error; use crate::events::PluginAuthFailedEvent; use smart_account_interfaces::SmartAccountPluginClient; use smart_account_interfaces::{Signer, SignerKey, SignerRole}; -use soroban_sdk::{auth::Context, crypto::Hash, Env, Vec}; -use soroban_sdk::{Address, Map, String, Symbol}; +use soroban_sdk::auth::Context; +use soroban_sdk::{crypto::Hash, Address, Env, InvokeError, Map, String, Symbol, Vec}; use storage::Storage; pub struct Authorizer; @@ -92,10 +92,10 @@ impl Authorizer { let res = SmartAccountPluginClient::new(env, &plugin) .try_on_auth(&env.current_contract_address(), auth_contexts); match res { - // Plugin executed successfully + // Plugin approved (new-style Ok(()) or old-style void return — + // both produce Void at the ABI level). Ok(Ok(_)) => {} - // Plugin return value conversion failure (ABI mismatch) - // Treat as technical failure: log and continue + // Return value conversion failure (ABI mismatch) — skip. Ok(Err(_)) => { env.events().publish( (TOPIC_PLUGIN, &plugin, VERB_AUTH_FAILED), @@ -105,7 +105,7 @@ impl Authorizer { }, ); } - // Plugin intentionally rejected (contracterror / panic_with_error!) + // Plugin explicitly rejected via Err(PluginRejection::Rejected). Err(Ok(_)) => { env.events().publish( (TOPIC_PLUGIN, &plugin, VERB_AUTH_FAILED), @@ -116,9 +116,22 @@ impl Authorizer { ); return Err(Error::PluginOnAuthFailed); } - // Plugin had a technical failure (panic!, host trap, TTL expiry) - // Non-blocking: log and continue to next plugin - Err(Err(_)) => { + // Old-style rejection: panic_with_error! with a contract error + // code that doesn't match PluginRejection. Treat as rejection + // to preserve backwards compatibility with existing plugins. + Err(Err(InvokeError::Contract(_))) => { + env.events().publish( + (TOPIC_PLUGIN, &plugin, VERB_AUTH_FAILED), + PluginAuthFailedEvent { + plugin: plugin.clone(), + error: String::from_str(env, "Plugin rejected authorization"), + }, + ); + return Err(Error::PluginOnAuthFailed); + } + // Technical failure: bare panic!, missing function, host trap, + // budget exhaustion, expired TTL. Non-blocking — skip. + Err(Err(InvokeError::Abort)) => { env.events().publish( (TOPIC_PLUGIN, &plugin, VERB_AUTH_FAILED), PluginAuthFailedEvent { diff --git a/contracts/smart-account/src/tests/plugin_test.rs b/contracts/smart-account/src/tests/plugin_test.rs index 7c9024f..a4ac1e6 100644 --- a/contracts/smart-account/src/tests/plugin_test.rs +++ b/contracts/smart-account/src/tests/plugin_test.rs @@ -16,7 +16,8 @@ use crate::{ use smart_account_interfaces::{SignerRole, SmartAccountInterface}; // ----------------------------------------------------------------------------- -// Dummy plugin contract that increments a counter on every on_auth +// Dummy plugin contract that increments a counter on every on_auth. +// Returns () (void) — exercises ABI backwards compatibility. // ----------------------------------------------------------------------------- const COUNT: Symbol = symbol_short!("cnt"); @@ -45,20 +46,49 @@ impl DummyPlugin { } // ----------------------------------------------------------------------------- -// Plugin that intentionally rejects authorization (panic_with_error!) -// Wrapped in a submodule to avoid contractimpl symbol collisions. +// Plugin that rejects via new-style Err(PluginRejection::Rejected). // ----------------------------------------------------------------------------- mod rejecting_plugin { - use soroban_sdk::{auth::Context, contract, contractimpl, panic_with_error, Address, Env, Vec}; + use soroban_sdk::{auth::Context, contract, contractimpl, Address, Env, Vec}; - use crate::error::Error; + use smart_account_interfaces::PluginRejection; #[contract] pub struct RejectingPlugin; #[contractimpl] impl RejectingPlugin { + pub fn on_install(_env: &Env, _source: Address) {} + + pub fn on_uninstall(_env: &Env, _source: Address) {} + + pub fn on_auth( + _env: &Env, + _source: Address, + _contexts: Vec, + ) -> Result<(), PluginRejection> { + Err(PluginRejection::Rejected) + } + } +} + +use rejecting_plugin::RejectingPlugin; + +// ----------------------------------------------------------------------------- +// Plugin that bare-panics (technical failure, no contract error code). +// ----------------------------------------------------------------------------- + +mod panicking_plugin { + use soroban_sdk::{auth::Context, contract, contractimpl, Address, Env, Vec}; + + use crate::error::Error; + + #[contract] + pub struct PanickingPlugin; + + #[contractimpl] + impl PanickingPlugin { pub fn on_install(_env: &Env, _source: Address) -> Result<(), Error> { Ok(()) } @@ -67,15 +97,99 @@ mod rejecting_plugin { Ok(()) } + pub fn on_auth(_env: &Env, _source: Address, _contexts: Vec) { + panic!("crashed"); + } + } +} + +use panicking_plugin::PanickingPlugin; + +// ----------------------------------------------------------------------------- +// Plugin that rejects with old-style panic_with_error! using a custom +// #[contracterror] (code 200, outside SmartAccountError/PluginRejection). +// Tests backwards compatibility with existing deployed plugins. +// ----------------------------------------------------------------------------- + +mod custom_error_plugin { + use soroban_sdk::{ + auth::Context, contract, contracterror, contractimpl, panic_with_error, Address, Env, Vec, + }; + + #[contracterror] + #[derive(Copy, Clone, Debug, PartialEq)] + #[repr(u32)] + pub enum CustomPluginError { + Rejected = 200, + } + + #[contract] + pub struct CustomErrorPlugin; + + #[contractimpl] + impl CustomErrorPlugin { + pub fn on_install(_env: &Env, _source: Address) {} + + pub fn on_uninstall(_env: &Env, _source: Address) {} + pub fn on_auth(env: &Env, _source: Address, _contexts: Vec) { - // This uses panic_with_error! which produces Err(Ok(e)) in try_on_auth, - // meaning the plugin intentionally rejected authorization. - panic_with_error!(env, Error::PluginOnAuthFailed); + panic_with_error!(env, CustomPluginError::Rejected); } } } -use rejecting_plugin::RejectingPlugin; +use custom_error_plugin::CustomErrorPlugin; + +// ----------------------------------------------------------------------------- +// Plugin that has on_install but no on_auth (missing callback). +// ----------------------------------------------------------------------------- + +mod no_auth_plugin { + use soroban_sdk::{contract, contractimpl, Address, Env}; + + use crate::error::Error; + + #[contract] + pub struct NoAuthPlugin; + + #[contractimpl] + impl NoAuthPlugin { + pub fn on_install(_env: &Env, _source: Address) -> Result<(), Error> { + Ok(()) + } + + pub fn on_uninstall(_env: &Env, _source: Address) -> Result<(), Error> { + Ok(()) + } + } +} + +use no_auth_plugin::NoAuthPlugin; + +// ============================================================================= +// Helper: run __check_auth with given auth contexts +// ============================================================================= + +fn check_auth( + env: &Env, + smart_account_id: &Address, + admin: &Ed25519TestSigner, + contexts: &Vec, +) -> Result<(), Result> { + let payload = BytesN::random(env); + let (admin_key, admin_proof) = admin.sign(env, &payload); + let auth_payloads = SignatureProofs(soroban_sdk::map![env, (admin_key, admin_proof)]); + env.try_invoke_contract_check_auth::( + smart_account_id, + &payload, + auth_payloads.into_val(env), + contexts, + ) +} + +// ============================================================================= +// Tests +// ============================================================================= // ----------------------------------------------------------------------------- // Test: Uninstall properly persists removal, plugin no longer receives on_auth @@ -86,7 +200,6 @@ fn test_uninstall_plugin_persists_removal() { let env = setup(); env.mock_all_auths(); - // Deploy SmartAccount with one admin signer let admin = Ed25519TestSigner::generate(SignerRole::Admin); let smart_account_id = env.register( SmartAccount, @@ -96,16 +209,13 @@ fn test_uninstall_plugin_persists_removal() { ), ); - // Deploy dummy plugin let plugin_id = env.register(DummyPlugin, ()); - - // Install plugin env.as_contract(&smart_account_id, || { SmartAccount::install_plugin(&env, plugin_id.clone()) }) .unwrap(); - // Verify plugin is installed by triggering on_auth + // Trigger on_auth let payload = BytesN::random(&env); let (admin_key, admin_proof) = admin.sign(&env, &payload); let auth_payloads = SignatureProofs(soroban_sdk::map![ @@ -121,30 +231,24 @@ fn test_uninstall_plugin_persists_removal() { ) .unwrap(); - // Verify plugin is installed assert!(env.as_contract(&smart_account_id, || { SmartAccount::is_plugin_installed(&env, plugin_id.clone()) })); - // Verify plugin received on_auth call let count_after_install = env.as_contract(&plugin_id, || DummyPlugin::get_count(&env)); - assert_eq!( - count_after_install, 1, - "Plugin should have received on_auth call" - ); + assert_eq!(count_after_install, 1); - // Uninstall plugin (FIX: removal now persisted to storage) + // Uninstall env.as_contract(&smart_account_id, || { SmartAccount::uninstall_plugin(&env, plugin_id.clone()) }) .unwrap(); - // Verify plugin is uninstalled assert!(!env.as_contract(&smart_account_id, || { SmartAccount::is_plugin_installed(&env, plugin_id.clone()) })); - // Trigger __check_auth again to see if plugin.on_auth still runs + // Trigger on_auth again — plugin should NOT run let payload2 = BytesN::random(&env); let (admin_key2, admin_proof2) = admin.sign(&env, &payload2); let auth_payloads2 = SignatureProofs(soroban_sdk::map![&env, (admin_key2, admin_proof2)]); @@ -157,24 +261,19 @@ fn test_uninstall_plugin_persists_removal() { ) .unwrap(); - // Verify plugin did NOT receive second on_auth call (count should still be 1) let count_after_uninstall = env.as_contract(&plugin_id, || DummyPlugin::get_count(&env)); - assert_eq!( - count_after_uninstall, 1, - "Plugin should NOT receive on_auth after uninstall" - ); + assert_eq!(count_after_uninstall, 1); } // ----------------------------------------------------------------------------- -// Test: Intentional plugin rejection blocks authorization +// Test: New-style Err(PluginRejection::Rejected) blocks auth // ----------------------------------------------------------------------------- #[test] -fn test_plugin_intentional_rejection_blocks_auth() { +fn test_plugin_new_style_rejection_blocks_auth() { let env = setup(); env.mock_all_auths(); - // Deploy SmartAccount with one admin signer let admin = Ed25519TestSigner::generate(SignerRole::Admin); let smart_account_id = env.register( SmartAccount, @@ -184,31 +283,182 @@ fn test_plugin_intentional_rejection_blocks_auth() { ), ); - // Deploy and install the rejecting plugin let plugin_id = env.register(RejectingPlugin, ()); env.as_contract(&smart_account_id, || { SmartAccount::install_plugin(&env, plugin_id.clone()) }) .unwrap(); - // Try to authenticate - let payload = BytesN::random(&env); - let (admin_key, admin_proof) = admin.sign(&env, &payload); - let auth_payloads = SignatureProofs(soroban_sdk::map![&env, (admin_key, admin_proof)]); + let contexts = vec![&env, get_token_auth_context(&env)]; + let result = check_auth(&env, &smart_account_id, &admin, &contexts); - let result = env.try_invoke_contract_check_auth::( - &smart_account_id, - &payload, - auth_payloads.into_val(&env), - &vec![&env, get_token_auth_context(&env)], + assert_eq!(result, Err(Ok(Error::PluginOnAuthFailed))); +} + +// ----------------------------------------------------------------------------- +// Test: Old-style panic_with_error! with custom error still blocks (backwards compat) +// ----------------------------------------------------------------------------- + +#[test] +fn test_plugin_old_style_rejection_still_blocks() { + let env = setup(); + env.mock_all_auths(); + + let admin = Ed25519TestSigner::generate(SignerRole::Admin); + let smart_account_id = env.register( + SmartAccount, + ( + vec![&env, admin.into_signer(&env)], + Vec::
::new(&env), + ), ); - // The rejecting plugin (panic_with_error!) should cause PluginOnAuthFailed, - // blocking authorization. This exercises the Err(Ok(_)) branch in - // call_plugins_on_auth. + let plugin_id = env.register(CustomErrorPlugin, ()); + env.as_contract(&smart_account_id, || { + SmartAccount::install_plugin(&env, plugin_id.clone()) + }) + .unwrap(); + + let contexts = vec![&env, get_token_auth_context(&env)]; + let result = check_auth(&env, &smart_account_id, &admin, &contexts); + assert_eq!(result, Err(Ok(Error::PluginOnAuthFailed))); } +// ----------------------------------------------------------------------------- +// Test: Old void-returning plugin still works (ABI backwards compat) +// ----------------------------------------------------------------------------- + +#[test] +fn test_old_void_plugin_still_works() { + let env = setup(); + env.mock_all_auths(); + + let admin = Ed25519TestSigner::generate(SignerRole::Admin); + let smart_account_id = env.register( + SmartAccount, + ( + vec![&env, admin.into_signer(&env)], + Vec::
::new(&env), + ), + ); + + // DummyPlugin returns () — not Result<(), PluginRejection> + let plugin_id = env.register(DummyPlugin, ()); + env.as_contract(&smart_account_id, || { + SmartAccount::install_plugin(&env, plugin_id.clone()) + }) + .unwrap(); + + let contexts = vec![&env, get_token_auth_context(&env)]; + let result = check_auth(&env, &smart_account_id, &admin, &contexts); + + // Void return and Ok(()) produce the same ABI representation. + assert!(result.is_ok()); + + let count = env.as_contract(&plugin_id, || DummyPlugin::get_count(&env)); + assert_eq!(count, 1); +} + +// ----------------------------------------------------------------------------- +// Test: Bare panic!() is skipped as technical failure +// ----------------------------------------------------------------------------- + +#[test] +fn test_plugin_bare_panic_skipped() { + let env = setup(); + env.mock_all_auths(); + + let admin = Ed25519TestSigner::generate(SignerRole::Admin); + let smart_account_id = env.register( + SmartAccount, + ( + vec![&env, admin.into_signer(&env)], + Vec::
::new(&env), + ), + ); + + let plugin_id = env.register(PanickingPlugin, ()); + env.as_contract(&smart_account_id, || { + SmartAccount::install_plugin(&env, plugin_id.clone()) + }) + .unwrap(); + + let contexts = vec![&env, get_token_auth_context(&env)]; + let result = check_auth(&env, &smart_account_id, &admin, &contexts); + + assert!(result.is_ok()); +} + +// ----------------------------------------------------------------------------- +// Test: Missing on_auth callback is skipped as technical failure +// ----------------------------------------------------------------------------- + +#[test] +fn test_plugin_missing_on_auth_skipped() { + let env = setup(); + env.mock_all_auths(); + + let admin = Ed25519TestSigner::generate(SignerRole::Admin); + let smart_account_id = env.register( + SmartAccount, + ( + vec![&env, admin.into_signer(&env)], + Vec::
::new(&env), + ), + ); + + let plugin_id = env.register(NoAuthPlugin, ()); + env.as_contract(&smart_account_id, || { + SmartAccount::install_plugin(&env, plugin_id.clone()) + }) + .unwrap(); + + let contexts = vec![&env, get_token_auth_context(&env)]; + let result = check_auth(&env, &smart_account_id, &admin, &contexts); + + assert!(result.is_ok()); +} + +// ----------------------------------------------------------------------------- +// Test: Crashing plugin does not prevent other plugins from running +// ----------------------------------------------------------------------------- + +#[test] +fn test_mixed_plugins_crash_does_not_block() { + let env = setup(); + env.mock_all_auths(); + + let admin = Ed25519TestSigner::generate(SignerRole::Admin); + let smart_account_id = env.register( + SmartAccount, + ( + vec![&env, admin.into_signer(&env)], + Vec::
::new(&env), + ), + ); + + let dummy_id = env.register(DummyPlugin, ()); + env.as_contract(&smart_account_id, || { + SmartAccount::install_plugin(&env, dummy_id.clone()) + }) + .unwrap(); + + let panicking_id = env.register(PanickingPlugin, ()); + env.as_contract(&smart_account_id, || { + SmartAccount::install_plugin(&env, panicking_id.clone()) + }) + .unwrap(); + + let contexts = vec![&env, get_token_auth_context(&env)]; + let result = check_auth(&env, &smart_account_id, &admin, &contexts); + + assert!(result.is_ok()); + + let count = env.as_contract(&dummy_id, || DummyPlugin::get_count(&env)); + assert_eq!(count, 1, "DummyPlugin should still receive on_auth"); +} + // ----------------------------------------------------------------------------- // Test: Installing more than MAX_PLUGINS plugins fails with MaxPluginsReached // ----------------------------------------------------------------------------- @@ -218,7 +468,6 @@ fn test_max_plugins_limit() { let env = setup(); env.mock_all_auths(); - // Deploy SmartAccount with one admin signer let admin = Ed25519TestSigner::generate(SignerRole::Admin); let smart_account_id = env.register( SmartAccount, @@ -228,7 +477,6 @@ fn test_max_plugins_limit() { ), ); - // Install 10 plugins (the maximum allowed) for i in 0..10u32 { let plugin_id = env.register(DummyPlugin, ()); env.as_contract(&smart_account_id, || { @@ -237,7 +485,6 @@ fn test_max_plugins_limit() { .unwrap_or_else(|e| panic!("Plugin {} install should succeed but got: {:?}", i, e)); } - // The 11th plugin should fail with MaxPluginsReached let extra_plugin_id = env.register(DummyPlugin, ()); let err = env .as_contract(&smart_account_id, || { @@ -247,3 +494,414 @@ fn test_max_plugins_limit() { assert_eq!(err, Error::MaxPluginsReached); } + +// ============================================================================= +// ABI backwards compatibility — exhaustive tests +// +// These tests verify that every old-style plugin pattern continues to work +// after the on_auth return type changed from () to Result<(), PluginRejection>. +// ============================================================================= + +// -- Additional old-style plugin variants ------------------------------------ + +/// Old-style plugin that does storage writes, event emission, and returns void. +/// Simulates a "real" plugin doing actual work. +mod storage_event_plugin { + use soroban_sdk::{ + auth::Context, contract, contractimpl, contracttype, symbol_short, Address, Env, Symbol, + Vec, + }; + + const KEY: Symbol = symbol_short!("se_cnt"); + + #[contracttype] + #[derive(Clone, Debug)] + pub struct PluginAuthEvent { + pub source: Address, + pub count: u32, + } + + #[contract] + pub struct StorageEventPlugin; + + #[contractimpl] + impl StorageEventPlugin { + pub fn on_install(_env: &Env, _source: Address) {} + pub fn on_uninstall(_env: &Env, _source: Address) {} + pub fn on_auth(env: &Env, source: Address, _contexts: Vec) { + let count: u32 = env.storage().instance().get(&KEY).unwrap_or(0); + let new_count = count + 1; + env.storage().instance().set(&KEY, &new_count); + env.events().publish( + (symbol_short!("AUTH"),), + PluginAuthEvent { + source, + count: new_count, + }, + ); + } + pub fn get_count(env: &Env) -> u32 { + env.storage().instance().get(&KEY).unwrap_or(0) + } + } +} + +use storage_event_plugin::StorageEventPlugin; + +/// Old-style plugin that returns Result<(), SmartAccountError> with Ok(()). +/// Some plugins may have adopted Result return early — this must still work. +mod result_ok_plugin { + use soroban_sdk::{auth::Context, contract, contractimpl, Address, Env, Vec}; + + use crate::error::Error; + + #[contract] + pub struct ResultOkPlugin; + + #[contractimpl] + impl ResultOkPlugin { + pub fn on_install(_env: &Env, _source: Address) -> Result<(), Error> { + Ok(()) + } + pub fn on_uninstall(_env: &Env, _source: Address) -> Result<(), Error> { + Ok(()) + } + pub fn on_auth(_env: &Env, _source: Address, _contexts: Vec) -> Result<(), Error> { + Ok(()) + } + } +} + +use result_ok_plugin::ResultOkPlugin; + +/// Old-style plugin that returns Result<(), SmartAccountError> with Err. +/// Tests that a plugin rejecting via Result::Err with a SmartAccountError +/// is correctly classified as an intentional rejection. +mod result_err_plugin { + use soroban_sdk::{auth::Context, contract, contractimpl, Address, Env, Vec}; + + use crate::error::Error; + + #[contract] + pub struct ResultErrPlugin; + + #[contractimpl] + impl ResultErrPlugin { + pub fn on_install(_env: &Env, _source: Address) -> Result<(), Error> { + Ok(()) + } + pub fn on_uninstall(_env: &Env, _source: Address) -> Result<(), Error> { + Ok(()) + } + pub fn on_auth(_env: &Env, _source: Address, _contexts: Vec) -> Result<(), Error> { + Err(Error::PluginOnAuthFailed) + } + } +} + +use result_err_plugin::ResultErrPlugin; + +/// Old-style plugin that uses panic_with_error! with a SmartAccountError code +/// (the pattern used by the original RejectingPlugin in pre-#115 code). +mod old_panic_with_known_error_plugin { + use soroban_sdk::{auth::Context, contract, contractimpl, panic_with_error, Address, Env, Vec}; + + use crate::error::Error; + + #[contract] + pub struct OldPanicKnownErrorPlugin; + + #[contractimpl] + impl OldPanicKnownErrorPlugin { + pub fn on_install(_env: &Env, _source: Address) {} + pub fn on_uninstall(_env: &Env, _source: Address) {} + pub fn on_auth(env: &Env, _source: Address, _contexts: Vec) { + panic_with_error!(env, Error::PluginOnAuthFailed); + } + } +} + +use old_panic_with_known_error_plugin::OldPanicKnownErrorPlugin; + +// -- Direct try_on_auth result verification ---------------------------------- + +/// Calls try_on_auth directly on each plugin type and asserts the exact +/// Result variant. This is the most granular verification that the ABI +/// change doesn't alter the observable error classification. +#[test] +fn test_try_on_auth_result_variants() { + use smart_account_interfaces::SmartAccountPluginClient; + use soroban_sdk::testutils::Address as _; + use soroban_sdk::InvokeError; + + let env = setup(); + env.mock_all_auths(); + + let source = soroban_sdk::Address::generate(&env); + let contexts = Vec::new(&env); + + // 1. DummyPlugin (void return, succeeds) → Ok(Ok(())) + let id = env.register(DummyPlugin, ()); + let res = SmartAccountPluginClient::new(&env, &id).try_on_auth(&source, &contexts); + assert!( + matches!(res, Ok(Ok(()))), + "void-returning success should be Ok(Ok(())): {:?}", + res + ); + + // 2. StorageEventPlugin (void return, does real work) → Ok(Ok(())) + let id = env.register(StorageEventPlugin, ()); + let res = SmartAccountPluginClient::new(&env, &id).try_on_auth(&source, &contexts); + assert!( + matches!(res, Ok(Ok(()))), + "void-returning plugin with storage+events should be Ok(Ok(())): {:?}", + res + ); + // Verify the plugin actually executed (side-effect check) + let count = env.as_contract(&id, || StorageEventPlugin::get_count(&env)); + assert_eq!(count, 1, "StorageEventPlugin should have executed its body"); + + // 3. ResultOkPlugin (returns Result<(), Error> with Ok(())) → Ok(Ok(())) + let id = env.register(ResultOkPlugin, ()); + let res = SmartAccountPluginClient::new(&env, &id).try_on_auth(&source, &contexts); + assert!( + matches!(res, Ok(Ok(()))), + "Result::Ok(()) should be Ok(Ok(())): {:?}", + res + ); + + // 4. RejectingPlugin (new-style Err(PluginRejection)) → Err(Ok(_)) + let id = env.register(RejectingPlugin, ()); + let res = SmartAccountPluginClient::new(&env, &id).try_on_auth(&source, &contexts); + assert!( + matches!(res, Err(Ok(_))), + "new-style Err(PluginRejection) should be Err(Ok(_)): {:?}", + res + ); + + // 5. ResultErrPlugin (Result::Err(SmartAccountError)) → Err(Err(Contract(_))) + // SmartAccountError code doesn't match PluginRejection, so TryFrom fails + // and it becomes InvokeError::Contract(code). + let id = env.register(ResultErrPlugin, ()); + let res = SmartAccountPluginClient::new(&env, &id).try_on_auth(&source, &contexts); + assert!( + matches!(res, Err(Err(InvokeError::Contract(_)))), + "Result::Err(SmartAccountError) should be Err(Err(Contract(_))): {:?}", + res + ); + + // 6. OldPanicKnownErrorPlugin (panic_with_error! with SmartAccountError) + // → Err(Err(Contract(_))) because the SmartAccountError code doesn't match + // PluginRejection variants. + let id = env.register(OldPanicKnownErrorPlugin, ()); + let res = SmartAccountPluginClient::new(&env, &id).try_on_auth(&source, &contexts); + assert!( + matches!(res, Err(Err(InvokeError::Contract(_)))), + "panic_with_error!(SmartAccountError) should be Err(Err(Contract(_))): {:?}", + res + ); + + // 7. CustomErrorPlugin (panic_with_error! with custom code 200) + // → Err(Err(Contract(200))) + let id = env.register(CustomErrorPlugin, ()); + let res = SmartAccountPluginClient::new(&env, &id).try_on_auth(&source, &contexts); + assert!( + matches!(res, Err(Err(InvokeError::Contract(200)))), + "panic_with_error!(CustomPluginError::Rejected=200) should be Err(Err(Contract(200))): {:?}", + res + ); + + // 8. PanickingPlugin (bare panic!) → Err(Err(Abort)) + let id = env.register(PanickingPlugin, ()); + let res = SmartAccountPluginClient::new(&env, &id).try_on_auth(&source, &contexts); + assert!( + matches!(res, Err(Err(InvokeError::Abort))), + "bare panic! should be Err(Err(Abort)): {:?}", + res + ); + + // 9. NoAuthPlugin (missing on_auth) → Err(Err(Abort)) + let id = env.register(NoAuthPlugin, ()); + let res = SmartAccountPluginClient::new(&env, &id).try_on_auth(&source, &contexts); + assert!( + matches!(res, Err(Err(InvokeError::Abort))), + "missing on_auth should be Err(Err(Abort)): {:?}", + res + ); +} + +// -- End-to-end auth tests for each old-style plugin ------------------------- + +#[test] +fn test_old_void_plugin_with_storage_and_events_works() { + let env = setup(); + env.mock_all_auths(); + + let admin = Ed25519TestSigner::generate(SignerRole::Admin); + let smart_account_id = env.register( + SmartAccount, + ( + vec![&env, admin.into_signer(&env)], + Vec::
::new(&env), + ), + ); + + let plugin_id = env.register(StorageEventPlugin, ()); + env.as_contract(&smart_account_id, || { + SmartAccount::install_plugin(&env, plugin_id.clone()) + }) + .unwrap(); + + let contexts = vec![&env, get_token_auth_context(&env)]; + let result = check_auth(&env, &smart_account_id, &admin, &contexts); + assert!( + result.is_ok(), + "void plugin with storage+events should pass auth" + ); + + // Verify the plugin's side effects actually ran + let count = env.as_contract(&plugin_id, || StorageEventPlugin::get_count(&env)); + assert_eq!(count, 1, "plugin should have incremented counter"); + + // Run auth again to verify counter increments + let result2 = check_auth(&env, &smart_account_id, &admin, &contexts); + assert!(result2.is_ok()); + let count2 = env.as_contract(&plugin_id, || StorageEventPlugin::get_count(&env)); + assert_eq!(count2, 2, "plugin should have incremented counter again"); +} + +#[test] +fn test_old_result_ok_plugin_works() { + let env = setup(); + env.mock_all_auths(); + + let admin = Ed25519TestSigner::generate(SignerRole::Admin); + let smart_account_id = env.register( + SmartAccount, + ( + vec![&env, admin.into_signer(&env)], + Vec::
::new(&env), + ), + ); + + let plugin_id = env.register(ResultOkPlugin, ()); + env.as_contract(&smart_account_id, || { + SmartAccount::install_plugin(&env, plugin_id.clone()) + }) + .unwrap(); + + let contexts = vec![&env, get_token_auth_context(&env)]; + let result = check_auth(&env, &smart_account_id, &admin, &contexts); + assert!( + result.is_ok(), + "plugin returning Result::Ok(()) should pass auth" + ); +} + +#[test] +fn test_old_result_err_plugin_blocks_auth() { + let env = setup(); + env.mock_all_auths(); + + let admin = Ed25519TestSigner::generate(SignerRole::Admin); + let smart_account_id = env.register( + SmartAccount, + ( + vec![&env, admin.into_signer(&env)], + Vec::
::new(&env), + ), + ); + + let plugin_id = env.register(ResultErrPlugin, ()); + env.as_contract(&smart_account_id, || { + SmartAccount::install_plugin(&env, plugin_id.clone()) + }) + .unwrap(); + + let contexts = vec![&env, get_token_auth_context(&env)]; + let result = check_auth(&env, &smart_account_id, &admin, &contexts); + assert_eq!( + result, + Err(Ok(Error::PluginOnAuthFailed)), + "plugin returning Result::Err should block auth" + ); +} + +#[test] +fn test_old_panic_with_known_error_blocks_auth() { + let env = setup(); + env.mock_all_auths(); + + let admin = Ed25519TestSigner::generate(SignerRole::Admin); + let smart_account_id = env.register( + SmartAccount, + ( + vec![&env, admin.into_signer(&env)], + Vec::
::new(&env), + ), + ); + + // This is the exact pattern used by the old RejectingPlugin before PR #115: + // panic_with_error!(env, Error::PluginOnAuthFailed) + let plugin_id = env.register(OldPanicKnownErrorPlugin, ()); + env.as_contract(&smart_account_id, || { + SmartAccount::install_plugin(&env, plugin_id.clone()) + }) + .unwrap(); + + let contexts = vec![&env, get_token_auth_context(&env)]; + let result = check_auth(&env, &smart_account_id, &admin, &contexts); + assert_eq!( + result, + Err(Ok(Error::PluginOnAuthFailed)), + "old-style panic_with_error!(SmartAccountError) should still block auth" + ); +} + +/// Mixed scenario: one old-style void plugin + one new-style Result plugin, +/// both approving. Verifies both can coexist. +#[test] +fn test_mixed_old_and_new_plugins_both_approve() { + let env = setup(); + env.mock_all_auths(); + + let admin = Ed25519TestSigner::generate(SignerRole::Admin); + let smart_account_id = env.register( + SmartAccount, + ( + vec![&env, admin.into_signer(&env)], + Vec::
::new(&env), + ), + ); + + // Old-style void plugin + let dummy_id = env.register(DummyPlugin, ()); + env.as_contract(&smart_account_id, || { + SmartAccount::install_plugin(&env, dummy_id.clone()) + }) + .unwrap(); + + // Old-style Result::Ok plugin + let result_ok_id = env.register(ResultOkPlugin, ()); + env.as_contract(&smart_account_id, || { + SmartAccount::install_plugin(&env, result_ok_id.clone()) + }) + .unwrap(); + + // Old-style void plugin with side effects + let storage_id = env.register(StorageEventPlugin, ()); + env.as_contract(&smart_account_id, || { + SmartAccount::install_plugin(&env, storage_id.clone()) + }) + .unwrap(); + + let contexts = vec![&env, get_token_auth_context(&env)]; + let result = check_auth(&env, &smart_account_id, &admin, &contexts); + assert!(result.is_ok(), "all three approving plugins should pass"); + + // Verify both side-effect plugins ran + let dummy_count = env.as_contract(&dummy_id, || DummyPlugin::get_count(&env)); + assert_eq!(dummy_count, 1); + let storage_count = env.as_contract(&storage_id, || StorageEventPlugin::get_count(&env)); + assert_eq!(storage_count, 1); +}