feat(subnet-spiltting): deliver Splitting batch to DSM - #11213
feat(subnet-spiltting): deliver Splitting batch to DSM#11213pierugo-dfinity wants to merge 1 commit into
Splitting batch to DSM#11213Conversation
There was a problem hiding this comment.
Pull request overview
Delivers subnet-splitting summary blocks to DSM with node-specific subnet assignments.
Changes:
- Passes the replica node ID into batch delivery.
- Emits
BatchContent::Splittingfor scheduled splits. - Adds source and destination assignment tests.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
rs/consensus/src/consensus/finalizer.rs |
Supplies the local node ID during delivery. |
rs/consensus/src/consensus/batch_delivery.rs |
Builds splitting batches and tests assignments. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| BatchContent::Splitting { | ||
| new_subnet_id, | ||
| other_subnet_id, | ||
| } |
There was a problem hiding this comment.
Sure, but according to this comment, this will be overwritten as soon as the destination subnet starts a new round after the split.
Maybe something worth looking into: both the Scheduled summary and the PostSplit summary have the same registry version, so DSM might use the cached OwnSubnetInfo by mistake. But because destination replicas are restarted before executing anything and I assume this cache is stored only in memory, this shouldn't be a problem.
cc @alin-at-dfinity for confirmation
There was a problem hiding this comment.
This will work as is, as per your analysis.
We can (and probably should) make it clearer by explicitly resetting it to default for subnet B in online_split() instead of carrying it over. Not perfect, as it will result in (apparently valid) disabled features and default limits, but it will work better as documentation (we really expect this to be populated, or else).
There was a problem hiding this comment.
Another alternative is to actually fill it with the proper values by reading the registry at the batch's registry version (which is precisely the version at which the split happened). But we'd then have two individual locations where the registry is read, not perfect either.
No description provided.