Skip to content

chore: Remove NonConsumed cycles use case#10154

Merged
alin-at-dfinity merged 2 commits into
dfinity:masterfrom
dsarlis:dimitris/drop-non-consumed
May 11, 2026
Merged

chore: Remove NonConsumed cycles use case#10154
alin-at-dfinity merged 2 commits into
dfinity:masterfrom
dsarlis:dimitris/drop-non-consumed

Conversation

@dsarlis
Copy link
Copy Markdown
Contributor

@dsarlis dsarlis commented May 11, 2026

The NonConsumed use case is no longer needed after previous refactorings of the code that more clearly separated the cases using an approach with generics. The NonConsumed was mainly meant to mark cases which should not be tracked in canister metrics (e.g. transfers of cycles from one canister to another which are not consumed), however, this is now obsolete as is obvious from the code removed, there were a couple of debug_asserts verifying that this variant is not present and it appeared in some match statements due to compilation requirements.

Removing the variant from the protobuf enum should also be safe as it should not be present in any checkpoint files.

@dsarlis dsarlis requested a review from a team as a code owner May 11, 2026 07:04
@github-actions github-actions Bot added the chore label May 11, 2026
@basvandijk basvandijk added the security-review-passed IDX or InfraSec have concluded it's safe to run CI on the external PR. label May 11, 2026
@github-actions
Copy link
Copy Markdown
Contributor

Copy link
Copy Markdown
Contributor

@alin-at-dfinity alin-at-dfinity left a comment

Choose a reason for hiding this comment

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

LGTM, thank you.

@alin-at-dfinity alin-at-dfinity enabled auto-merge May 11, 2026 07:11
@alin-at-dfinity
Copy link
Copy Markdown
Contributor

Looks like the compatibility_for_cycles_use_case test is failing because of the newly missing enum variant.

auto-merge was automatically disabled May 11, 2026 09:26

Head branch was pushed to by a user without write access

@github-actions github-actions Bot dismissed alin-at-dfinity’s stale review May 11, 2026 09:27

Review dismissed by automation script.

@dsarlis
Copy link
Copy Markdown
Contributor Author

dsarlis commented May 11, 2026

Looks like the compatibility_for_cycles_use_case test is failing because of the newly missing enum variant.

Makes sense. I updated the test to remove the missing variant's value from the expected result which should be fine given that it was never serialized (and so it never needs to be deserialized) by the implementation.

@dsarlis dsarlis requested a review from alin-at-dfinity May 11, 2026 09:28
@alin-at-dfinity alin-at-dfinity enabled auto-merge May 11, 2026 11:42
@github-actions
Copy link
Copy Markdown
Contributor

@alin-at-dfinity alin-at-dfinity added this pull request to the merge queue May 11, 2026
@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to no response for status checks May 11, 2026
@alin-at-dfinity alin-at-dfinity added this pull request to the merge queue May 11, 2026
Merged via the queue into dfinity:master with commit daa7f83 May 11, 2026
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

chore external-contributor security-review-passed IDX or InfraSec have concluded it's safe to run CI on the external PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants