feat(auth): add ECDSA single sig and multisig authentication component by onurinanc · Pull Request #2083 · 0xMiden/protocol

onurinanc · 2025-11-10T15:32:46Z

This PR implements the ECDSA Authentication Component for both single and multisig accounts.

Next Steps:

Implement the ECDSA Authentication Component in the rust-client withinmiden-client.
Implement the ECDSA Authentication Component in the web-client within miden-client.

Future Improvements:
This PR currently shares similar code patterns with the RpoFalcon512 and RpoFalcon512Multisig authentication components. A more pluggable design that unifies these authentication mechanisms into a single, configurable component will be explored.

bobbinth · 2025-11-10T15:52:58Z

Thank you, @onurinanc! Once question (w/o having looked at the code): should this go into main rather than next? I think we wanted to include this into v0.12 release - and if so, it should go into main - but not sure if there are any significant breaking changes here.

onurinanc · 2025-11-10T16:02:43Z

@bobbinth I think there are no any breaking changes in this PR. I’ll update the base branch to main.

mmagician

Just a superficial review on the MASM structure for now - I think we can reduce the diff significantly

mmagician · 2025-11-10T18:45:37Z

crates/miden-lib/asm/account_components/multisig_ecdsa_k256_keccak.masm

+proc assert_new_tx(msg: BeWord)
+    push.IS_EXECUTED_FLAG
+    # => [[0, 0, 0, is_executed], MSG]
+
+    swapw
+    # => [MSG, IS_EXECUTED_FLAG]
+
+    push.EXECUTED_TXS_SLOT
+    # => [index, MSG, IS_EXECUTED_FLAG]
+
+    # Set the key value pair in the map to mark transaction as executed
+    exec.native_account::set_map_item
+    # => [OLD_MAP_ROOT, [0, 0, 0, is_executed]]
+
+    dropw drop drop drop
+    # => [is_executed]
+
+    assertz.err=ERR_TX_ALREADY_EXECUTED
+    # => []
+end


AFAICS, this is the same logic as in the falcon file?

In that case, it would be best to reduce code duplication and move this file into a common file (see e.g. https://github.com/0xMiden/miden-base/blob/b160cabba65b6af4bbc6473e12bf0692dff7c855/crates/miden-lib/asm/miden/auth/mod.masm)

Yes, this and other similar procedures can probably go into miden::auth::multisig module. But also, in the interest of time, I'm also fine keeping this as is for this PR and addressing de-duplication in a follow-up.

mmagician · 2025-11-10T18:45:47Z

crates/miden-lib/asm/account_components/multisig_ecdsa_k256_keccak.masm

+#! Where:
+#! - init_num_of_approvers is the original number of approvers before the update
+#! - new_num_of_approvers is the new number of approvers after the update
+proc cleanup_pubkey_mapping(init_num_of_approvers: u32, new_num_of_approvers: u32)


mmagician · 2025-11-10T18:46:04Z

crates/miden-lib/asm/account_components/multisig_ecdsa_k256_keccak.masm

+#! Locals:
+#! 0: new_num_of_approvers
+#! 1: init_num_of_approvers
+pub proc update_signers_and_threshold.2(multisig_config_hash: BeWord)


bobbinth

Looks good! Thank you! I reviewed all non-test code and left some comments inline - most of them are small nits and thoughts for the future.

crates/miden-tx/src/auth/tx_authenticator.rs

crates/miden-lib/src/auth.rs

crates/miden-lib/src/account/interface/component.rs

crates/miden-lib/src/account/faucets/basic_fungible.rs

crates/miden-lib/asm/account_components/ecdsa_k256_keccak.masm

crates/miden-lib/src/account/auth/ecdsa_k256_keccak_multisig.rs

bobbinth · 2025-11-11T08:02:17Z

crates/miden-lib/asm/miden/auth/ecdsa_k256_keccak.masm

+    exec.auth::create_tx_summary
+    # => [SALT, OUTPUT_NOTES_COMMITMENT, INPUT_NOTES_COMMITMENT, ACCOUNT_DELTA_COMMITMENT, PUB_KEY]
+
+    exec.auth::adv_insert_hqword


Not for this PR, but I believe we can now replace this with adv.insert_hqword instruction.

@Fumuran could you create an issue for this?

bobbinth · 2025-11-11T08:16:04Z

crates/miden-lib/asm/miden/auth/ecdsa_k256_keccak.masm

+const.PUBLIC_KEY_SLOT=0
+
+# Local Memory Addresses for multisig operations
+const.NUM_OF_APPROVERS_LOC=0


Not from this PR, but I'm not seeing NUM_OF_APPROVERS_LOC. Maybe worth creating an issue to remove it. cc @partylikeits1983.

bobbinth · 2025-11-11T08:20:43Z

crates/miden-lib/asm/account_components/multisig_ecdsa_k256_keccak.masm

+proc assert_new_tx(msg: BeWord)
+    push.IS_EXECUTED_FLAG
+    # => [[0, 0, 0, is_executed], MSG]
+
+    swapw
+    # => [MSG, IS_EXECUTED_FLAG]
+
+    push.EXECUTED_TXS_SLOT
+    # => [index, MSG, IS_EXECUTED_FLAG]
+
+    # Set the key value pair in the map to mark transaction as executed
+    exec.native_account::set_map_item
+    # => [OLD_MAP_ROOT, [0, 0, 0, is_executed]]
+
+    dropw drop drop drop
+    # => [is_executed]
+
+    assertz.err=ERR_TX_ALREADY_EXECUTED
+    # => []
+end


Yes, this and other similar procedures can probably go into miden::auth::multisig module. But also, in the interest of time, I'm also fine keeping this as is for this PR and addressing de-duplication in a follow-up.

bobbinth · 2025-11-11T08:34:05Z

crates/miden-lib/asm/account_components/ecdsa_k256_keccak.masm

+pub proc auth_tx_ecdsa_k256_keccak(auth_args: BeWord)
+    dropw
+    # => [pad(16)]
+
+    # Fetch public key from storage.
+    # ---------------------------------------------------------------------------------------------
+    push.PUBLIC_KEY_SLOT exec.active_account::get_item
+    # => [PUB_KEY, pad(16)]
+
+    exec.ecdsa_k256_keccak::authenticate_transaction
+    # => [pad(16)]
+end


Not for this PR, but we should start thinking about how to reduce code duplication. Ideally, instead of have two (or more) identical components that differ only in signature scheme used, we would have something like AuthBasicSinglesig and AuthBasicMultisig and the signature scheme would be defined internally.

There are at least two ways to do this:

We could store the root of the signature procedure in the storage. In the case of single-sig authentication, the storage layout would be [PUB_KEY, VERIFY_PROC_ROOT]. Then, we'd just use dynexec instruction to execute the stored procedure.

We could still keep just the public key in the storage and add an event to retrieve the type of the public key. Then, we could have a set of if-else statements to figure out which verification procedure to run for a given public key.

Both of these have their prons and cons:

The first approach is more extensible: we don't need to modify the code to add support for a new signature scheme. But, we can't easily "mix" different signature schemes (mostly relevant in the context of the multisig).

The second approach avoids the use of dynexec (which feels a bit risky) and it allows us to easily mix different signature schemes (e.g., in the same multisig we could have both ECDSA and Falcon public keys) - but it is not as extensible: adding a new signature scheme would require MASM code changes. It is also a bit more work as we'll need to get the public key type from the authenticator (though, we may need to do this for other reasons anyways).

There may be other approaches to reducing code duplication as well - e.g., something along the lines of #1956. But I haven't thought about this too much.

We should create an issue to discuss this and ideally come up with the improved approach for v0.13.0 release.

bobbinth

Looks good! Thank you! As mentioned in the pervious comments, I'll merge this as is, and we can work on code de-duplication in follow-up PRs and try to wrap up with work for v0.13 release.

onurinanc changed the base branch from next to main November 10, 2025 16:11

onurinanc added 4 commits November 10, 2025 17:26

feat(auth): add ECDSA single sig and multisig authentication component

eedccb0

fmt

764ab3e

changelog

6649bd7

fmt

8191ed1

onurinanc force-pushed the feat-ecdsa-auth branch from 0cabf38 to 8191ed1 Compare November 10, 2025 16:32

onurinanc added 2 commits November 10, 2025 17:35

fmt

5f293ef

fix changelog

20f4e91

bobbinth requested review from PhilippGackstatter, bobbinth and mmagician November 10, 2025 18:11

mmagician requested changes Nov 10, 2025

View reviewed changes

bobbinth reviewed Nov 11, 2025

View reviewed changes

onurinanc added 5 commits November 11, 2025 10:51

fix comments

052de03

feat(auth): add ECDSA ACL Authentication Component

a9db141

more efficient duplicate approver check

3a20138

fmt

b7024d3

lint

6cf5c35

onurinanc requested review from bobbinth and mmagician November 11, 2025 18:44

bobbinth approved these changes Nov 12, 2025

View reviewed changes

bobbinth merged commit eca91ae into 0xMiden:main Nov 12, 2025
15 checks passed

bobbinth mentioned this pull request Nov 13, 2025

Improve BasicAuthenticator & TransactionAuthenticator for ECDSA Auth Component #2064

Closed

onurinanc mentioned this pull request Nov 25, 2025

Authentication and Accounts OpenZeppelin/miden-confidential-contracts#2

Open

bobbinth mentioned this pull request Dec 6, 2025

Consolidate authentication components #2143

Open

onurinanc mentioned this pull request Feb 10, 2026

Add ECDSA single sig and multisig authentication component OpenZeppelin/miden-confidential-contracts#55

Closed

Conversation

onurinanc commented Nov 10, 2025

Uh oh!

bobbinth commented Nov 10, 2025

Uh oh!

onurinanc commented Nov 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mmagician left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bobbinth left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bobbinth left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

onurinanc commented Nov 10, 2025 •

edited

Loading