non evm 1ca

[llbartekll]

Description

Resolves # (issue)

How Has This Been Tested?

Due Dilligence

• Breaking change
• Requires a documentation update

[sonarqubecloud]

Quality Gate passed

Issues
7 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[llbartekll]

@claude review

[claude]

Claude finished @llbartekll's task —— View job

---

Code Review Complete

• Read repository guidelines (CLAUDE.md)
• Examine PR diff and commit history
• Review key changed files for issues
• Check for security concerns
• Validate Swift best practices
• Scan for external URLs
• Provide comprehensive feedback

---

Overview

This PR implements CAIP-122 "Sign with X" support to enable non-EVM signature types (Bitcoin, Solana) for 1-Click Authentication (1CA). The implementation extends the existing EVM-only signature verification to support multiple blockchain types.

Critical Issues

1. Hardcoded Signature Type Verification Only Supports EVM ⚠️

File: Sources/WalletConnectSign/Auth/Services/App/AuthSignatureVerifier.swift:32

guard authObject.s.t.namespace == "eip155" else {
    throw Errors.unsupportedSignatureType(...)
}

Issue: The new AuthSignatureVerifier explicitly rejects non-EVM signatures, even though the PR adds infrastructure for Bitcoin and Solana. This contradicts the PR's stated purpose of "non evm 1ca".

Impact: Despite adding CacaoSignatureType support for bip122 and solana, the signature verifier will reject any non-EVM authentication attempts at runtime.

Recommendation: Either:

• Remove the EVM-only guard and implement proper verification for all supported signature types, OR
• If this is intentional (phased rollout), add clear documentation explaining that non-EVM signature verification will be added in a future PR

---

2. Example App Hardcodes EIP-191 Signature Type ⚠️

File: Example/WalletApp/PresentationLayer/Wallet/SessionProposal/SessionProposalPresenter.swift:114

let signature = try messageSigner.sign(message: SIWEmessages, privateKey: Data(hex: importAccount.privateKey), type: .eip155(.eip191))

Issue: The example app always signs with .eip155(.eip191) regardless of the actual chain type requested in the auth payload. For Bitcoin or Solana chains, this will produce invalid signatures.

Impact: Non-EVM authentication requests will fail in the example wallet app.

Recommendation:

// Determine signature type based on chain namespace
let signatureType: CacaoSignatureType = {
    switch chain.namespace {
    case "eip155": return .eip155(.eip191) // or detect if EIP-1271/EIP-6492 is needed
    case "bip122": return .bip122(.ecdsa) // or .bip322Simple
    case "solana": return .solana(.ed25519)
    default: throw Errors.unsupportedNamespace
    }
}()
let signature = try messageSigner.sign(message: SIWEmessages, privateKey: privateKey, type: signatureType)

---

3. Example App Limits to EVM Chains Only ⚠️

File: Example/WalletApp/PresentationLayer/Wallet/SessionProposal/SessionProposalPresenter.swift:137

let supportedChains: Set<Blockchain> = [Blockchain("eip155:1")!, Blockchain("eip155:137")!]
return requestedChains.intersection(supportedChains)

Issue: The example wallet only supports Ethereum mainnet and Polygon, filtering out all Bitcoin and Solana chains.

Impact: Users cannot test non-EVM authentication flows in the example app, even though the SDK now supports them.

Recommendation: Expand supported chains to include test Bitcoin and Solana chains:

let supportedChains: Set<Blockchain> = [
    Blockchain("eip155:1")!,      // Ethereum Mainnet
    Blockchain("eip155:137")!,    // Polygon
    Blockchain("bip122:000000000019d6689c085ae165831e93")!, // Bitcoin Mainnet
    Blockchain("solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp")! // Solana Mainnet
]

---

Major Issues

4. Missing Signature Type Selection Logic

Files: Sources/WalletConnectSign/Auth/Types/AuthRequestParams.swift, Sources/WalletConnectSign/Auth/Types/AuthPayload.swift

Issue: The signatureTypes field is added to auth request params but there's no logic to:

• Select which signature type to use when multiple are offered per chain
• Validate that the signature type matches the chain namespace
• Handle fallback if a preferred signature type isn't available

Recommendation: Add helper methods:

extension AuthPayload {
    func getSupportedSignatureTypes(for namespace: String) -> [CacaoSignatureType]? {
        return signatureTypes?[namespace]
    }
    
    func selectSignatureType(for namespace: String, preferring preferred: CacaoSignatureType? = nil) -> CacaoSignatureType? {
        guard let types = signatureTypes?[namespace], !types.isEmpty else { return nil }
        if let preferred = preferred, types.contains(preferred) { return preferred }
        return types.first
    }
}

---

5. Incomplete Error Messages for Signature Type Validation

File: Sources/WalletConnectSign/Auth/Services/App/AuthSignatureVerifier.swift:58-66

private func getRecommendedVerifier(for signatureType: CacaoSignatureType) -> String {
    switch signatureType.namespace {
    case "bip122":
        return "Use a Bitcoin signature verifier for ECDSA and BIP-322 signatures"
    case "solana":
        return "Use a Solana signature verifier for Ed25519 signatures"
    default:
        return "Use a signature verifier specific to the \(signatureType.namespace) namespace"
    }
}

Issue: While helpful error messages are provided, they don't guide developers on how to implement or where to find these verifiers.

Recommendation: Enhance error messages with actionable guidance or documentation links.

---

6. CacaoSignature Codable Implementation is Fragile

File: Sources/WalletConnectUtils/Cacao/CacaoSignature.swift:73-86

public init(from decoder: Decoder) throws {
    let container = try decoder.singleValueContainer()
    let stringValue = try container.decode(String.self)
    
    // Try to find which namespace supports this algorithm
    for namespace in Self.supportedNamespaces {
        if let signatureType = CacaoSignatureType.from(namespace: namespace, algorithm: stringValue) {
            self = signatureType
            return
        }
    }
    
    throw DecodingError.dataCorruptedError(in: container, debugDescription: "Unknown signature type: \(stringValue)")
}

Issue: The decoder iterates through all namespaces trying to match algorithms. This could lead to ambiguity if multiple namespaces use the same algorithm name (e.g., "ecdsa" could theoretically be used by multiple chains).

Impact: Currently works because algorithm names are unique across the three supported namespaces, but this could break if more chains are added with overlapping algorithm names.

Recommendation: Consider encoding the full type as "namespace:algorithm" (e.g., "eip155:eip191", "bip122:ecdsa") for unambiguous deserialization, or document that algorithm names must be globally unique.

---

Minor Issues

7. Unused WalletPayParams Field in Session Proposal

Files: Sources/WalletConnectSign/Types/WalletPayParams.swift, Sources/WalletConnectSign/Services/App/AppProposeService.swift:32

Issue: WalletPayParams is added to the connect() method signature but is never used in AppProposeService.propose(). The walletPay parameter is accepted but not included in the proposal.

Recommendation: Either:

• Implement wallet pay functionality by adding it to the session proposal, OR
• Remove the unused parameter from the public API if it's not ready yet

---

8. Typo in Error Case

File: Sources/WalletConnectSign/Engine/Common/ApproveEngine.swift:13

case emtySessionNamespacesForbidden

Issue: "emty" should be "empty"

Recommendation: Rename to emptySessionNamespacesForbidden for consistency.

---

9. Test Coverage Gap

File: Tests/WalletConnectSignTests/SIWEFromCacaoPayloadFormatterTests.swift

Observation: Tests cover encoding/decoding of CacaoSignatureType and message formatting for Bitcoin/Solana, but there are no integration tests for:

• End-to-end non-EVM authentication flows
• Signature verification with non-EVM signatures
• Error handling when signature types mismatch chain namespaces

Recommendation: Add integration tests to validate the complete non-EVM authentication flow.

---

10. Missing Documentation for Public API Changes

Files: Sources/WalletConnectSign/Sign/SignClient.swift:299, Sources/WalletConnectSign/Sign/SignClient.swift:422

Issue: New public API methods connect(authentication:walletPay:) and recoverAndVerifySignature() lack comprehensive documentation explaining:

• When to use authentication parameter
• Format of signatureTypes dictionary in AuthRequestParams
• Which signature types are currently verifiable vs. planned for future

Recommendation: Add detailed DocC documentation with code examples.

---

Security Considerations

11. No Validation of Signature Type Against Chain Namespace 🔒

Impact: A malicious or buggy dApp could request Bitcoin authentication but provide an Ethereum signature type in signatureTypes. Without validation, the wallet might attempt to sign with the wrong algorithm.

Recommendation: Add validation in AuthPayloadBuilder or CacaosBuilder:

func validateSignatureType(_ signatureType: CacaoSignatureType, for chain: Blockchain) throws {
    guard signatureType.namespace == chain.namespace else {
        throw ValidationError.signatureTypeMismatch(
            signatureNamespace: signatureType.namespace,
            chainNamespace: chain.namespace
        )
    }
}

---

12. Signature Verification is Async but Not Cancellable

Files: AuthSignatureVerifier.swift:30, ApproveSessionAuthenticateUtil.swift:135

Observation: Signature verification uses async throws but doesn't support cancellation. If multiple authentication attempts are made rapidly, old verifications could still be running.

Recommendation: Consider adding cancellation support using Task or ensuring only one verification runs at a time.

---

Best Practices

13. Magic Numbers in TTL Validation

File: Sources/WalletConnectSign/Auth/Types/AuthRequestParams.swift:25-26

static let minTtl: TimeInterval = 300    // 5 minutes
static let maxTtl: TimeInterval = 604800 // 7 days

Good: Clear constants with inline comments.

Suggestion: Consider extracting these to a shared configuration if they're used elsewhere, or document why these specific values were chosen (security vs. UX tradeoff).

---

14. Force Try in Production Code

File: Sources/WalletConnectSign/Services/App/AppProposeService.swift:49

let publicKey = try! kms.createX25519KeyPair()

Issue: Force try will crash the app if key generation fails (e.g., on jailbroken devices with compromised crypto libraries).

Recommendation:

guard let publicKey = try? kms.createX25519KeyPair() else {
    throw SessionProposeError.keyGenerationFailed
}

---

15. Inconsistent Error Handling Patterns

Observation: Some methods use LocalizedError conformance (AuthSignatureVerifier.Errors), others use plain Error enum, and some use generic error throwing.

Recommendation: Standardize on LocalizedError conformance for all public-facing errors to provide better UX.

---

Code Quality Observations

✅ Well Done

1. Comprehensive Test Coverage for Message Formatting: The SIWEFromCacaoPayloadFormatterTests thoroughly tests Bitcoin, Solana, and edge cases.

2. Type-Safe Signature Types: Using Swift enums with associated values for CacaoSignatureType is excellent for compile-time safety.

3. Clear Migration Path: Providing SignWithXFormatting as the new protocol name while keeping SIWEFromCacaoFormatting as a typealias is good for backwards compatibility.

4. Defensive Coding: The formatter throws errors for unsupported namespaces rather than failing silently.

---

External URLs

All URLs found point to approved domains (github.com, walletconnect.com, ethereum.org, IETF RFC specs) or are test/example data. No security concerns.

---

Summary

This PR makes significant progress toward non-EVM authentication support but is not yet functional for Bitcoin or Solana due to:

1. Signature verifier explicitly rejecting non-EVM signatures (Monorepo #1)
2. Example app hardcoded to EVM-only chains and signature types (1.0.0 #2, Monorepo #3)

Recommendation: Before merging, either:

• Option A: Complete the non-EVM implementation by addressing issues Monorepo #1-Monorepo #3, OR
• Option B: Clarify this is "Phase 1" (infrastructure only) and create follow-up issues for actual verification support

The architecture and type system changes are solid, but the runtime behavior doesn't yet match the PR's stated goal.

---

Job Run | Branch: signature-types

[llbartekll]

remove from the client interface?