User can participate in signing #379

ameba23 · 2023-07-28T07:10:35Z

If key visibility is private, we should assume the user is also going to connect to the threshold servers and participate in the signing process.

Because of this, i have changed the key visibility used in our existing tests to use 'permissioned', so the vanilla case we test for does not require participation from the user.

As explained in the related issue #362 making this practical to use from the SDK will also require having wasm bindings for Synedrion's sessions API.

…ipate in signing

…e as a single argument

vercel · 2023-07-28T07:10:38Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
entropy-core	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Sep 2, 2023 0:02am

ameba23 · 2023-07-28T11:53:07Z

Having starting trying to write a test for this, my feeling is that it might make sense that rather than having JS bindings for synedrion's signing session API, we could have wasm-compilable crate with a function which handles the connecting, handshaking and executing signing protocol, which is called by the SDK and passed a keyshare and an array of validator info.

Although this could all be done from JS and there are good noise implementations in JS, i feel like we would be repeating a lot of work we already have here, and would make things harder to maintain.

… - as the user themselves is not in the list

ameba23 · 2023-08-04T21:41:12Z

Test currently failing with:
thread 'user::tests::test_sign_tx_user_participates' panicked at 'called Option::unwrap()on aNone value', /home/turnip/.cargo/git/checkouts/synedrion-d2f4837b6efbe779/bfc23e6/synedrion/src/protocols/signing.rs:92:10

coming from this line in Synedrion.

and i am not sure why - relevant part of stack trace:

4: <synedrion::protocols::signing::Round1 as synedrion::protocols::generic::Round>::finalize
             at /home/turnip/.cargo/git/checkouts/synedrion-d2f4837b6efbe779/bfc23e6/synedrion/src/protocols/signing.rs:86:19
   5: <synedrion::protocols::interactive_signing::SigningRound as synedrion::protocols::generic::Round>::finalize
             at /home/turnip/.cargo/git/checkouts/synedrion-d2f4837b6efbe779/bfc23e6/synedrion/src/protocols/interactive_signing.rs:314:9
   6: synedrion::sessions::type_erased::RoundAndAccum<R>::finalize_typed
             at /home/turnip/.cargo/git/checkouts/synedrion-d2f4837b6efbe779/bfc23e6/synedrion/src/sessions/type_erased.rs:66:15
   7: <synedrion::sessions::type_erased::RoundAndAccum<R> as synedrion::sessions::type_erased::TypeErasedReceivingRound<<R as synedrion::protocols::generic::Round>::Result>>::finalize
             at /home/turnip/.cargo/git/checkouts/synedrion-d2f4837b6efbe779/bfc23e6/synedrion/src/sessions/type_erased.rs:206:15
   8: synedrion::sessions::states::ReceivingState<Res,Sig,Signer,Verifier>::finalize_regular_round
             at /home/turnip/.cargo/git/checkouts/synedrion-d2f4837b6efbe779/bfc23e6/synedrion/src/sessions/states.rs:348:15
   9: synedrion::sessions::states::ReceivingState<Res,Sig,Signer,Verifier>::finalize
             at /home/turnip/.cargo/git/checkouts/synedrion-d2f4837b6efbe779/bfc23e6/synedrion/src/sessions/states.rs:389:45
  10: server::signing_client::new_party::signing_protocol::execute_protocol::{{closure}}::{{closure}}
             at ./src/signing_client/new_party/signing_protocol.rs:136:15

so this line in execute_protocol.

… execute_protocol

… case

ameba23 · 2023-08-08T07:49:09Z

For the record, the problem was caused by calling make_interactive_signing_session with only two verifier keys (missing the verifier key for the user), and that the validators began executing the protocol without waiting for the user to connect, effectively causing protocol messages destined for the user to be ignored.

This meant the signing protocol ran, but failed when checking the signature, as a required signing party was missing.

… when connecting to validators

…t, to make it more generic

ameba23 · 2023-08-17T12:45:23Z

crypto/server/src/helpers/signing.rs

@@ -80,13 +82,17 @@ impl Default for SignatureState {
 pub async fn do_signing(


The reason we are now passing the app_state struct here is because otherwise clippy complains that there are now too many arguments to this function.

that is fine, you could also put in an ignore for clippy in the future if you want

ameba23 · 2023-08-17T14:10:23Z

crypto/server/src/signing_client/protocol_transport/listener.rs

+
+        for validator in validators_info {
+            if &validator.tss_account != my_id {
+                validators.insert(validator.tss_account, validator.x25519_public_key);


Here we also store the x25519 public key, so that when validators connect we can check both Account ID and x25519 public key. This is needed for one of our negative tests where a bad x25519 public key is passed.

…hare for the user

JesseAbram · 2023-09-01T15:37:18Z

crypto/server/src/helpers/signing.rs

@@ -80,13 +82,17 @@ impl Default for SignatureState {
 pub async fn do_signing(


that is fine, you could also put in an ignore for clippy in the future if you want

ameba23 added 2 commits July 28, 2023 08:43

If key visibility is private, assume the user will connect and partic…

5a76f64

…ipate in signing

Clippy says that do_signing now takes too many args, so pass app_stat…

5aa8d94

…e as a single argument

ameba23 marked this pull request as draft July 28, 2023 07:10

vercel bot deployed to Preview July 28, 2023 07:33 View deployment

ameba23 added 2 commits August 2, 2023 11:36

Make fns needed by the tests for user to participate in signing public

ea24896

WIP add test for user participates in signing

65d8eb8

vercel bot deployed to Preview August 2, 2023 10:07 View deployment

ameba23 added 2 commits August 3, 2023 19:29

WIP Improve test fn for user participates in signing

bc6d5cf

Comments

7923030

vercel bot deployed to Preview August 3, 2023 18:02 View deployment

WIP call user participates in signing helper fn from test (failing)

0254719

vercel bot deployed to Preview August 3, 2023 18:35 View deployment

ameba23 added 2 commits August 4, 2023 23:26

Remove check that connecting party is in the user transaction request…

2e0e291

… - as the user themselves is not in the list

Fmt

df28d17

vercel bot deployed to Preview August 4, 2023 22:28 View deployment

ameba23 added 2 commits August 8, 2023 07:18

If key visibility is private, add user to list of verifiers passed to…

0813f32

… execute_protocol

Fix parsing of sig_hash - user participates test now passes for happy…

15cac4b

… case

vercel bot deployed to Preview August 8, 2023 06:20 View deployment

ameba23 added 6 commits August 17, 2023 13:17

Listener tracks x25519 public keys of validators so we can check them…

5e507c6

… when connecting to validators

Clippy

1c0c6a9

Listener takes Vec<ValidatorInfo> rather than a UserTransactionReques…

f3ebbdd

…t, to make it more generic

Rm unused field for Listener

e1be713

Improve test for user participating by checking signatures match

573de3a

Improve error handling in user_participates test helper fn

1373cc2

vercel bot deployed to Preview August 17, 2023 12:29 View deployment

Improve error handling in user_participates test helper fn

d74d848

ameba23 commented Aug 17, 2023

View reviewed changes

Rename variable for clarity

36ad0bf

vercel bot deployed to Preview August 17, 2023 13:41 View deployment

ameba23 commented Aug 17, 2023

View reviewed changes

ameba23 marked this pull request as ready for review August 17, 2023 14:11

ameba23 requested a review from JesseAbram August 17, 2023 14:11

Merge branch 'master' into user-participates-in-signing

6052f97

vercel bot deployed to Preview August 31, 2023 09:08 View deployment

ameba23 added 2 commits August 31, 2023 12:12

fmt

323c7e8

Fixes following merge with master

2eeed29

vercel bot deployed to Preview August 31, 2023 11:09 View deployment

spawn_testing_validators helper function should optionally create a s…

8e68174

…hare for the user

vercel bot deployed to Preview August 31, 2023 16:10 View deployment

JesseAbram approved these changes Sep 1, 2023

View reviewed changes

Resolve merge conflict with master

1f69097

vercel bot deployed to Preview September 2, 2023 09:58 View deployment

Rm unused test helper function

f334eaa

vercel bot deployed to Preview September 2, 2023 12:02 View deployment

ameba23 merged commit 13d7f3f into master Sep 2, 2023

ameba23 deleted the user-participates-in-signing branch September 2, 2023 12:28

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

User can participate in signing #379

User can participate in signing #379

ameba23 commented Jul 28, 2023

vercel bot commented Jul 28, 2023 •

edited

Loading

ameba23 commented Jul 28, 2023

ameba23 commented Aug 4, 2023 •

edited

Loading

ameba23 commented Aug 8, 2023

ameba23 Aug 17, 2023

JesseAbram Sep 1, 2023

ameba23 Aug 17, 2023

JesseAbram Sep 1, 2023

		@@ -80,13 +82,17 @@ impl Default for SignatureState {
		pub async fn do_signing(

User can participate in signing #379

User can participate in signing #379

Conversation

ameba23 commented Jul 28, 2023

vercel bot commented Jul 28, 2023 • edited Loading

ameba23 commented Jul 28, 2023

ameba23 commented Aug 4, 2023 • edited Loading

ameba23 commented Aug 8, 2023

ameba23 Aug 17, 2023

Choose a reason for hiding this comment

JesseAbram Sep 1, 2023

Choose a reason for hiding this comment

ameba23 Aug 17, 2023

Choose a reason for hiding this comment

JesseAbram Sep 1, 2023

Choose a reason for hiding this comment

vercel bot commented Jul 28, 2023 •

edited

Loading

ameba23 commented Aug 4, 2023 •

edited

Loading