Skip to content

Wire secp256k1 eth [2/4]#25

Merged
Eric-Warehime merged 11 commits into
mainfrom
eric/wire-supported-algos
Jul 17, 2026
Merged

Wire secp256k1 eth [2/4]#25
Eric-Warehime merged 11 commits into
mainfrom
eric/wire-supported-algos

Conversation

@Eric-Warehime

Copy link
Copy Markdown
Collaborator

Wires the config, privval adapter, and validation for secp256k1eth

@Eric-Warehime
Eric-Warehime requested a review from a team as a code owner July 15, 2026 03:12
@greptile-apps

greptile-apps Bot commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR wires secp256k1eth support across the config, privval adapter, and PKCS11/GRPC/AWSKMS backends. The core change is in privkey_adapter.go, where a Keccak256 pre-hash is applied before calling the underlying signer for the secp256k1eth scheme — matching the hashing contract that cometbft/crypto/secp256k1eth uses for verification.

  • Privval adapter: signerPrivKey.Sign now applies sha3.NewLegacyKeccak256() when Scheme() == AlgoSecp256k1Eth, with a new end-to-end test proving the round-trip verifies against raw sign-bytes.
  • PKCS11: secp256k1eth is added to the algos registry using CKM_ECDSA + recoverSig; the adapter delivers a 32-byte Keccak256 digest so the HSM performs raw ECDSA and recoverSig correctly identifies the recovery byte.
  • Config/GRPC: AlgoSecp256k1Eth is added to supportedPKCS11Algorithms and supportedAWSKMSAlgorithms; newGRPCSigner gains a file/ed25519 case alongside file/secp256k1eth.

Confidence Score: 4/5

The core signing path and PKCS11 wiring are correct and backed by a real round-trip test; the only gap is a missing algorithm check in GRPC file-backend config validation.

The Keccak256 pre-hash, pubkey wiring, PKCS11 registry entry, and test coverage all look correct. One gap stands out: validateGRPC for BackendFile never checks the algorithm, so an operator who configures algorithm: secp256k1 (or any other unsupported value) with backend: file in the GRPC block passes config validation and gets a startup-time error instead of a clear config error. This is a quality issue rather than a correctness flaw in the happy path.

config/validate.go — the validateGRPC BackendFile case should enforce the {ed25519, secp256k1eth} algorithm set, mirroring the guards already present for AWSKMS and PKCS11.

Important Files Changed

Filename Overview
internal/signer/privkey_adapter.go Adds Keccak256 pre-hashing in Sign() for secp256k1eth scheme and wires the new pubkey type via cometsecpeth.NewPubKeyFromBytes; logic is correct and tested.
internal/signer/privkey_adapter_test.go Replaces the stub-only eth test with a real file.GenerateSecp256k1Eth round-trip test proving the adapter's Keccak256 pre-hash matches cometbft secp256k1eth verification.
config/validate.go Adds AlgoSecp256k1Eth to PKCS11 and AWSKMS algorithm sets; GRPC file backend still lacks an algorithm validation check against the supported {ed25519, secp256k1eth} set.
internal/app/build.go Adds file/ed25519 case to newGRPCSigner and broadens the AWSKMS case to accept any algorithm validated by config; changes are consistent with the updated validation maps.
signing/pkcs11/algo.go Adds secp256k1eth entry to the PKCS11 algo registry using CKM_ECDSA + recoverSig; correctly receives the pre-hashed 32-byte digest from the adapter.
signing/ecdsasig/ecdsasig.go Removes a stale TODO comment; no functional changes.
config/config.go Updates the GRPCKey comment to accurately reflect supported combinations (was incorrectly claiming file/secp256k1; now correctly says file/{ed25519,secp256k1eth}).

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant CometBFT as CometBFT Privval
    participant Adapter as signerPrivKey (adapter)
    participant Signer as signing.Signer (file/pkcs11/awskms)
    participant CometPub as secp256k1eth.PubKey

    CometBFT->>Adapter: Sign(rawSignBytes)
    Note over Adapter: if Scheme == secp256k1eth
    Adapter->>Adapter: "digest = Keccak256(rawSignBytes)"
    Adapter->>Signer: Sign(ctx, digest[32 bytes])
    Signer-->>Adapter: sig[65 bytes r‖s‖v]
    Adapter-->>CometBFT: sig

    CometBFT->>CometPub: VerifySignature(rawSignBytes, sig)
    Note over CometPub: applies Keccak256 internally
    CometPub-->>CometBFT: true/false
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant CometBFT as CometBFT Privval
    participant Adapter as signerPrivKey (adapter)
    participant Signer as signing.Signer (file/pkcs11/awskms)
    participant CometPub as secp256k1eth.PubKey

    CometBFT->>Adapter: Sign(rawSignBytes)
    Note over Adapter: if Scheme == secp256k1eth
    Adapter->>Adapter: "digest = Keccak256(rawSignBytes)"
    Adapter->>Signer: Sign(ctx, digest[32 bytes])
    Signer-->>Adapter: sig[65 bytes r‖s‖v]
    Adapter-->>CometBFT: sig

    CometBFT->>CometPub: VerifySignature(rawSignBytes, sig)
    Note over CometPub: applies Keccak256 internally
    CometPub-->>CometBFT: true/false
Loading

Comments Outside Diff (1)

  1. config/validate.go, line 251-259 (link)

    P2 Missing algorithm validation for file backend GRPC keys

    validateGRPC checks key_file existence for BackendFile keys but never validates the Algorithm field against the supported set {ed25519, secp256k1eth}. A config with backend: file, algorithm: secp256k1 (or any other unsupported value) passes validation successfully, then fails at startup with a cryptic "unsupported backend/algorithm" error from newGRPCSigner. The other backends (BackendAWSKMS, BackendPKCS11) both enforce their supported-algorithm sets at validation time via supportedAWSKMSAlgorithms and supportedPKCS11Algorithms. A symmetric supportedGRPCFileAlgorithms map (or an inline check) should be added in the BackendFile case of validateGRPC.

Reviews (1): Last reviewed commit: "Merge branch 'eric/pkcs11-all' into eric..." | Re-trigger Greptile

@Eric-Warehime Eric-Warehime changed the title Wire secp256k1 eth Wire secp256k1 eth [2/4] Jul 15, 2026
Comment thread internal/app/build.go Outdated
Eric-Warehime and others added 2 commits July 16, 2026 16:35
Co-authored-by: dianab-cl <diana@cosmoslabs.io>
Base automatically changed from eric/pkcs11-all to main July 17, 2026 02:53
@Eric-Warehime
Eric-Warehime enabled auto-merge July 17, 2026 03:05
@Eric-Warehime
Eric-Warehime added this pull request to the merge queue Jul 17, 2026
Merged via the queue into main with commit bb58e2a Jul 17, 2026
8 checks passed
@Eric-Warehime
Eric-Warehime deleted the eric/wire-supported-algos branch July 17, 2026 03:16
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.

3 participants