diff --git a/Cargo.lock b/Cargo.lock index 409bb7e20..8b6c08d81 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -980,6 +980,7 @@ dependencies = [ "async-trait", "coins-bip32 0.12.0", "coins-bip39 0.12.0", + "eth-keystore", "k256", "rand 0.8.5", "thiserror 2.0.17", @@ -15192,6 +15193,8 @@ dependencies = [ "alloy-rpc-types", "alloy-signer-local", "async-recursion", + "dirs 5.0.1", + "eth-keystore", "foundry-block-explorers", "foundry-compilers", "foundry-compilers-artifacts-solc", @@ -15200,11 +15203,13 @@ dependencies = [ "lazy_static", "libsecp256k1 0.7.1", "pbkdf2 0.12.2", + "rand 0.8.5", "semver 1.0.26", "serde", "serde_derive", "serde_json", "sha2 0.10.9", + "tempfile", "thiserror 1.0.69", "tiny-hderive", "toml 0.5.11", diff --git a/addons/evm/Cargo.toml b/addons/evm/Cargo.toml index 151d4ce24..d2ffd7198 100644 --- a/addons/evm/Cargo.toml +++ b/addons/evm/Cargo.toml @@ -38,7 +38,9 @@ alloy-chains = "0.2.14" alloy-primitives = { version = "1.4.1" } alloy-provider = { version = "1.0.41", default-features = false, features = ["debug-api"] } alloy-rpc-types = { version = "1.0.41", features = ["trace"] } -alloy-signer-local = { version = "1.0.41", features = ["mnemonic"] } +alloy-signer-local = { version = "1.0.41", features = ["mnemonic", "keystore"] } +eth-keystore = "0.5.0" +dirs = "5.0" thiserror = "1.0.62" toml = "0.5" foundry-block-explorers = "0.22.0" @@ -53,6 +55,10 @@ wasm = [ "txtx-addon-kit/wasm", ] +[dev-dependencies] +tempfile = "3" +rand = "0.8" + [lib] crate-type = ["cdylib", "rlib"] path = "src/lib.rs" diff --git a/addons/evm/src/codec/crypto.rs b/addons/evm/src/codec/crypto.rs index 1adacd69d..0a00b87ed 100644 --- a/addons/evm/src/codec/crypto.rs +++ b/addons/evm/src/codec/crypto.rs @@ -49,6 +49,84 @@ pub fn field_bytes_to_secret_key_signer(field_bytes: &Vec) -> Result, +) -> Result { + use std::path::PathBuf; + + let keystore_dir = match keystore_path { + Some(path) => PathBuf::from(path), + None => { + let home = dirs::home_dir() + .ok_or_else(|| "could not determine home directory".to_string())?; + home.join(".foundry").join("keystores") + } + }; + + // If keystore_account is already an absolute path, use it directly. + // Security: The user provides both the path and password interactively; only valid encrypted keystores are accepted. + // UX: The .json extension check is mainly to catch typos and provide clearer error messages. + let account_path = PathBuf::from(keystore_account); + if account_path.is_absolute() { + if !account_path.extension().map_or(false, |ext| ext == "json") { + return Err(format!( + "absolute keystore path should have .json extension: {:?}", + account_path + )); + } + return Ok(account_path); + } + + // Otherwise, join with the keystore directory + // Try with .json extension first, then without + let with_json = keystore_dir.join(format!("{}.json", keystore_account)); + if with_json.exists() { + return Ok(with_json); + } + + let without_json = keystore_dir.join(keystore_account); + if without_json.exists() { + return Ok(without_json); + } + + // Return the path with .json extension (will fail at decrypt time with better error) + Ok(with_json) +} + +/// Decrypts a keystore file and returns a SecretKeySigner +pub fn keystore_to_secret_key_signer( + keystore_path: &std::path::Path, + password: &str, +) -> Result { + if !keystore_path.exists() { + return Err(format!("keystore file not found: {:?}", keystore_path)); + } + + if keystore_path.is_dir() { + return Err(format!( + "keystore path is a directory, expected a file: {:?}", + keystore_path + )); + } + + let secret_key = eth_keystore::decrypt_key(keystore_path, password).map_err(|e| { + let err_str = e.to_string(); + if err_str.contains("Mac Mismatch") { + format!("incorrect password for keystore '{}'", keystore_path.display()) + } else { + format!("failed to decrypt keystore '{}': {}", keystore_path.display(), err_str) + } + })?; + + let signing_key = SigningKey::from_slice(&secret_key) + .map_err(|e| format!("invalid key in keystore: {}", e))?; + + Ok(SecretKeySigner::from_signing_key(signing_key)) +} + pub fn public_key_to_address(public_key_bytes: &Vec) -> Result { let pubkey = VerifyingKey::from_sec1_bytes(&public_key_bytes) .map_err(|e| format!("invalid public key: {}", e))?; @@ -93,3 +171,298 @@ pub fn public_key_from_signed_message( let public_key_bytes = public_key.serialize().to_vec(); Ok(public_key_bytes) } + +#[cfg(test)] +mod tests { + use super::*; + use std::fs; + use tempfile::TempDir; + + // ============ resolve_keystore_path tests ============ + + #[test] + fn test_resolve_keystore_path_absolute_path_with_json() { + let result = resolve_keystore_path("/absolute/path/to/keystore.json", None); + assert!(result.is_ok()); + assert_eq!( + result.unwrap().to_str().unwrap(), + "/absolute/path/to/keystore.json" + ); + } + + #[test] + fn test_resolve_keystore_path_absolute_path_without_json_rejected() { + let result = resolve_keystore_path("/some/path/without/extension", None); + assert!(result.is_err()); + assert!(result.unwrap_err().contains(".json extension")); + } + + #[test] + fn test_resolve_keystore_path_with_custom_directory() { + let temp_dir = TempDir::new().unwrap(); + let keystore_path = temp_dir.path().join("myaccount.json"); + fs::write(&keystore_path, "{}").unwrap(); + + let result = + resolve_keystore_path("myaccount", Some(temp_dir.path().to_str().unwrap())); + assert!(result.is_ok()); + assert_eq!(result.unwrap(), keystore_path); + } + + #[test] + fn test_resolve_keystore_path_adds_json_extension() { + let temp_dir = TempDir::new().unwrap(); + let keystore_path = temp_dir.path().join("myaccount.json"); + fs::write(&keystore_path, "{}").unwrap(); + + let result = + resolve_keystore_path("myaccount", Some(temp_dir.path().to_str().unwrap())); + assert!(result.is_ok()); + assert!(result.unwrap().to_string_lossy().ends_with(".json")); + } + + #[test] + fn test_resolve_keystore_path_without_json_extension() { + let temp_dir = TempDir::new().unwrap(); + let keystore_path = temp_dir.path().join("myaccount"); + fs::write(&keystore_path, "{}").unwrap(); + + let result = + resolve_keystore_path("myaccount", Some(temp_dir.path().to_str().unwrap())); + assert!(result.is_ok()); + assert_eq!(result.unwrap(), keystore_path); + } + + #[test] + fn test_resolve_keystore_path_nonexistent_returns_with_json() { + let temp_dir = TempDir::new().unwrap(); + + let result = + resolve_keystore_path("nonexistent", Some(temp_dir.path().to_str().unwrap())); + assert!(result.is_ok()); + assert!(result + .unwrap() + .to_string_lossy() + .ends_with("nonexistent.json")); + } + + #[test] + fn test_resolve_keystore_path_default_foundry_dir() { + let result = resolve_keystore_path("myaccount", None); + assert!(result.is_ok()); + let path = result.unwrap(); + assert!(path.to_string_lossy().contains(".foundry")); + assert!(path.to_string_lossy().contains("keystores")); + } + + #[test] + fn test_resolve_keystore_path_prefers_json_extension() { + let temp_dir = TempDir::new().unwrap(); + // Create both files + let with_json = temp_dir.path().join("myaccount.json"); + let without_json = temp_dir.path().join("myaccount"); + fs::write(&with_json, "json").unwrap(); + fs::write(&without_json, "no-json").unwrap(); + + let result = + resolve_keystore_path("myaccount", Some(temp_dir.path().to_str().unwrap())); + assert!(result.is_ok()); + // Should prefer .json extension + assert_eq!(result.unwrap(), with_json); + } + + // ============ keystore_to_secret_key_signer tests ============ + + #[test] + fn test_keystore_to_secret_key_signer_file_not_found() { + let result = keystore_to_secret_key_signer( + std::path::Path::new("/nonexistent/path.json"), + "password", + ); + assert!(result.is_err()); + assert!(result.unwrap_err().contains("not found")); + } + + #[test] + fn test_keystore_to_secret_key_signer_directory_error() { + let temp_dir = TempDir::new().unwrap(); + + let result = keystore_to_secret_key_signer(temp_dir.path(), "password"); + assert!(result.is_err()); + assert!(result.unwrap_err().contains("directory")); + } + + #[test] + fn test_keystore_to_secret_key_signer_invalid_json() { + let temp_dir = TempDir::new().unwrap(); + let keystore_path = temp_dir.path().join("invalid.json"); + fs::write(&keystore_path, "not valid json").unwrap(); + + let result = keystore_to_secret_key_signer(&keystore_path, "password"); + assert!(result.is_err()); + assert!(result.unwrap_err().contains("decrypt")); + } + + #[test] + fn test_keystore_to_secret_key_signer_wrong_password() { + let temp_dir = TempDir::new().unwrap(); + + // Create a valid keystore with known password + // Note: eth_keystore::encrypt_key returns UUID, but file is named by the `name` param + let secret_key = [1u8; 32]; + let mut rng = rand::thread_rng(); + let _uuid = eth_keystore::encrypt_key( + temp_dir.path(), + &mut rng, + &secret_key, + "correct_password", + Some("test"), + ) + .unwrap(); + + let keystore_path = temp_dir.path().join("test"); + let result = keystore_to_secret_key_signer(&keystore_path, "wrong_password"); + assert!(result.is_err()); + assert!(result.unwrap_err().contains("incorrect password")); + } + + #[test] + fn test_keystore_cannot_decrypt_without_password() { + let temp_dir = TempDir::new().unwrap(); + + let secret_key = [0xABu8; 32]; + let mut rng = rand::thread_rng(); + let _uuid = eth_keystore::encrypt_key( + temp_dir.path(), + &mut rng, + &secret_key, + "strong_password_123", + Some("secured"), + ) + .unwrap(); + + let keystore_path = temp_dir.path().join("secured"); + + // Verify keystore file exists and contains encrypted data + assert!(keystore_path.exists()); + let keystore_contents = fs::read_to_string(&keystore_path).unwrap(); + assert!(keystore_contents.contains("crypto")); + assert!(keystore_contents.contains("ciphertext")); + // The raw secret key should NOT appear in the file + assert!(!keystore_contents.contains("abababab")); + + // Empty password must fail + let result_empty = keystore_to_secret_key_signer(&keystore_path, ""); + assert!(result_empty.is_err(), "Empty password should fail decryption"); + + // Partial password must fail + let result_partial = keystore_to_secret_key_signer(&keystore_path, "strong"); + assert!(result_partial.is_err(), "Partial password should fail decryption"); + + // Correct password succeeds + let result_correct = keystore_to_secret_key_signer(&keystore_path, "strong_password_123"); + assert!(result_correct.is_ok(), "Correct password should succeed"); + } + + #[test] + fn test_keystore_to_secret_key_signer_success() { + let temp_dir = TempDir::new().unwrap(); + + let secret_key = [0x42u8; 32]; + let mut rng = rand::thread_rng(); + let _uuid = eth_keystore::encrypt_key( + temp_dir.path(), + &mut rng, + &secret_key, + "test_password", + Some("testaccount"), + ) + .unwrap(); + + // File is named by the `name` parameter, not the returned UUID + let keystore_path = temp_dir.path().join("testaccount"); + let result = keystore_to_secret_key_signer(&keystore_path, "test_password"); + + assert!(result.is_ok()); + let signer = result.unwrap(); + let _address = signer.address(); + } + + /// Tests the full keystore encryption/decryption roundtrip: + /// 1. Create a signer from a known mnemonic (source of truth) + /// 2. Extract the private key bytes and encrypt them into a keystore file + /// 3. Decrypt the keystore with the correct password + /// 4. Verify the decrypted signer produces the same address as the original + /// + /// This proves that providing the correct password extracts the original key, + /// since identical addresses can only be derived from identical private keys. + #[test] + fn test_keystore_roundtrip_extracts_correct_key_with_password() { + let temp_dir = TempDir::new().unwrap(); + + // Step 1: Create signer from known mnemonic - this is our source of truth + let mnemonic = "abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon about"; + let original_signer = + mnemonic_to_secret_key_signer(mnemonic, None, None, None).unwrap(); + let expected_address = original_signer.address(); + + // Step 2: Encrypt the private key into a keystore file + let field_bytes = original_signer.to_field_bytes().to_vec(); + let mut rng = rand::thread_rng(); + let _uuid = eth_keystore::encrypt_key( + temp_dir.path(), + &mut rng, + &field_bytes, + "password", + Some("test"), + ) + .unwrap(); + + // Step 3: Decrypt keystore with correct password + let keystore_path = temp_dir.path().join("test"); + let decrypted_signer = + keystore_to_secret_key_signer(&keystore_path, "password").unwrap(); + + // Step 4: Verify decrypted key matches original by comparing addresses + // (address is derived from private key, so matching = same key) + assert_eq!(decrypted_signer.address(), expected_address); + } + + // ============ mnemonic_to_secret_key_signer tests ============ + + #[test] + fn test_mnemonic_to_secret_key_signer_valid() { + let mnemonic = "abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon about"; + let result = mnemonic_to_secret_key_signer(mnemonic, None, None, None); + assert!(result.is_ok()); + } + + #[test] + fn test_mnemonic_to_secret_key_signer_invalid_mnemonic() { + let result = + mnemonic_to_secret_key_signer("invalid mnemonic words", None, None, None); + assert!(result.is_err()); + } + + #[test] + fn test_mnemonic_to_secret_key_signer_custom_derivation_path() { + let mnemonic = "abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon about"; + let result1 = mnemonic_to_secret_key_signer(mnemonic, None, None, None); + let result2 = + mnemonic_to_secret_key_signer(mnemonic, Some("m/44'/60'/0'/0/1"), None, None); + + assert!(result1.is_ok()); + assert!(result2.is_ok()); + // Different derivation paths should produce different addresses + assert_ne!(result1.unwrap().address(), result2.unwrap().address()); + } + + #[test] + fn test_mnemonic_to_secret_key_signer_encrypted_not_supported() { + let mnemonic = "abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon about"; + let result = + mnemonic_to_secret_key_signer(mnemonic, None, Some(true), Some("password")); + assert!(result.is_err()); + assert!(result.unwrap_err().contains("not yet supported")); + } +} diff --git a/addons/evm/src/constants.rs b/addons/evm/src/constants.rs index c50b5a635..cdd952963 100644 --- a/addons/evm/src/constants.rs +++ b/addons/evm/src/constants.rs @@ -25,6 +25,10 @@ pub const PUBLIC_KEYS: &str = "public_keys"; pub const SECRET_KEY_WALLET_UNSIGNED_TRANSACTION_BYTES: &str = "secret_key_wallet_unsigned_transaction_bytes"; pub const WEB_WALLET_UNSIGNED_TRANSACTION_BYTES: &str = "web_wallet_unsigned_transaction_bytes"; +pub const KEYSTORE_ACCOUNT: &str = "keystore_account"; +pub const KEYSTORE_PATH: &str = "keystore_path"; +pub const PASSWORD: &str = "password"; + pub const TRANSACTION_PAYLOAD_BYTES: &str = "transaction_payload_bytes"; pub const SIGNED_MESSAGE_BYTES: &str = "signed_message_bytes"; pub const MESSAGE_BYTES: &str = "message_bytes"; diff --git a/addons/evm/src/lib.rs b/addons/evm/src/lib.rs index ca9bc9aff..60b26072c 100644 --- a/addons/evm/src/lib.rs +++ b/addons/evm/src/lib.rs @@ -7,7 +7,7 @@ extern crate txtx_addon_kit; #[macro_use] extern crate serde_derive; -mod codec; +pub mod codec; mod commands; #[allow(dead_code)] mod constants; diff --git a/addons/evm/src/signers/common.rs b/addons/evm/src/signers/common.rs index 6d2ee4910..0151d9009 100644 --- a/addons/evm/src/signers/common.rs +++ b/addons/evm/src/signers/common.rs @@ -1,23 +1,31 @@ +use alloy::network::{EthereumWallet, TransactionBuilder}; use alloy::primitives::{utils::format_units, Address}; +use alloy_rpc_types::TransactionRequest; use txtx_addon_kit::{ + constants::SIGNATURE_APPROVED, indexmap::IndexMap, types::{ + commands::CommandExecutionResult, frontend::{ - ActionItemRequest, ActionItemRequestType, ActionItemStatus, ProvidePublicKeyRequest, - ReviewInputRequest, + ActionItemRequest, ActionItemRequestType, ActionItemStatus, Actions, + ProvidePublicKeyRequest, ProvideSignedTransactionRequest, ReviewInputRequest, }, + signers::{SignerActionErr, SignersState}, stores::ValueStore, - types::Value, + types::{RunbookSupervisionContext, Value}, ConstructDid, }, }; use crate::{ + codec::crypto::field_bytes_to_secret_key_signer, constants::{ ACTION_ITEM_CHECK_ADDRESS, ACTION_ITEM_CHECK_BALANCE, ACTION_ITEM_PROVIDE_PUBLIC_KEY, - DEFAULT_MESSAGE, NAMESPACE, + ACTION_ITEM_PROVIDE_SIGNED_TRANSACTION, CHAIN_ID, DEFAULT_MESSAGE, FORMATTED_TRANSACTION, + NAMESPACE, RPC_API_URL, SECRET_KEY_WALLET_UNSIGNED_TRANSACTION_BYTES, TX_HASH, }, - rpc::EvmRpc, + rpc::{EvmRpc, EvmWalletRpc}, + typing::EvmValue, }; pub async fn get_additional_actions_for_address( @@ -160,3 +168,407 @@ impl NonceManager { Ok(NonceManager(IndexMap::new())) } } + +/// Shared activate implementation for secret_key and keystore signers +pub fn activate_signer( + signer_state: ValueStore, + signers: SignersState, +) -> Result<(SignersState, ValueStore, CommandExecutionResult), (SignersState, ValueStore, txtx_addon_kit::types::diagnostics::Diagnostic)> { + let mut result = CommandExecutionResult::new(); + let address = signer_state + .get_expected_value("signer_address") + .map_err(|e| (signers.clone(), signer_state.clone(), e))?; + result.outputs.insert("address".into(), address.clone()); + Ok((signers, signer_state, result)) +} + +/// Shared check_signability implementation for secret_key and keystore signers +pub fn check_signability( + construct_did: &ConstructDid, + title: &str, + description: &Option, + meta_description: &Option, + markdown: &Option, + payload: &Value, + values: &ValueStore, + signer_state: ValueStore, + signers: SignersState, + supervision_context: &RunbookSupervisionContext, +) -> Result<(SignersState, ValueStore, Actions), SignerActionErr> { + let actions = if supervision_context.review_input_values { + let construct_did_str = construct_did.to_string(); + if signer_state.get_scoped_value(&construct_did_str, SIGNATURE_APPROVED).is_some() { + return Ok((signers, signer_state, Actions::none())); + } + + let chain_id = values + .get_expected_uint(CHAIN_ID) + .map_err(|e| (signers.clone(), signer_state.clone(), e))?; + + let formatted_payload = + signer_state.get_scoped_value(&construct_did_str, FORMATTED_TRANSACTION); + + let request = ProvideSignedTransactionRequest::new( + &signer_state.uuid, + payload, + NAMESPACE, + &chain_id.to_string(), + ) + .check_expectation_action_uuid(construct_did) + .only_approval_needed() + .formatted_payload(formatted_payload) + .to_action_type() + .to_request(title, ACTION_ITEM_PROVIDE_SIGNED_TRANSACTION) + .with_construct_did(construct_did) + .with_some_description(description.clone()) + .with_some_meta_description(meta_description.clone()) + .with_some_markdown(markdown.clone()) + .with_status(ActionItemStatus::Todo); + + Actions::append_item( + request, + Some("Review and sign the transactions from the list below"), + Some("Transaction Signing"), + ) + } else { + Actions::none() + }; + Ok((signers, signer_state, actions)) +} + +/// Shared sign implementation for secret_key and keystore signers +pub async fn sign_transaction( + caller_uuid: &ConstructDid, + values: &ValueStore, + signer_state: ValueStore, + signers: SignersState, +) -> Result<(SignersState, ValueStore, CommandExecutionResult), (SignersState, ValueStore, txtx_addon_kit::types::diagnostics::Diagnostic)> { + let mut result = CommandExecutionResult::new(); + + let rpc_api_url = values + .get_expected_string(RPC_API_URL) + .map_err(|e| (signers.clone(), signer_state.clone(), e))?; + + let signer_field_bytes = signer_state + .get_expected_buffer_bytes("signer_field_bytes") + .map_err(|e| (signers.clone(), signer_state.clone(), e))?; + + let payload_bytes = signer_state + .get_expected_scoped_buffer_bytes( + &caller_uuid.to_string(), + SECRET_KEY_WALLET_UNSIGNED_TRANSACTION_BYTES, + ) + .map_err(|e| (signers.clone(), signer_state.clone(), e))?; + + let secret_key_signer = field_bytes_to_secret_key_signer(&signer_field_bytes) + .map_err(|e| (signers.clone(), signer_state.clone(), diagnosed_error!("{e}")))?; + + let eth_signer = EthereumWallet::from(secret_key_signer); + + let mut tx: TransactionRequest = serde_json::from_slice(&payload_bytes).map_err(|e| { + ( + signers.clone(), + signer_state.clone(), + diagnosed_error!("failed to deserialize transaction: {e}"), + ) + })?; + + if tx.to.is_none() { + tx.set_create(); + } + + let tx_envelope = tx.build(ð_signer).await.map_err(|e| { + ( + signers.clone(), + signer_state.clone(), + diagnosed_error!("failed to build transaction envelope: {e}"), + ) + })?; + + let rpc = EvmWalletRpc::new(rpc_api_url, eth_signer) + .map_err(|e| (signers.clone(), signer_state.clone(), diagnosed_error!("{e}")))?; + + let tx_hash = rpc.sign_and_send_tx(tx_envelope).await.map_err(|e| { + (signers.clone(), signer_state.clone(), diagnosed_error!("{}", e.to_string())) + })?; + + result.outputs.insert(TX_HASH.to_string(), EvmValue::tx_hash(tx_hash.to_vec())); + + Ok((signers, signer_state, result)) +} + +#[cfg(test)] +mod tests { + use super::*; + use txtx_addon_kit::types::types::RunbookSupervisionContext; + use txtx_addon_kit::types::Did; + + fn create_test_construct_did(name: &str) -> ConstructDid { + ConstructDid(Did::from_components(vec![name.as_bytes()])) + } + + fn create_test_value_store(name: &str) -> ValueStore { + ValueStore::new(name, &Did::from_components(vec![name.as_bytes()])) + } + + // ============ NonceManager tests ============ + + #[test] + fn test_nonce_manager_empty_state() { + let signer_state = create_test_value_store("test"); + let result = NonceManager::from_signer_state(&signer_state); + assert!(result.is_ok()); + let manager = result.unwrap(); + assert!(manager.0.is_empty()); + } + + #[test] + fn test_nonce_manager_get_nonce_empty() { + let signer_state = create_test_value_store("test"); + let construct_did = create_test_construct_did("test-construct"); + + let nonce = NonceManager::get_nonce_for_construct(&signer_state, 1, &construct_did); + assert!(nonce.is_none()); + } + + #[test] + fn test_nonce_manager_serialization_roundtrip() { + let mut manager = NonceManager(IndexMap::new()); + let chain_id = 1u64; + let construct_did = create_test_construct_did("test-construct"); + + let mut chain_map = IndexMap::new(); + chain_map.insert(construct_did.clone(), 42u64); + manager.0.insert(chain_id, chain_map); + + let serialized = serde_json::to_string(&manager).unwrap(); + let deserialized: NonceManager = serde_json::from_str(&serialized).unwrap(); + + assert_eq!( + deserialized + .0 + .get(&chain_id) + .unwrap() + .get(&construct_did), + Some(&42u64) + ); + } + + #[test] + fn test_nonce_manager_get_nonce_after_storing() { + let mut signer_state = create_test_value_store("test"); + let chain_id = 1u64; + let construct_did = create_test_construct_did("test-construct"); + + // Manually create and store a nonce manager + let mut manager = NonceManager(IndexMap::new()); + let mut chain_map = IndexMap::new(); + chain_map.insert(construct_did.clone(), 100u64); + manager.0.insert(chain_id, chain_map); + + let serialized = serde_json::to_string(&manager).unwrap(); + signer_state.insert(NonceManager::KEY, Value::string(serialized)); + + // Now retrieve it + let nonce = + NonceManager::get_nonce_for_construct(&signer_state, chain_id, &construct_did); + assert_eq!(nonce, Some(100u64)); + } + + #[test] + fn test_nonce_manager_get_nonce_wrong_chain() { + let mut signer_state = create_test_value_store("test"); + let chain_id = 1u64; + let construct_did = create_test_construct_did("test-construct"); + + let mut manager = NonceManager(IndexMap::new()); + let mut chain_map = IndexMap::new(); + chain_map.insert(construct_did.clone(), 100u64); + manager.0.insert(chain_id, chain_map); + + let serialized = serde_json::to_string(&manager).unwrap(); + signer_state.insert(NonceManager::KEY, Value::string(serialized)); + + // Try to get nonce for a different chain + let nonce = + NonceManager::get_nonce_for_construct(&signer_state, 42u64, &construct_did); + assert!(nonce.is_none()); + } + + #[test] + fn test_nonce_manager_invalid_json() { + let mut signer_state = create_test_value_store("test"); + signer_state.insert(NonceManager::KEY, Value::string("not valid json".to_string())); + + let result = NonceManager::from_signer_state(&signer_state); + assert!(result.is_err()); + assert!(result.unwrap_err().contains("failed to parse")); + } + + // ============ activate_signer tests ============ + + #[test] + fn test_activate_signer_success() { + let mut signer_state = create_test_value_store("test"); + signer_state.insert( + "signer_address", + Value::string("0x1234567890abcdef".to_string()), + ); + let signers = SignersState::new(); + + let result = activate_signer(signer_state, signers); + assert!(result.is_ok()); + + let (_, _, execution_result) = result.unwrap(); + assert!(execution_result.outputs.contains_key("address")); + assert_eq!( + execution_result.outputs.get("address").unwrap().as_string(), + Some("0x1234567890abcdef") + ); + } + + #[test] + fn test_activate_signer_missing_address() { + let signer_state = create_test_value_store("test"); + let signers = SignersState::new(); + + let result = activate_signer(signer_state, signers); + assert!(result.is_err()); + } + + // ============ check_signability tests ============ + + #[test] + fn test_check_signability_no_review_returns_empty_actions() { + let signer_state = create_test_value_store("test"); + let signers = SignersState::new(); + let construct_did = create_test_construct_did("test"); + let mut values = create_test_value_store("values"); + values.insert(CHAIN_ID, Value::integer(1)); + + let supervision_context = RunbookSupervisionContext { + review_input_values: false, + review_input_default_values: false, + is_supervised: false, + }; + + let result = check_signability( + &construct_did, + "Test Transaction", + &None, + &None, + &None, + &Value::null(), + &values, + signer_state, + signers, + &supervision_context, + ); + + assert!(result.is_ok()); + let (_, _, actions) = result.unwrap(); + assert!(actions.store.is_empty()); + } + + #[test] + fn test_check_signability_with_review_returns_signing_action() { + let signer_state = create_test_value_store("test"); + let signers = SignersState::new(); + let construct_did = create_test_construct_did("test"); + let mut values = create_test_value_store("values"); + values.insert(CHAIN_ID, Value::integer(1)); + + let supervision_context = RunbookSupervisionContext { + review_input_values: true, + review_input_default_values: false, + is_supervised: true, + }; + + let result = check_signability( + &construct_did, + "Test Transaction", + &Some("Description".to_string()), + &None, + &None, + &Value::null(), + &values, + signer_state, + signers, + &supervision_context, + ); + + assert!(result.is_ok()); + let (_, _, actions) = result.unwrap(); + assert!(!actions.store.is_empty()); + } + + #[test] + fn test_check_signability_already_approved_returns_empty() { + let mut signer_state = create_test_value_store("test"); + let construct_did = create_test_construct_did("test-construct"); + + // Mark as already approved + signer_state.insert_scoped_value( + &construct_did.to_string(), + SIGNATURE_APPROVED, + Value::bool(true), + ); + + let signers = SignersState::new(); + let mut values = create_test_value_store("values"); + values.insert(CHAIN_ID, Value::integer(1)); + + let supervision_context = RunbookSupervisionContext { + review_input_values: true, + review_input_default_values: false, + is_supervised: true, + }; + + let result = check_signability( + &construct_did, + "Test Transaction", + &None, + &None, + &None, + &Value::null(), + &values, + signer_state, + signers, + &supervision_context, + ); + + assert!(result.is_ok()); + let (_, _, actions) = result.unwrap(); + assert!(actions.store.is_empty()); + } + + #[test] + fn test_check_signability_missing_chain_id() { + let signer_state = create_test_value_store("test"); + let signers = SignersState::new(); + let construct_did = create_test_construct_did("test"); + let values = create_test_value_store("values"); + // Note: not inserting CHAIN_ID + + let supervision_context = RunbookSupervisionContext { + review_input_values: true, + review_input_default_values: false, + is_supervised: true, + }; + + let result = check_signability( + &construct_did, + "Test Transaction", + &None, + &None, + &None, + &Value::null(), + &values, + signer_state, + signers, + &supervision_context, + ); + + assert!(result.is_err()); + } +} diff --git a/addons/evm/src/signers/keystore.rs b/addons/evm/src/signers/keystore.rs new file mode 100644 index 000000000..15d4cec93 --- /dev/null +++ b/addons/evm/src/signers/keystore.rs @@ -0,0 +1,249 @@ +use alloy::primitives::Address; +use std::collections::HashMap; +use txtx_addon_kit::channel; +use txtx_addon_kit::types::frontend::{Actions, BlockEvent}; +use txtx_addon_kit::types::signers::{ + return_synchronous_result, CheckSignabilityOk, SignerActionErr, SignerActionsFutureResult, + SignerActivateFutureResult, SignerImplementation, SignerInstance, SignerSignFutureResult, + SignerSpecification, SignersState, +}; +use txtx_addon_kit::types::stores::ValueStore; +use txtx_addon_kit::types::types::{RunbookSupervisionContext, Type, Value}; +use txtx_addon_kit::types::commands::CommandSpecification; +use txtx_addon_kit::types::{diagnostics::Diagnostic, AuthorizationContext, ConstructDid}; + +use super::common::{activate_signer, check_signability, sign_transaction}; +use crate::codec::crypto::{keystore_to_secret_key_signer, resolve_keystore_path}; +use crate::constants::{CHECKED_ADDRESS, KEYSTORE_ACCOUNT, KEYSTORE_PATH, PASSWORD, PUBLIC_KEYS}; +use crate::typing::EvmValue; +use txtx_addon_kit::types::signers::return_synchronous_actions; + +lazy_static! { + pub static ref EVM_KEYSTORE_SIGNER: SignerSpecification = define_signer! { + EvmKeystoreSigner => { + name: "EVM Keystore Signer", + matcher: "keystore", + documentation: txtx_addon_kit::indoc! {r#" +The `evm::keystore` signer uses a Foundry-compatible encrypted keystore file to sign transactions. + +**Note:** This signer is only supported in unsupervised mode (`--unsupervised` flag). For supervised/interactive mode, use `evm::web_wallet` instead. + +### Inputs + +| Name | Type | Required | Description | +|--------------------|--------|----------|-----------------------------------------------------------------------------| +| `keystore_account` | string | Yes | The account name (filename without .json) or full path to the keystore file | +| `keystore_path` | string | No | Directory containing keystores. Defaults to `~/.foundry/keystores` | + +### Outputs + +| Name | Type | Description | +|-----------|--------|------------------------------------------| +| `address` | string | The Ethereum address derived from the keystore | + +### Password + +The keystore password is prompted interactively at CLI startup (Foundry-style UX). The password is never stored in the manifest or on disk. + +### Security + +- Compatible with keystores created by `cast wallet import` +- Password is prompted securely (hidden input) before execution begins +- Password is held only in memory during execution +"#}, + inputs: [ + keystore_account: { + documentation: "The account name (filename without .json) in the keystores directory, or a full path to the keystore file.", + typing: Type::string(), + optional: false, + tainting: true, + sensitive: false + }, + keystore_path: { + documentation: "The directory containing keystore files. Defaults to ~/.foundry/keystores", + typing: Type::string(), + optional: true, + tainting: true, + sensitive: false + }, + password: { + documentation: "Internal use only - populated by CLI interactive prompt.", + typing: Type::string(), + optional: true, + tainting: false, + sensitive: true + } + ], + outputs: [ + address: { + documentation: "The address derived from the keystore.", + typing: Type::string() + } + ], + example: txtx_addon_kit::indoc! {r#" + // Use a keystore from the default Foundry keystores directory + signer "deployer" "evm::keystore" { + keystore_account = "my-deployer-account" + } + + // Use a keystore from a custom directory + signer "deployer" "evm::keystore" { + keystore_account = "deployer" + keystore_path = "./keystores" + } + + // Use a full path to a keystore file + signer "deployer" "evm::keystore" { + keystore_account = "/path/to/my-keystore.json" + } + "#} + } + }; +} + +pub struct EvmKeystoreSigner; + +impl SignerImplementation for EvmKeystoreSigner { + fn check_instantiability( + _ctx: &SignerSpecification, + _args: Vec, + ) -> Result { + unimplemented!() + } + + #[cfg(not(feature = "wasm"))] + fn check_activability( + _construct_did: &ConstructDid, + _instance_name: &str, + _spec: &SignerSpecification, + values: &ValueStore, + mut signer_state: ValueStore, + signers: SignersState, + _signers_instances: &HashMap, + supervision_context: &RunbookSupervisionContext, + _auth_ctx: &AuthorizationContext, + _is_balance_check_required: bool, + _is_public_key_required: bool, + ) -> SignerActionsFutureResult { + // Keystore signer only supports unsupervised mode - for supervised mode use web_wallet + if supervision_context.is_supervised { + return Err(( + signers, + signer_state, + diagnosed_error!( + "evm::keystore signer is only supported in unsupervised mode. \ + For supervised mode, use evm::web_wallet instead." + ), + )); + } + + let mut actions = Actions::none(); + + if signer_state.get_value(PUBLIC_KEYS).is_some() { + return return_synchronous_actions(Ok((signers, signer_state, actions))); + } + + let keystore_account = values + .get_expected_string(KEYSTORE_ACCOUNT) + .map_err(|e| (signers.clone(), signer_state.clone(), e))?; + let keystore_path = values.get_string(KEYSTORE_PATH); + + let resolved_path = resolve_keystore_path(keystore_account, keystore_path) + .map_err(|e| (signers.clone(), signer_state.clone(), diagnosed_error!("{e}")))?; + + signer_state.insert( + "resolved_keystore_path", + Value::string(resolved_path.to_string_lossy().into_owned()), + ); + + // Password is injected by CLI before execution (interactive prompt) + // Password is never stored in the manifest + let password = values.get_string(PASSWORD).ok_or_else(|| { + ( + signers.clone(), + signer_state.clone(), + diagnosed_error!( + "keystore password not provided. This should not happen in unsupervised mode - \ + the password should be prompted interactively before execution." + ), + ) + })?; + + // Password available - decrypt keystore + let signer = keystore_to_secret_key_signer(&resolved_path, password) + .map_err(|e| (signers.clone(), signer_state.clone(), diagnosed_error!("{e}")))?; + + let expected_address: Address = signer.address(); + + signer_state.insert( + "signer_field_bytes", + EvmValue::signer_field_bytes(signer.to_field_bytes().to_vec()), + ); + signer_state.insert("signer_address", Value::string(expected_address.to_string())); + + // In unsupervised mode, we don't need interactive address review + signer_state.insert(CHECKED_ADDRESS, Value::string(expected_address.to_string())); + + let future = async move { Ok((signers, signer_state, actions)) }; + Ok(Box::pin(future)) + } + + fn activate( + _construct_id: &ConstructDid, + _spec: &SignerSpecification, + _values: &ValueStore, + signer_state: ValueStore, + signers: SignersState, + _signers_instances: &HashMap, + _progress_tx: &channel::Sender, + ) -> SignerActivateFutureResult { + let (signers, signer_state, result) = activate_signer(signer_state, signers)?; + return_synchronous_result(Ok((signers, signer_state, result))) + } + + fn check_signability( + construct_did: &ConstructDid, + title: &str, + description: &Option, + meta_description: &Option, + markdown: &Option, + payload: &Value, + _spec: &SignerSpecification, + values: &ValueStore, + signer_state: ValueStore, + signers: SignersState, + _signers_instances: &HashMap, + supervision_context: &RunbookSupervisionContext, + _auth_ctx: &AuthorizationContext, + ) -> Result { + check_signability( + construct_did, + title, + description, + meta_description, + markdown, + payload, + values, + signer_state, + signers, + supervision_context, + ) + } + + fn sign( + caller_uuid: &ConstructDid, + _title: &str, + _payload: &Value, + _spec: &SignerSpecification, + values: &ValueStore, + signer_state: ValueStore, + signers: SignersState, + _signers_instances: &HashMap, + ) -> SignerSignFutureResult { + let caller_uuid = caller_uuid.clone(); + let values = values.clone(); + + let future = async move { sign_transaction(&caller_uuid, &values, signer_state, signers).await }; + Ok(Box::pin(future)) + } +} diff --git a/addons/evm/src/signers/mod.rs b/addons/evm/src/signers/mod.rs index e4b3e11f9..73d1f6441 100644 --- a/addons/evm/src/signers/mod.rs +++ b/addons/evm/src/signers/mod.rs @@ -1,13 +1,18 @@ use txtx_addon_kit::types::signers::SignerSpecification; pub mod common; +mod keystore; mod secret_key; mod web_wallet; +use keystore::EVM_KEYSTORE_SIGNER; use secret_key::EVM_SECRET_KEY_SIGNER; use web_wallet::EVM_WEB_WALLET; lazy_static! { - pub static ref WALLETS: Vec = - vec![EVM_SECRET_KEY_SIGNER.clone(), EVM_WEB_WALLET.clone()]; + pub static ref WALLETS: Vec = vec![ + EVM_SECRET_KEY_SIGNER.clone(), + EVM_KEYSTORE_SIGNER.clone(), + EVM_WEB_WALLET.clone(), + ]; } diff --git a/addons/evm/src/signers/secret_key.rs b/addons/evm/src/signers/secret_key.rs index 6313a9203..02158b50a 100644 --- a/addons/evm/src/signers/secret_key.rs +++ b/addons/evm/src/signers/secret_key.rs @@ -1,41 +1,22 @@ -use alloy::network::{EthereumWallet, TransactionBuilder}; use alloy::primitives::Address; -use alloy_rpc_types::TransactionRequest; use std::collections::HashMap; use txtx_addon_kit::channel; -use txtx_addon_kit::constants::SIGNATURE_APPROVED; -use txtx_addon_kit::types::commands::CommandExecutionResult; -use txtx_addon_kit::types::frontend::{ - ActionItemStatus, ProvideSignedTransactionRequest, ReviewInputRequest, -}; -use txtx_addon_kit::types::frontend::{Actions, BlockEvent}; +use txtx_addon_kit::constants::DESCRIPTION; +use txtx_addon_kit::types::frontend::{Actions, BlockEvent, ReviewInputRequest}; use txtx_addon_kit::types::signers::{ - return_synchronous_result, CheckSignabilityOk, SignerActionErr, SignerActionsFutureResult, - SignerActivateFutureResult, SignerInstance, SignerSignFutureResult, + return_synchronous_actions, return_synchronous_result, CheckSignabilityOk, SignerActionErr, + SignerActionsFutureResult, SignerActivateFutureResult, SignerImplementation, SignerInstance, + SignerSignFutureResult, SignerSpecification, SignersState, }; -use txtx_addon_kit::types::signers::{SignerImplementation, SignerSpecification}; use txtx_addon_kit::types::stores::ValueStore; -use txtx_addon_kit::types::AuthorizationContext; -use txtx_addon_kit::types::{ - commands::CommandSpecification, - diagnostics::Diagnostic, - types::{Type, Value}, -}; -use txtx_addon_kit::types::{ - signers::SignersState, types::RunbookSupervisionContext, ConstructDid, -}; +use txtx_addon_kit::types::types::{RunbookSupervisionContext, Type, Value}; +use txtx_addon_kit::types::commands::CommandSpecification; +use txtx_addon_kit::types::{diagnostics::Diagnostic, AuthorizationContext, ConstructDid}; -use crate::codec::crypto::field_bytes_to_secret_key_signer; -use crate::constants::{ - ACTION_ITEM_CHECK_ADDRESS, ACTION_ITEM_PROVIDE_SIGNED_TRANSACTION, CHAIN_ID, - FORMATTED_TRANSACTION, NAMESPACE, RPC_API_URL, SECRET_KEY_WALLET_UNSIGNED_TRANSACTION_BYTES, - TX_HASH, -}; -use crate::rpc::EvmWalletRpc; +use super::common::{activate_signer, check_signability, sign_transaction}; +use crate::codec::crypto::{mnemonic_to_secret_key_signer, secret_key_to_secret_key_signer}; +use crate::constants::{ACTION_ITEM_CHECK_ADDRESS, CHECKED_ADDRESS, PUBLIC_KEYS}; use crate::typing::EvmValue; -use txtx_addon_kit::types::signers::return_synchronous_actions; - -use crate::constants::PUBLIC_KEYS; lazy_static! { pub static ref EVM_SECRET_KEY_SIGNER: SignerSpecification = define_signer! { @@ -124,22 +105,16 @@ impl SignerImplementation for EvmSecretKeySigner { signers: SignersState, _signers_instances: &HashMap, supervision_context: &RunbookSupervisionContext, - auth_ctx: &txtx_addon_kit::types::AuthorizationContext, + auth_ctx: &AuthorizationContext, _is_balance_check_required: bool, _is_public_key_required: bool, ) -> SignerActionsFutureResult { - use txtx_addon_kit::constants::DESCRIPTION; - - use crate::{ - codec::crypto::{mnemonic_to_secret_key_signer, secret_key_to_secret_key_signer}, - constants::CHECKED_ADDRESS, - }; - let mut actions = Actions::none(); if signer_state.get_value(PUBLIC_KEYS).is_some() { return return_synchronous_actions(Ok((signers, signer_state, actions))); } + let description = values.get_string(DESCRIPTION).map(|d| d.to_string()); let markdown = values .get_markdown(auth_ctx) @@ -162,15 +137,15 @@ impl SignerImplementation for EvmSecretKeySigner { let expected_address: Address = expected_signer.address(); + signer_state.insert( + "signer_field_bytes", + EvmValue::signer_field_bytes(expected_signer.to_field_bytes().to_vec()), + ); + signer_state.insert("signer_address", Value::string(expected_address.to_string())); + if supervision_context.review_input_values { - if let Ok(_) = signer_state.get_expected_string(CHECKED_ADDRESS) { - signer_state.insert( - "signer_field_bytes", - EvmValue::signer_field_bytes(expected_signer.to_field_bytes().to_vec()), - ); - signer_state.insert("signer_address", Value::string(expected_address.to_string())); - signer_state.insert(CHECKED_ADDRESS, Value::string(expected_address.to_string())); - } else { + // Only show review action if address hasn't been checked yet + if signer_state.get_expected_string(CHECKED_ADDRESS).is_err() { actions.push_sub_group( None, vec![ReviewInputRequest::new("", &Value::string(expected_address.to_string())) @@ -184,12 +159,8 @@ impl SignerImplementation for EvmSecretKeySigner { } } else { signer_state.insert(CHECKED_ADDRESS, Value::string(expected_address.to_string())); - signer_state.insert( - "signer_field_bytes", - EvmValue::signer_field_bytes(expected_signer.to_field_bytes().to_vec()), - ); - signer_state.insert("signer_address", Value::string(expected_address.to_string())); } + let future = async move { Ok((signers, signer_state, actions)) }; Ok(Box::pin(future)) } @@ -203,11 +174,7 @@ impl SignerImplementation for EvmSecretKeySigner { _signers_instances: &HashMap, _progress_tx: &channel::Sender, ) -> SignerActivateFutureResult { - let mut result = CommandExecutionResult::new(); - let address = signer_state - .get_expected_value("signer_address") - .map_err(|e| (signers.clone(), signer_state.clone(), e))?; - result.outputs.insert("address".into(), address.clone()); + let (signers, signer_state, result) = activate_signer(signer_state, signers)?; return_synchronous_result(Ok((signers, signer_state, result))) } @@ -226,47 +193,18 @@ impl SignerImplementation for EvmSecretKeySigner { supervision_context: &RunbookSupervisionContext, _auth_ctx: &AuthorizationContext, ) -> Result { - let actions = if supervision_context.review_input_values { - let construct_did_str = &construct_did.to_string(); - if let Some(_) = signer_state.get_scoped_value(&construct_did_str, SIGNATURE_APPROVED) { - return Ok((signers, signer_state, Actions::none())); - } - - let chain_id = values - .get_expected_uint(CHAIN_ID) - .map_err(|e| (signers.clone(), signer_state.clone(), e))?; - - let status = ActionItemStatus::Todo; - - let formatted_payload = - signer_state.get_scoped_value(&construct_did_str, FORMATTED_TRANSACTION); - - let request = ProvideSignedTransactionRequest::new( - &signer_state.uuid, - &payload, - NAMESPACE, - &chain_id.to_string(), - ) - .check_expectation_action_uuid(construct_did) - .only_approval_needed() - .formatted_payload(formatted_payload) - .to_action_type() - .to_request(title, ACTION_ITEM_PROVIDE_SIGNED_TRANSACTION) - .with_construct_did(construct_did) - .with_some_description(description.clone()) - .with_some_meta_description(meta_description.clone()) - .with_some_markdown(markdown.clone()) - .with_status(status); - - Actions::append_item( - request, - Some("Review and sign the transactions from the list below"), - Some("Transaction Signing"), - ) - } else { - Actions::none() - }; - Ok((signers, signer_state, actions)) + check_signability( + construct_did, + title, + description, + meta_description, + markdown, + payload, + values, + signer_state, + signers, + supervision_context, + ) } fn sign( @@ -282,54 +220,7 @@ impl SignerImplementation for EvmSecretKeySigner { let caller_uuid = caller_uuid.clone(); let values = values.clone(); - let future = async move { - let mut result = CommandExecutionResult::new(); - - let rpc_api_url = values - .get_expected_string(RPC_API_URL) - .map_err(|e| (signers.clone(), signer_state.clone(), e))?; - - let signer_field_bytes = signer_state - .get_expected_buffer_bytes("signer_field_bytes") - .map_err(|e| (signers.clone(), signer_state.clone(), e))?; - - let payload_bytes = signer_state - .get_expected_scoped_buffer_bytes( - &caller_uuid.to_string(), - SECRET_KEY_WALLET_UNSIGNED_TRANSACTION_BYTES, - ) - .unwrap(); - - let secret_key_signer = field_bytes_to_secret_key_signer(&signer_field_bytes) - .map_err(|e| (signers.clone(), signer_state.clone(), diagnosed_error!("{e}")))?; - - let eth_signer = EthereumWallet::from(secret_key_signer); - - let mut tx: TransactionRequest = serde_json::from_slice(&payload_bytes).unwrap(); - if tx.to.is_none() { - // there's no to address on the tx, which is invalid unless it's set as "create" - tx.set_create(); - } - - let tx_envelope = tx.build(ð_signer).await.map_err(|e| { - ( - signers.clone(), - signer_state.clone(), - diagnosed_error!("failed to build transaction envelope: {e}"), - ) - })?; - - let rpc = EvmWalletRpc::new(&rpc_api_url, eth_signer) - .map_err(|e| (signers.clone(), signer_state.clone(), diagnosed_error!("{e}")))?; - - let tx_hash = rpc.sign_and_send_tx(tx_envelope).await.map_err(|e| { - (signers.clone(), signer_state.clone(), diagnosed_error!("{}", e.to_string())) - })?; - - result.outputs.insert(TX_HASH.to_string(), EvmValue::tx_hash(tx_hash.to_vec())); - - Ok((signers, signer_state, result)) - }; + let future = async move { sign_transaction(&caller_uuid, &values, signer_state, signers).await }; Ok(Box::pin(future)) } } diff --git a/crates/txtx-addon-kit/src/types/signers.rs b/crates/txtx-addon-kit/src/types/signers.rs index 287800b5f..fbf4d4aff 100644 --- a/crates/txtx-addon-kit/src/types/signers.rs +++ b/crates/txtx-addon-kit/src/types/signers.rs @@ -417,7 +417,18 @@ impl SignerInstance { } } } - + ActionItemResponseType::ProvideInput(response) => { + let Some(requests) = action_item_requests else { continue }; + let Some(request) = requests.iter().find(|r| r.id.eq(&action_item_id)) + else { + continue; + }; + let Some(signer_did) = &request.construct_did else { continue }; + + let mut signer_state = signers.pop_signer_state(signer_did).unwrap(); + signer_state.insert(&response.input_name, response.updated_value.clone()); + signers.push_signer_state(signer_state); + } _ => {} } } diff --git a/crates/txtx-cli/src/cli/lint/validator.rs b/crates/txtx-cli/src/cli/lint/validator.rs index a59a153be..94658754c 100644 --- a/crates/txtx-cli/src/cli/lint/validator.rs +++ b/crates/txtx-cli/src/cli/lint/validator.rs @@ -5,6 +5,7 @@ use std::path::PathBuf; use txtx_core::validation::{ValidationResult, Diagnostic}; use txtx_core::manifest::WorkspaceManifest; use txtx_addon_kit::helpers::fs::FileLocation; +use txtx_addon_network_evm::codec::crypto::resolve_keystore_path; use crate::cli::common::addon_registry; use super::config::LinterConfig; @@ -133,10 +134,12 @@ impl Linter { file_path, addon_specs, ) { - Ok(input_refs) => { + Ok(refs) => { if let Some(ref manifest) = manifest { - self.validate_with_rules(&input_refs, content, file_path, manifest, environment, &mut result); + self.validate_with_rules(&refs.inputs, content, file_path, manifest, environment, &mut result); } + // Validate signers (e.g., keystore paths) + self.validate_signers(&refs.signers, file_path, &mut result); } Err(e) => { result.errors.push( @@ -275,6 +278,55 @@ impl Linter { let formatter = super::formatter::get_formatter(self.config.format); formatter.format(&result); } + + /// Validate signers, specifically keystore signers for path validity + fn validate_signers( + &self, + signers: &[txtx_core::validation::LocatedSignerRef], + file_path: &str, + result: &mut ValidationResult, + ) { + for signer in signers.iter().filter(|s| s.signer_type == "evm::keystore") { + let Some(keystore_account) = signer.attributes.get("keystore_account") else { + continue; // Missing required attribute - caught by HCL validation + }; + + let keystore_path = signer.attributes.get("keystore_path").map(|s| s.as_str()); + + match resolve_keystore_path(keystore_account, keystore_path) { + Ok(resolved_path) if !resolved_path.exists() => { + let location_hint = keystore_path + .map(|p| format!("looked in directory '{}'", p)) + .unwrap_or_else(|| "looked in ~/.foundry/keystores".to_string()); + + result.warnings.push( + Diagnostic::warning(format!( + "keystore file not found: '{}' ({})", + keystore_account, location_hint + )) + .with_code("keystore-not-found".to_string()) + .with_file(file_path.to_string()) + .with_line(signer.line) + .with_column(signer.column) + .with_suggestion(format!( + "Ensure the keystore file exists. Create one with: cast wallet import {} --interactive", + keystore_account + )) + ); + } + Ok(_) => {} // File exists, all good + Err(e) => { + result.errors.push( + Diagnostic::error(format!("invalid keystore configuration: {}", e)) + .with_code("keystore-invalid".to_string()) + .with_file(file_path.to_string()) + .with_line(signer.line) + .with_column(signer.column) + ); + } + } + } + } } #[cfg(test)] @@ -917,4 +969,181 @@ action "test" { "Warning should have code: {:?}", warning.message); } } + + mod keystore_linting_tests { + use super::*; + use tempfile::TempDir; + use std::fs; + + fn minimal_manifest() -> WorkspaceManifest { + WorkspaceManifest { + name: "test".to_string(), + id: "test-id".to_string(), + runbooks: vec![], + environments: Default::default(), + location: None, + } + } + + // TODO: This local builder is a workaround. The proper fix is to extend + // RunbookBuilder in txtx-test-utils with a `validate_with_cli_linter()` method + // that uses the full CLI Linter instead of just hcl_validator. + struct SignerBuilder { + name: String, + signer_type: String, + attributes: Vec<(String, String)>, + } + + impl SignerBuilder { + fn new(name: &str, signer_type: &str) -> Self { + Self { + name: name.to_string(), + signer_type: signer_type.to_string(), + attributes: Vec::new(), + } + } + + fn keystore(name: &str) -> Self { + Self::new(name, "evm::keystore") + } + + fn web_wallet(name: &str) -> Self { + Self::new(name, "evm::web_wallet") + } + + fn attr(mut self, key: &str, value: &str) -> Self { + self.attributes.push((key.to_string(), value.to_string())); + self + } + + fn keystore_account(self, account: &str) -> Self { + self.attr("keystore_account", account) + } + + fn keystore_path(self, path: &str) -> Self { + self.attr("keystore_path", path) + } + + fn expected_address(self, address: &str) -> Self { + self.attr("expected_address", address) + } + + fn build(&self) -> String { + let attrs = self.attributes + .iter() + .map(|(k, v)| format!(" {} = \"{}\"", k, v)) + .collect::>() + .join("\n"); + + format!( + "signer \"{}\" \"{}\" {{\n{}\n}}", + self.name, self.signer_type, attrs + ) + } + + fn validate(&self) -> ValidationResult { + let linter = Linter::with_defaults(); + linter.validate_content(&self.build(), "test.tx", minimal_manifest(), None) + } + } + + #[test] + fn test_keystore_signer_with_existing_file() { + // Arrange + let temp_dir = TempDir::new().unwrap(); + let keystore_file = temp_dir.path().join("myaccount.json"); + fs::write(&keystore_file, "{}").unwrap(); + + // Act + let result = SignerBuilder::keystore("deployer") + .keystore_account("myaccount") + .keystore_path(&temp_dir.path().display().to_string()) + .validate(); + + // Assert + let keystore_warnings: Vec<_> = result.warnings.iter() + .filter(|w| w.code.as_ref().is_some_and(|c| c.contains("keystore"))) + .collect(); + assert!(keystore_warnings.is_empty(), + "Should not warn about existing keystore file: {:?}", keystore_warnings); + } + + #[test] + fn test_keystore_signer_with_missing_file() { + // Arrange + let temp_dir = TempDir::new().unwrap(); + let nonexistent_path = temp_dir.path().join("nonexistent"); + + // Act + let result = SignerBuilder::keystore("deployer") + .keystore_account("myaccount") + .keystore_path(&nonexistent_path.display().to_string()) + .validate(); + + // Assert + let keystore_warnings: Vec<_> = result.warnings.iter() + .filter(|w| w.code.as_deref() == Some("keystore-not-found")) + .collect(); + assert_eq!(keystore_warnings.len(), 1, + "Should warn about missing keystore file"); + assert!(keystore_warnings[0].message.contains("keystore file not found"), + "Warning should mention 'keystore file not found': {:?}", keystore_warnings[0].message); + } + + #[test] + fn test_keystore_signer_absolute_path_without_json_extension() { + // Arrange & Act + let result = SignerBuilder::keystore("deployer") + .keystore_account("/some/absolute/path/without/extension") + .validate(); + + // Assert + let keystore_errors: Vec<_> = result.errors.iter() + .filter(|e| e.code.as_deref() == Some("keystore-invalid")) + .collect(); + assert_eq!(keystore_errors.len(), 1, + "Should error on absolute path without .json extension"); + assert!(keystore_errors[0].message.contains(".json extension"), + "Error should mention .json extension: {:?}", keystore_errors[0].message); + } + + #[test] + fn test_keystore_signer_includes_suggestion() { + // Arrange + let temp_dir = TempDir::new().unwrap(); + + // Act + let result = SignerBuilder::keystore("deployer") + .keystore_account("myaccount") + .keystore_path(&temp_dir.path().display().to_string()) + .validate(); + + // Assert + let keystore_warnings: Vec<_> = result.warnings.iter() + .filter(|w| w.code.as_deref() == Some("keystore-not-found")) + .collect(); + assert_eq!(keystore_warnings.len(), 1); + assert!(keystore_warnings[0].suggestion.is_some(), + "Warning should include a suggestion"); + let suggestion = keystore_warnings[0].suggestion.as_ref().unwrap(); + assert!(suggestion.contains("cast wallet import"), + "Suggestion should mention 'cast wallet import': {:?}", suggestion); + } + + #[test] + fn test_non_keystore_signer_not_validated() { + // Arrange & Act + let result = SignerBuilder::web_wallet("deployer") + .expected_address("0x1234567890123456789012345678901234567890") + .validate(); + + // Assert + let keystore_issues: Vec<_> = result.warnings.iter() + .chain(result.errors.iter()) + .filter(|d| d.code.as_ref().is_some_and(|c| c.contains("keystore"))) + .collect(); + assert!(keystore_issues.is_empty(), + "Non-keystore signers should not trigger keystore validation"); + } + } } diff --git a/crates/txtx-cli/src/cli/runbooks/mod.rs b/crates/txtx-cli/src/cli/runbooks/mod.rs index e85dcebc9..2be86c1c5 100644 --- a/crates/txtx-cli/src/cli/runbooks/mod.rs +++ b/crates/txtx-cli/src/cli/runbooks/mod.rs @@ -1,8 +1,9 @@ use super::{env::TxtxEnv, CheckRunbook, Context, CreateRunbook, ExecuteRunbook, ListRunbooks}; use crate::{get_addon_by_namespace, get_available_addons}; +use txtx_addon_network_evm::codec::crypto::{keystore_to_secret_key_signer, resolve_keystore_path}; use ascii_table::AsciiTable; use console::Style; -use dialoguer::{theme::ColorfulTheme, Confirm, Input, Select}; +use dialoguer::{theme::ColorfulTheme, Confirm, Input, Password, Select}; use indicatif::{MultiProgress, ProgressBar, ProgressStyle}; use itertools::Itertools; use log::{debug, error, info, trace, warn}; @@ -17,7 +18,7 @@ use std::{ use tokio::sync::RwLock; use txtx_cloud::router::TxtxAuthenticatedCloudServiceRouter; use txtx_core::{ - kit::types::{commands::UnevaluatedInputsMap, stores::ValueStore}, + kit::types::{commands::UnevaluatedInputsMap, stores::{ValueMap, ValueStore}}, mustache, templates::{TXTX_MANIFEST_TEMPLATE, TXTX_README_TEMPLATE}, utils::try_write_outputs_to_file, @@ -684,6 +685,11 @@ pub async fn handle_run_command( setup_logger(runbook.to_instance_context(), &cmd.log_level).unwrap(); let log_filter: LogLevel = cmd.log_level.as_str().into(); + // Prompt for keystore passwords before unsupervised execution + if is_execution_unsupervised { + prompt_for_keystore_passwords(&mut runbook)?; + } + // should not be generating actions if is_execution_unsupervised { let _ = hiro_system_kit::thread_named("Display background tasks logs").spawn(move || { @@ -1274,3 +1280,372 @@ fn handle_log_event( }, } } + +/// Trait for providing passwords for keystore signers. +/// This abstraction enables testing without interactive prompts. +trait PasswordProvider { + fn get_password( + &mut self, + keystore_account: &str, + keystore_path: Option<&str>, + ) -> Result; +} + +/// Default password provider using dialoguer for interactive prompts. +struct DialoguerPasswordProvider; + +impl PasswordProvider for DialoguerPasswordProvider { + fn get_password( + &mut self, + keystore_account: &str, + _keystore_path: Option<&str>, + ) -> Result { + Password::with_theme(&ColorfulTheme::default()) + .with_prompt(format!("Enter password for keystore '{}'", keystore_account)) + .interact() + .map_err(|e| format!("Failed to read password: {}", e)) + } +} + +/// Prompts for passwords for all keystore signers in the runbook. +/// This is called before unsupervised execution to gather passwords interactively. +/// Password is never stored in the manifest - always prompted securely. +/// If the same keystore account appears in multiple flows, only prompts once. +fn prompt_for_keystore_passwords(runbook: &mut Runbook) -> Result<(), String> { + prompt_for_keystore_passwords_with_provider(runbook, &mut DialoguerPasswordProvider) +} + +/// Cached password resolver that prompts only once per unique (account, path) pair. +/// Returns the password for each request, using cached values when available. +struct CachedPasswordResolver<'a, P: PasswordProvider> { + provider: &'a mut P, + cache: std::collections::HashMap<(String, Option), String>, +} + +impl<'a, P: PasswordProvider> CachedPasswordResolver<'a, P> { + fn new(provider: &'a mut P) -> Self { + Self { + provider, + cache: std::collections::HashMap::new(), + } + } + + /// Get password for the given account/path, prompting only if not cached. + fn get_password( + &mut self, + keystore_account: &str, + keystore_path: Option<&str>, + ) -> Result { + let key = (keystore_account.to_string(), keystore_path.map(String::from)); + + if let Some(password) = self.cache.get(&key) { + return Ok(password.clone()); + } + + let password = self.provider.get_password(keystore_account, keystore_path)?; + self.cache.insert(key, password.clone()); + Ok(password) + } +} + +/// Extracts a string literal value from an HCL attribute. +/// Returns None if the attribute value is not a simple string literal. +/// +/// This uses `.value()` to get the decoded string content, which properly handles: +/// - Embedded quotes: `"my\"account"` -> `my"account` +/// - Unicode escapes: `"\u0041"` -> `A` +/// - Other escape sequences +/// +/// This is preferred over `attr.value.to_string().trim_matches('"')` which is fragile +/// and fails on strings with embedded quotes or other edge cases. +/// +/// Note: A similar helper `extract_string_value` exists in block_processors.rs but is +/// private to that module. If this pattern is needed more broadly, consider making +/// that function public and exporting it from txtx-core, or promoting to a shared +/// helper in txtx-addon-kit/helpers/hcl.rs. +fn get_string_literal(attr: &txtx_addon_kit::hcl::structure::Attribute) -> Option { + use txtx_addon_kit::hcl::expr::Expression; + match &attr.value { + Expression::String(s) => Some(s.value().to_string()), + _ => None, + } +} + +/// Internal implementation that accepts a password provider for testability. +fn prompt_for_keystore_passwords_with_provider( + runbook: &mut Runbook, + provider: &mut P, +) -> Result<(), String> { + let mut resolver = CachedPasswordResolver::new(provider); + + for flow_context in runbook.flow_contexts.iter_mut() { + // Collect keystore signer info first (immutable borrow of signers_instances) + let keystore_entries: Vec<_> = flow_context + .execution_context + .signers_instances + .iter() + .filter(|(_, signer)| signer.specification.matcher == "keystore") + .map(|(construct_did, signer)| { + let keystore_account = signer + .block + .body + .get_attribute("keystore_account") + .and_then(get_string_literal) + .unwrap_or_else(|| signer.name.clone()); + + let keystore_path = signer + .block + .body + .get_attribute("keystore_path") + .and_then(get_string_literal); + + (construct_did.clone(), signer.name.clone(), keystore_account, keystore_path) + }) + .collect(); + + // Now prompt for passwords, validate them, and update evaluation results (mutable borrow) + for (construct_did, signer_name, keystore_account, keystore_path) in keystore_entries { + let password = resolver.get_password(&keystore_account, keystore_path.as_deref())?; + + // Validate password immediately by attempting to decrypt the keystore. + // The returned signer is discarded; its key material is automatically zeroed + // on drop via ZeroizeOnDrop implemented by the underlying SigningKey. + let resolved_path = resolve_keystore_path(&keystore_account, keystore_path.as_deref())?; + let _ = keystore_to_secret_key_signer(&resolved_path, &password)?; + + let eval_result = flow_context + .execution_context + .commands_inputs_evaluation_results + .entry(construct_did) + .or_insert_with(|| { + CommandInputsEvaluationResult::new(&signer_name, &ValueMap::new()) + }); + + eval_result.inputs.insert("password", Value::string(password)); + } + } + + Ok(()) +} + +#[cfg(test)] +mod keystore_password_tests { + use super::*; + use std::collections::HashMap; + + /// Mock password provider for testing the password caching logic + struct MockPasswordProvider { + passwords: HashMap<(String, Option), String>, + requests: Vec<(String, Option)>, + } + + impl MockPasswordProvider { + fn new(passwords: HashMap<(String, Option), String>) -> Self { + Self { passwords, requests: Vec::new() } + } + + fn request_count(&self) -> usize { + self.requests.len() + } + + fn get_requests(&self) -> &[(String, Option)] { + &self.requests + } + } + + impl PasswordProvider for MockPasswordProvider { + fn get_password( + &mut self, + keystore_account: &str, + keystore_path: Option<&str>, + ) -> Result { + let key = (keystore_account.to_string(), keystore_path.map(String::from)); + self.requests.push(key.clone()); + self.passwords + .get(&key) + .cloned() + .ok_or_else(|| format!("No password for '{}'", keystore_account)) + } + } + + #[test] + fn test_mock_provider_returns_configured_password() { + let mut passwords = HashMap::new(); + passwords.insert(("account1".to_string(), None), "password1".to_string()); + passwords.insert( + ("account2".to_string(), Some("/path".to_string())), + "password2".to_string(), + ); + + let mut provider = MockPasswordProvider::new(passwords); + + assert_eq!( + provider.get_password("account1", None).unwrap(), + "password1" + ); + assert_eq!( + provider.get_password("account2", Some("/path")).unwrap(), + "password2" + ); + assert_eq!(provider.request_count(), 2); + } + + #[test] + fn test_mock_provider_tracks_requests() { + let mut passwords = HashMap::new(); + passwords.insert(("acc".to_string(), None), "pass".to_string()); + + let mut provider = MockPasswordProvider::new(passwords); + + let _ = provider.get_password("acc", None); + let _ = provider.get_password("acc", None); + let _ = provider.get_password("acc", None); + + assert_eq!(provider.request_count(), 3); + } + + #[test] + fn test_mock_provider_missing_password_returns_error() { + let provider_passwords = HashMap::new(); + let mut provider = MockPasswordProvider::new(provider_passwords); + + let result = provider.get_password("unknown", None); + assert!(result.is_err()); + assert!(result.unwrap_err().contains("unknown")); + } + + #[test] + fn test_mock_provider_different_paths_are_different_keys() { + let mut passwords = HashMap::new(); + passwords.insert(("acc".to_string(), Some("/a".to_string())), "pass_a".to_string()); + passwords.insert(("acc".to_string(), Some("/b".to_string())), "pass_b".to_string()); + + let mut provider = MockPasswordProvider::new(passwords); + + assert_eq!(provider.get_password("acc", Some("/a")).unwrap(), "pass_a"); + assert_eq!(provider.get_password("acc", Some("/b")).unwrap(), "pass_b"); + + // Same account with no path should fail + assert!(provider.get_password("acc", None).is_err()); + } + + #[test] + fn test_dialoguer_provider_struct_exists() { + // Verify the DialoguerPasswordProvider can be instantiated + let _provider = DialoguerPasswordProvider; + } + + // ==================== CachedPasswordResolver Tests ==================== + // These tests prove that the caching logic only prompts once per unique key + + #[test] + fn test_cached_resolver_same_account_prompts_once() { + // Simulates: same keystore account in 2 different flows + let mut passwords = HashMap::new(); + passwords.insert(("deployer".to_string(), None), "secret123".to_string()); + let mut provider = MockPasswordProvider::new(passwords); + + let mut resolver = CachedPasswordResolver::new(&mut provider); + + // First flow requests password for "deployer" + let pw1 = resolver.get_password("deployer", None).unwrap(); + // Second flow requests password for same "deployer" + let pw2 = resolver.get_password("deployer", None).unwrap(); + + assert_eq!(pw1, "secret123"); + assert_eq!(pw2, "secret123"); + // Provider should only have been called ONCE + assert_eq!(provider.request_count(), 1); + } + + #[test] + fn test_cached_resolver_different_accounts_prompt_separately() { + let mut passwords = HashMap::new(); + passwords.insert(("alice".to_string(), None), "alice_pw".to_string()); + passwords.insert(("bob".to_string(), None), "bob_pw".to_string()); + let mut provider = MockPasswordProvider::new(passwords); + + let mut resolver = CachedPasswordResolver::new(&mut provider); + + let pw_alice = resolver.get_password("alice", None).unwrap(); + let pw_bob = resolver.get_password("bob", None).unwrap(); + + assert_eq!(pw_alice, "alice_pw"); + assert_eq!(pw_bob, "bob_pw"); + // Two different accounts = two prompts + assert_eq!(provider.request_count(), 2); + } + + #[test] + fn test_cached_resolver_same_account_different_paths_prompt_separately() { + // Same account name but different keystore_path = different keystores + let mut passwords = HashMap::new(); + passwords.insert( + ("deployer".to_string(), Some("/keys/prod".to_string())), + "prod_pw".to_string(), + ); + passwords.insert( + ("deployer".to_string(), Some("/keys/dev".to_string())), + "dev_pw".to_string(), + ); + let mut provider = MockPasswordProvider::new(passwords); + + let mut resolver = CachedPasswordResolver::new(&mut provider); + + let pw_prod = resolver.get_password("deployer", Some("/keys/prod")).unwrap(); + let pw_dev = resolver.get_password("deployer", Some("/keys/dev")).unwrap(); + + assert_eq!(pw_prod, "prod_pw"); + assert_eq!(pw_dev, "dev_pw"); + // Different paths = different keystores = two prompts + assert_eq!(provider.request_count(), 2); + } + + #[test] + fn test_cached_resolver_multiple_flows_same_signer_prompts_once() { + // Simulates: 3 flows all using the same keystore signer + let mut passwords = HashMap::new(); + passwords.insert(("shared_signer".to_string(), None), "shared_pw".to_string()); + let mut provider = MockPasswordProvider::new(passwords); + + let mut resolver = CachedPasswordResolver::new(&mut provider); + + // Flow 1 + let pw1 = resolver.get_password("shared_signer", None).unwrap(); + // Flow 2 + let pw2 = resolver.get_password("shared_signer", None).unwrap(); + // Flow 3 + let pw3 = resolver.get_password("shared_signer", None).unwrap(); + + assert_eq!(pw1, "shared_pw"); + assert_eq!(pw2, "shared_pw"); + assert_eq!(pw3, "shared_pw"); + // Only ONE prompt despite 3 flows + assert_eq!(provider.request_count(), 1); + } + + #[test] + fn test_cached_resolver_mixed_signers_correct_prompt_count() { + // Complex scenario: 2 flows, each with 2 signers + // Flow 1: alice, bob + // Flow 2: alice, charlie + // Expected: 3 prompts (alice cached on second use) + let mut passwords = HashMap::new(); + passwords.insert(("alice".to_string(), None), "alice_pw".to_string()); + passwords.insert(("bob".to_string(), None), "bob_pw".to_string()); + passwords.insert(("charlie".to_string(), None), "charlie_pw".to_string()); + let mut provider = MockPasswordProvider::new(passwords); + + let mut resolver = CachedPasswordResolver::new(&mut provider); + + // Flow 1 + resolver.get_password("alice", None).unwrap(); + resolver.get_password("bob", None).unwrap(); + // Flow 2 + resolver.get_password("alice", None).unwrap(); // Should be cached + resolver.get_password("charlie", None).unwrap(); + + // alice=1, bob=1, charlie=1 = 3 total prompts + assert_eq!(provider.request_count(), 3); + } +} diff --git a/crates/txtx-core/src/runbook/variables.rs b/crates/txtx-core/src/runbook/variables.rs index 1c9809cfa..08e3a84d7 100644 --- a/crates/txtx-core/src/runbook/variables.rs +++ b/crates/txtx-core/src/runbook/variables.rs @@ -104,7 +104,7 @@ impl RunbookVariableIterator { // Run HCL validation to collect input references let mut validation_result = ValidationResult::default(); - let input_refs = validate_with_hcl_and_addons( + let refs = validate_with_hcl_and_addons( &combined_content, &mut validation_result, "runbook", @@ -112,7 +112,7 @@ impl RunbookVariableIterator { )?; // Process collected input references - for input_ref in input_refs { + for input_ref in refs.inputs { let var_name = Self::extract_variable_name(&input_ref.name); // Find which file this reference is in diff --git a/crates/txtx-core/src/validation/context.rs b/crates/txtx-core/src/validation/context.rs index 0979efc4b..921a17a49 100644 --- a/crates/txtx-core/src/validation/context.rs +++ b/crates/txtx-core/src/validation/context.rs @@ -230,17 +230,17 @@ impl ValidationContextExt for ValidationContext { fn validate_hcl(&mut self, result: &mut ValidationResult) -> Result<(), String> { // Delegate to HCL validator if let Some(specs) = self.addon_specs.clone() { - let input_refs = super::hcl_validator::validate_with_hcl_and_addons( + let refs = super::hcl_validator::validate_with_hcl_and_addons( &self.content, result, &self.file_path, specs, )?; - self.input_refs = input_refs; + self.input_refs = refs.inputs; } else { - let input_refs = + let refs = super::hcl_validator::validate_with_hcl(&self.content, result, &self.file_path)?; - self.input_refs = input_refs; + self.input_refs = refs.inputs; } Ok(()) } diff --git a/crates/txtx-core/src/validation/hcl_validator/block_processors.rs b/crates/txtx-core/src/validation/hcl_validator/block_processors.rs index 0c41b64ee..936dbcd96 100644 --- a/crates/txtx-core/src/validation/hcl_validator/block_processors.rs +++ b/crates/txtx-core/src/validation/hcl_validator/block_processors.rs @@ -38,7 +38,7 @@ pub fn process_block( source_mapper: &SourceMapper, ) -> Result, ValidationError> { match block_type { - BlockType::Signer => process_signer(block), + BlockType::Signer => process_signer(block, source_mapper), BlockType::Variable => process_variable(block, source_mapper), BlockType::Output => process_output(block), BlockType::Action => process_action(block, addon_specs, source_mapper), @@ -47,21 +47,42 @@ pub fn process_block( } } -fn process_signer(block: &Block) -> Result, ValidationError> { +fn process_signer(block: &Block, source_mapper: &SourceMapper) -> Result, ValidationError> { let name = block.labels.extract_name() .ok_or(ValidationError::MissingLabel("signer name"))?; let signer_type = block.labels.extract_type() .ok_or(ValidationError::MissingLabel("signer type"))?; + let position = extract_block_position(block, source_mapper); + + // Extract string attributes from the block body + let mut attributes = HashMap::new(); + for attr in block.body.attributes() { + if let Some(value) = extract_string_value(&attr.value) { + attributes.insert(attr.key.to_string(), value); + } + } + Ok(vec![ CollectedItem::Definition(DefinitionItem::Signer { name: name.to_string(), signer_type: signer_type.to_string(), + attributes, + position, }) ]) } +/// Extract a string value from an expression, if it is a simple string literal +fn extract_string_value(expr: &txtx_addon_kit::hcl::expr::Expression) -> Option { + use txtx_addon_kit::hcl::expr::Expression; + match expr { + Expression::String(s) => Some(s.value().to_string()), + _ => None, + } +} + fn process_variable(block: &Block, source_mapper: &SourceMapper) -> Result, ValidationError> { use txtx_addon_kit::hcl::visit::{visit_expr, Visit}; use txtx_addon_kit::hcl::expr::{Expression, TraversalOperator}; diff --git a/crates/txtx-core/src/validation/hcl_validator/visitor.rs b/crates/txtx-core/src/validation/hcl_validator/visitor.rs index 4aeb67e71..6a5f625f0 100644 --- a/crates/txtx-core/src/validation/hcl_validator/visitor.rs +++ b/crates/txtx-core/src/validation/hcl_validator/visitor.rs @@ -28,7 +28,7 @@ use txtx_addon_kit::hcl::{ use crate::runbook::location::{SourceMapper, BlockContext}; use crate::types::ConstructType; -use crate::validation::types::{LocatedInputRef, ValidationResult}; +use crate::validation::types::{HclValidationRefs, LocatedInputRef, LocatedSignerRef, ValidationResult}; use txtx_addon_kit::types::diagnostics::Diagnostic; use crate::kit::types::commands::CommandSpecification; @@ -147,7 +147,12 @@ pub enum CollectedItem { #[derive(Debug)] pub enum DefinitionItem { Variable { name: String, position: Position }, - Signer { name: String, signer_type: String }, + Signer { + name: String, + signer_type: String, + attributes: std::collections::HashMap, + position: Position, + }, Output(String), } @@ -235,6 +240,7 @@ struct Declarations { variables: HashMap, actions: HashMap, flows: HashMap, + signers: HashMap, } struct VariableDeclaration { @@ -252,6 +258,12 @@ struct FlowDeclaration { position: Position, } +struct SignerDeclaration { + signer_type: String, + attributes: HashMap, + position: Position, +} + #[derive(Default)] struct DependencyGraphs { variables: DependencyGraph, @@ -272,8 +284,13 @@ impl ValidationState { self.dependency_graphs.variables.add_node(name.clone(), None); self.declarations.variables.insert(name, VariableDeclaration { position }); } - Signer { name, signer_type } => { - self.definitions.signers.insert(name, signer_type); + Signer { name, signer_type, attributes, position } => { + self.definitions.signers.insert(name.clone(), signer_type.clone()); + self.declarations.signers.insert(name, SignerDeclaration { + signer_type, + attributes, + position, + }); } Output(name) => { self.definitions.outputs.insert(name); @@ -450,7 +467,7 @@ impl<'a> HclValidationVisitor<'a> { } } - pub fn validate(&mut self, body: &Body) -> Vec { + pub fn validate(&mut self, body: &Body) -> HclValidationRefs { // Phase 1: Collection (functional approach) self.collect_definitions(body); @@ -466,7 +483,22 @@ impl<'a> HclValidationVisitor<'a> { // Validate flow inputs after references are collected self.validate_all_flow_inputs(); - std::mem::take(&mut self.state.input_refs) + // Collect signer refs from declarations + let signer_refs: Vec = self.state.declarations.signers + .iter() + .map(|(name, decl)| LocatedSignerRef { + name: name.clone(), + signer_type: decl.signer_type.clone(), + attributes: decl.attributes.clone(), + line: decl.position.line, + column: decl.position.column, + }) + .collect(); + + HclValidationRefs { + inputs: std::mem::take(&mut self.state.input_refs), + signers: signer_refs, + } } fn collect_definitions(&mut self, body: &Body) { @@ -1000,7 +1032,7 @@ impl<'a> BasicHclValidator<'a> { Self { result, file_path, source } } - pub fn validate(&mut self, body: &Body) -> Vec { + pub fn validate(&mut self, body: &Body) -> HclValidationRefs { // Create empty specs inline - no self-reference needed let empty_specs = HashMap::new(); let mut validator = HclValidationVisitor::new( @@ -1023,7 +1055,7 @@ impl<'a> FullHclValidator<'a> { Self { result, file_path, source, addon_specs } } - pub fn validate(&mut self, body: &Body) -> Vec { + pub fn validate(&mut self, body: &Body) -> HclValidationRefs { let mut validator = HclValidationVisitor::new( self.result, self.file_path, @@ -1038,7 +1070,7 @@ pub fn validate_with_hcl( content: &str, result: &mut ValidationResult, file_path: &str, -) -> Result, String> { +) -> Result { let body: Body = content.parse().map_err(|e| format!("Failed to parse: {}", e))?; let mut validator = BasicHclValidator::new(result, file_path, content); Ok(validator.validate(&body)) @@ -1049,7 +1081,7 @@ pub fn validate_with_hcl_and_addons( result: &mut ValidationResult, file_path: &str, addon_specs: HashMap>, -) -> Result, String> { +) -> Result { let body: Body = content.parse().map_err(|e| format!("Failed to parse: {}", e))?; let mut validator = FullHclValidator::new(result, file_path, content, addon_specs); Ok(validator.validate(&body)) diff --git a/crates/txtx-core/src/validation/mod.rs b/crates/txtx-core/src/validation/mod.rs index eeda17690..d3b49f8e5 100644 --- a/crates/txtx-core/src/validation/mod.rs +++ b/crates/txtx-core/src/validation/mod.rs @@ -27,7 +27,7 @@ pub use manifest_validator::{ pub use rule_id::{AddonScope, CoreRuleId, RuleIdentifier}; pub use file_boundary::FileBoundaryMap; pub use types::{ - LocatedInputRef, ValidationResult, ValidationSuggestion, + HclValidationRefs, LocatedInputRef, LocatedSignerRef, ValidationResult, ValidationSuggestion, }; pub use txtx_addon_kit::types::diagnostics::Diagnostic; pub use validator::{validate_runbook, ValidatorConfig}; diff --git a/crates/txtx-core/src/validation/types.rs b/crates/txtx-core/src/validation/types.rs index 2c9a54504..3d21f695f 100644 --- a/crates/txtx-core/src/validation/types.rs +++ b/crates/txtx-core/src/validation/types.rs @@ -14,6 +14,23 @@ pub struct LocatedInputRef { pub column: usize, } +/// A located signer reference with its type and attributes +#[derive(Debug, Clone)] +pub struct LocatedSignerRef { + pub name: String, + pub signer_type: String, + pub attributes: std::collections::HashMap, + pub line: usize, + pub column: usize, +} + +/// Result of HCL validation containing located references +#[derive(Debug, Clone, Default)] +pub struct HclValidationRefs { + pub inputs: Vec, + pub signers: Vec, +} + #[derive(Debug, Default)] pub struct ValidationResult { pub errors: Vec, diff --git a/crates/txtx-test-utils/src/builders/runbook_builder_enhanced.rs b/crates/txtx-test-utils/src/builders/runbook_builder_enhanced.rs index 000533e1a..5ee8dec09 100644 --- a/crates/txtx-test-utils/src/builders/runbook_builder_enhanced.rs +++ b/crates/txtx-test-utils/src/builders/runbook_builder_enhanced.rs @@ -155,14 +155,14 @@ impl RunbookBuilder { &file_path_str, addon_specs, ) { - Ok(input_refs) => { + Ok(refs) => { // If we have manifest context, validate inputs if let (Some(manifest), Some(env_name)) = (&manifest, &environment) { // Convert CLI inputs from builder let cli_inputs: Vec<(String, String)> = vec![]; validate_inputs_against_manifest( - &input_refs, + &refs.inputs, &content, manifest, Some(env_name),