diff --git a/config.example.toml b/config.example.toml index f88ef2270..3548c92f1 100644 --- a/config.example.toml +++ b/config.example.toml @@ -227,6 +227,17 @@ risk-server-url = "http://127.0.0.1:3001" # Env: MBV_CHAINLINK__RISK__REQUEST_TIMEOUT request-timeout = "5s" +# Which post-delegation action signers to risk check. +# "all-signers": check every signer of every action (conservative). +# "relevant-programs": only check when an action involves a value +# transferring program: SPL Token (legacy and 2022), +# ephemeral SPL (eATA/ESPL), Magic, or the (ephemeral) +# system program. Actions touching none of these +# activate without any risk server call. +# Default: "all-signers" +# Env: MBV_CHAINLINK__RISK__CHECK_STRATEGY +check-strategy = "all-signers" + # ============================================================================== # Leader Administration (Optional) # ============================================================================== diff --git a/magicblock-aml/src/lib.rs b/magicblock-aml/src/lib.rs index ef941a919..2b2c6a495 100644 --- a/magicblock-aml/src/lib.rs +++ b/magicblock-aml/src/lib.rs @@ -6,7 +6,7 @@ //! asks the server whether a set of addresses is risky. use futures_util::future::try_join_all; -use magicblock_config::config::RiskConfig; +use magicblock_config::config::{AmlCheckStrategy, RiskConfig}; use reqwest::{Client, redirect}; use serde::Deserialize; use thiserror::Error; @@ -68,6 +68,7 @@ fn validate_base_url(base_url: &str) -> RiskResult<()> { pub struct RiskService { client: Client, base_url: String, + check_strategy: AmlCheckStrategy, } impl RiskService { @@ -90,7 +91,17 @@ impl RiskService { .build() .map_err(RiskError::ClientBuild)?; - Ok(Some(Self { client, base_url })) + Ok(Some(Self { + client, + base_url, + check_strategy: config.check_strategy, + })) + } + + /// The configured strategy for deciding which post-delegation action + /// signers to risk check. + pub fn check_strategy(&self) -> AmlCheckStrategy { + self.check_strategy } /// Asks the risk server about each address concurrently, returning @@ -234,6 +245,7 @@ mod tests { enabled: true, risk_server_url, request_timeout: Duration::from_secs(2), + check_strategy: AmlCheckStrategy::AllSigners, } } diff --git a/magicblock-chainlink/src/chainlink/fetch_cloner/mod.rs b/magicblock-chainlink/src/chainlink/fetch_cloner/mod.rs index 2ff049bba..1200900ad 100644 --- a/magicblock-chainlink/src/chainlink/fetch_cloner/mod.rs +++ b/magicblock-chainlink/src/chainlink/fetch_cloner/mod.rs @@ -22,10 +22,10 @@ use engine::Engine; use keeper::MissingAccount; use lru::LruCache; use magicblock_aml::RiskService; -use magicblock_config::config::AllowedProgram; +use magicblock_config::config::{AllowedProgram, AmlCheckStrategy}; use magicblock_core::token_programs::{ - ASSOCIATED_TOKEN_PROGRAM_ID, EATA_PROGRAM_ID, TOKEN_PROGRAM_ID, is_ata, - normalize_native_token_account_for_local_clone, + ASSOCIATED_TOKEN_PROGRAM_ID, EATA_PROGRAM_ID, TOKEN_2022_PROGRAM_ID, + TOKEN_PROGRAM_ID, is_ata, normalize_native_token_account_for_local_clone, }; use magicblock_metrics::metrics::{ self, AccountFetchContext, AccountFetchReason, BankPrecheckOutcome, @@ -352,6 +352,47 @@ fn log_companion_fetch_failure( ); } +/// Programs whose presence in a post-delegation action triggers a risk check +/// under [`AmlCheckStrategy::RelevantPrograms`]: SPL Token (legacy and 2022), +/// the ephemeral SPL / eATA program (ESPL), the Magic program, and the system +/// programs. The latter move native SOL, which is as much value as a token +/// balance, so leaving them out would let a plain lamport transfer signed by a +/// sanctioned address through unchecked. +const RISK_RELEVANT_PROGRAMS: [Pubkey; 6] = [ + TOKEN_PROGRAM_ID, + TOKEN_2022_PROGRAM_ID, + EATA_PROGRAM_ID, + magicblock_magic_program_api::ID, + solana_sdk_ids::system_program::ID, + magicblock_magic_program_api::EPHEMERAL_SYSTEM_PROGRAM_ID, +]; + +/// Decides whether the configured [`AmlCheckStrategy`] requires risk checking +/// the signers of these post-delegation actions. +fn delegation_actions_require_risk_check( + strategy: AmlCheckStrategy, + delegation_actions: &DelegationActions, +) -> bool { + match strategy { + AmlCheckStrategy::AllSigners => true, + AmlCheckStrategy::RelevantPrograms => delegation_actions + .iter() + .any(instruction_involves_risk_relevant_program), + } +} + +/// Returns true when a risk-relevant program is invoked by the instruction or +/// referenced by any of its accounts (e.g. as the target of a CPI). +fn instruction_involves_risk_relevant_program( + instruction: &solana_instruction::Instruction, +) -> bool { + RISK_RELEVANT_PROGRAMS.contains(&instruction.program_id) + || instruction + .accounts + .iter() + .any(|meta| RISK_RELEVANT_PROGRAMS.contains(&meta.pubkey)) +} + impl FetchCloner where T: ChainRpcClient, @@ -1734,6 +1775,22 @@ where return Ok(()); }; + let strategy = risk_service.check_strategy(); + if !delegation_actions_require_risk_check(strategy, delegation_actions) + { + // A suppressed check is a compliance-relevant event, so leave a + // record that the delegation was activated without a risk query. + debug!( + strategy = ?strategy, + action_programs = ?delegation_actions + .iter() + .map(|ix| ix.program_id) + .collect::>(), + "Skipping risk check for post-delegation actions" + ); + return Ok(()); + } + let mut signers = delegation_actions .iter() .flat_map(|instruction| { diff --git a/magicblock-chainlink/src/chainlink/fetch_cloner/tests.rs b/magicblock-chainlink/src/chainlink/fetch_cloner/tests.rs index cd8abe8f3..8c1e428ea 100644 --- a/magicblock-chainlink/src/chainlink/fetch_cloner/tests.rs +++ b/magicblock-chainlink/src/chainlink/fetch_cloner/tests.rs @@ -82,3 +82,115 @@ fn clone_request_classification() { ChainlinkCloneIntent::ActionDependency ); } + +mod aml_check_strategy { + use solana_instruction::AccountMeta; + + use super::*; + + fn action_for_program(program_id: Pubkey) -> Instruction { + Instruction::new_with_bytes(program_id, &[], vec![]) + } + + fn action_referencing(program_id: Pubkey, account: Pubkey) -> Instruction { + Instruction::new_with_bytes( + program_id, + &[], + vec![AccountMeta::new_readonly(account, false)], + ) + } + + #[test] + fn all_signers_strategy_always_requires_check() { + // An action that touches no risk-relevant program still gets checked. + let actions: DelegationActions = + vec![action_for_program(Pubkey::new_unique())].into(); + assert!(delegation_actions_require_risk_check( + AmlCheckStrategy::AllSigners, + &actions, + )); + } + + #[test] + fn relevant_programs_strategy_skips_unrelated_actions() { + let actions: DelegationActions = vec![ + action_for_program(Pubkey::new_unique()), + action_for_program(Pubkey::new_unique()), + ] + .into(); + assert!(!delegation_actions_require_risk_check( + AmlCheckStrategy::RelevantPrograms, + &actions, + )); + } + + #[test] + fn relevant_programs_strategy_matches_each_relevant_program() { + // Spelled out rather than iterating RISK_RELEVANT_PROGRAMS, so that + // dropping a program from that list fails this test. + for program in [ + TOKEN_PROGRAM_ID, + TOKEN_2022_PROGRAM_ID, + EATA_PROGRAM_ID, + magicblock_magic_program_api::ID, + system_program::ID, + magicblock_magic_program_api::EPHEMERAL_SYSTEM_PROGRAM_ID, + ] { + let actions: DelegationActions = + vec![action_for_program(program)].into(); + assert!( + delegation_actions_require_risk_check( + AmlCheckStrategy::RelevantPrograms, + &actions, + ), + "program {program} invoked as program_id should require check", + ); + + // Referenced as a CPI target account, not the invoked program. + let actions: DelegationActions = + vec![action_referencing(Pubkey::new_unique(), program)].into(); + assert!( + delegation_actions_require_risk_check( + AmlCheckStrategy::RelevantPrograms, + &actions, + ), + "program {program} referenced as account should require check", + ); + } + } + + #[test] + fn default_strategy_checks_all_signers() { + // The narrower strategy must be opted into: defaulting to it would + // silently drop coverage for deployments that only set `enabled`. + assert_eq!(AmlCheckStrategy::default(), AmlCheckStrategy::AllSigners); + } + + #[test] + fn relevant_programs_strategy_matches_native_sol_transfers() { + let actions: DelegationActions = + vec![solana_system_interface::instruction::transfer( + &Pubkey::new_unique(), + &Pubkey::new_unique(), + 1_000, + )] + .into(); + assert!(delegation_actions_require_risk_check( + AmlCheckStrategy::RelevantPrograms, + &actions, + )); + } + + #[test] + fn relevant_programs_strategy_matches_when_any_action_is_relevant() { + let actions: DelegationActions = vec![ + action_for_program(Pubkey::new_unique()), + action_for_program(EATA_PROGRAM_ID), + ] + .into(); + assert!(delegation_actions_require_risk_check( + AmlCheckStrategy::RelevantPrograms, + &actions, + )); + } +} diff --git a/magicblock-chainlink/src/chainlink/mod.rs b/magicblock-chainlink/src/chainlink/mod.rs index efc54830a..e2dacd13d 100644 --- a/magicblock-chainlink/src/chainlink/mod.rs +++ b/magicblock-chainlink/src/chainlink/mod.rs @@ -220,6 +220,16 @@ impl InnerChainlink { let risk_service = RiskService::try_from_config(&chainlink_config.risk)? .map(Arc::new); + match risk_service.as_ref() { + // Which policy is live decides whether an action can activate + // unchecked, so make it visible at startup. + Some(service) => info!( + risk_server_url = %chainlink_config.risk.risk_server_url, + check_strategy = ?service.check_strategy(), + "Address risk checks enabled" + ), + None => info!("Address risk checks disabled"), + } let fetch_cloner = FetchCloner::new_with_undelegation_request_sender( &provider, diff --git a/magicblock-chainlink/tests/10_aml_undelegation.rs b/magicblock-chainlink/tests/10_aml_undelegation.rs index 55bed1e8e..1c0106aef 100644 --- a/magicblock-chainlink/tests/10_aml_undelegation.rs +++ b/magicblock-chainlink/tests/10_aml_undelegation.rs @@ -16,7 +16,7 @@ use dlp_api::{ }; use magicblock_aml::RiskService; use magicblock_chainlink::testing::{context::TestContext, init_logger}; -use magicblock_config::config::RiskConfig; +use magicblock_config::config::{AmlCheckStrategy, RiskConfig}; use solana_account::{Account, AccountMode}; use solana_pubkey::Pubkey; use tokio::task::JoinHandle; @@ -107,6 +107,7 @@ fn risk_config(risk_server_url: String) -> RiskConfig { enabled: true, risk_server_url, request_timeout: Duration::from_secs(2), + check_strategy: AmlCheckStrategy::AllSigners, } } diff --git a/magicblock-config/src/config/chain.rs b/magicblock-config/src/config/chain.rs index 14856d87b..c61336dff 100644 --- a/magicblock-config/src/config/chain.rs +++ b/magicblock-config/src/config/chain.rs @@ -78,6 +78,25 @@ impl Default for ChainLinkConfig { } } +/// Strategy for deciding which post-delegation action signers get AML/risk +/// checked. +#[derive( + Deserialize, Serialize, Debug, Clone, Copy, PartialEq, Eq, Default, +)] +#[serde(rename_all = "kebab-case")] +pub enum AmlCheckStrategy { + /// Check every signer of every post-delegation action, regardless of which + /// programs the action invokes. The default, since it is the only strategy + /// that cannot silently let an action through unchecked. + #[default] + AllSigners, + /// Only check signers when a post-delegation action involves a value + /// transferring program: SPL Token (legacy and 2022), ephemeral SPL + /// (eATA/ESPL), Magic, or the (ephemeral) system program. Actions touching + /// none of these programs skip the risk check entirely. + RelevantPrograms, +} + /// Configuration for checking address risk against the risk server. The risk /// server owns the upstream provider credentials, caching, and threshold; the /// validator is a thin client. @@ -91,6 +110,8 @@ pub struct RiskConfig { /// Request timeout for risk server calls. #[serde(with = "humantime")] pub request_timeout: Duration, + /// Which post-delegation action signers to risk check. + pub check_strategy: AmlCheckStrategy, } impl Default for RiskConfig { @@ -101,6 +122,7 @@ impl Default for RiskConfig { request_timeout: Duration::from_secs( consts::DEFAULT_RISK_REQUEST_TIMEOUT_SEC, ), + check_strategy: AmlCheckStrategy::default(), } } } diff --git a/magicblock-config/src/config/mod.rs b/magicblock-config/src/config/mod.rs index 4923b27f7..22ee1e68f 100644 --- a/magicblock-config/src/config/mod.rs +++ b/magicblock-config/src/config/mod.rs @@ -11,7 +11,8 @@ pub mod scheduler; pub use aperture::ApertureConfig; pub use chain::{ - AdminConfig, AllowedProgram, ChainLinkConfig, CommittorConfig, RiskConfig, + AdminConfig, AllowedProgram, AmlCheckStrategy, ChainLinkConfig, + CommittorConfig, RiskConfig, }; pub use engine::{EngineConfig, FollowerReplication, LeaderReplication}; pub use grpc::GrpcConfig; diff --git a/test-integration/test-aml/src/lib.rs b/test-integration/test-aml/src/lib.rs index c6ad959b2..4651e6cf7 100644 --- a/test-integration/test-aml/src/lib.rs +++ b/test-integration/test-aml/src/lib.rs @@ -23,7 +23,10 @@ use integration_test_tools::{ IntegrationTestContext, }; use magicblock_config::{ - config::{ChainLinkConfig, LifecycleMode, LoadableProgram, RiskConfig}, + config::{ + AmlCheckStrategy, ChainLinkConfig, LifecycleMode, LoadableProgram, + RiskConfig, + }, types::Remote, LeaderParams, }; @@ -187,6 +190,10 @@ pub fn setup_validator_with_local_remote( risk: RiskConfig { enabled: true, risk_server_url, + // Pinned rather than defaulted: the suite asserts that risky + // signers are blocked, which is only meaningful if we know + // which strategy decided to check them. + check_strategy: AmlCheckStrategy::AllSigners, ..Default::default() }, ..Default::default()