feat(execution): set SEV-enabled subnet reference size to 7#9754
feat(execution): set SEV-enabled subnet reference size to 7#9754
Conversation
Introduces `SEV_REFERENCE_SUBNET_SIZE = 7` and adds an `is_sev_enabled` parameter to `CyclesAccountManagerConfig::application_subnet` and `SubnetConfig::new`. When SEV is enabled, the reference subnet size used for cycle cost scaling is set to 7 instead of the default 13. In the replica setup, `sev_enabled` is read from `SubnetFeatures` in the registry and passed through to `SubnetConfig::new`.
… calls Two call sites were missed in the initial refactor.
…les_account_manager The two tests asserting reference_subnet_size for SEV enabled/disabled are a better fit alongside the existing reference_subnet_size tests in cycles_account_manager. SubnetConfig-level tests remain in subnet_config.rs.
r-birkner
changed the title
![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
r-birkner
dismissed
github-actions[bot]’s stale review
No canister behavior changes.
daniel-wong-dfinity-org
left a comment
daniel-wong-dfinity-org
left a comment
There was a problem hiding this comment.
Approved for Governance.
| .get_features(subnet_id, registry_version) | ||
| .ok() | ||
| .flatten() |
There was a problem hiding this comment.
Perhaps we could print something if this fails and we need to fall back to the default
| .ok() | ||
| .flatten() |
There was a problem hiding this comment.
What are the effects of falling back to false if this fails?
There was a problem hiding this comment.
It will just be cheaper to run your canister. SEV protection is independent of that.
There was a problem hiding this comment.
I'm more worried if it could lead to a state divergence if this fails for some replicas on the subnet but not for others. We could take inspiration of the get_subnet_type function above
5 participants
SEV_REFERENCE_SUBNET_SIZE = 7constant inCyclesAccountManagerConfigis_sev_enabled: boolparameter toCyclesAccountManagerConfig::application_subnet,SubnetConfig::new, and related methodssetup_ic_stack.rs), readssev_enabledfromSubnetFeaturesin the registry and passes it through toSubnetConfig::newfalse(no behaviour change)