Proposal to elect new release rc--2025-01-16_16-18

Hello there!

We are happy to announce that voting is now open for a new IC release.
The NNS proposal is here: IC NNS Proposal 134900.

Here is a summary of the changes since the last release:

Release Notes for release-2025-01-16_16-18-base (233c1ee2ef68c1c8800b8151b2b9f38e17b8440a)

This release is based on changes since release-2025-01-09_03-19-base (aa705aaa621c2e0d4f146f3a1de801edcb0fa0d5).

Please note that some commits may be excluded from this release if they’re not relevant, or not modifying the GuestOS image. Additionally, descriptions of some changes might have been slightly modified to fit the release notes format.

To see a full list of commits added since last release, compare the revisions on GitHub.

Features:

  • 572afbcdb Crypto,Interface,Node(node): add fstrim datadir observability (#3322)
  • c4739e9ad Execution,Interface: Use Wasmtime deserialize_open_file. (#3412)
  • 760e1f764 Execution,Interface,Message Routing: Implement MutableIntMap (#3303)
  • c16efb073 Interface: Limit cache disk space (#3366)
  • fb3fc37ae Owners(icrc-ledger-types): Add default encoding and decoding of a Principal in a Subaccount. (#3459)
  • 19e3c685a Node: Add log-config service to GuestOS (#3389)

Bugfixes:

  • 80c6776c1 Consensus,Interface: Consider all fields in is_empty and count_bytes implementation for BatchPayload (#3471)
  • cf2f2cc97 Consensus,Interface: Make the consensus manager API resemble a classic channel interface (#3233)
  • 57047d6d8 Execution,Interface: Low Wasm memory hook will run once the canister’s unfrozen if it was scheduled before freezing (#3455)
  • 38a497106 Execution,Interface: Ignore empty data segment pages (#3435)
  • 4e0a28df6 Execution,Interface: Account instructions for bytes accessed and dirty pages on system subnets (#3396)
  • f9f2491d3 Interface,Message Routing: mocked xnet in PocketIC (#3376)
  • 233c1ee2e Node: deployments when no ipv4 or domain name provided (#3477)

Performance improvements:

  • 69981dc71 Execution,Interface,Message Routing: Use MutableIntMap in SystemState (#3304)

Chores:

  • 5cce4f5cb Consensus,Interface: Split the user ingress artifacts and the artifacts coming from P2P (#3419)
  • 4a7957ba6 Consensus,Interface: Log HTTP request body on signature verification failure (#3239)
  • 8054acfac Execution,Interface: upgrade wasmtime to v.28 (#3330)
  • 111bac358 Interface,Message Routing: state-tool copy commands (#3440)
  • dce918ac8 Owners(IDX): bump rules_rust (#3449)
  • 1f1d8dd8c Node: Update Base Image Refs [2025-01-16-0807] (#3466)
  • 85af5fc7b Node: Update Base Image Refs [2025-01-16-0145] (#3463)
  • 145aff3e5 Node(nftables): update IPv6 prefix list in the HostOS firewall (#3414)
  • 6704c1438 Node: Update Base Image Refs [2025-01-09-0807] (#3374)
  • ba5e99bf1 Node(IDX): don’t cache ic-os images (#3256)

Refactoring:

  • f491f848c Consensus,Interface(consensus): simplify IngressPayload implementation (#3444)
  • 86357ae40 Execution,Interface,Message Routing: Use test_strategy over proptest macro in replicated state tests (#3462)
  • 6da5c715e Interface: move serve_journal into upgrade_journal.rs (#3393)

Tests:

  • df2145592 Consensus,Interface(consensus): move the IngressPayload serialization/deserialization unit test from test_utilities/types/ to rs/types/types (#3384)
  • 6b7b92b24 Interface(ICRC_Ledger): Verify ICRC ledger and archive block equality (#3404)
  • d6bb598cf Interface(ICRC_Ledger): canbench benchmarks for icrc2_approve, icrc2_transfer_from and icrc3_get_blocks (#3400)

Full list of changes (including the ones that are not relevant to GuestOS) can be found on GitHub.

IC-OS Verification

To build and verify the IC-OS disk image, run:

# From https://github.com/dfinity/ic#verifying-releases
sudo apt-get install -y curl && curl --proto '=https' --tlsv1.2 -sSLO https://raw.githubusercontent.com/dfinity/ic/233c1ee2ef68c1c8800b8151b2b9f38e17b8440a/ci/tools/repro-check.sh && chmod +x repro-check.sh && ./repro-check.sh -c 233c1ee2ef68c1c8800b8151b2b9f38e17b8440a --guestos

The two SHA256 sums printed above from a) the downloaded CDN image and b) the locally built image, must be identical, and must match the SHA256 from the payload of the NNS proposal.

While not required for this NNS proposal, as we are only electing a new GuestOS version here, you have the option to verify the build reproducibility of the HostOS by passing --hostos to the script above instead of --guestos, or the SetupOS by passing --setupos.

2 Likes

proposal - 134900 - Cyberowl | CodeGov

Vote: ADOPT

Reason:

All commits match their description and no errors were found. The verification build hash also matches release_package_sha256_hex.

Hash Match: MATCH

2 Urls: MATCH

Feedback: NONE

Proposer Check: MATCH

Overall Summary:

These commits introduce features such as reading pre-compiled Wasm modules from disk, a new MemoryDiskBytes trait for refined memory/disk usage tracking, and a generalized patricia tree-based IntMap. They also unify logging configurations across OS environments, remove the special zero overhead for dirty pages on system subnets, and add a “copy” feature to the state-tool utility to streamline state checkpoint handling. Additionally, they upgrade dependencies Wasmtime to 28.0.0 and enhance test frameworks for broader coverage.

Commits Summary

572afbcdb
This commit adds separate tracking for fstrim runs on the data directory by introducing new fields (e.g., last_duration_milliseconds_datadir). It updates the parser, writer, and tests to handle these additional metrics alongside the existing ones. The changes ensure that both the main target and datadir target have their own metrics counters and success indicators without interfering with each other.

c4739e9ad
New branch in get_embedder_cache to handle reading pre-compiled Wasm modules from disk. It introduces StoredCompilation::Disk and leverages a new read_file_and_pre_instantiate method in WasmtimeEmbedder for non-sandboxed scenarios. This ensures that file-based modules can be deserialized and cached similarly to memory-based ones.

760e1f764
Generalizes the patricia tree-based IntMap from a fixed 64-bit integer key to a generic interface (AsInt) supporting custom integer-like types (e.g., u64 and u128). It introduces a MutableIntMap variant for in-place modifications. References to page indexes now use typed keys PageIndex

c16efb073
New MemoryDiskBytes trait that tracks both in-memory size and on-disk size for cached items. It replaces the older CountBytes trait and updates various data structures (like the LRU cache) to handle separate capacities for memory and disk.

fb3fc37ae
Two new methods to the ICRC Ledger. principal_to_subaccount can be used to create a separate Subaccount for each Principal. And subaccount_to_principal can be used to create a separate Principal for each Subaccount.

19e3c685a
Unifies and simplifies the logging of configuration partitions across both host OS and guest OS. It moves the shared log-config script files into a common directory (misc/log-config) and adjusts the relevant systemd service unit files to point to these consolidated paths. A key change is switching from logging a config.ini file plus deployment.json to instead reading from a single config.json file.

80c6776c1
More explicit field destructuring and adds new is_empty() checks for various data types in order to better clarify code intentions and reduce mistakes. It updates existing count_bytes() methods so that they rely on field destructuring, making it easier to understand which parts contribute to the byte count.

cf2f2cc97
Replaces multiple per-artifact broadcast channels with a standardized “abortable broadcast” channel builder approach. It introduces a single abortable_broadcast_channel method to create both the outbound (bounded) and inbound (unbounded) channels for each artifact type, cleaning up repetitive logic. The main motivation is to consolidate how artifacts flow between the processing threads and to enforce consistent channel sizing and error-handling.

57047d6d8
Adds logic to remove the hook from the canister’s task queue and reinsert it in a “ready” state so the hook can be run once the canister is unfrozen.

38a497106
Matches description of avoid triggering our critical error alert in the case where a module has empty data segments.

4e0a28df6
Removes the special zero overhead for dirty pages on system subnets, making them match the standard dirty page cost used for application subnets. Now, all subnets pay the same instruction cost for creating or modifying dirty pages.

f9f2491d3
New PocketXNetImpl struct and a shared CertifiedSlicePool. It adds the refill_stream_slice_indices function, which computes the correct message and byte limits for partial or complete stream segments, and then updates the PoolRefillTask to use this new logic.

233c1ee2e
Checks for "null" before validating the domain or setting up IPv4 network, skipping those steps when corresponding config fields are missing or explicitly null.

69981dc71
Replaces various BTreeMap and BTreeSet data structures with the new MutableIntMap type to store and track callbacks, call contexts, and queue references. It updates insertions, removals, and lookups accordingly to leverage the custom map’s integer-key optimizations.

5cce4f5cb
tokio-stream to version 0.1.17. Replaces multiple unbounded channels with combined tokio_stream merges, so the artifact processor now batches messages from potentially multiple inbound sources. A new read_batch function handles timeouts and end-of-stream behavior.

4a7957ba6
validation_error_to_http_error function can log additional request context, particularly in the case of invalid signatures. Instead of just passing in a message ID, it now receives and logs the full HttpRequest object when validation fails.

8054acfac
Upgrade Wasmtime to 28.0.0. Simplifies the handling of the counter for benchmark iterations and modifies the iteration logic for running benchmarks.

111bac358
Adds a new “copy” feature to the state-tool utility, allowing users to migrate or duplicate state checkpoints from one directory to another. It supports copying all checkpoints, just the latest one, or a specific set of heights. It now calls StateLayout::copy_and_sync_checkpoint directly instead of going through a scratchpad.

dce918ac8
Updates the Bazel workspace to use a newer version of rules_rust. Removes the existing patch file (rules_rust.patch) and adjusts the relevant download and checksum information.

1f1d8dd8c
Updates the base Docker images boundary-guestos, guestos, hostos, and setupos.

85af5fc7b
Updates the base Docker images boundary-guestos, guestos, hostos, and setupos.

145aff3e5
Updated hostos nftables to remove -old elements and add SH1.

6704c1438
Updated hostos nftables to remove -old elements and add SH1.

ba5e99bf1
Updates Bazel build rules by adding the “no-cache” tag alongside the existing “manual” tag for multiple build targets in defs.bzl and various BUILD.bazel files.

f491f848c
Refactor the way the code stores and retrieves ingress messages, using a map keyed by IngressMessageId instead of index-based access. This provides clearer error handling, makes message lookups more direct.

86357ae40
Updates the testing approach by replacing proptest! macros with the #[test_strategy::proptest] attribute and associated #[strategy(...)] annotations, improving test readability and maintainability. Refactors the IngressPayload structure to use a BTreeMap for more efficient message management.

6da5c715e
Refactors the serve_journal function by changing its parameter from only the journal entries to the entire UpgradeJournal object. By moving the serve_journal function from a shared library into upgrade_journal.rs within the SNS Governance crate and making it accept the specific UpgradeJournal type, the code ensures the function is only used in its intended context.

df2145592
Remove the assert_matches dependency from the ic-test-utilities-types crate. Delete the old test code in ingress_payload.rs within the test utilities module. The new tests in ingress.rs within the ic/types module replicate and extend the original functionality, ensuring messages are correctly serialized and deserialized.

6b7b92b24
New test that verifies the consistency of the Block type across the ledger and archive .did files.

d6bb598cf
Expand the performance benchmarks for the ICRC ledger by adding new tests for icrc2_approve, icrc2_transfer_from, and icrc3_get_blocks. Please note that icrc2_approve got refactored and icrc2_approve_not_async is now used as helper function and accepts caller as an arg. This is secure because it removes candid update.

About CodeGov…(click to expand)

CodeGov has a team of developers who review and vote independently on the following proposal topics: IC-OS Version Election, Protocol Canister Management, Subnet Management, Node Admin, and Participant Management. The CodeGov NNS known neuron is configured to follow our reviewers on these topics and Synapse on most other topics. We strive to be a credible and reliable Followee option that votes on every proposal and every proposal topic in the NNS. We also support decentralization of SNS projects such as WaterNeuron and KongSwap with a known neuron and credible Followees.

Learn more about CodeGov and it’s mission at codegov.org.

2 Likes

Proposal 134900 - HPeebles | CodeGov

Vote: Adopt

Reason:

In my opinion all commits listed look fine and match their descriptions. Unfortunately I was again unable to successfully run the build verification script (it said “When not specifying any option please run this script inside an IC git repository”, even though I was running it in a directory where I’d cloned a fresh copy of the IC repo), but given that others have done so successfully I am happy to proceed with giving an approval.

Full review:

Features:

  • 572afbcdb Crypto,Interface,Node(node): add fstrim datadir observability (#3322)
    Review: Looks fine + matches description
    Notes: Exposes metrics from each node showing the details of fstrim executions.

  • c4739e9ad Execution,Interface: Use Wasmtime deserialize_open_file. (#3412)
    Review: Looks fine + matches description
    Notes: Uses the new Module::deserialize_open_file within deserialize_from_file which avoids having to copy the wasm’s bytes into the sandbox. Also implements loading wasms from the on disk compilation cache.

  • 760e1f764 Execution,Interface,Message Routing: Implement MutableIntMap (#3303)
    Review: Looks fine + matches description
    Notes: Makes IntMap generic over u64 or u128, modifies insert so that it returns the previous value at the inserted key, and extends it with a few new methods (remove and split). Then uses the same underlying Tree type to implement MutableIntMap which is exactly the same as IntMap but allows each instance to be mutated rather than having each mutable operation consume the map and return a new one.

  • c16efb073 Interface: Limit cache disk space (#3366)
    Review: Looks fine + matches description
    Notes: Supports setting memory_capacity and disk_capacity when creating an LRU cache, then uses these limits after inserting new items to evict existing items as necessary.

  • fb3fc37ae Owners(icrc-ledger-types): Add default encoding and decoding of a Principal in a Subaccount. (#3459)
    Review: Looks fine + matches description
    Notes: Adds principal_to_subaccount and subaccount_to_principal to icrc-ledger-types which makes it easy to create a unique subaccount for each principal.

  • 19e3c685a Node: Add log-config service to GuestOS (#3389)
    Review: Looks fine + matches description
    Notes: Adds the new log-config service to GuestOS to log the configuration.

Bugfixes:

  • 80c6776c1 Consensus,Interface: Consider all fields in is_empty and count_bytes implementation for BatchPayload (#3471)
    Review: Looks fine + matches description
    Notes: Includes all fields are taken into account when calculating is_empty and count_bytes on a few types. Ensures newly added fields will be handled by first deconstructing the input values.

  • cf2f2cc97 Consensus,Interface: Make the consensus manager API resemble a classic channel interface (#3233)
    Review: Looks fine + matches description
    Notes: Refactors run_artifact_processor, create_ingress_handlers and create_artifact_handler to each take as input the sending and receiving channels rather than only taking the sender and creating the receiver internally. Also renames ConsensusManagerBuilder to AbortableBroadcastChannelBuilder.

  • 57047d6d8 Execution,Interface: Low Wasm memory hook will run once the canister’s unfrozen if it was scheduled before freezing (#3455)
    Review: Looks fine + matches description
    Notes: Requeues the OnLowWasmMemory task if it attempts to execute but the canister is frozen, this ensures it will run once the canister is unfrozen.

  • 38a497106 Execution,Interface: Ignore empty data segment pages (#3435)
    Review: Looks fine + matches description
    Notes: Within Segments::as_pages, which returns heap data as wasm pages, ignore empty segments rather than including them as pages where the bytes are all zero.

  • 4e0a28df6 Execution,Interface: Account instructions for bytes accessed and dirty pages on system subnets (#3396)
    Review: Looks fine + matches description
    Notes: Updates the SchedulerConfig for system subnets to set the dirty_page_overhead to DEFAULT_DIRTY_PAGE_OVERHEAD rather than it being 0 instructions.

  • f9f2491d3 Interface,Message Routing: mocked xnet in PocketIC (#3376)
    Review: Looks fine + matches description
    Notes: Fixes an error case within PocketIC by reusing refill_stream_slice_indices from within the xnet::payload_builder rather than

  • 233c1ee2e Node: deployments when no ipv4 or domain name provided (#3477)
    Review: Looks fine + matches description
    Notes: Fixes the check-network.sh script to only run validate_domain_name if a domain name is provided and to only run setup_ipv4_network and ping_ipv4_gateway if an IPv4 address is provided.

Performance improvements:

  • 69981dc71 Execution,Interface,Message Routing: Use MutableIntMap in SystemState (#3304)
    Review: Looks fine + matches description
    Notes: Replaces a few BTreeMaps and BTreeSets with the new MutableIntMap because it is far quicker to clone (which more than makes up for it being slower when performing other operations).

Chores:

  • 5cce4f5cb Consensus,Interface: Split the user ingress artifacts and the artifacts coming from P2P (#3419)
    Review: Looks fine + matches description
    Notes: Splits incoming messages into user_ingress_rx and inbound_rx, makes run_artifact_processor generic over a stream of artifacts, then merges the incoming messages into a single stream which is passed to run_artifact_processor.

  • 4a7957ba6 Consensus,Interface: Log HTTP request body on signature verification failure (#3239)
    Review: Looks fine + matches description
    Notes: Updates validation_error_to_http_error to include the full request in the log message rather than only including the messageId.

  • 8054acfac Execution,Interface: upgrade wasmtime to v.28 (#3330)
    Review: Looks fine + matches description
    Notes: Bumps wasmtime from version 27.0.0 to 28.0.0 and also adds a readme to the execution environment benchmarks.

  • 111bac358 Interface,Message Routing: state-tool copy commands (#3440)
    Review: Looks fine + matches description
    Notes: Adds the new copy command to the state tool which copies checkpoints and metadata from a chosen source to a destination. Also updates the import_state command to reuse the copy code.

  • dce918ac8 Owners(IDX): bump rules_rust (#3449)
    Review: Looks fine + matches description
    Notes: Bumps rules_rust to 0.54.1, removing the need for a patched version to be applied.

  • 1f1d8dd8c Node: Update Base Image Refs [2025-01-16-0807] (#3466)
    Review: Looks fine + matches description
    Notes: Update base IC-OS image references.

  • 85af5fc7b Node: Update Base Image Refs [2025-01-16-0145] (#3463)
    Review: Looks fine + matches description
    Notes: Update base IC-OS image references.

  • 145aff3e5 Node(nftables): update IPv6 prefix list in the HostOS firewall (#3414)
    Review: Looks fine + matches description
    Notes: Removes 3 old datacenters from the IPv6 prefix list in the firewall + adds the new SH1 datacenter.

  • 6704c1438 Node: Update Base Image Refs [2025-01-09-0807] (#3374)
    Review: Looks fine + matches description
    Notes: Update base IC-OS image references.

  • ba5e99bf1 Node(IDX): don’t cache ic-os images (#3256)
    Review: Looks fine + matches description
    Notes: Adds the no-cache tag to IC-OS image builds so that the images are not cached.

Refactoring:

  • f491f848c Consensus,Interface(consensus): simplify IngressPayload implementation (#3444)
    Review: Looks fine + matches description
    Notes: Updates IngressPayload to contain the serialized ingress messages rather than being a single buffer alongside the ids and indexes of each message. This makes it far simpler to deal with and the increased serialization cost is tiny.

  • 86357ae40 Execution,Interface,Message Routing: Use test_strategy over proptest macro in replicated state tests (#3462)
    Review: Looks fine + matches description
    Notes: Switches replicated_state tests to use #[test_strategy::proptest] rather than the proptest! macro.

  • 6da5c715e Interface: move serve_journal into upgrade_journal.rs (#3393)
    Review: Looks fine + matches description
    Notes: Moves the serve_journal function out of ic-nervous-system-common and into ic-sns-governance since it is only used by the SNS governance canister.

Tests:

  • df2145592 Consensus,Interface(consensus): move the IngressPayload serialization/deserialization unit test from test_utilities/types/ to rs/types/types (#3384)
    Review: Looks fine + matches description
    Notes: Moves the test_ingress_payload_deserialization test from test_utilities/types/batch/ingress_payload to types/types/batch/ingress.

  • 6b7b92b24 Interface(ICRC_Ledger): Verify ICRC ledger and archive block equality (#3404)
    Review: Looks fine + matches description
    Notes: Adds a test to verify that the definitions of Block are the same within the candid files for the ICRC ledger and the archive canister.

  • d6bb598cf Interface(ICRC_Ledger): canbench benchmarks for icrc2_approve, icrc2_transfer_from and icrc3_get_blocks (#3400)
    Review: Looks fine + matches description
    Notes: Adds benchmarks to the ICRC ledger covering icrc2_approve, icrc2_transfer_from and icrc3_get_blocks.

About CodeGov…(click to expand)

CodeGov has a team of developers who review and vote independently on the following proposal topics: IC-OS Version Election, Protocol Canister Management, Subnet Management, Node Admin, and Participant Management. The CodeGov NNS known neuron is configured to follow our reviewers on these topics and Synapse on most other topics. We strive to be a credible and reliable Followee option that votes on every proposal and every proposal topic in the NNS. We also support decentralization of SNS projects such as WaterNeuron and KongSwap with a known neuron and credible Followees.

Learn more about CodeGov and its mission at codegov.org.

2 Likes

Proposal #134900 — Zack | CodeGov

for release-2025-01-16_16-18-base.

Vote: Adopted
Reason: Builds fine and the hash matches for GUESTOS, HOSTOS and SETUPOS along with all the listed commits.

Review of commits:

Chores:
5cce4f5cb Consensus,Interface:
This improves the API of the abortable broadcast crate by splitting the user_ingress_rx artifacts and the inbound_rxartifacts coming from P2P.
4a7957ba6 Consensus,Interface:
For help with debugging, if the signature verification fails validation_error_to_http_error logs the full HTTP request body.
8054acfac Execution,Interface:
Wasmtime bumped to version 28.0.0 from 27, and adds a Quick Start guide for Execution Benchmarks.
111bac358 Interface,Message Routing:
Adds the “copy” subcommand to the state-tool, used to copy checkpoints from the state directory at source to the state directory at destination.
dce918ac8 Owners(IDX):
This gets rules_rust bumped to release 0.54.1 this gets rid of the patch.
1f1d8dd8c Node:
Update IC-OS Base image references [2025-01-16-0807]
85af5fc7b Node:
Update IC-OS Base image references [2025-01-16-0145]
145aff3e5 Node(nftables):
This update for the IPv6 prefix list in the HostOS firewall adds the new SH1 (Stockholm,SE) DC and removes the old CH!, FR1 and SF1.
6704c1438 Node:
Update IC-OS Base image references [2025-01-09-0807]
ba5e99bf1 Node(IDX):
This is great for us who do the weekly builds. When everything in ic-os is marked as no-cache it saves quite a few gigabytes of cache space per build.

Refactoring:
f491f848c Consensus,Interface(consensus):
This simplifies the IngressPayload implementation by replacing the buffer with a map from IngressMessageIds to a serialized ingress messages.
86357ae40 Execution,Interface,Message Routing:
The attribute #[test_strategy::proptest] is commonly used in conjunction with the test-strategy crate in Rust. It integrates with the proptest crate, which is a property-based testing framework for generating test cases automatically.
6da5c715e Interface:
The serve_journal function that serializes a journal object to JSON and serves it as an HTTP response is moved to upgrade_journal.rs

Tests:
df2145592 Consensus,Interface(consensus):
The test_ingress_payload_deserialization test is moved rs/types/types from test_utilities/types/ .
6b7b92b24 Interface(ICRC_Ledger):
The test function check_archive_and_ledger_block_equality makes sure that the Block type definitions in the ledger.did and archive.did Candid interface files are equivalent.
d6bb598cf Interface(ICRC_Ledger):
Adds benchmarks for icrc2_approve, icrc2_transfer_from and icrc3_get_blocks while before_upgrade was renamed to icrc1_transfer.

About CodeGov (click to expand).

CodeGov has a team of developers who review and vote independently on the following proposal topics: IC-OS Version Election, Protocol Canister Management, Subnet Management, Node Admin, and Participant Management. The CodeGov NNS known neuron is configured to follow our reviewers on these topics and Synapse on most other topics. We strive to be a credible and reliable Followee option that votes on every proposal and every proposal topic in the NNS. We also support decentralization of SNS projects such as WaterNeuron and KongSwap with a known neuron and credible Followees.


Learn more about CodeGov and its mission at codegov.org.

1 Like

Proposal 134900 – ilbert | CodeGov

Vote: ADOPTED.
Reason:
I wasn’t able to build the image from the build script at this commit, as it was failing with the “When not specifying any option please run this script inside an IC git repository” error. I think a regression was introduced at commit 4bd2abd. The workaround I found is to run the script from the parent commit b556319 and pass the current commit as argument:

sudo apt-get install -y curl && curl --proto '=https' --tlsv1.2 -sSLO https://raw.githubusercontent.com/dfinity/ic/b5563194861aa63efe9d0472579f2e00ea38c386/ci/tools/repro-check.sh && chmod +x repro-check.sh && ./repro-check.sh -c 233c1ee2ef68c1c8800b8151b2b9f38e17b8440a --guestos

Since I was able to reproduce the build with this workaround and all the reviewed commits are matching their description, I’ve decided to ADOPT the proposal.

Review:
For the Execution and Runtime layers, the main changes introduced in this proposal are:

  • bug fixes and optimizations

I’ve reviewed all the commits listed in the proposal, for the Execution and Runtime layers:

c4739e9ad:
Continues the work from the previous release to add support for deserializing the compiled Wasm module in case the compilation cache is stored on disk. Updates the get_embedder_cache method of the WasmExecutorImpl struct to match the case in which the compilation cache returns a StoredCompilation::Disk variant. It calls the read_file_and_pre_instantiate method of the WasmtimeEmbedder struct to use the updated deserialize_from_file method. This method has been updated to use the deserialize_open_file function from the wasmtime library. Calling this function is unsafe, so a “SAFETY” comment has been added explaining that the compilation cache guarantees that the module is valid.

760e1f764:
Updates the IntMap structure to accept any key that can be mapped to u64 or u128.
Adds the MutableIntMap structure, which enables to copy the same instance multiple times and modify each copy independently.

57047d6d8:
Updates the execute_update function to handle the case in which the update to execute is a OnLowWasmMemory task. In this case, it removes the OnLowWasmMemory task from the canister’s task queue and enqueues it again. This is needed to set the on_low_wasm_memory_hook_status field of the TaskQueue struct to Ready.

38a497106:
Updates the as_pages method of the Segments struct to avoid creating any pages if the data chunk doesn’t have any bytes inside.

4e0a28df6:
Removes the SYSTEM_SUBNET_DIRTY_PAGE_OVERHEAD constant and sets the dirty_page_overhead field of the system subnet config to the same as the other subnets (DEFAULT_DIRTY_PAGE_OVERHEAD).
Updates the get_num_instructions_from_bytes field of the SystemApiImpl struct to return the same number of instructions regardless of the subnet type.

69981dc71:
Changes the callbacks_with_enqueued_response field of the CanisterQueues struct, the expired_callbacks and shed_responses fields of the MessageStoreImpl struct to use the newly introduced MutableIntMap instead of BTreeSet or BTreeMap. It adapts the code where needed to use the MutableIntMap properly.

8054acfac:
Matches description.

86357ae40:
Updates tests only. Matches description.

2 Likes

Hey @DRE-Team please note that this is the second week in a row where there were build reproducibility issues when using the script in the proposal. It required workarounds by some of the CodeGov reviewers to verify the hash. Is this there any effort to understand and resolve these issues? I recall build reproducibility being a major concern at DFINITY in the past. It would be nice if everyone could produce these builds without having to hack around the issue, so hopefully these new issues will be addressed.

1 Like

Proposal 134900 - Ipsita | ZenithCode

Summary

  1. Vote: Adopt
  2. Hash: All the hashes match
  3. Reasons to adopt: The release notes match the commits and the code changes. Builds successfully and all the hashes match

Commits

Features

  • 572afbcdb
    Summary: add fstrim datadir observability
    Notes: Adds visiblity for fstrim operations by exposing metrics from each node.
    Review: Code changes matches the commit message.

  • c4739e9ad
    Summary: Use Wasmtime deserialize_open_file.
    Notes: Introduces the Module::deserialize_open_file for wasm deserialization, optimizing sandbox performance by avoiding unnecessary byte copying. Also integrates on-disk wasm compilation cache.
    Review: Code changes matches the commit message.

  • 760e1f764
    Summary: Implement MutableIntMap
    Notes: Implements MutableIntMap, a mutable variant of IntMap using the same Tree type, with added methods such as remove and split. Updated to return previous values upon key insertion and applied in various instances.
    Review: Code changes matches the commit message.

  • c16efb073
    Summary: Limit cache disk space
    Notes: Implements memory and disk capacity limits for an LRU cache, automatically evicting older items when limits are reached.
    Review: Code changes matches the commit message.

  • fb3fc37ae
    Summary: Add default encoding and decoding of a Principal in a Subaccount.
    Notes: Introduces functions to encode and decode Principal in Subaccount, simplifying unique subaccount creation for each principal.
    Review: Code changes matches the commit message.

  • 19e3c685a
    Summary: Add log-config service to GuestOS
    Notes: Adds a log configuration service to GuestOS for managing configuration logs.
    Review: Code changes matches the commit message.

Bugfixes

  • 80c6776c1
    Summary: Consider all fields in is_empty and count_bytes implementation for BatchPayload
    Notes: Adjusts is_empty and count_bytes calculations to account for all fields, ensuring newly added fields are automatically included.
    Review: Code changes matches the commit message.

  • cf2f2cc97
    Summary: Make the consensus manager API resemble a classic channel interface
    Notes: Refactors API to align with classic channel interfaces, modifying methods to take channels as input and renaming relevant builders.
    Review: Code changes matches the commit message.

  • 57047d6d8
    Summary: Low Wasm memory hook will run once the canister’s unfrozen if it was scheduled before freezing
    Notes: Ensures low wasm memory hooks execute post-unfreeze if initially blocked by a frozen canister.
    Review: Code changes matches the commit message.

  • 38a497106
    Summary: Ignore empty data segment pages
    Notes: Excludes empty segments from heap data representation as wasm pages, improving efficiency.
    Review: Code changes matches the commit message.

  • 4e0a28df6
    Summary: Account instructions for bytes accessed and dirty pages on system subnets
    Notes: Updates system subnet configuration to include overhead for dirty pages in instruction accounting.
    Review: Code changes matches the commit message.

  • f9f2491d3
    Summary: mocked xnet in PocketIC
    Notes: Fixes error handling in PocketIC by reusing existing methods for stream slice management.
    Review: Code changes matches the commit message.

  • 233c1ee2e
    Summary: deployments when no ipv4 or domain name provided
    Notes: Adjusts deployment processes to validate domain names and IPv4 configurations only when provided.
    Review: Code changes matches the commit message.

Performance improvements:

  • 69981dc71
    Summary: Use MutableIntMap in SystemState
    Notes: Replaces BTreeMaps and BTreeSets with MutableIntMap, optimizing cloning processes despite slightly slower operations.
    Review: Code changes matches the commit message.

Chores

  • 5cce4f5cb
    Summary: Split the user ingress artifacts and the artifacts coming from P2P
    Notes: Splits user ingress artifacts and artifacts received from P2P into separate streams (user_ingress_rx and inbound_rx). The run_artifact_processor function is updated to be generic over a unified stream of artifacts.
    Review: Code changes matches the commit message.

  • 4a7957ba6
    Summary: Log HTTP request body on signature verification failure
    Notes: Enhances error logging for signature verification failures by including the full HTTP request body in the log, rather than just the messageId.
    Review: Code changes matches the commit message.

  • 8054acfac
    Summary: upgrade wasmtime to v.28
    Notes: Upgrades Wasmtime from version 27.0.0 to 28.0.0 and includes a README for execution environment benchmarks.
    Review: Code changes matches the commit message.

  • 111bac358
    Summary: state-tool copy commands
    Notes: Adds a copy command to the state tool, allowing for checkpoint and metadata duplication between sources and destinations. The import_state command now reuses the copy code.
    Review: Code changes matches the commit message.

  • dce918ac8
    Summary: bump rules_rust
    Notes: Updates rules_rust to version 0.54.1, removing the need for a patched version.
    Review: Code changes matches the commit message.

  • 1f1d8dd8c
    Summary: Update Base Image Refs [2025-01-16-0807]
    Notes: Updates base IC-OS image references to align with the latest versions as of [2025-01-16-0807].
    Review: Code changes matches the commit message.

  • 85af5fc7b
    Summary: Update Base Image Refs [2025-01-16-0145]
    Notes: Refreshes base IC-OS image references, aligning them with the latest updates as of [2025-01-16-0145].
    Review: Code changes matches the commit message.

  • 145aff3e5
    Summary: update IPv6 prefix list in the HostOS firewall
    Notes: Updates the IPv6 prefix list in the HostOS firewall by removing outdated datacenter entries and adding a new datacenter (SH1).
    Review: Code changes matches the commit message.

  • 6704c1438
    Summary: Update Base Image Refs [2025-01-09-0807]
    Notes: Refreshes IC-OS base image references to align with updates from [2025-01-09-0807].
    Review: Code changes matches the commit message.

  • ba5e99bf1
    Summary: don’t cache ic-os images
    Notes: Disables caching for IC-OS image builds by adding a no-cache tag.
    Review: Code changes matches the commit message.

Refactoring

  • f491f848c
    Summary: simplify IngressPayload implementation
    Notes: Simplifies IngressPayload by using serialized ingress messages instead of separate buffers and indexes.
    Review: Code changes matches the commit message.

  • 86357ae40
    Summary: Use test_strategy over proptest macro in replicated state tests
    Notes: Switches replicated state tests to use #[test_strategy::proptest] for improved test handling.
    Review: Code changes matches the commit message.

  • 6da5c715e
    Summary: move serve_journal into upgrade_journal.rs
    Notes: Relocates the serve_journal function to align with its usage in SNS governance.
    Review: Code changes matches the commit message.

Tests

  • df2145592
    Summary: move the IngressPayload serialization/deserialization unit test from test_utilities/types/ to rs/types/types
    Notes: Moves the IngressPayload serialization test to a more appropriate directory for better organization.
    Review: Code changes matches the commit message.

  • 6b7b92b24
    Summary: Verify ICRC ledger and archive block equality
    Notes: Adds a test to ensure block definitions in candid files are consistent across the ICRC ledger and archive.
    Review: Code changes matches the commit message.

  • d6bb598cf
    Summary: Crypto, Interface, Node:
    Notes: canbench benchmarks for icrc2_approve, icrc2_transfer_from and icrc3_get_blocks
    Review: Code changes matches the commit message.

1 Like

Proposal: 134900

Summary:

  1. Build Hash: Build has from the proposal, local build and CDN matches and is “4058c67bbbcf8f95c6e050eb4a1fc2b15af23c5faf9f218d2bf01a6f9dfbc041”.
  2. Summary: The release notes matches the code changes
  3. Vote: I vote to adopt the proposals

Detailed Review:

Features:

  • 572afbcdb Crypto,Interface,Node(node): add fstrim datadir observability (#3322)
    Notes: Added observability metrics around fstrim execution, which were left before in hurry.
    Review: Code changes look good and match release notes.
  • c4739e9ad Execution,Interface: Use Wasmtime deserialize_open_file. (#3412)
    Notes: This is an optimisation to use Module::deserialize_open_file instead of Module::deserialize which allowes sandbox to map the existing files to canister code rather than canister using resident memory in sandbox.
    Review: Code changes look good and match release notes.
  • 760e1f764 Execution,Interface,Message Routing: Implement MutableIntMap (#3303)
    Notes: IntMap is now replaced with MutableIntMap which is mostly the same as both use the same Tree type in their implementation. Now we have the support on insert() returning previous value, and also the size of the Map can be calculated in constant time. Since it is a drop-in replacement of BTreeMap it also implemented and extends the functionality with remove(), split_off(), extend().
    Review: Code changes look good and match release notes.
  • c16efb073 Interface: Limit cache disk space (#3366)
    Notes: LRU cache implementation is modified to limit disk and memory space by introducing memory_bytes and disk_bytes separately. These limits are then used in CompilationCache to prevent it to grow to large.
    Review: Code changes look good and match release notes.
  • fb3fc37ae Owners(icrc-ledger-types): Add default encoding and decoding of a Principal in a Subaccount. (#3459)
    Notes: To add default encoding and decoding of a principal in subaccount principal_to_subaccount() and subaccount_to_principal() are added to ICRC Ledger.
    Review: Code changes look good and match release notes.
  • 19e3c685a Node: Add log-config service to GuestOS (#3389)
    Notes: log-config service is now added to GuestOS, which was only available to hostOS before.
    Review: Code changes look good and match release notes.

Bugfixes:

  • 80c6776c1 Consensus,Interface: Consider all fields in is_empty and count_bytes implementation for BatchPayload (#3471)
    Notes: In the bitcoinadapterresponse is_empty and count_bytes are modified to also consider other fields first in BatchPayload deconstruction.
    Review: Code changes look good and match release notes.
  • cf2f2cc97 Consensus,Interface: Make the consensus manager API resemble a classic channel interface (#3233)
    Notes: This PR is mainly focused on making the consensus manager API similar to normal channel API. For this three functions have been modified to pass in outbound_tx and inbound_tx both as inputs. A couple of minor name change has also been made.
    Review: Code changes look good and match release notes.
  • 57047d6d8 Execution,Interface: Low Wasm memory hook will run once the canister’s unfrozen if it was scheduled before freezing (#3455)
    Notes: If the canister is frozen while the OnLowWasmMemory is called, it is requeued again so it is called, when the canister becomes unfrozen.
    Review: Code changes look good and match release notes.
  • 38a497106 Execution,Interface: Ignore empty data segment pages (#3435)
    Notes: The PR introduces a change to not trigger critical alert if the data segment pages are empty by adding bytes.is_empty() check.
    Review: Code changes look good and match release notes.
  • 4e0a28df6 Execution,Interface: Account instructions for bytes accessed and dirty pages on system subnets (#3396)
    Notes: In the SchedulerConfig dirty_page_overhead are now set to DEFAULT_DIRTY_PAGE_OVERHEAD instead of SYSTEM_SUBNET_DIRTY_PAGE_OVERHEAD. This is in the effort to make system subnet much closer to application subnet.
    Review: Code changes look good and match release notes.
  • f9f2491d3 Interface,Message Routing: mocked xnet in PocketIC (#3376)
    Notes: An assertion failure in mocked xnet in ICPocket is fixed by reusing XNetSlicePoolImpl and only mocking the fetch of XNetslice from remote subnet within xnet::payload_builder.
    Review: Code changes look good and match release notes.
  • 233c1ee2e Node: deployments when no ipv4 or domain name provided (#3477)
    Notes: This is a simple fix for deployment to proceed even if the ipv4 config is not specified in config.ini during ic-os installation.
    Review: Code changes look good and match release notes.

Performance improvements:

  • 69981dc71 Execution,Interface,Message Routing: Use MutableIntMap in SystemState (#3304)
    Notes: MutableIntMap is now used instead of BTreeMap and BTreeSet in SystemState as it is faster to clone ( despite it is slower for some other operations ).
    Review: Code changes look good and match release notes.

Chores:

  • 5cce4f5cb Consensus,Interface: Split the user ingress artifacts and the artifacts coming from P2P (#3419)
    Notes: The incoming message is now split between user_ingress_rx: UnboundedReceiver and inbound_rx: UnboundedReceiver in the run_artifact_processor to make it more maintainable.
    Review: Code changes look good and match release notes.
  • 4a7957ba6 Consensus,Interface: Log HTTP request body on signature verification failure (#3239)
    Notes: To assist debugging on signature verification failures, temporarily the request body of the http body is being logged.
    Review: Code changes look good and match release notes.
  • 8054acfac Execution,Interface: upgrade wasmtime to v.28 (#3330)
    Notes: Wastime has been bumped from v27 to v28.
    Review: Code changes look good and match release notes.
  • 111bac358 Interface,Message Routing: state-tool copy commands (#3440)
    Notes: A new copy command has been added to state-tool to facilitate the copy of existing state or to import existing state into a directory ( import_state has been updated ).
    Review: Code changes look good and match release notes.
  • dce918ac8 Owners(IDX): bump rules_rust (#3449)
    Notes: rules_rust has been upgraded from 0.53.0 to 0.54.1
    Review: Code changes look good and match release notes.
  • 1f1d8dd8c Node: Update Base Image Refs [2025-01-16-0807] (#3466)
    Notes: Base image references has been updated.
    Review: Code changes look good and match release notes.
  • 85af5fc7b Node: Update Base Image Refs [2025-01-16-0145] (#3463)
    Notes: Base image reference has been updates.
    Review: Code changes look good and match release notes.
  • 145aff3e5 Node(nftables): update IPv6 prefix list in the HostOS firewall (#3414)
    Notes: Obsolete ipv6 prefixes has been removed and a new one is added for sh1 data center.
    Review: Code changes look good and match release notes.
  • 6704c1438 Node: Update Base Image Refs [2025-01-09-0807] (#3374)
    Notes: base image references has been updated.
    Review: Code changes look good and match release notes.
  • ba5e99bf1 Node(IDX): don’t cache ic-os images (#3256)
    Notes: no-cache has been added as a tag to not let ic-os images being cached, this saves some gigabyte of space.
    Review: Code changes look good and match release notes.

Refactoring:

  • f491f848c Consensus,Interface(consensus): simplify IngressPayload implementation (#3444)
    Notes: IngressPayload implementation has been simplified by removing one big bytes buffer which contains all ingress messages and replacing it with a map. This helps with easy handling of serialization.
    Review: Code changes look good and match release notes.
  • 86357ae40 Execution,Interface,Message Routing: Use test_strategy over proptest macro in replicated state tests (#3462)
    Notes: replicated_tests has been modified to use #[test_strategy::proptest] instead of proptest!
    Review: Code changes look good and match release notes.
  • 6da5c715e Interface: move serve_journal into upgrade_journal.rs (#3393)
    Notes: A simple refactor to move serve_journal from ic-nervous-system-common to upgrade_journal.rs within SNS Governance.
    Review: Code changes look good and match release notes.

Tests:

  • df2145592 Consensus,Interface(consensus): move the IngressPayload serialization/deserialization unit test from test_utilities/types/ to rs/types/types (#3384)
    Notes: Again a minor refactor to move test_ingress_payload_deserialization from test_utilities/types/ to rs/types/types.
    Review: Code changes look good and match release notes.
  • 6b7b92b24 Interface(ICRC_Ledger): Verify ICRC ledger and archive block equality (#3404)
    Notes: The test verifies that the type Block which is returned from the ledger canister and archive canister is the same.
    Review: Code changes look good and match release notes.
  • d6bb598cf Interface(ICRC_Ledger): canbench benchmarks for icrc2_approve, icrc2_transfer_from and icrc3_get_blocks (#3400)
    Notes: Benchmarks has been added for icrc2_approve,icrc2_transfer_from, and icrc3_get_blocks .
    Review: Code changes look good and match release notes.
1 Like

Proposal 134900 - Yuvika | ZenithCode

Summary

  1. Vote: Adopt
  2. Hash: Hashes match
  3. Reasons to adopt: Builds fine + hashes match + release notes match the commits
Commits

Features:

  1. 572afbcdb
    Summary: add fstrim datadir observability.
    Notes: Enhance visibility into fstrim operations by exposing metrics from each node.
    Review: The description matches the code changes.

  2. c4739e9ad
    Summary: Use Wasmtime deserialize_open_file.
    Notes: Optimize wasm deserialization by introducing Module::deserialize_open_file to avoid unnecessary byte copying, and integrates an on-disk wasm compilation cache for improved performance.
    Review: The description matches the code changes.

  3. 760e1f764
    Summary: Implement MutableIntMap.
    Notes: Implement MutableIntMap, a mutable variant of IntMap using the same Tree type, with added methods such as remove and split. Updated to return previous values upon key insertion and applied in various instances.
    Review: The description matches the code changes.

  4. c16efb073
    Summary: Limit cache disk space.
    Notes: Implement memory and disk capacity limits for an LRU cache, automatically evicting the least recently used items to maintain resource constraints.
    Review: The description matches the code changes.

  5. fb3fc37ae
    Summary: Add default encoding and decoding of a Principal in a Subaccount.
    Notes: Introduce functions to encode and decode Principals into Subaccounts, streamlining the creation of unique subaccounts for each principal.
    Review: The description matches the code changes.

  6. 19e3c685a
    Summary: Add log-config service to GuestOS.
    Notes: Introduce a log configuration service to GuestOS for managing and configuring log settings.
    Review: The description matches the code changes

Bugfixes:

  1. 80c6776c1
    Summary: Consider all fields in is_empty and count_bytes implementation for BatchPayload.
    Notes: Modify is_empty and count_bytes calculations to encompass all fields, guaranteeing that newly introduced fields are automatically considered in these computations.
    Review: The description matches the code changes.

  2. cf2f2cc97
    Summary: Make the consensus manager API resemble a classic channel interface.
    Notes: Refactor the API to align with classic channel interfaces by modifying methods to accept channels as input and renaming relevant builders accordingly.
    Review: The description matches the code changes.

  3. 57047d6d8
    Summary: Low Wasm memory hook will run once the canister’s unfrozen if it was scheduled before freezing.
    Notes: Ensure low wasm memory hooks execute as expected after a canister is unfrozen, even if their initial execution was blocked due to the canister’s frozen state.
    Review: The description matches the code changes.

  4. 38a497106
    Summary: Ignore empty data segment pages.
    Notes: Enhance efficiency by optimizing heap data representation by excluding empty segments.
    Review: The description matches the code changes.

  5. 4e0a28df6
    Summary: Account instructions for bytes accessed and dirty pages on system subnets.
    Notes: Refine system subnet configuration by incorporating overhead for dirty pages into instruction accounting.
    Review: The description matches the code changes.

  6. f9f2491d3
    Summary: mocked xnet in PocketIC.
    Notes: Improve PocketIC error handling by leveraging existing stream slice management methods.
    Review: The description matches the code changes.

  7. 233c1ee2e
    Summary: deployments when no ipv4 or domain name provided.
    Notes: Adjust deployment processes to validate domain names and IPv4 configurations only when provided.
    Review: The description matches the code changes.

Performance improvements:

  1. 69981dc71
    Summary: Use MutableIntMap in SystemState.
    Notes: Optimize cloning by replacing BTreeMaps and BTreeSets with MutableIntMap, despite being little slower.
    Review: The description matches the code changes.

Chores:

  1. 5cce4f5cb
    Summary: Split the user ingress artifacts and the artifacts coming from P2P.
    Notes: Separate user ingress and P2P artifacts into user_ingress_rx and inbound_rx, while adapting the run_artifact_processor function to handle this unified artifact stream.
    Review: The description matches the code changes.

  2. 4a7957ba6
    Summary: Log HTTP request body on signature verification failure.
    Notes: Temporarily log the HTTP request body to aid in debugging signature verification failures.
    Review: The description matches the code changes.

  3. 8054acfac
    Summary: upgrade wasmtime to v.28.
    Notes: Update Wastime from v.27 to v.28.
    Review: The description matches the code changes.

  4. 111bac358
    Summary: Routing: state-tool copy commands.
    Notes: Add a new copy command to state-tool for copying or importing existing state into a directory, and also update the import_state command.
    Review: The description matches the code changes.

  5. dce918ac8
    Summary: bump rules_rust.
    Notes: Update rules_rust from 0.53.0. to version 0.54.1.
    Review: The description matches the code changes.

  6. 1f1d8dd8c
    Summary: Update Base Image Refs [2025-01-16-0807].
    Notes: Update the base image references used in the build process.
    Review: The description matches the code changes.

  7. 85af5fc7b
    Summary: Update Base Image Refs [2025-01-16-0145].
    Notes: Update the base image references used in the build process.
    Review: The description matches the code changes.

  8. 145aff3e5
    Summary: update IPv6 prefix list in the HostOS firewall.
    Notes: Update the list of IPv6 prefixes, removing obsolete entries and adding a new prefix specifically for the sh1 data center.
    Review: The description matches the code changes.

  9. 6704c1438
    Summary: Update Base Image Refs [2025-01-09-0807].
    Notes: Update the base image references used in the build process.
    Review: The description matches the code changes.

  10. ba5e99bf1
    Summary: don’t cache ic-os images.
    Notes: Add the no-cache tag to IC-OS images, preventing them from being cached and potentially saving significant disk space.
    Review: The description matches the code changes.

Refactoring:

  1. f491f848c
    Summary: simplify IngressPayload implementation.
    Notes: Update implementation of IngressPayload by replacing a large buffer with a map for ingress messages, streamlining serialization thereby making it more efficient.
    Review: The description matches the code changes.

  2. 86357ae40
    Summary: Use test_strategy over proptest macro in replicated state tests.
    Notes: Update replicated_state tests to use the #[test_strategy::proptest] instead of the proptest! macro.
    Review: The description matches the code changes.

  3. 6da5c715e
    Summary: move serve_journal into upgrade_journal.rs.
    Notes: Refactor the serve_journal function from ic-nervous-system-common to ic-sns-governance.
    Review: The description matches the code changes.

Tests:

  1. df2145592
    Summary: move the IngressPayload serialization/deserialization unit test from test_utilities/types/ to rs/types/types.
    Notes: Refactor the test_ingress_payload_deserialization function from test_utilities/types/ to rs/types/types .
    Review: The description matches the code changes.

  2. 6b7b92b24
    Summary: Verify ICRC ledger and archive block equality.
    Notes: Verify type compatibility between the Block structures returned by the ledger and archive canisters.
    Review: The description matches the code changes.

  3. d6bb598cf
    Summary: canbench benchmarks for icrc2_approve, icrc2_transfer_from and icrc3_get_blocks.
    Notes: Add benchmarks for icrc2_approve, icrc2_transfer_from, and icrc3_get_blocks functions.
    Review: The description matches the code changes.

1 Like

Proposal 134900 | Tim - CodeGov

Vote: Adopt

Reason: Build is successful, hashes match, commits match descriptions and the reasoning behind the changes is sound. I’ve selectively reviewed Consensus, Crypto, Interface and Owner commits as detailed below.

@sat @marko Some of our team have found the build has failed since commit 4bd2abd was pushed about 2 weeks ago. It might be that the build only works if the folder into which the repo is cloned is named dfinity rather than some other name. If this is the case would you consider changing this line accordingly or modifying the instructions in the proposal text template?

Review

Features:

[572afbcdb]
Adds fields last_duration_milliseconds_datadir, last_run_success_datadir and total_runs_datadir to FsTrimMetrics, utilised in fstrim_tool, plus associated tests.

[c16efb073]
Adds the ability for the LRU (least-recently used) cache to limit space used on disk as well as in memory. Within LruCache struct, splits capacity and size into memory_capacity, disk_capacity, memory_size and disk_size, and uses these throughout the code. Similarly splits count_bytes into memory_bytes and disk_bytes.

[fb3fc37ae]
Adds new mapping functions principal_to_subaccount and subaccount_to_principal to packages/icrc-ledger-types/src/icrc1/account.rs.

Bugfixes:

[80c6776c1]
Within impl BatchPayload, adds additional conditions to return is_empty as true, specifically xnet.stream_slices.is_empty() (replacing xnet.is_empty()) and query_stats.is_empty(). Modifies count_bytes method in impl CountBytes for CanisterHttpResponse to include additional items. This and other CountBytes impls are modified to deconstruct self first so as to enable easier error spotting in future.

[cf2f2cc97]
Replaces send_advert with outbound_tx and inbound_rx in run_artifact_processor, create_ingress_handlers and create_artifact_handler functions. Related changes in the consensus manager API code including renaming of some types.

[f9f2491d3]
Several changes to PocketIC matching the description. Renames PocketXNetSlicePoolImpl struct and impl to PocketXNetImpl and adds new logic for storing stream indices and the byte limit used for refilling subnet stream slices.

Chores:

[5cce4f5cb]
Modifies the artifact processor thread loop to use a generic stream for artifact events so that event input is not limited to the UnboundedReceiver type. Removes UnboundedSender from the output of abortable_broadcast_channel. Further changes matching description.

[4a7957ba6]
Adds the full body of an HTTP request to logging on signature verification failure, rather than just the message ID, to assist with debugging of a current issue.

[111bac358]
Adds command copy to state_tool as explained in the commit notes. Removes copy_recursively from commands/import_state.rs as this is presumably now redundant.

[dce918ac8]
Version update for rules_rust, allowing for removal of bazel/rules_rust.patch.

Refactoring:

[f491f848c]
Converts IngressPayload into a map of serialised ingress messages rather than a single buffer of bytes, as explained further in the description.

[6da5c715e]
Moves pub fn serve_journal from rs/nervous_system/common/src/lib.rs to rs/sns/governance/src/upgrade_journal.rs in order to prevent it from being called incorrectly.

Tests:

[df2145592]
Moves test_ingress_payload_deserialization from test_utilities/types to types/types.

[6b7b92b24]
Adds a test check_archive_and_ledger_block_equality to check that the ICRC1 ledger.did and archive.did agree on the block format.

[d6bb598cf]
Changes to ICRC1 ledger canbench benchmarks, as per description.

About CodeGov…

CodeGov has a team of developers who review and vote independently on the following proposal topics: IC-OS Version Election, Protocol Canister Management, Subnet Management, Node Admin, and Participant Management. The CodeGov NNS known neuron is configured to follow our reviewers on these topics and Synapse on most other topics. We strive to be a credible and reliable Followee option that votes on every proposal and every proposal topic in the NNS. We also support decentralisation of SNS projects such as WaterNeuron and KongSwap with a known neuron and credible Followees.

Learn more about CodeGov and its mission at codegov.org.

2 Likes

Proposal 134900 – Zane | CodeGov

Vote: ADOPT

Reason: I’ve run into the same issue as last week with the build script, but reverting the changes the build completes successfully with matching hashes. Reviewed commits also match their descriptions, so I’ve decided voted to adopt.

Features:

c4739e9ad Updated the deserialize_from_file method of wasmtime embedder to use wasmtime’s deserialize_open_file API instead of directly handling manual mapping the memory and module deserialization. In get_embedder_cache make use of read_file_and_pre_instantiate to enable on disk compilation cache.

760e1f764 Defined AsInt trait which is used to mark types that implement IntKey, both IntMap and the tree struct used by it have been modified, adding a AsInt trait bound for the key type. The insert method of IntMap has been modified and returns the replaced values if any, alongside the mutated map. Added MutableIntMap, an internally mutable version of IntMap with constant time size calculation.

c16efb073 Extended LruCache struct with disk_capacity and disk_size fields, so that it can track the size of the disk cache and a limit can be set to prevent it from growing too large, existing memory fields have been renamed to memory_capacity and memory_size for clarity. Its methods have also been updated to account for the new fields, properly updating the size of the disk cache when adding or removing entries that use it and ensuring when new entries are added, the sum of the bytes used by the key value pair doesn’t exceed the MAX_SIZE limit. The eviction logic has been modified to keep removing entries until both the disk and memory size of the cache are below the set capacity. The CountBytes trait, which is implemented by key, value pairs of the cache has been renamed to MemoryDiskBytes and now requires implementation of 2 methods, one to return the number of bytes taken on disk and one to return the number of bytes taken in memory.
The CompilationCache too implements this trait, returning memory and disk byte usage based on the storage strategy with which it is configured. When the new constructor is used to initialize an in memory compilation cache, the disk capacity is set to 1GB. Finally compilation_cache_size field was added to HypervisorMetrics struct to monitor memory and disk usage of the cache, these values are updated everytime observe_compilation_metrics is called.

fb3fc37ae Added bidirectional encoding methods for Principal/Subaccount.

Bugfixes:

80c6776c1 Modified count_bytes and is_empty implementation of various struct to explicitly deconstruct from Self, so that in case new fields are added in future they won’t be mistakenly omitted from the calculations. Fixed BatchPayloads is_empty method by accounting for query stats and CanisterHttpResponses count_bytes by accounting for the canister id size. Implemented is_empty for DkgDataPayload and XNetPayload.

57047d6d8 Modified execute_update method so that when it is called due to OnLowWasmMemory hook being triggered, but the canister doesn’t have enough cycles to run it, the task is popped and immediately after readded to the task queue in order to schedule it to be executed once the canister is unfrozen.

38a497106 Modified as_pages to skip processing of empty chunks.

4e0a28df6 Use same value for dirty_page_overhead on both app and system subnets. Removed system subnet specific behaviour from get_num_instructions_from_bytes, so that number of instructions for bytes is properly accounted on all subnet types.

Performance improvements:

69981dc71 Replaced BTreeMap/Set usage for canister state queues with int convertible keys to the new MutableIntMap and updated code to match the new data structure’s interface. Added calculate_outstanding_callbacks helper method to calculate the number of callbacks for each call context. Implemented AsInt trait required for MutableIntMap entries for all types used in the queues.

Chores:

8054acfac Bumped wasmtime version from v27 to v28.

6704c1438, 1f1d8dd8c, 85af5fc7b Updated base image references.

dce918ac8 Bumped rules_rust to v 0.54.1 and removed backported patch.

ba5e99bf1 Added no-cache tag to ic-os images.

Refactoring:

86357ae40 Replace proptest macro usages with test_strategy::proptest in replicated state tests.

6da5c715e Same as description.

Tests:

df2145592 Same as description.

6b7b92b24 Added test to ensure block types used by ledger and archive are the same.

d6bb598cf Added icrc2_approve, icrc2_transfer_from and icrc3_get_blocks benchmarks.

About CodeGov

CodeGov has a team of developers who review and vote independently on the following proposal topics: IC-OS Version Election, Protocol Canister Management, Subnet Management, Node Admin, and Participant Management. The CodeGov NNS known neuron is configured to follow our reviewers on these topics and Synapse on most other topics. We strive to be a credible and reliable Followee option that votes on every proposal and every proposal topic in the NNS. We also support decentralization of SNS projects such as WaterNeuron and KongSwap with a known neuron and credible Followees.

Learn more about CodeGov and its mission at codegov.org.

1 Like

@timk11 should this fix be enough? fix(ci): broaden `git_remote` match in repro-check script by sasa-tomic · Pull Request #3515 · dfinity/ic · GitHub

4 Likes

Yepp, I can confirm this does work. Just tried it out.

3 Likes

That’s the exact change I had in mind. Thanks for sorting that out!

2 Likes