Repository navigation
chore: rustc: 1.98.1 -> 1.99.0 - #11735
Conversation
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
The canbench baseline updates promised by the PR description are absent from the current diff.
Review effort: Balanced
Findings: 1
Open (1)
What changed in this PR
Upgrades the repository’s Rust toolchain from 1.98.1 to 1.99.0.
Changes:
- Updates the rustup toolchain pin.
- Synchronizes the Bazel Rust toolchain version.
| File | Description |
|---|---|
rust-toolchain.toml |
Pins Rust 1.99.0. |
bazel/rust.MODULE.bazel |
Pins Bazel’s Rust toolchain to 1.99.0. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
This pull request changes code owned by the Governance team. Therefore, make sure that
you have considered the following (for Governance-owned code):
-
Update
unreleased_changelog.md(if there are behavior changes, even if they are
non-breaking). -
Are there BREAKING changes?
-
Is a data migration needed?
-
Security review?
How to Satisfy This Automatic Review
-
Go to the bottom of the pull request page.
-
Look for where it says this bot is requesting changes.
-
Click the three dots to the right.
-
Select "Dismiss review".
-
In the text entry box, respond to each of the numbered items in the previous
section, declare one of the following:
-
Done.
-
$REASON_WHY_NO_NEED. E.g. for
unreleased_changelog.md, "No
canister behavior changes.", or for item 2, "Existing APIs
behave as before.".
Brief Guide to "Externally Visible" Changes
"Externally visible behavior change" is very often due to some NEW canister API.
Changes to EXISTING APIs are more likely to be "breaking".
If these changes are breaking, make sure that clients know how to migrate, how to
maintain their continuity of operations.
If your changes are behind a feature flag, then, do NOT add entrie(s) to
unreleased_changelog.md in this PR! But rather, add entrie(s) later, in the PR
that enables these changes in production.
Reference(s)
For a more comprehensive checklist, see here.
GOVERNANCE_CHECKLIST_REMINDER_DEDUP
No behavioural canister changes.
|
✅ No security or compliance issues detected. Reviewed everything up to 2c50207. Security OverviewDetected Code Changes
|
|
✅ No security or compliance issues detected. Reviewed everything up to 2c50207. Security OverviewDetected Code Changes
|
daniel-wong-dfinity-org-twin
left a comment
daniel-wong-dfinity-org-twin
left a comment
There was a problem hiding this comment.
Thank you for including the tables in the PR description. Much easier to understand than the noisy canbench_results.yml diffs.
d3db099 to
5634179
Compare
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Regenerated with `bazel run //rs/bitcoin/ckbtc/minter:ckbtc_minter_canbench_update`. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Regenerated with `bazel run //rs/nns/governance:governance-canbench_update`. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The totals are unchanged; the split-finder picks a different but equally balanced partition. All ten values come from one 1.99.0 reference run, in its source/destination orientation. Five uncached runs give byte-identical estimates. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Regenerated with `bazel run //rs/registry/canister:registry-canbench_update`. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Regenerated with `bazel run //rs/ledger_suite/icrc1/ledger:canbench_u256_update`. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Regenerated with `bazel run //rs/ledger_suite/icrc1/ledger:canbench_u64_update`. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
5634179 to
2c50207
Compare
Upgrade the rust toolchain from 1.98.1 to 1.99.0. See:
https://blog.rust-lang.org/2026/10/01/Rust-1.99.0/.
The
cargo clippyfixes this needs are in the PR below this one in the stack (#11734). This PR istherefore the version bump in
rust-toolchain.tomlandbazel/rust.MODULE.bazel, plus the testdata that the new codegen (LLVM 22 → 23) shifts.
//rs/bitcoin/ckbtc/minter:ckbtc_minter_canbench_testwas regenerated withbazel run //rs/bitcoin/ckbtc/minter:ckbtc_minter_canbench_update. All 7 benchmark totals staywithin the noise threshold (+0.10% to +0.49%), but 7 small
::greedyscopes moved beyond it:build_unsigned_transaction_6_10_btc::greedybuild_unsigned_transaction_4_10m_sats::greedybuild_unsigned_transaction_5_1_btc::greedybuild_estimate_retrieve_btc_fee_1_50k_sats::greedybuild_unsigned_transaction_1_50k_sats::greedybuild_unsigned_transaction_2_100k_sats::greedybuild_unsigned_transaction_3_1m_sats::greedy//rs/nns/governance:governance-canbench_testwas regenerated withbazel run //rs/nns/governance:governance-canbench_update. One of its 28 benchmarks moved beyondthe 5% noise threshold:
neuron_metrics_calculation//rs/registry/canister:registry-canbench_testwas regenerated withbazel run //rs/registry/canister:registry-canbench_update. One of its 14 benchmarks moved beyondthe noise threshold:
measure_routing_table_invariant_checks_shards_and_unsharded//rs/ledger_suite/icrc1/ledger:canbench_u64_testand:canbench_u256_testwereregenerated with
bazel run //rs/ledger_suite/icrc1/ledger:canbench_u64_updateand:canbench_u256_update. Both moved the same way. u64:bench_icrc1_transfers::icrc103_get_allowancesbench_icrc1_transfers::icrc2_approvebench_icrc1_transfers::icrc2_transfer_frombench_icrc1_transfersbench_icrc1_transfers::upgradebench_icrc1_transfers::post_upgradeu256:
bench_icrc1_transfers::icrc103_get_allowancesbench_icrc1_transfers::icrc2_approvebench_icrc1_transfers::icrc2_transfer_frombench_icrc1_transfersbench_icrc1_transfers::upgradebench_icrc1_transfers::post_upgrade//rs/recovery/subnet_splitting:integration_testhas no_updatetarget. Its expected valuesare hardcoded in the test, so all ten were re-pinned by hand from one 1.99.0 reference run, in that
run's source/destination orientation.
As with 1.98.1 (#11534), what changed is the partition, not the workload:
heartbeats 694. Total instructions moved by 0.013% and total state size by 0.26%.
that is enough to flip a near-tied optimum to a different partition.
states_sizes_bytescanisters_installedingress_messages_executedremote_subnet_messages_executed_lower_boundlocal_subnet_messages_executed_upper_boundhttp_outcalls_executedheartbeats_and_global_timers_executedThe test is deterministic: 5/5 uncached runs on 1.99.0 give byte-identical estimates. As noted in
#11534, it pins the exact solution of a degenerate optimisation, so it will keep flipping on
unrelated changes too. Asserting on the totals and on the ~50/50 instruction balance would make it
robust.
The other three canbench suites (ckETH minter, ICP ledger, SNS governance) stay within their noise
thresholds. CI passes on all targets (
CI_ALL_BAZEL_TARGETS), including the Windows PocketIC jobsand Build Determinism.
No repin is needed. The
Cargo.Bazel.*.lockdigests hash thecargo/rustcof rules_rust's ownhost tools (pinned at 1.98.0 by rules_rust 0.74.0), not the version selected by
rust.toolchain,and no third-party crate changed. rules_rust 0.74.0 ships no checksums for 1.99.0 (nor for the
current 1.98.1), so the toolchain is downloaded unverified, exactly as 1.98.1 is today.
Also checked, since tests won't necessarily catch these:
RangeInclusive(rust#155114): an exhaustedrange now reports different
start()/end(). No code in the repo observes a range afteriterating it.
wasm32-unknown-unknowntarget features are identical on both toolchains.Canister-like modules built with the canister flags use no new Wasm proposal, and they pass
validation configured like the IC's.
lldcannot read.This is harmless today: canisters (
-C linker-plugin-lto) link with rustc's bundledrust-lld, andnative links carry no bitcode. Until the hermetic toolchain reaches LLVM 23, though, don't route
wasm links through it or add
-C linker-plugin-ltoto native targets.🤖 Generated with Claude Code