Skip to content
62 changes: 62 additions & 0 deletions crates/contract/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2018,6 +2018,17 @@ impl MpcContract {
Ok(())
}

/// Cancels a previously started node migration for the calling account.
#[handle_result]
pub fn cancel_node_migration(&mut self) -> Result<(), Error> {
let account_id = Self::assert_caller_is_signer();
log!("cancel_node_migration: signer={:?}", account_id);
if self.node_migrations.remove_migration(&account_id).is_none() {
return Err(errors::NodeMigrationError::MigrationNotFound.into());
}
Ok(())
}

/// Updates the calling participant's registered URL, keeping the TLS key and participant ID.
///
/// Requires a deposit of at least [`MINIMUM_NODE_MANAGEMENT_DEPOSIT`] (excess is refunded), so
Expand Down Expand Up @@ -7041,4 +7052,55 @@ mod tests {
{WORST_CASE_ENTRY_COST_CEILING} at today's storage price"
);
}

#[test]
fn test_cancel_node_migration() {
let running_state = ProtocolContractState::Running(gen_running_state(NUM_DOMAINS));
let mut contract = MpcContract::new_from_protocol_state(running_state);
let participants = {
let ProtocolContractState::Running(running) = &contract.protocol_state else {
panic!("expected running state");
};
running.parameters.participants().clone()
};
let (account_id, _, _) = participants
.participants()
.first()
.expect("expected at least one participant")
.clone();

testing_env!(
VMContextBuilder::new()
.signer_account_id(account_id.clone())
.predecessor_account_id(account_id.clone())
.attached_deposit(NearToken::from_yoctonear(1))
.build()
);
Comment thread
metalurgical marked this conversation as resolved.
let destination_node_info = gen_random_destination_info();
contract
.start_node_migration(destination_node_info.clone())
.expect("participant should be able to start node migration");
assert_eq!(
migration_info(&contract, &account_id),
(account_id.clone(), None, Some(destination_node_info))
);

let mut test_env = Environment::new(None, Some(account_id.clone()), None);
test_env.set_signer(&account_id);
// Cancel the migration
contract
.cancel_node_migration()
.expect("caller should be able to cancel their pending migration");
assert_eq!(
migration_info(&contract, &account_id),
(account_id.clone(), None, None)
);
// Check migration was cancelled
assert!(contract.migration_info().is_empty());
let res = contract.cancel_node_migration();
assert_matches!(
res.unwrap_err(),
Error::NodeMigrationError(NodeMigrationError::MigrationNotFound)
);
}
}
11 changes: 11 additions & 0 deletions crates/contract/tests/snapshots/abi__abi_has_not_changed.snap
Original file line number Diff line number Diff line change
Expand Up @@ -431,6 +431,17 @@ expression: abi
}
}
},
{
"name": "cancel_node_migration",
"doc": " Cancels a previously started node migration for the calling account.",
"kind": "call",
"result": {
"serialization_type": "json",
"type_schema": {
"type": "null"
}
}
},
{
"name": "clean_foreign_chain_data",
"doc": " Private endpoint to clean up foreign chain policy votes and node configurations\n for non-participants after resharing.",
Expand Down
12 changes: 12 additions & 0 deletions crates/e2e-tests/src/cluster.rs
Original file line number Diff line number Diff line change
Expand Up @@ -998,6 +998,18 @@ impl MpcCluster {
.context("failed to start node migration")
}

/// Cancel a previously started node migration for a specific node.
pub async fn cancel_node_migration(
&self,
node_index: usize,
) -> anyhow::Result<near_kit::FinalExecutionOutcome> {
self.contract
.handle_for(self.operator_client_for(node_index)?)
.cancel_node_migration()
.await
.context("failed to cancel node migration")
}

/// Update the registered URL of a specific node, called from that node's own operator account.
pub async fn update_participant_url(
&self,
Expand Down
1 change: 1 addition & 0 deletions crates/e2e-tests/tests/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ pub const DISTINCT_RECONSTRUCTION_THRESHOLDS_PORT_SEED: u16 = 23;
pub const UPDATE_PARTICIPANT_URL_PORT_SEED: u16 = 24;
pub const AVAILABLE_FOREIGN_CHAINS_PORT_SEED: u16 = 25;
pub const BACKUP_SERVICE_RUN_PORT_SEED: u16 = 26;
pub const CANCEL_NODE_MIGRATION_PORT_SEED: u16 = 27;

/// Start a cluster, wait for Running state and presignatures to buffer.
///
Expand Down
93 changes: 93 additions & 0 deletions crates/e2e-tests/tests/migration_service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -996,3 +996,96 @@ async fn migration_service__should_handle_back_migration_a_to_b_to_a() {
.await
.expect("ckd request failed after back-migration");
}

#[tokio::test]
#[expect(non_snake_case)]
async fn migration_service__cancel_node_migration_clears_ongoing_migration_info() {
// Given: a cluster with 2 participants and 2 migration targets.
let (cluster, _running) =
common::must_setup_cluster(common::CANCEL_NODE_MIGRATION_PORT_SEED, |c| {
c.num_nodes = 2;
c.threshold = 2;
c.migration_targets = vec![0, 1];
})
.await;
let source_idx = 0;
let target_idx = 2;
let source_account_id = cluster.nodes[source_idx].account_id().to_string();
assert_eq!(
cluster.nodes[target_idx].account_id().to_string(),
source_account_id,
"migration target must share the source account"
);

// When: the source registers a migration destination.
start_migration_and_wait(&cluster, source_idx, target_idx)
.await
.expect("start_migration_and_wait failed");

// Then: migration_info reports a pending destination for the source.
(|| async {
let info: serde_json::Value = cluster
.view_migration_info()
.await
.context("failed to view migration info")?;
let entry = info.get(&source_account_id);
anyhow::ensure!(
entry.is_some_and(|e| !e.get(1).unwrap_or(&serde_json::Value::Null).is_null()),
"contract has not indexed migration information yet"
);
Ok(())
})
.retry(
ConstantBuilder::default()
.with_delay(common::POLL_INTERVAL)
.with_max_times(
(INDEXER_SYNC_TIMEOUT.as_millis() / common::POLL_INTERVAL.as_millis()) as usize,
),
)
.await
.expect("timed out waiting for migration info");

// When: the source cancels the migration.
let outcome = cluster
.cancel_node_migration(source_idx)
.await
.expect("failed to call cancel_node_migration");
assert!(
outcome.is_success(),
"cancel_node_migration failed: {:?}",
outcome.failure_message()
);

// Then: migration_info no longer shows a destination for that account.
(|| async {
let info: serde_json::Value = cluster
.view_migration_info()
.await
.context("failed to view migration info")?;
let entry = info.get(&source_account_id);
anyhow::ensure!(
entry.is_none(),
"expected destination to be cleared after cancel, got {entry:?}"
);
Ok(())
})
.retry(
ConstantBuilder::default()
.with_delay(common::POLL_INTERVAL)
.with_max_times(
(INDEXER_SYNC_TIMEOUT.as_millis() / common::POLL_INTERVAL.as_millis()) as usize,
),
)
.await
.expect("timed out waiting for contract to reflect cancelled migration");

// And: cancelling again fails — the record was already removed
let outcome = cluster
.cancel_node_migration(source_idx)
.await
.expect("failed to call cancel_node_migration a second time");
assert!(
!outcome.is_success(),
"expected the second cancel_node_migration call to fail, but it succeeded"
);
}
13 changes: 12 additions & 1 deletion crates/near-mpc-contract-interface/src/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ use crate::deposits::{
STORAGE_BYTE_COST_YOCTONEAR, propose_update_required_deposit_yoctonear,
};
use crate::method_names::{
PROPOSE_UPDATE, REGISTER_BACKUP_SERVICE, REGISTER_FOREIGN_CHAIN_SUPPORT,
CANCEL_NODE_MIGRATION, PROPOSE_UPDATE, REGISTER_BACKUP_SERVICE, REGISTER_FOREIGN_CHAIN_SUPPORT,
REQUEST_APP_PRIVATE_KEY, SIGN, START_NODE_MIGRATION, SUBMIT_PARTICIPANT_INFO,
UPDATE_PARTICIPANT_URL, VERIFY_FOREIGN_TRANSACTION, VERIFY_TEE, VOTE_ADD_DOMAINS,
VOTE_CANCEL_KEYGEN, VOTE_CANCEL_RESHARING, VOTE_NEW_PARAMETERS, VOTE_UPDATE,
Expand Down Expand Up @@ -241,6 +241,17 @@ impl<C: CallContract> MpcContractHandle<C> {
.await
}

pub async fn cancel_node_migration(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

oh yeah, claude might have pointed it out already, but this should be added to the mpc_contract_handle__should_match_the_wire_format_catalog test

&self,
) -> Result<C::Output, MpcContractHandleError<C::Error>> {
self.call(FunctionCallArgs::no_deposit(
CANCEL_NODE_MIGRATION,
b"{}".to_vec(),
MAX_GAS,
))
.await
}

pub async fn register_foreign_chain_support(
&self,
foreign_chain_support: SupportedForeignChains,
Expand Down
1 change: 1 addition & 0 deletions crates/near-mpc-contract-interface/src/method_names.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ pub const UPDATE_PARTICIPANT_URL: &str = "update_participant_url";
pub const VERIFY_TEE: &str = "verify_tee";
pub const CONCLUDE_NODE_MIGRATION: &str = "conclude_node_migration";
pub const START_NODE_MIGRATION: &str = "start_node_migration";
pub const CANCEL_NODE_MIGRATION: &str = "cancel_node_migration";
pub const REGISTER_BACKUP_SERVICE: &str = "register_backup_service";
pub const CLEANUP_ORPHANED_NODE_MIGRATIONS: &str = "cleanup_orphaned_node_migrations";
pub const CLEAN_TEE_STATUS: &str = "clean_tee_status";
Expand Down
8 changes: 4 additions & 4 deletions docs/migration-service.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The 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:
a node concluding the migration sends its keyset to the contract, which includes the epoch id and all public keys. The contract compares that against the current epoch and keyset. The transaction will fail in case there is a discrepancy.


## Implementation Details

Expand Down Expand Up @@ -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:
Expand All @@ -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
Expand Down
Loading