# Upgrade the Governance Canister to Commit aa91eca
__Proposer__: andre at popovit.ch\
__Source Code__: [aa91ecacdf3824e193e21b70e0127e8d3edab51a][new-commit]
[new-commit]: https://github.com/dfinity/ic/tree/aa91ecacdf3824e193e21b70e0127e8d3edab51a
## New Commits
```
$ git log --format="%C(auto) %h %s" 4bed17bfc82cddc5691743db6228992cdc2740f4..aa91ecacdf3824e193e21b70e0127e8d3edab51a -- ./rs/nns/governance ./rs/sns/init
aa91ecacdf3 feat(sns): Add `SnsRoot.reset_timers` (#2216)
d7eb6e9851e feat(nns): Enable private neuron enforcement. (#2247)
50d0a23e0e2 fix(nns): Neuron conversion did not transcribe visibility correctly. (#2250)
a00685bd42a test(IC-1579): Isolate TLA tests in Rust-based (non-canister) tests (#2241)
102306234dd feat(RES-151): support for multiple calls of the same function (#2079)
44a8e8d766e feat(RES-153): deal with Apalache state explosion (#2188)
588ad7a46b1 chore: upgrade rust version to 1.82 (#2137)
```
## Current Version
- Current Git Hash: 4bed17bfc82cddc5691743db6228992cdc2740f4
- Current Wasm Hash: f458575214cfade0a49041ba1234c0794a45244ac57647cba601670203fe1427
## Verification
See the general instructions on [how to verify] proposals like this. A "quick
start" guide is provided here.
[how to verify]: https://github.com/dfinity/ic/tree/aa91ecacdf3824e193e21b70e0127e8d3edab51a/rs/nervous_system/docs/proposal_verification.md
### WASM Verification
See ["Building the code"][prereqs] for prerequisites.
[prereqs]: https://github.com/dfinity/ic?tab=readme-ov-file#building-the-code
```
# 1. Get a copy of the code.
git clone git@github.com:dfinity/ic.git
cd ic
# Or, if you already have a copy of the ic repo,
git fetch
git checkout aa91ecacdf3824e193e21b70e0127e8d3edab51a
# 2. Build canisters.
./ci/container/build-ic.sh -c
# 3. Fingerprint the result.
sha256sum ./artifacts/canisters/governance-canister.wasm.gz
```
This should match `wasm_module_hash` field of this proposal.
Build is successful and all the changes match the commit description.
Hash Match: MATCH
Feedback: NONE
Proposer Check: MATCH
Summary:
SNS canisters, introducing functions like get_timers and reset_timers for efficient timer management and reduced cycle consumption. A new LocalKey-based approach for managing trace data in Rust tests was implemented, which improves test isolation and accuracy by keeping traces local to specific tests. Updates to TLA+ models include a mechanism for better function call context distinction using tla_function and log_label, along with improvements to error reporting.
Commits Summary
aa91ecacdf3
Adjustments for managing timers and state within the SNS canisters. Added ic-nervous-system-proto as a dependency, reflecting new functionalities related to the management of timers within canister operations.
Support for managing timers through get_timers and reset_timers functions, allowing the canister to track and reset timers with a cooldown period.
This is from the continued move of new versions of the SNS canisters have been created and are in the process of being adopted by the NNS, and the new versions no longer use heartbeats but timers, which significantly reduces the cycles consumption and load on the system incurred by these canisters.
50d0a23e0e2
This is a fix to how the flag is_private_neuron_enforcement_enabled was been processed. Additional tests were added to validate the proper functioning of neuron visibility settings.
a00685bd42a LocalKey-based approach for managing trace data in Rust-based tests, replacing the previous reliance on a global RwLock. Traces are scoped locally to individual test executions. While the global RwLock is still retained as a fallback to store traces when a LocalKey is not active. This matches the commit description.
102306234dd
Mechanism to better distinguish function call contexts in TLA+ models, especially when a function like a ledger transfer is invoked multiple times within a single method (e.g., burning fees and transferring funds). To achieve this, new annotations (tla_function and log_label) have been added to label function calls, allowing the TLA model to track the specific context of each invocation. Additionally, the changes include improvements to error reporting in TLA+ checks and adjustments to testing frameworks.
44a8e8d766e
A size-based filter has been introduced to limit the memory usage of Apalache by skipping state checks when the state size exceeds a specified threshold. Enhancements include adding a method to estimate the size of TLA+ values and incorporating tests to validate the size calculations, making the tracing process more focused.
588ad7a46b1
Updating the base image for the build and CI processes to a new version across multiple configuration files. Update Rust toolchain from version 1.81.0 to 1.82.0.
From reading the changes it appears so. Unless there are some other changes from other deps that are needed but usually you have the feature enabled at the end and it goes live.
Yeah there was a bug associated with it and commit d7eb6e9851e enabled it. After fixing the bug obvs. Only the team knows the full extent of what is needed to complete the feature. This feature can touch many parts of the codebase not just here so we would have to get feedback from Dfinity to know what the current status is on the feature.
[aa91ecacdf3] Adds field timers to type SnsRootCanister. Adds query method get_timers and update method update_timers to SNS root canister.
[d7eb6e9851e] Switches IS_PRIVATE_NEURON_ENFORCEMENT_ENABLED flag to true.
[50d0a23e0e2] Simplifies the use of the visibility field in Neuron and NeuronInfo such that it consistently calls neuron.visibility.
[a00685bd42a] Further develops the state size hard limit for TLA trace checks that is implemented in 44a8e8d766e below. Also replaces RwLock with LocalKey in most instances. This is said to eliminate interference between tests but I was unable to find a clear explanation of this in the documentation so I’d be grateful @oggy if you could point us towards some resources.
[102306234dd] Several changes to TLA testing code, outlined in the commit notes, intended to allow the TLA model to distinguish between different calls of the same function.
[44a8e8d766e] Implements a hard limit of 500 on the state size for TLA trace checks.
[588ad7a46b1] Rust version update and associated code changes.
At a high level: the previous TLA instrumentation setup could cause TLA check errors to be reported in tests different from the ones actually causing problems, and this is now gone. Note that all this is only test infrastructure and has no implications on the production code.
Technically, the TLA instrumentation previously used a global RwLock to store the traces (sequences of states) recorded in tests. The traces behind the RwLock were then checked at the end of selected tests with a manual tla::check_traces invocation. But since this RwLock is a global, this meant that even traces that were recorded in other tests could/would get checked. This could lead to the situation where a problematic trace was reported as belonging to the wrong test. It also meant that we’d be checking traces even if we weren’t intending to do so (from tests which don’t invoke check_traces), leading to larger test times. Now, the instrumentation first tries to write to a LocalKey (an equivalent of thread local storage for async tasks, see the tokio docs), and the check_traces method is only invoked on the traces found in this LocalKey, which eliminates the interference.
Wasm hash, upgrade args and install mode are correct.
Code changes
I would recommend looking at the timers used in ckBTC, with the current implementation you cannot retry if a task fails, you only get another chance a day later. If the only task we need to do is to poll_for_new_archive_canisters, I’m fine with current implementation.
Why do we need to reset the timers?
Out of curisoty, @Andre-Popovitch switch to wasm64 will make these checks obsolete, isn’t it?
+fn now_nanoseconds() -> u64 {
+ if cfg!(target_arch = "wasm32") {
+ time()
+ } else {
+ SystemTime::now()
+ .duration_since(SystemTime::UNIX_EPOCH)
+ .expect("Failed to get time since epoch")
+ .as_nanos()
+ .try_into()
+ .expect("Failed to convert time to u64")
+ }
+}
Configurable TLA in CI: TLA checks can now be toggled in CI via feature flags, optimizing memory by limiting Apalache state size.
Streamlined Neuron Visibility: Visibility logic is simplified; known neurons default to “public” unless marked otherwise, with targeted test coverage.
Enhanced Concurrency & Tracing: Introduced task-local tracing with RefCell for safe concurrent access and a macro for automated TLA validation in tests.
Reason: Build is successful and hashes match. All commits match their descriptions, therefore I’ve voted to adopt.
Reviews
[aa91ecacdf3] Added timers field to SnsRootCanister struct, which contains infos on whether the canisters has to run periodic tasks and timestamps for last timer execution and reset. TIMER_ID has been added to the SNS root canister global state. These changes were made to make it possible to manually reset sns timers. The canister also exposes 2 new endpoints, one used to get the aforementioned timer infos and one to reset the timer with a cooldown interval of one week between each reset.
[d7eb6e9851e] Enable private neuron enforcement by default.
[50d0a23e0e2] Simplified conversion of neuron visibility by using visibility method.
[a00685bd42a], [102306234dd], [44a8e8d766e] Improvements to TLA tests.
[588ad7a46b1] Bump rust version used to 1.82 and some minor code changes to satisfy clippy warnings.
