migrate_node_contract: NodeNotAvailableToDeploy conflates three preconditions, one of them about the source node

[sameh-farouk]

Describe the bug

migrate_node_contract (added in #1113, spec 159) has three unrelated preconditions that all return the same NodeNotAvailableToDeploy error, and one of them is about the source node — which the caller never names in the call.

substrate-node/pallets/pallet-smart-contract/src/grid_contract.rs:

// L358 — (1) SOURCE or DESTINATION is rented
ensure!(
    !ActiveRentContractForNode::<T>::contains_key(source_node_id)
        && !ActiveRentContractForNode::<T>::contains_key(node_id),
    Error::<T>::NodeNotAvailableToDeploy
);

// L368 — (2) DESTINATION is in standby
let node_power = pallet_tfgrid::NodePower::<T>::get(node_id);
ensure!(
    !node_power.is_standby_phase(),
    Error::<T>::NodeNotAvailableToDeploy
);

// L372 — (3) DESTINATION has an extra fee, or the FARM is dedicated
ensure!(
    DedicatedNodesExtraFee::<T>::get(node_id) == 0 && !farm.dedicated_farm,
    Error::<T>::NodeNotAvailableToDeploy
);

The call signature is migrate_node_contract(contract_id, node_id, deployment_hash). The only node the caller names is the destination, so the error reads as "the node you are moving to is not ready". Two of the three causes match that reading. The third does not — and it is the one that fired in practice.

The neighbouring guard already does this correctly:

// L354 — same both-sides shape, but its own specific error
ensure!(
    !pallet_tfgrid::NodeV3BillingOptOut::<T>::contains_key(source_node_id)
        && !pallet_tfgrid::NodeV3BillingOptOut::<T>::contains_key(node_id),
    Error::<T>::NodeIsOptedOutOfV3Billing
);

So the pallet is internally inconsistent: one both-sides precondition gets a precise error, the three next to it share a generic one.

To Reproduce

Observed on devnet, which has been on spec 159 since block 22730338 (2026-08-20 12:54).

Two council-dispatched migrations of contract 270558 → node 371 failed:

#22771911  Council.Executed  Err Module { index: 26, error: 0x19 }   // SmartContractModule[25] = NodeNotAvailableToDeploy
#22772040  Council.Executed  Err Module { index: 26, error: 0x19 }

Historical state at those blocks shows the destination was clean on all three conditions:

#22771910   node 371: farm=130   power = { state: Up, target: Up }
            activeRentContractForNode(371) = null
            dedicatedNodesExtraFee(371)    = 0
            farm(130).dedicated_farm       = false

The blocker was the source node, 396:

#22771910   activeRentContractForNode(396) = 269179   (rent contract, twin 58)
#22772039   activeRentContractForNode(396) = 269179
#22772190   activeRentContractForNode(396) = null

Behaviour is correct — a rent contract flips the cost basis, and the comment above the guard says so explicitly. Only the diagnosis is the problem: nothing in the error, the event, or the call args points at node 396.

For completeness, the sequence resolved on its own rather than being fixed:

#22772110  NodeContractCanceled   contract=270558  nodeId=396  twinId=58
#22772112  RentContractCanceled   contract=269179
#22772191  migrate 270751 -> 371  Ok

Expected

The returned error should identify which precondition failed, and on which node.

Suggested mapping — three of the four reuse variants that already exist, so only one new variant is needed:

condition	suggested error
source node has an active rent contract	new, e.g. SourceNodeHasRentContract
destination node has an active rent contract	NodeHasRentContract (existing, [23])
destination node is in standby	NodeNotAvailableToDeploy (keep)
destination has extra fee / farm is dedicated	FarmIsNotDedicated is taken; suggest NodeIsDedicated (new)

This is also the remedy proposed by @sameh-farouk in #914 for the create path:

If this is not enough, we can reuse other errors type like NodeHasActiveContracts, NodeHasRentContract […] If node is rented the error returned is NodeHasRentContract

Additional context

Why #914 does not already cover this. #914 raised the same ambiguity for create_node_contract and was closed as completed because the SDK's AvailableFor node filter made it moot — nodes returned by the filter are guaranteed deployable, so a client never sees the ambiguous error. That resolution does not transfer to migrate_node_contract:

1. It is RestrictedOrigin (root or 3/5 council), so there is no SDK filter in the loop. A council member picks a node ID by hand.
2. The failure surfaces only as a raw Council.Executed { result: Err(Module { index, error }) } event. There is no client layer to translate it into a message.
3. Please specify why node NodeNotAvailableToDeploy when creating contract #914's three causes were all about the target node. This call adds a fourth that is about a node the caller never named, which is a new failure mode rather than the one Please specify why node NodeNotAvailableToDeploy when creating contract #914 discussed.

Timing. Error variants are part of the runtime metadata and are decoded positionally by clients. migrate_node_contract is currently devnet-only, so variants can still be added or renumbered without breaking anyone. After it reaches mainnet, that stops being free.

Related, optional. migrate_node_contract reuses ContractUpdated, so migrations cannot be queried directly — they can only be reconstructed by diffing nodeId across successive events, which works only because update_node_contract happens not to touch node_id. A dedicated NodeContractMigrated { contract_id, from_node, to_node } event would make migrations observable and would stay correct if that ever changes. Happy to split this into its own issue if preferred.