Skip to content

chore(registry): Update registry helpers to return keyed types#10735

Merged
Bownairo merged 2 commits into
masterfrom
eero/misc-followups-2
Jul 13, 2026
Merged

chore(registry): Update registry helpers to return keyed types#10735
Bownairo merged 2 commits into
masterfrom
eero/misc-followups-2

Conversation

@Bownairo

@Bownairo Bownairo commented Jul 11, 2026

Copy link
Copy Markdown
Contributor

Followup from #9772 (comment).

The values returned are derived from RegistrySnapshot, which is a BTreeMap, and so are already unique. The new definitions express this on the type level.

@github-actions github-actions Bot added the chore label Jul 11, 2026
@Bownairo
Bownairo marked this pull request as ready for review July 11, 2026 05:17
@Bownairo
Bownairo requested review from a team as code owners July 11, 2026 05:17

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This pull request changes code owned by the Governance team. Therefore, make sure that
you have considered the following (for Governance-owned code):

  1. Update unreleased_changelog.md (if there are behavior changes, even if they are
    non-breaking).

  2. Are there BREAKING changes?

  3. Is a data migration needed?

  4. Security review?

How to Satisfy This Automatic Review

  1. Go to the bottom of the pull request page.

  2. Look for where it says this bot is requesting changes.

  3. Click the three dots to the right.

  4. Select "Dismiss review".

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

@zeropath-ai

zeropath-ai Bot commented Jul 11, 2026

Copy link
Copy Markdown

No security or compliance issues detected. Reviewed everything up to b0d5e12.

Security Overview
Detected Code Changes
Change Type Relevant files
Enhancement ► rs/protobuf/generator/src/lib.rs
    Add Ord, PartialOrd derive to hostos_version type attribute
► rs/protobuf/src/gen/registry/registry.hostos_version.v1.rs
    Enhance HostosVersionRecord derives to include Ord, PartialOrd and related traits
Refactor ► rs/registry/canister/src/invariants/common.rs
    Use BTreeSet in place of Vec for hostos version and API boundary node records; switch insert/collect patterns
► rs/registry/canister/src/invariants/replica_version.rs
    Simplify elected versions handling by converting to a BTreeSet from keys

@daniel-wong-dfinity-org-twin daniel-wong-dfinity-org-twin changed the title chore: Convert a few collections to sets perf(registry): Convert a few return types to sets to avoid linearly scanning Vecs. Jul 13, 2026
@github-actions github-actions Bot added perf and removed chore labels Jul 13, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I updated the title of this PR. PTAL.

The important things that I added:

  1. Motivation (avoid linear scan). If you can add more (e.g. some important method is "too" slow), that would be great. Doesn't have to be in the title.

  2. Scope: otherwise, "a few" could be anywhere in this large repo.

Comment thread rs/protobuf/generator/src/lib.rs Outdated
Comment thread rs/registry/canister/src/invariants/common.rs
@Bownairo Bownairo changed the title perf(registry): Convert a few return types to sets to avoid linearly scanning Vecs. chore(registry): Update registry helpers to return keyed types Jul 13, 2026
@github-actions github-actions Bot added chore and removed perf labels Jul 13, 2026
@Bownairo
Bownairo dismissed github-actions[bot]’s stale review July 13, 2026 20:55

No canister behavior changes.

@Bownairo
Bownairo enabled auto-merge July 13, 2026 20:56
@Bownairo
Bownairo force-pushed the eero/misc-followups-2 branch from 05ec22a to b0d5e12 Compare July 13, 2026 22:01
@Bownairo
Bownairo added this pull request to the merge queue Jul 13, 2026
Merged via the queue into master with commit 0cf0b13 Jul 13, 2026
37 checks passed
@Bownairo
Bownairo deleted the eero/misc-followups-2 branch July 13, 2026 23:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants