fix: CanisterHttpPoolImpl::get() serves full response for flexible & non-replicated requests

[eichhorl]

Background

For fully replicated HTTP outcalls requests, the gossiped shares include only the response hash, because each replica determines the HTTP response content by making the outcall themselves.

For flexible & non-replicated requests, not all replicas make the HTTP outcall. Yet, all replicas should still know the results of all outcalls, such that all replicas can potentially include them in a block. Therefore, the gossiped responses also include the full HTTP response content. Otherwise, a replica making a non-replicated request (where only one replica makes the outcall), would need to wait 1-13 blocks (13 node subnet) until it is the blockmaker to include the response in a block.

Additionally, HTTP outcalls shares are not relayed, or advertised. They are pushed to all peers immediately (is_latency_sensitive: true).

Problem

Because HTTP outcalls shares are pushed without adverts, the code before this PR assumed that CanisterHttpPoolImpl::get() would never actually be called (get() is usually how a node returns an artifact that is requested / "pulled" by other nodes, i.e. when resolving an advert). Because of this assumption, get() never returned the full response content, and instead set it to None for simplicity:

ic/rs/artifact_pool/src/canister_http_pool.rs

Lines 194 to 207 in 5ee93a4

	fn get(&self, id: &CanisterHttpResponseId) -> Option<CanisterHttpResponseArtifact> {
	// Important: this is actually never used, as Http artifacts are always sent directly (no adverts).
	// If we ever decide to use adverts, we should make the distinction between artifacts with or without a
	// response. We should either:
	// - only use adverts when gossiping a full response; this way, this should always return the response
	// - store a flag in the share which says whether the full response needs to be gossiped or not.
	// This is to avoid sending the response in full in the fully replicated case.
	self.validated
	.get(id)
	.map(|()| CanisterHttpResponseArtifact {
	share: id.clone(),
	response: None,
	})
	}

However, this assumption is incorrect due to the following reason: When a node receives an HTTP outcalls share, it calls its "bouncer" to see if the artifact is Wanted, Unwanted or MaybeWantsLater:

ic/rs/https_outcalls/consensus/src/gossip.rs

Lines 58 to 67 in 478eb83

	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 {
	BouncerValue::MaybeWantsLater
	} else {
	BouncerValue::Unwanted
	}

If the bouncer returns MaybeWantsLater, then P2P will only stash the advert, and take out the (larger) artifact content in order to conserve memory, even if the advert was pushed together with its content:

ic/rs/p2p/artifact_downloader/src/fetch_artifact/download.rs

Lines 180 to 184 in 478eb83

	// Clear the artifact from memory if it was pushed.
	if let BouncerValue::MaybeWantsLater = bouncer_value {
	artifact.take();
	metrics.download_task_stashed_total.inc();
	}

If at a later point, the bouncer value switches to Wants, then the advert needs to be resolved from scratch, meaning the artifact content is downloaded from a peer which calls CanisterHttpPoolImpl::get().

In case of non-replicated or flexible outcalls, due to the assumption above, the contacted peer will then serve the artifact without the HTTP response content. The HTTP share without response content is unexpected for non-replicated and flexible requests, meaning the receiving peer will invalidate it.

Proposed Changes

This PR fixes the problem by recording in the pool (at insertion time), if the response content should be served via get() or not. This will allow nodes to correctly fetch shares for flexible or non-replicated requests (including the response content) from other nodes.

[zeropath-ai]

✅ No security or compliance issues detected. Reviewed everything up to 478eb83.

Security Overview

• 🔎 Scanned files: 1 changed file(s)
• 🔗 Scan Link: https://zeropath.com/app/repositories/f5e9d2bb-29a4-4b64-902a-391d65fd8b2d?scanId=ba268a55-1cc2-405c-bd6a-27c1b7297b7d&codeScanTypes=PrScan&tab=issues

Detected Code Changes

Change Type

Relevant files

Enhancement

► rs/artifact_pool/src/canister_http_pool.rs     Introduce ServedResponse enum and implement label, with Include/Exclude variants for serving responses in validated pool ► rs/artifact_pool/src/canister_http_pool.rs     Track served state in ValidatedCanisterHttpPoolSection using ServedResponse instead of unit type ► rs/artifact_pool/src/canister_http_pool.rs     Update insert logic to store ServedResponse::Include or ServedResponse::Exclude accordingly ► rs/artifact_pool/src/canister_http_pool.rs     Update MoveToValidated handling to set ServedResponse based on presence of response content ► rs/artifact_pool/src/canister_http_pool.rs     Update get() to return response content only when ServedResponse::Include, otherwise None ► rs/artifact_pool/src/canister_http_pool.rs     Add tests scaffolding for new ServedResponse behavior (e.g., get_serves_response_only_for_non_fully_replicated_shares)