Skip to content

fix: Bound subtype checks on shared recursive types; release candid 0.10.39 - #776

Merged
lwshang merged 6 commits into
masterfrom
fix-subtype-check-all-exponential
Oct 21, 2026
Merged

lwshang merged 6 commits into
masterfrom
fix-subtype-check-all-exponential

Conversation

@lwshang

@lwshang lwshang commented Oct 19, 2026 •

Copy link
Copy Markdown
Contributor

What

A subtype check could take exponential time on a recursive type reachable along many paths. This happened in both subtype_check_all (and so service_compatibility_report and didc check) and subtype/subtype_with_config (and so service_compatible and the decoder's check of func/service references):

// new.did: A0 … A22, each level pointing at the next twice
type A0 = record { A1; A1 };
…
type A22 = nat;
service : { go : () -> (A0) }
// old.did: the same shape with B0 … B22, leaf `text`

On master, didc check new.did old.did runs for about 13 s and then runs out of memory. At depth 18 it reports 262,144 = 2^18 "incompatible changes", all for the same nat → text. The opt variant A_i = record { opt A_{i+1}; opt A_{i+1}; nat } is exponential in time through the opt rule's probes.

Why

A failed pair is withdrawn from Gamma, which is necessary because the opt-rule probes share it and must not assume a failed pair. But nothing remembered that the pair had failed, so every later path to it checked it again from scratch. The collector keeps going after errors, and a probe falls back after failing, so both reach the same pair along every path.

Fix

  • Refuted pairs are remembered. Assumptions keeps the pairs the check has refuted, and subtype_ fails at once on one of them. This is sound because assumptions only make a check more permissive: a pair that fails under some assumptions fails under any.
  • The collector reports each failed pair once. Its own failed set is separate from the refuted pairs. A later path to a reported pair adds one entry, A1 is not a subtype of B1 (see above), so a type shared by two methods is still reported under both. A pair refuted only inside a probe is reported in full when the collector reaches it.
  • Hitting the recursion limit fails the whole check. That failure depends on the stack, not on the types, so it can't be cached. But re-exploring it would be exponential again, because an opt probe that hit the limit used to fall back and accept with a warning.
    • RecursionDepth now counts its overflows. After the first one, subtype_ and the collector return at once, the opt fallback no longer accepts, and nothing the overflow touched is cached.
    • As a backstop, subtype_with_config and subtype_check_all check the count when they finish. If the limit was hit, they withdraw the check's assumptions and fail. subtype_check_all adds recursion limit exceeded; the check stopped there, so this report may be incomplete.

The depth-64 versions of both examples now finish instantly. The first reports 65 entries and the opt one reports 1.

Behaviour change: an opt probe that hits the recursion limit now fails the check instead of accepting with a warning. Public signatures are unchanged, and equal is unchanged.

Tests

In rust/candid_parser/tests/compatibility.rs:

  • shared_recursive_types_are_checked_once
  • type_shared_by_two_methods_is_reported_under_both
  • shared_recursive_types_under_opt_are_probed_once
  • pair_refuted_by_a_probe_is_still_reported

rust/candid/tests/subtype_recursion_limit.rs runs on a thread with a fixed 1 MiB stack, after measuring the shortest chain that overflows there:

  • probe_that_hits_the_limit_fails_the_check: an opt diamond whose field only fails by hitting the limit.
  • pair_that_hit_the_limit_is_not_reported_as_incompatible: a pair that overflows when reached deep is not reported as "see above" when reached shallow.
  • overflow_while_tracing_an_alias_fails_the_check: tries every wrapper depth around the limit, so the overflow lands while tracing an alias for one of them.

Each of these fails or hangs without the change it covers. cargo test passes under the three feature sets rust.yml uses, and cargo clippy --features all --tests -D warnings is clean.

Release: candid 0.10.39

Patch release of candid / candid_derive, 0.10.38 → 0.10.39, cut in this PR together with the fix. It also ships #775.

  • Bump candid and candid_derive to 0.10.39, and the = pin between them
  • Date the changelog entries as 2026-10-21 / Candid 0.10.39 (fix: Withdraw assumptions derived from a failed subtype step #775 and this PR)
  • Refresh all four lockfiles: Cargo.lock, rust/bench/Cargo.lock, tools/ui/Cargo.lock, rust/candid/fuzz/Cargo.lock (version bumps only, no dependency churn)

candid_parser stays at 0.4.1; its candid dependency is a 0.10.16 floor.

Test plan

  • cargo check -p candid -p candid_derive -p candid_parser
  • cargo test -p candid --features all: all suites pass
  • cd tools/ui && cargo build --target wasm32-unknown-unknown --profile canister --package didjs --locked, the release workflow's exact command (built against placeholder dist/ assets, since npm was not available locally)
  • cd rust/candid/fuzz && cargo check
  • Reviewer to confirm release scope and changelog date

After merge

  1. Tag 2026-10-21 on master. candid-ui-release.yml triggers on it to publish the candid_ui canister wasm
  2. Run the Crates Publish workflow (workflow_dispatch) with the candid input checked, which publishes candid_derive then candid

🤖 Generated with Claude Code

@lwshang
lwshang requested a review from a team as a code owner October 19, 2026 09:28
@zeropath-ai

zeropath-ai Bot commented Oct 19, 2026 •

Copy link
Copy Markdown
Contributor

✅ No security or compliance issues detected. Reviewed everything up to 53f7c03.

Security Overview
Detected Code Changes
Change Type Relevant files
Enhancement ► rust/candid/src/types/subtype.rs
      Refactor Assumptions to track refuted pairs and extend recursion depth handling
► rust/candid/src/utils.rs
      Rewrite RecursionDepth with overflow tracking and extended guard logic
► rust/candid/tests/subtype_recursion_limit.rs
      Add tests for recursion limit behavior and overflow handling
► rust/candid_derive/Cargo.toml
      Bump version to 0.10.39
► rust/candid_derive/src/lib.rs (implicit from Cargo.toml version bump)
Bug Fix ► rust/candid/src/types/subtype.rs
      Ensure recursion limit affects all relevant subtype paths and prevents caching of failed probes
► rust/candid/src/utils.rs
      Guard behavior updated to propagate overflow info and prevent unchecked caching
► rust/candid/tests/subtype_recursion_limit.rs
      New tests validating corrected overflow behavior
Improvement ► rust/candid_parser/tests/compatibility.rs
      Add tests ensuring shared recursive types are checked once and propagate limits correctly
► CHANGELOG.md (implicit by version bumps in various crates)

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Changes recommended

Optional-type probes bypass failure caching and still allow exponential traversal of shared types.

1 open issue
What changed in this PR

Builds on #775’s assumption rollback fix by caching failed subtype pairs to reduce repeated compatibility-check work.

Changes:

  • Adds per-call failure caching and abbreviated repeat diagnostics.
  • Adds shared-type and multi-method regression tests.
  • Documents the optimization under Unreleased.
File Description
rust/​candid/​src/​types/​subtype.rs Caches failures during incompatibility collection.
rust/​candid_parser/​tests/​compatibility.rs Tests shared types and reporting across methods.
CHANGELOG.md Documents reduced repeated traversal.

Review effort: Balanced

Comment thread rust/candid/src/types/subtype.rs Resolved

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Changes recommended

The collector still caches recursion-limit failures, preventing shallow rechecks and producing misleading incompatibility reports.

1 open issue
1 resolved since last review

Review effort: Balanced

Comment thread rust/candid/src/types/subtype.rs Resolved

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Changes recommended

An alias-tracing overflow can hide incompatibilities, and the test helper misses uppercase overflow messages.

2 open issues
1 resolved since last review

Review effort: Balanced

Comment thread rust/candid/src/types/subtype.rs Resolved

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔵 Needs a closer look

Failure caching in recursive subtype checks and decoder-visible overflow behavior warrant final human validation.

No open issues 🎉

2 resolved since last review

Review effort: Balanced

Base automatically changed from fix-subtype-gamma-leak to master October 21, 2026 02:18
lwshang and others added 6 commits October 21, 2026 02:23
subtype_check_all keeps going after an error, but it withdrew a failed
pair from Gamma, so every later path to that pair re-checked it from
scratch. A recursive type reachable along many paths, such as
`type T_i = record { T_{i+1}; T_{i+1} }`, then took 2^n time and
produced 2^n errors.

The collector now remembers the pairs it has found incompatible.
Reaching one again adds a single entry that points back to the first
report. The pair still leaves Gamma, so the opt-rule probes that share
Gamma do not take it as an assumption.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The opt rule probes `t <: t'` and falls back when the probe fails. A
failed probe retracted its assumptions but kept no record of the
failure, so a type like `A_i = record { opt A_{i+1}; opt A_{i+1}; nat }`
against the same shape ending in `text` re-explored every level below
from both fields: 2^n work in `subtype_` and in the collector's probes.

`Assumptions` now keeps the pairs a check has refuted. Assumptions only
make a check more permissive, so a pair that failed under some fails
under any, and a later visit can fail at once. Failures that hit the
recursion limit depend on the stack, not on the types, so they are not
recorded: `RecursionDepth` now counts its overflows and `refute` checks
that none happened during the pair's check.

The collector also records its own failures here, but still keys its
"see above" entries on the pairs it reported, so a pair refuted only
inside a probe is reported in full when the collector reaches it.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The collector still cached failures that hit the recursion limit, so a
pair whose check overflowed below a deep path was reported as
"not a subtype (see above)" when reached again from a shallow one, where
it may well hold.

Not caching those failures is not enough on its own: an opt-rule probe
that hits the limit falls back to the special rule and succeeds, so the
next path re-explores it, and a shared recursive type with a deep enough
field is exponential again (2.5 s -> 39 s from depth 8 to 12).

So once the limit is hit, `subtype_` and the collector stop: later calls
fail at once, and the opt-rule fallback no longer turns a probe that hit
the limit into a success. The check was already failed by then, and no
result derived from it is cached.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…s handled

The collector's ordinary opt arm read an `Err` from
`trace_type_with_depth` as "not null, reserved or opt" and passed. The
collector stops after an overflow, so a later mismatch went unreported
and the report could be empty: `nat <: opt L0`, with `L0 = nat` already
proved and the alias chain overflowing as it is traced, then hid a
`nat <: text` in the next field.

The arm now also requires no overflow. In case another path swallows
one, `subtype_with_config` and `subtype_check_all` now check the
overflow count when they finish. If the limit was hit, they withdraw
the check's assumptions and fail: `subtype` with an error, and
`subtype_check_all` with an entry saying that the report stopped at the
limit.

The test helper now matches the recursion-limit message regardless of
case: type lookups capitalise it.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Bump candid and candid_derive 0.10.38 -> 0.10.39 and the `=` pin
between them, date the Unreleased changelog entries as 2026-10-21 /
Candid 0.10.39, and refresh all four lockfiles (version bumps only).

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@lwshang
lwshang force-pushed the fix-subtype-check-all-exponential branch from 4649f14 to 53f7c03 Compare October 21, 2026 02:25
@lwshang lwshang changed the title fix: Report each failed subtype pair once in subtype_check_all fix: Bound subtype checks on shared recursive types; release candid 0.10.39 Oct 21, 2026
@lwshang
lwshang merged commit 53dcc82 into master Oct 21, 2026
18 checks passed
@lwshang
lwshang deleted the fix-subtype-check-all-exponential branch October 21, 2026 02:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants