Proposal to elect new release rc--2024-06-12_23-01

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 130392.

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

Release Notes for release-2024-06-12_23-01-base (246d0ce0784d9990c06904809722ce5c2c816269)

Changelog since git revision d19fa446ab35780b2c6d8b82ea32d808cca558d5

Features:

  • 7479249dc Consensus: Improve HTTP outcall divergence error message
  • f98ab7b34 Consensus: Add current_interval_length to Batch
  • be6ee06ee Consensus(schnorr): Implement tSchnorr signing phase
  • fa60cfe69 Consensus(schnorr): Switch signer to new generalized signature requests contexts
  • 885342376 Consensus(schnorr): Allow creation of generalized summary payloads
  • 8698fa51b Consensus(schnorr): Respond to new IDkgDealingsContexts
  • 9a8dc694d Execution,Message Routing: Backward compatibility for CanisterQueue
  • 9577153a8 Execution,Runtime: Delete snapshots when canister is out of cycles
  • 7f625a6ba Execution,Runtime: enable tSchnorr related ic00 API endpoints
  • c0c9a040a Execution,Runtime(schnorr): Match tSchnorr pre-signatures with request contexts
  • 038fbc38f Message Routing: Rename CanisterQueues stats to reflect slot vs memory reservations
  • ea20c80a0 Networking(quic_transport): allow setting quic stream priority
  • c792b2854 Networking(sync-call): Added networking related changes for the sync-call feature.
  • 246d0ce07 Consensus(ecdsa): Add system tests checking several multiple ecdsa keys scenarios & add handle the case when new keys are added to the subnet

Bugfixes:

  • bc0117af2 Execution,Message Routing,Interface: Store priority_credit and long_execution_mode in the state
  • 5891b4554 Execution,Runtime: Treat the Wasm memory limit 0 as unlimited
  • e5d15eeaa Execution,Runtime: apply a separate fee for Schnorr signature
  • d78df25ba Execution,Runtime: count sign_with_schnorr contexts per key
  • 51940c015 Interface: Fix Request and Payload debug formatting
  • 0f9702f9c Networking: add tower-http tracing instrumentation for the axum routers
  • c78e0fba8 Networking(https_outcalls): Allow headers with same header name
  • 460490e98 Node: whitelist new LN1 prefix
  • 9729d93e5 Runtime: Reject Wasm table modification instructions

Performance improvements:

  • e9a0c5533 Runtime,Message Routing: Only aggressively prefetch if it is free

Chores:

  • d53cb04cd Consensus(schnorr): Set key_id in IDkgReshareRequest to None
  • fcbf596e1 Consensus: Remove unused ecdsa code
  • e47aefb56 Consensus(schnorr): Generalize naming of orchestrator tasks for tSchnorr
  • 1ca6cd53f Crypto: remove unused deps or move to dev deps
  • 3f9873509 Crypto: Reject non-canonical Ed25519 point encodings in IDKG
  • 5c2cd9784 Execution,Message Routing: remove obsolete ECDSA metadata fields
  • 096f3d2b1 Execution,Message Routing: Get rid of unused QueueId
  • 87f1babbb Execution,Runtime: Rename instructions_overhead_per_message
  • 4bc8a4eb3 Execution,Runtime,Message Routing: rename fields in CanisterMetrics
  • f0dde6c89 Networking(http-metrics): Add metrics to ingress watcher
  • de55c8ed3 Networking(sync-call): Handle messages completing execution, state reader failing.
  • b5971e6ac Node(IC-OS): switch to podman
  • 777bce706 Runtime: Allow unused field that will be used in a follow-up MR.
  • a6c63e547 Runtime: Fix instruction limit error message for DTS
  • 8c23cee02 Runtime,Execution: Add metric for number of tables

Refactoring:

  • 471879bb5 Crypto: add more information to serialization errors in tSchnorr and tECDSA

Tests:

  • ff5231caf Consensus(schnorr): Allow generation of generalized test signature inputs
  • 6c4c3cfcc Consensus(schnorr): Generalize pre-signer unit tests and helpers
  • c42da8c73 Consensus,Interface(consensus): Create integration test for subnet with stalled clocks
  • 988481884 Crypto(github-sync): PR#183 / refactor(crypto): improve error mapping in TLS test client and server
  • 711d5d25b Execution,Runtime: add helper function for derivation path type conversion and cleanup some execution tests
  • 4e36ea022 Message Routing: debug_assert that serialize-deserialize doesnā€™t modify ReplicatedState
  • da75b7dd6 Message Routing: Move testing-only method into testing trait/module

Other changes:

  • 912946fea Node: Updating container base images refs [2024-06-06-0906]

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/246d0ce0784d9990c06904809722ce5c2c816269/gitlab-ci/tools/repro-check.sh && chmod +x repro-check.sh && ./repro-check.sh -c 246d0ce0784d9990c06904809722ce5c2c816269

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.

6 Likes

nice release guys!

did this thing broke almost whole network for 4+ hours(nns front-end, icp icrc2 ledger and god knows what else)?

No this isnā€™t the cause of the issue, the release hasnā€™t been approved yet by the NNS.

4 Likes

Thanks for the release DFINITY. Itā€™s always useful to have a summary of the relevant changes. Can I ask why commit 48afaff wasnā€™t included in this summary? Same question for commit 13225f8.

3 Likes

Reviewers for the CodeGov project have completed our review of these replica updates.

Proposal ID: 130392
Vote: ADOPT
Full report on OpenChat

At the time of this comment on the forum there are still 2 days left in the voting period, which means there is still plenty of time for others to review the proposal and vote independently.

We had several very good reviews of the Release Notes on these proposals by @Zane, @cyberowl, @ZackDS, @massimoalbarello, @ilbert, and @Lorimer. The IC-OS Verification was also performed by @tiago89 and @Gekctek. I recommend folks take a look and see the excellent work that was performed on these reviews by the entire CodeGov team. Feel free to comment here or in the thread of each respective proposal in our community on OpenChat if you have any questions or suggestions about these reviews.

3 Likes

@Lorimer thanks for spotting the missing commits and reporting them. The issue has been identified to be a tooling bug that filtered them out inadvertently. The team will fix it moving forward.

For the specific commits:

  1. fix: [EXC-1627] limit the max number of nodes in compute_initial_*_deā€¦ Ā· dfinity/ic@48afaff Ā· GitHub is mostly strengthening the code as there shouldnā€™t be a (valid) request with more than 100 nodes to this API anyway (at least not until there are subnets of that size :)).
  2. EXC-1530: Fix CanisterSnapshotResponse type Ā· dfinity/ic@13225f8 Ā· GitHub was fixing a type in the unreleased canister snapshots feature so no effect in production.
3 Likes

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 130408.

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

Release Notes for release-2024-06-12_23-01-storage-layer-disabled (2dfe3a1864d1b9a6df462e9503adf351036e7965)

Changelog since git revision 246d0ce0784d9990c06904809722ce5c2c816269

Bugfixes:

  • 2dfe3a186 Interface: Disable new storage layer

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/2dfe3a1864d1b9a6df462e9503adf351036e7965/gitlab-ci/tools/repro-check.sh && chmod +x repro-check.sh && ./repro-check.sh -c 2dfe3a1864d1b9a6df462e9503adf351036e7965

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.

Thanks @dsarlis!

Out of interest can I ask how this tool detects changes that are deemend to be relevant? This would be interesting to know.

Also, while thereā€™s some attention on this tool, could I ask if it would be feasible to adjust it such that future proposal summaries reference the merge commits (which tend to have significantly more informative commit messages). I think this could make a big difference to the perceived transparency of the proposal to some members of the community who are interested in peaking at some of the commits (as opposed to developers who are inclined to pull and explore the git graph more fully).

2 Likes

I will have to defer to our @DRE-Team about the specific questions on the tool.

2 Likes

The code for the release notes generation is here: dre/release-controller/release_notes.py at main Ā· dfinity/dre Ā· GitHub
(please feel free to review the code and suggest changes / make PRs)

In the past we were using merge commits for changes instead of regular commits but then we faced some edge cases where git wasnā€™t detecting merge commits properly so some changes were missed. WARNING(technical details): The reason is that git detects merge commits by checking if a commit has more than one parent. And this does not always work. For instance, if someone force-merges/pushes something, without creating a merge commit, or if itā€™s the last commit in the branch.
So Iā€™ve reverted the code to show regular commits. There are two possible paths that I see, going forward:

  1. changing to code to opportunistically search for a merge commit for a particular commit, and use the merge commit if it exists otherwise fall back to the regular commit, or
  2. wait until we migrate to github, when the formatting of the commit messages will change, and by then we may not need to rely on the merge commits at all. The ETA for this is a few months, based on the latest info Iā€™ve heard.

Iā€™ll check how hard it would be to do 1.

4 Likes

It was a bit of work after all, but should now be resolved in feat(release-notes): Use merge commits if available, and fall back to the non-merge by sasa-tomic Ā· Pull Request #502 Ā· dfinity/dre Ā· GitHub
I hope it works as expected. Git internals are tricky. :crossed_fingers:

4 Likes

Thanks for doing this @sat. Iā€™m looking forward to seeing this in action.

I had a quick gander at release_notes.py this morning, and was interested to see that the included commits are initially established based on files changed by the commit intersecting with files that are part of any replica packages (established by bazel). But then this decision is overridden if the file(s) are not considered to be owned by the replica teams. It seems like the important thing should be that a file that constitutes a replica dependency has changed (regardless of who is considered to be the owner of that file). Can I ask what sorts of scenarios this is designed to filter out?

I was also interested to see the EXCLUDE_PACKAGES_FILTERS. Does GuestOS actually take a dependency on ā€˜/sns/ā€™, ā€˜/nns/ā€™, ā€˜ckbtcā€™, and/or ā€˜ckethā€™ files? Iā€™m interested in why these are explicitly filtered out too :slightly_smiling_face:

1 Like

Sns, nns, and other canisters have a separate deployment and separate set of proposals. So there is no benefit from including them into the IC-OS release notes, it would only increase noise.

1 Like

Sure, but Iā€™m wondering why theyā€™re resolved as dependencies of GuestOS in the first place. This implies that changes to these components have the potential to change the behaviour of GuestOS. Is that correct?

  • if not, Iā€™m wondering why bazel considers them to be dependencies
  • if yes, Iā€™m wondering why theyā€™re filtered out (given that the proposal is about voting on changes that have been made that will/can affect GuestOS)

Iā€™m not meaning to be difficult, Iā€™m just interested in understanding this better.

1 Like

Thatā€™s an excellent observation actually. Itā€™s an issue in our build setup, in essence. Guest OS depends on the canister sandbox, which depends on some libraries used by the canisters, and that ruins so many things at the moment. The primary focus of the node team at the moment is sorting this out. So you are pointing out a real problem. :clap:

3 Likes

Thanks @sat, this is really useful to know and helps clarify some other stuff Iā€™ve been wondering :+1:

2 Likes

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 130749.

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

Release Notes for release-2024-06-12_23-01-cycle-hotfix (48c500d1501e4165fc183e508872a2ef13fd0bef)

Changelog since git revision 2dfe3a1864d1b9a6df462e9503adf351036e7965

Other changes:

  • 48c500d15 Execution,Runtime: [hotfix]: Prevent lowering Cycles reservation limit below existing

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/48c500d1501e4165fc183e508872a2ef13fd0bef/gitlab-ci/tools/repro-check.sh && chmod +x repro-check.sh && ./repro-check.sh -c 48c500d1501e4165fc183e508872a2ef13fd0bef

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.

Thanks for this hotfix release DFINITY. The hotfix itself makes sense, but the circumstances and deployment of this hotfix seem curious.

It would appear to be a response to the lhg73 subnet having stalled earlier today (itā€™s since had the replica version elected by this proposal rolled out to it). However, this hotfix is applied on top of a previous replica version that had LSMT disabled. The lhg73 subnet was previously running a replica version that had the LSMT feature enabled. This means two changes have effectively gone in for subnet lhg73 (disabling LSMT, and this particular new hotfix to consistently enforce the reserved cycles limit during canister updates).

Iā€™ve since noticed that the ā€˜Changelog since git revisionā€™ commit reference on both hotfix proposals is misleading/incorrect. 130749 references 2dfe3a1 as the commit upon which the hotfix is applied, but itā€™s actually 246d0ce (LSMT enabled). Similarly 130748 references ae3c4f3 as the commit upon which the hotfix is applied, but itā€™s actually e3fca54 (LSMT enabled). Is there something that Iā€™m missing about the choice of ā€˜Changelog since git revisionā€™ references in both proposals?

Given that a version of this hotfix isnā€™t being elected with the LSMT flag switched off, presumably the NNS subnet is ready to have LSMT enabled again (since the change for supporting deterministic time slicing)?

Thanks in advance :pray:

2 Likes

Hi @DRE-Team,

Thanks again for referencing merge commits in the proposal summary. Are you able to offer any commentary on the question above? Iā€™m wondering if this is down to a quirk of the proposal authoring tool, or a one-off issue.

If thereā€™s a need to reference a prior hotfix or feature flag that wasnā€™t merged into master (and not contained by the current proposal), it seems like a ā€œDiscarded commitsā€ reference in the proposal summary would be a more correct way of referring to them (or orphaned/abandoned commits, with relation to the proposal).

Then the ā€œChange log sinceā€ reference is free to refer to the last deployed commit that is actually included in this proposals commit lineage.

What do you think?

1 Like

Itā€™s an issue in the tooling. The logic for finding the previous commit is rather fundamental at the moment and cannot properly detect previous commit when thereā€™s lots of different versions. Weā€™ll improve this for the next time.

2 Likes