diff --git a/rs/https_outcalls/consensus/src/payload_builder.rs b/rs/https_outcalls/consensus/src/payload_builder.rs index 805b71b6725c..b7594a9a7a02 100644 --- a/rs/https_outcalls/consensus/src/payload_builder.rs +++ b/rs/https_outcalls/consensus/src/payload_builder.rs @@ -5,9 +5,10 @@ use crate::{ payload_builder::{ parse::bytes_to_payload, utils::{ - estimate_response_with_consensus_size, find_flexible_responses, + FlexibleFindResult, estimate_response_with_consensus_size, find_flexible_result, find_fully_replicated_response, find_non_replicated_response, group_shares_by_callback_id, grouped_shares_meet_divergence_criteria, + validate_flexible_response_with_proof, validate_response_share, }, }, }; @@ -38,7 +39,8 @@ use ic_replicated_state::ReplicatedState; use ic_types::{ CountBytes, Height, NodeId, NumBytes, RegistryVersion, SubnetId, batch::{ - CanisterHttpPayload, ConsensusResponse, FlexibleCanisterHttpResponses, + CanisterHttpPayload, ConsensusResponse, FlexibleCanisterHttpError, + FlexibleCanisterHttpResponseWithProof, FlexibleCanisterHttpResponses, MAX_CANISTER_HTTP_PAYLOAD_SIZE, ValidationContext, }, canister_http::{ @@ -47,7 +49,7 @@ use ic_types::{ CanisterHttpResponseMetadata, CanisterHttpResponseWithConsensus, Replication, }, consensus::Committee, - crypto::{Signed, crypto_hash}, + crypto::Signed, messages::{CallbackId, Payload, RejectContext}, registry::RegistryClientError, signature::BasicSignature, @@ -217,6 +219,7 @@ impl CanisterHttpPayloadBuilderImpl { let mut timeouts = vec![]; let mut divergence_responses = vec![]; let mut flexible_responses = vec![]; + let mut flexible_errors = vec![]; // Metrics counters let mut total_share_count = 0; @@ -257,14 +260,26 @@ impl CanisterHttpPayloadBuilderImpl { continue; } if request.time + CANISTER_HTTP_TIMEOUT_INTERVAL < validation_context.time { - let candidate_size = callback_id.count_bytes(); - let size = NumBytes::new((accumulated_size + candidate_size) as u64); - if size < max_payload_size { - timeouts.push(*callback_id); - accumulated_size += candidate_size; - // Because timeouts are very cheap to verify, they are - // not counted as responses (so that they are irrelevant - // for the CANISTER_HTTP_MAX_RESPONSES_PER_BLOCK limit. + // Because timeouts are very cheap to verify, they are + // not counted as responses (so that they are irrelevant + // for the CANISTER_HTTP_MAX_RESPONSES_PER_BLOCK limit. + if matches!(request.replication, Replication::Flexible { .. }) { + let error = FlexibleCanisterHttpError::Timeout { + callback_id: *callback_id, + }; + let candidate_size = error.count_bytes(); + let size = NumBytes::new((accumulated_size + candidate_size) as u64); + if size < max_payload_size { + flexible_errors.push(error); + accumulated_size += candidate_size; + } + } else { + let candidate_size = callback_id.count_bytes(); + let size = NumBytes::new((accumulated_size + candidate_size) as u64); + if size < max_payload_size { + timeouts.push(*callback_id); + accumulated_size += candidate_size; + } } continue; } @@ -329,22 +344,33 @@ impl CanisterHttpPayloadBuilderImpl { committee, min_responses, max_responses, - } => { - if let Some((group, group_size)) = find_flexible_responses( - *callback_id, - grouped_shares, - committee, - *min_responses, - *max_responses, - accumulated_size, - max_payload_size, - &*pool_access, - ) { + } => match find_flexible_result( + *callback_id, + grouped_shares, + committee, + *min_responses, + *max_responses, + accumulated_size, + max_payload_size, + &*pool_access, + ) { + FlexibleFindResult::OkResponses(group, group_size) => { + // Note: budget tracking w.r.t. `max_payload_size` + // is done directly in `find_flexible_result`. flexible_responses.push(group); responses_included += 1; accumulated_size += group_size; } - } + FlexibleFindResult::Error(error, error_size) => { + let size = NumBytes::new((accumulated_size + error_size) as u64); + if size < max_payload_size { + flexible_errors.push(error); + responses_included += 1; + accumulated_size += error_size; + } + } + FlexibleFindResult::Pending => {} + }, } } } @@ -359,7 +385,7 @@ impl CanisterHttpPayloadBuilderImpl { timeouts, divergence_responses, flexible_responses, - flexible_errors: vec![], + flexible_errors, } } @@ -642,28 +668,19 @@ impl CanisterHttpPayloadBuilderImpl { let mut seen_signers = HashSet::new(); - for entry in &group.responses { - // Callback id consistency - if entry.response.id != callback_id { - return invalid_artifact( - InvalidCanisterHttpPayloadReason::FlexibleCallbackIdMismatch { - callback_id, - mismatched_id: entry.response.id, - }, - ); - } - if entry.proof.content.id != callback_id { - return invalid_artifact( - InvalidCanisterHttpPayloadReason::FlexibleCallbackIdMismatch { - callback_id, - mismatched_id: entry.proof.content.id, - }, - ); - } + for response_with_proof in &group.responses { + validate_flexible_response_with_proof( + response_with_proof, + callback_id, + flex_committee, + &mut seen_signers, + consensus_registry_version, + &*self.crypto, + ) + .map_err(CanisterHttpPayloadValidationError::InvalidArtifact)?; - // Rejects are not allowed in flexible ok-responses if matches!( - entry.response.content, + response_with_proof.response.content, CanisterHttpResponseContent::Reject(_) ) { return invalid_artifact( @@ -672,68 +689,133 @@ impl CanisterHttpPayloadBuilderImpl { }, ); } + } + } - // No duplicate signers - let signer = entry.proof.signature.signer; - if !seen_signers.insert(signer) { - return invalid_artifact( - InvalidCanisterHttpPayloadReason::FlexibleDuplicateSigner { - callback_id, - signer, - }, - ); - } + // Validate flexible errors + for error in &payload.flexible_errors { + let callback_id = error.callback_id(); - // Signer must be in the flexible committee - if !flex_committee.contains(&signer) { - return invalid_artifact( - InvalidCanisterHttpPayloadReason::FlexibleSignerNotInCommittee { + if !delivered_ids.insert(callback_id) { + return invalid_artifact(InvalidCanisterHttpPayloadReason::DuplicateResponse( + callback_id, + )); + } + + // Look up the request context and verify it's a Flexible replication + let context = http_contexts.get(&callback_id).ok_or( + CanisterHttpPayloadValidationError::InvalidArtifact( + InvalidCanisterHttpPayloadReason::UnknownCallbackId(callback_id), + ), + )?; + let Replication::Flexible { + committee: flex_committee, + min_responses, + .. + } = &context.replication + else { + return invalid_artifact(InvalidCanisterHttpPayloadReason::InvalidPayloadSection( + callback_id, + )); + }; + + match error { + FlexibleCanisterHttpError::Timeout { .. } => { + if context.time + CANISTER_HTTP_TIMEOUT_INTERVAL >= validation_context.time { + return invalid_artifact(InvalidCanisterHttpPayloadReason::NotTimedOut( callback_id, - signer, - }, - ); + )); + } } + FlexibleCanisterHttpError::TooManyRequestErrors { + reject_responses, .. + } => { + let mut seen_signers = HashSet::new(); + + for response_with_proof in reject_responses { + validate_flexible_response_with_proof( + response_with_proof, + callback_id, + flex_committee, + &mut seen_signers, + consensus_registry_version, + &*self.crypto, + ) + .map_err(CanisterHttpPayloadValidationError::InvalidArtifact)?; - // Content hash must match - let calculated_hash = crypto_hash(&entry.response); - if calculated_hash != entry.proof.content.content_hash { - return invalid_artifact( - InvalidCanisterHttpPayloadReason::ContentHashMismatch { - metadata_hash: entry.proof.content.content_hash.clone(), - calculated_hash, - }, - ); - } + if !matches!( + response_with_proof.response.content, + CanisterHttpResponseContent::Reject(_) + ) { + return invalid_artifact( + InvalidCanisterHttpPayloadReason::FlexibleRejectExpectedInErrorResponse( + callback_id, + ), + ); + } + } - // Content size must match - let calculated_size = entry.response.content.count_bytes() as u32; - if calculated_size != entry.proof.content.content_size { - return invalid_artifact( - InvalidCanisterHttpPayloadReason::ContentSizeMismatch { - metadata_size: entry.proof.content.content_size, - calculated_size, - }, - ); + let max_allowed_rejects = + flex_committee.len().saturating_sub(*min_responses as usize); + if reject_responses.len() <= max_allowed_rejects { + return invalid_artifact( + InvalidCanisterHttpPayloadReason::FlexibleInsufficientRejectCount { + callback_id, + reject_count: reject_responses.len(), + min_needed: max_allowed_rejects + 1, + }, + ); + } } + FlexibleCanisterHttpError::ResponsesTooLarge { + metadata_shares, .. + } => { + let mut seen_signers = HashSet::new(); + + for share in metadata_shares { + validate_response_share( + share, + callback_id, + flex_committee, + &mut seen_signers, + consensus_registry_version, + &*self.crypto, + ) + .map_err(CanisterHttpPayloadValidationError::InvalidArtifact)?; + } - // Registry version must match - if entry.proof.content.registry_version != consensus_registry_version { - return invalid_artifact( - InvalidCanisterHttpPayloadReason::RegistryVersionMismatch { - expected: consensus_registry_version, - received: entry.proof.content.registry_version, - }, - ); + if metadata_shares.len() < *min_responses as usize { + return invalid_artifact( + InvalidCanisterHttpPayloadReason::FlexibleInsufficientMetadataShareCount { + callback_id, + share_count: metadata_shares.len(), + min_needed: *min_responses as usize, + }, + ); + } + // Verify the smallest `min_responses` response-with-proof sizes + // actually exceed MAX_CANISTER_HTTP_PAYLOAD_SIZE. + let canister_id = &context.request.sender; + let mut entry_sizes: Vec = metadata_shares + .iter() + .map(|share| { + FlexibleCanisterHttpResponseWithProof::count_bytes_from_parts( + canister_id, + share.content.content_size as usize, + share, + ) + }) + .collect(); + entry_sizes.sort_unstable(); + let smallest_sum: usize = entry_sizes[..*min_responses as usize].iter().sum(); + if smallest_sum <= MAX_CANISTER_HTTP_PAYLOAD_SIZE { + return invalid_artifact( + InvalidCanisterHttpPayloadReason::FlexibleResponsesNotTooLarge( + callback_id, + ), + ); + } } - - // Verify the individual share signature - self.crypto - .verify(&entry.proof, consensus_registry_version) - .map_err(|err| { - CanisterHttpPayloadValidationError::InvalidArtifact( - InvalidCanisterHttpPayloadReason::SignatureError(Box::new(err)), - ) - })?; } } diff --git a/rs/https_outcalls/consensus/src/payload_builder/tests.rs b/rs/https_outcalls/consensus/src/payload_builder/tests.rs index e506b035b80d..f0a6889d9742 100644 --- a/rs/https_outcalls/consensus/src/payload_builder/tests.rs +++ b/rs/https_outcalls/consensus/src/payload_builder/tests.rs @@ -39,8 +39,8 @@ use ic_test_utilities_types::{ use ic_types::{ CountBytes, Height, NodeId, NumBytes, RegistryVersion, ReplicaVersion, batch::{ - CanisterHttpPayload, FlexibleCanisterHttpResponseWithProof, FlexibleCanisterHttpResponses, - MAX_CANISTER_HTTP_PAYLOAD_SIZE, ValidationContext, + CanisterHttpPayload, FlexibleCanisterHttpError, FlexibleCanisterHttpResponseWithProof, + FlexibleCanisterHttpResponses, MAX_CANISTER_HTTP_PAYLOAD_SIZE, ValidationContext, }, canister_http::{ CANISTER_HTTP_MAX_RESPONSES_PER_BLOCK, CANISTER_HTTP_TIMEOUT_INTERVAL, CanisterHttpMethod, @@ -1751,6 +1751,36 @@ fn flexible_build_respects_payload_size_limit() { }); } +#[test] +fn flexible_build_delivers_ok_with_fewer_than_max_when_size_limited() { + let num_nodes = 4; + let committee: BTreeSet<_> = (0..num_nodes as u64).map(node_test_id).collect(); + let callback_id = CallbackId::from(42); + let min = 2_u32; + let max = 4_u32; + + setup_test_with_flexible_context(num_nodes, callback_id, committee, min, max, |pb, pool| { + // For responses just under half of MAX_CANISTER_HTTP_PAYLOAD_SIZE, only exactly 2 fit. + let content_size = MAX_CANISTER_HTTP_PAYLOAD_SIZE / 2 - 1000; + { + let mut pool_access = pool.write().unwrap(); + for node_idx in 0..num_nodes as u64 { + let (response, metadata) = test_response_and_metadata_with_content( + callback_id.get(), + CanisterHttpResponseContent::Success(vec![0xAB; content_size]), + ); + let share = metadata_to_share(node_idx, &metadata); + add_own_share_to_pool(pool_access.deref_mut(), &share, &response); + } + } + + let parsed = build_and_validate_and_parse_payload(&pb); + + assert_eq!(parsed.flexible_responses.len(), 1); + assert_eq!(parsed.flexible_responses[0].responses.len(), 2); + }); +} + #[test] fn flexible_build_respects_max_responses_per_block() { let num_nodes = 4; @@ -2177,9 +2207,9 @@ fn flexible_invalid_callback_id_mismatch_in_proof() { result, Err(ValidationError::InvalidArtifact( InvalidPayloadReason::InvalidCanisterHttpPayload( - InvalidCanisterHttpPayloadReason::FlexibleCallbackIdMismatch { callback_id, mismatched_id } + InvalidCanisterHttpPayloadReason::FlexibleCallbackIdMismatch { callback_id: cb_id, mismatched_id: mm_id } ) - )) if callback_id == callback_id && mismatched_id == mismatched_id + )) if cb_id == callback_id && mm_id == mismatched_id ); }); } @@ -2485,9 +2515,9 @@ fn flexible_invalid_callback_id_mismatch_in_response() { result, Err(ValidationError::InvalidArtifact( InvalidPayloadReason::InvalidCanisterHttpPayload( - InvalidCanisterHttpPayloadReason::FlexibleCallbackIdMismatch { callback_id, mismatched_id } + InvalidCanisterHttpPayloadReason::FlexibleCallbackIdMismatch { callback_id: cb_id, mismatched_id: mm_id } ) - )) if callback_id == callback_id && mismatched_id == mismatched_id + )) if cb_id == callback_id && mm_id == mismatched_id ); }); } @@ -2541,18 +2571,7 @@ fn flexible_invalid_signature_error() { 1, 4, |mut payload_builder, _pool| { - let mut mock_crypto = MockCrypto::new(); - mock_crypto - .expect_verify_basic_sig_http() - .returning(|_, _, _, _| { - Err(ic_types::crypto::CryptoError::SignatureVerification { - algorithm: ic_types::crypto::AlgorithmId::Ed25519, - public_key_bytes: vec![], - sig_bytes: vec![], - internal_error: "mock rejection".to_string(), - }) - }); - payload_builder.crypto = Arc::new(mock_crypto); + payload_builder.crypto = Arc::new(mock_crypto_rejecting_signatures()); let payload = flexible_payload(vec![FlexibleCanisterHttpResponses { callback_id, @@ -2725,182 +2744,1387 @@ fn flexible_ok_responses_into_messages_decode_failure_is_skipped() { assert_eq!(stats.flexible_ok_responses_candid_failures, 1); } -fn setup_test_with_contexts( - num_nodes: usize, - contexts: Vec<(CallbackId, CanisterHttpRequestContext)>, - run: impl FnOnce(CanisterHttpPayloadBuilderImpl, Arc>), -) { - test_config_with_http_feature(true, num_nodes, |mut payload_builder, pool| { - inject_request_contexts(&mut payload_builder, contexts); - run(payload_builder, pool); +#[test] +fn flexible_build_timeout() { + let num_nodes = 4; + let committee: BTreeSet<_> = (0..num_nodes as u64).map(node_test_id).collect(); + let callback_id = CallbackId::from(42); + + setup_test_with_flexible_context(num_nodes, callback_id, committee, 2, 4, |pb, _pool| { + let timed_out_context = ValidationContext { + registry_version: RegistryVersion::new(1), + certified_height: Height::new(0), + time: UNIX_EPOCH + CANISTER_HTTP_TIMEOUT_INTERVAL + Duration::from_secs(1), + }; + let parsed = build_and_validate_and_parse_payload_with_context(&pb, &timed_out_context); + + // Should NOT be in regular timeouts + assert!(parsed.timeouts.is_empty()); + // Should be a flexible timeout error + assert_eq!(parsed.flexible_errors.len(), 1); + assert_matches!( + &parsed.flexible_errors[0], + FlexibleCanisterHttpError::Timeout { callback_id: cb } => { + assert_eq!(*cb, callback_id); + } + ); }); } -fn setup_test_with_flexible_context( - num_nodes: usize, - callback_id: CallbackId, - committee: BTreeSet, - min_responses: u32, - max_responses: u32, - run: impl FnOnce(CanisterHttpPayloadBuilderImpl, Arc>), -) { - setup_test_with_contexts( - num_nodes, - vec![( - callback_id, - flexible_request_context(committee, min_responses, max_responses), - )], - run, - ); -} +#[test] +fn flexible_build_responses_too_large() { + let num_nodes = 4; + let committee: BTreeSet<_> = (0..num_nodes as u64).map(node_test_id).collect(); + let callback_id = CallbackId::from(42); -/// Replaces the payload_builder's state_reader with one containing the given request contexts. -pub(crate) fn inject_request_contexts( - payload_builder: &mut CanisterHttpPayloadBuilderImpl, - contexts: impl IntoIterator, -) { - let mut init_state = ic_test_utilities_state::get_initial_state(0, 0); - for (cb, ctx) in contexts { - init_state - .metadata - .subnet_call_context_manager - .canister_http_request_contexts - .insert(cb, ctx); - } - let state_manager = Arc::new(RefMockStateManager::default()); - state_manager - .get_mut() - .expect_get_state_at() - .return_const(Ok(ic_interfaces_state_manager::Labeled::new( - Height::new(0), - Arc::new(init_state), - ))); - payload_builder.state_reader = state_manager; -} + // min_responses = 2; all 4 nodes submit OK responses each >1 MiB so that + // the smallest 2 exceed MAX_CANISTER_HTTP_PAYLOAD_SIZE when summed. + // All members must respond so that num_unseen=0, otherwise the check + // correctly stays Pending (unseen members could still send small responses). + let body_size = (MAX_CANISTER_HTTP_PAYLOAD_SIZE / 2) + 100_000; + setup_test_with_flexible_context(num_nodes, callback_id, committee, 2, 4, |pb, pool| { + { + let mut pool_access = pool.write().unwrap(); + for node_idx in 0..4_u64 { + let body = vec![0xAA_u8; body_size]; + let (response, metadata) = test_response_and_metadata_with_content( + callback_id.get(), + CanisterHttpResponseContent::Success(body), + ); + let share = metadata_to_share(node_idx, &metadata); + add_own_share_to_pool(pool_access.deref_mut(), &share, &response); + } + } -fn fully_replicated_contexts( - ids: impl IntoIterator, -) -> Vec<(CallbackId, CanisterHttpRequestContext)> { - ids.into_iter() - .map(|id| { - ( - CallbackId::new(id), - request_context(Replication::FullyReplicated), - ) - }) - .collect() -} + let parsed = build_and_validate_and_parse_payload(&pb); -pub(crate) fn request_context(replication: Replication) -> CanisterHttpRequestContext { - CanisterHttpRequestContext { - request: RequestBuilder::default().build(), - url: "https://example.com".to_string(), - max_response_bytes: None, - headers: vec![], - body: None, - http_method: CanisterHttpMethod::GET, - transform: None, - time: UNIX_EPOCH, - replication, - pricing_version: ic_types::canister_http::PricingVersion::Legacy, - refund_status: ic_types::canister_http::RefundStatus::default(), - } + assert!(parsed.flexible_responses.is_empty()); + assert_eq!(parsed.flexible_errors.len(), 1); + assert_matches!( + &parsed.flexible_errors[0], + FlexibleCanisterHttpError::ResponsesTooLarge { + callback_id: cb, + metadata_shares, + } => { + assert_eq!(*cb, callback_id); + assert_eq!(metadata_shares.len(), 2); + } + ); + }); } -fn flexible_request_context( - committee: BTreeSet, - min_responses: u32, - max_responses: u32, -) -> CanisterHttpRequestContext { - CanisterHttpRequestContext { - request: RequestBuilder::default().build(), - url: "https://example.com".to_string(), - max_response_bytes: None, - headers: vec![], - body: None, - http_method: CanisterHttpMethod::GET, - transform: None, - time: UNIX_EPOCH, - replication: Replication::Flexible { - committee, - min_responses, - max_responses, - }, - pricing_version: ic_types::canister_http::PricingVersion::PayAsYouGo, - refund_status: ic_types::canister_http::RefundStatus::default(), - } -} +#[test] +fn flexible_build_responses_too_large_stays_pending_when_unseen_members_could_help() { + let num_nodes = 6; + let committee: BTreeSet<_> = (0..num_nodes as u64).map(node_test_id).collect(); + let callback_id = CallbackId::from(42); -fn flexible_response( - callback_id: u64, - signer_node: u64, - content: &[u8], -) -> FlexibleCanisterHttpResponseWithProof { - let (response, metadata) = test_response_and_metadata_with_content( - callback_id, - CanisterHttpResponseContent::Success(content.to_vec()), - ); - FlexibleCanisterHttpResponseWithProof { - response, - proof: metadata_to_share(signer_node, &metadata), - } -} + // min_responses=3, committee=6. We submit 3 large OK shares that individually exceed + // MAX/3, making their sum exceed MAX. But 3 committee members are still unseen and + // could submit small responses, so the result should be Pending, not ResponsesTooLarge. + let body_size = (MAX_CANISTER_HTTP_PAYLOAD_SIZE / 3) + 100_000; + setup_test_with_flexible_context(num_nodes, callback_id, committee, 3, 6, |pb, pool| { + { + let mut pool_access = pool.write().unwrap(); + for node_idx in 0..3_u64 { + let body = vec![0xAA_u8; body_size]; + let (response, metadata) = test_response_and_metadata_with_content( + callback_id.get(), + CanisterHttpResponseContent::Success(body), + ); + let share = metadata_to_share(node_idx, &metadata); + add_own_share_to_pool(pool_access.deref_mut(), &share, &response); + } + } -fn flexible_reject_response( - callback_id: u64, - signer_node: u64, -) -> FlexibleCanisterHttpResponseWithProof { - use ic_types::canister_http::CanisterHttpReject; + let parsed = build_and_validate_and_parse_payload(&pb); - let (response, metadata) = test_response_and_metadata_with_content( - callback_id, - CanisterHttpResponseContent::Reject(CanisterHttpReject { - reject_code: RejectCode::SysTransient, - message: "could not connect".to_string(), - }), - ); - FlexibleCanisterHttpResponseWithProof { - response, - proof: metadata_to_share(signer_node, &metadata), - } + // Unseen members could still submit small OK responses → Pending. + assert!(parsed.flexible_responses.is_empty()); + assert!(parsed.flexible_errors.is_empty()); + }); } -fn flexible_payload(groups: Vec) -> CanisterHttpPayload { - CanisterHttpPayload { - responses: vec![], - timeouts: vec![], - divergence_responses: vec![], - flexible_responses: groups, - flexible_errors: vec![], - } -} +#[test] +fn flexible_build_responses_too_large_with_rejects_reducing_unseen() { + let num_nodes = 6; + let committee: BTreeSet<_> = (0..num_nodes as u64).map(node_test_id).collect(); + let callback_id = CallbackId::from(42); -fn add_flexible_shares_to_pool( - pool: &Arc>, - callback_id: CallbackId, - node_range: std::ops::Range, -) { - let mut pool_access = pool.write().unwrap(); - for node_idx in node_range { - let (response, metadata) = test_response_and_metadata_with_content( - callback_id.get(), - CanisterHttpResponseContent::Success(format!("resp_{node_idx}").into_bytes()), + // min_responses=3, committee=6. + // 3 large OK shares (sum > MAX) from nodes 0..3. + // 2 rejects from nodes 3..5 → only 1 unseen member remains. + // min_minus_unseen = 3 - 1 = 2, and even the 2 smallest known OK shares exceed MAX. + let body_size = (MAX_CANISTER_HTTP_PAYLOAD_SIZE / 2) + 100_000; + setup_test_with_flexible_context(num_nodes, callback_id, committee, 3, 6, |pb, pool| { + { + use ic_types::canister_http::CanisterHttpReject; + + let mut pool_access = pool.write().unwrap(); + for node_idx in 0..3_u64 { + let body = vec![0xAA_u8; body_size]; + let (response, metadata) = test_response_and_metadata_with_content( + callback_id.get(), + CanisterHttpResponseContent::Success(body), + ); + let share = metadata_to_share(node_idx, &metadata); + add_own_share_to_pool(pool_access.deref_mut(), &share, &response); + } + for node_idx in 3..5_u64 { + let (response, metadata) = test_response_and_metadata_with_content( + callback_id.get(), + CanisterHttpResponseContent::Reject(CanisterHttpReject { + reject_code: RejectCode::SysTransient, + message: format!("error_{node_idx}"), + }), + ); + let share = metadata_to_share(node_idx, &metadata); + add_own_share_to_pool(pool_access.deref_mut(), &share, &response); + } + } + + let parsed = build_and_validate_and_parse_payload(&pb); + + // Rejects reduce unseen count, making it impossible to fit → ResponsesTooLarge. + // The error includes min_responses (3) shares for validation, even though + // only min_minus_unseen (2) were needed to prove impossibility. + assert!(parsed.flexible_responses.is_empty()); + assert_eq!(parsed.flexible_errors.len(), 1); + assert_matches!( + &parsed.flexible_errors[0], + FlexibleCanisterHttpError::ResponsesTooLarge { + callback_id: cb, + metadata_shares, + } => { + assert_eq!(*cb, callback_id); + assert_eq!(metadata_shares.len(), 3); + } ); - let share = metadata_to_share(node_idx, &metadata); - add_own_share_to_pool(pool_access.deref_mut(), &share, &response); - } + }); } -fn build_and_validate_and_parse_payload( - payload_builder: &CanisterHttpPayloadBuilderImpl, -) -> CanisterHttpPayload { - let context = default_validation_context(); - let max_size = NumBytes::new(MAX_CANISTER_HTTP_PAYLOAD_SIZE as u64); - let payload = payload_builder.build_payload(Height::new(1), max_size, &[], &context); - assert_matches!( - payload_builder.validate_payload( - Height::new(1), - &test_proposal_context(&context), +#[test] +fn flexible_build_too_many_request_errors() { + let num_nodes = 4; + let committee: BTreeSet<_> = (0..num_nodes as u64).map(node_test_id).collect(); + let callback_id = CallbackId::from(42); + + // min_responses=3, so at most 1 reject is tolerable. 2 rejects should trigger TooManyRequestErrors. + setup_test_with_flexible_context(num_nodes, callback_id, committee, 3, 4, |pb, pool| { + { + use ic_types::canister_http::CanisterHttpReject; + + let mut pool_access = pool.write().unwrap(); + // Nodes 0 and 1 produce Reject responses + for node_idx in 0..2_u64 { + let (response, metadata) = test_response_and_metadata_with_content( + callback_id.get(), + CanisterHttpResponseContent::Reject(CanisterHttpReject { + reject_code: RejectCode::SysTransient, + message: format!("error_{node_idx}"), + }), + ); + let share = metadata_to_share(node_idx, &metadata); + add_own_share_to_pool(pool_access.deref_mut(), &share, &response); + } + // Nodes 2 and 3 produce OK responses + for node_idx in 2..4_u64 { + let (response, metadata) = test_response_and_metadata_with_content( + callback_id.get(), + CanisterHttpResponseContent::Success(format!("resp_{node_idx}").into_bytes()), + ); + let share = metadata_to_share(node_idx, &metadata); + add_own_share_to_pool(pool_access.deref_mut(), &share, &response); + } + } + + let parsed = build_and_validate_and_parse_payload(&pb); + + assert!(parsed.flexible_responses.is_empty()); + assert_eq!(parsed.flexible_errors.len(), 1); + assert_matches!( + &parsed.flexible_errors[0], + FlexibleCanisterHttpError::TooManyRequestErrors { + callback_id: cb, + reject_responses, + } => { + assert_eq!(*cb, callback_id); + assert_eq!(reject_responses.len(), 2); + for entry in reject_responses { + assert_matches!( + entry.response.content, + CanisterHttpResponseContent::Reject(_) + ); + } + } + ); + }); +} + +#[test] +fn flexible_build_not_enough_rejects_stays_pending() { + let num_nodes = 4; + let committee: BTreeSet<_> = (0..num_nodes as u64).map(node_test_id).collect(); + let callback_id = CallbackId::from(42); + + // min_responses=3, so at most 1 reject is tolerable. Only 1 reject + 1 OK → pending. + setup_test_with_flexible_context(num_nodes, callback_id, committee, 3, 4, |pb, pool| { + { + use ic_types::canister_http::CanisterHttpReject; + + let mut pool_access = pool.write().unwrap(); + // Node 0 produces a Reject + let (response, metadata) = test_response_and_metadata_with_content( + callback_id.get(), + CanisterHttpResponseContent::Reject(CanisterHttpReject { + reject_code: RejectCode::SysTransient, + message: "error".to_string(), + }), + ); + let share = metadata_to_share(0, &metadata); + add_own_share_to_pool(pool_access.deref_mut(), &share, &response); + // Node 1 produces an OK response + let (response, metadata) = test_response_and_metadata_with_content( + callback_id.get(), + CanisterHttpResponseContent::Success(b"ok_resp".to_vec()), + ); + let share = metadata_to_share(1, &metadata); + add_own_share_to_pool(pool_access.deref_mut(), &share, &response); + } + + let parsed = build_and_validate_and_parse_payload(&pb); + + assert_eq!(parsed.flexible_responses, vec![]); + assert_eq!(parsed.flexible_errors, vec![]); + }); +} + +#[test] +fn flexible_build_ok_takes_precedence_over_rejects() { + let num_nodes = 5; + let committee: BTreeSet<_> = (0..num_nodes as u64).map(node_test_id).collect(); + let callback_id = CallbackId::from(42); + + // min_responses=2, max_responses=5. 2 rejects + 3 OK → OkResponses even though + // reject_count(2) > committee.len()-min_responses(3). Because we have enough OK responses. + setup_test_with_flexible_context(num_nodes, callback_id, committee, 2, 5, |pb, pool| { + { + use ic_types::canister_http::CanisterHttpReject; + + let mut pool_access = pool.write().unwrap(); + for node_idx in 0..2_u64 { + let (response, metadata) = test_response_and_metadata_with_content( + callback_id.get(), + CanisterHttpResponseContent::Reject(CanisterHttpReject { + reject_code: RejectCode::SysTransient, + message: format!("error_{node_idx}"), + }), + ); + let share = metadata_to_share(node_idx, &metadata); + add_own_share_to_pool(pool_access.deref_mut(), &share, &response); + } + for node_idx in 2..5_u64 { + let (response, metadata) = test_response_and_metadata_with_content( + callback_id.get(), + CanisterHttpResponseContent::Success(format!("resp_{node_idx}").into_bytes()), + ); + let share = metadata_to_share(node_idx, &metadata); + add_own_share_to_pool(pool_access.deref_mut(), &share, &response); + } + } + + let parsed = build_and_validate_and_parse_payload(&pb); + + assert_eq!(parsed.flexible_responses.len(), 1); + assert_eq!(parsed.flexible_errors, vec![]); + }); +} + +#[test] +fn flexible_build_prioritizes_smaller_responses() { + let num_nodes = 4; + let committee: BTreeSet<_> = (0..num_nodes as u64).map(node_test_id).collect(); + let callback_id = CallbackId::from(42); + + // max_responses = 2 so only 2 of the 3 responses can be included. + setup_test_with_flexible_context(num_nodes, callback_id, committee, 2, 2, |pb, pool| { + let small = b"s"; + let medium = b"medium_content"; + let large = b"large_content_that_is_significantly_bigger"; + + { + let mut pool_access = pool.write().unwrap(); + // Insert shares from 3 different nodes with different content sizes. + // Deliberately insert large before small to ensure it's the sort, + // not insertion order, that determines the result. + for (node, content) in [(0_u64, large.as_slice()), (1, small), (2, medium)] { + let (response, metadata) = test_response_and_metadata_with_content( + callback_id.get(), + CanisterHttpResponseContent::Success(content.to_vec()), + ); + let share = metadata_to_share(node, &metadata); + add_own_share_to_pool(pool_access.deref_mut(), &share, &response); + } + } + + let parsed = build_and_validate_and_parse_payload(&pb); + assert_eq!(parsed.flexible_responses.len(), 1); + let group = &parsed.flexible_responses[0]; + assert_eq!(group.responses.len(), 2); + + let included_bodies: Vec<_> = group + .responses + .iter() + .filter_map(|e| match &e.response.content { + CanisterHttpResponseContent::Success(bytes) => Some(bytes.clone()), + _ => None, + }) + .collect(); + + assert_eq!(included_bodies[0], small); + assert_eq!(included_bodies[1], medium); + }); +} + +#[test] +fn flexible_error_timeout_valid() { + let num_nodes = 4; + let committee: BTreeSet<_> = (0..num_nodes as u64).map(node_test_id).collect(); + let callback_id = CallbackId::from(42); + + setup_test_with_contexts( + num_nodes, + vec![(callback_id, flexible_request_context(committee, 2, 4))], + |pb, _pool| { + let timed_out_context = ValidationContext { + registry_version: RegistryVersion::new(1), + certified_height: Height::new(0), + time: UNIX_EPOCH + CANISTER_HTTP_TIMEOUT_INTERVAL + Duration::from_secs(1), + }; + let payload = CanisterHttpPayload { + flexible_errors: vec![FlexibleCanisterHttpError::Timeout { callback_id }], + ..Default::default() + }; + let result = pb.validate_payload( + Height::new(1), + &test_proposal_context(&timed_out_context), + &payload_to_bytes_max_4mb(payload), + &[], + ); + assert_matches!(result, Ok(())); + }, + ); +} + +#[test] +fn flexible_error_timeout_invalid_not_expired() { + let num_nodes = 4; + let committee: BTreeSet<_> = (0..num_nodes as u64).map(node_test_id).collect(); + let callback_id = CallbackId::from(42); + + setup_test_with_flexible_context(num_nodes, callback_id, committee, 2, 4, |pb, _pool| { + let payload = CanisterHttpPayload { + flexible_errors: vec![FlexibleCanisterHttpError::Timeout { callback_id }], + ..Default::default() + }; + let result = pb.validate_payload( + Height::new(1), + &test_proposal_context(&default_validation_context()), + &payload_to_bytes_max_4mb(payload), + &[], + ); + assert_matches!( + result, + Err(ValidationError::InvalidArtifact( + InvalidPayloadReason::InvalidCanisterHttpPayload( + InvalidCanisterHttpPayloadReason::NotTimedOut(id) + ) + )) if id == callback_id + ); + }); +} + +#[test] +fn flexible_error_timeout_invalid_non_flexible_request() { + let num_nodes = 4; + let callback_id = CallbackId::from(42); + + // A non-flexible request should not be able to produce a flexible timeout error + setup_test_with_contexts(num_nodes, fully_replicated_contexts([42]), |pb, _pool| { + let timed_out_context = ValidationContext { + registry_version: RegistryVersion::new(1), + certified_height: Height::new(0), + time: UNIX_EPOCH + CANISTER_HTTP_TIMEOUT_INTERVAL + Duration::from_secs(1), + }; + let payload = CanisterHttpPayload { + flexible_errors: vec![FlexibleCanisterHttpError::Timeout { callback_id }], + ..Default::default() + }; + let result = pb.validate_payload( + Height::new(1), + &test_proposal_context(&timed_out_context), + &payload_to_bytes_max_4mb(payload), + &[], + ); + assert_matches!( + result, + Err(ValidationError::InvalidArtifact( + InvalidPayloadReason::InvalidCanisterHttpPayload( + InvalidCanisterHttpPayloadReason::InvalidPayloadSection(id) + ) + )) if id == callback_id + ); + }); +} + +#[test] +fn flexible_error_duplicate_callback_id() { + let num_nodes = 4; + let committee: BTreeSet<_> = (0..num_nodes as u64).map(node_test_id).collect(); + let callback_id = CallbackId::from(42); + + setup_test_with_flexible_context(num_nodes, callback_id, committee, 2, 4, |pb, _pool| { + let timed_out_context = ValidationContext { + registry_version: RegistryVersion::new(1), + certified_height: Height::new(0), + time: UNIX_EPOCH + CANISTER_HTTP_TIMEOUT_INTERVAL + Duration::from_secs(1), + }; + let payload = CanisterHttpPayload { + flexible_errors: vec![ + FlexibleCanisterHttpError::Timeout { callback_id }, + FlexibleCanisterHttpError::Timeout { callback_id }, + ], + ..Default::default() + }; + let result = pb.validate_payload( + Height::new(1), + &test_proposal_context(&timed_out_context), + &payload_to_bytes_max_4mb(payload), + &[], + ); + assert_matches!( + result, + Err(ValidationError::InvalidArtifact( + InvalidPayloadReason::InvalidCanisterHttpPayload( + InvalidCanisterHttpPayloadReason::DuplicateResponse(id) + ) + )) if id == callback_id + ); + }); +} + +#[test] +fn flexible_error_duplicate_callback_id_cross_type() { + let num_nodes = 4; + let committee: BTreeSet<_> = (0..num_nodes as u64).map(node_test_id).collect(); + let callback_id = CallbackId::from(42); + + setup_test_with_flexible_context(num_nodes, callback_id, committee, 1, 4, |pb, _pool| { + let timed_out_context = ValidationContext { + registry_version: RegistryVersion::new(1), + certified_height: Height::new(0), + time: UNIX_EPOCH + CANISTER_HTTP_TIMEOUT_INTERVAL + Duration::from_secs(1), + }; + let payload = CanisterHttpPayload { + flexible_responses: vec![FlexibleCanisterHttpResponses { + callback_id, + responses: vec![flexible_response(42, 0, b"a")], + }], + flexible_errors: vec![FlexibleCanisterHttpError::Timeout { callback_id }], + ..Default::default() + }; + let result = pb.validate_payload( + Height::new(1), + &test_proposal_context(&timed_out_context), + &payload_to_bytes_max_4mb(payload), + &[], + ); + assert_matches!( + result, + Err(ValidationError::InvalidArtifact( + InvalidPayloadReason::InvalidCanisterHttpPayload( + InvalidCanisterHttpPayloadReason::DuplicateResponse(id) + ) + )) if id == callback_id + ); + }); +} + +#[test] +fn flexible_error_responses_too_large_valid() { + let num_nodes = 4; + let committee: BTreeSet<_> = (0..num_nodes as u64).map(node_test_id).collect(); + let callback_id = CallbackId::from(42); + + // min_responses = 2; each share claims ~1.1 MiB of content → 2 × ~1.1 MiB > 2 MiB. + let huge_content_size = (MAX_CANISTER_HTTP_PAYLOAD_SIZE as u32 / 2) + 100_000; + setup_test_with_flexible_context(num_nodes, callback_id, committee, 2, 4, |pb, _pool| { + let share_a = metadata_share_with_content_size(callback_id.get(), 0, huge_content_size); + let share_b = metadata_share_with_content_size(callback_id.get(), 1, huge_content_size); + + let payload = CanisterHttpPayload { + flexible_errors: vec![FlexibleCanisterHttpError::ResponsesTooLarge { + callback_id, + metadata_shares: vec![share_a, share_b], + }], + ..Default::default() + }; + let result = pb.validate_payload( + Height::new(1), + &test_proposal_context(&default_validation_context()), + &payload_to_bytes_max_4mb(payload), + &[], + ); + assert_matches!(result, Ok(())); + }); +} + +#[test] +fn flexible_error_responses_too_large_invalid_when_small() { + let num_nodes = 4; + let committee: BTreeSet<_> = (0..num_nodes as u64).map(node_test_id).collect(); + let callback_id = CallbackId::from(42); + + // Shares with small content_size should not pass the ResponsesTooLarge check + setup_test_with_flexible_context(num_nodes, callback_id, committee, 2, 4, |pb, _pool| { + let entry_a = flexible_response(callback_id.get(), 0, b"small_a"); + let entry_b = flexible_response(callback_id.get(), 1, b"small_b"); + + let payload = CanisterHttpPayload { + flexible_errors: vec![FlexibleCanisterHttpError::ResponsesTooLarge { + callback_id, + metadata_shares: vec![entry_a.proof.clone(), entry_b.proof.clone()], + }], + ..Default::default() + }; + let result = pb.validate_payload( + Height::new(1), + &test_proposal_context(&default_validation_context()), + &payload_to_bytes_max_4mb(payload), + &[], + ); + assert_matches!( + result, + Err(ValidationError::InvalidArtifact( + InvalidPayloadReason::InvalidCanisterHttpPayload( + InvalidCanisterHttpPayloadReason::FlexibleResponsesNotTooLarge(id) + ) + )) if id == callback_id + ); + }); +} + +#[test] +fn flexible_error_responses_too_large_too_few_shares() { + let num_nodes = 4; + let committee: BTreeSet<_> = (0..num_nodes as u64).map(node_test_id).collect(); + let callback_id = CallbackId::from(42); + + // min_responses = 3 but only 2 shares provided → invalid + let huge = (MAX_CANISTER_HTTP_PAYLOAD_SIZE as u32 / 2) + 100_000; + setup_test_with_flexible_context(num_nodes, callback_id, committee, 3, 4, |pb, _pool| { + let share_a = metadata_share_with_content_size(callback_id.get(), 0, huge); + let share_b = metadata_share_with_content_size(callback_id.get(), 1, huge); + + let payload = CanisterHttpPayload { + flexible_errors: vec![FlexibleCanisterHttpError::ResponsesTooLarge { + callback_id, + metadata_shares: vec![share_a, share_b], + }], + ..Default::default() + }; + let result = pb.validate_payload( + Height::new(1), + &test_proposal_context(&default_validation_context()), + &payload_to_bytes_max_4mb(payload), + &[], + ); + assert_matches!( + result, + Err(ValidationError::InvalidArtifact( + InvalidPayloadReason::InvalidCanisterHttpPayload( + InvalidCanisterHttpPayloadReason::FlexibleInsufficientMetadataShareCount { + callback_id: id, + share_count: 2, + min_needed: 3, + } + ) + )) if id == callback_id + ); + }); +} + +#[test] +fn flexible_error_responses_too_large_callback_id_mismatch() { + let num_nodes = 4; + let committee: BTreeSet<_> = (0..num_nodes as u64).map(node_test_id).collect(); + let callback_id = CallbackId::from(42); + + let huge = (MAX_CANISTER_HTTP_PAYLOAD_SIZE as u32 / 2) + 100_000; + setup_test_with_flexible_context(num_nodes, callback_id, committee, 2, 4, |pb, _pool| { + let share_ok = metadata_share_with_content_size(callback_id.get(), 0, huge); + // Share with wrong callback_id + let mismatched_id = CallbackId::new(999); + let share_wrong = metadata_share_with_content_size(mismatched_id.get(), 1, huge); + + let payload = CanisterHttpPayload { + flexible_errors: vec![FlexibleCanisterHttpError::ResponsesTooLarge { + callback_id, + metadata_shares: vec![share_ok, share_wrong], + }], + ..Default::default() + }; + let result = pb.validate_payload( + Height::new(1), + &test_proposal_context(&default_validation_context()), + &payload_to_bytes_max_4mb(payload), + &[], + ); + assert_matches!( + result, + Err(ValidationError::InvalidArtifact( + InvalidPayloadReason::InvalidCanisterHttpPayload( + InvalidCanisterHttpPayloadReason::FlexibleCallbackIdMismatch { callback_id: cb_id, mismatched_id: mm_id } + ) + )) if cb_id == callback_id && mm_id == mismatched_id + ); + }); +} + +#[test] +fn flexible_error_responses_too_large_duplicate_signer() { + let num_nodes = 4; + let committee: BTreeSet<_> = (0..num_nodes as u64).map(node_test_id).collect(); + let callback_id = CallbackId::from(42); + + let huge = (MAX_CANISTER_HTTP_PAYLOAD_SIZE as u32 / 2) + 100_000; + setup_test_with_flexible_context(num_nodes, callback_id, committee, 2, 4, |pb, _pool| { + // Both shares from the same signer (node 0) + let share_a = metadata_share_with_content_size(callback_id.get(), 0, huge); + let share_b = metadata_share_with_content_size(callback_id.get(), 0, huge); + + let payload = CanisterHttpPayload { + flexible_errors: vec![FlexibleCanisterHttpError::ResponsesTooLarge { + callback_id, + metadata_shares: vec![share_a, share_b], + }], + ..Default::default() + }; + let result = pb.validate_payload( + Height::new(1), + &test_proposal_context(&default_validation_context()), + &payload_to_bytes_max_4mb(payload), + &[], + ); + assert_matches!( + result, + Err(ValidationError::InvalidArtifact( + InvalidPayloadReason::InvalidCanisterHttpPayload( + InvalidCanisterHttpPayloadReason::FlexibleDuplicateSigner { callback_id: cb_id, signer: s } + ) + )) if cb_id == callback_id && s == node_test_id(0) + ); + }); +} + +#[test] +fn flexible_error_responses_too_large_signer_not_in_committee() { + let num_nodes = 4; + let committee: BTreeSet<_> = (0..num_nodes as u64).map(node_test_id).collect(); + let callback_id = CallbackId::from(42); + + let huge = (MAX_CANISTER_HTTP_PAYLOAD_SIZE as u32 / 2) + 100_000; + setup_test_with_flexible_context(num_nodes, callback_id, committee, 2, 4, |pb, _pool| { + let share_ok = metadata_share_with_content_size(callback_id.get(), 0, huge); + let share_bad = metadata_share_with_content_size(callback_id.get(), 99, huge); + + let payload = CanisterHttpPayload { + flexible_errors: vec![FlexibleCanisterHttpError::ResponsesTooLarge { + callback_id, + metadata_shares: vec![share_ok, share_bad], + }], + ..Default::default() + }; + let result = pb.validate_payload( + Height::new(1), + &test_proposal_context(&default_validation_context()), + &payload_to_bytes_max_4mb(payload), + &[], + ); + assert_matches!( + result, + Err(ValidationError::InvalidArtifact( + InvalidPayloadReason::InvalidCanisterHttpPayload( + InvalidCanisterHttpPayloadReason::FlexibleSignerNotInCommittee { callback_id: cb_id, signer: s } + ) + )) if cb_id == callback_id && s == node_test_id(99) + ); + }); +} + +#[test] +fn flexible_error_responses_too_large_registry_version_mismatch() { + let num_nodes = 4; + let committee: BTreeSet<_> = (0..num_nodes as u64).map(node_test_id).collect(); + let callback_id = CallbackId::from(42); + + let huge = (MAX_CANISTER_HTTP_PAYLOAD_SIZE as u32 / 2) + 100_000; + setup_test_with_flexible_context(num_nodes, callback_id, committee, 2, 4, |pb, _pool| { + let share_ok = metadata_share_with_content_size(callback_id.get(), 0, huge); + // Share with wrong registry version + let mut share_bad = metadata_share_with_content_size(callback_id.get(), 1, huge); + share_bad.content.registry_version = RegistryVersion::new(999); + + let payload = CanisterHttpPayload { + flexible_errors: vec![FlexibleCanisterHttpError::ResponsesTooLarge { + callback_id, + metadata_shares: vec![share_ok, share_bad], + }], + ..Default::default() + }; + let result = pb.validate_payload( + Height::new(1), + &test_proposal_context(&default_validation_context()), + &payload_to_bytes_max_4mb(payload), + &[], + ); + assert_matches!( + result, + Err(ValidationError::InvalidArtifact( + InvalidPayloadReason::InvalidCanisterHttpPayload( + InvalidCanisterHttpPayloadReason::RegistryVersionMismatch { expected: e, received: r } + ) + )) if e == RegistryVersion::new(1) && r == RegistryVersion::new(999) + ); + }); +} + +#[test] +fn flexible_error_responses_too_large_invalid_signature() { + let committee: BTreeSet<_> = (0..4).map(node_test_id).collect(); + let callback_id = CallbackId::from(42); + + let huge = (MAX_CANISTER_HTTP_PAYLOAD_SIZE as u32 / 2) + 100_000; + setup_test_with_flexible_context(4, callback_id, committee, 2, 4, |mut pb, _pool| { + pb.crypto = Arc::new(mock_crypto_rejecting_signatures()); + + let share_a = metadata_share_with_content_size(callback_id.get(), 0, huge); + let share_b = metadata_share_with_content_size(callback_id.get(), 1, huge); + + let payload = CanisterHttpPayload { + flexible_errors: vec![FlexibleCanisterHttpError::ResponsesTooLarge { + callback_id, + metadata_shares: vec![share_a, share_b], + }], + ..Default::default() + }; + let result = pb.validate_payload( + Height::new(1), + &test_proposal_context(&default_validation_context()), + &payload_to_bytes_max_4mb(payload), + &[], + ); + assert_matches!( + result, + Err(ValidationError::InvalidArtifact( + InvalidPayloadReason::InvalidCanisterHttpPayload( + InvalidCanisterHttpPayloadReason::SignatureError(_) + ) + )) + ); + }); +} + +#[test] +fn flexible_error_too_many_request_errors_valid() { + let num_nodes = 4; + let committee: BTreeSet<_> = (0..num_nodes as u64).map(node_test_id).collect(); + let callback_id = CallbackId::from(42); + + // min_responses=3, committee=4. max_allowed_rejects = 4-3 = 1. + // 2 rejects should be valid for TooManyRequestErrors. + setup_test_with_flexible_context(num_nodes, callback_id, committee, 3, 4, |pb, _pool| { + let reject_entries: Vec<_> = (0..2_u64) + .map(|node_idx| flexible_reject_response(callback_id.get(), node_idx)) + .collect(); + + let payload = CanisterHttpPayload { + flexible_errors: vec![FlexibleCanisterHttpError::TooManyRequestErrors { + callback_id, + reject_responses: reject_entries, + }], + ..Default::default() + }; + let result = pb.validate_payload( + Height::new(1), + &test_proposal_context(&default_validation_context()), + &payload_to_bytes_max_4mb(payload), + &[], + ); + assert_matches!(result, Ok(())); + }); +} + +#[test] +fn flexible_error_too_many_request_errors_insufficient_rejects() { + let num_nodes = 4; + let committee: BTreeSet<_> = (0..num_nodes as u64).map(node_test_id).collect(); + let callback_id = CallbackId::from(42); + + // min_responses=3, committee=4. max_allowed_rejects = 1. + // Only 1 reject → not enough for TooManyRequestErrors. + setup_test_with_flexible_context(num_nodes, callback_id, committee, 3, 4, |pb, _pool| { + let reject_entries: Vec<_> = (0..1_u64) + .map(|node_idx| flexible_reject_response(callback_id.get(), node_idx)) + .collect(); + + let payload = CanisterHttpPayload { + flexible_errors: vec![FlexibleCanisterHttpError::TooManyRequestErrors { + callback_id, + reject_responses: reject_entries, + }], + ..Default::default() + }; + let result = pb.validate_payload( + Height::new(1), + &test_proposal_context(&default_validation_context()), + &payload_to_bytes_max_4mb(payload), + &[], + ); + assert_matches!( + result, + Err(ValidationError::InvalidArtifact( + InvalidPayloadReason::InvalidCanisterHttpPayload( + InvalidCanisterHttpPayloadReason::FlexibleInsufficientRejectCount { + callback_id: cb_id, + reject_count: rc, + min_needed: mn, + } + ) + )) if cb_id == callback_id && rc == 1 && mn == 2 + ); + }); +} + +#[test] +fn flexible_error_too_many_request_errors_non_reject_content() { + let num_nodes = 4; + let committee: BTreeSet<_> = (0..num_nodes as u64).map(node_test_id).collect(); + let callback_id = CallbackId::from(42); + + setup_test_with_flexible_context(num_nodes, callback_id, committee, 3, 4, |pb, _pool| { + let ok_entry = flexible_response(callback_id.get(), 0, b"ok data"); + let reject_entry = flexible_reject_response(callback_id.get(), 1); + + let payload = CanisterHttpPayload { + flexible_errors: vec![FlexibleCanisterHttpError::TooManyRequestErrors { + callback_id, + reject_responses: vec![ok_entry, reject_entry], + }], + ..Default::default() + }; + let result = pb.validate_payload( + Height::new(1), + &test_proposal_context(&default_validation_context()), + &payload_to_bytes_max_4mb(payload), + &[], + ); + assert_matches!( + result, + Err(ValidationError::InvalidArtifact( + InvalidPayloadReason::InvalidCanisterHttpPayload( + InvalidCanisterHttpPayloadReason::FlexibleRejectExpectedInErrorResponse(cb_id) + ) + )) if cb_id == callback_id + ); + }); +} + +#[test] +fn flexible_error_too_many_request_errors_duplicate_signer() { + let num_nodes = 4; + let committee: BTreeSet<_> = (0..num_nodes as u64).map(node_test_id).collect(); + let callback_id = CallbackId::from(42); + + setup_test_with_flexible_context(num_nodes, callback_id, committee, 3, 4, |pb, _pool| { + let entry_a = flexible_reject_response(callback_id.get(), 0); + let entry_b = flexible_reject_response(callback_id.get(), 0); + + let payload = CanisterHttpPayload { + flexible_errors: vec![FlexibleCanisterHttpError::TooManyRequestErrors { + callback_id, + reject_responses: vec![entry_a, entry_b], + }], + ..Default::default() + }; + let result = pb.validate_payload( + Height::new(1), + &test_proposal_context(&default_validation_context()), + &payload_to_bytes_max_4mb(payload), + &[], + ); + assert_matches!( + result, + Err(ValidationError::InvalidArtifact( + InvalidPayloadReason::InvalidCanisterHttpPayload( + InvalidCanisterHttpPayloadReason::FlexibleDuplicateSigner { callback_id: cb_id, signer: s } + ) + )) if cb_id == callback_id && s == node_test_id(0) + ); + }); +} + +#[test] +fn flexible_error_too_many_request_errors_signer_not_in_committee() { + let num_nodes = 4; + let committee: BTreeSet<_> = (0..num_nodes as u64).map(node_test_id).collect(); + let callback_id = CallbackId::from(42); + + setup_test_with_flexible_context(num_nodes, callback_id, committee, 3, 4, |pb, _pool| { + // Node 99 is not in the committee + let entry_a = flexible_reject_response(callback_id.get(), 0); + let entry_b = flexible_reject_response(callback_id.get(), 99); + + let payload = CanisterHttpPayload { + flexible_errors: vec![FlexibleCanisterHttpError::TooManyRequestErrors { + callback_id, + reject_responses: vec![entry_a, entry_b], + }], + ..Default::default() + }; + let result = pb.validate_payload( + Height::new(1), + &test_proposal_context(&default_validation_context()), + &payload_to_bytes_max_4mb(payload), + &[], + ); + assert_matches!( + result, + Err(ValidationError::InvalidArtifact( + InvalidPayloadReason::InvalidCanisterHttpPayload( + InvalidCanisterHttpPayloadReason::FlexibleSignerNotInCommittee { callback_id: cb_id, signer: s } + ) + )) if cb_id == callback_id && s == node_test_id(99) + ); + }); +} + +#[test] +fn flexible_error_too_many_request_errors_callback_id_mismatch_in_response() { + let num_nodes = 4; + let committee: BTreeSet<_> = (0..num_nodes as u64).map(node_test_id).collect(); + let callback_id = CallbackId::from(42); + + setup_test_with_flexible_context(num_nodes, callback_id, committee, 3, 4, |pb, _pool| { + let entry_ok = flexible_reject_response(callback_id.get(), 0); + // Entry with wrong callback_id + let entry_wrong = flexible_reject_response(999, 1); + + let payload = CanisterHttpPayload { + flexible_errors: vec![FlexibleCanisterHttpError::TooManyRequestErrors { + callback_id, + reject_responses: vec![entry_ok, entry_wrong], + }], + ..Default::default() + }; + let result = pb.validate_payload( + Height::new(1), + &test_proposal_context(&default_validation_context()), + &payload_to_bytes_max_4mb(payload), + &[], + ); + assert_matches!( + result, + Err(ValidationError::InvalidArtifact( + InvalidPayloadReason::InvalidCanisterHttpPayload( + InvalidCanisterHttpPayloadReason::FlexibleCallbackIdMismatch { callback_id: cb_id, mismatched_id: mm_id } + ) + )) if cb_id == callback_id && mm_id == CallbackId::new(999) + ); + }); +} + +#[test] +fn flexible_error_too_many_request_errors_registry_version_mismatch() { + let num_nodes = 4; + let committee: BTreeSet<_> = (0..num_nodes as u64).map(node_test_id).collect(); + let callback_id = CallbackId::from(42); + + setup_test_with_flexible_context(num_nodes, callback_id, committee, 3, 4, |pb, _pool| { + let entry_ok = flexible_reject_response(callback_id.get(), 0); + let mut entry_bad = flexible_reject_response(callback_id.get(), 1); + entry_bad.proof.content.registry_version = RegistryVersion::new(999); + + let payload = CanisterHttpPayload { + flexible_errors: vec![FlexibleCanisterHttpError::TooManyRequestErrors { + callback_id, + reject_responses: vec![entry_ok, entry_bad], + }], + ..Default::default() + }; + let result = pb.validate_payload( + Height::new(1), + &test_proposal_context(&default_validation_context()), + &payload_to_bytes_max_4mb(payload), + &[], + ); + assert_matches!( + result, + Err(ValidationError::InvalidArtifact( + InvalidPayloadReason::InvalidCanisterHttpPayload( + InvalidCanisterHttpPayloadReason::RegistryVersionMismatch { expected: e, received: r } + ) + )) if e == RegistryVersion::new(1) && r == RegistryVersion::new(999) + ); + }); +} + +#[test] +fn flexible_error_too_many_request_errors_content_hash_mismatch() { + let num_nodes = 4; + let committee: BTreeSet<_> = (0..num_nodes as u64).map(node_test_id).collect(); + let callback_id = CallbackId::from(42); + + setup_test_with_flexible_context(num_nodes, callback_id, committee, 3, 4, |pb, _pool| { + let entry_ok = flexible_reject_response(callback_id.get(), 0); + let mut entry_bad = flexible_reject_response(callback_id.get(), 1); + entry_bad.proof.content.content_hash = CryptoHashOf::new(CryptoHash(vec![0xFF; 32])); + + let payload = CanisterHttpPayload { + flexible_errors: vec![FlexibleCanisterHttpError::TooManyRequestErrors { + callback_id, + reject_responses: vec![entry_ok, entry_bad], + }], + ..Default::default() + }; + let result = pb.validate_payload( + Height::new(1), + &test_proposal_context(&default_validation_context()), + &payload_to_bytes_max_4mb(payload), + &[], + ); + assert_matches!( + result, + Err(ValidationError::InvalidArtifact( + InvalidPayloadReason::InvalidCanisterHttpPayload( + InvalidCanisterHttpPayloadReason::ContentHashMismatch { metadata_hash: mh, calculated_hash: ch } + ) + )) if mh == CryptoHashOf::new(CryptoHash(vec![0xFF; 32])) && ch != mh + ); + }); +} + +#[test] +fn flexible_error_too_many_request_errors_content_size_mismatch() { + let num_nodes = 4; + let committee: BTreeSet<_> = (0..num_nodes as u64).map(node_test_id).collect(); + let callback_id = CallbackId::from(42); + + setup_test_with_flexible_context(num_nodes, callback_id, committee, 3, 4, |pb, _pool| { + let entry_ok = flexible_reject_response(callback_id.get(), 0); + let mut entry_bad = flexible_reject_response(callback_id.get(), 1); + entry_bad.proof.content.content_size = 999_999; + + let payload = CanisterHttpPayload { + flexible_errors: vec![FlexibleCanisterHttpError::TooManyRequestErrors { + callback_id, + reject_responses: vec![entry_ok, entry_bad], + }], + ..Default::default() + }; + let result = pb.validate_payload( + Height::new(1), + &test_proposal_context(&default_validation_context()), + &payload_to_bytes_max_4mb(payload), + &[], + ); + assert_matches!( + result, + Err(ValidationError::InvalidArtifact( + InvalidPayloadReason::InvalidCanisterHttpPayload( + InvalidCanisterHttpPayloadReason::ContentSizeMismatch { metadata_size: ms, calculated_size: cs } + ) + )) if ms == 999_999 && cs < ms && cs != 0 + ); + }); +} + +#[test] +fn flexible_error_too_many_request_errors_proof_id_mismatch() { + let num_nodes = 4; + let committee: BTreeSet<_> = (0..num_nodes as u64).map(node_test_id).collect(); + let callback_id = CallbackId::from(42); + + setup_test_with_flexible_context(num_nodes, callback_id, committee, 3, 4, |pb, _pool| { + let entry_ok = flexible_reject_response(callback_id.get(), 0); + let mut entry_bad = flexible_reject_response(callback_id.get(), 1); + // response.id stays correct, but proof.content.id is wrong + entry_bad.proof.content.id = CallbackId::new(999); + + let payload = CanisterHttpPayload { + flexible_errors: vec![FlexibleCanisterHttpError::TooManyRequestErrors { + callback_id, + reject_responses: vec![entry_ok, entry_bad], + }], + ..Default::default() + }; + let result = pb.validate_payload( + Height::new(1), + &test_proposal_context(&default_validation_context()), + &payload_to_bytes_max_4mb(payload), + &[], + ); + assert_matches!( + result, + Err(ValidationError::InvalidArtifact( + InvalidPayloadReason::InvalidCanisterHttpPayload( + InvalidCanisterHttpPayloadReason::FlexibleCallbackIdMismatch { + callback_id: cb_id, + mismatched_id: mm_id, + .. + } + ) + )) if cb_id == callback_id && mm_id == CallbackId::new(999) + ); + }); +} + +#[test] +fn flexible_error_too_many_request_errors_invalid_signature() { + let committee: BTreeSet<_> = (0..4).map(node_test_id).collect(); + let callback_id = CallbackId::from(42); + + setup_test_with_flexible_context(4, callback_id, committee, 3, 4, |mut pb, _pool| { + pb.crypto = Arc::new(mock_crypto_rejecting_signatures()); + + let reject_entries: Vec<_> = (0..2_u64) + .map(|node_idx| flexible_reject_response(callback_id.get(), node_idx)) + .collect(); + + let payload = CanisterHttpPayload { + flexible_errors: vec![FlexibleCanisterHttpError::TooManyRequestErrors { + callback_id, + reject_responses: reject_entries, + }], + ..Default::default() + }; + let result = pb.validate_payload( + Height::new(1), + &test_proposal_context(&default_validation_context()), + &payload_to_bytes_max_4mb(payload), + &[], + ); + assert_matches!( + result, + Err(ValidationError::InvalidArtifact( + InvalidPayloadReason::InvalidCanisterHttpPayload( + InvalidCanisterHttpPayloadReason::SignatureError(_) + ) + )) + ); + }); +} + +fn setup_test_with_contexts( + num_nodes: usize, + contexts: Vec<(CallbackId, CanisterHttpRequestContext)>, + run: impl FnOnce(CanisterHttpPayloadBuilderImpl, Arc>), +) { + test_config_with_http_feature(true, num_nodes, |mut payload_builder, pool| { + inject_request_contexts(&mut payload_builder, contexts); + run(payload_builder, pool); + }); +} + +fn setup_test_with_flexible_context( + num_nodes: usize, + callback_id: CallbackId, + committee: BTreeSet, + min_responses: u32, + max_responses: u32, + run: impl FnOnce(CanisterHttpPayloadBuilderImpl, Arc>), +) { + setup_test_with_contexts( + num_nodes, + vec![( + callback_id, + flexible_request_context(committee, min_responses, max_responses), + )], + run, + ); +} + +/// Replaces the payload_builder's state_reader with one containing the given request contexts. +pub(crate) fn inject_request_contexts( + payload_builder: &mut CanisterHttpPayloadBuilderImpl, + contexts: impl IntoIterator, +) { + let mut init_state = ic_test_utilities_state::get_initial_state(0, 0); + for (cb, ctx) in contexts { + init_state + .metadata + .subnet_call_context_manager + .canister_http_request_contexts + .insert(cb, ctx); + } + let state_manager = Arc::new(RefMockStateManager::default()); + state_manager + .get_mut() + .expect_get_state_at() + .return_const(Ok(ic_interfaces_state_manager::Labeled::new( + Height::new(0), + Arc::new(init_state), + ))); + payload_builder.state_reader = state_manager; +} + +fn fully_replicated_contexts( + ids: impl IntoIterator, +) -> Vec<(CallbackId, CanisterHttpRequestContext)> { + ids.into_iter() + .map(|id| { + ( + CallbackId::new(id), + request_context(Replication::FullyReplicated), + ) + }) + .collect() +} + +pub(crate) fn request_context(replication: Replication) -> CanisterHttpRequestContext { + CanisterHttpRequestContext { + request: RequestBuilder::default().build(), + url: "https://example.com".to_string(), + max_response_bytes: None, + headers: vec![], + body: None, + http_method: CanisterHttpMethod::GET, + transform: None, + time: UNIX_EPOCH, + replication, + pricing_version: ic_types::canister_http::PricingVersion::Legacy, + refund_status: ic_types::canister_http::RefundStatus::default(), + } +} + +fn flexible_request_context( + committee: BTreeSet, + min_responses: u32, + max_responses: u32, +) -> CanisterHttpRequestContext { + CanisterHttpRequestContext { + request: RequestBuilder::default().build(), + url: "https://example.com".to_string(), + max_response_bytes: None, + headers: vec![], + body: None, + http_method: CanisterHttpMethod::GET, + transform: None, + time: UNIX_EPOCH, + replication: Replication::Flexible { + committee, + min_responses, + max_responses, + }, + pricing_version: ic_types::canister_http::PricingVersion::PayAsYouGo, + refund_status: ic_types::canister_http::RefundStatus::default(), + } +} + +fn flexible_response( + callback_id: u64, + signer_node: u64, + content: &[u8], +) -> FlexibleCanisterHttpResponseWithProof { + let (response, metadata) = test_response_and_metadata_with_content( + callback_id, + CanisterHttpResponseContent::Success(content.to_vec()), + ); + FlexibleCanisterHttpResponseWithProof { + response, + proof: metadata_to_share(signer_node, &metadata), + } +} + +fn flexible_reject_response( + callback_id: u64, + signer_node: u64, +) -> FlexibleCanisterHttpResponseWithProof { + use ic_types::canister_http::CanisterHttpReject; + + let (response, metadata) = test_response_and_metadata_with_content( + callback_id, + CanisterHttpResponseContent::Reject(CanisterHttpReject { + reject_code: RejectCode::SysTransient, + message: "could not connect".to_string(), + }), + ); + FlexibleCanisterHttpResponseWithProof { + response, + proof: metadata_to_share(signer_node, &metadata), + } +} + +fn flexible_payload(groups: Vec) -> CanisterHttpPayload { + CanisterHttpPayload { + responses: vec![], + timeouts: vec![], + divergence_responses: vec![], + flexible_responses: groups, + flexible_errors: vec![], + } +} + +fn metadata_share_with_content_size( + callback_id: u64, + signer_node: u64, + content_size: u32, +) -> CanisterHttpResponseShare { + let metadata = CanisterHttpResponseMetadata { + id: CallbackId::new(callback_id), + content_hash: CryptoHashOf::new(CryptoHash(vec![0xAB; 32])), + content_size, + registry_version: RegistryVersion::new(1), + replica_version: ReplicaVersion::default(), + }; + metadata_to_share(signer_node, &metadata) +} + +fn add_flexible_shares_to_pool( + pool: &Arc>, + callback_id: CallbackId, + node_range: std::ops::Range, +) { + let mut pool_access = pool.write().unwrap(); + for node_idx in node_range { + let (response, metadata) = test_response_and_metadata_with_content( + callback_id.get(), + CanisterHttpResponseContent::Success(format!("resp_{node_idx}").into_bytes()), + ); + let share = metadata_to_share(node_idx, &metadata); + add_own_share_to_pool(pool_access.deref_mut(), &share, &response); + } +} + +fn build_and_validate_and_parse_payload( + payload_builder: &CanisterHttpPayloadBuilderImpl, +) -> CanisterHttpPayload { + build_and_validate_and_parse_payload_with_context( + payload_builder, + &default_validation_context(), + ) +} + +fn build_and_validate_and_parse_payload_with_context( + payload_builder: &CanisterHttpPayloadBuilderImpl, + context: &ValidationContext, +) -> CanisterHttpPayload { + let max_size = NumBytes::new(MAX_CANISTER_HTTP_PAYLOAD_SIZE as u64); + let payload = payload_builder.build_payload(Height::new(1), max_size, &[], context); + assert_matches!( + payload_builder.validate_payload( + Height::new(1), + &test_proposal_context(context), &payload, &[], ), @@ -2912,3 +4136,18 @@ fn build_and_validate_and_parse_payload( fn payload_to_bytes_max_4mb(payload: CanisterHttpPayload) -> Vec { payload_to_bytes(payload, TEST_MAX_PAYLOAD_BYTES) } + +fn mock_crypto_rejecting_signatures() -> MockCrypto { + let mut mock_crypto = MockCrypto::new(); + mock_crypto + .expect_verify_basic_sig_http() + .returning(|_, _, _, _| { + Err(ic_types::crypto::CryptoError::SignatureVerification { + algorithm: ic_types::crypto::AlgorithmId::Ed25519, + public_key_bytes: vec![], + sig_bytes: vec![], + internal_error: "mock rejection".to_string(), + }) + }); + mock_crypto +} diff --git a/rs/https_outcalls/consensus/src/payload_builder/utils.rs b/rs/https_outcalls/consensus/src/payload_builder/utils.rs index 9ad4e4debf34..33cd11b31579 100644 --- a/rs/https_outcalls/consensus/src/payload_builder/utils.rs +++ b/rs/https_outcalls/consensus/src/payload_builder/utils.rs @@ -1,7 +1,12 @@ +use CanisterHttpResponseContent::Reject; +use ic_consensus_utils::crypto::ConsensusCrypto; use ic_interfaces::canister_http::{CanisterHttpPool, InvalidCanisterHttpPayloadReason}; use ic_types::{ - CountBytes, NodeId, NumBytes, - batch::{FlexibleCanisterHttpResponseWithProof, FlexibleCanisterHttpResponses}, + CountBytes, NodeId, NumBytes, RegistryVersion, + batch::{ + FlexibleCanisterHttpError, FlexibleCanisterHttpResponseWithProof, + FlexibleCanisterHttpResponses, MAX_CANISTER_HTTP_PAYLOAD_SIZE, + }, canister_http::{ CanisterHttpResponse, CanisterHttpResponseContent, CanisterHttpResponseMetadata, CanisterHttpResponseShare, CanisterHttpResponseWithConsensus, @@ -11,7 +16,7 @@ use ic_types::{ signature::BasicSignature, }; use std::{ - collections::{BTreeMap, BTreeSet}, + collections::{BTreeMap, BTreeSet, HashSet}, mem::size_of, }; @@ -58,6 +63,106 @@ pub(crate) fn check_response_consistency( Ok(()) } +/// Validates a single [`FlexibleCanisterHttpResponseWithProof`]. +/// +/// Checks callback-id consistency, share validity (using +/// [`validate_response_share`]), content hash, and content size. +pub(crate) fn validate_flexible_response_with_proof( + response_with_proof: &FlexibleCanisterHttpResponseWithProof, + callback_id: CallbackId, + flex_committee: &BTreeSet, + seen_signers: &mut HashSet, + consensus_registry_version: RegistryVersion, + crypto: &dyn ConsensusCrypto, +) -> Result<(), InvalidCanisterHttpPayloadReason> { + if response_with_proof.response.id != callback_id { + return Err( + InvalidCanisterHttpPayloadReason::FlexibleCallbackIdMismatch { + callback_id, + mismatched_id: response_with_proof.response.id, + }, + ); + } + + validate_response_share( + &response_with_proof.proof, + callback_id, + flex_committee, + seen_signers, + consensus_registry_version, + crypto, + )?; + + let calculated_hash = crypto_hash(&response_with_proof.response); + if calculated_hash != response_with_proof.proof.content.content_hash { + return Err(InvalidCanisterHttpPayloadReason::ContentHashMismatch { + metadata_hash: response_with_proof.proof.content.content_hash.clone(), + calculated_hash, + }); + } + + let calculated_size = response_with_proof.response.content.count_bytes() as u32; + if calculated_size != response_with_proof.proof.content.content_size { + return Err(InvalidCanisterHttpPayloadReason::ContentSizeMismatch { + metadata_size: response_with_proof.proof.content.content_size, + calculated_size, + }); + } + + Ok(()) +} + +/// Validates a single [`CanisterHttpResponseShare`] (metadata + signature). +/// +/// Checks callback-id consistency, duplicate signers, committee membership, +/// registry version, and performs signature verification. +pub(crate) fn validate_response_share( + share: &CanisterHttpResponseShare, + callback_id: CallbackId, + flex_committee: &BTreeSet, + seen_signers: &mut HashSet, + consensus_registry_version: RegistryVersion, + crypto: &dyn ConsensusCrypto, +) -> Result<(), InvalidCanisterHttpPayloadReason> { + if share.content.id != callback_id { + return Err( + InvalidCanisterHttpPayloadReason::FlexibleCallbackIdMismatch { + callback_id, + mismatched_id: share.content.id, + }, + ); + } + + let signer = share.signature.signer; + if !seen_signers.insert(signer) { + return Err(InvalidCanisterHttpPayloadReason::FlexibleDuplicateSigner { + callback_id, + signer, + }); + } + if !flex_committee.contains(&signer) { + return Err( + InvalidCanisterHttpPayloadReason::FlexibleSignerNotInCommittee { + callback_id, + signer, + }, + ); + } + + if share.content.registry_version != consensus_registry_version { + return Err(InvalidCanisterHttpPayloadReason::RegistryVersionMismatch { + expected: consensus_registry_version, + received: share.content.registry_version, + }); + } + + crypto + .verify(share, consensus_registry_version) + .map_err(|err| InvalidCanisterHttpPayloadReason::SignatureError(Box::new(err)))?; + + Ok(()) +} + /// This function takes a mapping of response metadata to supporting shares /// and determines, whether the divergence criterium is met. /// @@ -200,14 +305,32 @@ pub(crate) fn estimate_response_with_consensus_size( + content.count_bytes() } -/// Collects distinct HTTP outcall OK-responses from flexible committee members. +/// Result of scanning flexible HTTP outcall shares for a single callback. +pub(crate) enum FlexibleFindResult { + /// Collected enough OK responses for consensus. + OkResponses(FlexibleCanisterHttpResponses, usize), + /// Detected an error condition (too many rejects or responses too large). + Error(FlexibleCanisterHttpError, usize), + /// Not enough data to decide yet; more shares may arrive. + Pending, +} + +/// Scans grouped shares for a flexible HTTP outcall and determines the result. /// -/// Gathers up to `max_responses` individually-signed `(ok-response, share)` pairs -/// from unique committee members while disregarding rejects, and skipping any -/// that would exceed `max_payload_size`. -/// Returns the group and its accumulated byte size if at least `min_responses` -/// were collected. -pub(crate) fn find_flexible_responses( +/// Iterates shares sorted by `content_size` ascending (preferring smaller +/// responses), collecting OK responses from distinct committee members. +/// +/// If enough OK responses are gathered, returns [`FlexibleFindResult::OkResponses`]. +/// +/// Otherwise checks for error conditions: +/// - **TooManyRequestErrors**: more nodes returned rejects than the slack +/// allows (`committee.len() - min_responses`). +/// - **ResponsesTooLarge**: even the smallest `min_responses` many OK responses +/// (approximated by `count_bytes()`) exceed [`MAX_CANISTER_HTTP_PAYLOAD_SIZE`]. +/// - **Pending**: not enough data to decide yet. +/// +/// The cloning of the share is only done when building the [`FlexibleCanisterHttpResponses`] result. +pub(crate) fn find_flexible_result( callback_id: CallbackId, grouped_shares: &BTreeMap>, committee: &BTreeSet, @@ -216,57 +339,111 @@ pub(crate) fn find_flexible_responses( accumulated_size: usize, max_payload_size: NumBytes, pool_access: &dyn CanisterHttpPool, -) -> Option<(FlexibleCanisterHttpResponses, usize)> { - let mut flexible_responses = Vec::new(); - let mut flexible_responses_size = size_of::(); - let mut signers = BTreeSet::new(); - - 'outer: for (metadata, shares) in grouped_shares { - for share in shares { - if flexible_responses.len() >= max_responses as usize { +) -> FlexibleFindResult { + let mut entries_sorted_asc: Vec<_> = grouped_shares.iter().collect(); + entries_sorted_asc.sort_unstable_by_key(|(metadata, _)| metadata.content_size); + + let min_responses = min_responses as usize; + let mut ok_responses: Vec<(CanisterHttpResponse, &CanisterHttpResponseShare)> = Vec::new(); + let mut ok_responses_size = size_of::(); + // Tracks all signers processed (both OK and reject) + let mut seen_signers = BTreeSet::new(); + let mut reject_responses: Vec<(CanisterHttpResponse, &CanisterHttpResponseShare)> = Vec::new(); + let mut all_ok_shares_sorted_asc: Vec<(&CanisterHttpResponseShare, usize)> = Vec::new(); + + 'outer: for (metadata, shares) in entries_sorted_asc { + for &share in shares { + if ok_responses.len() >= max_responses as usize { break 'outer; } - if !committee.contains(&share.signature.signer) - || !signers.insert(share.signature.signer) - { + let signer = share.signature.signer; + if !committee.contains(&signer) || !seen_signers.insert(signer) { + continue; + } + let Some(response) = pool_access.get_response_content_by_hash(&metadata.content_hash) + else { + continue; + }; + + if matches!(response.content, Reject(_)) { + reject_responses.push((response, share)); continue; } - if let Some(http_response) = - pool_access.get_response_content_by_hash(&metadata.content_hash) - { - if matches!( - http_response.content, - CanisterHttpResponseContent::Reject(_) - ) { - // Disregard rejects, as we are collecting ok-responses. - continue; - } - let response = FlexibleCanisterHttpResponseWithProof { - response: http_response, - proof: (*share).clone(), - }; - let response_size = response.count_bytes(); - let new_total = NumBytes::new( - (accumulated_size + flexible_responses_size + response_size) as u64, - ); - if new_total >= max_payload_size { - continue; - } - flexible_responses_size += response_size; - flexible_responses.push(response); + + let response_with_proof_size = + FlexibleCanisterHttpResponseWithProof::count_bytes(&response, share); + all_ok_shares_sorted_asc.push((share, response_with_proof_size)); + + let new_total = NumBytes::new( + (accumulated_size + ok_responses_size + response_with_proof_size) as u64, + ); + if new_total >= max_payload_size { + // We `continue` rather than `break` here, to further populate + // the Vec later used to detect ResponsesTooLarge errors. + continue; } + ok_responses_size += response_with_proof_size; + ok_responses.push((response, share)); } } - if flexible_responses.len() >= min_responses as usize { - Some(( + // 1. Enough OK responses collected? + if ok_responses.len() >= min_responses { + return FlexibleFindResult::OkResponses( FlexibleCanisterHttpResponses { callback_id, - responses: flexible_responses, + responses: ok_responses + .into_iter() + .map(|(response, share)| FlexibleCanisterHttpResponseWithProof { + response, + proof: share.clone(), + }) + .collect(), }, - flexible_responses_size, - )) - } else { - None + ok_responses_size, + ); + } + + // 2. Too many nodes returned rejects (so that we can never reach min_responses OK responses)? + if reject_responses.len() > committee.len().saturating_sub(min_responses) { + let error = FlexibleCanisterHttpError::TooManyRequestErrors { + callback_id, + reject_responses: reject_responses + .into_iter() + .map(|(response, share)| FlexibleCanisterHttpResponseWithProof { + response, + proof: share.clone(), + }) + .collect(), + }; + let error_size = error.count_bytes(); + return FlexibleFindResult::Error(error, error_size); } + + // 3. Even the smallest OK responses exceed the absolute payload limit? + if all_ok_shares_sorted_asc.len() >= min_responses { + let num_unseen = committee.len().saturating_sub(seen_signers.len()); + // Unseen responses could still submit small OK responses, so we account for them. + let min_minus_unseen = min_responses.saturating_sub(num_unseen); + + let smallest_content_sum: usize = all_ok_shares_sorted_asc[..min_minus_unseen] + .iter() + .map(|(_share, response_with_proof_size)| response_with_proof_size) + .sum(); + + if smallest_content_sum > MAX_CANISTER_HTTP_PAYLOAD_SIZE { + let error = FlexibleCanisterHttpError::ResponsesTooLarge { + callback_id, + metadata_shares: all_ok_shares_sorted_asc[..min_responses] + .iter() + .map(|(share, _size)| (*share).clone()) + .collect(), + }; + let error_size = error.count_bytes(); + return FlexibleFindResult::Error(error, error_size); + } + } + + // 4. Not enough data yet + FlexibleFindResult::Pending } diff --git a/rs/interfaces/src/canister_http.rs b/rs/interfaces/src/canister_http.rs index 8ea7673f2e69..9220302aefda 100644 --- a/rs/interfaces/src/canister_http.rs +++ b/rs/interfaces/src/canister_http.rs @@ -98,6 +98,22 @@ pub enum InvalidCanisterHttpPayloadReason { /// For example, a non-flexible response is not in the responses section /// or a flexible response is not in the flexible_responses section. InvalidPayloadSection(CallbackId), + /// A TooManyRequestErrors error does not carry enough rejects. + FlexibleInsufficientRejectCount { + callback_id: CallbackId, + reject_count: usize, + min_needed: usize, + }, + /// A TooManyRequestErrors entry contains a non-Reject response. + FlexibleRejectExpectedInErrorResponse(CallbackId), + /// A ResponsesTooLarge error does not carry enough metadata shares. + FlexibleInsufficientMetadataShareCount { + callback_id: CallbackId, + share_count: usize, + min_needed: usize, + }, + /// A ResponsesTooLarge error is invalid: the smallest responses actually fit. + FlexibleResponsesNotTooLarge(CallbackId), /// The payload could not be deserialized DecodeError(ProxyDecodeError), } diff --git a/rs/types/types/src/batch/canister_http.rs b/rs/types/types/src/batch/canister_http.rs index 95292d9ae7e6..c873662891ea 100644 --- a/rs/types/types/src/batch/canister_http.rs +++ b/rs/types/types/src/batch/canister_http.rs @@ -1,5 +1,5 @@ use crate::{ - CountBytes, ReplicaVersion, + CanisterId, CountBytes, ReplicaVersion, canister_http::{ CanisterHttpReject, CanisterHttpRequestId, CanisterHttpResponse, CanisterHttpResponseArtifact, CanisterHttpResponseContent, CanisterHttpResponseDivergence, @@ -83,10 +83,57 @@ pub struct FlexibleCanisterHttpResponseWithProof { pub proof: CanisterHttpResponseShare, } +impl FlexibleCanisterHttpResponseWithProof { + pub fn count_bytes( + response: &CanisterHttpResponse, + proof: &CanisterHttpResponseShare, + ) -> usize { + Self::count_bytes_from_parts(&response.canister_id, response.content.count_bytes(), proof) + } + + /// Same calculation as [`Self::count_bytes`] but from decomposed parts. + pub fn count_bytes_from_parts( + canister_id: &CanisterId, + content_size: usize, + proof: &CanisterHttpResponseShare, + ) -> usize { + let response_size = CanisterHttpResponse::count_bytes_from_parts(canister_id, content_size); + response_size + proof.count_bytes() + } +} + impl CountBytes for FlexibleCanisterHttpResponseWithProof { fn count_bytes(&self) -> usize { let Self { response, proof } = self; - response.count_bytes() + proof.count_bytes() + Self::count_bytes(response, proof) + } +} + +impl CountBytes for FlexibleCanisterHttpError { + fn count_bytes(&self) -> usize { + match self { + Self::Timeout { callback_id } => callback_id.count_bytes(), + Self::ResponsesTooLarge { + callback_id, + metadata_shares, + } => { + callback_id.count_bytes() + + metadata_shares + .iter() + .map(|s| s.count_bytes()) + .sum::() + } + Self::TooManyRequestErrors { + callback_id, + reject_responses, + } => { + callback_id.count_bytes() + + reject_responses + .iter() + .map(|r| r.count_bytes()) + .sum::() + } + } } } diff --git a/rs/types/types/src/canister_http.rs b/rs/types/types/src/canister_http.rs index f857190d27e4..3e244aa04ff1 100644 --- a/rs/types/types/src/canister_http.rs +++ b/rs/types/types/src/canister_http.rs @@ -803,14 +803,21 @@ pub struct CanisterHttpResponse { pub content: CanisterHttpResponseContent, } +impl CanisterHttpResponse { + /// Same calculation as `Self::count_bytes` but from decomposed parts. + pub fn count_bytes_from_parts(canister_id: &CanisterId, content_size: usize) -> usize { + size_of::() + canister_id.get_ref().data_size() + content_size + } +} + impl CountBytes for CanisterHttpResponse { fn count_bytes(&self) -> usize { let CanisterHttpResponse { - id, + id: _, canister_id, content, } = &self; - size_of_val(id) + canister_id.get_ref().data_size() + content.count_bytes() + Self::count_bytes_from_parts(canister_id, content.count_bytes()) } } @@ -986,7 +993,18 @@ pub struct CanisterHttpResponseMetadata { impl CountBytes for CanisterHttpResponseMetadata { fn count_bytes(&self) -> usize { - size_of::() + let Self { + id, + content_hash, + content_size, + registry_version, + replica_version, + } = self; + size_of_val(id) + + content_hash.get_ref().0.len() + + size_of_val(content_size) + + size_of_val(registry_version) + + replica_version.as_ref().len() } }