Skip to content

add ecdsa support#178

Open
childhoodisend wants to merge 1 commit intoaspectron:wallet-daemonfrom
childhoodisend:feature/ecdsa-support
Open

add ecdsa support#178
childhoodisend wants to merge 1 commit intoaspectron:wallet-daemonfrom
childhoodisend:feature/ecdsa-support

Conversation

@childhoodisend
Copy link

No description provided.

@childhoodisend childhoodisend changed the title feat: add ecdsa support add ecdsa support Dec 14, 2025
@childhoodisend childhoodisend force-pushed the feature/ecdsa-support branch 4 times, most recently from 784bfb4 to a8033b1 Compare December 15, 2025 08:19
@biryukovmaxim
Copy link
Collaborator

ECDSA Support Review — Wallet Integration Analysis

This PR adds the low-level ECDSA primitives (tx signing, message signing/verification) but does not wire them into the wallet account lifecycle. The infrastructure exists at the storage layer (ecdsa: bool in account payloads) — and that part is backward-compatible — but nothing in the Account trait, CLI, or transaction-signing flow actually uses it. Below is a detailed gap analysis.


1. Transaction Signing Always Defaults to Schnorr

Files: wallet/core/src/account/mod.rs (lines 324, 363, 528)

Every transaction method (sweep, send, transfer) constructs the signer as:

let signer = Arc::new(Signer::new(self.clone().as_dyn_arc(), keydata, payment_secret));

Signer::new() hardcodes SignatureType::Schnorr. The new Signer::new_with_signature_type() is dead code — never called anywhere.

What needs to change: The Account trait needs a method to expose the account's signature mode (e.g. fn signature_type(&self) -> SignatureType), defaulting to Schnorr. Account implementations that store ecdsa: bool (Bip32, Keypair, Multisig) would override it. Then sweep/send/transfer should call Signer::new_with_signature_type(…, self.signature_type()).


2. Keypair::receive_address() / change_address() Ignore ecdsa Flag

File: wallet/core/src/account/variants/keypair.rs (lines 145-153)

fn receive_address(&self) -> Result<Address> {
    let (xonly_public_key, _) = self.public_key.x_only_public_key();
    Ok(Address::new(, Version::PubKey, &xonly_public_key.serialize()))
}

This always uses Version::PubKey with the 32-byte x-only key, regardless of self.ecdsa. When ecdsa == true, it should use Version::PubKeyECDSA with the full 33-byte compressed public key (self.public_key.serialize()). Without this fix, an ECDSA keypair account would generate Schnorr addresses, making it fundamentally broken — UTXOs would be locked to Schnorr script but signed with ECDSA.


3. PSKB (Partially Signed Kaspa Transaction Bundle) Signing Also Ignores ECDSA

File: wallet/core/src/account/mod.rs (lines 450, 464) and wallet/core/src/account/pskb.rs

The PSKBSigner used in pskb_from_send_generator and pskb_sign has no SignatureType awareness at all. It always uses Schnorr signing. The same pattern needs to be applied — PSKBSigner needs a signature type parameter.


4. CLI: No Way to Create ECDSA Accounts

File: cli/src/wizards/account.rs

The create wizard only supports BIP32 account creation via AccountCreateArgsBip32. There is:

  • No CLI path to create a Keypair account (with or without ECDSA)
  • No --ecdsa flag on any account creation command
  • The account create help text lists only bip32, legacy, multisig

What needs to change: Add a keypair account kind to the CLI, with an optional --ecdsa flag. Also consider adding --ecdsa to bip32 and multisig creation, since those account types also store the ecdsa flag.


5. CLI Message Signing Hardcodes Schnorr

File: cli/src/modules/message.rs (line 91)

let sign_options = SignMessageOptions { no_aux_rand: false, signature_type: SignatureType::Schnorr };

Always signs with Schnorr, regardless of the account type. The sign command should detect the account's signature type and use the appropriate algorithm.

Additionally, message sign and message verify explicitly reject non-PubKey addresses (line 85-87, 107-109):

if kaspa_address.version != Version::PubKey {
    return Err(Error::custom("Address not supported for message signing. Only supports PubKey addresses"));
}

This blocks PubKeyECDSA addresses entirely. The verify path also only calls verify_message (Schnorr), never verify_message_ecdsa.


6. Duplicate SignatureType Enum

Files: wallet/core/src/tx/generator/signer.rs and wallet/core/src/message.rs

Two separate SignatureType enums are defined:

  • crate::tx::generator::signer::SignatureType (for tx signing)
  • crate::message::SignatureType (for message signing)

These should be unified into a single type (e.g. in a common module) to avoid confusion and ensure consistency.


7. Storage Layer — Backward Compatibility (Good News)

The storage layer is already backward-compatible. The ecdsa: bool flag is part of the Borsh-serialized payload for Bip32, Keypair, Multisig, and Bip32Watch accounts, and has been there before this PR. The STORAGE_VERSION is still 0 for all account types. No storage migration is needed. Existing accounts with ecdsa: false will continue to work unchanged.


Summary: What Needs to Change

Area Status Action Required
consensus_core::sign::sign_with_multiple_ecdsa ✅ Done
message::sign_message / verify_message_ecdsa ✅ Done
WASM signMessage / verifyMessage ✅ Done
Storage layer backward compat ✅ OK
Account trait: expose signature_type() ❌ Missing Add trait method
Keypair::receive_address / change_address ❌ Bug Use PubKeyECDSA when ecdsa == true
Account::send/sweep/transfer signer ❌ Missing Use new_with_signature_type
PSKBSigner ECDSA support ❌ Missing Add signature type awareness
CLI: ECDSA account creation ❌ Missing Add keypair account kind + --ecdsa flag
CLI: message sign / message verify ❌ Missing Detect account type, support ECDSA
Duplicate SignatureType enums ⚠️ Cleanup Unify into single type

The core cryptographic work is solid. What's missing is the plumbing to make wallet accounts actually use ECDSA end-to-end: address derivation, transaction signing, and CLI exposure.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants