diff --git a/Cargo.lock b/Cargo.lock index 7b6976262a35..314a0a00c842 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -10261,6 +10261,7 @@ dependencies = [ "ic-test-utilities-time", "ic-test-utilities-types", "ic-types", + "ic-types-cycles", "ic-utils 0.9.0", "mockall", "prometheus", diff --git a/rs/artifact_pool/src/canister_http_pool.rs b/rs/artifact_pool/src/canister_http_pool.rs index 1cb9f1c2b755..a14fc2b0a476 100644 --- a/rs/artifact_pool/src/canister_http_pool.rs +++ b/rs/artifact_pool/src/canister_http_pool.rs @@ -234,7 +234,10 @@ mod tests { use ic_types::{ CanisterId, RegistryVersion, ReplicaVersion, artifact::IdentifiableArtifact, - canister_http::{CanisterHttpResponseContent, CanisterHttpResponseMetadata}, + canister_http::{ + CanisterHttpPaymentReceipt, CanisterHttpResponseContent, CanisterHttpResponseMetadata, + CanisterHttpResponseReceiptShare, + }, crypto::{CryptoHash, Signed}, messages::CallbackId, signature::BasicSignature, @@ -259,13 +262,16 @@ mod tests { fn fake_share(id: u64) -> CanisterHttpResponseShare { Signed { - content: CanisterHttpResponseMetadata { - id: CallbackId::from(id), - content_hash: CryptoHashOf::from(CryptoHash(vec![1, 2, 3])), - content_size: 42, - is_reject: false, - registry_version: RegistryVersion::from(id), - replica_version: ReplicaVersion::default(), + content: CanisterHttpResponseReceiptShare { + metadata: CanisterHttpResponseMetadata { + id: CallbackId::from(id), + content_hash: CryptoHashOf::from(CryptoHash(vec![1, 2, 3])), + content_size: 42, + is_reject: false, + registry_version: RegistryVersion::from(id), + replica_version: ReplicaVersion::default(), + }, + payment_receipt: CanisterHttpPaymentReceipt::default(), }, signature: BasicSignature::fake(node_test_id(id)), } diff --git a/rs/consensus/utils/src/crypto.rs b/rs/consensus/utils/src/crypto.rs index d2c6f38fa17e..be91ca43d3f4 100644 --- a/rs/consensus/utils/src/crypto.rs +++ b/rs/consensus/utils/src/crypto.rs @@ -1,7 +1,7 @@ use ic_interfaces::{crypto::*, validation::ValidationResult}; use ic_types::{ NodeId, RegistryVersion, - canister_http::CanisterHttpResponseMetadata, + canister_http::CanisterHttpResponseReceiptShare, consensus::{ BlockMetadata, CatchUpContent, FinalizationContent, NotarizationContent, RandomBeaconContent, RandomTapeContent, dkg, @@ -430,12 +430,8 @@ pub trait ConsensusCrypto: + SignVerify, NiDkgId> + SignVerify, RegistryVersion> + SignVerify< - CanisterHttpResponseMetadata, - BasicSignature, - RegistryVersion, - > + SignVerify< - CanisterHttpResponseMetadata, - BasicSignature, + CanisterHttpResponseReceiptShare, + BasicSignature, RegistryVersion, > + Aggregate< NotarizationContent, @@ -468,10 +464,10 @@ pub trait ConsensusCrypto: NiDkgId, ThresholdSignature, > + Aggregate< - CanisterHttpResponseMetadata, - BasicSignature, + CanisterHttpResponseReceiptShare, + BasicSignature, RegistryVersion, - BasicSignatureBatch, + BasicSignatureBatch, > + Crypto + Send + Sync diff --git a/rs/https_outcalls/client/src/client.rs b/rs/https_outcalls/client/src/client.rs index df3149679bf0..d9c8e74ed2b8 100644 --- a/rs/https_outcalls/client/src/client.rs +++ b/rs/https_outcalls/client/src/client.rs @@ -158,7 +158,7 @@ impl NonBlockingChannel for CanisterHttpAdapterClientImpl { "Canister HTTP request with PayAsYouGo pricing is not supported yet: \ request_id {}, sender {}, process_id: {}", request_id, - request_sender, + request_context.request.sender, std::process::id(), ); let _ = permit.send(( @@ -210,7 +210,7 @@ impl NonBlockingChannel for CanisterHttpAdapterClientImpl { &mut *budget, query_handler, canister_http_payload, - request_sender, + request_context.request.sender, transform, ) .await; diff --git a/rs/https_outcalls/consensus/BUILD.bazel b/rs/https_outcalls/consensus/BUILD.bazel index 2d9b2f0b9fd9..0b19f0062c85 100644 --- a/rs/https_outcalls/consensus/BUILD.bazel +++ b/rs/https_outcalls/consensus/BUILD.bazel @@ -73,6 +73,7 @@ rust_test( "//rs/test_utilities/state", "//rs/test_utilities/time", "//rs/test_utilities/types", + "//rs/types/cycles", "//rs/types/management_canister_types", "//rs/types/types", "//rs/utils", diff --git a/rs/https_outcalls/consensus/Cargo.toml b/rs/https_outcalls/consensus/Cargo.toml index 20b031f3535e..135ce16fd0c0 100644 --- a/rs/https_outcalls/consensus/Cargo.toml +++ b/rs/https_outcalls/consensus/Cargo.toml @@ -40,6 +40,7 @@ ic-test-utilities-registry = { path = "../../test_utilities/registry" } ic-test-utilities-state = { path = "../../test_utilities/state" } ic-test-utilities-time = { path = "../../test_utilities/time" } ic-test-utilities-types = { path = "../../test_utilities/types" } +ic-types-cycles = { path = "../../types/cycles" } assert_matches = { workspace = true } mockall = { workspace = true } proptest = { workspace = true } diff --git a/rs/https_outcalls/consensus/src/gossip.rs b/rs/https_outcalls/consensus/src/gossip.rs index 45deb32d11f6..818fd4b61ae3 100644 --- a/rs/https_outcalls/consensus/src/gossip.rs +++ b/rs/https_outcalls/consensus/src/gossip.rs @@ -61,12 +61,12 @@ impl BouncerFactory }; let log = self.log.clone(); Box::new(move |id: &'_ CanisterHttpResponseId| { - if id.content.registry_version != registry_version { + if id.content.registry_version() != registry_version { warn!( log, "Dropping canister http response share with callback id: {}, because registry version {} does not match expected version {}", - id.content.id, - id.content.registry_version, + id.content.id(), + id.content.registry_version(), registry_version ); return BouncerValue::Unwanted; @@ -83,12 +83,12 @@ impl BouncerFactory // not higher that `MAX_NUMBER_OF_REQUESTS_AHEAD`. // Receiving an callback Id higher is possible because the priority fn is updated periodically (every 3s) with the latest state // and can therefore store stale `known_request_ids` and stale `next_callback_id`. - if known_request_ids.contains(&id.content.id) - || (id.content.id >= next_callback_id - && id.content.id <= highest_accepted_request_id) + if known_request_ids.contains(&id.content.id()) + || (id.content.id() >= next_callback_id + && id.content.id() <= highest_accepted_request_id) { BouncerValue::Wants - } else if id.content.id > highest_accepted_request_id { + } else if id.content.id() > highest_accepted_request_id { BouncerValue::MaybeWantsLater } else { BouncerValue::Unwanted diff --git a/rs/https_outcalls/consensus/src/payload_builder.rs b/rs/https_outcalls/consensus/src/payload_builder.rs index 96b3e8f45d90..3719dec8dd2c 100644 --- a/rs/https_outcalls/consensus/src/payload_builder.rs +++ b/rs/https_outcalls/consensus/src/payload_builder.rs @@ -5,10 +5,10 @@ use crate::{ payload_builder::{ parse::bytes_to_payload, utils::{ - 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, + FlexibleFindResult, 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, }, }, }; @@ -39,7 +39,7 @@ use ic_metrics::MetricsRegistry; use ic_registry_client_helpers::subnet::SubnetRegistry; use ic_replicated_state::ReplicatedState; use ic_types::{ - CountBytes, Height, NodeId, NumBytes, RegistryVersion, SubnetId, + CountBytes, Height, NodeId, NumBytes, SubnetId, batch::{ CanisterHttpPayload, ConsensusResponse, FlexibleCanisterHttpError, FlexibleCanisterHttpResponseWithProof, FlexibleCanisterHttpResponses, @@ -47,17 +47,14 @@ use ic_types::{ }, canister_http::{ CANISTER_HTTP_MAX_RESPONSES_PER_BLOCK, CANISTER_HTTP_TIMEOUT_INTERVAL, - CanisterHttpResponse, CanisterHttpResponseContent, CanisterHttpResponseDivergence, - CanisterHttpResponseMetadata, CanisterHttpResponseWithConsensus, Replication, + CanisterHttpResponseContent, CanisterHttpResponseDivergence, Replication, }, consensus::Committee, - crypto::Signed, messages::{CallbackId, Payload, RejectContext}, registry::RegistryClientError, - signature::BasicSignature, }; use std::{ - collections::{BTreeMap, BTreeSet, HashSet}, + collections::{BTreeMap, HashSet}, sync::{Arc, RwLock}, }; @@ -134,35 +131,6 @@ impl CanisterHttpPayloadBuilderImpl { .map(|features| features.unwrap_or_default().http_requests) } - /// Aggregates the signature and creates the [`CanisterHttpResponseWithConsensus`] message. - fn aggregate( - &self, - registry_version: RegistryVersion, - metadata: CanisterHttpResponseMetadata, - shares: BTreeSet>, - content: CanisterHttpResponse, - ) -> Option { - match self - .crypto - .aggregate(shares.iter().collect(), registry_version) - { - Err(err) => { - warn!( - self.log, - "Failed to aggregate signature for CanisterHttpResponse: {:?}", err - ); - None - } - Ok(signature) => Some(CanisterHttpResponseWithConsensus { - content, - proof: Signed { - content: metadata, - signature, - }, - }), - } - } - fn get_canister_http_payload_impl( &self, height: Height, @@ -219,7 +187,7 @@ impl CanisterHttpPayloadBuilderImpl { let mut accumulated_size = 0; let mut responses_included = 0; - let mut candidates = vec![]; + let mut responses = vec![]; let mut timeouts = vec![]; let mut divergence_responses = vec![]; let mut flexible_responses = vec![]; @@ -229,9 +197,7 @@ impl CanisterHttpPayloadBuilderImpl { let mut total_share_count = 0; let mut active_shares = 0; - // Since aggregating signatures is potentially expensive (currently for - // BasicSignatures it is not expensive), we pick the candidates first - // (under the pool lock), then aggregate in a separate step. + // Pick the candidates under the pool lock. { let pool_access = self.pool.read().unwrap(); @@ -242,14 +208,14 @@ impl CanisterHttpPayloadBuilderImpl { total_share_count += 1; }) // Filter out shares with the wrong registry version - .filter(|&share| share.content.registry_version == consensus_registry_version) + .filter(|&share| share.content.registry_version() == consensus_registry_version) .inspect(|_| { active_shares += 1; }) // Filter out shares for responses to requests that already have // responses in the block chain up to the point we are creating a // new payload. - .filter(|&response| !delivered_ids.contains(&response.content.id)); + .filter(|&share| !delivered_ids.contains(&share.content.id())); // Group the shares by their metadata let shares_by_callback_id = group_shares_by_callback_id(share_candidates); @@ -297,14 +263,13 @@ impl CanisterHttpPayloadBuilderImpl { }; match &request.replication { Replication::FullyReplicated => { - if let Some((metadata, shares, content)) = + if let Some(response) = find_fully_replicated_response(grouped_shares, threshold, &*pool_access) { - let candidate_size = - estimate_response_with_consensus_size(&metadata, &shares, &content); + let candidate_size = response.count_bytes(); let size = NumBytes::new((accumulated_size + candidate_size) as u64); if size < max_payload_size { - candidates.push((metadata, shares, content)); + responses.push(response); responses_included += 1; accumulated_size += candidate_size; } @@ -329,16 +294,16 @@ impl CanisterHttpPayloadBuilderImpl { } } Replication::NonReplicated(designated_node_id) => { - if let Some((metadata, shares, content)) = find_non_replicated_response( + if let Some(response) = find_non_replicated_response( grouped_shares, designated_node_id, &*pool_access, ) { - let candidate_size = - estimate_response_with_consensus_size(&metadata, &shares, &content); - let size = NumBytes::new((accumulated_size + candidate_size) as u64); + let candidate_size = response.count_bytes(); + let size = + NumBytes::new((accumulated_size + response.count_bytes()) as u64); if size < max_payload_size { - candidates.push((metadata, shares, content)); + responses.push(response); responses_included += 1; accumulated_size += candidate_size; } @@ -380,12 +345,7 @@ impl CanisterHttpPayloadBuilderImpl { } CanisterHttpPayload { - responses: candidates - .into_iter() - .filter_map(|(metadata, shares, content)| { - self.aggregate(consensus_registry_version, metadata, shares, content) - }) - .collect(), + responses, timeouts, divergence_responses, flexible_responses, @@ -473,11 +433,11 @@ impl CanisterHttpPayloadBuilderImpl { .map_err(CanisterHttpPayloadValidationError::InvalidArtifact)?; // Validate response against consensus registry version - if response.proof.content.registry_version != consensus_registry_version { + if response.proof.registry_version() != consensus_registry_version { return invalid_artifact( InvalidCanisterHttpPayloadReason::RegistryVersionMismatch { expected: consensus_registry_version, - received: response.proof.content.registry_version, + received: response.proof.registry_version(), }, ); } @@ -536,8 +496,7 @@ impl CanisterHttpPayloadBuilderImpl { let (valid_signers, invalid_signers): (Vec, Vec) = response .proof - .signature - .signatures_map + .signatures .keys() .cloned() .partition(|signer| effective_committee.iter().any(|id| id == signer)); @@ -557,13 +516,34 @@ impl CanisterHttpPayloadBuilderImpl { }); } - self.crypto - .verify_aggregate(&response.proof, consensus_registry_version) - .map_err(|err| { - CanisterHttpPayloadValidationError::InvalidArtifact( - InvalidCanisterHttpPayloadReason::SignatureError(Box::new(err)), - ) - })?; + // Each per-signer entry of the proof carries that signer's + // payment receipt alongside their signature. Each signature is + // verified individually against the shared metadata combined + // with that signer's receipt. + utils::verify_aggregate_proof( + &response.proof, + consensus_registry_version, + self.crypto.as_ref(), + ) + .map_err(CanisterHttpPayloadValidationError::InvalidArtifact)?; + + // Additionally enforce the per-replica refund allowance from + // the request context: no signer may claim a refund larger + // than the allowance set in `refund_status`. + for sig in response.proof.signatures.values() { + if sig.payment_receipt.refund > request_context.refund_status.per_replica_allowance + { + return invalid_artifact(InvalidCanisterHttpPayloadReason::SignatureError( + Box::new(ic_types::crypto::CryptoError::MalformedSignature { + algorithm: ic_types::crypto::AlgorithmId::Ed25519, + sig_bytes: vec![], + internal_error: + "Refund in payment receipt is greater than per-replica allowance" + .to_string(), + }), + )); + } + } } let faults_tolerated = match self.membership.get_canister_http_committee(height) { @@ -597,6 +577,24 @@ impl CanisterHttpPayloadBuilderImpl { InvalidCanisterHttpPayloadReason::SignatureError(Box::new(err)), ) })?; + // Enforce per-replica refund allowance for divergence + // shares. While a divergence proof does not deliver a + // refund directly, malformed refund claims should still + // be rejected to avoid leaking budget through the + // divergence codepath in any future extensions. + if let Some(ctx) = http_contexts.get(&share.content.id()) + && share.content.refund() > ctx.refund_status.per_replica_allowance + { + return invalid_artifact(InvalidCanisterHttpPayloadReason::SignatureError( + Box::new(ic_types::crypto::CryptoError::MalformedSignature { + algorithm: ic_types::crypto::AlgorithmId::Ed25519, + sig_bytes: vec![], + internal_error: + "Refund in divergence share exceeds per-replica allowance" + .to_string(), + }), + )); + } } let grouped_shares = group_shares_by_callback_id(response.shares.iter()); @@ -690,6 +688,21 @@ impl CanisterHttpPayloadBuilderImpl { }, ); } + + // Enforce the per-replica refund allowance: the share's + // refund claim must be at most the per-replica allowance. + if response_with_proof.proof.content.refund() + > context.refund_status.per_replica_allowance + { + return invalid_artifact(InvalidCanisterHttpPayloadReason::SignatureError( + Box::new(ic_types::crypto::CryptoError::MalformedSignature { + algorithm: ic_types::crypto::AlgorithmId::Ed25519, + sig_bytes: vec![], + internal_error: + "Refund in flexible share exceeds per-replica allowance".to_string(), + }), + )); + } } } @@ -811,11 +824,11 @@ impl CanisterHttpPayloadBuilderImpl { let mut ok_entry_sizes: Vec = all_seen_shares .iter() - .filter(|share| !share.content.is_reject) + .filter(|share| !share.content.is_reject()) .map(|share| { FlexibleCanisterHttpResponseWithProof::count_bytes_from_parts( &context.request.sender, - share.content.content_size as usize, + share.content.content_size() as usize, share, ) }) @@ -937,7 +950,7 @@ impl IntoMessages<(Vec, CanisterHttpBatchStats)> .expect("Failed to parse a payload that was already validated"); let responses = messages.responses.into_iter().map(|response| { - if response.proof.signature.signatures_map.len() == 1 { + if response.proof.signatures.len() == 1 { stats.single_signature_responses += 1; } stats.responses += 1; @@ -1093,7 +1106,7 @@ fn flexible_error_into_consensus_response( } => { let num_ok = all_seen_shares .iter() - .filter(|s| !s.content.is_reject) + .filter(|s| !s.content.is_reject()) .count() as u32; let num_reject = all_seen_shares.len() as u32 - num_ok; let num_unseen = total_requests.saturating_sub(all_seen_shares.len() as u32); @@ -1102,7 +1115,7 @@ fn flexible_error_into_consensus_response( let node_details: Vec<_> = all_seen_shares .iter() .map(|share| { - let code = if share.content.is_reject { + let code = if share.content.is_reject() { "reject" } else { "ok" @@ -1112,7 +1125,7 @@ fn flexible_error_into_consensus_response( report: HttpRequestResourceReport::default(), error: Some(FlexibleHttpNodeError { code: code.to_string(), - message: format!("{} bytes", share.content.content_size), + message: format!("{} bytes", share.content.content_size()), }), } }) @@ -1120,8 +1133,8 @@ fn flexible_error_into_consensus_response( let mut ok_sizes: Vec<_> = all_seen_shares .iter() - .filter(|s| !s.content.is_reject) - .map(|share| share.content.content_size) + .filter(|s| !s.content.is_reject()) + .map(|share| share.content.content_size()) .collect(); // Sort defensively, as validator doesn't enforce ordering on `all_seen_shares` ok_sizes.sort_unstable(); @@ -1166,7 +1179,7 @@ fn flexible_error_into_consensus_response( fn divergence_response_into_reject( response: CanisterHttpResponseDivergence, ) -> Option { - let Some(id) = response.shares.first().map(|share| share.content.id) else { + let Some(id) = response.shares.first().map(|share| share.content.id()) else { // NOTE: We skip delivering the divergence response, if it has no shares // Such a divergence response should never validate, therefore this should never happen // However, if it where ever to happen, we can ignore it here. @@ -1179,7 +1192,7 @@ fn divergence_response_into_reject( response .shares .into_iter() - .map(|share| share.content.content_hash.get().0) + .map(|share| share.content.metadata.content_hash.get().0) .for_each(|hash| { hash_counts .entry(hash) diff --git a/rs/https_outcalls/consensus/src/payload_builder/tests.rs b/rs/https_outcalls/consensus/src/payload_builder/tests.rs index f6307203435e..9c2144e6c5ce 100644 --- a/rs/https_outcalls/consensus/src/payload_builder/tests.rs +++ b/rs/https_outcalls/consensus/src/payload_builder/tests.rs @@ -45,16 +45,17 @@ use ic_types::{ }, canister_http::{ CANISTER_HTTP_MAX_RESPONSES_PER_BLOCK, CANISTER_HTTP_TIMEOUT_INTERVAL, CanisterHttpMethod, - CanisterHttpReject, CanisterHttpRequestContext, CanisterHttpResponse, - CanisterHttpResponseArtifact, CanisterHttpResponseContent, CanisterHttpResponseDivergence, - CanisterHttpResponseMetadata, CanisterHttpResponseShare, CanisterHttpResponseWithConsensus, - Replication, + CanisterHttpPaymentReceipt, CanisterHttpReject, CanisterHttpRequestContext, + CanisterHttpResponse, CanisterHttpResponseArtifact, CanisterHttpResponseContent, + CanisterHttpResponseDivergence, CanisterHttpResponseMetadata, CanisterHttpResponseProof, + CanisterHttpResponseReceiptShare, CanisterHttpResponseShare, CanisterHttpResponseSignature, + CanisterHttpResponseWithConsensus, Replication, }, consensus::get_faults_tolerated, crypto::{BasicSig, BasicSigOf, CryptoHash, CryptoHashOf, Signed, crypto_hash}, messages::{CallbackId, Payload, RejectContext}, registry::RegistryClientError, - signature::{BasicSignature, BasicSignatureBatch}, + signature::BasicSignature, time::UNIX_EPOCH, }; use rand::Rng; @@ -202,11 +203,9 @@ fn multiple_payload_test() { let past_payload = CanisterHttpPayload { responses: vec![CanisterHttpResponseWithConsensus { content: past_response, - proof: Signed { - content: past_metadata, - signature: BasicSignatureBatch { - signatures_map: BTreeMap::new(), - }, + proof: CanisterHttpResponseProof { + metadata: past_metadata, + signatures: BTreeMap::new(), }, }], timeouts: vec![], @@ -782,7 +781,10 @@ fn divergence_error_message() { sample.content_hash = CryptoHashOf::from(CryptoHash(new_hash.to_vec())); Signed { - content: sample, + content: CanisterHttpResponseReceiptShare { + metadata: sample, + payment_receipt: CanisterHttpPaymentReceipt::default(), + }, signature: BasicSignature { signature: BasicSigOf::new(BasicSig(vec![])), signer: node_test_id(node_id), @@ -877,15 +879,12 @@ fn non_replicated_request_response_coming_in_gossip_payload_created() { // The response must contain one signature. let proof = &parsed_payload.responses[0].proof; assert_eq!( - proof.signature.signatures_map.len(), + proof.signatures.len(), 1, "Proof should contain exactly one signature" ); assert!( - proof - .signature - .signatures_map - .contains_key(&delegated_node_id), + proof.signatures.contains_key(&delegated_node_id), "The single signature must be from the delegated node" ); }); @@ -950,15 +949,12 @@ fn non_replicated_request_with_extra_share_includes_only_delegated_share() { // The response must contain EXACTLY ONE signature, proving the "extra" share was ignored. let proof = &parsed_payload.responses[0].proof; assert_eq!( - proof.signature.signatures_map.len(), + proof.signatures.len(), 1, "Proof should contain exactly one signature" ); assert!( - proof - .signature - .signatures_map - .contains_key(&delegated_node_id), + proof.signatures.contains_key(&delegated_node_id), "The single signature must be from the delegated node" ); }); @@ -1050,11 +1046,7 @@ fn validate_payload_succeeds_for_valid_non_replicated_response() { let (response, metadata) = test_response_and_metadata(callback_id.get()); let mut proof = response_and_metadata_to_proof(&response, &metadata); // The proof must contain exactly ONE signature, from the DELEGATED node. - proof - .proof - .signature - .signatures_map - .insert(delegated_node_id, BasicSigOf::new(BasicSig(vec![]))); + add_signer_to_proof(&mut proof, delegated_node_id); let payload = CanisterHttpPayload { responses: vec![proof], @@ -1105,10 +1097,8 @@ fn validate_payload_fails_for_non_replicated_response_with_wrong_signer() { let (response, metadata) = test_response_and_metadata(callback_id.get()); let mut proof = response_and_metadata_to_proof(&response, &metadata); - proof.proof.signature.signatures_map.insert( - wrong_signer_node_id, // The illegal signature - BasicSigOf::new(BasicSig(vec![])), - ); + // The illegal signature from a non-member node. + add_signer_to_proof(&mut proof, wrong_signer_node_id); let payload = CanisterHttpPayload { responses: vec![proof], @@ -1175,7 +1165,7 @@ fn validate_payload_fails_for_response_with_no_signatures() { let mut proof = response_and_metadata_to_proof(&response, &metadata); // Ensure the signature map is empty. - proof.proof.signature.signatures_map = BTreeMap::new(); + proof.proof.signatures = BTreeMap::new(); let payload = CanisterHttpPayload { responses: vec![proof], @@ -1251,11 +1241,7 @@ fn validate_payload_fails_when_non_replicated_proof_is_for_fully_replicated_requ let mut proof = response_and_metadata_to_proof(&response, &metadata); // The proof only contains one signature. - proof - .proof - .signature - .signatures_map - .insert(signer_node_id, BasicSigOf::new(BasicSig(vec![]))); + add_signer_to_proof(&mut proof, signer_node_id); let payload = CanisterHttpPayload { responses: vec![proof], @@ -1331,11 +1317,7 @@ fn validate_payload_fails_for_duplicate_non_replicated_response() { // 3. Craft a valid proof for the NonReplicated response. let (response, metadata) = test_response_and_metadata(duplicate_callback_id.get()); let mut proof = response_and_metadata_to_proof(&response, &metadata); - proof - .proof - .signature - .signatures_map - .insert(delegated_node_id, BasicSigOf::new(BasicSig(vec![]))); + add_signer_to_proof(&mut proof, delegated_node_id); // 4. Create a payload that includes this same proof twice. let payload = CanisterHttpPayload { @@ -1475,31 +1457,48 @@ pub(crate) fn metadata_to_share_with_signature( metadata: &CanisterHttpResponseMetadata, signature: Vec, ) -> CanisterHttpResponseShare { + let signer = node_test_id(from_node); Signed { - content: metadata.clone(), + content: CanisterHttpResponseReceiptShare { + metadata: metadata.clone(), + payment_receipt: CanisterHttpPaymentReceipt::default(), + }, signature: BasicSignature { signature: BasicSigOf::new(BasicSig(signature)), - signer: node_test_id(from_node), + signer, }, } } -/// Creates a [`CanisterHttpResponseWithConsensus`] from a [`CanisterHttpResponse`] and [`CanisterHttpResponseMetadata`] +/// Creates a [`CanisterHttpResponseWithConsensus`] from a [`CanisterHttpResponse`] and [`CanisterHttpResponseMetadata`]. +/// +/// The proof starts out with an empty signatures map; tests insert +/// per-signer receipt + signature pairs via [`add_signer_to_proof`]. pub(crate) fn response_and_metadata_to_proof( response: &CanisterHttpResponse, metadata: &CanisterHttpResponseMetadata, ) -> CanisterHttpResponseWithConsensus { CanisterHttpResponseWithConsensus { content: response.clone(), - proof: Signed { - content: metadata.clone(), - signature: BasicSignatureBatch { - signatures_map: BTreeMap::new(), - }, + proof: CanisterHttpResponseProof { + metadata: metadata.clone(), + signatures: BTreeMap::new(), }, } } +/// Inserts a fake signature together with a default payment receipt for +/// `signer` into an aggregated proof. +pub(crate) fn add_signer_to_proof(proof: &mut CanisterHttpResponseWithConsensus, signer: NodeId) { + proof.proof.signatures.insert( + signer, + CanisterHttpResponseSignature { + payment_receipt: CanisterHttpPaymentReceipt::default(), + signature: BasicSigOf::new(BasicSig(vec![])), + }, + ); +} + /// Creates a vector of [`CanisterHttpResponseShare`]s by calling [`metadata_to_share`] pub(crate) fn metadata_to_shares( num_nodes: usize, @@ -2212,7 +2211,7 @@ fn flexible_invalid_callback_id_mismatch_in_proof() { setup_test_with_flexible_context(4, callback_id, committee, 1, 4, |payload_builder, _pool| { let mut entry = flexible_response(42, 0, b"data"); - entry.proof.content.id = mismatched_id; + entry.proof.content.metadata.id = mismatched_id; let payload = flexible_payload(vec![FlexibleCanisterHttpResponses { callback_id, @@ -2307,7 +2306,7 @@ fn flexible_invalid_content_hash_mismatch() { let mut entry = flexible_response(42, 0, b"data"); let expected_calculated_hash = crypto_hash(&entry.response); let wrong_metadata_hash = CryptoHashOf::new(CryptoHash(vec![0xff; 32])); - entry.proof.content.content_hash = wrong_metadata_hash.clone(); + entry.proof.content.metadata.content_hash = wrong_metadata_hash.clone(); let payload = flexible_payload(vec![FlexibleCanisterHttpResponses { callback_id, @@ -2342,8 +2341,8 @@ fn flexible_invalid_content_size_mismatch() { setup_test_with_flexible_context(4, callback_id, committee, 1, 4, |payload_builder, _pool| { let mut entry = flexible_response(42, 0, b"data"); let expected_size = entry.response.content.count_bytes() as u32; - entry.proof.content.content_size = expected_size.wrapping_add(1); - let wrong_size = entry.proof.content.content_size; + entry.proof.content.metadata.content_size = expected_size.wrapping_add(1); + let wrong_size = entry.proof.content.content_size(); let payload = flexible_payload(vec![FlexibleCanisterHttpResponses { callback_id, @@ -2377,7 +2376,7 @@ fn flexible_invalid_is_reject_mismatch() { setup_test_with_flexible_context(4, callback_id, committee, 1, 4, |payload_builder, _pool| { let mut entry = flexible_response(42, 0, b"data"); - entry.proof.content.is_reject = !entry.proof.content.is_reject; + entry.proof.content.metadata.is_reject = !entry.proof.content.is_reject(); let payload = flexible_payload(vec![FlexibleCanisterHttpResponses { callback_id, @@ -2410,11 +2409,7 @@ fn flexible_response_in_regular_section_rejected() { setup_test_with_flexible_context(4, callback_id, committee, 2, 4, |payload_builder, _pool| { let (response, metadata) = test_response_and_metadata(callback_id.get()); let mut proof = response_and_metadata_to_proof(&response, &metadata); - proof - .proof - .signature - .signatures_map - .insert(node_test_id(0), BasicSigOf::new(BasicSig(vec![]))); + add_signer_to_proof(&mut proof, node_test_id(0)); let payload = CanisterHttpPayload { responses: vec![proof], @@ -2583,7 +2578,7 @@ fn flexible_invalid_registry_version_mismatch() { setup_test_with_flexible_context(4, callback_id, committee, 1, 4, |payload_builder, _pool| { let wrong_registry_version = RegistryVersion::new(999); let mut entry = flexible_response(42, 0, b"data"); - entry.proof.content.registry_version = wrong_registry_version; + entry.proof.content.metadata.registry_version = wrong_registry_version; let validation_context = default_validation_context(); let expected_registry_version = validation_context.registry_version; @@ -3040,7 +3035,7 @@ fn flexible_build_responses_too_large() { } => { assert_eq!(*cb, callback_id); assert_eq!(all_seen_shares.len(), 4); - assert!(all_seen_shares.iter().all(|s| !s.content.is_reject)); + assert!(all_seen_shares.iter().all(|s| !s.content.is_reject())); assert_eq!(*total_requests, 4); assert_eq!(*min_responses, 2); } @@ -3133,8 +3128,8 @@ fn flexible_build_responses_too_large_with_rejects_reducing_unseen() { } => { assert_eq!(*cb, callback_id); assert_eq!(all_seen_shares.len(), 5); - let ok_count = all_seen_shares.iter().filter(|s| !s.content.is_reject).count(); - let reject_count = all_seen_shares.iter().filter(|s| s.content.is_reject).count(); + let ok_count = all_seen_shares.iter().filter(|s| !s.content.is_reject()).count(); + let reject_count = all_seen_shares.iter().filter(|s| s.content.is_reject()).count(); assert_eq!(ok_count, 3); assert_eq!(reject_count, 2); assert_eq!(*total_requests, 6); @@ -3195,8 +3190,8 @@ fn flexible_build_responses_too_large_fewer_ok_than_min_responses() { } => { assert_eq!(*cb, callback_id); assert_eq!(all_seen_shares.len(), 5); - let ok_count = all_seen_shares.iter().filter(|s| !s.content.is_reject).count(); - let reject_count = all_seen_shares.iter().filter(|s| s.content.is_reject).count(); + let ok_count = all_seen_shares.iter().filter(|s| !s.content.is_reject()).count(); + let reject_count = all_seen_shares.iter().filter(|s| s.content.is_reject()).count(); assert_eq!(ok_count, 3); assert_eq!(reject_count, 2); assert_eq!(*total_requests, 6); @@ -3984,7 +3979,7 @@ fn flexible_error_responses_too_large_registry_version_mismatch() { 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); + share_bad.content.metadata.registry_version = RegistryVersion::new(999); let payload = CanisterHttpPayload { flexible_errors: vec![FlexibleCanisterHttpError::ResponsesTooLarge { @@ -4268,7 +4263,7 @@ fn flexible_error_too_many_rejects_registry_version_mismatch() { 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); + entry_bad.proof.content.metadata.registry_version = RegistryVersion::new(999); let payload = CanisterHttpPayload { flexible_errors: vec![FlexibleCanisterHttpError::TooManyRejects { @@ -4303,7 +4298,8 @@ fn flexible_error_too_many_rejects_content_hash_mismatch() { 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])); + entry_bad.proof.content.metadata.content_hash = + CryptoHashOf::new(CryptoHash(vec![0xFF; 32])); let payload = CanisterHttpPayload { flexible_errors: vec![FlexibleCanisterHttpError::TooManyRejects { @@ -4338,7 +4334,7 @@ fn flexible_error_too_many_rejects_content_size_mismatch() { 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; + entry_bad.proof.content.metadata.content_size = 999_999; let payload = CanisterHttpPayload { flexible_errors: vec![FlexibleCanisterHttpError::TooManyRejects { @@ -4373,7 +4369,7 @@ fn flexible_error_too_many_rejects_is_reject_mismatch() { 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.is_reject = !entry_bad.proof.content.is_reject; + entry_bad.proof.content.metadata.is_reject = !entry_bad.proof.content.is_reject(); let payload = CanisterHttpPayload { flexible_errors: vec![FlexibleCanisterHttpError::TooManyRejects { @@ -4409,7 +4405,7 @@ fn flexible_error_too_many_rejects_proof_id_mismatch() { 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); + entry_bad.proof.content.metadata.id = CallbackId::new(999); let payload = CanisterHttpPayload { flexible_errors: vec![FlexibleCanisterHttpError::TooManyRejects { diff --git a/rs/https_outcalls/consensus/src/payload_builder/utils.rs b/rs/https_outcalls/consensus/src/payload_builder/utils.rs index 800e452e453f..9bbab3b1dfa6 100644 --- a/rs/https_outcalls/consensus/src/payload_builder/utils.rs +++ b/rs/https_outcalls/consensus/src/payload_builder/utils.rs @@ -9,11 +9,11 @@ use ic_types::{ }, canister_http::{ CanisterHttpResponse, CanisterHttpResponseContent, CanisterHttpResponseMetadata, - CanisterHttpResponseShare, CanisterHttpResponseWithConsensus, + CanisterHttpResponseProof, CanisterHttpResponseReceiptShare, CanisterHttpResponseShare, + CanisterHttpResponseSignature, CanisterHttpResponseWithConsensus, }, crypto::crypto_hash, messages::CallbackId, - signature::BasicSignature, }; use std::{ collections::{BTreeMap, BTreeSet, HashSet}, @@ -33,7 +33,7 @@ pub(crate) fn check_response_consistency( response: &CanisterHttpResponseWithConsensus, ) -> Result<(), InvalidCanisterHttpPayloadReason> { let content = &response.content; - let metadata = &response.proof.content; + let metadata = &response.proof.metadata; // Check metadata field consistency if metadata.id != content.id { @@ -73,6 +73,42 @@ pub(crate) fn check_response_consistency( Ok(()) } +/// Verifies every per-signer basic signature in the aggregated proof. +/// +/// Each signer signs its own [`CanisterHttpResponseReceiptShare`]: the +/// shared [`CanisterHttpResponseMetadata`] combined with that signer's own +/// [`CanisterHttpPaymentReceipt`]. Since the signed messages differ per +/// signer, the whole batch is verified in one call with the multi-message +/// batch verification, which is cheaper than verifying each signature +/// individually. +pub(crate) fn verify_aggregate_proof( + proof: &CanisterHttpResponseProof, + consensus_registry_version: RegistryVersion, + crypto: &dyn ConsensusCrypto, +) -> Result<(), InvalidCanisterHttpPayloadReason> { + // The reconstructed per-signer messages must outlive `inputs`, which + // only borrows them. + let messages: Vec = proof + .signatures + .values() + .map(|sig| CanisterHttpResponseReceiptShare { + metadata: proof.metadata.clone(), + payment_receipt: sig.payment_receipt.clone(), + }) + .collect(); + + let inputs: Vec<_> = proof + .signatures + .iter() + .zip(messages.iter()) + .map(|((signer, sig), message)| (*signer, &sig.signature, message)) + .collect(); + + crypto + .verify_basic_sig_batch_multi_msg(&inputs, consensus_registry_version) + .map_err(|err| InvalidCanisterHttpPayloadReason::SignatureError(Box::new(err))) +} + /// Validates a single [`FlexibleCanisterHttpResponseWithProof`]. /// /// Checks callback-id consistency, share validity (using @@ -104,25 +140,25 @@ pub(crate) fn validate_flexible_response_with_proof( )?; let calculated_hash = crypto_hash(&response_with_proof.response); - if calculated_hash != response_with_proof.proof.content.content_hash { + if &calculated_hash != response_with_proof.proof.content.content_hash() { return Err(InvalidCanisterHttpPayloadReason::ContentHashMismatch { - metadata_hash: response_with_proof.proof.content.content_hash.clone(), + 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 { + if calculated_size != response_with_proof.proof.content.content_size() { return Err(InvalidCanisterHttpPayloadReason::ContentSizeMismatch { - metadata_size: response_with_proof.proof.content.content_size, + metadata_size: response_with_proof.proof.content.content_size(), calculated_size, }); } let calculated_is_reject = response_with_proof.response.content.is_reject(); - if calculated_is_reject != response_with_proof.proof.content.is_reject { + if calculated_is_reject != response_with_proof.proof.content.is_reject() { return Err(InvalidCanisterHttpPayloadReason::IsRejectMismatch { - metadata_is_reject: response_with_proof.proof.content.is_reject, + metadata_is_reject: response_with_proof.proof.content.is_reject(), calculated_is_reject, }); } @@ -142,11 +178,11 @@ pub(crate) fn validate_response_share( consensus_registry_version: RegistryVersion, crypto: &dyn ConsensusCrypto, ) -> Result<(), InvalidCanisterHttpPayloadReason> { - if share.content.id != callback_id { + if share.content.id() != callback_id { return Err( InvalidCanisterHttpPayloadReason::FlexibleCallbackIdMismatch { callback_id, - mismatched_id: share.content.id, + mismatched_id: share.content.id(), }, ); } @@ -167,10 +203,10 @@ pub(crate) fn validate_response_share( ); } - if share.content.registry_version != consensus_registry_version { + if share.content.registry_version() != consensus_registry_version { return Err(InvalidCanisterHttpPayloadReason::RegistryVersionMismatch { expected: consensus_registry_version, - received: share.content.registry_version, + received: share.content.registry_version(), }); } @@ -222,6 +258,13 @@ pub(crate) fn grouped_shares_meet_divergence_criteria( } } +/// Groups shares by callback id and then by their shared metadata. +/// +/// Shares from different replicas for the same outcall agree on the +/// shared metadata but each carry their own payment receipt. We key the +/// inner `BTreeMap` on the metadata (taken from +/// `share.content.metadata`), so each group holds all shares that voted +/// for the same response and differ only in their per-replica receipt. pub(crate) fn group_shares_by_callback_id< 'a, Shares: Iterator, @@ -234,9 +277,9 @@ pub(crate) fn group_shares_by_callback_id< BTreeMap>, > = BTreeMap::new(); for share in shares { - map.entry(share.content.id) + map.entry(share.content.id()) .or_default() - .entry(share.content.clone()) + .entry(share.content.metadata.clone()) .or_default() .push(share); } @@ -245,29 +288,23 @@ pub(crate) fn group_shares_by_callback_id< /// Finds a fully-replicated HTTP outcall response ready for consensus. /// -/// Iterates over response shares grouped by metadata, looking for one where -/// at least `threshold` distinct replicas produced the same response hash. -/// If found, returns the metadata, collected signatures, and response body. +/// Iterates over response shares grouped by metadata, looking for one +/// where at least `threshold` distinct replicas produced the same +/// response hash. If found, returns the assembled +/// [`CanisterHttpResponseWithConsensus`]. pub(crate) fn find_fully_replicated_response( grouped_shares: &BTreeMap>, threshold: usize, pool_access: &dyn CanisterHttpPool, -) -> Option<( - CanisterHttpResponseMetadata, - BTreeSet>, - CanisterHttpResponse, -)> { +) -> Option { grouped_shares.iter().find_map(|(metadata, shares)| { let signers: BTreeSet<_> = shares.iter().map(|share| share.signature.signer).collect(); if signers.len() >= threshold { pool_access .get_response_content_by_hash(&metadata.content_hash) - .map(|content| { - ( - metadata.clone(), - shares.iter().map(|share| share.signature.clone()).collect(), - content, - ) + .map(|content| CanisterHttpResponseWithConsensus { + content, + proof: proof_from_shares(metadata.clone(), shares), }) } else { None @@ -278,16 +315,12 @@ pub(crate) fn find_fully_replicated_response( /// Finds a non-replicated HTTP outcall response from the designated node. /// /// Looks through the grouped shares for one signed by `designated_node_id`. -/// If found, returns the metadata, the single signature, and response body. +/// If found, returns the assembled [`CanisterHttpResponseWithConsensus`]. pub(crate) fn find_non_replicated_response( grouped_shares: &BTreeMap>, designated_node_id: &NodeId, pool_access: &dyn CanisterHttpPool, -) -> Option<( - CanisterHttpResponseMetadata, - BTreeSet>, - CanisterHttpResponse, -)> { +) -> Option { grouped_shares.iter().find_map(|(metadata, shares)| { shares .iter() @@ -295,32 +328,43 @@ pub(crate) fn find_non_replicated_response( .and_then(|correct_share| { pool_access .get_response_content_by_hash(&metadata.content_hash) - .map(|content| { - ( - metadata.clone(), - BTreeSet::from([correct_share.signature.clone()]), - content, - ) + .map(|content| CanisterHttpResponseWithConsensus { + content, + proof: proof_from_shares(metadata.clone(), &[correct_share]), }) }) }) } -/// Estimates the byte size of a [`CanisterHttpResponseWithConsensus`] before -/// the proof has been aggregated. +/// Assembles a [`CanisterHttpResponseProof`] from a slice of contributing +/// shares: the shared `metadata` together with, for each signer, the +/// basic signature and payment receipt taken directly from that signer's +/// share. /// -/// This function mirrors the implementation of -/// `CanisterHttpResponseWithConsensus::count_bytes()`: -/// proof.count_bytes() → metadata.count_bytes() + Σ share.count_bytes() -/// content.count_bytes() → content.count_bytes() -pub(crate) fn estimate_response_with_consensus_size( - metadata: &CanisterHttpResponseMetadata, - shares: &BTreeSet>, - content: &CanisterHttpResponse, -) -> usize { - metadata.count_bytes() - + shares.iter().map(|s| s.count_bytes()).sum::() - + content.count_bytes() +/// Basic signatures are not cryptographically combined, so there is no +/// separate aggregation step — each share already carries its own +/// signature and receipt. In practice the upstream code feeds at most one +/// share per signer. +pub(crate) fn proof_from_shares( + metadata: CanisterHttpResponseMetadata, + shares: &[&CanisterHttpResponseShare], +) -> CanisterHttpResponseProof { + let signatures = shares + .iter() + .map(|share| { + ( + share.signature.signer, + CanisterHttpResponseSignature { + payment_receipt: share.content.payment_receipt.clone(), + signature: share.signature.signature.clone(), + }, + ) + }) + .collect(); + CanisterHttpResponseProof { + metadata, + signatures, + } } /// Result of scanning flexible HTTP outcall shares for a single callback. diff --git a/rs/https_outcalls/consensus/src/pool_manager.rs b/rs/https_outcalls/consensus/src/pool_manager.rs index eac24c4f2b6f..f65d9987511d 100644 --- a/rs/https_outcalls/consensus/src/pool_manager.rs +++ b/rs/https_outcalls/consensus/src/pool_manager.rs @@ -167,7 +167,7 @@ impl CanisterHttpPoolManagerImpl { canister_http_pool .get_validated_shares() .filter_map(|share| { - if active_callback_ids.contains(&share.content.id) { + if active_callback_ids.contains(&share.content.id()) { None } else { Some(CanisterHttpChangeAction::RemoveValidated(share.clone())) @@ -177,10 +177,10 @@ impl CanisterHttpPoolManagerImpl { canister_http_pool .get_unvalidated_artifacts() // Only check the unvalidated shares belonging to the requests that we can validate. - .filter(|artifact| artifact.share.content.id < next_callback_id) + .filter(|artifact| artifact.share.content.id() < next_callback_id) .filter_map(|artifact| { let share = &artifact.share; - if active_callback_ids.contains(&share.content.id) { + if active_callback_ids.contains(&share.content.id()) { None } else { Some(CanisterHttpChangeAction::RemoveUnvalidated(share.clone())) @@ -270,7 +270,7 @@ impl CanisterHttpPoolManagerImpl { .get_validated_shares() .filter_map(|share| { if share.signature.signer == self.replica_config.node_id { - Some(share.content.id) + Some(share.content.id()) } else { None } @@ -344,7 +344,7 @@ impl CanisterHttpPoolManagerImpl { loop { match self.http_adapter_shim.lock().unwrap().try_receive() { Err(TryReceiveError::Empty) => break, - Ok((mut response, _payment_receipt)) => { + Ok((mut response, payment_receipt)) => { // Drop the response if its context is no longer present in the replicated state // (e.g. the request has timed out or has already been answered by enough other nodes). let Some(context) = active_contexts.get(&response.id) else { @@ -380,18 +380,21 @@ impl CanisterHttpPoolManagerImpl { .ellipsize(MAXIMUM_ALLOWED_ERROR_MESSAGE_BYTES, 90); } - let response_metadata = CanisterHttpResponseMetadata { - id: response.id, - registry_version, - content_hash: ic_types::crypto::crypto_hash(&response), - content_size: response.content.count_bytes() as u32, - is_reject: response.content.is_reject(), - replica_version: ReplicaVersion::default(), + let receipt_share = CanisterHttpResponseReceiptShare { + metadata: CanisterHttpResponseMetadata { + id: response.id, + registry_version, + content_hash: ic_types::crypto::crypto_hash(&response), + content_size: response.content.count_bytes() as u32, + is_reject: response.content.is_reject(), + replica_version: ReplicaVersion::default(), + }, + payment_receipt, }; let signature = if let Ok(signature) = self .crypto .sign( - &response_metadata, + &receipt_share, self.replica_config.node_id, registry_version, ) @@ -402,7 +405,7 @@ impl CanisterHttpPoolManagerImpl { continue; }; let share = Signed { - content: response_metadata, + content: receipt_share, signature, }; self.requested_id_cache.borrow_mut().remove(&response.id); @@ -469,7 +472,7 @@ impl CanisterHttpPoolManagerImpl { let next_callback_id = self.next_callback_id(); let key_from_share = - |share: &CanisterHttpResponseShare| (share.signature.signer, share.content.id); + |share: &CanisterHttpResponseShare| (share.signature.signer, share.content.id()); let mut existing_signed_requests: HashSet<_> = canister_http_pool .get_validated_shares() @@ -478,12 +481,12 @@ impl CanisterHttpPoolManagerImpl { canister_http_pool .get_unvalidated_artifacts() - .filter(|artifact| artifact.share.content.id < next_callback_id) + .filter(|artifact| artifact.share.content.id() < next_callback_id) .filter_map(|artifact| { let share = &artifact.share; if existing_signed_requests.contains(&key_from_share(share)) { - return match is_current_protocol_version(&share.content.replica_version) { + return match is_current_protocol_version(&share.content.replica_version()) { true => Some(CanisterHttpChangeAction::HandleInvalid( share.clone(), "Redundant share".into(), @@ -492,10 +495,20 @@ impl CanisterHttpPoolManagerImpl { }; } - let Some(context) = active_contexts.get(&share.content.id) else { + let Some(context) = active_contexts.get(&share.content.id()) else { return Some(CanisterHttpChangeAction::RemoveUnvalidated(share.clone())); }; + // Invalidate shares whose refund exceeds what a single + // replica is allowed to claim. An honest replica cannot + // legitimately refund more than its per-replica allowance. + if share.content.refund() > context.refund_status.per_replica_allowance { + return Some(CanisterHttpChangeAction::HandleInvalid( + share.clone(), + "Refund is greater than replica allowance".to_string(), + )); + } + match &context.replication { Replication::FullyReplicated => { if artifact.response.is_some() { @@ -521,21 +534,22 @@ impl CanisterHttpPoolManagerImpl { )); }; - if share.content.content_hash != ic_types::crypto::crypto_hash(response) { + if share.content.content_hash() != &ic_types::crypto::crypto_hash(response) + { return Some(CanisterHttpChangeAction::HandleInvalid( share.clone(), "Content hash does not match the response".to_string(), )); } - if share.content.content_size != response.content.count_bytes() as u32 { + if share.content.content_size() != response.content.count_bytes() as u32 { return Some(CanisterHttpChangeAction::HandleInvalid( share.clone(), "Content size does not match the response".to_string(), )); } - if share.content.is_reject != response.content.is_reject() { + if share.content.is_reject() != response.content.is_reject() { return Some(CanisterHttpChangeAction::HandleInvalid( share.clone(), "is_reject does not match the response content".to_string(), @@ -711,6 +725,7 @@ pub mod test { messages::CallbackId, time::UNIX_EPOCH, }; + use ic_types_cycles::Cycles; use mockall::predicate::*; use mockall::*; use std::{collections::BTreeMap, str::FromStr}; @@ -816,13 +831,16 @@ pub mod test { // Try to insert a share for request id 1 (while the next expected one is the // default value 0). { - let response_metadata = CanisterHttpResponseMetadata { - id: CallbackId::from(1), - registry_version: RegistryVersion::from(1), - content_hash: CryptoHashOf::new(CryptoHash(vec![])), - content_size: 0, - is_reject: false, - replica_version: ReplicaVersion::default(), + let response_metadata = CanisterHttpResponseReceiptShare { + metadata: CanisterHttpResponseMetadata { + id: CallbackId::from(1), + registry_version: RegistryVersion::from(1), + content_hash: CryptoHashOf::new(CryptoHash(vec![])), + content_size: 0, + is_reject: false, + replica_version: ReplicaVersion::default(), + }, + payment_receipt: CanisterHttpPaymentReceipt::default(), }; let signature = crypto @@ -913,13 +931,16 @@ pub mod test { )]))), )); - let response_metadata = CanisterHttpResponseMetadata { - id: CallbackId::from(0), - registry_version: RegistryVersion::from(1), - content_hash: CryptoHashOf::new(CryptoHash(vec![])), - content_size: 0, - is_reject: false, - replica_version: ReplicaVersion::default(), + let response_metadata = CanisterHttpResponseReceiptShare { + metadata: CanisterHttpResponseMetadata { + id: CallbackId::from(0), + registry_version: RegistryVersion::from(1), + content_hash: CryptoHashOf::new(CryptoHash(vec![])), + content_size: 0, + is_reject: false, + replica_version: ReplicaVersion::default(), + }, + payment_receipt: CanisterHttpPaymentReceipt::default(), }; let mut canister_http_pool = @@ -945,7 +966,7 @@ pub mod test { )]); // add an unvalidated copy of the share, that has an outdated version instead - share.content.replica_version = + share.content.metadata.replica_version = ReplicaVersion::from_str("outdated_version").unwrap(); let artifact = CanisterHttpResponseArtifact { @@ -1019,13 +1040,16 @@ pub mod test { )]))), )); - let response_metadata = CanisterHttpResponseMetadata { - id: CallbackId::from(0), - registry_version: RegistryVersion::from(1), - content_hash: CryptoHashOf::new(CryptoHash(vec![])), - content_size: 0, - is_reject: false, - replica_version: ReplicaVersion::default(), + let response_metadata = CanisterHttpResponseReceiptShare { + metadata: CanisterHttpResponseMetadata { + id: CallbackId::from(0), + registry_version: RegistryVersion::from(1), + content_hash: CryptoHashOf::new(CryptoHash(vec![])), + content_size: 0, + is_reject: false, + replica_version: ReplicaVersion::default(), + }, + payment_receipt: CanisterHttpPaymentReceipt::default(), }; let mut canister_http_pool = @@ -1147,13 +1171,16 @@ pub mod test { )); let response = empty_canister_http_response(0); - let response_metadata = CanisterHttpResponseMetadata { - id: CallbackId::from(0), - registry_version: RegistryVersion::from(1), - content_hash: ic_types::crypto::crypto_hash(&response), - content_size: response.content.count_bytes() as u32, - is_reject: false, - replica_version: ReplicaVersion::default(), + let response_metadata = CanisterHttpResponseReceiptShare { + metadata: CanisterHttpResponseMetadata { + id: CallbackId::from(0), + registry_version: RegistryVersion::from(1), + content_hash: ic_types::crypto::crypto_hash(&response), + content_size: response.content.count_bytes() as u32, + is_reject: false, + replica_version: ReplicaVersion::default(), + }, + payment_receipt: CanisterHttpPaymentReceipt::default(), }; let signature = crypto @@ -1215,7 +1242,8 @@ pub mod test { CanisterHttpPoolImpl::new(MetricsRegistry::new(), no_op_logger()); let mut bad_share = share.clone(); - bad_share.content.content_hash = CryptoHashOf::new(CryptoHash(vec![1, 2, 3])); + bad_share.content.metadata.content_hash = + CryptoHashOf::new(CryptoHash(vec![1, 2, 3])); let artifact_with_mismatched_hash = CanisterHttpResponseArtifact { share: bad_share, @@ -1247,7 +1275,8 @@ pub mod test { CanisterHttpPoolImpl::new(MetricsRegistry::new(), no_op_logger()); let mut bad_share = share.clone(); - bad_share.content.content_size = bad_share.content.content_size.wrapping_add(1); + bad_share.content.metadata.content_size = + bad_share.content.metadata.content_size.wrapping_add(1); let artifact_with_mismatched_size = CanisterHttpResponseArtifact { share: bad_share, @@ -1279,7 +1308,7 @@ pub mod test { CanisterHttpPoolImpl::new(MetricsRegistry::new(), no_op_logger()); let mut bad_share = share.clone(); - bad_share.content.is_reject = !bad_share.content.is_reject; + bad_share.content.metadata.is_reject = !bad_share.content.is_reject(); let artifact_with_mismatched_is_reject = CanisterHttpResponseArtifact { share: bad_share, @@ -1344,13 +1373,16 @@ pub mod test { // 3. MALICIOUS ARTIFACT: Create a share that is signed by the `wrong_signer_id`. let response = empty_canister_http_response(callback_id.get()); - let response_metadata = CanisterHttpResponseMetadata { - id: callback_id, - registry_version: RegistryVersion::from(1), - content_hash: ic_types::crypto::crypto_hash(&response), - content_size: response.content.count_bytes() as u32, - is_reject: false, - replica_version: ReplicaVersion::default(), + let response_metadata = CanisterHttpResponseReceiptShare { + metadata: CanisterHttpResponseMetadata { + id: callback_id, + registry_version: RegistryVersion::from(1), + content_hash: ic_types::crypto::crypto_hash(&response), + content_size: response.content.count_bytes() as u32, + is_reject: false, + replica_version: ReplicaVersion::default(), + }, + payment_receipt: CanisterHttpPaymentReceipt::default(), }; let share = Signed { content: response_metadata.clone(), @@ -1442,13 +1474,16 @@ pub mod test { )); let response = empty_canister_http_response(0); - let response_metadata = CanisterHttpResponseMetadata { - id: CallbackId::from(0), - registry_version: RegistryVersion::from(1), - content_hash: ic_types::crypto::crypto_hash(&response), - content_size: response.content.count_bytes() as u32, - is_reject: false, - replica_version: ReplicaVersion::default(), + let response_metadata = CanisterHttpResponseReceiptShare { + metadata: CanisterHttpResponseMetadata { + id: CallbackId::from(0), + registry_version: RegistryVersion::from(1), + content_hash: ic_types::crypto::crypto_hash(&response), + content_size: response.content.count_bytes() as u32, + is_reject: false, + replica_version: ReplicaVersion::default(), + }, + payment_receipt: CanisterHttpPaymentReceipt::default(), }; let signature = crypto @@ -1580,13 +1615,16 @@ pub mod test { content: CanisterHttpResponseContent::Success(response_body_too_large), }; - let response_metadata = CanisterHttpResponseMetadata { - id: CallbackId::from(0), - registry_version: RegistryVersion::from(1), - content_hash: ic_types::crypto::crypto_hash(&response), - content_size: response.content.count_bytes() as u32, - is_reject: false, - replica_version: ReplicaVersion::default(), + let response_metadata = CanisterHttpResponseReceiptShare { + metadata: CanisterHttpResponseMetadata { + id: CallbackId::from(0), + registry_version: RegistryVersion::from(1), + content_hash: ic_types::crypto::crypto_hash(&response), + content_size: response.content.count_bytes() as u32, + is_reject: false, + replica_version: ReplicaVersion::default(), + }, + payment_receipt: CanisterHttpPaymentReceipt::default(), }; let share = Signed { content: response_metadata.clone(), @@ -1646,13 +1684,16 @@ pub mod test { content: CanisterHttpResponseContent::Success(response_body_ok), }; - let response_metadata = CanisterHttpResponseMetadata { - id: CallbackId::from(0), - registry_version: RegistryVersion::from(1), - content_hash: ic_types::crypto::crypto_hash(&response), - content_size: response.content.count_bytes() as u32, - is_reject: false, - replica_version: ReplicaVersion::default(), + let response_metadata = CanisterHttpResponseReceiptShare { + metadata: CanisterHttpResponseMetadata { + id: CallbackId::from(0), + registry_version: RegistryVersion::from(1), + content_hash: ic_types::crypto::crypto_hash(&response), + content_size: response.content.count_bytes() as u32, + is_reject: false, + replica_version: ReplicaVersion::default(), + }, + payment_receipt: CanisterHttpPaymentReceipt::default(), }; let share = Signed { content: response_metadata.clone(), @@ -1750,13 +1791,16 @@ pub mod test { ..empty_canister_http_response(0) }; - let response_metadata = CanisterHttpResponseMetadata { - id: CallbackId::from(0), - registry_version: RegistryVersion::from(1), - content_hash: ic_types::crypto::crypto_hash(&response), - content_size: response.content.count_bytes() as u32, - is_reject: true, - replica_version: ReplicaVersion::default(), + let response_metadata = CanisterHttpResponseReceiptShare { + metadata: CanisterHttpResponseMetadata { + id: CallbackId::from(0), + registry_version: RegistryVersion::from(1), + content_hash: ic_types::crypto::crypto_hash(&response), + content_size: response.content.count_bytes() as u32, + is_reject: true, + replica_version: ReplicaVersion::default(), + }, + payment_receipt: CanisterHttpPaymentReceipt::default(), }; let share = Signed { @@ -1904,7 +1948,7 @@ pub mod test { // Assert that the hash in the share matches the hash of the *truncated* response. let expected_hash = ic_types::crypto::crypto_hash(response); assert_eq!( - share.content.content_hash, expected_hash, + share.content.content_hash(), &expected_hash, "The share's content hash must match the pruned response" ); } @@ -2036,7 +2080,7 @@ pub mod test { // Assert that the hash in the share matches the hash of the now-pruned response. let expected_hash = ic_types::crypto::crypto_hash(response); assert_eq!( - share.content.content_hash, expected_hash, + share.content.content_hash(), &expected_hash, "The share's content hash must match the pruned response" ); } @@ -2092,13 +2136,16 @@ pub mod test { }; let dishonest_hash = ic_types::crypto::crypto_hash(&dishonest_response); - let response_metadata = CanisterHttpResponseMetadata { - id: callback_id, - registry_version: RegistryVersion::from(1), - content_hash: dishonest_hash, - content_size: dishonest_response.content.count_bytes() as u32, - is_reject: true, - replica_version: ReplicaVersion::default(), + let response_metadata = CanisterHttpResponseReceiptShare { + metadata: CanisterHttpResponseMetadata { + id: callback_id, + registry_version: RegistryVersion::from(1), + content_hash: dishonest_hash, + content_size: dishonest_response.content.count_bytes() as u32, + is_reject: true, + replica_version: ReplicaVersion::default(), + }, + payment_receipt: CanisterHttpPaymentReceipt::default(), }; let share = Signed { content: response_metadata.clone(), @@ -2231,13 +2278,16 @@ pub mod test { ..empty_canister_http_response(0) }; - let response_metadata = CanisterHttpResponseMetadata { - id: CallbackId::from(0), - registry_version: RegistryVersion::from(1), - content_hash: ic_types::crypto::crypto_hash(&response), - content_size: response.content.count_bytes() as u32, - is_reject: true, - replica_version: ReplicaVersion::default(), + let response_metadata = CanisterHttpResponseReceiptShare { + metadata: CanisterHttpResponseMetadata { + id: CallbackId::from(0), + registry_version: RegistryVersion::from(1), + content_hash: ic_types::crypto::crypto_hash(&response), + content_size: response.content.count_bytes() as u32, + is_reject: true, + replica_version: ReplicaVersion::default(), + }, + payment_receipt: CanisterHttpPaymentReceipt::default(), }; let share = Signed { @@ -2309,13 +2359,16 @@ pub mod test { )]))), )); - let response_metadata = CanisterHttpResponseMetadata { - id: CallbackId::from(7), - registry_version: RegistryVersion::from(1), - content_hash: CryptoHashOf::new(CryptoHash(vec![])), - content_size: 0, - is_reject: false, - replica_version: ReplicaVersion::default(), + let response_metadata = CanisterHttpResponseReceiptShare { + metadata: CanisterHttpResponseMetadata { + id: CallbackId::from(7), + registry_version: RegistryVersion::from(1), + content_hash: CryptoHashOf::new(CryptoHash(vec![])), + content_size: 0, + is_reject: false, + replica_version: ReplicaVersion::default(), + }, + payment_receipt: CanisterHttpPaymentReceipt::default(), }; let signature = crypto @@ -2532,7 +2585,7 @@ pub mod test { assert_matches!( &change_set[0], CanisterHttpChangeAction::AddToValidated(share, response) => { - assert_eq!(share.content.id, active_callback_id); + assert_eq!(share.content.id(), active_callback_id); assert_eq!(response.id, active_callback_id); } ); @@ -2628,12 +2681,12 @@ pub mod test { let expected_response = empty_canister_http_response(callback_id.get()); assert_eq!(*response, expected_response); - assert_eq!(share.content.id, callback_id); + assert_eq!(share.content.id(), callback_id); assert_eq!( - share.content.content_hash, - ic_types::crypto::crypto_hash(&expected_response) + share.content.content_hash(), + &ic_types::crypto::crypto_hash(&expected_response) ); - assert_eq!(share.content.registry_version, RegistryVersion::from(1)); + assert_eq!(share.content.registry_version(), RegistryVersion::from(1)); assert_eq!(share.signature.signer, replica_config.node_id); } else { panic!( @@ -2710,13 +2763,16 @@ pub mod test { let change_set = pool_manager.generate_change_set(&canister_http_pool); assert_eq!(change_set.len(), 0); - let response_metadata = CanisterHttpResponseMetadata { - id: CallbackId::from(7), - registry_version: RegistryVersion::from(1), - content_hash: CryptoHashOf::new(CryptoHash(vec![])), - content_size: 0, - is_reject: false, - replica_version: ReplicaVersion::default(), + let response_metadata = CanisterHttpResponseReceiptShare { + metadata: CanisterHttpResponseMetadata { + id: CallbackId::from(7), + registry_version: RegistryVersion::from(1), + content_hash: CryptoHashOf::new(CryptoHash(vec![])), + content_size: 0, + is_reject: false, + replica_version: ReplicaVersion::default(), + }, + payment_receipt: CanisterHttpPaymentReceipt::default(), }; let signature = crypto @@ -2874,13 +2930,16 @@ pub mod test { // 3. MALICIOUS ARTIFACT: Create a share that is signed by the `wrong_signer_id`. let response = empty_canister_http_response(callback_id.get()); - let response_metadata = CanisterHttpResponseMetadata { - id: callback_id, - registry_version: RegistryVersion::from(1), - content_hash: crypto_hash(&response), - content_size: response.content.count_bytes() as u32, - is_reject: false, - replica_version: ReplicaVersion::default(), + let response_metadata = CanisterHttpResponseReceiptShare { + metadata: CanisterHttpResponseMetadata { + id: callback_id, + registry_version: RegistryVersion::from(1), + content_hash: crypto_hash(&response), + content_size: response.content.count_bytes() as u32, + is_reject: false, + replica_version: ReplicaVersion::default(), + }, + payment_receipt: CanisterHttpPaymentReceipt::default(), }; let share = Signed { content: response_metadata.clone(), @@ -2973,13 +3032,16 @@ pub mod test { )); let response = empty_canister_http_response(callback_id.get()); - let response_metadata = CanisterHttpResponseMetadata { - id: callback_id, - registry_version: RegistryVersion::from(1), - content_hash: crypto_hash(&response), - content_size: response.content.count_bytes() as u32, - is_reject: false, - replica_version: ReplicaVersion::default(), + let response_metadata = CanisterHttpResponseReceiptShare { + metadata: CanisterHttpResponseMetadata { + id: callback_id, + registry_version: RegistryVersion::from(1), + content_hash: crypto_hash(&response), + content_size: response.content.count_bytes() as u32, + is_reject: false, + replica_version: ReplicaVersion::default(), + }, + payment_receipt: CanisterHttpPaymentReceipt::default(), }; let signature = crypto @@ -3046,7 +3108,8 @@ pub mod test { CanisterHttpPoolImpl::new(MetricsRegistry::new(), no_op_logger()); let mut bad_share = share.clone(); - bad_share.content.content_hash = CryptoHashOf::new(CryptoHash(vec![1, 2, 3])); + bad_share.content.metadata.content_hash = + CryptoHashOf::new(CryptoHash(vec![1, 2, 3])); canister_http_pool.insert(UnvalidatedArtifact { message: CanisterHttpResponseArtifact { @@ -3077,7 +3140,8 @@ pub mod test { CanisterHttpPoolImpl::new(MetricsRegistry::new(), no_op_logger()); let mut bad_share = share.clone(); - bad_share.content.content_size = bad_share.content.content_size.wrapping_add(1); + bad_share.content.metadata.content_size = + bad_share.content.metadata.content_size.wrapping_add(1); canister_http_pool.insert(UnvalidatedArtifact { message: CanisterHttpResponseArtifact { @@ -3108,7 +3172,7 @@ pub mod test { CanisterHttpPoolImpl::new(MetricsRegistry::new(), no_op_logger()); let mut bad_share = share.clone(); - bad_share.content.is_reject = !bad_share.content.is_reject; + bad_share.content.metadata.is_reject = !bad_share.content.is_reject(); canister_http_pool.insert(UnvalidatedArtifact { message: CanisterHttpResponseArtifact { @@ -3206,13 +3270,16 @@ pub mod test { content: CanisterHttpResponseContent::Success(vec![0; oversized_len]), }; - let response_metadata = CanisterHttpResponseMetadata { - id: callback_id, - registry_version: RegistryVersion::from(1), - content_hash: ic_types::crypto::crypto_hash(&response), - content_size: response.content.count_bytes() as u32, - is_reject: false, - replica_version: ReplicaVersion::default(), + let response_metadata = CanisterHttpResponseReceiptShare { + metadata: CanisterHttpResponseMetadata { + id: callback_id, + registry_version: RegistryVersion::from(1), + content_hash: ic_types::crypto::crypto_hash(&response), + content_size: response.content.count_bytes() as u32, + is_reject: false, + replica_version: ReplicaVersion::default(), + }, + payment_receipt: CanisterHttpPaymentReceipt::default(), }; let share = Signed { content: response_metadata.clone(), @@ -3270,13 +3337,16 @@ pub mod test { ]), }; - let response_metadata = CanisterHttpResponseMetadata { - id: callback_id, - registry_version: RegistryVersion::from(1), - content_hash: ic_types::crypto::crypto_hash(&response), - content_size: response.content.count_bytes() as u32, - is_reject: false, - replica_version: ReplicaVersion::default(), + let response_metadata = CanisterHttpResponseReceiptShare { + metadata: CanisterHttpResponseMetadata { + id: callback_id, + registry_version: RegistryVersion::from(1), + content_hash: ic_types::crypto::crypto_hash(&response), + content_size: response.content.count_bytes() as u32, + is_reject: false, + replica_version: ReplicaVersion::default(), + }, + payment_receipt: CanisterHttpPaymentReceipt::default(), }; let share = Signed { content: response_metadata.clone(), @@ -3389,15 +3459,136 @@ pub mod test { CanisterHttpChangeAction::AddToValidatedAndGossipResponse(share, response) => { let expected_response = empty_response; assert_eq!(*response, expected_response); - assert_eq!(share.content.id, callback_id); + assert_eq!(share.content.id(), callback_id); assert_eq!(share.signature.signer, replica_config.node_id); assert_eq!( - share.content.content_hash, - crypto_hash(&expected_response) + share.content.content_hash(), + &crypto_hash(&expected_response) ); } ); }); }); } + + /// If the refund reported in the payment receipt exceeds the per-replica + /// allowance configured on the request context, the share is rejected + /// as invalid: an honest replica cannot legitimately refund more than + /// its allowance. + #[test] + fn test_refund_greater_than_replica_allowance_is_invalid() { + ic_test_utilities::artifact_pool_config::with_test_pool_config(|pool_config| { + with_test_replica_logger(|log| { + let Dependencies { + pool, + replica_config, + crypto, + state_manager, + registry, + .. + } = dependencies(pool_config.clone(), 5); + let mut shim_mock = MockNonBlockingChannel::::new(); + shim_mock + .expect_try_receive() + .return_const(Err(TryReceiveError::Empty)); + + // Use a context with a small per-replica allowance. + let request = CanisterHttpRequestContext { + request: ic_test_utilities_types::messages::RequestBuilder::new().build(), + url: "".to_string(), + max_response_bytes: None, + headers: vec![], + body: None, + http_method: CanisterHttpMethod::GET, + transform: None, + time: ic_types::Time::from_nanos_since_unix_epoch(10), + replication: Replication::FullyReplicated, + pricing_version: PricingVersion::Legacy, + refund_status: RefundStatus { + refundable_cycles: Cycles::new(1000), + per_replica_allowance: Cycles::new(100), + refunded_cycles: Cycles::new(0), + refunding_nodes: BTreeSet::new(), + }, + }; + + state_manager + .get_mut() + .expect_get_latest_state() + .return_const(Labeled::new( + Height::from(1), + Arc::new(state_with_pending_http_calls(BTreeMap::from([( + CallbackId::from(0), + request, + )]))), + )); + + let mut canister_http_pool = + CanisterHttpPoolImpl::new(MetricsRegistry::new(), no_op_logger()); + + // Build a per-replica receipt share whose refund claim is + // larger than the per-replica allowance. + let receipt_share = CanisterHttpResponseReceiptShare { + metadata: CanisterHttpResponseMetadata { + id: CallbackId::from(0), + registry_version: RegistryVersion::from(1), + content_hash: CryptoHashOf::new(CryptoHash(vec![])), + content_size: 0, + is_reject: false, + replica_version: ReplicaVersion::default(), + }, + payment_receipt: CanisterHttpPaymentReceipt { + refund: Cycles::new(200), + }, + }; + let signature = crypto + .sign( + &receipt_share, + replica_config.node_id, + RegistryVersion::from(1), + ) + .unwrap(); + let share = Signed { + content: receipt_share, + signature, + }; + + canister_http_pool.insert(UnvalidatedArtifact { + message: CanisterHttpResponseArtifact { + share, + response: None, + }, + peer_id: replica_config.node_id, + timestamp: UNIX_EPOCH, + }); + + let shim: Arc> = + Arc::new(Mutex::new(Box::new(shim_mock))); + let pool_manager = CanisterHttpPoolManagerImpl::new( + state_manager as Arc<_>, + shim, + crypto, + pool.get_cache(), + replica_config, + SubnetType::Application, + Arc::clone(®istry) as Arc<_>, + MetricsRegistry::new(), + log, + ); + + let changes = pool_manager.validate_shares( + pool.get_cache().as_ref(), + &canister_http_pool, + Height::from(0), + ); + + assert_eq!(changes.len(), 1); + assert_matches!( + &changes[0], + CanisterHttpChangeAction::HandleInvalid(_, reason) + if reason == "Refund is greater than replica allowance" + ); + }) + }); + } } diff --git a/rs/interfaces/mocks/src/crypto.rs b/rs/interfaces/mocks/src/crypto.rs index 9673378ebd5b..a3708dda5674 100644 --- a/rs/interfaces/mocks/src/crypto.rs +++ b/rs/interfaces/mocks/src/crypto.rs @@ -25,7 +25,7 @@ use ic_interfaces::crypto::{ ThresholdEcdsaSigner, ThresholdSchnorrSigVerifier, ThresholdSchnorrSigner, ThresholdSigVerifier, ThresholdSigVerifierByPublicKey, ThresholdSigner, VetKdProtocol, }; -use ic_types::canister_http::CanisterHttpResponseMetadata; +use ic_types::canister_http::CanisterHttpResponseReceiptShare; use ic_types::consensus::{ BlockMetadata, CatchUpContent, CatchUpContentProtobufBytes, FinalizationContent, NotarizationContent, RandomBeaconContent, RandomTapeContent, @@ -299,8 +299,8 @@ mockall::mock! { ) -> CryptoResult>; pub fn sign_basic_http( - &self, message: &CanisterHttpResponseMetadata, - ) -> CryptoResult>; + &self, message: &CanisterHttpResponseReceiptShare, + ) -> CryptoResult>; pub fn sign_basic_query( &self, message: &QueryResponseHash, @@ -457,24 +457,24 @@ mockall::mock! { registry_version: RegistryVersion, ) -> CryptoResult<()>; - // CanisterHttpResponseMetadata + // CanisterHttpResponseReceiptShare pub fn verify_basic_sig_http( &self, - signature: &BasicSigOf, - message: &CanisterHttpResponseMetadata, signer: NodeId, + signature: &BasicSigOf, + message: &CanisterHttpResponseReceiptShare, signer: NodeId, registry_version: RegistryVersion, ) -> CryptoResult<()>; pub fn combine_basic_sig_http( &self, - signatures: BTreeMap>, + signatures: BTreeMap>, registry_version: RegistryVersion, - ) -> CryptoResult>; + ) -> CryptoResult>; pub fn verify_basic_sig_batch_http( &self, - signature_batch: &BasicSignatureBatch, - message: &CanisterHttpResponseMetadata, + signature_batch: &BasicSignatureBatch, + message: &CanisterHttpResponseReceiptShare, registry_version: RegistryVersion, ) -> CryptoResult<()>; @@ -482,8 +482,8 @@ mockall::mock! { &self, inputs: Vec<( NodeId, - BasicSigOf, - CanisterHttpResponseMetadata, + BasicSigOf, + CanisterHttpResponseReceiptShare, )>, registry_version: RegistryVersion, ) -> CryptoResult<()>; @@ -783,7 +783,7 @@ impl_basic_signer!(SignedIDkgDealing, sign_basic_signed_idkg_dealing); impl_basic_signer!(IDkgDealing, sign_basic_idkg_dealing); impl_basic_signer!(IDkgComplaintContent, sign_basic_idkg_complaint); impl_basic_signer!(IDkgOpeningContent, sign_basic_idkg_opening); -impl_basic_signer!(CanisterHttpResponseMetadata, sign_basic_http); +impl_basic_signer!(CanisterHttpResponseReceiptShare, sign_basic_http); impl_basic_signer!(QueryResponseHash, sign_basic_query); impl_basic_sig_verifier!( @@ -829,7 +829,7 @@ impl_basic_sig_verifier!( verify_basic_sig_batch_multi_msg_idkg_opening ); impl_basic_sig_verifier!( - CanisterHttpResponseMetadata, + CanisterHttpResponseReceiptShare, verify_basic_sig_http, combine_basic_sig_http, verify_basic_sig_batch_http, diff --git a/rs/interfaces/src/crypto.rs b/rs/interfaces/src/crypto.rs index 3510043c8687..4eaba2c007fd 100644 --- a/rs/interfaces/src/crypto.rs +++ b/rs/interfaces/src/crypto.rs @@ -1,7 +1,7 @@ //! The crypto public interface. mod keygen; -use ic_types::canister_http::CanisterHttpResponseMetadata; +use ic_types::canister_http::CanisterHttpResponseReceiptShare; pub use keygen::*; mod errors; @@ -80,8 +80,8 @@ pub trait Crypto: + ThresholdSchnorrSigVerifier + VetKdProtocol // CanisterHttpResponse - + BasicSigner - + BasicSigVerifier + + BasicSigner + + BasicSigVerifier // Signed Queries + BasicSigner // RequestId/WebAuthn @@ -141,8 +141,8 @@ impl Crypto for T where + BasicSigVerifier + BasicSigner + BasicSigVerifier - + BasicSigner - + BasicSigVerifier + + BasicSigner + + BasicSigVerifier + BasicSigner + IDkgProtocol + ThresholdEcdsaSigner diff --git a/rs/protobuf/def/types/v1/canister_http.proto b/rs/protobuf/def/types/v1/canister_http.proto index c2a21156a99b..f2587b029219 100644 --- a/rs/protobuf/def/types/v1/canister_http.proto +++ b/rs/protobuf/def/types/v1/canister_http.proto @@ -56,6 +56,7 @@ message CanisterHttpReject { message CanisterHttpResponseSignature { bytes signer = 1; bytes signature = 2; + CanisterHttpPaymentReceipt payment_receipt = 3; } message CanisterHttpResponseWithConsensus { diff --git a/rs/protobuf/src/gen/types/types.v1.rs b/rs/protobuf/src/gen/types/types.v1.rs index 552e975bb2e1..64f8f32a02b4 100644 --- a/rs/protobuf/src/gen/types/types.v1.rs +++ b/rs/protobuf/src/gen/types/types.v1.rs @@ -590,6 +590,8 @@ pub struct CanisterHttpResponseSignature { pub signer: ::prost::alloc::vec::Vec, #[prost(bytes = "vec", tag = "2")] pub signature: ::prost::alloc::vec::Vec, + #[prost(message, optional, tag = "3")] + pub payment_receipt: ::core::option::Option, } #[derive(Clone, PartialEq, ::prost::Message)] pub struct CanisterHttpResponseWithConsensus { diff --git a/rs/state_machine_tests/src/lib.rs b/rs/state_machine_tests/src/lib.rs index 08c3b8c7d055..bdb9051e4203 100644 --- a/rs/state_machine_tests/src/lib.rs +++ b/rs/state_machine_tests/src/lib.rs @@ -149,8 +149,9 @@ use ic_types::{ ValidationContext, XNetPayload, }, canister_http::{ - CanisterHttpRequestContext, CanisterHttpRequestId, CanisterHttpResponse, - CanisterHttpResponseContent, CanisterHttpResponseMetadata, + CanisterHttpPaymentReceipt, CanisterHttpRequestContext, CanisterHttpRequestId, + CanisterHttpResponse, CanisterHttpResponseContent, CanisterHttpResponseMetadata, + CanisterHttpResponseReceiptShare, }, consensus::{ block_maker::SubnetRecords, @@ -1867,7 +1868,7 @@ impl StateMachine { .unwrap() .get_validated_shares() .filter_map(|share| { - if inducted.contains(&share.content.id) { + if inducted.contains(&share.content.id()) { Some(CanisterHttpChangeAction::RemoveValidated(share.clone())) } else { None @@ -2700,19 +2701,22 @@ impl StateMachine { canister_id, content: content.clone(), }; - let response_metadata = CanisterHttpResponseMetadata { - id: CallbackId::from(request_id), - registry_version, - content_hash: ic_types::crypto::crypto_hash(&response), - content_size: content.count_bytes() as u32, - is_reject: content.is_reject(), - replica_version: ReplicaVersion::default(), + let receipt_share = CanisterHttpResponseReceiptShare { + metadata: CanisterHttpResponseMetadata { + id: CallbackId::from(request_id), + registry_version, + content_hash: ic_types::crypto::crypto_hash(&response), + content_size: content.count_bytes() as u32, + is_reject: content.is_reject(), + replica_version: ReplicaVersion::default(), + }, + payment_receipt: CanisterHttpPaymentReceipt::default(), }; let signature = CryptoReturningOk::default() - .sign(&response_metadata, node.node_id, registry_version) + .sign(&receipt_share, node.node_id, registry_version) .unwrap(); let share = Signed { - content: response_metadata, + content: receipt_share, signature, }; self.canister_http_pool.write().unwrap().apply(vec![ @@ -5115,7 +5119,7 @@ impl StateMachine { .read() .unwrap() .get_validated_shares() - .map(|share| share.content.id) + .map(|share| share.content.id()) .collect(); let state = self.state_manager.get_latest_state().take(); state diff --git a/rs/types/types/src/batch/canister_http.rs b/rs/types/types/src/batch/canister_http.rs index f22811d17448..049f622239b5 100644 --- a/rs/types/types/src/batch/canister_http.rs +++ b/rs/types/types/src/batch/canister_http.rs @@ -3,12 +3,13 @@ use crate::{ canister_http::{ CanisterHttpPaymentReceipt, CanisterHttpReject, CanisterHttpRequestId, CanisterHttpResponse, CanisterHttpResponseArtifact, CanisterHttpResponseContent, - CanisterHttpResponseDivergence, CanisterHttpResponseMetadata, CanisterHttpResponseShare, + CanisterHttpResponseDivergence, CanisterHttpResponseMetadata, CanisterHttpResponseProof, + CanisterHttpResponseReceiptShare, CanisterHttpResponseShare, CanisterHttpResponseSignature, CanisterHttpResponseWithConsensus, }, crypto::{BasicSig, BasicSigOf, CryptoHash, CryptoHashOf, Signed}, messages::CallbackId, - signature::{BasicSignature, BasicSignatureBatch}, + signature::BasicSignature, }; use ic_base_types::{NodeId, PrincipalId, RegistryVersion}; use ic_error_types::RejectCode; @@ -201,6 +202,10 @@ impl TryFrom for CanisterHttpPaymentReceipt { impl From for pb::CanisterHttpResponseWithConsensus { fn from(payload: CanisterHttpResponseWithConsensus) -> Self { + let CanisterHttpResponseProof { + metadata, + signatures, + } = payload.proof; pb::CanisterHttpResponseWithConsensus { response: Some(pb::CanisterHttpResponse { id: payload.content.id.get(), @@ -209,21 +214,19 @@ impl From for pb::CanisterHttpResponseWithCon )), canister_id: Some(pb::CanisterId::from(payload.content.canister_id)), }), - hash: payload.proof.content.content_hash.get().0, - registry_version: payload.proof.content.registry_version.get(), - replica_version: payload.proof.content.replica_version.into(), - signatures: payload - .proof - .signature - .signatures_map + hash: metadata.content_hash.get().0, + registry_version: metadata.registry_version.get(), + replica_version: metadata.replica_version.into(), + signatures: signatures .into_iter() - .map(|(signer, signature)| pb::CanisterHttpResponseSignature { + .map(|(signer, sig)| pb::CanisterHttpResponseSignature { signer: signer.get().into_vec(), - signature: signature.get().0, + signature: sig.signature.get().0, + payment_receipt: Some(sig.payment_receipt.into()), }) .collect(), - content_size: payload.proof.content.content_size, - is_reject: payload.proof.content.is_reject, + content_size: metadata.content_size, + is_reject: metadata.is_reject, } } } @@ -249,6 +252,22 @@ impl TryFrom for CanisterHttpResponseWith "CanisterHttpResponseWithConsensus::canister_id", )?; + let mut signatures = BTreeMap::new(); + for signature in payload.signatures { + let signer = NodeId::from(PrincipalId::try_from(signature.signer)?); + let payment_receipt = try_from_option_field( + signature.payment_receipt, + "CanisterHttpResponseSignature::payment_receipt", + )?; + signatures.insert( + signer, + CanisterHttpResponseSignature { + payment_receipt, + signature: BasicSigOf::new(BasicSig(signature.signature)), + }, + ); + } + Ok(CanisterHttpResponseWithConsensus { content: CanisterHttpResponse { id, @@ -258,8 +277,8 @@ impl TryFrom for CanisterHttpResponseWith "CanisterHttpResponseWithConsensus::content", )?, }, - proof: Signed { - content: CanisterHttpResponseMetadata { + proof: CanisterHttpResponseProof { + metadata: CanisterHttpResponseMetadata { id, content_hash: CryptoHashOf::::new(CryptoHash( payload.hash, @@ -270,18 +289,7 @@ impl TryFrom for CanisterHttpResponseWith replica_version: ReplicaVersion::try_from(payload.replica_version) .map_err(|err| ProxyDecodeError::ReplicaVersionParseError(Box::new(err)))?, }, - signature: BasicSignatureBatch { - signatures_map: payload - .signatures - .into_iter() - .map(|signature| { - Ok(( - NodeId::from(PrincipalId::try_from(signature.signer)?), - BasicSigOf::new(BasicSig(signature.signature)), - )) - }) - .collect::>, ProxyDecodeError>>()?, - }, + signatures, }, }) } @@ -354,18 +362,23 @@ impl TryFrom for CanisterHttpResponseContent { impl From for pb::CanisterHttpShare { fn from(share: CanisterHttpResponseShare) -> Self { + let CanisterHttpResponseReceiptShare { + metadata, + payment_receipt, + } = share.content; pb::CanisterHttpShare { metadata: Some(pb::CanisterHttpResponseMetadata { - id: share.content.id.get(), - content_hash: share.content.content_hash.clone().get().0, - registry_version: share.content.registry_version.get(), - replica_version: share.content.replica_version.into(), - content_size: share.content.content_size, - is_reject: share.content.is_reject, + id: metadata.id.get(), + content_hash: metadata.content_hash.get().0, + registry_version: metadata.registry_version.get(), + replica_version: metadata.replica_version.into(), + content_size: metadata.content_size, + is_reject: metadata.is_reject, }), signature: Some(pb::CanisterHttpResponseSignature { signer: share.signature.signer.get().into_vec(), - signature: share.signature.signature.clone().get().0, + signature: share.signature.signature.get().0, + payment_receipt: Some(payment_receipt.into()), }), } } @@ -378,21 +391,28 @@ impl TryFrom for CanisterHttpResponseShare { .metadata .ok_or(ProxyDecodeError::MissingField("share.metadata"))?; let id = CanisterHttpRequestId::new(metadata.id); - let content_hash = CryptoHashOf::new(CryptoHash(metadata.content_hash.clone())); + let content_hash = CryptoHashOf::new(CryptoHash(metadata.content_hash)); let registry_version = RegistryVersion::new(metadata.registry_version); let replica_version = ReplicaVersion::try_from(metadata.replica_version) .map_err(|err| ProxyDecodeError::ReplicaVersionParseError(Box::new(err)))?; let signature = share .signature .ok_or(ProxyDecodeError::MissingField("share.signature"))?; + let payment_receipt = try_from_option_field( + signature.payment_receipt, + "CanisterHttpResponseSignature::payment_receipt", + )?; Ok(Signed { - content: CanisterHttpResponseMetadata { - id, - content_hash, - content_size: metadata.content_size, - is_reject: metadata.is_reject, - registry_version, - replica_version, + content: CanisterHttpResponseReceiptShare { + metadata: CanisterHttpResponseMetadata { + id, + content_hash, + content_size: metadata.content_size, + is_reject: metadata.is_reject, + registry_version, + replica_version, + }, + payment_receipt, }, signature: BasicSignature { signer: NodeId::from(PrincipalId::try_from(signature.signer)?), @@ -611,19 +631,26 @@ mod tests { /// works correctly, both with and without a full response. #[test] fn canister_http_response_artifact_conversion() { + use ic_types_cycles::Cycles; + let signer = NodeId::from(PrincipalId::new_node_test_id(2)); let share = Signed { - content: CanisterHttpResponseMetadata { - id: CanisterHttpRequestId::new(2), - content_hash: CryptoHashOf::::new(CryptoHash(vec![ - 4, 5, 6, 7, - ])), - content_size: 42, - is_reject: false, - registry_version: RegistryVersion::new(2), - replica_version: ReplicaVersion::default(), + content: CanisterHttpResponseReceiptShare { + metadata: CanisterHttpResponseMetadata { + id: CanisterHttpRequestId::new(2), + content_hash: CryptoHashOf::::new(CryptoHash(vec![ + 4, 5, 6, 7, + ])), + content_size: 42, + is_reject: false, + registry_version: RegistryVersion::new(2), + replica_version: ReplicaVersion::default(), + }, + payment_receipt: CanisterHttpPaymentReceipt { + refund: Cycles::new(42), + }, }, signature: BasicSignature { - signer: NodeId::from(PrincipalId::new_node_test_id(2)), + signer, signature: BasicSigOf::new(BasicSig(vec![4, 5, 6, 7])), }, }; @@ -677,7 +704,7 @@ mod tests { // store the id separately in the metadata — it reuses the response's // value on deserialization. Normalize here so the roundtrip // comparison holds. - response.proof.content.id = response.content.id; + response.proof.metadata.id = response.content.id; let pb = pb::CanisterHttpResponseWithConsensus::from(response.clone()); let roundtripped = CanisterHttpResponseWithConsensus::try_from(pb).unwrap(); diff --git a/rs/types/types/src/canister_http.rs b/rs/types/types/src/canister_http.rs index 27a58178b3a5..ed3369a922a7 100644 --- a/rs/types/types/src/canister_http.rs +++ b/rs/types/types/src/canister_http.rs @@ -44,7 +44,7 @@ use crate::{ CanisterId, CountBytes, RegistryVersion, ReplicaVersion, Time, artifact::{CanisterHttpResponseId, IdentifiableArtifact, PbArtifact}, - crypto::{CryptoHashOf, Signed}, + crypto::{BasicSigOf, CryptoHashOf}, messages::{CallbackId, RejectContext, Request}, node_id_into_protobuf, node_id_try_from_protobuf, signature::*, @@ -67,7 +67,7 @@ use rand::RngCore; use rand::seq::IteratorRandom; use serde::{Deserialize, Serialize}; use std::{ - collections::BTreeSet, + collections::{BTreeMap, BTreeSet}, convert::{TryFrom, TryInto}, mem::size_of, time::Duration, @@ -1055,15 +1055,145 @@ impl CountBytes for CanisterHttpResponseMetadata { } } -impl crate::crypto::SignedBytesWithoutDomainSeparator for CanisterHttpResponseMetadata { +/// The content a single replica signs over: the shared +/// [`CanisterHttpResponseMetadata`] together with that replica's own +/// [`CanisterHttpPaymentReceipt`]. +/// +/// Including the receipt in the signed bytes binds the per-replica refund +/// claim to the response so it cannot be forged or altered after the +/// fact. +#[derive(Clone, Eq, PartialEq, Ord, PartialOrd, Hash, Debug, Deserialize, Serialize)] +#[cfg_attr(test, derive(ExhaustiveSet))] +pub struct CanisterHttpResponseReceiptShare { + pub metadata: CanisterHttpResponseMetadata, + pub payment_receipt: CanisterHttpPaymentReceipt, +} + +impl CountBytes for CanisterHttpResponseReceiptShare { + fn count_bytes(&self) -> usize { + let Self { + metadata, + payment_receipt, + } = self; + metadata.count_bytes() + payment_receipt.count_bytes() + } +} + +impl CanisterHttpResponseReceiptShare { + pub fn id(&self) -> CallbackId { + self.metadata.id + } + + pub fn content_hash(&self) -> &CryptoHashOf { + &self.metadata.content_hash + } + + pub fn content_size(&self) -> u32 { + self.metadata.content_size + } + + pub fn is_reject(&self) -> bool { + self.metadata.is_reject + } + + pub fn registry_version(&self) -> RegistryVersion { + self.metadata.registry_version + } + + pub fn replica_version(&self) -> &ReplicaVersion { + &self.metadata.replica_version + } + + pub fn refund(&self) -> Cycles { + self.payment_receipt.refund + } +} + +impl crate::crypto::SignedBytesWithoutDomainSeparator for CanisterHttpResponseReceiptShare { fn write_signed_bytes_without_domain_separator(&self, bytes: &mut Vec) { serde_cbor::to_writer(bytes, &self).unwrap(); } } -/// A signature share of of [`CanisterHttpResponseMetadata`]. -pub type CanisterHttpResponseShare = - Signed>; +/// A single signer's contribution to an aggregated proof: the +/// [`CanisterHttpPaymentReceipt`] that signer signed over, together with +/// their basic signature on the corresponding +/// [`CanisterHttpResponseReceiptShare`]. +#[derive(Clone, Eq, PartialEq, Hash, Debug, Deserialize, Serialize)] +#[cfg_attr(test, derive(ExhaustiveSet))] +pub struct CanisterHttpResponseSignature { + pub payment_receipt: CanisterHttpPaymentReceipt, + pub signature: BasicSigOf, +} + +impl CountBytes for CanisterHttpResponseSignature { + fn count_bytes(&self) -> usize { + let Self { + payment_receipt, + signature, + } = self; + payment_receipt.count_bytes() + signature.get_ref().count_bytes() + } +} + +/// An aggregated proof for a canister HTTP response with consensus. +/// +/// Holds the shared [`CanisterHttpResponseMetadata`] together with, for +/// each contributing signer, the [`CanisterHttpPaymentReceipt`] they +/// signed over and their basic signature (see +/// [`CanisterHttpResponseSignature`]). A validator reconstructs, for each +/// signer, the exact [`CanisterHttpResponseReceiptShare`] that signer +/// signed — the shared metadata plus that signer's receipt — and verifies +/// the individual basic signature. +#[derive(Clone, Eq, PartialEq, Hash, Debug, Deserialize, Serialize)] +#[cfg_attr(test, derive(ExhaustiveSet))] +pub struct CanisterHttpResponseProof { + pub metadata: CanisterHttpResponseMetadata, + pub signatures: BTreeMap, +} + +impl CountBytes for CanisterHttpResponseProof { + fn count_bytes(&self) -> usize { + let Self { + metadata, + signatures, + } = self; + metadata.count_bytes() + + signatures + .values() + .map(|s| std::mem::size_of::() + s.count_bytes()) + .sum::() + } +} + +impl CanisterHttpResponseProof { + pub fn id(&self) -> CallbackId { + self.metadata.id + } + + pub fn content_hash(&self) -> &CryptoHashOf { + &self.metadata.content_hash + } + + pub fn content_size(&self) -> u32 { + self.metadata.content_size + } + + pub fn is_reject(&self) -> bool { + self.metadata.is_reject + } + + pub fn registry_version(&self) -> RegistryVersion { + self.metadata.registry_version + } + + pub fn replica_version(&self) -> &ReplicaVersion { + &self.metadata.replica_version + } +} + +/// A signature share of [`CanisterHttpResponseReceiptShare`]. +pub type CanisterHttpResponseShare = BasicSigned; /// Contains a share and optionally the full response. /// @@ -1090,10 +1220,6 @@ impl PbArtifact for CanisterHttpResponseArtifact { type PbMessageError = ProxyDecodeError; } -/// A signature of of [`CanisterHttpResponseMetadata`]. -pub type CanisterHttpResponseProof = - Signed>; - #[cfg(test)] mod tests { use crate::{messages::NO_DEADLINE, time::UNIX_EPOCH}; diff --git a/rs/types/types/src/crypto/hash/tests.rs b/rs/types/types/src/crypto/hash/tests.rs index 27194a242a79..9ef72d679aea 100644 --- a/rs/types/types/src/crypto/hash/tests.rs +++ b/rs/types/types/src/crypto/hash/tests.rs @@ -126,6 +126,7 @@ mod crypto_hash_stability { use ic_crypto_test_utils_ni_dkg::ni_dkg_csp_dealing; use ic_crypto_tree_hash::{Digest, Witness}; use ic_protobuf::types::v1 as pb; + use ic_types_cycles::Cycles; use std::collections::BTreeMap; use std::sync::Arc; @@ -1086,6 +1087,7 @@ mod crypto_hash_stability { /// Test stability of CanisterHttpResponseShare hash output #[test] fn canister_http_response_share_stability() { + use crate::canister_http::{CanisterHttpPaymentReceipt, CanisterHttpResponseReceiptShare}; let metadata = CanisterHttpResponseMetadata { id: CallbackId::from(42), content_hash: test_crypto_hash_of(0x42), @@ -1094,8 +1096,14 @@ mod crypto_hash_stability { registry_version: RegistryVersion::from(1), replica_version: ReplicaVersion::default(), }; + let receipt_share = CanisterHttpResponseReceiptShare { + metadata, + payment_receipt: CanisterHttpPaymentReceipt { + refund: Cycles::from(42_u64), + }, + }; let data = Signed { - content: metadata, + content: receipt_share, signature: BasicSignature { signature: BasicSigOf::new(BasicSig(vec![0x42; 64])), signer: NodeId::from(PrincipalId::new_node_test_id(42)), @@ -1104,7 +1112,7 @@ mod crypto_hash_stability { let hash = crypto_hash(&data); assert_eq!( hex::encode(hash.get_ref().0.as_slice()), - "7bff0af6053ad0f648acffecf0434e299e9ce1d04b6752934935a13e390de986", + "a9372188df0e1057515013fa0208e8a6bf6aec7a5f5271c02585454c2bd32a2a", "Hash of CanisterHttpResponseShare changed" ); } diff --git a/rs/types/types/src/crypto/sign.rs b/rs/types/types/src/crypto/sign.rs index d4d886e29d69..cac73bcea83a 100644 --- a/rs/types/types/src/crypto/sign.rs +++ b/rs/types/types/src/crypto/sign.rs @@ -1,7 +1,7 @@ //! Defines signature types. use super::hash::domain_separator::DomainSeparator; -use crate::canister_http::CanisterHttpResponseMetadata; +use crate::canister_http::CanisterHttpResponseReceiptShare; use crate::consensus::{ BlockMetadata, CatchUpContent, CatchUpContentProtobufBytes, FinalizationContent, NotarizationContent, RandomBeaconContent, RandomTapeContent, @@ -70,7 +70,7 @@ mod private { impl SignatureDomainSeal for IDkgOpeningContent {} impl SignatureDomainSeal for WebAuthnEnvelope {} impl SignatureDomainSeal for Delegation {} - impl SignatureDomainSeal for CanisterHttpResponseMetadata {} + impl SignatureDomainSeal for CanisterHttpResponseReceiptShare {} impl SignatureDomainSeal for MessageId {} impl<'a> SignatureDomainSeal for SenderInfoContent<'a> {} impl SignatureDomainSeal for CertificationContent {} @@ -83,7 +83,7 @@ mod private { impl SignatureDomainSeal for VetKdEncryptedKeyShareContent {} } -impl SignatureDomain for CanisterHttpResponseMetadata { +impl SignatureDomain for CanisterHttpResponseReceiptShare { fn domain(&self) -> Vec { domain_with_prepended_length( DomainSeparator::CryptoHashOfCanisterHttpResponseMetadata.as_str(), diff --git a/rs/types/types/src/exhaustive.rs b/rs/types/types/src/exhaustive.rs index 59879ba83bdd..95d399fb41fe 100644 --- a/rs/types/types/src/exhaustive.rs +++ b/rs/types/types/src/exhaustive.rs @@ -2,6 +2,7 @@ use crate::artifact::IngressMessageId; use crate::batch::ChainKeyAgreement; +use crate::canister_http::CanisterHttpResponseSignature; use crate::consensus::dkg::RemoteDkgAttempts; use crate::consensus::hashed::Hashed; use crate::consensus::idkg::IDkgMasterPublicKeyId; @@ -601,7 +602,7 @@ impl ExhaustiveSet for Signed ExhaustiveSet for Signed> { +impl ExhaustiveSet for Signed> { fn exhaustive_set(rng: &mut R) -> Vec { let signatures_map: BTreeMap<_, _> = NodeId::exhaustive_set(rng) .into_iter() @@ -1030,6 +1031,7 @@ impl HasId for CertifiedStreamSlice {} impl HasId for RemoteDkgAttempts {} impl HasId for PreSignatureInCreation {} impl HasId for PreSignatureRef {} +impl HasId for CanisterHttpResponseSignature {} #[cfg(test)] mod tests {