-
Notifications
You must be signed in to change notification settings - Fork 2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix test for rejecting a TSS node who is not part of the signing committe #1028
Fix test for rejecting a TSS node who is not part of the signing committe #1028
Conversation
@@ -186,8 +185,7 @@ async fn handle_initial_incoming_ws_message( | |||
// Make the signing process fail, since one of the commitee has misbehaved | |||
listeners.remove(&msg.session_id); | |||
return Err(SubscribeErr::Decryption( | |||
"Public key does not match that given in UserTransactionRequest or register \ | |||
transaction" | |||
"Public key does not match any of those expected for this protocol session" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I changed this error message and the comment above because they mentioned registration which no longer involves a protocol session.
for res in test_user_bad_connection_res { | ||
assert_eq!( | ||
res.unwrap().text().await.unwrap(), | ||
"{\"Err\":\"Timed out waiting for remote party\"}" | ||
"{\"Err\":\"Oneshot timeout error: channel closed\"}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think what error we get here is not too important - the main thing is that the connection attempt fails in the assertion below.
@@ -780,20 +779,16 @@ async fn test_fails_to_sign_if_non_signing_group_participants_are_used() { | |||
signature_request.signature_verifying_key = verifying_key.to_vec(); | |||
|
|||
let test_user_bad_connection_res = submit_transaction_requests( | |||
vec![validator_ips_and_keys[1].clone()], | |||
vec![validator_ips_and_keys[0].clone()], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was the bit that was wrong before. We should be sending the signature request to the same TS server that we attempt to make a connection to.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🙏
a47156c
into
hc/update-registration-tests-on-tss
* Remove signing integration test The `register_and_sign` integration test covers the same functionality now. * Update ETH integration test to use new registration flow * Get program config test working again * Delete `test_store_share` test This isn't relevant in the context of registration anymore. The functionality of this test was mostly moved to `test_jumpstart_network` anyways. * Fix `test_fail_infinite_program` * Fix `test_device_key_proxy` test * Get `test_sign_tx_no_chain` mostly passing * Move request limit tests to their own test * Rename failing signing test * Move a failing connection test out of `test_sign_tx_no_chain` * Ignore failing test Not sure if we still want this, or tbh how to fix it * Update test name to better reflect what it covers * Rename `generic_msg` to `signature_request` This better reflects what the variable actually is * Fix test for rejecting a TSS node who is not part of the signing committe (#1028) * Update error message for unexpected participant * Fix test * Fix used import and variable warnings * Use `test_client` for registration in all non-registration tests * Rustfmt * Move `update_programs` call to relevant location --------- Co-authored-by: peg <[email protected]>
This is based off #1026 and fixes the test which checks that a TSS server who is not a member of the expected signing committee is unable to participate in the signing protocol.
This test was actually pretty broken before - and i am to blame. It was sending the signature request to one validator, and then attempting a bogus connection to another validator. Which correctly failed because they were not expecting a signing session at all. But what we want to test here is that when they are expecting a signing session, they will only allow members of the signing committee to connect.