Skip to content
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

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -170,8 +170,7 @@ async fn handle_initial_incoming_ws_message(
};

{
// Check that the given public key matches the public key we got in the
// UserTransactionRequest or register transaction
// Check that the given public key is of the ones we are expecting for this protocol session
let mut listeners = app_state
.listener_state
.listeners
Expand All @@ -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"
Copy link
Contributor Author

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.

.to_string(),
));
}
Expand Down
13 changes: 4 additions & 9 deletions crates/threshold-signature-server/src/user/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -672,7 +672,6 @@ async fn test_request_limit_are_updated_during_signing() {
clean_tests();
}

#[ignore]
#[tokio::test]
#[serial]
async fn test_fails_to_sign_if_non_signing_group_participants_are_used() {
Expand Down Expand Up @@ -728,7 +727,7 @@ async fn test_fails_to_sign_if_non_signing_group_participants_are_used() {
};

let with_parent_key = true;
let (validators_info, mut signature_request, validator_ips_and_keys) =
let (_validators_info, mut signature_request, validator_ips_and_keys) =
get_sign_tx_data(&entropy_api, &rpc, hex::encode(PREIMAGE_SHOULD_SUCCEED), with_parent_key)
.await;

Expand Down Expand Up @@ -769,7 +768,7 @@ async fn test_fails_to_sign_if_non_signing_group_participants_are_used() {
let subscribe_response: Result<(), String> =
bincode::deserialize(&response_message).unwrap();

assert_eq!(Err("NoListener(\"no listener\")".to_string()), subscribe_response);
assert_eq!(Err("Decryption(\"Public key does not match any of those expected for this protocol session\")".to_string()), subscribe_response);

// The stream should not continue to send messages
// returns true if this part of the test passes
Expand All @@ -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()],
Copy link
Contributor Author

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.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🙏

signature_request.clone(),
one,
)
.await;

// thread 'user::tests::test_fails_to_sign_if_non_signing_group_participants_are_used' panicked at crates/threshold-signature-server/src/user/tests.rs:848:9:
// assertion `left == right` failed
// left: "{\"Err\":\"Subscribe message rejected: NoListener(\\\"no listener\\\")\"}"
// right: "{\"Err\":\"Timed out waiting for remote party\"}"
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\"}"
Copy link
Contributor Author

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.

);
}

Expand Down
Loading