Skip to content

Commit

Permalink
Merge branch 'maksym/ic-1687-enable-test' into 'master'
Browse files Browse the repository at this point in the history
test: [EXC-1658] support signing disabled iDKG keys in state_machine_tests

This MR adds support for signing disabled iDKG keys to `state_machine_tests` and unifies corresponding error messages in message routing and execution.

Changes:
- add `with_signing_disabled_idkg_key(key)` to `StateMachineBuilder`
- consolidated `route_idkg_message()` error messages with similar cases in executing management canister requests
- clarified in tests the difference in public key request and signing for signing-disabled and unknown keys 

See merge request dfinity-lab/public/ic!20364
  • Loading branch information
maksymar committed Jul 11, 2024
2 parents 47c7302 + c12b4b2 commit b5b9e24
Show file tree
Hide file tree
Showing 9 changed files with 217 additions and 180 deletions.
2 changes: 1 addition & 1 deletion packages/pocket-ic/tests/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1167,5 +1167,5 @@ fn test_ecdsa_disabled() {
.unwrap()
.0
.unwrap_err();
assert!(ecdsa_signature_err.contains("Requested unknown iDKG key: ecdsa:Secp256k1:dfx_test_key1, existing keys with signing enabled: []"));
assert!(ecdsa_signature_err.contains("Requested unknown or signing disabled iDKG key: ecdsa:Secp256k1:dfx_test_key1, existing keys with signing enabled: []"));
}
6 changes: 3 additions & 3 deletions rs/consensus/src/ecdsa/payload_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -843,7 +843,7 @@ mod tests {
create_available_pre_signature(&mut idkg_payload, valid_key_id.clone(), 10);
let pre_sig_for_disabled_key =
create_available_pre_signature(&mut idkg_payload, disabled_key_id.clone(), 11);
let non_existant_pre_sig_for_valid_key = idkg_payload.uid_generator.next_pre_signature_id();
let non_existent_pre_sig_for_valid_key = idkg_payload.uid_generator.next_pre_signature_id();

let contexts = set_up_signature_request_contexts(vec![
// Two request contexts without pre-signature
Expand All @@ -863,12 +863,12 @@ mod tests {
UNIX_EPOCH,
Some(pre_sig_for_disabled_key),
),
// One valid context matched to non-existant pre-signature
// One valid context matched to non-existent pre-signature
(
valid_key_id.clone(),
4,
UNIX_EPOCH,
Some(non_existant_pre_sig_for_valid_key),
Some(non_existent_pre_sig_for_valid_key),
),
]);

Expand Down
4 changes: 2 additions & 2 deletions rs/execution_environment/src/execution_environment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2664,7 +2664,7 @@ impl ExecutionEnvironment {
return Err(UserError::new(
ErrorCode::CanisterRejectedMessage,
format!(
"{} request failed: invalid or disabled key {}.",
"{} request failed: unknown or signing disabled iDKG key {}.",
request.method_name, threshold_key
),
));
Expand Down Expand Up @@ -2759,7 +2759,7 @@ impl ExecutionEnvironment {
return Err(UserError::new(
ErrorCode::CanisterRejectedMessage,
format!(
"{} request failed: invalid or disabled key {}.",
"{} request failed: unknown or signing disabled iDKG key {}.",
request.method_name, threshold_key
),
));
Expand Down
49 changes: 30 additions & 19 deletions rs/execution_environment/src/execution_environment/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2416,7 +2416,7 @@ fn test_sign_with_threshold_key_unknown_key_rejected() {
assert_eq!(
WasmResult::Reject(
format!(
"Unable to route management canister request {}: IDkgKeyError(\"Requested unknown iDKG key: {}, existing keys with signing enabled: [{}]\")",
"Unable to route management canister request {}: IDkgKeyError(\"Requested unknown or signing disabled iDKG key: {}, existing keys with signing enabled: [{}]\")",
method,
wrong_key,
correct_key,
Expand All @@ -2427,52 +2427,61 @@ fn test_sign_with_threshold_key_unknown_key_rejected() {
}

#[test]
fn test_threshold_key_signature_request_with_disabled_key_rejected() {
fn test_signing_disabled_vs_unknown_key_on_public_key_and_signing_requests() {
// Test the disabled key succeeds for public key request but fails for signing,
// and the unknown key fails for both.
let test_cases = vec![
(
Method::ECDSAPublicKey,
Method::SignWithECDSA,
make_ecdsa_key("disabled_key"),
make_ecdsa_key("wrong_key"),
make_ecdsa_key("signing_disabled_key"),
make_ecdsa_key("unknown_key"),
),
(
Method::SchnorrPublicKey,
Method::SignWithSchnorr,
make_schnorr_key("disabled_key"),
make_schnorr_key("wrong_key"),
make_schnorr_key("signing_disabled_key"),
make_schnorr_key("unknown_key"),
),
];
for (public_key_method, sign_with_method, disabled_key, wrong_key) in test_cases {
for (public_key_method, sign_with_method, signing_disabled_key, unknown_key) in test_cases {
let canister_id = canister_test_id(0x10);
let own_subnet_id = subnet_test_id(1);
let mut test = ExecutionTestBuilder::new()
.with_subnet_type(SubnetType::System)
.with_own_subnet_id(own_subnet_id)
.with_nns_subnet_id(subnet_test_id(2))
.with_disabled_idkg_key(disabled_key.clone())
.with_signing_disabled_idkg_key(signing_disabled_key.clone())
.with_caller(own_subnet_id, canister_id)
.with_ic00_schnorr_public_key(FlagStatus::Enabled)
.with_ic00_sign_with_schnorr(FlagStatus::Enabled)
.build();

// Requesting disabled public key (should succeed)
// Requesting disabled public key (should succeed).
test.inject_call_to_ic00(
public_key_method,
threshold_public_key_payload(public_key_method, disabled_key.clone()),
threshold_public_key_payload(public_key_method, signing_disabled_key.clone()),
Cycles::from(100_000_000_000u128),
);

// Signing with disabled key (should fail)
// Signing with disabled key (should fail).
test.inject_call_to_ic00(
sign_with_method,
sign_with_threshold_key_payload(sign_with_method, disabled_key.clone()),
sign_with_threshold_key_payload(sign_with_method, signing_disabled_key.clone()),
Cycles::from(100_000_000_000u128),
);

// Signing with non-existant key (should fail)
// Requesting non-existent public key (should fail).
test.inject_call_to_ic00(
public_key_method,
threshold_public_key_payload(public_key_method, unknown_key.clone()),
Cycles::from(100_000_000_000u128),
);

// Signing with non-existent key (should fail).
test.inject_call_to_ic00(
sign_with_method,
sign_with_threshold_key_payload(sign_with_method, wrong_key.clone()),
sign_with_threshold_key_payload(sign_with_method, unknown_key.clone()),
Cycles::from(100_000_000_000u128),
);
test.execute_all();
Expand All @@ -2481,18 +2490,20 @@ fn test_threshold_key_signature_request_with_disabled_key_rejected() {
// Note this fails with internal error as the test environment doesn't hold a valid key.
// However, this is enough to assert that the correct endpoint is reached.
"InternalError(\"InvalidPoint\")".to_string(),
format!("invalid or disabled key {}", disabled_key),
format!("does not hold iDKG key {}", wrong_key),
format!(
"unknown or signing disabled iDKG key {}",
signing_disabled_key
),
format!("does not hold iDKG key {}", unknown_key),
format!("does not hold iDKG key {}", unknown_key),
];

for (i, expected) in expected.iter().enumerate() {
let result = test.xnet_messages()[i].clone();
let message = get_reject_message(result);
assert!(
message.contains(expected),
"Expected: {} \nActual: {}",
expected,
message
"Expected: {expected}\nActual: {message}",
);
}
}
Expand Down
Loading

0 comments on commit b5b9e24

Please sign in to comment.