refactor(contract): use UpdateId and Keyset from dtos in the API

[gilcu3]

Closes #4257

[claude]

PR title type suggestion: This PR restructures code to use centralized DTO types across multiple source files. The type prefix should probably be refactor: instead of chore:

Suggested title: refactor: use UpdateId and Keyset from dtos

[claude]

Pull request overview

This moves the two remaining contract-internal types off #[near] entry-point signatures: init_running and conclude_node_migration now take dtos::Keyset, and propose_update/vote_update now use a new dtos::UpdateId newtype instead of the contract's update::UpdateId. The missing inbound conversions (dtos::Keyset → Keyset, dtos::KeyForDomain → KeyForDomain, dtos::PublicKeyExtended → PublicKeyExtended) are added to dto_mapping.rs, and the internal UpdateId loses its JsonSchema derive so the ABI stops emitting duplicate Keyset2/KeyForDomain2/PublicKeyExtended2/SerializableEdwardsPoint definitions.

I verified the wire format is genuinely unchanged: the retained ABI definitions for Keyset/KeyForDomain/PublicKeyExtended/UpdateId are structurally identical to the ones they replace (abi__abi_has_not_changed.snap:6046 is byte-identical), dtos::PublicKey/Secp256k1PublicKey/Bls12381G2PublicKey all serialize as prefixed strings, and the new round-trip test pins JSON equality per curve — which matters because upgrade_to_current_contract.rs calls init_running on released binaries that still parse the contract-internal Keyset. The conclude_node_migration equality check also still round-trips exactly, including for non-canonical Ed25519 encodings, because both directions go through SerializableEdwardsPoint::to_bytes().

Changes:

• init_running / conclude_node_migration take dtos::Keyset (by value, no longer &Keyset); conversion happens inside, after the state/participant checks in the migration path.
• propose_update returns dtos::UpdateId, vote_update accepts it; new UpdateId(pub u64) DTO in near-mpc-contract-interface, and dtos::ProposedUpdates maps re-keyed from u64 to it.
• New TryIntoContractType impls for Keyset, KeyForDomain, PublicKeyExtended, plus UpdateId in both directions; the PublicKeyExtended impl newly rejects an edwards_point that disagrees with the compressed key.
• Internal UpdateId drops JsonSchema; ABI snapshot loses four duplicated definitions.
• Call sites updated across contract unit/integration/sandbox tests, devnet, e2e cluster, and docs/migration-service.md.

Reviewed changes

Per-file summary

File	Description
crates/contract/src/api/lifecycle.rs	init_running takes dtos::Keyset, converts before logging/validation
crates/contract/src/api/node_migration.rs	conclude_node_migration takes dtos::Keyset by value; conversion deferred past state/participant checks
crates/contract/src/api/update.rs	propose_update → dtos::UpdateId, vote_update accepts it; tests switched to typed ids
crates/contract/src/dto_mapping.rs	New inbound conversions for PublicKeyExtended/KeyForDomain/Keyset and UpdateId both ways; adds keyset round-trip and Edwards-mismatch tests
crates/contract/src/update.rs	Removes JsonSchema derive from internal UpdateId
crates/near-mpc-contract-interface/src/types/updates.rs	Adds UpdateId DTO; ProposedUpdates maps re-keyed to it
crates/near-mpc-contract-interface/src/{lib,call_args,client}.rs	Re-export UpdateId; VoteUpdateArgs/vote_update typed
crates/contract/tests/snapshots/abi__abi_has_not_changed.snap	Drops Keyset2/KeyForDomain2/PublicKeyExtended2/SerializableEdwardsPoint; votes now $ref: UpdateId
crates/contract/src/api/{attestation,foreign_chain_support,governance,test_utils}.rs	Test/fixture call sites pass (&keyset).into_dto_type()
crates/contract/tests/inprocess/common.rs, tests/sandbox/{common,participants_gas}.rs	Build keysets from DTO types instead of crypto_shared::types
crates/contract/tests/sandbox/{contract_configuration,update_votes_cleanup_after_resharing,upgrade_from_current_contract,upgrade_to_current_contract}.rs	Proposal ids/keysets typed as DTOs
crates/devnet/src/mpc.rs, crates/e2e-tests/src/cluster.rs	Wrap/parse update ids as UpdateId
crates/node/src/tests/dto_conversions.rs	Doc comment no longer claims to pin the init_running/conclude_node_migration wire format
docs/migration-service.md	conclude_node_migration(keyset: Keyset) signature updated

Findings

Non-blocking (nits, follow-ups, suggestions):

• crates/contract/src/dto_mapping.rs:682 — the new Edwards-point guard only fires when the DTO's variant tag happens to agree with the parsed key's curve, because the conversion routes through dtos::PublicKey::try_from(&self) and never checks the tag. So {"Ed25519": {"near_public_key_compressed": "secp256k1:…", "edwards_point": […]}} is accepted and edwards_point is silently dropped — precisely what the comment at :696 says it prevents. It is also a small validation loosening versus the previous signature: with &Keyset, serde required the string prefix to match the variant (Bls12381G2PublicKey::from_str → WrongPrefix, crates/near-mpc-crypto-types/src/crypto.rs:420), so {"Bls12381": {"public_key": "ed25519:…"}} used to be rejected at deserialization and now yields an Ed25519 contract key. Nothing exploitable today (init_running is #[private], and conclude_node_migration compares the result against the stored keyset), but the guard is worth making total, e.g.:

  match self {
      dtos::PublicKeyExtended::Ed25519 { near_public_key_compressed, edwards_point } => {
          let compressed: dtos::Ed25519PublicKey = near_public_key_compressed.parse().map_err(…)?;
          let derived = SerializableEdwardsPoint::from_bytes(&compressed).into_option().ok_or(…)?;
          if derived.to_bytes() != edwards_point { return Err(…); }
          Ok(PublicKeyExtended::Ed25519 { near_public_key_compressed: compressed, edwards_point: derived })
      }
      dtos::PublicKeyExtended::Bls12381 { public_key } => {
          let dtos::PublicKey::Bls12381(public_key) = public_key else { return Err(…) };
          Ok(PublicKeyExtended::Bls12381 { public_key })
      }
      dtos::PublicKeyExtended::Secp256k1 { near_public_key } => { … }
  }

  That also removes the need for the comment, since each arm validates what it consumes.
• crates/near-mpc-contract-interface/src/types/updates.rs:28 — borsh::BorshSchema on UpdateId is unused (no borsh-serialized ABI position reaches it; its siblings ProposedUpdates/UpdateHash derive only JsonSchema) and it declares the name "UpdateId", which the contract's own UpdateId also declares (crates/contract/src/update.rs:26). This workspace already needed a manual "MpcPublicKey" declaration to dodge exactly this kind of collision (crates/near-mpc-crypto-types/src/crypto.rs:25), so dropping the derive avoids a latent trap. While there: UpdateId is now a type external callers are expected to import, and it has no doc comment.
• crates/contract/src/api/node_migration.rs:168 — the # Errors list is now incomplete: a malformed keyset produces ConversionError::DataConversion from the conversion added at :198, which is not listed.
• crates/devnet/src/mpc.rs:498 — propose_update's result is still deserialized as u64 here while the adjacent vote_update call was switched to UpdateId(...); using UpdateId on both sides would make the CLI consistent with the newly typed API.
• Pre-existing, worth a follow-up rather than a change here: dtos::PublicKey derives JsonSchema (crates/near-mpc-crypto-types/src/crypto.rs:15) and so publishes a tagged oneOf schema, while its hand-written Serialize/Deserialize (:434, :443) use a plain prefixed string — unlike Secp256k1PublicKey/Ed25519PublicKey/Bls12381G2PublicKey, which all have manual JsonSchema impls returning String. This already misdescribes vote_pk/derived_public_key, and after this PR it is also the schema behind PublicKeyExtended::Bls12381.public_key for init_running/conclude_node_migration (abi__abi_has_not_changed.snap:5290). A #[schemars(with = "String")] on PublicKey would fix all of them.

✅ Approved

[claude]

PR title type suggestion: This PR restructures code to use centralized DTO types across multiple files, which is a refactoring activity. Consider using refactor: instead of chore:.

Suggested title: refactor: use UpdateId and Keyset from dtos

[claude]

PR title type suggestion: Since this PR restructures code across multiple crates to use UpdateId and Keyset from the DTOs module, the type prefix should probably be refactor: instead of chore:.
Suggested title: refactor: use UpdateId and Keyset from dtos

[claude]

PR title type suggestion: This PR restructures code across multiple crates to use shared DTO types, which is refactoring rather than routine maintenance.

Suggested title: refactor: use UpdateId and Keyset from dtos

[claude]

PR title type suggestion: This PR restructures the API to use types from DTOs across multiple modules. refactor might be more appropriate than chore for this kind of change.

Suggested title: refactor(contract): use UpdateId and Keyset from dtos in the API

[claude]

PR title type suggestion: This PR restructures code to use types from the DTO module rather than local definitions. Since source code files are being reorganized, the type prefix should probably be refactor: instead of chore:.

Suggested title: refactor(contract): use UpdateId and Keyset from dtos in the API

[pbeza]

Thanks!

BTW, do we have any other issues besides #4255 and #2480 tracking the use of internal types in the contract?

I think it’d be worth making sure the Claude reviewer catches cases where someone reintroduces internal types into the contract. This refactor has been dragging on for quite a while and has been split across a bunch of random PRs, so once we finally clean it up, we probably don’t want to accidentally bring them back.

[netrome]

Thanks

[gilcu3]

BTW, do we have any other issues besides #4255 and #2480 tracking the use of internal types in the contract?

There is also #4055

I think it’d be worth making sure the Claude reviewer catches cases where someone reintroduces internal types into the contract.

Probably yes, but I have also a less advanced idea as part of #2061. We can have a clippy/deny lint rule forbidding the use of json schemars in the contract. That should catch most cases where a type in the contract is used on the interface.