use super::tempo_transaction::{
    MAX_WEBAUTHN_SIGNATURE_LENGTH, P256_SIGNATURE_LENGTH, SECP256K1_SIGNATURE_LENGTH, SignatureType,
};
use alloc::vec::Vec;
use alloy_primitives::{Address, B256, Bytes, Signature, U256, keccak256, uint};
use base64::{Engine as _, engine::general_purpose::URL_SAFE_NO_PAD};
use sha2::{Digest, Sha256};

// Always mark `p256` as used to avoid `unused_crate_dependencies` warnings in `std` builds.
use p256 as _;

#[cfg(not(feature = "std"))]
use once_cell::race::OnceBox as OnceLock;
#[cfg(feature = "std")]
use std::sync::OnceLock;

/// The P256 (secp256r1/prime256v1) curve order n.
pub const P256_ORDER: U256 =
    uint!(0xFFFFFFFF00000000FFFFFFFFFFFFFFFFBCE6FAADA7179E84F3B9CAC2FC632551_U256);

/// Half of the P256 curve order (n/2).
///
/// For signatures to be valid, the s value must be less than or equal to n/2
/// (low-s requirement). This prevents signature malleability where (r, s) and
/// (r, n-s) are both valid signatures for the same message.
pub const P256N_HALF: U256 =
    uint!(0x7FFFFFFF800000007FFFFFFFFFFFFFFFDE737D56D38BCF4279DCE5617E3192A8_U256);

/// Normalize P256 signature s value to low-s form.
///
/// For any ECDSA signature (r, s), both (r, s) and (r, n-s) are valid.
/// To prevent signature malleability, we require s <= n/2.
/// If s > n/2, we replace it with n - s.
///
/// Returns an error if `s` is zero or `s >= P256_ORDER` (out of range for a
/// valid scalar). This function should be called by all P256 signing code
/// before creating a signature, as the p256 crate does not guarantee low-s
/// signatures.
pub fn normalize_p256_s(s_bytes: &[u8]) -> Result<B256, &'static str> {
    let s = U256::from_be_slice(s_bytes);
    if s.is_zero() || s >= P256_ORDER {
        return Err("P256 s value out of range");
    }
    let normalized_s = if s > P256N_HALF { P256_ORDER - s } else { s };
    Ok(B256::from(normalized_s.to_be_bytes::<32>()))
}

/// Signature type identifiers
/// Note: Secp256k1 has no identifier - detected by length (65 bytes)
pub const SIGNATURE_TYPE_P256: u8 = 0x01;
pub const SIGNATURE_TYPE_WEBAUTHN: u8 = 0x02;
pub const SIGNATURE_TYPE_KEYCHAIN: u8 = 0x03;
pub const SIGNATURE_TYPE_KEYCHAIN_V2: u8 = 0x04;

// Minimum authenticatorData is 37 bytes (32 rpIdHash + 1 flags + 4 signCount)
const MIN_AUTH_DATA_LEN: usize = 37;

/// WebAuthn authenticator data flags (byte 32)
/// ref: <https://www.w3.org/TR/webauthn-2/#sctn-authenticator-data>
const UP: u8 = 0x01; // User Presence (bit 0)
const UV: u8 = 0x04; // User Verified (bit 2)
const AT: u8 = 0x40; // Attested credential data (bit 6)
const ED: u8 = 0x80; // Extension data present (bit 7)

/// P256 signature with pre-hash flag
#[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)]
#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
#[cfg_attr(feature = "serde", serde(rename_all = "camelCase"))]
#[cfg_attr(any(test, feature = "arbitrary"), derive(arbitrary::Arbitrary))]
pub struct P256SignatureWithPreHash {
    pub r: B256,
    pub s: B256,
    pub pub_key_x: B256,
    pub pub_key_y: B256,
    pub pre_hash: bool,
}

/// WebAuthn signature with authenticator data
#[derive(Clone, Debug, PartialEq, Eq, Hash)]
#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
#[cfg_attr(feature = "serde", serde(rename_all = "camelCase"))]
#[cfg_attr(any(test, feature = "arbitrary"), derive(arbitrary::Arbitrary))]
pub struct WebAuthnSignature {
    pub r: B256,
    pub s: B256,
    pub pub_key_x: B256,
    pub pub_key_y: B256,
    /// authenticatorData || clientDataJSON (variable length)
    pub webauthn_data: Bytes,
}

/// Primitive signature types that can be used standalone or within a Keychain signature.
/// This enum contains only the base signature types: Secp256k1, P256, and WebAuthn.
/// It does NOT support Keychain signatures to prevent recursion.
///
/// Note: This enum uses custom RLP encoding via `to_bytes()` and does NOT derive Compact.
/// The Compact encoding is handled at the parent struct level (e.g., KeyAuthorization).
#[derive(Clone, Debug, PartialEq, Eq, Hash)]
#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
#[cfg_attr(feature = "serde", serde(tag = "type", rename_all = "camelCase"))]
#[cfg_attr(any(test, feature = "arbitrary"), derive(arbitrary::Arbitrary))]
pub enum PrimitiveSignature {
    /// Standard secp256k1 ECDSA signature (65 bytes: r, s, v)
    Secp256k1(Signature),

    /// P256 signature with embedded public key (129 bytes)
    P256(P256SignatureWithPreHash),

    /// WebAuthn signature with variable-length authenticator data
    WebAuthn(WebAuthnSignature),
}

impl PrimitiveSignature {
    /// Parse signature from bytes with backward compatibility
    ///
    /// For backward compatibility with existing secp256k1 signatures:
    /// - If length is 65 bytes: treat as secp256k1 signature (no type identifier)
    /// - Otherwise: first byte is the signature type identifier
    pub fn from_bytes(data: &[u8]) -> Result<Self, &'static str> {
        if data.is_empty() {
            return Err("Signature data is empty");
        }

        // Backward compatibility: exactly 65 bytes means secp256k1 without type identifier
        if data.len() == SECP256K1_SIGNATURE_LENGTH {
            let sig = Signature::try_from(data)
                .map_err(|_| "Failed to parse secp256k1 signature: invalid signature values")?;
            return Ok(Self::Secp256k1(sig));
        }

        // For all other lengths, first byte is the type identifier
        if data.len() < 2 {
            return Err("Signature data too short: expected type identifier + signature data");
        }

        let type_id = data[0];
        let sig_data = &data[1..];

        match type_id {
            SIGNATURE_TYPE_P256 => {
                if sig_data.len() != P256_SIGNATURE_LENGTH {
                    return Err("Invalid P256 signature length");
                }
                Ok(Self::P256(P256SignatureWithPreHash {
                    r: B256::from_slice(&sig_data[0..32]),
                    s: B256::from_slice(&sig_data[32..64]),
                    pub_key_x: B256::from_slice(&sig_data[64..96]),
                    pub_key_y: B256::from_slice(&sig_data[96..128]),
                    pre_hash: sig_data[128] != 0,
                }))
            }
            SIGNATURE_TYPE_WEBAUTHN => {
                let len = sig_data.len();
                if !(128..=MAX_WEBAUTHN_SIGNATURE_LENGTH).contains(&len) {
                    return Err("Invalid WebAuthn signature length");
                }
                Ok(Self::WebAuthn(WebAuthnSignature {
                    r: B256::from_slice(&sig_data[len - 128..len - 96]),
                    s: B256::from_slice(&sig_data[len - 96..len - 64]),
                    pub_key_x: B256::from_slice(&sig_data[len - 64..len - 32]),
                    pub_key_y: B256::from_slice(&sig_data[len - 32..]),
                    webauthn_data: Bytes::copy_from_slice(&sig_data[..len - 128]),
                }))
            }

            _ => Err("Unknown signature type identifier"),
        }
    }

    /// Encode signature to bytes
    ///
    /// For backward compatibility:
    /// - Secp256k1: encoded WITHOUT type identifier (65 bytes)
    /// - P256/WebAuthn: encoded WITH type identifier prefix
    pub fn to_bytes(&self) -> Bytes {
        match self {
            Self::Secp256k1(sig) => {
                // Backward compatibility: no type identifier for secp256k1
                // Ensure exactly 65 bytes by using a fixed-size buffer
                let sig_bytes = sig.as_bytes();
                assert_eq!(
                    sig_bytes.len(),
                    SECP256K1_SIGNATURE_LENGTH,
                    "Secp256k1 signature must be exactly 65 bytes"
                );
                Bytes::copy_from_slice(&sig_bytes)
            }
            Self::P256(p256_sig) => {
                let mut bytes = Vec::with_capacity(1 + 129);
                bytes.push(SIGNATURE_TYPE_P256);
                bytes.extend_from_slice(p256_sig.r.as_slice());
                bytes.extend_from_slice(p256_sig.s.as_slice());
                bytes.extend_from_slice(p256_sig.pub_key_x.as_slice());
                bytes.extend_from_slice(p256_sig.pub_key_y.as_slice());
                bytes.push(if p256_sig.pre_hash { 1 } else { 0 });
                Bytes::from(bytes)
            }
            Self::WebAuthn(webauthn_sig) => {
                let mut bytes = Vec::with_capacity(1 + webauthn_sig.webauthn_data.len() + 128);
                bytes.push(SIGNATURE_TYPE_WEBAUTHN);
                bytes.extend_from_slice(&webauthn_sig.webauthn_data);
                bytes.extend_from_slice(webauthn_sig.r.as_slice());
                bytes.extend_from_slice(webauthn_sig.s.as_slice());
                bytes.extend_from_slice(webauthn_sig.pub_key_x.as_slice());
                bytes.extend_from_slice(webauthn_sig.pub_key_y.as_slice());
                Bytes::from(bytes)
            }
        }
    }

    /// Get the length of the encoded signature in bytes
    ///
    /// For backward compatibility:
    /// - Secp256k1: 65 bytes (no type identifier)
    /// - P256/WebAuthn: includes 1-byte type identifier prefix
    pub fn encoded_length(&self) -> usize {
        match self {
            Self::Secp256k1(_) => SECP256K1_SIGNATURE_LENGTH,
            Self::P256(_) => 1 + P256_SIGNATURE_LENGTH,
            Self::WebAuthn(webauthn_sig) => 1 + webauthn_sig.webauthn_data.len() + 128,
        }
    }

    /// Get signature type
    pub fn signature_type(&self) -> SignatureType {
        match self {
            Self::Secp256k1(_) => SignatureType::Secp256k1,
            Self::P256(_) => SignatureType::P256,
            Self::WebAuthn(_) => SignatureType::WebAuthn,
        }
    }

    /// Get the in-memory size of the signature
    pub fn size(&self) -> usize {
        size_of::<Self>()
            + match self {
                Self::Secp256k1(_) | Self::P256(_) => 0,
                Self::WebAuthn(webauthn_sig) => webauthn_sig.webauthn_data.len(),
            }
    }

    /// Recover the signer address from the signature
    ///
    /// This function verifies the signature and extracts the address based on signature type:
    /// - secp256k1: Uses standard ecrecover (signature verification + address recovery)
    /// - P256: Verifies P256 signature then derives address from public key
    /// - WebAuthn: Parses WebAuthn data, verifies P256 signature, derives address
    pub fn recover_signer(
        &self,
        sig_hash: &B256,
    ) -> Result<Address, alloy_consensus::crypto::RecoveryError> {
        match self {
            Self::Secp256k1(sig) => {
                // Standard secp256k1 recovery using alloy's built-in methods
                // This simultaneously verifies the signature AND recovers the address
                alloy_consensus::crypto::secp256k1::recover_signer(sig, *sig_hash)
            }
            Self::P256(p256_sig) => {
                // Prepare message hash for verification
                let message_hash = if p256_sig.pre_hash {
                    // Some P256 implementations (like Web Crypto) require pre-hashing
                    B256::from_slice(Sha256::digest(sig_hash).as_ref())
                } else {
                    *sig_hash
                };

                // Verify P256 signature cryptographically
                verify_p256_signature_internal(
                    p256_sig.r.as_slice(),
                    p256_sig.s.as_slice(),
                    p256_sig.pub_key_x.as_slice(),
                    p256_sig.pub_key_y.as_slice(),
                    &message_hash,
                )
                .map_err(|_| alloy_consensus::crypto::RecoveryError::new())?;

                // Derive and return address
                Ok(derive_p256_address(
                    &p256_sig.pub_key_x,
                    &p256_sig.pub_key_y,
                ))
            }
            Self::WebAuthn(webauthn_sig) => {
                // Parse and verify WebAuthn data, compute challenge hash
                let message_hash =
                    verify_webauthn_data_internal(&webauthn_sig.webauthn_data, sig_hash)
                        .map_err(|_| alloy_consensus::crypto::RecoveryError::new())?;

                // Verify P256 signature over the computed message hash
                verify_p256_signature_internal(
                    webauthn_sig.r.as_slice(),
                    webauthn_sig.s.as_slice(),
                    webauthn_sig.pub_key_x.as_slice(),
                    webauthn_sig.pub_key_y.as_slice(),
                    &message_hash,
                )
                .map_err(|_| alloy_consensus::crypto::RecoveryError::new())?;

                // Derive and return address
                Ok(derive_p256_address(
                    &webauthn_sig.pub_key_x,
                    &webauthn_sig.pub_key_y,
                ))
            }
        }
    }
}

impl Default for PrimitiveSignature {
    fn default() -> Self {
        Self::Secp256k1(Signature::test_signature())
    }
}

impl alloy_rlp::Encodable for PrimitiveSignature {
    fn encode(&self, out: &mut dyn alloy_rlp::BufMut) {
        let bytes = self.to_bytes();
        alloy_rlp::Encodable::encode(&bytes, out);
    }

    fn length(&self) -> usize {
        self.to_bytes().length()
    }
}

impl alloy_rlp::Decodable for PrimitiveSignature {
    fn decode(buf: &mut &[u8]) -> alloy_rlp::Result<Self> {
        let bytes: Bytes = alloy_rlp::Decodable::decode(buf)?;
        Self::from_bytes(&bytes).map_err(alloy_rlp::Error::Custom)
    }
}

/// Keychain signature version.
///
/// Determines how the signature hash is computed for the inner signature.
#[derive(Clone, Copy, Debug, Default, PartialEq, Eq, Hash)]
#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
#[cfg_attr(feature = "serde", serde(rename_all = "camelCase"))]
#[cfg_attr(any(test, feature = "arbitrary"), derive(arbitrary::Arbitrary))]
pub enum KeychainVersion {
    /// Legacy (V1): inner signature signs `sig_hash` directly.
    /// Deprecated at T1C.
    /// TODO(tanishk): change default to V2 after T1C
    #[default]
    V1,
    /// V2: inner signature signs `keccak256(0x04 || sig_hash || user_address)`.
    /// Binds the signature to the specific user account with a domain separator.
    V2,
}

/// Keychain version validation error.
///
/// Returned by [`TempoSignature::validate_version`] when a keychain
/// signature's version is incompatible with the current hardfork.
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub enum KeychainVersionError {
    /// Legacy V1 keychain signature used after T1C activation (permanently invalid).
    LegacyPostT1C,
    /// V2 keychain signature used before T1C activation (not yet valid).
    V2BeforeActivation,
}

/// Keychain signature wrapping another signature with a user address.
/// This allows an access key to sign on behalf of a root account.
///
/// No `Compact` impl — always wrapped in [`TempoSignature`] whose `Compact` delegates
/// to `to_bytes()`/`from_bytes()` which encodes the version via the wire type byte
/// (`0x03` = V1, `0x04` = V2).
///
/// Format (V1): 0x03 || user_address (20 bytes) || inner_signature
/// Format (V2): 0x04 || user_address (20 bytes) || inner_signature
///
/// The user_address is the root account this transaction is being executed for.
/// The inner signature proves an authorized access key signed the transaction.
/// The handler validates that user_address has authorized the access key in the KeyChain precompile.
#[derive(Clone, Debug)]
#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
#[cfg_attr(feature = "serde", serde(rename_all = "camelCase"))]
pub struct KeychainSignature {
    /// Root account address that this transaction is being executed for
    pub user_address: Address,
    /// The actual signature from the access key (can be Secp256k1, P256, or WebAuthn, but NOT another Keychain)
    pub signature: PrimitiveSignature,
    /// Keychain signature version (V1 = legacy, V2 = includes user_address in sig hash)
    #[cfg_attr(feature = "serde", serde(default))]
    pub version: KeychainVersion,
    /// Cached access key ID recovered from the inner signature.
    /// This is an implementation detail - use `key_id()` to access.
    /// Uses OnceLock for thread-safe interior mutability.
    /// Note: Excluded from PartialEq, Eq, Hash, and Compact as it's a cache.
    #[cfg_attr(
        feature = "serde",
        serde(
            serialize_with = "serialize_once_lock",
            rename = "keyId",
            skip_deserializing,
        )
    )]
    cached_key_id: OnceLock<Address>,
}

impl KeychainSignature {
    /// Create a new V2 KeychainSignature (recommended).
    ///
    /// V2 signatures include the user_address in the signature hash.
    pub fn new(user_address: Address, signature: PrimitiveSignature) -> Self {
        Self {
            user_address,
            signature,
            version: KeychainVersion::V2,
            cached_key_id: OnceLock::new(),
        }
    }

    /// Create a legacy V1 KeychainSignature.
    ///
    /// V1 signatures do NOT include the user_address in the signature hash
    /// and are deprecated at the T1C hardfork.
    pub fn new_v1(user_address: Address, signature: PrimitiveSignature) -> Self {
        Self {
            user_address,
            signature,
            version: KeychainVersion::V1,
            cached_key_id: OnceLock::new(),
        }
    }

    /// Compute the effective signature hash for key recovery.
    ///
    /// - V1: returns `sig_hash` directly (legacy, deprecated)
    /// - V2: returns `keccak256(0x04 || sig_hash || user_address)`
    fn effective_sig_hash(&self, sig_hash: &B256) -> B256 {
        match self.version {
            KeychainVersion::V1 => *sig_hash,
            KeychainVersion::V2 => Self::signing_hash(*sig_hash, self.user_address),
        }
    }

    /// Get the access key ID for Keychain signatures.
    ///
    /// For Keychain signatures, this returns the access key address that signed the transaction.
    /// The key_id is recovered from the inner signature on first access and cached for
    /// subsequent calls. Returns None for non-Keychain signatures.
    ///
    /// This follows the pattern used in alloy for lazy hash computation.
    pub fn key_id(
        &self,
        sig_hash: &B256,
    ) -> Result<Address, alloy_consensus::crypto::RecoveryError> {
        // Check if already cached
        if let Some(cached) = self.cached_key_id.get() {
            return Ok(*cached);
        }

        // Not cached - recover and cache
        let effective_hash = self.effective_sig_hash(sig_hash);
        let key_id = self.signature.recover_signer(&effective_hash)?;
        #[allow(clippy::useless_conversion)]
        let _ = self.cached_key_id.set(key_id.into());
        Ok(key_id)
    }

    /// Returns true if this is a legacy V1 keychain signature.
    pub fn is_legacy(&self) -> bool {
        self.version == KeychainVersion::V1
    }

    /// Compute the hash that an access key should sign for a V2 keychain transaction.
    ///
    /// Returns `keccak256(0x04 || sig_hash || user_address)`.
    /// The `0x04` domain separator ([`SIGNATURE_TYPE_KEYCHAIN_V2`]) prevents
    /// cross-scheme signature confusion, following the same pattern as
    /// EIP-7702 (`0x05`) and Tempo fee-payer signatures (`0x78`).
    pub fn signing_hash(sig_hash: B256, user_address: Address) -> B256 {
        let mut buf = [0u8; 53]; // 1 + 32 + 20
        buf[0] = SIGNATURE_TYPE_KEYCHAIN_V2;
        buf[1..33].copy_from_slice(sig_hash.as_slice());
        buf[33..].copy_from_slice(user_address.as_slice());
        keccak256(buf)
    }
}

// Manual implementations of PartialEq, Eq, and Hash that exclude cached_key_id
// since it's just a cache and doesn't affect the logical equality of signatures
impl PartialEq for KeychainSignature {
    fn eq(&self, other: &Self) -> bool {
        self.user_address == other.user_address
            && self.signature == other.signature
            && self.version == other.version
    }
}

impl Eq for KeychainSignature {}

impl core::hash::Hash for KeychainSignature {
    fn hash<H: core::hash::Hasher>(&self, state: &mut H) {
        self.user_address.hash(state);
        self.signature.hash(state);
        self.version.hash(state);
    }
}

// Manual Arbitrary implementation that excludes cached_key_id (cache field)
#[cfg(any(test, feature = "arbitrary"))]
impl<'a> arbitrary::Arbitrary<'a> for KeychainSignature {
    fn arbitrary(u: &mut arbitrary::Unstructured<'a>) -> arbitrary::Result<Self> {
        Ok(Self {
            user_address: u.arbitrary()?,
            signature: u.arbitrary()?,
            version: u.arbitrary()?,
            cached_key_id: OnceLock::new(), // Always start with empty cache
        })
    }
}

/// AA transaction signature supporting multiple signature schemes
///
/// Note: Uses custom Compact implementation that delegates to `to_bytes()` / `from_bytes()`.
#[derive(Clone, Debug, PartialEq, Eq, Hash)]
#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
#[cfg_attr(feature = "serde", serde(untagged, rename_all = "camelCase"))]
#[cfg_attr(any(test, feature = "arbitrary"), derive(arbitrary::Arbitrary))]
pub enum TempoSignature {
    /// Primitive signature types: Secp256k1, P256, or WebAuthn
    Primitive(PrimitiveSignature),

    /// Keychain signature - wraps another signature with a key identifier
    /// Format: key_id (20 bytes) + inner signature
    /// IMP: The inner signature MUST NOT be another Keychain (validated at runtime)
    /// Note: Recursion is prevented by KeychainSignature's custom Arbitrary impl
    Keychain(KeychainSignature),
}

impl TempoSignature {
    /// Parse signature from bytes with backward compatibility
    ///
    /// For backward compatibility with existing secp256k1 signatures:
    /// - If length is 65 bytes: treat as secp256k1 signature (no type identifier)
    /// - Otherwise: first byte is the signature type identifier
    pub fn from_bytes(data: &[u8]) -> Result<Self, &'static str> {
        if data.is_empty() {
            return Err("Signature data is empty");
        }

        // Check if this is a Keychain signature (type identifier 0x03 or 0x04)
        // We need to handle this specially before delegating to PrimitiveSignature
        if data.len() > 1
            && data.len() != SECP256K1_SIGNATURE_LENGTH
            && (data[0] == SIGNATURE_TYPE_KEYCHAIN || data[0] == SIGNATURE_TYPE_KEYCHAIN_V2)
        {
            let version = if data[0] == SIGNATURE_TYPE_KEYCHAIN {
                KeychainVersion::V1
            } else {
                KeychainVersion::V2
            };
            let sig_data = &data[1..];

            // Keychain format: user_address (20 bytes) || inner_signature
            if sig_data.len() < 20 {
                return Err("Invalid Keychain signature: too short for user_address");
            }

            let user_address = Address::from_slice(&sig_data[0..20]);
            let inner_sig_bytes = &sig_data[20..];

            // Parse inner signature using PrimitiveSignature (which doesn't support Keychain)
            // This automatically prevents recursive keychain signatures at compile time
            let inner_signature = PrimitiveSignature::from_bytes(inner_sig_bytes)?;

            return Ok(Self::Keychain(KeychainSignature {
                user_address,
                signature: inner_signature,
                version,
                cached_key_id: OnceLock::new(),
            }));
        }

        // For all non-Keychain signatures, delegate to PrimitiveSignature
        let primitive = PrimitiveSignature::from_bytes(data)?;
        Ok(Self::Primitive(primitive))
    }

    /// Encode signature to bytes
    ///
    /// For backward compatibility:
    /// - Secp256k1: encoded WITHOUT type identifier (65 bytes)
    /// - P256/WebAuthn: encoded WITH type identifier prefix
    pub fn to_bytes(&self) -> Bytes {
        match self {
            Self::Primitive(primitive_sig) => primitive_sig.to_bytes(),
            Self::Keychain(keychain_sig) => {
                // Format: type_byte | user_address (20 bytes) | inner_signature
                let inner_bytes = keychain_sig.signature.to_bytes();
                let mut bytes = Vec::with_capacity(1 + 20 + inner_bytes.len());
                let type_byte = match keychain_sig.version {
                    KeychainVersion::V1 => SIGNATURE_TYPE_KEYCHAIN,
                    KeychainVersion::V2 => SIGNATURE_TYPE_KEYCHAIN_V2,
                };
                bytes.push(type_byte);
                bytes.extend_from_slice(keychain_sig.user_address.as_slice());
                bytes.extend_from_slice(&inner_bytes);
                Bytes::from(bytes)
            }
        }
    }

    /// Get the length of the encoded signature in bytes
    ///
    /// For backward compatibility:
    /// - Secp256k1: 65 bytes (no type identifier)
    /// - P256/WebAuthn: includes 1-byte type identifier prefix
    pub fn encoded_length(&self) -> usize {
        match self {
            Self::Primitive(primitive_sig) => primitive_sig.encoded_length(),
            Self::Keychain(keychain_sig) => 1 + 20 + keychain_sig.signature.encoded_length(),
        }
    }

    /// Get signature type
    pub fn signature_type(&self) -> SignatureType {
        match self {
            Self::Primitive(primitive_sig) => primitive_sig.signature_type(),
            Self::Keychain(keychain_sig) => keychain_sig.signature.signature_type(),
        }
    }

    /// Get the in-memory size of the signature
    pub fn size(&self) -> usize {
        match self {
            Self::Primitive(primitive_sig) => primitive_sig.size(),
            Self::Keychain(keychain_sig) => 1 + 20 + keychain_sig.signature.size(),
        }
    }

    /// Recover the signer address from the signature
    ///
    /// This function verifies the signature and extracts the address based on signature type:
    /// - secp256k1: Uses standard ecrecover (signature verification + address recovery)
    /// - P256: Verifies P256 signature then derives address from public key
    /// - WebAuthn: Parses WebAuthn data, verifies P256 signature, derives address
    /// - Keychain: Validates inner signature and returns user_address
    ///
    /// For Keychain signatures, this performs full validation of the inner signature.
    /// The access key address is cached in the KeychainSignature for later use.
    /// Note: This pattern has a big footgun, that someone using recover_signer, cannot assume
    /// that the signature is valid for the keychain. They also need to check the access key is authorized
    /// in the keychain precompile.
    /// We cannot check this here, as we don't have access to the keychain precompile.
    pub fn recover_signer(
        &self,
        sig_hash: &B256,
    ) -> Result<Address, alloy_consensus::crypto::RecoveryError> {
        match self {
            Self::Primitive(primitive_sig) => primitive_sig.recover_signer(sig_hash),
            Self::Keychain(keychain_sig) => {
                // Ensure validity of the keychain signature and cache the key id
                keychain_sig.key_id(sig_hash)?;

                // Return the user_address - the root account this transaction is for
                Ok(keychain_sig.user_address)
            }
        }
    }

    /// Check if this is a Keychain signature
    pub fn is_keychain(&self) -> bool {
        matches!(self, Self::Keychain(_))
    }

    /// Check if this is a legacy V1 Keychain signature (deprecated at T1C).
    pub fn is_legacy_keychain(&self) -> bool {
        matches!(self, Self::Keychain(k) if k.is_legacy())
    }

    /// Check if this is a V2 Keychain signature.
    pub fn is_v2_keychain(&self) -> bool {
        matches!(
            self,
            Self::Keychain(KeychainSignature {
                version: KeychainVersion::V2,
                ..
            })
        )
    }

    /// Validates keychain signature version compatibility with the current hardfork.
    ///
    /// - Post-T1C: legacy V1 keychain signatures are rejected.
    /// - Pre-T1C: V2 keychain signatures are rejected to prevent chain splits.
    pub fn validate_version(&self, is_t1c: bool) -> Result<(), KeychainVersionError> {
        if is_t1c && self.is_legacy_keychain() {
            return Err(KeychainVersionError::LegacyPostT1C);
        }
        if !is_t1c && self.is_v2_keychain() {
            return Err(KeychainVersionError::V2BeforeActivation);
        }
        Ok(())
    }

    /// Get the Keychain signature if this is a Keychain signature
    pub fn as_keychain(&self) -> Option<&KeychainSignature> {
        match self {
            Self::Keychain(keychain_sig) => Some(keychain_sig),
            _ => None,
        }
    }
}

impl Default for TempoSignature {
    fn default() -> Self {
        Self::Primitive(PrimitiveSignature::default())
    }
}

impl alloy_rlp::Encodable for TempoSignature {
    fn encode(&self, out: &mut dyn alloy_rlp::BufMut) {
        let bytes = self.to_bytes();
        alloy_rlp::Encodable::encode(&bytes, out);
    }

    fn length(&self) -> usize {
        self.to_bytes().length()
    }
}

impl alloy_rlp::Decodable for TempoSignature {
    fn decode(buf: &mut &[u8]) -> alloy_rlp::Result<Self> {
        let bytes: Bytes = alloy_rlp::Decodable::decode(buf)?;
        Self::from_bytes(&bytes).map_err(alloy_rlp::Error::Custom)
    }
}

impl From<Signature> for TempoSignature {
    fn from(signature: Signature) -> Self {
        Self::Primitive(PrimitiveSignature::Secp256k1(signature))
    }
}

// ============================================================================
// Helper Functions for Signature Verification
// ============================================================================

/// Derives a P256 address from public key coordinates
pub fn derive_p256_address(pub_key_x: &B256, pub_key_y: &B256) -> Address {
    let hash = keccak256([pub_key_x.as_slice(), pub_key_y.as_slice()].concat());

    // Take last 20 bytes as address
    Address::from_slice(&hash[12..])
}

/// Concatenates byte slices into a fixed-size array without heap allocations.
fn concat<const N: usize>(slices: &[&[u8]]) -> [u8; N] {
    let mut out = [0u8; N];
    let mut offset = 0;
    for s in slices {
        out[offset..offset + s.len()].copy_from_slice(s);
        offset += s.len();
    }
    debug_assert_eq!(offset, N, "slices length doesn't match array size");
    out
}

#[cfg(feature = "std")]
fn verify_p256_signature_with_aws_lc(
    r: &[u8],
    s: &[u8],
    pub_key_x: &[u8],
    pub_key_y: &[u8],
    message_hash: &B256,
) -> Result<(), &'static str> {
    use aws_lc_rs::{
        digest::{Digest as AwsLcDigest, SHA256 as AwsLcSha256},
        signature::{ECDSA_P256_SHA256_FIXED, ParsedPublicKey as AwsLcParsedPublicKey},
    };

    let encoded_point = concat::<65>(&[&[0x04], pub_key_x, pub_key_y]);
    let verifying_key = AwsLcParsedPublicKey::new(&ECDSA_P256_SHA256_FIXED, encoded_point)
        .map_err(|_| "Invalid P256 public key")?;

    let signature = concat::<64>(&[r, s]);

    // Tempo verifies already-computed 32-byte message digests.
    // Switching to message-based aws-lc verifier would hash again and change consensus behavior.
    let digest = AwsLcDigest::import_less_safe(message_hash.as_slice(), &AwsLcSha256)
        .map_err(|_| "Invalid P256 message digest")?;

    verifying_key
        .verify_digest_sig(&digest, &signature)
        .map_err(|_| "P256 signature verification failed")
}

#[cfg(any(test, not(feature = "std")))]
fn verify_p256_signature_with_p256(
    r: &[u8],
    s: &[u8],
    pub_key_x: &[u8],
    pub_key_y: &[u8],
    message_hash: &B256,
) -> Result<(), &'static str> {
    use p256::{
        EncodedPoint,
        ecdsa::{Signature as P256Signature, VerifyingKey, signature::hazmat::PrehashVerifier},
    };

    let encoded_point =
        EncodedPoint::from_affine_coordinates(pub_key_x.into(), pub_key_y.into(), false);
    let verifying_key =
        VerifyingKey::from_encoded_point(&encoded_point).map_err(|_| "Invalid P256 public key")?;

    let signature = P256Signature::from_slice(&concat::<64>(&[r, s]))
        .map_err(|_| "Invalid P256 signature encoding")?;

    // Tempo verifies already-computed 32-byte message digests.
    verifying_key
        .verify_prehash(message_hash.as_slice(), &signature)
        .map_err(|_| "P256 signature verification failed")
}

/// Verifies a P256 signature using the provided components
///
/// This performs actual cryptographic verification of the P256 signature
/// according to the spec. Called during `recover_signer()` to ensure only
/// valid signatures enter the mempool.
///
/// Includes a high-s value check to prevent signature malleability. For any
/// ECDSA signature (r, s), a second valid signature (r, n-s) exists. By
/// requiring s <= n/2 (the "low-s" requirement), we ensure only one canonical
/// form is accepted, preventing transaction hash malleability attacks.
///
/// NOTE: this function conditionally compiles based on the cfg
/// - !std → p256
/// - std && !test → aws-lc-rs (best performance)
/// - std && test → both (ensures verification backend alignment)
fn verify_p256_signature_internal(
    r: &[u8],
    s: &[u8],
    pub_key_x: &[u8],
    pub_key_y: &[u8],
    message_hash: &B256,
) -> Result<(), &'static str> {
    // High-s value check: reject signatures where s > n/2 to prevent malleability
    if U256::from_be_slice(s) > P256N_HALF {
        return Err("P256 signature has high s value");
    }

    #[cfg(all(feature = "std", not(test)))]
    {
        // production `std` builds use the `aws-lc-rs` crate
        verify_p256_signature_with_aws_lc(r, s, pub_key_x, pub_key_y, message_hash)
    }

    #[cfg(not(feature = "std"))]
    {
        // production `no-std` builds use the `p256` crate
        verify_p256_signature_with_p256(r, s, pub_key_x, pub_key_y, message_hash)
    }

    #[cfg(all(feature = "std", test))]
    {
        // test builds use both crates to verify alignment
        let aws_lc = verify_p256_signature_with_aws_lc(r, s, pub_key_x, pub_key_y, message_hash);
        let p256 = verify_p256_signature_with_p256(r, s, pub_key_x, pub_key_y, message_hash);

        assert_eq!(
            aws_lc.is_ok(),
            p256.is_ok(),
            "aws-lc and p256 verification backends disagreed"
        );

        aws_lc
    }
}

/// Minimal struct to deserialize only the fields we need from clientDataJSON.
/// serde_json will ignore unknown fields and only parse `type` and `challenge`.
#[derive(serde::Deserialize)]
struct ClientDataJson<'a> {
    #[serde(rename = "type")]
    type_field: &'a str,
    challenge: &'a str,
}

/// Parses and validates WebAuthn data, returning the message hash for P256 verification.
/// ref: <https://www.w3.org/TR/webauthn-2/#sctn-authenticator-data>
///
/// 1. Parses authenticatorData and clientDataJSON
/// 2. Validates authenticatorData (min 37 bytes, UP flag set)
/// 3. Validates clientDataJSON (type="webauthn.get", challenge matches tx_hash)
/// 4. Computes message hash = sha256(authenticatorData || sha256(clientDataJSON))
fn verify_webauthn_data_internal(
    webauthn_data: &[u8],
    tx_hash: &B256,
) -> Result<B256, &'static str> {
    // Ensure that we have clientDataJSON after authenticatorData
    if webauthn_data.len() < MIN_AUTH_DATA_LEN + 32 {
        return Err("WebAuthn data too short");
    }

    // Check flags (byte 32)
    let flags = webauthn_data[32];
    let (up_flag, uv_flag, at_flag, ed_flag) = (flags & UP, flags & UV, flags & AT, flags & ED);

    // UP or UV flag MUST be set (UV implies user presence per WebAuthn spec)
    if up_flag == 0 && uv_flag == 0 {
        return Err("neither UP, nor UV flag set");
    }

    // AT flag must NOT be set for assertion signatures (`webauthn.get`)
    if at_flag != 0 {
        return Err("AT flag must not be set for assertion signatures");
    }

    // Determine authenticatorData length
    let auth_data_len = if ed_flag == 0 {
        // If ED flag is not set, exactly 37 bytes (no extensions)
        MIN_AUTH_DATA_LEN
    } else {
        // ED flag must NOT be set, as Tempo AA doesn't support extensions
        // NOTE: If we ever want to support extensions, we will have to parse CBOR data
        return Err("ED flag must not be set, as Tempo doesn't support extensions");
    };

    let authenticator_data = &webauthn_data[..auth_data_len];
    let client_data_json = &webauthn_data[auth_data_len..];

    // Parse clientDataJSON (only extracts type and challenge fields)
    // NOTE: Size is already bounded by MAX_WEBAUTHN_SIGNATURE_LENGTH (2KB) at signature parsing
    let client_data: ClientDataJson<'_> =
        serde_json::from_slice(client_data_json).map_err(|_| "clientDataJSON is not valid JSON")?;

    // Validate type field
    if client_data.type_field != "webauthn.get" {
        return Err("clientDataJSON type must be webauthn.get");
    }

    // Validate challenge matches tx_hash (Base64URL encoded)
    if client_data.challenge != URL_SAFE_NO_PAD.encode(tx_hash.as_slice()) {
        return Err("clientDataJSON challenge does not match transaction hash");
    }

    // Compute message hash according to spec:
    // messageHash = sha256(authenticatorData || sha256(clientDataJSON))
    let client_data_hash = Sha256::digest(client_data_json);

    let mut final_hasher = Sha256::new();
    final_hasher.update(authenticator_data);
    final_hasher.update(client_data_hash);
    let message_hash = final_hasher.finalize();

    Ok(B256::from_slice(&message_hash))
}

#[cfg(feature = "serde")]
/// Helper function to serialize a [`OnceLock`] as an [`Option`] if it's initialized.
fn serialize_once_lock<S>(value: &OnceLock<Address>, serializer: S) -> Result<S::Ok, S::Error>
where
    S: serde::Serializer,
{
    serde::Serialize::serialize(&value.get(), serializer)
}

#[cfg(test)]
mod tests {
    use super::*;
    use alloy_primitives::hex;
    use base64::engine::general_purpose::URL_SAFE_NO_PAD;
    use p256::{
        ecdsa::{SigningKey as P256SigningKey, signature::hazmat::PrehashSigner},
        elliptic_curve::rand_core::OsRng,
    };

    /// Generate P256 keypair, return (signing_key, pub_key_x, pub_key_y)
    fn generate_p256_keypair() -> (P256SigningKey, B256, B256) {
        let signing_key = P256SigningKey::random(&mut OsRng);
        let verifying_key = signing_key.verifying_key();
        let encoded_point = verifying_key.to_encoded_point(false);
        let pub_key_x = B256::from_slice(encoded_point.x().unwrap().as_ref());
        let pub_key_y = B256::from_slice(encoded_point.y().unwrap().as_ref());
        (signing_key, pub_key_x, pub_key_y)
    }

    /// Sign a message hash with P256, normalize s, return (r, s)
    fn sign_p256_normalized(signing_key: &P256SigningKey, message_hash: &B256) -> (B256, B256) {
        let signature: p256::ecdsa::Signature =
            signing_key.sign_prehash(message_hash.as_slice()).unwrap();
        let sig_bytes = signature.to_bytes();
        let r = B256::from_slice(&sig_bytes[0..32]);
        let s = normalize_p256_s(&sig_bytes[32..64]).expect("p256 crate produces valid s");
        (r, s)
    }

    /// Build webauthn data with given flags and optional extension bytes
    fn build_webauthn_data(flags: u8, extension: Option<&[u8]>, tx_hash: &B256) -> Vec<u8> {
        let mut data = vec![0u8; 32]; // rpIdHash
        data.push(flags);
        data.extend_from_slice(&[0u8; 4]); // signCount
        if let Some(ext) = extension {
            data.extend_from_slice(ext);
        }
        let challenge = URL_SAFE_NO_PAD.encode(tx_hash.as_slice());
        data.extend_from_slice(
            format!("{{\"type\":\"webauthn.get\",\"challenge\":\"{challenge}\"}}").as_bytes(),
        );
        data
    }

    #[test]
    fn test_p256_high_s_normalization() {
        // s < P256N_HALF → unchanged
        let low_s = U256::from(1u64);
        let low_s_bytes: [u8; 32] = low_s.to_be_bytes();
        assert_eq!(
            U256::from_be_slice(normalize_p256_s(&low_s_bytes).unwrap().as_slice()),
            low_s,
            "s < P256N_HALF should remain unchanged"
        );

        // s == P256N_HALF → unchanged
        let half_bytes: [u8; 32] = P256N_HALF.to_be_bytes();
        assert_eq!(
            U256::from_be_slice(normalize_p256_s(&half_bytes).unwrap().as_slice()),
            P256N_HALF,
            "s == P256N_HALF should remain unchanged"
        );

        // s == P256N_HALF + 1 → normalized to P256_ORDER - s
        let high_s = P256N_HALF + U256::from(1u64);
        let high_s_bytes: [u8; 32] = high_s.to_be_bytes();
        assert_eq!(
            U256::from_be_slice(normalize_p256_s(&high_s_bytes).unwrap().as_slice()),
            P256_ORDER - high_s,
            "s > P256N_HALF should be normalized"
        );

        // s == P256_ORDER - 1 → normalized to 1
        let max_s = P256_ORDER - U256::from(1u64);
        let max_s_bytes: [u8; 32] = max_s.to_be_bytes();
        assert_eq!(
            U256::from_be_slice(normalize_p256_s(&max_s_bytes).unwrap().as_slice()),
            U256::from(1u64),
            "s == P256_ORDER - 1 should normalize to 1"
        );

        // s == 0 → rejected
        let zero_bytes: [u8; 32] = U256::ZERO.to_be_bytes();
        assert!(
            normalize_p256_s(&zero_bytes).is_err(),
            "s == 0 should be rejected"
        );

        // s == P256_ORDER → rejected
        let order_bytes: [u8; 32] = P256_ORDER.to_be_bytes();
        assert!(
            normalize_p256_s(&order_bytes).is_err(),
            "s == P256_ORDER should be rejected"
        );

        // s == P256_ORDER + 1 → rejected
        let over_bytes: [u8; 32] = (P256_ORDER + U256::from(1u64)).to_be_bytes();
        assert!(
            normalize_p256_s(&over_bytes).is_err(),
            "s > P256_ORDER should be rejected"
        );

        // s == U256::MAX → rejected
        let max_bytes: [u8; 32] = U256::MAX.to_be_bytes();
        assert!(
            normalize_p256_s(&max_bytes).is_err(),
            "s == U256::MAX should be rejected"
        );
    }

    #[test]
    fn test_p256_signature_verification_invalid_pubkey() {
        // Invalid public key should fail
        let r = [0u8; 32];
        let s = [0u8; 32];
        let pub_key_x = [0u8; 32]; // Invalid: point not on curve
        let pub_key_y = [0u8; 32];
        let message_hash = B256::ZERO;

        let result = verify_p256_signature_internal(&r, &s, &pub_key_x, &pub_key_y, &message_hash);
        assert!(result.is_err());
    }

    #[test]
    fn test_p256_signature_verification_invalid_signature() {
        let (_, pub_key_x, pub_key_y) = generate_p256_keypair();
        let message_hash = B256::ZERO;

        let assert_invalid = |r: &[u8], s: &[u8], context: &str| {
            let result = verify_p256_signature_internal(
                r,
                s,
                pub_key_x.as_slice(),
                pub_key_y.as_slice(),
                &message_hash,
            );
            assert!(result.is_err(), "{context} should fail verification");
        };

        // Use invalid signature (all zeros)
        let r = [0u8; 32];
        let s = [0u8; 32];
        assert_invalid(&r, &s, "all-zero signature");

        let one = U256::from(1u64).to_be_bytes::<32>();
        let order = P256_ORDER.to_be_bytes::<32>();
        assert_invalid(&order, &one, "signature with r == P256_ORDER");
        assert_invalid(&one, &order, "signature with s == P256_ORDER");
    }

    #[test]
    fn test_p256_signature_verification_valid() {
        let (signing_key, pub_key_x, pub_key_y) = generate_p256_keypair();
        let message_hash = B256::from_slice(&Sha256::digest(b"test message"));
        let (r, s) = sign_p256_normalized(&signing_key, &message_hash);

        let result = verify_p256_signature_internal(
            r.as_slice(),
            s.as_slice(),
            pub_key_x.as_slice(),
            pub_key_y.as_slice(),
            &message_hash,
        );
        assert!(
            result.is_ok(),
            "Valid P256 signature should verify successfully"
        );
    }

    #[test]
    fn test_p256_high_s_rejection() {
        let (signing_key, pub_key_x, pub_key_y) = generate_p256_keypair();
        let message_hash = B256::from_slice(&Sha256::digest(b"test message for high s"));

        // Sign and get raw (non-normalized) signature
        let signature: p256::ecdsa::Signature =
            signing_key.sign_prehash(message_hash.as_slice()).unwrap();
        let sig_bytes = signature.to_bytes();
        let r = &sig_bytes[0..32];
        let original_s = &sig_bytes[32..64];

        // Convert s to U256 and compute n - s (the high-s equivalent)
        let s_value = alloy_primitives::U256::from_be_slice(original_s);
        let computed_high_s = P256_ORDER - s_value;
        let computed_high_s_bytes: [u8; 32] = computed_high_s.to_be_bytes();

        // Depending on which s was originally produced, either original or high-s
        // should be rejected
        let s_is_low = s_value <= P256N_HALF;
        if s_is_low {
            // Original s is low, so high-s version should be rejected
            let result = verify_p256_signature_internal(
                r,
                &computed_high_s_bytes,
                pub_key_x.as_slice(),
                pub_key_y.as_slice(),
                &message_hash,
            );
            assert!(
                result.is_err(),
                "High-s signature should be rejected for signature malleability prevention"
            );
            assert_eq!(result.unwrap_err(), "P256 signature has high s value");
        } else {
            // Original s was already high, so the computed "high_s" is actually low
            // This means the original should fail
            let original_result = verify_p256_signature_internal(
                r,
                original_s,
                pub_key_x.as_slice(),
                pub_key_y.as_slice(),
                &message_hash,
            );
            assert!(
                original_result.is_err(),
                "Original high-s signature should be rejected"
            );
        }
    }

    #[test]
    fn test_webauthn_data_verification_too_short() {
        // WebAuthn data must be at least 37 bytes (authenticatorData minimum)
        let short_data = vec![0u8; 36];
        let tx_hash = B256::ZERO;

        let result = verify_webauthn_data_internal(&short_data, &tx_hash);
        assert!(result.is_err());
        assert_eq!(result.unwrap_err(), "WebAuthn data too short");
    }

    #[test]
    fn test_webauthn_data_verification_missing_up_and_uv_flags() {
        let tx_hash = B256::ZERO;
        let client_data = b"{\"type\":\"webauthn.get\",\"challenge\":\"AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA\"}";

        // Create valid authenticatorData without UV nor UP flag
        let mut auth_data = vec![0u8; 37];
        auth_data[32] = 0x00;
        let mut webauthn_data = auth_data;
        webauthn_data.extend_from_slice(client_data);

        let result = verify_webauthn_data_internal(&webauthn_data, &tx_hash);
        assert!(result.is_err());
        assert_eq!(result.unwrap_err(), "neither UP, nor UV flag set");

        // Create valid authenticatorData with UV flag
        let mut auth_data = vec![0u8; 37];
        auth_data[32] = 0x04;
        let mut webauthn_data = auth_data;
        webauthn_data.extend_from_slice(client_data);

        assert!(verify_webauthn_data_internal(&webauthn_data, &tx_hash).is_ok());
    }

    #[test]
    fn test_webauthn_data_verification_invalid_type() {
        // Create valid authenticatorData with UP flag
        let mut auth_data = vec![0u8; 37];
        auth_data[32] = 0x01; // flags byte with UP flag set

        // Add clientDataJSON with wrong type
        let client_data = b"{\"type\":\"webauthn.create\",\"challenge\":\"AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA\"}";
        let mut webauthn_data = auth_data;
        webauthn_data.extend_from_slice(client_data);

        let tx_hash = B256::ZERO;
        let result = verify_webauthn_data_internal(&webauthn_data, &tx_hash);

        assert!(result.is_err());
        assert_eq!(
            result.unwrap_err(),
            "clientDataJSON type must be webauthn.get"
        );
    }

    #[test]
    fn test_webauthn_data_verification_invalid_challenge() {
        // Create valid authenticatorData with UP flag
        let mut auth_data = vec![0u8; 37];
        auth_data[32] = 0x01; // flags byte with UP flag set

        // Add clientDataJSON with wrong challenge
        let client_data =
            b"{\"type\":\"webauthn.get\",\"challenge\":\"wrong_challenge_value_here\"}";
        let mut webauthn_data = auth_data;
        webauthn_data.extend_from_slice(client_data);

        let tx_hash = B256::ZERO;
        let result = verify_webauthn_data_internal(&webauthn_data, &tx_hash);

        assert!(result.is_err());
        assert_eq!(
            result.unwrap_err(),
            "clientDataJSON challenge does not match transaction hash"
        );
    }

    #[test]
    fn test_webauthn_data_verification_valid() {
        let tx_hash = B256::from_slice(&[0xAA; 32]);
        let webauthn_data = build_webauthn_data(0x01, None, &tx_hash); // UP flag

        let result = verify_webauthn_data_internal(&webauthn_data, &tx_hash);
        assert!(
            result.is_ok(),
            "Valid WebAuthn data should verify successfully"
        );

        // Verify the computed message hash is correct
        let message_hash = result.unwrap();
        let auth_data = &webauthn_data[..37];
        let client_data = &webauthn_data[37..];

        let client_data_hash = Sha256::digest(client_data);
        let mut final_hasher = Sha256::new();
        final_hasher.update(auth_data);
        final_hasher.update(client_data_hash);
        let expected_hash = final_hasher.finalize();

        assert_eq!(message_hash.as_slice(), &expected_hash[..]);
    }

    #[test]
    fn test_p256_address_derivation() {
        let pub_key_x =
            hex!("1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef").into();
        let pub_key_y =
            hex!("fedcba0987654321fedcba0987654321fedcba0987654321fedcba0987654321").into();

        let addr1 = derive_p256_address(&pub_key_x, &pub_key_y);
        let addr2 = derive_p256_address(&pub_key_x, &pub_key_y);

        // Should be deterministic
        assert_eq!(addr1, addr2);

        // Should not be zero address
        assert_ne!(addr1, Address::ZERO);
    }

    #[test]
    fn test_p256_address_derivation_deterministic() {
        // Test that address derivation is deterministic
        let pub_key_x =
            hex!("1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef").into();
        let pub_key_y =
            hex!("fedcba0987654321fedcba0987654321fedcba0987654321fedcba0987654321").into();

        let addr1 = derive_p256_address(&pub_key_x, &pub_key_y);
        let addr2 = derive_p256_address(&pub_key_x, &pub_key_y);

        assert_eq!(addr1, addr2, "Address derivation should be deterministic");
    }

    #[test]
    fn test_p256_address_different_keys_different_addresses() {
        // Different keys should produce different addresses
        let pub_key_x1 =
            hex!("1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef").into();
        let pub_key_y1 =
            hex!("fedcba0987654321fedcba0987654321fedcba0987654321fedcba0987654321").into();

        let pub_key_x2 =
            hex!("fedcba0987654321fedcba0987654321fedcba0987654321fedcba0987654321").into();
        let pub_key_y2 =
            hex!("1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef").into();

        let addr1 = derive_p256_address(&pub_key_x1, &pub_key_y1);
        let addr2 = derive_p256_address(&pub_key_x2, &pub_key_y2);

        assert_ne!(
            addr1, addr2,
            "Different keys should produce different addresses"
        );
    }

    #[test]
    fn test_tempo_signature_from_bytes_secp256k1() {
        use super::SECP256K1_SIGNATURE_LENGTH;

        // Secp256k1 signatures are detected by length (65 bytes), no type identifier
        let sig_bytes = vec![0u8; SECP256K1_SIGNATURE_LENGTH];
        let result = TempoSignature::from_bytes(&sig_bytes);

        assert!(result.is_ok());
        if let TempoSignature::Primitive(PrimitiveSignature::Secp256k1(_)) = result.unwrap() {
            // Expected
        } else {
            panic!("Expected Primitive(Secp256k1) variant");
        }
    }

    #[test]
    fn test_tempo_signature_from_bytes_p256() {
        use super::{P256_SIGNATURE_LENGTH, SIGNATURE_TYPE_P256};

        let mut sig_bytes = vec![SIGNATURE_TYPE_P256];
        sig_bytes.extend_from_slice(&[0u8; P256_SIGNATURE_LENGTH]);
        let result = TempoSignature::from_bytes(&sig_bytes);

        assert!(result.is_ok());
        if let TempoSignature::Primitive(PrimitiveSignature::P256(_)) = result.unwrap() {
            // Expected
        } else {
            panic!("Expected Primitive(P256) variant");
        }
    }

    #[test]
    fn test_tempo_signature_from_bytes_webauthn() {
        use super::SIGNATURE_TYPE_WEBAUTHN;

        let mut sig_bytes = vec![SIGNATURE_TYPE_WEBAUTHN];
        sig_bytes.extend_from_slice(&[0u8; 200]); // 200 bytes of WebAuthn data
        let result = TempoSignature::from_bytes(&sig_bytes);

        assert!(result.is_ok());
        if let TempoSignature::Primitive(PrimitiveSignature::WebAuthn(_)) = result.unwrap() {
            // Expected
        } else {
            panic!("Expected Primitive(WebAuthn) variant");
        }
    }

    #[test]
    fn test_tempo_signature_from_bytes_validation() {
        // Empty input
        assert_eq!(
            TempoSignature::from_bytes(&[]).unwrap_err(),
            "Signature data is empty"
        );
        assert_eq!(
            PrimitiveSignature::from_bytes(&[]).unwrap_err(),
            "Signature data is empty"
        );

        // Too short (1 byte, not secp256k1 length)
        assert_eq!(
            TempoSignature::from_bytes(&[0x01]).unwrap_err(),
            "Signature data too short: expected type identifier + signature data"
        );

        // Wrong length for P256 (should be 129 bytes after type byte)
        let mut bad_p256 = vec![SIGNATURE_TYPE_P256];
        bad_p256.extend_from_slice(&[0u8; 100]); // wrong length
        assert_eq!(
            TempoSignature::from_bytes(&bad_p256).unwrap_err(),
            "Invalid P256 signature length"
        );

        // Wrong length for WebAuthn (too short, < 128 bytes after type byte)
        let mut bad_webauthn = vec![SIGNATURE_TYPE_WEBAUTHN];
        bad_webauthn.extend_from_slice(&[0u8; 50]); // too short
        assert_eq!(
            TempoSignature::from_bytes(&bad_webauthn).unwrap_err(),
            "Invalid WebAuthn signature length"
        );

        // Invalid type identifier
        let mut unknown_type = vec![0xFF];
        unknown_type.extend_from_slice(&[0u8; 100]);
        assert_eq!(
            TempoSignature::from_bytes(&unknown_type).unwrap_err(),
            "Unknown signature type identifier"
        );
    }

    #[test]
    fn test_tempo_signature_roundtrip() {
        use super::{
            P256_SIGNATURE_LENGTH, SECP256K1_SIGNATURE_LENGTH, SIGNATURE_TYPE_P256,
            SIGNATURE_TYPE_WEBAUTHN,
        };

        // Test secp256k1 (no type identifier, detected by 65-byte length)
        let sig1_bytes = vec![1u8; SECP256K1_SIGNATURE_LENGTH];
        let sig1 = TempoSignature::from_bytes(&sig1_bytes).unwrap();
        let encoded1 = sig1.to_bytes();
        assert_eq!(encoded1.len(), SECP256K1_SIGNATURE_LENGTH); // No type identifier
        // Verify roundtrip
        let decoded1 = TempoSignature::from_bytes(&encoded1).unwrap();
        assert_eq!(sig1, decoded1);

        // Test P256
        let mut sig2_bytes = vec![SIGNATURE_TYPE_P256];
        sig2_bytes.extend_from_slice(&[2u8; P256_SIGNATURE_LENGTH]);
        let sig2 = TempoSignature::from_bytes(&sig2_bytes).unwrap();
        let encoded2 = sig2.to_bytes();
        assert_eq!(encoded2.len(), 1 + P256_SIGNATURE_LENGTH);
        // Verify roundtrip
        let decoded2 = TempoSignature::from_bytes(&encoded2).unwrap();
        assert_eq!(sig2, decoded2);

        // Test WebAuthn
        let mut sig3_bytes = vec![SIGNATURE_TYPE_WEBAUTHN];
        sig3_bytes.extend_from_slice(&[3u8; 200]);
        let sig3 = TempoSignature::from_bytes(&sig3_bytes).unwrap();
        let encoded3 = sig3.to_bytes();
        assert_eq!(encoded3.len(), 1 + 200);
        // Verify roundtrip
        let decoded3 = TempoSignature::from_bytes(&encoded3).unwrap();
        assert_eq!(sig3, decoded3);
    }

    #[test]
    #[cfg(feature = "serde")]
    fn test_tempo_signature_serde_roundtrip() {
        // Test serde roundtrip for all signature types

        // Test Secp256k1
        let r_bytes = hex!("1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef");
        let s_bytes = hex!("fedcba0987654321fedcba0987654321fedcba0987654321fedcba0987654321");
        let sig = Signature::new(
            alloy_primitives::U256::from_be_slice(&r_bytes),
            alloy_primitives::U256::from_be_slice(&s_bytes),
            false,
        );
        let secp256k1_sig = TempoSignature::Primitive(PrimitiveSignature::Secp256k1(sig));

        let json = serde_json::to_string(&secp256k1_sig).unwrap();
        let decoded: TempoSignature = serde_json::from_str(&json).unwrap();
        assert_eq!(secp256k1_sig, decoded, "Secp256k1 serde roundtrip failed");

        // Test P256
        let p256_sig =
            TempoSignature::Primitive(PrimitiveSignature::P256(P256SignatureWithPreHash {
                r: B256::from([1u8; 32]),
                s: B256::from([2u8; 32]),
                pub_key_x: B256::from([3u8; 32]),
                pub_key_y: B256::from([4u8; 32]),
                pre_hash: true,
            }));

        let json = serde_json::to_string(&p256_sig).unwrap();
        let decoded: TempoSignature = serde_json::from_str(&json).unwrap();
        assert_eq!(p256_sig, decoded, "P256 serde roundtrip failed");

        // Verify camelCase naming
        assert!(
            json.contains("\"pubKeyX\""),
            "Should use camelCase for pubKeyX"
        );
        assert!(
            json.contains("\"pubKeyY\""),
            "Should use camelCase for pubKeyY"
        );
        assert!(
            json.contains("\"preHash\""),
            "Should use camelCase for preHash"
        );

        // Test WebAuthn
        let webauthn_sig =
            TempoSignature::Primitive(PrimitiveSignature::WebAuthn(WebAuthnSignature {
                r: B256::from([5u8; 32]),
                s: B256::from([6u8; 32]),
                pub_key_x: B256::from([7u8; 32]),
                pub_key_y: B256::from([8u8; 32]),
                webauthn_data: Bytes::from(vec![9u8; 50]),
            }));

        let json = serde_json::to_string(&webauthn_sig).unwrap();
        let decoded: TempoSignature = serde_json::from_str(&json).unwrap();
        assert_eq!(webauthn_sig, decoded, "WebAuthn serde roundtrip failed");

        // Verify camelCase naming
        assert!(
            json.contains("\"pubKeyX\""),
            "Should use camelCase for pubKeyX"
        );
        assert!(
            json.contains("\"pubKeyY\""),
            "Should use camelCase for pubKeyY"
        );
        assert!(
            json.contains("\"webauthnData\""),
            "Should use camelCase for webauthnData"
        );
    }

    #[test]
    fn test_webauthn_flag_validation() {
        let tx_hash = B256::ZERO;

        // AT flag must be rejected for assertion signatures
        let data = build_webauthn_data(0x41, None, &tx_hash); // UP + AT
        let err = verify_webauthn_data_internal(&data, &tx_hash).unwrap_err();
        assert!(err.contains("AT flag"), "Should reject AT flag");

        // ED flag must be rejected, as extensions are not supported
        let data = build_webauthn_data(0x81, Some(&[0xa0]), &tx_hash); // UP + ED, empty map
        let err = verify_webauthn_data_internal(&data, &tx_hash).unwrap_err();
        assert!(err.contains("ED flag"), "Should reject ED flag");

        // Valid with only UP flag set
        let data = build_webauthn_data(0x01, None, &tx_hash); // UP only
        assert!(
            verify_webauthn_data_internal(&data, &tx_hash).is_ok(),
            "Should accept valid webauthn data with only UP flag"
        );
    }

    #[test]
    fn test_recover_signer_p256() {
        let (signing_key, pub_key_x, pub_key_y) = generate_p256_keypair();
        let expected_address = derive_p256_address(&pub_key_x, &pub_key_y);

        let sig_hash = B256::from([0xAA; 32]);
        let (r, s) = sign_p256_normalized(&signing_key, &sig_hash);

        let p256_sig =
            TempoSignature::Primitive(PrimitiveSignature::P256(P256SignatureWithPreHash {
                r,
                s,
                pub_key_x,
                pub_key_y,
                pre_hash: false,
            }));

        let recovered = p256_sig.recover_signer(&sig_hash).unwrap();
        assert_eq!(
            recovered, expected_address,
            "P256 recovery should match derived address"
        );
    }

    #[test]
    fn test_recover_signer_p256_with_prehash() {
        let (signing_key, pub_key_x, pub_key_y) = generate_p256_keypair();
        let expected_address = derive_p256_address(&pub_key_x, &pub_key_y);

        // For pre_hash=true, signature is over sha256(sig_hash)
        let sig_hash = B256::from([0xBB; 32]);
        let prehashed = B256::from_slice(Sha256::digest(sig_hash).as_ref());
        let (r, s) = sign_p256_normalized(&signing_key, &prehashed);

        let p256_sig =
            TempoSignature::Primitive(PrimitiveSignature::P256(P256SignatureWithPreHash {
                r,
                s,
                pub_key_x,
                pub_key_y,
                pre_hash: true,
            }));

        let recovered = p256_sig.recover_signer(&sig_hash).unwrap();
        assert_eq!(
            recovered, expected_address,
            "P256 pre_hash recovery should match"
        );
    }

    #[test]
    fn test_recover_signer_p256_high_s_rejected() {
        let (signing_key, pub_key_x, pub_key_y) = generate_p256_keypair();
        let sig_hash = B256::from([0xCD; 32]);
        let signature: p256::ecdsa::Signature =
            signing_key.sign_prehash(sig_hash.as_slice()).unwrap();
        let sig_bytes = signature.to_bytes();
        let r = B256::from_slice(&sig_bytes[..32]);
        let s_value = U256::from_be_slice(&sig_bytes[32..64]);
        let high_s = if s_value > P256N_HALF {
            s_value
        } else {
            P256_ORDER - s_value
        };

        let p256_sig =
            TempoSignature::Primitive(PrimitiveSignature::P256(P256SignatureWithPreHash {
                r,
                s: B256::from(high_s.to_be_bytes::<32>()),
                pub_key_x,
                pub_key_y,
                pre_hash: false,
            }));

        assert!(
            p256_sig.recover_signer(&sig_hash).is_err(),
            "high-s P256 signatures must be rejected"
        );
    }

    #[test]
    fn test_recover_signer_webauthn() {
        let (signing_key, pub_key_x, pub_key_y) = generate_p256_keypair();
        let expected_address = derive_p256_address(&pub_key_x, &pub_key_y);

        let tx_hash = B256::from([0xCC; 32]);
        let webauthn_data = build_webauthn_data(0x01, None, &tx_hash);

        let message_hash = verify_webauthn_data_internal(&webauthn_data, &tx_hash).unwrap();

        let (r, s) = sign_p256_normalized(&signing_key, &message_hash);

        let webauthn_sig =
            TempoSignature::Primitive(PrimitiveSignature::WebAuthn(WebAuthnSignature {
                r,
                s,
                pub_key_x,
                pub_key_y,
                webauthn_data: Bytes::from(webauthn_data),
            }));

        let recovered = webauthn_sig.recover_signer(&tx_hash).unwrap();
        assert_eq!(
            recovered, expected_address,
            "WebAuthn recovery should match derived address"
        );
    }

    #[test]
    fn test_recover_signer_webauthn_invalid_payload_rejected() {
        let (signing_key, pub_key_x, pub_key_y) = generate_p256_keypair();
        let tx_hash = B256::from([0xEF; 32]);
        let (r, s) = sign_p256_normalized(&signing_key, &B256::ZERO);

        let invalid_webauthn_sig =
            TempoSignature::Primitive(PrimitiveSignature::WebAuthn(WebAuthnSignature {
                r,
                s,
                pub_key_x,
                pub_key_y,
                webauthn_data: Bytes::from(build_webauthn_data(0x41, None, &tx_hash)),
            }));

        assert!(
            invalid_webauthn_sig.recover_signer(&tx_hash).is_err(),
            "invalid WebAuthn payloads must be rejected"
        );
    }

    #[test]
    fn test_recover_signer_keychain_v1() {
        use crate::transaction::tt_authorization::tests::{generate_secp256k1_keypair, sign_hash};

        let (signing_key, access_key_address) = generate_secp256k1_keypair();
        let user_address = Address::repeat_byte(0xDD);

        // V1: inner signature signs sig_hash directly
        let sig_hash = B256::from([0x22; 32]);
        let inner_sig = sign_hash(&signing_key, &sig_hash);

        let keychain_sig = TempoSignature::Keychain(KeychainSignature::new_v1(
            user_address,
            match inner_sig {
                TempoSignature::Primitive(p) => p,
                _ => panic!("Expected primitive signature"),
            },
        ));

        // recover_signer returns user_address
        let recovered = keychain_sig.recover_signer(&sig_hash).unwrap();
        assert_eq!(
            recovered, user_address,
            "Keychain V1 recovery should return user_address"
        );

        // key_id should be cached and return access key address
        let keychain = keychain_sig.as_keychain().unwrap();
        let key_id = keychain.key_id(&sig_hash).unwrap();
        assert_eq!(
            key_id, access_key_address,
            "key_id should return access key address"
        );

        // V1 should be legacy
        assert!(keychain_sig.is_legacy_keychain());
    }

    #[test]
    fn test_recover_signer_keychain_v2() {
        use crate::transaction::tt_authorization::tests::{generate_secp256k1_keypair, sign_hash};

        let (signing_key, access_key_address) = generate_secp256k1_keypair();
        let user_address = Address::repeat_byte(0xDD);

        // V2: inner signature signs keccak256(0x04 || sig_hash || user_address)
        let sig_hash = B256::from([0x22; 32]);
        let mut buf = [0u8; 53]; // 1 + 32 + 20
        buf[0] = SIGNATURE_TYPE_KEYCHAIN_V2;
        buf[1..33].copy_from_slice(sig_hash.as_slice());
        buf[33..].copy_from_slice(user_address.as_slice());
        let effective_hash = keccak256(buf);
        let inner_sig = sign_hash(&signing_key, &effective_hash);

        let keychain_sig = TempoSignature::Keychain(KeychainSignature::new(
            user_address,
            match inner_sig {
                TempoSignature::Primitive(p) => p,
                _ => panic!("Expected primitive signature"),
            },
        ));

        // recover_signer returns user_address
        let recovered = keychain_sig.recover_signer(&sig_hash).unwrap();
        assert_eq!(
            recovered, user_address,
            "Keychain V2 recovery should return user_address"
        );

        // key_id should be cached and return access key address
        let keychain = keychain_sig.as_keychain().unwrap();
        let key_id = keychain.key_id(&sig_hash).unwrap();
        assert_eq!(
            key_id, access_key_address,
            "key_id should return access key address"
        );

        // V2 should NOT be legacy
        assert!(!keychain_sig.is_legacy_keychain());
    }

    #[test]
    fn test_keychain_v2_binds_user_address() {
        use crate::transaction::tt_authorization::tests::{generate_secp256k1_keypair, sign_hash};

        let (signing_key, _access_key_address) = generate_secp256k1_keypair();
        let user_a = Address::repeat_byte(0xAA);
        let user_b = Address::repeat_byte(0xBB);

        // Sign for user_a with V2
        let sig_hash = B256::from([0x22; 32]);
        let effective_hash = KeychainSignature::signing_hash(sig_hash, user_a);
        let inner_sig = sign_hash(&signing_key, &effective_hash);

        let inner_primitive = match inner_sig {
            TempoSignature::Primitive(p) => p,
            _ => panic!("Expected primitive signature"),
        };

        // Valid for user_a
        let sig_a =
            TempoSignature::Keychain(KeychainSignature::new(user_a, inner_primitive.clone()));
        let recovered_a = sig_a.recover_signer(&sig_hash).unwrap();
        assert_eq!(recovered_a, user_a);

        // Same inner signature but for user_b — key_id will differ
        // because user_address is part of the signed hash
        let sig_b = TempoSignature::Keychain(KeychainSignature::new(user_b, inner_primitive));
        let recovered_b = sig_b.recover_signer(&sig_hash).unwrap();
        assert_eq!(
            recovered_b, user_b,
            "recover_signer returns the claimed user_address"
        );

        // But the key_id recovered under user_b will be a garbage address (not the real access key)
        let key_id_a = sig_a.as_keychain().unwrap().key_id(&sig_hash).unwrap();
        let key_id_b = sig_b.as_keychain().unwrap().key_id(&sig_hash).unwrap();
        assert_ne!(
            key_id_a, key_id_b,
            "V2 should recover different key_ids for different user_addresses"
        );
    }

    #[test]
    fn test_signing_hash_properties() {
        let hash_a = B256::from([0x11; 32]);
        let hash_b = B256::from([0x22; 32]);
        let addr_a = Address::repeat_byte(0xAA);
        let addr_b = Address::repeat_byte(0xBB);

        // Different addresses produce different signing hashes
        assert_ne!(
            KeychainSignature::signing_hash(hash_a, addr_a),
            KeychainSignature::signing_hash(hash_a, addr_b),
        );

        // Different tx hashes produce different signing hashes
        assert_ne!(
            KeychainSignature::signing_hash(hash_a, addr_a),
            KeychainSignature::signing_hash(hash_b, addr_a),
        );

        // Deterministic: same inputs yield same output
        assert_eq!(
            KeychainSignature::signing_hash(hash_a, addr_a),
            KeychainSignature::signing_hash(hash_a, addr_a),
        );
    }

    #[test]
    fn test_webauthn_rejects_challenge_injection() {
        let (tx_hash, attack_hash) = (B256::from([0xAA; 32]), B256::from([0xFF; 32]));
        let (challenge, attack_challenge) = (
            URL_SAFE_NO_PAD.encode(tx_hash.as_slice()),
            URL_SAFE_NO_PAD.encode(attack_hash.as_slice()),
        );

        // Ensure that the happy path works
        let valid_payload = format!(r#"{{"type":"webauthn.get","challenge":"{challenge}"}}"#);

        let mut auth_data = vec![0u8; 37];
        auth_data[32] = 0x01;
        let mut webauthn_data = auth_data;
        webauthn_data.extend_from_slice(valid_payload.as_bytes());

        let result = verify_webauthn_data_internal(&webauthn_data, &tx_hash);
        assert!(result.is_ok());

        // Ensure that malicious payloads cannot pass validation
        let attack_variants = [
            format!(
                r#"{{"type":"webauthn.get","challenge":"{attack_challenge}","extra":{{"challenge":"{challenge}"}}}}"#
            ),
            format!(
                r#"{{"type":"webauthn.get","data":[{{"challenge":"{challenge}"}}],"challenge":"{attack_challenge}"}}"#
            ),
        ];

        for (i, attack_json) in attack_variants.iter().enumerate() {
            let mut auth_data = vec![0u8; 37];
            auth_data[32] = 0x01;
            let mut webauthn_data = auth_data;
            webauthn_data.extend_from_slice(attack_json.as_bytes());

            let result = verify_webauthn_data_internal(&webauthn_data, &tx_hash);
            assert!(
                result.is_err(),
                "Attack variant {i} should be rejected: {attack_json}"
            );
        }
    }

    #[test]
    fn test_keychain_signature_eq_same() {
        let sig = PrimitiveSignature::Secp256k1(Signature::test_signature());
        let addr = Address::repeat_byte(0x01);
        let a = KeychainSignature::new(addr, sig.clone());
        let b = KeychainSignature::new(addr, sig);
        assert_eq!(a, b);
    }

    #[test]
    fn test_keychain_signature_eq_different_address() {
        let sig = PrimitiveSignature::Secp256k1(Signature::test_signature());
        let a = KeychainSignature::new(Address::repeat_byte(0x01), sig.clone());
        let b = KeychainSignature::new(Address::repeat_byte(0x02), sig);
        assert_ne!(a, b);
    }

    #[test]
    fn test_keychain_signature_eq_different_signature() {
        let addr = Address::repeat_byte(0x01);
        let sig_a = PrimitiveSignature::Secp256k1(Signature::test_signature());
        let sig_b = PrimitiveSignature::P256(P256SignatureWithPreHash {
            r: B256::from([1u8; 32]),
            s: B256::from([2u8; 32]),
            pub_key_x: B256::from([3u8; 32]),
            pub_key_y: B256::from([4u8; 32]),
            pre_hash: false,
        });
        let a = KeychainSignature::new(addr, sig_a);
        let b = KeychainSignature::new(addr, sig_b);
        assert_ne!(a, b);
    }

    #[test]
    fn test_keychain_signature_hash_differs_for_different_sigs() {
        use std::{
            collections::hash_map::DefaultHasher,
            hash::{Hash, Hasher},
        };

        let sig = PrimitiveSignature::Secp256k1(Signature::test_signature());
        let a = KeychainSignature::new(Address::repeat_byte(0x01), sig.clone());
        let b = KeychainSignature::new(Address::repeat_byte(0x02), sig.clone());
        let c = KeychainSignature::new(Address::repeat_byte(0x01), sig);

        let hash = |k: &KeychainSignature| {
            let mut h = DefaultHasher::new();
            k.hash(&mut h);
            h.finish()
        };

        assert_ne!(
            hash(&a),
            hash(&b),
            "different address should produce different hash"
        );
        assert_eq!(hash(&a), hash(&c), "same fields should produce same hash");
    }

    #[test]
    fn test_primitive_signature_from_bytes_one_byte() {
        let result = PrimitiveSignature::from_bytes(&[0x01]);
        assert!(result.is_err());
        assert!(result.unwrap_err().contains("too short"));
    }

    #[test]
    fn test_tempo_signature_keychain_too_short_for_address() {
        for type_byte in [SIGNATURE_TYPE_KEYCHAIN, SIGNATURE_TYPE_KEYCHAIN_V2] {
            let mut data = vec![type_byte];
            data.extend_from_slice(&[0u8; 19]);
            let result = TempoSignature::from_bytes(&data);
            assert!(result.is_err());
            assert!(result.unwrap_err().contains("too short"));
        }
    }

    #[test]
    fn test_tempo_signature_keychain_exactly_20_bytes_inner_empty() {
        let mut data = vec![SIGNATURE_TYPE_KEYCHAIN];
        data.extend_from_slice(&[0u8; 20]);
        let result = TempoSignature::from_bytes(&data);
        assert!(result.is_err());
    }

    #[test]
    fn test_is_keychain_returns_false_for_primitive() {
        let sig =
            TempoSignature::Primitive(PrimitiveSignature::Secp256k1(Signature::test_signature()));
        assert!(!sig.is_keychain());
    }

    #[test]
    fn test_is_keychain_returns_true_for_keychain() {
        let inner = PrimitiveSignature::Secp256k1(Signature::test_signature());
        let sig = TempoSignature::Keychain(KeychainSignature::new(Address::ZERO, inner));
        assert!(sig.is_keychain());
    }

    #[test]
    fn test_keychain_v1_v2_bytes_roundtrip_and_wire_format() {
        let inner = PrimitiveSignature::Secp256k1(Signature::test_signature());
        let user = Address::repeat_byte(0xAA);

        // V1 round-trips and uses 0x03 wire byte
        let v1 = TempoSignature::Keychain(KeychainSignature::new_v1(user, inner.clone()));
        let v1_bytes = v1.to_bytes();
        assert_eq!(v1_bytes[0], SIGNATURE_TYPE_KEYCHAIN);
        let v1_decoded = TempoSignature::from_bytes(&v1_bytes).unwrap();
        assert_eq!(v1, v1_decoded);
        assert!(v1_decoded.is_legacy_keychain());

        // V2 round-trips and uses 0x04 wire byte
        let v2 = TempoSignature::Keychain(KeychainSignature::new(user, inner));
        let v2_bytes = v2.to_bytes();
        assert_eq!(v2_bytes[0], SIGNATURE_TYPE_KEYCHAIN_V2);
        let v2_decoded = TempoSignature::from_bytes(&v2_bytes).unwrap();
        assert_eq!(v2, v2_decoded);
        assert!(!v2_decoded.is_legacy_keychain());

        // V1 and V2 with same inner sig are NOT equal
        assert_ne!(v1, v2);
    }

    #[test]
    #[cfg(feature = "serde")]
    fn test_keychain_serde_roundtrip_and_backward_compat() {
        let inner = PrimitiveSignature::Secp256k1(Signature::test_signature());
        let user = Address::repeat_byte(0xBB);

        // V2 serde roundtrip preserves version
        let v2 = TempoSignature::Keychain(KeychainSignature::new(user, inner.clone()));
        let json = serde_json::to_string(&v2).unwrap();
        let decoded: TempoSignature = serde_json::from_str(&json).unwrap();
        assert_eq!(v2, decoded);
        assert!(!decoded.is_legacy_keychain());

        // V1 serde roundtrip preserves version
        let v1 = TempoSignature::Keychain(KeychainSignature::new_v1(user, inner));
        let json_v1 = serde_json::to_string(&v1).unwrap();
        let decoded_v1: TempoSignature = serde_json::from_str(&json_v1).unwrap();
        assert_eq!(v1, decoded_v1);
        assert!(decoded_v1.is_legacy_keychain());

        // Backward compat: JSON without "version" field deserializes as V1
        let json_no_version = json_v1.replace(r#","version":"v1""#, "");
        assert!(
            !json_no_version.contains("version"),
            "version field should be stripped"
        );
        let decoded_no_version: TempoSignature = serde_json::from_str(&json_no_version).unwrap();
        assert!(decoded_no_version.is_legacy_keychain());
    }

    #[test]
    fn test_keychain_rlp_roundtrip_preserves_version() {
        use alloy_rlp::Decodable;

        let inner = PrimitiveSignature::Secp256k1(Signature::test_signature());
        let user = Address::repeat_byte(0xCC);

        for (sig, expect_legacy) in [
            (
                TempoSignature::Keychain(KeychainSignature::new_v1(user, inner.clone())),
                true,
            ),
            (
                TempoSignature::Keychain(KeychainSignature::new(user, inner)),
                false,
            ),
        ] {
            let mut buf = Vec::new();
            alloy_rlp::Encodable::encode(&sig, &mut buf);
            let decoded = TempoSignature::decode(&mut buf.as_slice()).unwrap();
            assert_eq!(sig, decoded);
            assert_eq!(decoded.is_legacy_keychain(), expect_legacy);
        }
    }
}

