feat(toolkit): ECDSA contract maintenance & deploy committees#1861
Open
jtsang586 wants to merge 6 commits into
Open
feat(toolkit): ECDSA contract maintenance & deploy committees#1861jtsang586 wants to merge 6 commits into
jtsang586 wants to merge 6 commits into
Conversation
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
Extends the toolkit's ECDSA unshielded-signature support to contract maintenance and deploy committees, completing acceptance criterion #3 of issue #1542 (previously committees were Schnorr-only). - Committee members are now full `UnshieldedWallet`s rather than bare Schnorr keys, so each member's signature scheme (Schnorr or ledger-9 ECDSA) travels with it. `MaintenanceUpdateInfo`, `ContractMaintenanceAuthorityInfo`, `ContractDeployInfo` and `Contract::deploy` take committee wallets and derive the maintenance verifying key / sign via the wallet's scheme-agnostic methods. - New `UnshieldedWallet::maintenance_verifying_key()` dispatches to the Schnorr/ECDSA `ContractMaintenanceVerifyingKey` variant, backed by a per-generation `MaintenanceVerifyingKey` type alias. - CLI: `--authority-seed` / `--new-authority-seed` accept the `schnorr:`/`ecdsa:` scheme prefix (bare = Schnorr, backwards compatible); mixed-scheme committees are supported. ECDSA committees are rejected on pre-ledger-9 chains via the shared `relevant_wallet_schemes` guard. The contract funding seed stays Schnorr. - The ledger 7/8 `VerifyingKeyEcdsa` stub gains `PartialEq, Eq` to mirror the real type so cross-generation comparisons type-check. Tests: unit coverage for scheme-dispatched address/maintenance-key derivation and tagged wallet (de)serialization (ledger/helpers); plus a ledger-acceptance test that the ledger's `SignatureKind::signature_verify` (the primitive `Transaction::well_formed` runs) accepts the toolkit's wrapped ECDSA signature and rejects cross-scheme mismatches (util/toolkit). Issue: #1542 Assisted-by: Claude:claude-opus-4-8 Signed-off-by: Joe Tsang <joe.tsang@shielded.io>
Adds an end-to-end test exercising the toolkit's ledger-9 ECDSA unshielded-signature support against a live `dev` node (whose genesis is built on ledger 9, so the ECDSA scheme is accepted on-chain). Because every submitted transaction runs the ledger's `Transaction::well_formed` (the real `signature_verify`), the test confirms end-to-end that: - ECDSA unshielded address derivation is wired via `show-address --seed ecdsa:<seed>` and is distinct from the Schnorr address for the same seed. - A contract deploys with an ECDSA maintenance committee and a maintenance update signed by that ECDSA committee is accepted (ECDSA-only signing). - A maintenance update signed by a mixed Schnorr+ECDSA committee is accepted (per-member scheme dispatch), and authority rotations persist across sequential updates. Wires it in as `scripts/tests/toolkit-ecdsa-e2e.sh`, a `just toolkit-ecdsa-e2e` recipe, and a `Toolkit ECDSA E2E` CI job mirroring the other toolkit e2e jobs. Issue: #1542 Assisted-by: Claude:claude-opus-4-8 Signed-off-by: Joe Tsang <joe.tsang@shielded.io>
…ed, reject ECDSA `generate-intent deploy --authority-seed` now accepts the same optional `schnorr:`/`ecdsa:` scheme prefix as the other seed flags (bare = Schnorr, backwards compatible), instead of a bare seed only. Unlike the native `generate-txs contract-simple deploy` path, the toolkit-js deploy path cannot build an ECDSA contract-maintenance authority: it hands the authority key to the `@midnight-ntwrk/compact-js` toolchain as a value-only `--signing` argument, and that toolchain exposes no channel to carry the ledger-9 signature scheme (`signingKind`) through to the deployed authority, so an ECDSA seed would silently deploy a Schnorr authority. An `ecdsa:` seed is therefore rejected up front (before any toolkit-js process is spawned) via the new `ToolkitJsError::EcdsaAuthorityUnsupported`, pointing at the native `generate-txs contract-simple deploy --authority-seed ecdsa:<seed>` path (which fully supports ECDSA and mixed-scheme committees). Schnorr behaviour is unchanged. This keeps the seed-flag surface consistent and turns what used to be a confusing hex parse error (`ecdsa:` is not valid hex) into a clear, actionable message. Tests: unit coverage that an `ecdsa:` authority seed is rejected before spawning toolkit-js and that a Schnorr seed passes the scheme guard. Issue: #1542 Assisted-by: Claude:claude-opus-4-8 Signed-off-by: Joe Tsang <joe.tsang@shielded.io>
Adds `ecdsa_address_mip0003_conformance`, pinning the ECDSA address formula
(`SHA-256("midnight:ecdsa:" || compressed-SEC1-vk)`, applied via
`UserAddress::from(ecdsa::VerifyingKey)`) to the official vectors from the
`midnight-wallet` spec-reference implementation. Each `uniform_bytes` input is
fed directly as the secp256k1 scalar (the HD-path leaf output, not a root
seed), matching what `UnshieldedWallet::from_bytes_ecdsa` consumes; the ledger-9
derivation reproduces the published addresses byte-for-byte.
The existing full-HD-path `ecdsa_address_golden_vector` stays self-generated:
the published vectors don't cover the root-seed to HD-leaf mapping, so there is
no official vector to pin the whole path against. Its comment now explains this.
Issue: #1542
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Joe Tsang <joe.tsang@shielded.io>
Removes `ecdsa_address_matches_verifying_key_derivation`. It re-derived the address through the same `from_bytes_ecdsa` path it was checking and asserted self-consistency, so it could not catch a derivation bug (LHS and RHS move together) and, for regressions, its coverage was a strict subset of the byte-anchored `ecdsa_address_golden_vector`. The remaining pair is stronger and non-overlapping: `ecdsa_address_golden_vector` anchors the full HD path to exact bytes (catches any regression) and `ecdsa_address_mip0003_conformance` pins the address formula to the official external vectors (catches an originally-wrong derivation). Prunes the now-unused imports. Issue: #1542 Assisted-by: Claude:claude-opus-4-8 Signed-off-by: Joe Tsang <joe.tsang@shielded.io>
jtsang586
force-pushed
the
joe-ecdsa-contract-committees
branch
from
8bdc166 to
d311690
Compare
![jina-simulation[bot]](https://avatars.githubusercontent.com/u/97225700?s=60&v=4){kind=small}
There was a problem hiding this comment.
Runtime Review - Merge Readiness 5/5
0 issues found - Ready to merge
Ready to merge based on the absence of accepted runtime issues.
Final review: No publishable runtime issues were identified; all investigation areas returned no candidates.
Issues
No medium/high-confidence runtime issues were found.
Summary
- Status: warned
- Areas investigated: 5
- Tasks/probes performed: 0
- Issues reported: 0
- Publishable issues: 0
- Inline comments: 0
- File-level comments: 0
- Review comments: 0
- Unanchored publishable findings posted as file-level comments: 0
- Blocked validations: 0
- Dashboard: full runtime review
- Commit: d311690
- Changed files: 20
- Tool calls: none
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Overview
Completes the toolkit side of ledger-9 ECDSA unshielded-signature support (issue #1542),
building on the ECDSA core landed in #1837. Scope of this branch:
UnshieldedWallets, so each member's scheme (Schnorr or ledger-9 ECDSA) travels with it;--authority-seed/--new-authority-seedon the nativegenerate-txs contract-simple deploy/maintenanceaccept theschnorr:/ecdsa:prefix, including mixed-scheme committees.generate-intent deploy --authority-seednow accepts the schemeprefix for CLI consistency but rejects
ecdsa:up front with an actionable error: theunderlying
@midnight-ntwrk/compact-jstoolchain passes only a value-only--signingkey and hasno channel for the ledger-9 signature scheme, so an ECDSA seed would silently deploy a Schnorr
authority. The error points at the native
contract-simple deploypath (which fully supports it).scripts/tests/toolkit-ecdsa-e2e.sh+just toolkit-ecdsa-e2e+ a CIjob, exercising ECDSA address derivation, ECDSA-committee deploy, and ECDSA-only + mixed-scheme
maintenance signing against a live
devnode (ledger-9 genesis).midnight-walletspec-reference vectors; our ledger-9 derivation reproduces them byte-for-byte.🗹 TODO before merging
218051d01(pre-dating this session's work) has noSigned-off-byand will trip the DCO check; needs a sign-off before merge. The other commits are signed off.
📌 Submission Checklist
git commit -s) for the DCO🧪 Testing Evidence
Aside from CI:
cargo test -p midnight-node-ledger-helpers --features can-panic 'common::wallet::unshielded'—15 pass (5 tests × ledger 7/8/9), including the MIP-0003 conformance vector and the
serialization round-trip.
cargo test -p midnight-node-toolkit --test ecdsa_signature— ledger accepts a wrapped ECDSAunshielded signature and rejects cross-scheme mismatches.
cargo test -p midnight-node-toolkit --lib toolkit_js— the toolkit-js deploy path rejects anecdsa:authority seed before spawning toolkit-js and lets a Schnorr seed through the guard.Verified
show-address --seed ecdsa:<seed>yields a distinctmn_addraddress locally.The full ECDSA deploy/maintenance round-trip runs against a live node via the new
toolkit-ecdsa-e2eCI job.Additional tests are provided
🔱 Fork Strategy
Links
Closes #1542