Skip to content

Commit b71cc84

Browse files
authored
fix: Don't count flexible HTTP outcall timeouts towards maximum responses during validation (#10677)
To limit the effect on the finalization rate, we limit the number of (non-timeout) HTTP outcalls responses that are allowed to be included into a single payload. https://github.com/dfinity/ic/blob/38556a3a0f8445310a9fd7323e28e6fb2eae2d30/rs/types/types/src/canister_http.rs#L81-L85 The exception are timeout responses, which are very cheap to verify. Therefore, they do not count towards this limit during payload building: https://github.com/dfinity/ic/blob/38556a3a0f8445310a9fd7323e28e6fb2eae2d30/rs/https_outcalls/consensus/src/payload_builder.rs#L206-L227 However, before this PR, flexible timeouts did count towards the limit during payload validation. A honest block maker could therefore generate a payload with more the 500 flexible timeouts, which would be rejected by other honest block makers. Note that the flexible outcalls feature isn't enabled yet.
1 parent c3da11f commit b71cc84

2 files changed

Lines changed: 62 additions & 1 deletion

File tree

rs/https_outcalls/consensus/src/payload_builder/tests.rs

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -420,6 +420,64 @@ fn timeouts_bypass_max_responses_per_block() {
420420
);
421421
}
422422

423+
#[test]
424+
fn flexible_timeouts_bypass_max_responses_per_block() {
425+
let subnet_size = 4;
426+
let num_contexts = CANISTER_HTTP_MAX_RESPONSES_PER_BLOCK + 50;
427+
428+
test_config_with_http_feature(
429+
true,
430+
subnet_size,
431+
|mut payload_builder, _canister_http_pool| {
432+
let committee: BTreeSet<_> = (0..subnet_size as u64).map(node_test_id).collect();
433+
434+
// `num_contexts` flexible request contexts, all at time = UNIX_EPOCH.
435+
let contexts: Vec<_> = (0..num_contexts as u64)
436+
.map(|id| {
437+
(
438+
CallbackId::new(id),
439+
flexible_request_context(committee.clone(), 1, subnet_size as u32),
440+
)
441+
})
442+
.collect();
443+
inject_request_contexts(&mut payload_builder, contexts);
444+
445+
// Validation time past the timeout interval so every context times out.
446+
let validation_context = ValidationContext {
447+
registry_version: RegistryVersion::new(1),
448+
certified_height: Height::new(0),
449+
time: UNIX_EPOCH + CANISTER_HTTP_TIMEOUT_INTERVAL + Duration::from_secs(1),
450+
};
451+
452+
let payload = payload_builder.build_payload(
453+
Height::new(1),
454+
TEST_MAX_PAYLOAD_BYTES,
455+
&[],
456+
&validation_context,
457+
);
458+
459+
let parsed = bytes_to_payload(&payload).expect("Failed to parse payload");
460+
461+
// The builder emits all timed-out flexible requests, ungated by the cap.
462+
assert_eq!(parsed.flexible_errors.len(), num_contexts);
463+
assert!(parsed.responses.is_empty());
464+
assert!(parsed.timeouts.is_empty());
465+
// Flexible timeouts must not count against the per-block response cap.
466+
assert_eq!(parsed.num_non_timeout_responses(), 0);
467+
468+
// The builder's own honest payload must pass validation.
469+
payload_builder
470+
.validate_payload(
471+
Height::new(1),
472+
&test_proposal_context(&validation_context),
473+
&payload,
474+
&[],
475+
)
476+
.unwrap();
477+
},
478+
);
479+
}
480+
423481
/// Divergence responses must be counted by num_non_timeout_responses() and
424482
/// therefore be subject to the CANISTER_HTTP_MAX_RESPONSES_PER_BLOCK limit
425483
/// during validation.

rs/types/types/src/batch/canister_http.rs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -174,7 +174,10 @@ impl CanisterHttpPayload {
174174
responses.len()
175175
+ divergence_responses.len()
176176
+ flexible_responses.len()
177-
+ flexible_errors.len()
177+
+ flexible_errors
178+
.iter()
179+
.filter(|error| !matches!(error, FlexibleCanisterHttpError::Timeout { .. }))
180+
.count()
178181
}
179182

180183
/// Returns true, if this is an empty payload

0 commit comments

Comments
 (0)