-
Notifications
You must be signed in to change notification settings - Fork 39
feat(mpc-contract): cancel_node_migration() function in contract #3886
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 4 commits
23ad606
2a01273
d702b6d
259262a
3c0becf
d9fca27
56cc37f
94b759e
63e0c9a
38108d1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -321,7 +321,7 @@ flowchart TD | |
| For security reasons and to avoid edge cases and race conditions, the MPC network allows migration of nodes only while the protocol is in a `Running` state (as opposed to `Resharing` or `Initializing`, which are the two other well-defined states). | ||
|
|
||
| Note that starting a migration workflow does not require a signing quorum. Instead, each participant can migrate their node at their own discretion. However, to avoid making the migration process a DoS attack vector, protocol state changes must have priority over any ongoing migrations. | ||
| If the protocol state changes into a `Resharing` or `Initializing` state, any ongoing migration processes will simply be cancelled. | ||
| If the protocol state changes into a `Resharing` or `Initializing` state, the pending `OngoingNodeMigration` record itself is **not** cleared by the transition and remains unless the operator explicitly withdraws it with `cancel_node_migration` or starts a new migration which will replace it. | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It wold be good to add a sentence that explains why this is ok: |
||
|
|
||
| ## Implementation Details | ||
|
|
||
|
|
@@ -431,15 +431,15 @@ The contract provides the following methods: | |
|
|
||
| - **`start_node_migration(destination_node_info: ParticipantInfo)`** - Initiates a node migration: | ||
| - Called by the node operator | ||
| - Creates an `OngoingNodeMigration` record for the given `AccountId` | ||
| - Creates an `OngoingNodeMigration` record for the node operator's account. | ||
| - Stores the destination node's `ParticipantInfo` (new TLS keys, etc.) | ||
| - Can be called multiple times to update the destination node info (only the last value is retained) | ||
| - Returns an error if the protocol is not in `Running` state | ||
| - Returns an error if caller is not a current participant | ||
|
|
||
| - **`cancel_node_migration()`** - Cancels an ongoing node migration: | ||
| - Called by the node operator | ||
| - Removes the `OngoingNodeMigration` record for the given `AccountId` | ||
| - Removes the `OngoingNodeMigration` record for the node operator's account. | ||
| - Useful if the new node is not functioning correctly or wrong information was provided | ||
|
|
||
| - **`conclude_node_migration(keyset: &Keyset)`** - Finalizes a node migration: | ||
|
|
@@ -461,7 +461,7 @@ The contract provides the following methods: | |
|
|
||
| #### Migration Related Behavior | ||
|
|
||
| - The `OngoingNodeMigration` records are automatically cleared when the protocol transitions from `Running` state to `Resharing` or `Initializing` state, effectively cancelling any in-progress migrations. | ||
| - The `OngoingNodeMigration` records are **not** automatically cleared when the protocol transitions from `Running` state to `Resharing` or `Initializing` state. | ||
| - **Future Enhancement**: It may be desirable for the contract to verify that calls to `conclude_node_migration(keyset)` come from the actual onboarding node by checking the transaction signer's public key _(see [(#1086)](https://github.com/near/mpc/issues/1086))_. This would prevent ill-behaved decommissioned nodes from making spurious migration calls. This would require: | ||
| - Comparing `env::signer_account_pk()` with the public key associated with the participant (note: this is different from the TLS key currently stored as [`signer_pk`](https://github.com/near/mpc/blob/b5a9d1b2eef4de47d19b66cb25b577da2b897560/crates/contract/src/tee/tee_state.rs#L32) in TEEState) | ||
| - Including this public key in the TEE attestation | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.