Skip to content

fix: little fixes and tests for replicated canister log fetching#10665

Open
mraszyk wants to merge 10 commits into
masterfrom
mraszyk/replicated-canister-logs-fetch
Open

fix: little fixes and tests for replicated canister log fetching#10665
mraszyk wants to merge 10 commits into
masterfrom
mraszyk/replicated-canister-logs-fetch

Conversation

@mraszyk

@mraszyk mraszyk commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

This PR implements the following fix: separates the maximum size when computing a fetch_canister_logs response (checked w.r.t. estimated Rust in-memory data size) from the maximum Candid-encode size of the response (relevant for computing the cycles amount that must be attached to the call) - the latter is larger by the Candid-encoding overhead (approximated by 4KiB).

This PR also adds new tests:

  • unit tests of the fee formula;
  • checking that a call is rejected if too few cycles are attached (attaching just one cycle less than required to be as tight as possible);
  • checking that unused cycles are refunded exactly;
  • checking that the result is trimmed to be within the maximum result size and its Candid-encoded size does not exceed the corresponding constant accounting for up to 4KiB overhead.

@github-actions github-actions Bot added the feat label Jul 6, 2026
@mraszyk mraszyk changed the title feat: enable fetching canister logs via inter-canister calls fix: little fixes and tests for replicated canister log fetching Jul 7, 2026
@github-actions github-actions Bot added fix and removed feat labels Jul 7, 2026
@mraszyk mraszyk marked this pull request as ready for review July 7, 2026 12:00
@mraszyk mraszyk requested a review from a team as a code owner July 7, 2026 12:00
@zeropath-ai

zeropath-ai Bot commented Jul 7, 2026

Copy link
Copy Markdown

No security or compliance issues detected. Reviewed everything up to 7c95918.

Security Overview
Detected Code Changes
Change Type Relevant files
Enhancement ► rs/cycles_account_manager/src/cycles_account_manager/tests.rs
      Add tests for fetch_canister_logs_fee and fee scaling behavior
Enhancement ► rs/execution_environment/BUILD.bazel
      Add rs/limits to test suite dependencies
Enhancement ► rs/execution_environment/tests/canister_logging.rs
      Add cycles_account_manager helpers and subnet config setup; rename/adjust tests to reflect new fee logic; integrate max fee calculations in tests
Enhancement ► rs/replicated_state/src/canister_state/system_state/log_memory_store/ring_buffer.rs
      Use MAX_FETCH_CANISTER_LOGS_RESULT_BYTES to define RESULT_MAX_SIZE and document reasoning
Enhancement ► rs/replicated_state/src/canister_state/system_state/log_memory_store/tests/log_memory_store.rs
      Update imports to include FetchCanisterLogsResponse, Payload and related types; use NEW constants for limits and response handling; extend tests to validate limit-related behavior and response sizing
Enhancement ► rs/types/types/src/canister_log.rs
      Add MAX_FETCH_CANISTER_LOGS_RESULT_BYTES and MAX_FETCH_CANISTER_LOGS_RESPONSE_BYTES; document relationship to result size and framing; include validation consts

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant