diff --git a/crates/core/src/error.rs b/crates/core/src/error.rs index 6a8b9ae0..8679ce7c 100644 --- a/crates/core/src/error.rs +++ b/crates/core/src/error.rs @@ -10,6 +10,7 @@ use solana_clock::Slot; use solana_pubkey::Pubkey; use solana_transaction::TransactionError; use solana_transaction_status::EncodeError; +use surfpool_types::ScenarioError; use crate::storage::StorageError; @@ -470,6 +471,21 @@ impl SurfpoolError { error.message = format!("Expected profile not found for key {key}"); Self(error) } + + pub fn invalid_scenario(scenario_name: &str, failures: &[(String, ScenarioError)]) -> Self { + let mut error = Error::invalid_request(); + let failure_messages = failures + .iter() + .map(|(id, err)| format!("override '{}': {}", id, err)) + .collect::>() + .join("; "); + let message = format!( + "Cannot register scenario '{}': {}", + scenario_name, failure_messages + ); + error.message = message; + Self(error) + } } impl From for SurfpoolError { @@ -496,6 +512,14 @@ impl From for SurfpoolError { } } +impl From for SurfpoolError { + fn from(e: ScenarioError) -> Self { + let mut error = Error::invalid_request(); + error.data = Some(json!(format!("Scenario error: {}", e))); + Self(error) + } +} + /// Error returned by [`crate::surfnet::svm::SurfnetSvm::airdrop`] when the /// requested airdrop is rejected up front, before any synthetic transaction /// is constructed or any account state is touched. diff --git a/crates/core/src/rpc/surfnet_cheatcodes.rs b/crates/core/src/rpc/surfnet_cheatcodes.rs index 154057c5..16a56f90 100644 --- a/crates/core/src/rpc/surfnet_cheatcodes.rs +++ b/crates/core/src/rpc/surfnet_cheatcodes.rs @@ -1286,6 +1286,13 @@ pub trait SurfnetCheatcodes { /// ## Returns /// A `RpcResponse<()>` indicating whether the Scenario registration was successful. /// + /// Registration validates every enabled override before scheduling anything: each + /// override's account address must resolve with the provided values. If any address + /// cannot resolve (a malformed pubkey, a PDA seed referencing a missing property, a + /// value of the wrong type), the request is rejected with an error message listing + /// each failing override id and its cause, and no overrides are scheduled. Disabled + /// overrides are not validated, since they are never materialized. + /// /// ## Example Request (with slot) /// ```json /// { diff --git a/crates/core/src/scenarios/README.md b/crates/core/src/scenarios/README.md index 4368f2b8..4f294b49 100644 --- a/crates/core/src/scenarios/README.md +++ b/crates/core/src/scenarios/README.md @@ -25,6 +25,7 @@ For custom protocols, an IDL can be registered at runtime using the [`surfnet_re Scenarios can be registered at runtime using the [`surfnet_registerScenario`](https://docs.surfpool.run/rpc/cheatcodes#surfnet-registerscenario) RPC cheatcode. This cheatcode takes in a scenario definition in JSON format, which includes the scenario name, description, and a list of overrides to apply to accounts. Each override contains a map of the field in the account to override (as indexed in the IDL), and the value to apply for that key. +Registration validates every enabled override up front: each override's account address must resolve with the provided values, and a scenario containing an unresolvable address (a malformed pubkey, a PDA seed referencing a missing property, a value of the wrong type) is rejected with an error listing each failing override and its cause. ### Override Templates Directly using the `surfnet_registerScenario` endpoint requires building out a map of account keys that are specific to the schema of the account that is being written to. diff --git a/crates/core/src/scenarios/registry.rs b/crates/core/src/scenarios/registry.rs index 9d69b0ee..3620abd6 100644 --- a/crates/core/src/scenarios/registry.rs +++ b/crates/core/src/scenarios/registry.rs @@ -129,7 +129,10 @@ impl TemplateRegistry { }; // Convert all templates in the collection - let templates = collection.to_override_templates(idl); + let templates = match collection.to_override_templates(idl) { + Ok(t) => t, + Err(e) => panic!("unable to convert {} templates: {}", protocol_name, e), + }; // Register each template for template in templates { @@ -223,7 +226,7 @@ mod tests { )]); let bytes = derived_pda_seed .to_bytes(Some(&values)) - .unwrap_or_else(|| panic!("option {} did not resolve", option.id)); + .unwrap_or_else(|e| panic!("option {} did not resolve: {e}", option.id)); assert_eq!( Pubkey::try_from(bytes.as_slice()).expect("32 bytes"), @@ -313,14 +316,13 @@ mod tests { ]); assert!( - template.address.resolve(Some(&values)).is_some(), + template.address.resolve(Some(&values)).is_ok(), "every seed resolves, so the pool address does too" ); values.remove("config_index"); - assert_eq!( - template.address.resolve(Some(&values)), - None, + assert!( + template.address.resolve(Some(&values)).is_err(), "a seed that cannot resolve must not derive a shorter address" ); } diff --git a/crates/core/src/surfnet/svm.rs b/crates/core/src/surfnet/svm.rs index b48d8a68..16b2d78f 100644 --- a/crates/core/src/surfnet/svm.rs +++ b/crates/core/src/surfnet/svm.rs @@ -55,7 +55,7 @@ use spl_token_2022_interface::extension::{ }; use surfpool_types::{ AccountChange, AccountProfileState, AccountSnapshot, DEFAULT_PROFILING_MAP_CAPACITY, - DEFAULT_SLOT_TIME_MS, ExportSnapshotConfig, ExportSnapshotScope, FifoMap, Idl, + DEFAULT_SLOT_TIME_MS, ExportSnapshotConfig, ExportSnapshotScope, FifoMap, Idl, OverrideError, OverrideInstance, ProfileResult, RpcProfileDepth, RpcProfileResultConfig, RunbookExecutionStatusReport, SimnetEvent, SimnetEventsTx, StartupError, SurfnetStartupStatus, SurfnetStartupTask, SvmFeatureConfig, TransactionConfirmationStatus, TransactionStatusEvent, @@ -2551,6 +2551,189 @@ impl SurfnetSvm { Ok(()) } + /// Apply a single scheduled override. Returns Ok(()) when the override was + /// applied or was a no-op by design (no values beyond PDA seed references). + pub async fn apply_override( + &mut self, + remote_ctx: &Option<(SurfnetRemoteClient, CommitmentConfig)>, + override_instance: &OverrideInstance, + ) -> Result<(), OverrideError> { + let account_pubkey = override_instance + .account + .resolve(Some(&override_instance.values))?; + + debug!( + "Processing override {} for account {} (label: {:?})", + override_instance.id, account_pubkey, override_instance.label + ); + + if override_instance.fetch_before_use { + if let Some((client, _)) = remote_ctx { + debug!( + "Fetching fresh account data for {} from remote", + account_pubkey + ); + + match client + .get_account(&account_pubkey, CommitmentConfig::confirmed()) + .await + { + Ok(GetAccountResult::FoundAccount(_pubkey, remote_account, _)) => { + debug!( + "Fetched account {} from remote: {} lamports, {} bytes", + account_pubkey, + remote_account.lamports(), + remote_account.data().len() + ); + + if let Err(e) = self.inner.set_account(account_pubkey, remote_account) { + warn!( + "Failed to set account {} from remote: {}", + account_pubkey, e + ); + } + } + Ok(GetAccountResult::None(_)) => { + debug!("Account {} not found on remote", account_pubkey); + } + Ok(_) => { + debug!("Account {} fetched (other variant)", account_pubkey); + } + Err(e) => { + warn!( + "Failed to fetch account {} from remote: {}", + account_pubkey, e + ); + } + } + } else { + debug!( + "fetch_before_use enabled but no remote client available for override {}", + override_instance.id + ); + } + } + + if !override_instance.values.is_empty() { + let pda_refs = override_instance.account.get_pda_seed_references(); + let account_values: HashMap = override_instance + .values + .iter() + .filter(|(key, _)| !pda_refs.contains(key)) + .map(|(k, v)| (k.clone(), v.clone())) + .collect(); + + if account_values.is_empty() { + debug!( + "Override {} has no account data modifications (all values are PDA seeds)", + override_instance.id + ); + return Ok(()); + } + + debug!( + "Override {} applying {} field modification(s) to account {} (filtered {} PDA seed refs)", + override_instance.id, + account_values.len(), + account_pubkey, + pda_refs.len() + ); + + let Some(account) = + self.inner + .get_account(&account_pubkey) + .map_err(|e| OverrideError::Forge { + account: account_pubkey.to_string(), + message: format!("failed to read account from SVM: {}", e), + })? + else { + return Err(OverrideError::AccountNotFound { + account: account_pubkey.to_string(), + }); + }; + + // The 8-byte discriminator requirement is independent of any IDL, so + // check it before the IDL lookup to report the more precise cause. + if account.data().len() < 8 { + return Err(OverrideError::AccountTooSmall { + account: account_pubkey.to_string(), + len: account.data().len(), + }); + } + + let owner_program_id = account.owner(); + + let idl_versions = match self.registered_idls.get(&owner_program_id.to_string()) { + Ok(Some(versions)) => versions, + Ok(None) => { + return Err(OverrideError::NoIdlForOwner { + program_id: owner_program_id.to_string(), + account: account_pubkey.to_string(), + }); + } + Err(e) => { + return Err(OverrideError::Forge { + account: account_pubkey.to_string(), + message: format!("IDL lookup failed: {}", e), + }); + } + }; + + let Some(versioned_idl) = idl_versions.first() else { + return Err(OverrideError::NoIdlForOwner { + program_id: owner_program_id.to_string(), + account: account_pubkey.to_string(), + }); + }; + + let idl = &versioned_idl.1; + + let account_data = account.data(); + + let new_account_data = match self.get_forged_account_data( + &account_pubkey, + account_data, + idl, + &account_values, + ) { + Ok(data) => data, + Err(e) => { + return Err(OverrideError::Forge { + account: account_pubkey.to_string(), + message: e.to_string(), + }); + } + }; + + let modified_account = Account { + lamports: account.lamports(), + data: new_account_data, + owner: *account.owner(), + executable: account.executable(), + rent_epoch: account.rent_epoch(), + }; + + match self.inner.set_account(account_pubkey, modified_account) { + Ok(_) => { + debug!( + "Successfully applied {} override(s) to account {} (override {})", + override_instance.values.len(), + account_pubkey, + override_instance.id + ); + } + Err(e) => { + return Err(OverrideError::Forge { + account: account_pubkey.to_string(), + message: format!("failed to write account: {}", e), + }); + } + } + } + + Ok(()) + } + /// Materializes scheduled overrides for the current slot /// /// This function: @@ -2593,211 +2776,8 @@ impl SurfnetSvm { continue; } - // Resolve account address using the centralized method - let account_pubkey = match override_instance - .account - .resolve(Some(&override_instance.values)) - { - Some(pubkey) => { - if matches!( - &override_instance.account, - surfpool_types::AccountAddress::Pda { .. } - ) { - debug!( - "Derived PDA {} for override {}", - pubkey, override_instance.id - ); - } - pubkey - } - None => { - warn!( - "Failed to resolve account address for override {}", - override_instance.id - ); - continue; - } - }; - - debug!( - "Processing override {} for account {} (label: {:?})", - override_instance.id, account_pubkey, override_instance.label - ); - - // Fetch fresh account data from remote if requested - if override_instance.fetch_before_use { - if let Some((client, _)) = remote_ctx { - debug!( - "Fetching fresh account data for {} from remote", - account_pubkey - ); - - match client - .get_account(&account_pubkey, CommitmentConfig::confirmed()) - .await - { - Ok(GetAccountResult::FoundAccount(_pubkey, remote_account, _)) => { - debug!( - "Fetched account {} from remote: {} lamports, {} bytes", - account_pubkey, - remote_account.lamports(), - remote_account.data().len() - ); - - // Set the fresh account data in the SVM - if let Err(e) = self.inner.set_account(account_pubkey, remote_account) { - warn!( - "Failed to set account {} from remote: {}", - account_pubkey, e - ); - } - } - Ok(GetAccountResult::None(_)) => { - debug!("Account {} not found on remote", account_pubkey); - } - Ok(_) => { - debug!("Account {} fetched (other variant)", account_pubkey); - } - Err(e) => { - warn!( - "Failed to fetch account {} from remote: {}", - account_pubkey, e - ); - } - } - } else { - debug!( - "fetch_before_use enabled but no remote client available for override {}", - override_instance.id - ); - } - } - - // Apply the override values to the account data - if !override_instance.values.is_empty() { - // Filter out values that are only used for PDA derivation (not account data) - let pda_refs = override_instance.account.get_pda_seed_references(); - let account_values: HashMap = override_instance - .values - .iter() - .filter(|(key, _)| !pda_refs.contains(key)) - .map(|(k, v)| (k.clone(), v.clone())) - .collect(); - - if account_values.is_empty() { - debug!( - "Override {} has no account data modifications (all values are PDA seeds)", - override_instance.id - ); - continue; - } - - debug!( - "Override {} applying {} field modification(s) to account {} (filtered {} PDA seed refs)", - override_instance.id, - account_values.len(), - account_pubkey, - pda_refs.len() - ); - - // Get the account from the SVM - let Some(account) = self.inner.get_account(&account_pubkey)? else { - warn!( - "Account {} not found in SVM for override {}, skipping modifications", - account_pubkey, override_instance.id - ); - continue; - }; - - // Get the account owner (program ID) - let owner_program_id = account.owner(); - - // Look up the IDL for the owner program - let idl_versions = match self.registered_idls.get(&owner_program_id.to_string()) { - Ok(Some(versions)) => versions, - Ok(None) => { - warn!( - "No IDL registered for program {} (owner of account {}), skipping override {}", - owner_program_id, account_pubkey, override_instance.id - ); - continue; - } - Err(e) => { - warn!( - "Failed to get IDL for program {}: {}, skipping override {}", - owner_program_id, e, override_instance.id - ); - continue; - } - }; - - // Get the latest IDL version (first in the sorted Vec) - let Some(versioned_idl) = idl_versions.first() else { - warn!( - "IDL versions empty for program {}, skipping override {}", - owner_program_id, override_instance.id - ); - continue; - }; - - let idl = &versioned_idl.1; - - // Get account data - let account_data = account.data(); - - // Check if account data is valid (has at least discriminator) - if account_data.len() < 8 { - warn!( - "Account {} has insufficient data ({} bytes) for override {}. \ - Enable fetchBeforeUse: true to fetch account data from mainnet first.", - account_pubkey, - account_data.len(), - override_instance.id - ); - continue; - } - - // Use get_forged_account_data to apply the overrides (with PDA refs filtered out) - let new_account_data = match self.get_forged_account_data( - &account_pubkey, - account_data, - idl, - &account_values, - ) { - Ok(data) => data, - Err(e) => { - warn!( - "Failed to forge account data for {} (override {}): {}. \ - If the account doesn't exist locally, enable fetchBeforeUse: true.", - account_pubkey, override_instance.id, e - ); - continue; - } - }; - - // Create a new account with modified data - let modified_account = Account { - lamports: account.lamports(), - data: new_account_data, - owner: *account.owner(), - executable: account.executable(), - rent_epoch: account.rent_epoch(), - }; - - // Update the account in the SVM - if let Err(e) = self.inner.set_account(account_pubkey, modified_account) { - warn!( - "Failed to set modified account {} in SVM: {}", - account_pubkey, e - ); - } else { - debug!( - "Successfully applied {} override(s) to account {} (override {})", - override_instance.values.len(), - account_pubkey, - override_instance.id - ); - } + if let Err(e) = self.apply_override(remote_ctx, &override_instance).await { + warn!("Skipping override {}: {}", override_instance.id, e); } } @@ -4046,6 +4026,24 @@ impl SurfnetSvm { base_slot ); + // Validate enabled overrides before scheduling + let mut failures = Vec::new(); + for override_instance in &scenario.overrides { + if !override_instance.enabled { + continue; + } + if let Err(e) = override_instance + .account + .resolve(Some(&override_instance.values)) + { + failures.push((override_instance.id.clone(), e)); + } + } + + if !failures.is_empty() { + return Err(SurfpoolError::invalid_scenario(&scenario.name, &failures)); + } + // Schedule overrides by adding base slot to their scenario-relative slots for override_instance in scenario.overrides { let scenario_relative_slot = override_instance.scenario_relative_slot; @@ -6002,7 +6000,7 @@ mod tests { }; let result = address.resolve_simple(); - assert!(result.is_some(), "Should derive PDA with string seed"); + assert!(result.is_ok(), "Should derive PDA with string seed"); // Verify it matches direct derivation let program_pubkey = Pubkey::from_str(program_id).unwrap(); @@ -6022,7 +6020,7 @@ mod tests { }; let result = address.resolve_simple(); - assert!(result.is_some(), "Should derive PDA with pubkey seed"); + assert!(result.is_ok(), "Should derive PDA with pubkey seed"); // Verify it matches direct derivation let program_pubkey = Pubkey::from_str(program_id).unwrap(); @@ -6048,7 +6046,7 @@ mod tests { }; let result = address.resolve_simple(); - assert!(result.is_some(), "Should derive PDA with multiple seeds"); + assert!(result.is_ok(), "Should derive PDA with multiple seeds"); // Verify it matches direct derivation let program_pubkey = Pubkey::from_str(program_id).unwrap(); @@ -6073,7 +6071,7 @@ mod tests { }; let result = address.resolve_simple(); - assert!(result.is_some(), "Should derive PDA with bytes seed"); + assert!(result.is_ok(), "Should derive PDA with bytes seed"); // Verify it matches direct derivation let program_pubkey = Pubkey::from_str(program_id).unwrap(); @@ -6101,10 +6099,7 @@ mod tests { ); let result = address.resolve(Some(&values)); - assert!( - result.is_some(), - "Should derive PDA with property ref pubkey" - ); + assert!(result.is_ok(), "Should derive PDA with property ref pubkey"); // Verify it matches direct derivation let program_pubkey = Pubkey::from_str(program_id).unwrap(); @@ -6133,7 +6128,7 @@ mod tests { ); let result = address.resolve(Some(&values)); - assert!(result.is_some(), "Should derive PDA with property ref u64"); + assert!(result.is_ok(), "Should derive PDA with property ref u64"); // Verify it matches direct derivation let program_pubkey = Pubkey::from_str(program_id).unwrap(); @@ -6151,7 +6146,7 @@ mod tests { }; let result = address.resolve_simple(); - assert!(result.is_none(), "Should fail with invalid program ID"); + assert!(result.is_err(), "Should fail with invalid program ID"); } #[test] @@ -6166,7 +6161,7 @@ mod tests { }; let result = address.resolve_simple(); - assert!(result.is_none(), "Should fail with invalid pubkey seed"); + assert!(result.is_err(), "Should fail with invalid pubkey seed"); } #[test] @@ -6181,7 +6176,7 @@ mod tests { }; let result = address.resolve_simple(); // No values provided - assert!(result.is_none(), "Should fail with missing property ref"); + assert!(result.is_err(), "Should fail with missing property ref"); } #[test] @@ -6191,7 +6186,7 @@ mod tests { let address = surfpool_types::AccountAddress::Pubkey(pubkey_str.to_string()); let result = address.resolve_simple(); - assert!(result.is_some(), "Should resolve pubkey address"); + assert!(result.is_ok(), "Should resolve pubkey address"); assert_eq!(result.unwrap(), Pubkey::from_str(pubkey_str).unwrap()); } @@ -6217,7 +6212,7 @@ mod tests { let result2 = address.resolve_simple(); let result3 = address.resolve_simple(); - assert!(result1.is_some()); + assert!(result1.is_ok()); assert_eq!(result1, result2, "PDA derivation should be deterministic"); assert_eq!(result2, result3, "PDA derivation should be deterministic"); } @@ -6257,7 +6252,7 @@ mod tests { }; let result = address.resolve_simple(); - assert!(result.is_some(), "Should derive Raydium CLMM pool PDA"); + assert!(result.is_ok(), "Should derive Raydium CLMM pool PDA"); // Verify it matches the known pool address let expected_pool = @@ -6301,10 +6296,7 @@ mod tests { ); let result = address.resolve(Some(&values)); - assert!( - result.is_some(), - "Should derive pool PDA with property refs" - ); + assert!(result.is_ok(), "Should derive pool PDA with property refs"); let expected_pool = Pubkey::from_str("3ucNos4NbumPLZNWztqGHNFFgkHeRMBQAVemeeomsUxv").unwrap(); @@ -6350,7 +6342,7 @@ mod tests { let result = address.resolve_simple(); assert!( - result.is_some(), + result.is_ok(), "Should derive pool for {} fee tier", tier_name ); @@ -6473,7 +6465,7 @@ mod tests { let result = address.resolve_simple(); assert!( - result.is_some(), + result.is_ok(), "Should derive AMM config for index {}", index ); @@ -6519,7 +6511,7 @@ mod tests { }; let result = address.resolve_simple(); - assert!(result.is_some(), "Should derive pool with nested PDA"); + assert!(result.is_ok(), "Should derive pool with nested PDA"); // Should match the known SOL/USDC pool let expected_pool = @@ -6569,7 +6561,7 @@ mod tests { ); let result = address.resolve(Some(&values)); - assert!(result.is_some(), "Should derive pool with property refs"); + assert!(result.is_ok(), "Should derive pool with property refs"); let expected_pool = Pubkey::from_str("3ucNos4NbumPLZNWztqGHNFFgkHeRMBQAVemeeomsUxv").unwrap(); @@ -6580,6 +6572,204 @@ mod tests { ); } + /// An enabled override targeting the given address, with a single value + /// under the given field name. + fn override_with_value( + address: surfpool_types::AccountAddress, + field: &str, + ) -> surfpool_types::OverrideInstance { + let mut instance = + surfpool_types::OverrideInstance::new("test-template".to_string(), 0, address); + instance.values = HashMap::from([( + field.to_string(), + serde_json::Value::String("test_value".to_string()), + )]); + instance.enabled = true; + instance + } + + /// A rent-holding account with zeroed data of the given length. + fn seed_account(svm: &mut SurfnetSvm, pubkey: &Pubkey, data_len: usize, owner: Pubkey) { + let account = Account { + lamports: 1_000_000, + data: vec![0u8; data_len], + owner, + executable: false, + rent_epoch: 0, + }; + svm.set_account(pubkey, account).unwrap(); + } + + #[tokio::test] + async fn apply_override_reports_missing_account() { + let (mut svm, _events_rx, _geyser_rx) = + SurfnetSvm::new(SurfnetSvmConfig::default()).unwrap(); + let account_pubkey = Pubkey::new_unique(); + let override_instance = override_with_value( + surfpool_types::AccountAddress::Pubkey(account_pubkey.to_string()), + "dummy_field", + ); + + let result = svm.apply_override(&None, &override_instance).await; + + assert_eq!( + result, + Err(surfpool_types::OverrideError::AccountNotFound { + account: account_pubkey.to_string() + }) + ); + } + + #[tokio::test] + async fn apply_override_reports_account_too_small() { + let (mut svm, _events_rx, _geyser_rx) = + SurfnetSvm::new(SurfnetSvmConfig::default()).unwrap(); + let account_pubkey = Pubkey::new_unique(); + seed_account(&mut svm, &account_pubkey, 4, Pubkey::new_unique()); + let override_instance = override_with_value( + surfpool_types::AccountAddress::Pubkey(account_pubkey.to_string()), + "dummy_field", + ); + + let result = svm.apply_override(&None, &override_instance).await; + + assert_eq!( + result, + Err(surfpool_types::OverrideError::AccountTooSmall { + account: account_pubkey.to_string(), + len: 4 + }) + ); + } + + #[tokio::test] + async fn apply_override_reports_missing_idl() { + let (mut svm, _events_rx, _geyser_rx) = + SurfnetSvm::new(SurfnetSvmConfig::default()).unwrap(); + let account_pubkey = Pubkey::new_unique(); + let owner_program_id = Pubkey::new_unique(); + seed_account(&mut svm, &account_pubkey, 16, owner_program_id); + let override_instance = override_with_value( + surfpool_types::AccountAddress::Pubkey(account_pubkey.to_string()), + "dummy_field", + ); + + let result = svm.apply_override(&None, &override_instance).await; + + assert_eq!( + result, + Err(surfpool_types::OverrideError::NoIdlForOwner { + program_id: owner_program_id.to_string(), + account: account_pubkey.to_string() + }) + ); + } + + #[tokio::test] + async fn apply_override_resolve_failure_is_typed() { + let (mut svm, _events_rx, _geyser_rx) = + SurfnetSvm::new(SurfnetSvmConfig::default()).unwrap(); + let override_instance = surfpool_types::OverrideInstance::new( + "test-template".to_string(), + 0, + surfpool_types::AccountAddress::Pubkey("garbage".to_string()), + ); + + let result = svm.apply_override(&None, &override_instance).await; + + assert!( + matches!(result, Err(surfpool_types::OverrideError::Resolve(_))), + "Expected OverrideError::Resolve, got {:?}", + result + ); + } + + #[tokio::test] + async fn apply_override_pda_seed_only_values_is_ok() { + let (mut svm, _events_rx, _geyser_rx) = + SurfnetSvm::new(SurfnetSvmConfig::default()).unwrap(); + let pda_account = surfpool_types::AccountAddress::Pda { + program_id: Pubkey::new_unique().to_string(), + seeds: vec![surfpool_types::PdaSeed::PropertyRef( + "seed_value".to_string(), + )], + }; + let override_instance = override_with_value(pda_account, "seed_value"); + + let result = svm.apply_override(&None, &override_instance).await; + + assert!( + result.is_ok(), + "PDA with seed-only values should be ok (no-op): {:?}", + result + ); + } + + #[test] + fn register_scenario_rejects_unresolvable_override() { + // Test that register_scenario rejects scenarios with unresolvable override addresses + let (mut svm, _events_rx, _geyser_rx) = + SurfnetSvm::new(SurfnetSvmConfig::default()).unwrap(); + + let mut scenario = surfpool_types::Scenario::new( + "test_unresolvable".to_string(), + "Test scenario with invalid pubkey".to_string(), + ); + + let override_instance = surfpool_types::OverrideInstance::new( + "test-template".to_string(), + 0, + surfpool_types::AccountAddress::Pubkey("not-a-pubkey".to_string()), + ); + + scenario.add_override(override_instance.clone()); + + let result = svm.register_scenario(scenario.clone(), Some(0)); + + assert!( + result.is_err(), + "Should reject scenario with invalid pubkey address" + ); + let error_msg = format!("{}", result.unwrap_err()); + assert!( + error_msg.contains(&override_instance.id), + "Error message should contain the override ID" + ); + assert!( + error_msg.contains("not a valid pubkey") || error_msg.contains("invalid"), + "Error message should indicate invalid pubkey" + ); + } + + #[test] + fn register_scenario_accepts_resolvable_override() { + // Test that register_scenario accepts scenarios with resolvable override addresses + let (mut svm, _events_rx, _geyser_rx) = + SurfnetSvm::new(SurfnetSvmConfig::default()).unwrap(); + + let mut scenario = surfpool_types::Scenario::new( + "test_resolvable".to_string(), + "Test scenario with valid pubkey".to_string(), + ); + + // Use a valid base58 pubkey + let valid_pubkey = solana_pubkey::Pubkey::new_unique().to_string(); + let override_instance = surfpool_types::OverrideInstance::new( + "test-template".to_string(), + 0, + surfpool_types::AccountAddress::Pubkey(valid_pubkey.clone()), + ); + + scenario.add_override(override_instance); + + let result = svm.register_scenario(scenario, Some(0)); + + assert!( + result.is_ok(), + "Should accept scenario with valid pubkey address" + ); + } + #[test] fn test_snapshot_export_restore_round_trip() { use std::{collections::HashMap, io::Write}; diff --git a/crates/mcp/src/surfpool/mod.rs b/crates/mcp/src/surfpool/mod.rs index b7568cc9..b6c41198 100644 --- a/crates/mcp/src/surfpool/mod.rs +++ b/crates/mcp/src/surfpool/mod.rs @@ -613,6 +613,7 @@ impl Surfpool { 2. `values` keys MUST be from the template's `properties` array 3. For PDA addresses, DO NOT provide `account` - it will be generated from template + values 4. For constant_ref properties (like feed_id), the value MUST come from search_constant_options results + 5. Every enabled override's account address is validated at registration: if it cannot be resolved from the values (bad pubkey, missing property, wrong type), the whole scenario is rejected with a message naming each failing override and its cause - fix the values and retry CORRECT JSON STRUCTURE FOR PYTH PRICE FEED: { diff --git a/crates/types/src/scenarios.rs b/crates/types/src/scenarios.rs index fb385957..20dc4155 100644 --- a/crates/types/src/scenarios.rs +++ b/crates/types/src/scenarios.rs @@ -60,6 +60,70 @@ impl ConstantDefinition { // Core Scenarios Types // ======================================== +/// Why a single PDA seed could not be converted to bytes. +#[derive(Debug, Clone, PartialEq, thiserror::Error)] +pub enum SeedError { + #[error("'{0}' is not a valid pubkey")] + InvalidPubkey(String), + #[error("seed references property '{0}' but no values were provided")] + NoValues(String), + #[error("property '{0}' not found in values")] + UnknownProperty(String), + #[error("property '{name}' is {found}, expected {expected}")] + WrongType { + name: String, + expected: &'static str, + found: &'static str, + }, + #[error("property '{name}' value {value} does not fit in u16")] + U16OutOfRange { name: String, value: u64 }, + #[error("'{0}' is not a 32-byte hex string")] + InvalidBytes32(String), + #[error("seed {index} of derived PDA for program {program_id}: {source}")] + DerivedSeed { + program_id: String, + index: usize, + source: Box, + }, +} + +/// Why an account address could not be resolved to a pubkey. +#[derive(Debug, Clone, PartialEq, thiserror::Error)] +pub enum ScenarioError { + #[error("invalid account address: {0}")] + InvalidAddress(#[from] SeedError), + #[error("invalid program id '{0}'")] + InvalidProgramId(String), + #[error("PDA for program {program_id}, seed {index}: {source}")] + Seed { + program_id: String, + index: usize, + source: SeedError, + }, + #[error("template '{template_id}': {source}")] + Template { + template_id: String, + source: Box, + }, +} + +/// Why a scheduled override could not be applied at its slot. +#[derive(Debug, Clone, PartialEq, thiserror::Error)] +pub enum OverrideError { + #[error("{0}")] + Resolve(#[from] ScenarioError), + #[error( + "account {account} does not exist locally; overrides patch existing accounts (enable fetchBeforeUse or create the account first)" + )] + AccountNotFound { account: String }, + #[error("account {account} has {len} byte(s), too small for an 8-byte discriminator")] + AccountTooSmall { account: String, len: usize }, + #[error("no IDL registered for program {program_id} (owner of account {account})")] + NoIdlForOwner { program_id: String, account: String }, + #[error("failed to forge account data for {account}: {message}")] + Forge { account: String, message: String }, +} + /// Defines how an account address should be determined #[derive(Clone, Debug, PartialEq, Serialize, Deserialize, schemars::JsonSchema)] #[serde(rename_all = "camelCase")] @@ -105,77 +169,160 @@ pub enum PdaSeed { }, } +/// Look up a property referenced by a seed, distinguishing a missing values +/// map from a missing key. +fn property_value<'a>( + values: Option<&'a HashMap>, + prop: &str, +) -> Result<&'a serde_json::Value, SeedError> { + values + .ok_or_else(|| SeedError::NoValues(prop.to_string()))? + .get(prop) + .ok_or_else(|| SeedError::UnknownProperty(prop.to_string())) +} + +fn json_type_name(v: &serde_json::Value) -> &'static str { + match v { + serde_json::Value::Null => "null", + serde_json::Value::Bool(_) => "bool", + serde_json::Value::Number(_) => "number", + serde_json::Value::String(_) => "string", + serde_json::Value::Array(_) => "array", + serde_json::Value::Object(_) => "object", + } +} + impl PdaSeed { + /// Check that literal pubkey content parses. Reference seeds are not + /// validated here; they depend on instance values. + pub(crate) fn validate_literals(&self) -> Result<(), SeedError> { + match self { + PdaSeed::Pubkey(s) => Pubkey::from_str(s) + .map(|_| ()) + .map_err(|_| SeedError::InvalidPubkey(s.clone())), + PdaSeed::DerivedPda { program_id, seeds } => { + Pubkey::from_str(program_id) + .map_err(|_| SeedError::InvalidPubkey(program_id.clone()))?; + for (i, seed) in seeds.iter().enumerate() { + seed.validate_literals() + .map_err(|e| SeedError::DerivedSeed { + program_id: program_id.clone(), + index: i, + source: Box::new(e), + })?; + } + Ok(()) + } + _ => Ok(()), + } + } + /// Convert a seed to bytes, optionally using values for PropertyRef resolution - pub fn to_bytes(&self, values: Option<&HashMap>) -> Option> { + pub fn to_bytes( + &self, + values: Option<&HashMap>, + ) -> Result, SeedError> { match self { PdaSeed::Pubkey(pk_str) => Pubkey::from_str(pk_str) - .ok() - .map(|pk| pk.to_bytes().to_vec()), - PdaSeed::String(s) => Some(s.as_bytes().to_vec()), - PdaSeed::Bytes(b) => Some(b.clone()), + .map(|pk| pk.to_bytes().to_vec()) + .map_err(|_| SeedError::InvalidPubkey(pk_str.clone())), + PdaSeed::String(s) => Ok(s.as_bytes().to_vec()), + PdaSeed::Bytes(b) => Ok(b.clone()), PdaSeed::PropertyRef(prop) => { - values?.get(prop).and_then(|v| { - // Handle string values (could be pubkey or raw string) - if let Some(s) = v.as_str() { - if let Ok(pk) = Pubkey::from_str(s) { - return Some(pk.to_bytes().to_vec()); - } - return Some(s.as_bytes().to_vec()); - } - // Handle numeric values (u64) - if let Some(n) = v.as_u64() { - return Some(n.to_le_bytes().to_vec()); + let v = property_value(values, prop)?; + + // Handle string values (could be pubkey or raw string) + if let Some(s) = v.as_str() { + if let Ok(pk) = Pubkey::from_str(s) { + return Ok(pk.to_bytes().to_vec()); } - None + return Ok(s.as_bytes().to_vec()); + } + // Handle numeric values (u64) + if let Some(n) = v.as_u64() { + return Ok(n.to_le_bytes().to_vec()); + } + Err(SeedError::WrongType { + name: prop.clone(), + expected: "string or u64", + found: json_type_name(v), }) } - PdaSeed::U16Be(n) => Some(n.to_be_bytes().to_vec()), - PdaSeed::U16BeRef(prop) => values?.get(prop).and_then(|v| { - let index = match v { - serde_json::Value::String(s) => s.parse::().ok()?, - _ => u16::try_from(v.as_u64()?).ok()?, - }; - Some(index.to_be_bytes().to_vec()) - }), - PdaSeed::U16Le(n) => Some(n.to_le_bytes().to_vec()), + PdaSeed::U16Be(n) => Ok(n.to_be_bytes().to_vec()), + PdaSeed::U16BeRef(prop) => { + let v = property_value(values, prop)?; + + // A scenario file may carry the index as a JSON number or as a + // decimal string; both mean the same two big-endian bytes. + if let serde_json::Value::String(s) = v { + let index = s.parse::().map_err(|_| SeedError::WrongType { + name: prop.clone(), + expected: "u16", + found: json_type_name(v), + })?; + return Ok(index.to_be_bytes().to_vec()); + } + + // Handle numeric values - convert to u16 big-endian + if let Some(n) = v.as_u64() { + let n16 = u16::try_from(n).map_err(|_| SeedError::U16OutOfRange { + name: prop.clone(), + value: n, + })?; + return Ok(n16.to_be_bytes().to_vec()); + } + Err(SeedError::WrongType { + name: prop.clone(), + expected: "u64", + found: json_type_name(v), + }) + } + PdaSeed::U16Le(n) => Ok(n.to_le_bytes().to_vec()), PdaSeed::Bytes32Ref(prop) => { - values?.get(prop).and_then(|v| { - // Handle hex string values (e.g., "0xef0d8b6f..." for Pyth feed IDs) - if let Some(s) = v.as_str() { - // Remove 0x prefix if present - let hex_str = s.strip_prefix("0x").unwrap_or(s); - // Parse as 32-byte hex - if let Ok(bytes) = hex::decode(hex_str) { - if bytes.len() == 32 { - return Some(bytes); - } + let v = property_value(values, prop)?; + + // Handle hex string values (e.g., "0xef0d8b6f..." for Pyth feed IDs) + if let Some(s) = v.as_str() { + // Remove 0x prefix if present + let hex_str = s.strip_prefix("0x").unwrap_or(s); + // Parse as 32-byte hex + match hex::decode(hex_str) { + Ok(bytes) if bytes.len() == 32 => return Ok(bytes), + _ => { + return Err(SeedError::InvalidBytes32(hex_str.to_string())); } } - None + } + Err(SeedError::WrongType { + name: prop.clone(), + expected: "hex string", + found: json_type_name(v), }) } PdaSeed::DerivedPda { program_id, seeds } => { // Derive a nested PDA and use its pubkey as the seed - let program_pubkey = Pubkey::from_str(program_id).ok()?; + let program_pubkey = Pubkey::from_str(program_id) + .map_err(|_| SeedError::InvalidPubkey(program_id.clone()))?; // Convert inner seeds to bytes let seed_bytes: Vec> = seeds .iter() - .filter_map(|seed| seed.to_bytes(values)) - .collect(); - - // Ensure all seeds were converted successfully - if seed_bytes.len() != seeds.len() { - return None; - } + .enumerate() + .map(|(i, seed)| { + seed.to_bytes(values).map_err(|e| SeedError::DerivedSeed { + program_id: program_id.clone(), + index: i, + source: Box::new(e), + }) + }) + .collect::, _>>()?; // Create seed slices for find_program_address let seed_slices: Vec<&[u8]> = seed_bytes.iter().map(|s| s.as_slice()).collect(); // Derive the nested PDA let (pda, _bump) = Pubkey::find_program_address(&seed_slices, &program_pubkey); - Some(pda.to_bytes().to_vec()) + Ok(pda.to_bytes().to_vec()) } } } @@ -185,36 +332,44 @@ impl AccountAddress { /// Resolve the account address to a Pubkey /// For PDA addresses, this derives the address from the program_id and seeds /// For PropertyRef seeds, values map is used to resolve the reference - pub fn resolve(&self, values: Option<&HashMap>) -> Option { + pub fn resolve( + &self, + values: Option<&HashMap>, + ) -> Result { match self { - AccountAddress::Pubkey(pubkey_str) => Pubkey::from_str(pubkey_str).ok(), + AccountAddress::Pubkey(pubkey_str) => Pubkey::from_str(pubkey_str).map_err(|_| { + ScenarioError::InvalidAddress(SeedError::InvalidPubkey(pubkey_str.clone())) + }), AccountAddress::Pda { program_id, seeds } => { - let program_pubkey = Pubkey::from_str(program_id).ok()?; + let program_pubkey = Pubkey::from_str(program_id) + .map_err(|_| ScenarioError::InvalidProgramId(program_id.clone()))?; // Convert all seeds to bytes let seed_bytes: Vec> = seeds .iter() - .filter_map(|seed| seed.to_bytes(values)) - .collect(); - - // Ensure all seeds were converted successfully - if seed_bytes.len() != seeds.len() { - return None; - } + .enumerate() + .map(|(i, seed)| { + seed.to_bytes(values).map_err(|e| ScenarioError::Seed { + program_id: program_id.clone(), + index: i, + source: e, + }) + }) + .collect::, _>>()?; // Create seed slices for find_program_address let seed_slices: Vec<&[u8]> = seed_bytes.iter().map(|s| s.as_slice()).collect(); // Derive the PDA let (pda, _bump) = Pubkey::find_program_address(&seed_slices, &program_pubkey); - Some(pda) + Ok(pda) } } } /// Resolve the account address to a Pubkey without any values for PropertyRef /// This is a convenience method when no PropertyRef seeds are expected - pub fn resolve_simple(&self) -> Option { + pub fn resolve_simple(&self) -> Result { self.resolve(None) } @@ -638,16 +793,29 @@ pub struct YamlOverrideTemplateFile { pub llm_context: Option, } +/// Convert a template's YAML address, attaching the template id to any failure. +fn template_address( + template_id: &str, + address: YamlAccountAddress, +) -> Result { + AccountAddress::try_from(address).map_err(|e| ScenarioError::Template { + template_id: template_id.to_string(), + source: Box::new(e), + }) +} + impl YamlOverrideTemplateFile { /// Convert file-based template to runtime OverrideTemplate with loaded IDL - pub fn to_override_template(self, idl: Idl) -> OverrideTemplate { - OverrideTemplate { + pub fn to_override_template(self, idl: Idl) -> Result { + let address = template_address(&self.id, self.address)?; + + Ok(OverrideTemplate { id: self.id, name: self.name, description: self.description, protocol: self.protocol, idl, - address: self.address.into(), + address, account_type: self.account_type, properties: self.properties.into_iter().map(Into::into).collect(), constants: self @@ -657,7 +825,7 @@ impl YamlOverrideTemplateFile { .collect(), tags: self.tags, llm_context: self.llm_context, - } + }) } } @@ -932,8 +1100,8 @@ pub struct YamlOverrideTemplateEntry { } impl YamlOverrideTemplateCollection { - /// Convert collection to runtime OverrideTemplates with loaded IDL - pub fn to_override_templates(self, idl: Idl) -> Vec { + /// Convert collection to runtime OverrideTemplates with loaded IDL, validating addresses + pub fn to_override_templates(self, idl: Idl) -> Result, ScenarioError> { // Convert constants once for sharing let constants: HashMap = self .constants @@ -943,15 +1111,17 @@ impl YamlOverrideTemplateCollection { let default_account_type = self.account_type.clone().unwrap_or_default(); - self.templates - .into_iter() - .map(|entry| OverrideTemplate { + let mut templates = Vec::new(); + for entry in self.templates { + let address = template_address(&entry.id, entry.address)?; + + templates.push(OverrideTemplate { id: entry.id, name: entry.name, description: entry.description, protocol: self.protocol.clone(), idl: idl.clone(), - address: entry.address.into(), + address, account_type: entry .idl_account_name .unwrap_or_else(|| default_account_type.clone()), @@ -959,8 +1129,10 @@ impl YamlOverrideTemplateCollection { constants: constants.clone(), tags: self.tags.clone(), llm_context: entry.llm_context, - }) - .collect() + }); + } + + Ok(templates) } } @@ -989,14 +1161,16 @@ pub struct YamlOverrideTemplate { impl YamlOverrideTemplate { /// Convert to runtime OverrideTemplate - pub fn to_override_template(self) -> OverrideTemplate { - OverrideTemplate { + pub fn to_override_template(self) -> Result { + let address = template_address(&self.id, self.address)?; + + Ok(OverrideTemplate { id: self.id, name: self.name, description: self.description, protocol: self.protocol, idl: self.idl, - address: self.address.into(), + address, account_type: self.account_type, properties: self.properties.into_iter().map(Into::into).collect(), constants: self @@ -1006,7 +1180,7 @@ impl YamlOverrideTemplate { .collect(), tags: self.tags, llm_context: self.llm_context, - } + }) } } @@ -1024,16 +1198,41 @@ pub enum YamlAccountAddress { }, } -impl From for AccountAddress { - fn from(yaml: YamlAccountAddress) -> Self { +impl std::convert::TryFrom for AccountAddress { + type Error = ScenarioError; + + fn try_from(yaml: YamlAccountAddress) -> Result { match yaml { - YamlAccountAddress::Pubkey { value } => { - AccountAddress::Pubkey(value.unwrap_or_default()) + // The empty string is the established wire placeholder for "address + // supplied per override instance" (the spl-token templates rely on + // it), so it stays until templates model the absent address + // explicitly. + YamlAccountAddress::Pubkey { value: None } => Ok(AccountAddress::Pubkey(String::new())), + YamlAccountAddress::Pubkey { value: Some(s) } => { + if Pubkey::from_str(&s).is_err() { + return Err(ScenarioError::InvalidAddress(SeedError::InvalidPubkey(s))); + } + Ok(AccountAddress::Pubkey(s)) + } + YamlAccountAddress::Pda { program_id, seeds } => { + Pubkey::from_str(&program_id) + .map_err(|_| ScenarioError::InvalidProgramId(program_id.clone()))?; + + let converted_seeds: Vec = seeds.into_iter().map(|s| s.into()).collect(); + + for (i, seed) in converted_seeds.iter().enumerate() { + seed.validate_literals().map_err(|e| ScenarioError::Seed { + program_id: program_id.clone(), + index: i, + source: e, + })?; + } + + Ok(AccountAddress::Pda { + program_id, + seeds: converted_seeds, + }) } - YamlAccountAddress::Pda { program_id, seeds } => AccountAddress::Pda { - program_id, - seeds: seeds.into_iter().map(|s| s.into()).collect(), - }, } } } @@ -1098,18 +1297,150 @@ impl From for PdaSeed { #[cfg(test)] mod tests { - use std::collections::HashMap; + use std::{collections::HashMap, str::FromStr}; use serde_json::json; + use solana_pubkey::Pubkey; - use super::PdaSeed; + use super::{ + AccountAddress, Idl, OverrideError, PdaSeed, ScenarioError, SeedError, YamlAccountAddress, + YamlOverrideTemplateCollection, YamlOverrideTemplateEntry, YamlPdaSeed, + }; + + /// Pin the rendered message of every error variant. Run with + /// `--nocapture` to view them. + #[test] + fn error_messages_render_with_full_context() { + let program = "TokenzQdBNbLqP5VEhdkAS6EPFLC1PHnBqCXEpPxuEb"; + let account = "9WzDXwBbmkg8ZTbNMqUxvQRAyrZzDsGYdLVL9zYtAWWM"; + let cases: Vec<(String, String)> = vec![ + ( + SeedError::InvalidPubkey("garbage".to_string()).to_string(), + "'garbage' is not a valid pubkey".to_string(), + ), + ( + SeedError::NoValues("owner".to_string()).to_string(), + "seed references property 'owner' but no values were provided".to_string(), + ), + ( + SeedError::UnknownProperty("owner".to_string()).to_string(), + "property 'owner' not found in values".to_string(), + ), + ( + SeedError::WrongType { + name: "owner".to_string(), + expected: "string", + found: "bool", + } + .to_string(), + "property 'owner' is bool, expected string".to_string(), + ), + ( + SeedError::U16OutOfRange { + name: "index".to_string(), + value: 70_000, + } + .to_string(), + "property 'index' value 70000 does not fit in u16".to_string(), + ), + ( + SeedError::InvalidBytes32("zz".to_string()).to_string(), + "'zz' is not a 32-byte hex string".to_string(), + ), + ( + SeedError::DerivedSeed { + program_id: program.to_string(), + index: 1, + source: Box::new(SeedError::InvalidPubkey("garbage".to_string())), + } + .to_string(), + format!( + "seed 1 of derived PDA for program {program}: 'garbage' is not a valid pubkey" + ), + ), + ( + ScenarioError::InvalidAddress(SeedError::InvalidPubkey("garbage".to_string())) + .to_string(), + "invalid account address: 'garbage' is not a valid pubkey".to_string(), + ), + ( + ScenarioError::InvalidProgramId("garbage".to_string()).to_string(), + "invalid program id 'garbage'".to_string(), + ), + ( + ScenarioError::Seed { + program_id: program.to_string(), + index: 2, + source: SeedError::UnknownProperty("owner".to_string()), + } + .to_string(), + format!("PDA for program {program}, seed 2: property 'owner' not found in values"), + ), + ( + ScenarioError::Template { + template_id: "spl-token".to_string(), + source: Box::new(ScenarioError::InvalidProgramId("garbage".to_string())), + } + .to_string(), + "template 'spl-token': invalid program id 'garbage'".to_string(), + ), + ( + OverrideError::Resolve(ScenarioError::InvalidProgramId("garbage".to_string())) + .to_string(), + "invalid program id 'garbage'".to_string(), + ), + ( + OverrideError::AccountNotFound { + account: account.to_string(), + } + .to_string(), + format!( + "account {account} does not exist locally; overrides patch existing accounts (enable fetchBeforeUse or create the account first)" + ), + ), + ( + OverrideError::AccountTooSmall { + account: account.to_string(), + len: 4, + } + .to_string(), + format!("account {account} has 4 byte(s), too small for an 8-byte discriminator"), + ), + ( + OverrideError::NoIdlForOwner { + program_id: program.to_string(), + account: account.to_string(), + } + .to_string(), + format!("no IDL registered for program {program} (owner of account {account})"), + ), + ( + OverrideError::Forge { + account: account.to_string(), + message: "borsh decode failed".to_string(), + } + .to_string(), + format!("failed to forge account data for {account}: borsh decode failed"), + ), + ]; + for (rendered, expected) in cases { + println!("{rendered}"); + assert_eq!(rendered, expected); + } + } #[test] fn u16_be_ref_rejects_out_of_range_values() { let seed = PdaSeed::U16BeRef("index".to_string()); let values = HashMap::from([("index".to_string(), json!(70_000))]); - assert_eq!(seed.to_bytes(Some(&values)), None); + assert_eq!( + seed.to_bytes(Some(&values)), + Err(SeedError::U16OutOfRange { + name: "index".to_string(), + value: 70_000, + }) + ); } #[test] @@ -1117,7 +1448,156 @@ mod tests { let seed = PdaSeed::U16BeRef("index".to_string()); let values = HashMap::from([("index".to_string(), json!(513))]); - assert_eq!(seed.to_bytes(Some(&values)), Some(vec![2, 1])); + assert_eq!(seed.to_bytes(Some(&values)), Ok(vec![2, 1])); + } + + #[test] + fn pubkey_seed_with_garbage_string() { + let seed = PdaSeed::Pubkey("not-a-pubkey".to_string()); + assert_eq!( + seed.to_bytes(None), + Err(SeedError::InvalidPubkey("not-a-pubkey".to_string())) + ); + } + + #[test] + fn property_ref_with_no_values() { + let seed = PdaSeed::PropertyRef("some_prop".to_string()); + assert_eq!( + seed.to_bytes(None), + Err(SeedError::NoValues("some_prop".to_string())) + ); + } + + #[test] + fn property_ref_with_missing_key() { + let seed = PdaSeed::PropertyRef("missing_key".to_string()); + let values = HashMap::from([("other_key".to_string(), json!("value"))]); + assert_eq!( + seed.to_bytes(Some(&values)), + Err(SeedError::UnknownProperty("missing_key".to_string())) + ); + } + + #[test] + fn property_ref_with_wrong_type() { + let seed = PdaSeed::PropertyRef("my_prop".to_string()); + let values = HashMap::from([("my_prop".to_string(), json!(true))]); + assert_eq!( + seed.to_bytes(Some(&values)), + Err(SeedError::WrongType { + name: "my_prop".to_string(), + expected: "string or u64", + found: "bool", + }) + ); + } + + #[test] + fn bytes32_ref_with_invalid_hex() { + let seed = PdaSeed::Bytes32Ref("feed_id".to_string()); + let values = HashMap::from([("feed_id".to_string(), json!("0xzz"))]); + assert_eq!( + seed.to_bytes(Some(&values)), + Err(SeedError::InvalidBytes32("zz".to_string())) + ); + } + + #[test] + fn derived_pda_with_bad_inner_seed() { + let seed = PdaSeed::DerivedPda { + program_id: "TokenkegQfeZyiNwAJbNbGKPFXCWuBvf9Ss623VQ5DA".to_string(), + seeds: vec![ + PdaSeed::String("valid_string".to_string()), + PdaSeed::Pubkey("not-a-pubkey".to_string()), + ], + }; + + match seed.to_bytes(None) { + Ok(_) => panic!("expected error"), + Err(SeedError::DerivedSeed { + program_id, + index, + source, + }) => { + assert_eq!(program_id, "TokenkegQfeZyiNwAJbNbGKPFXCWuBvf9Ss623VQ5DA"); + assert_eq!(index, 1); + assert_eq!( + *source, + SeedError::InvalidPubkey("not-a-pubkey".to_string()) + ); + } + Err(e) => panic!("wrong error: {:?}", e), + } + } + + #[test] + fn account_address_pubkey_with_garbage_resolve_simple() { + let addr = AccountAddress::Pubkey("garbage".to_string()); + assert_eq!( + addr.resolve_simple(), + Err(ScenarioError::InvalidAddress(SeedError::InvalidPubkey( + "garbage".to_string() + ))) + ); + } + + #[test] + fn account_address_pda_with_invalid_program_id() { + let addr = AccountAddress::Pda { + program_id: "invalid-program".to_string(), + seeds: vec![PdaSeed::String("test".to_string())], + }; + + match addr.resolve_simple() { + Ok(_) => panic!("expected error"), + Err(ScenarioError::InvalidProgramId(id)) => { + assert_eq!(id, "invalid-program"); + } + Err(e) => panic!("wrong error: {:?}", e), + } + } + + #[test] + fn account_address_pda_with_property_ref_no_values() { + let addr = AccountAddress::Pda { + program_id: "TokenkegQfeZyiNwAJbNbGKPFXCWuBvf9Ss623VQ5DA".to_string(), + seeds: vec![PdaSeed::PropertyRef("owner".to_string())], + }; + + match addr.resolve_simple() { + Ok(_) => panic!("expected error"), + Err(ScenarioError::Seed { + program_id, + index, + source, + }) => { + assert_eq!(program_id, "TokenkegQfeZyiNwAJbNbGKPFXCWuBvf9Ss623VQ5DA"); + assert_eq!(index, 0); + assert_eq!(source, SeedError::NoValues("owner".to_string())); + } + Err(e) => panic!("wrong error: {:?}", e), + } + } + + #[test] + fn account_address_pda_success() { + let seed_str = "test_seed"; + let addr = AccountAddress::Pda { + program_id: "TokenkegQfeZyiNwAJbNbGKPFXCWuBvf9Ss623VQ5DA".to_string(), + seeds: vec![PdaSeed::String(seed_str.to_string())], + }; + + let resolved = addr.resolve_simple(); + assert!(resolved.is_ok()); + + // Verify it matches the expected PDA computed via find_program_address + let expected_program = + Pubkey::from_str("TokenkegQfeZyiNwAJbNbGKPFXCWuBvf9Ss623VQ5DA").unwrap(); + let (expected_pda, _) = + Pubkey::find_program_address(&[seed_str.as_bytes()], &expected_program); + + assert_eq!(resolved.ok(), Some(expected_pda)); } #[test] @@ -1125,16 +1605,32 @@ mod tests { let seed = PdaSeed::U16BeRef("index".to_string()); let values = HashMap::from([("index".to_string(), json!("513"))]); - assert_eq!(seed.to_bytes(Some(&values)), Some(vec![2, 1])); + assert_eq!(seed.to_bytes(Some(&values)), Ok(vec![2, 1])); } + /// Every non-u16 string reports `WrongType`, including "65536": the + /// string path rejects at the parse, so an overflowing decimal string + /// reports the type, not the range. Only JSON numbers get + /// `U16OutOfRange`. #[test] fn u16_be_ref_rejects_strings_that_are_not_a_u16() { let seed = PdaSeed::U16BeRef("index".to_string()); for value in [json!("65536"), json!("abc"), json!("-1"), json!("")] { let values = HashMap::from([("index".to_string(), value.clone())]); - assert_eq!(seed.to_bytes(Some(&values)), None, "value {value}"); + let err = seed + .to_bytes(Some(&values)) + .expect_err(&format!("value {value}")); + assert_eq!( + err, + SeedError::WrongType { + name: "index".to_string(), + expected: "u16", + found: "string", + }, + "value {value}" + ); + assert_eq!(err.to_string(), "property 'index' is string, expected u16"); } } @@ -1144,7 +1640,168 @@ mod tests { for value in [json!(1.5), json!(true), json!(null), json!([1]), json!({})] { let values = HashMap::from([("index".to_string(), value.clone())]); - assert_eq!(seed.to_bytes(Some(&values)), None, "value {value}"); + assert!(seed.to_bytes(Some(&values)).is_err(), "value {value}"); + } + } + + #[test] + fn yaml_account_address_pubkey_valid_base58_converts() { + let yaml_addr = YamlAccountAddress::Pubkey { + value: Some("So11111111111111111111111111111111111111112".to_string()), + }; + + let result = AccountAddress::try_from(yaml_addr); + assert!(result.is_ok()); + + if let Ok(AccountAddress::Pubkey(s)) = result { + assert_eq!(s, "So11111111111111111111111111111111111111112"); + } else { + panic!("Expected Pubkey variant"); + } + } + + #[test] + fn yaml_account_address_pubkey_invalid_string_errors() { + let yaml_addr = YamlAccountAddress::Pubkey { + value: Some("not-a-pubkey".to_string()), + }; + + let result = AccountAddress::try_from(yaml_addr); + assert!(result.is_err()); + + if let Err(ScenarioError::InvalidAddress(SeedError::InvalidPubkey(s))) = result { + assert_eq!(s, "not-a-pubkey"); + } else { + panic!("Expected InvalidAddress error with InvalidPubkey"); + } + } + + #[test] + fn yaml_account_address_pubkey_none_stays_empty_string() { + let yaml_addr = YamlAccountAddress::Pubkey { value: None }; + + let result = AccountAddress::try_from(yaml_addr); + assert!(result.is_ok()); + + if let Ok(AccountAddress::Pubkey(s)) = result { + assert_eq!(s, ""); + } else { + panic!("Expected Pubkey variant with empty string"); + } + } + + #[test] + fn yaml_account_address_pda_bad_program_id_errors() { + let yaml_addr = YamlAccountAddress::Pda { + program_id: "not-a-pubkey".to_string(), + seeds: vec![], + }; + + let result = AccountAddress::try_from(yaml_addr); + assert!(result.is_err()); + + if let Err(ScenarioError::InvalidProgramId(id)) = result { + assert_eq!(id, "not-a-pubkey"); + } else { + panic!("Expected InvalidProgramId error"); + } + } + + #[test] + fn yaml_account_address_pda_bad_literal_pubkey_seed_errors() { + let yaml_addr = YamlAccountAddress::Pda { + program_id: "TokenkegQfeZyiNwAJbNbGKPFXCWuBvf9Ss623VQ5DA".to_string(), + seeds: vec![ + YamlPdaSeed::String { + value: "seed0".to_string(), + }, + YamlPdaSeed::Pubkey { + value: "oops".to_string(), + }, + ], + }; + + let result = AccountAddress::try_from(yaml_addr); + assert!(result.is_err()); + + if let Err(ScenarioError::Seed { + program_id, + index, + source, + }) = result + { + assert_eq!(program_id, "TokenkegQfeZyiNwAJbNbGKPFXCWuBvf9Ss623VQ5DA"); + assert_eq!(index, 1); + assert_eq!(source, SeedError::InvalidPubkey("oops".to_string())); + } else { + panic!("Expected Seed error at index 1"); + } + } + + #[test] + fn yaml_account_address_pda_property_ref_seed_not_validated() { + use std::convert::TryFrom; + + let yaml_addr = YamlAccountAddress::Pda { + program_id: "TokenkegQfeZyiNwAJbNbGKPFXCWuBvf9Ss623VQ5DA".to_string(), + seeds: vec![YamlPdaSeed::PropertyRef { + value: "some_property".to_string(), + }], + }; + + let result = AccountAddress::try_from(yaml_addr); + assert!( + result.is_ok(), + "PropertyRef seeds should not be validated at load time" + ); + } + + #[test] + fn yaml_override_template_collection_with_bad_address_wraps_error() { + let entry = YamlOverrideTemplateEntry { + id: "test-template".to_string(), + name: "Test".to_string(), + description: "Test template".to_string(), + idl_account_name: None, + properties: vec![], + address: YamlAccountAddress::Pubkey { + value: Some("invalid-pubkey".to_string()), + }, + llm_context: None, + }; + + let collection = YamlOverrideTemplateCollection { + protocol: "Test".to_string(), + version: "1.0".to_string(), + account_type: None, + idl_file_path: "test.json".to_string(), + tags: vec![], + constants: HashMap::new(), + templates: vec![entry], + }; + + let idl_json = r#"{ + "address": "11111111111111111111111111111111", + "instructions": [], + "accounts": [], + "types": [], + "errors": [], + "metadata": {"name": "test", "version": "0.1.0", "spec": "0.1.0"} + }"#; + + let idl: Idl = serde_json::from_str(idl_json).expect("Valid minimal IDL"); + + let result = collection.to_override_templates(idl); + assert!(result.is_err()); + + if let Err(ScenarioError::Template { + template_id, + source: _, + }) = result + { + assert_eq!(template_id, "test-template"); + } else { + panic!("Expected Template error wrapper"); } } }