From 39d12f4e54222bfa2b73032eec95d15d47468a29 Mon Sep 17 00:00:00 2001 From: Jordan Jennings Date: Thu, 19 Sep 2024 17:57:04 -0700 Subject: [PATCH] Implement upgrade compatibility checks client side --- crates/sui-types/src/digests.rs | 56 ++- .../fixtures/upgrade_errors/all_v1/Move.toml | 6 + .../all_v1/sources/UpgradeErrors.move | 95 ++++ .../fixtures/upgrade_errors/all_v2/Move.toml | 6 + .../all_v2/sources/UpgradeErrors.move | 89 ++++ .../upgrade_errors/entry_linking_v1/Move.toml | 6 + .../sources/UpgradeErrors.move | 10 + .../upgrade_errors/entry_linking_v2/Move.toml | 6 + .../sources/UpgradeErrors.move | 9 + .../friend_linking_v1/Move.toml | 6 + .../sources/UpgradeErrors.move | 15 + .../friend_linking_v2/Move.toml | 6 + .../sources/UpgradeErrors.move | 12 + .../struct_missing_v1/Move.toml | 6 + .../sources/UpgradeErrors.move | 13 + .../struct_missing_v2/Move.toml | 6 + .../sources/UpgradeErrors.move | 11 + ...upgrade_compatibility_tests__all_fail.snap | 23 + ...e_compatibility_tests__struct_missing.snap | 6 + .../unit_tests/upgrade_compatibility_tests.rs | 65 +++ crates/sui/src/upgrade_compatibility.rs | 466 ++++++++++++++++++ .../move-binary-format/src/compatibility.rs | 13 + .../programmable_transactions/execution.rs | 11 +- 23 files changed, 929 insertions(+), 13 deletions(-) create mode 100644 crates/sui/src/unit_tests/fixtures/upgrade_errors/all_v1/Move.toml create mode 100644 crates/sui/src/unit_tests/fixtures/upgrade_errors/all_v1/sources/UpgradeErrors.move create mode 100644 crates/sui/src/unit_tests/fixtures/upgrade_errors/all_v2/Move.toml create mode 100644 crates/sui/src/unit_tests/fixtures/upgrade_errors/all_v2/sources/UpgradeErrors.move create mode 100644 crates/sui/src/unit_tests/fixtures/upgrade_errors/entry_linking_v1/Move.toml create mode 100644 crates/sui/src/unit_tests/fixtures/upgrade_errors/entry_linking_v1/sources/UpgradeErrors.move create mode 100644 crates/sui/src/unit_tests/fixtures/upgrade_errors/entry_linking_v2/Move.toml create mode 100644 crates/sui/src/unit_tests/fixtures/upgrade_errors/entry_linking_v2/sources/UpgradeErrors.move create mode 100644 crates/sui/src/unit_tests/fixtures/upgrade_errors/friend_linking_v1/Move.toml create mode 100644 crates/sui/src/unit_tests/fixtures/upgrade_errors/friend_linking_v1/sources/UpgradeErrors.move create mode 100644 crates/sui/src/unit_tests/fixtures/upgrade_errors/friend_linking_v2/Move.toml create mode 100644 crates/sui/src/unit_tests/fixtures/upgrade_errors/friend_linking_v2/sources/UpgradeErrors.move create mode 100644 crates/sui/src/unit_tests/fixtures/upgrade_errors/struct_missing_v1/Move.toml create mode 100644 crates/sui/src/unit_tests/fixtures/upgrade_errors/struct_missing_v1/sources/UpgradeErrors.move create mode 100644 crates/sui/src/unit_tests/fixtures/upgrade_errors/struct_missing_v2/Move.toml create mode 100644 crates/sui/src/unit_tests/fixtures/upgrade_errors/struct_missing_v2/sources/UpgradeErrors.move create mode 100644 crates/sui/src/unit_tests/snapshots/sui__upgrade_compatibility__upgrade_compatibility_tests__all_fail.snap create mode 100644 crates/sui/src/unit_tests/snapshots/sui__upgrade_compatibility__upgrade_compatibility_tests__struct_missing.snap create mode 100644 crates/sui/src/unit_tests/upgrade_compatibility_tests.rs create mode 100644 crates/sui/src/upgrade_compatibility.rs diff --git a/crates/sui-types/src/digests.rs b/crates/sui-types/src/digests.rs index 52444720d158d..0b19e5772253d 100644 --- a/crates/sui-types/src/digests.rs +++ b/crates/sui-types/src/digests.rs @@ -4,7 +4,7 @@ use std::{env, fmt}; use crate::{error::SuiError, sui_serde::Readable}; -use fastcrypto::encoding::{Base58, Encoding}; +use fastcrypto::encoding::{Base58, Encoding, Hex}; use once_cell::sync::{Lazy, OnceCell}; use schemars::JsonSchema; use serde::{Deserialize, Serialize}; @@ -157,6 +157,9 @@ impl fmt::UpperHex for Digest { )] pub struct ChainIdentifier(CheckpointDigest); +pub const MAINNET_CHAIN_IDENTIFIER_BASE58: &str = "4btiuiMPvEENsttpZC7CZ53DruC3MAgfznDbASZ7DR6S"; +pub const TESTNET_CHAIN_IDENTIFIER_BASE58: &str = "69WiPg3DAQiwdxfncX6wYQ2siKwAe6L9BZthQea3JNMD"; + pub static MAINNET_CHAIN_IDENTIFIER: OnceCell = OnceCell::new(); pub static TESTNET_CHAIN_IDENTIFIER: OnceCell = OnceCell::new(); @@ -179,6 +182,24 @@ static SUI_PROTOCOL_CONFIG_CHAIN_OVERRIDE: Lazy> = Lazy::new(|| { }); impl ChainIdentifier { + /// take a short 4 byte identifier and convert it into a ChainIdentifier + /// short ids come from the JSON RPC getChainIdentifier and are encoded in hex + pub fn from_chain_short_id(short_id: &String) -> Option { + if Hex::from_bytes(&Base58::decode(MAINNET_CHAIN_IDENTIFIER_BASE58).ok()?) + .encoded_with_format() + .starts_with(&format!("0x{}", short_id)) + { + Some(get_mainnet_chain_identifier()) + } else if Hex::from_bytes(&Base58::decode(TESTNET_CHAIN_IDENTIFIER_BASE58).ok()?) + .encoded_with_format() + .starts_with(&format!("0x{}", short_id)) + { + Some(get_testnet_chain_identifier()) + } else { + None + } + } + pub fn chain(&self) -> Chain { let mainnet_id = get_mainnet_chain_identifier(); let testnet_id = get_testnet_chain_identifier(); @@ -206,7 +227,7 @@ impl ChainIdentifier { pub fn get_mainnet_chain_identifier() -> ChainIdentifier { let digest = MAINNET_CHAIN_IDENTIFIER.get_or_init(|| { let digest = CheckpointDigest::new( - Base58::decode("4btiuiMPvEENsttpZC7CZ53DruC3MAgfznDbASZ7DR6S") + Base58::decode(MAINNET_CHAIN_IDENTIFIER_BASE58) .expect("mainnet genesis checkpoint digest literal is invalid") .try_into() .expect("Mainnet genesis checkpoint digest literal has incorrect length"), @@ -219,7 +240,7 @@ pub fn get_mainnet_chain_identifier() -> ChainIdentifier { pub fn get_testnet_chain_identifier() -> ChainIdentifier { let digest = TESTNET_CHAIN_IDENTIFIER.get_or_init(|| { let digest = CheckpointDigest::new( - Base58::decode("69WiPg3DAQiwdxfncX6wYQ2siKwAe6L9BZthQea3JNMD") + Base58::decode(TESTNET_CHAIN_IDENTIFIER_BASE58) .expect("testnet genesis checkpoint digest literal is invalid") .try_into() .expect("Testnet genesis checkpoint digest literal has incorrect length"), @@ -1043,3 +1064,32 @@ impl fmt::Debug for ConsensusCommitDigest { .finish() } } + +mod test { + #[allow(unused_imports)] + use crate::digests::ChainIdentifier; + // check that the chain id returns mainnet + #[test] + fn test_chain_id_mainnet() { + let chain_id = ChainIdentifier::from_chain_short_id(&String::from("35834a8a")); + assert_eq!( + chain_id.unwrap().chain(), + sui_protocol_config::Chain::Mainnet + ); + } + + #[test] + fn test_chain_id_testnet() { + let chain_id = ChainIdentifier::from_chain_short_id(&String::from("4c78adac")); + assert_eq!( + chain_id.unwrap().chain(), + sui_protocol_config::Chain::Testnet + ); + } + + #[test] + fn test_chain_id_unknown() { + let chain_id = ChainIdentifier::from_chain_short_id(&String::from("unknown")); + assert_eq!(chain_id, None); + } +} diff --git a/crates/sui/src/unit_tests/fixtures/upgrade_errors/all_v1/Move.toml b/crates/sui/src/unit_tests/fixtures/upgrade_errors/all_v1/Move.toml new file mode 100644 index 0000000000000..70f14c105d35c --- /dev/null +++ b/crates/sui/src/unit_tests/fixtures/upgrade_errors/all_v1/Move.toml @@ -0,0 +1,6 @@ +[package] +name = "upgrades" +edition = "2024.beta" # edition = "legacy" to use legacy (pre-2024) Move + +[addresses] +upgrades = "0x0" \ No newline at end of file diff --git a/crates/sui/src/unit_tests/fixtures/upgrade_errors/all_v1/sources/UpgradeErrors.move b/crates/sui/src/unit_tests/fixtures/upgrade_errors/all_v1/sources/UpgradeErrors.move new file mode 100644 index 0000000000000..f09e4424df3d1 --- /dev/null +++ b/crates/sui/src/unit_tests/fixtures/upgrade_errors/all_v1/sources/UpgradeErrors.move @@ -0,0 +1,95 @@ +// Copyright (c) Mysten Labs, Inc. +// SPDX-License-Identifier: Apache-2.0 + +/// Module: UpgradeErrors + +#[allow(unused_field)] +module upgrades::upgrades { + // struct missing + public struct StructToBeRemoved { + b: u64 + } + + // struct ability mismatch (add) + public struct StructAbilityMismatchAdd {} + + // struct ability mismatch (remove) + public struct StructAbilityMismatchRemove has copy {} + + // struct ability mismatch (change) + public struct StructAbilityMismatchChange has copy {} + + // struct type param mismatch + public struct StructTypeParamMismatch { a: S } + + // struct field mismatch (add) + public struct StructFieldMismatchAdd { + a: u64, + b: u64 + } + + // struct field mismatch (remove) + public struct StructFieldMismatchRemove { + a: u64, + b: u64 + } + + // struct field mismatch (change) + public struct StructFieldMismatchChange { + a: u64, + b: u64 + } + + // enum missing + public enum EnumToBeRemoved { + A, + B + } + + // enum ability mismatch (add) + public enum EnumAbilityMismatchAdd { + A, + } + + // enum ability mismatch (remove) + public enum EnumAbilityMismatchRemove has copy { + A, + } + + // enum ability mismatch (change) + public enum EnumAbilityMismatchChange has copy { + A, + } + + // enum new variant + public enum EnumNewVariant { + A, + B, + C + } + + // enum variant missing + public enum EnumVariantMissing { + A, + B, + } + + // function missing public + public fun function_to_have_public_removed() {} + + // function missing friend + public(package) fun function_to_have_friend_removed() {} + + // function missing entry + + + // function signature mismatch (add) + public fun function_add_arg() {} + + // function signature mismatch (remove) + public fun function_remove_arg(a: u64) {} + + // function signature mismatch (change) + public fun function_change_arg(a: u64) {} +} + diff --git a/crates/sui/src/unit_tests/fixtures/upgrade_errors/all_v2/Move.toml b/crates/sui/src/unit_tests/fixtures/upgrade_errors/all_v2/Move.toml new file mode 100644 index 0000000000000..70f14c105d35c --- /dev/null +++ b/crates/sui/src/unit_tests/fixtures/upgrade_errors/all_v2/Move.toml @@ -0,0 +1,6 @@ +[package] +name = "upgrades" +edition = "2024.beta" # edition = "legacy" to use legacy (pre-2024) Move + +[addresses] +upgrades = "0x0" \ No newline at end of file diff --git a/crates/sui/src/unit_tests/fixtures/upgrade_errors/all_v2/sources/UpgradeErrors.move b/crates/sui/src/unit_tests/fixtures/upgrade_errors/all_v2/sources/UpgradeErrors.move new file mode 100644 index 0000000000000..788cd34148cd6 --- /dev/null +++ b/crates/sui/src/unit_tests/fixtures/upgrade_errors/all_v2/sources/UpgradeErrors.move @@ -0,0 +1,89 @@ +// Copyright (c) Mysten Labs, Inc. +// SPDX-License-Identifier: Apache-2.0 +/// Module: UpgradeErrors + +#[allow(unused_field)] +module upgrades::upgrades { + // struct missing + // public struct StructToBeRemoved {} + + // struct ability mismatch (add) + public struct StructAbilityMismatchAdd has copy {} // added the copy ability where none existed + + // struct field mismatch (remove) + public struct StructAbilityMismatchRemove {} // removed the copy ability + + // struct field mismatch (change) + public struct StructAbilityMismatchChange has drop {} // changed from drop to copy + + // struct type param mismatch + public struct StructTypeParamMismatch { a: T } // changed S to T + + // struct field mismatch (add) + public struct StructFieldMismatchAdd { + a: u64, + b: u64, + c: u64, // added + } + + // struct field mismatch (remove) + public struct StructFieldMismatchRemove { + a: u64, + // removed b: u64 + } + + // struct field mismatch (change) + public struct StructFieldMismatchChange { + a: u64, + b: u8 // changed b from u64 to u8 + } + + // enum missing + // public enum EnumToBeRemoved {} + + // enum ability mismatch (add) + public enum EnumAbilityMismatchAdd has copy { + A, + } + + // enum ability mismatch (remove) + public enum EnumAbilityMismatchRemove { + A, + } + + // enum ability mismatch (change) + public enum EnumAbilityMismatchChange has drop { + A, + } + + // enum new variant + public enum EnumNewVariant { + A, + B, + C, + D // new variant + } + + // enum variant missing + public enum EnumVariantMissing { + A, + // remove B, + } + + // function missing public + fun function_to_have_public_removed() {} + + // function missing friend + fun function_to_have_friend_removed() {} + + // function missing entry + + // function signature mismatch (add) + public fun function_add_arg(a: u64) {} + + // function signature mismatch (remove) + public fun function_remove_arg() {} + + // function signature mismatch (change) + public fun function_change_arg(a: u8) {} // now has u8 instead of u64 +} diff --git a/crates/sui/src/unit_tests/fixtures/upgrade_errors/entry_linking_v1/Move.toml b/crates/sui/src/unit_tests/fixtures/upgrade_errors/entry_linking_v1/Move.toml new file mode 100644 index 0000000000000..39cc2752ab46d --- /dev/null +++ b/crates/sui/src/unit_tests/fixtures/upgrade_errors/entry_linking_v1/Move.toml @@ -0,0 +1,6 @@ +[package] +name = "upgrades" +edition = "2024.beta" # edition = "legacy" to use legacy (pre-2024) Move + +[addresses] +upgrades = "0x0" diff --git a/crates/sui/src/unit_tests/fixtures/upgrade_errors/entry_linking_v1/sources/UpgradeErrors.move b/crates/sui/src/unit_tests/fixtures/upgrade_errors/entry_linking_v1/sources/UpgradeErrors.move new file mode 100644 index 0000000000000..626e013bc3218 --- /dev/null +++ b/crates/sui/src/unit_tests/fixtures/upgrade_errors/entry_linking_v1/sources/UpgradeErrors.move @@ -0,0 +1,10 @@ +// Copyright (c) Mysten Labs, Inc. +// SPDX-License-Identifier: Apache-2.0 + +/// Module: UpgradeErrors + +#[allow(unused_field)] +module upgrades::upgrades { + entry fun entry_to_be_removed() {} +} + diff --git a/crates/sui/src/unit_tests/fixtures/upgrade_errors/entry_linking_v2/Move.toml b/crates/sui/src/unit_tests/fixtures/upgrade_errors/entry_linking_v2/Move.toml new file mode 100644 index 0000000000000..39cc2752ab46d --- /dev/null +++ b/crates/sui/src/unit_tests/fixtures/upgrade_errors/entry_linking_v2/Move.toml @@ -0,0 +1,6 @@ +[package] +name = "upgrades" +edition = "2024.beta" # edition = "legacy" to use legacy (pre-2024) Move + +[addresses] +upgrades = "0x0" diff --git a/crates/sui/src/unit_tests/fixtures/upgrade_errors/entry_linking_v2/sources/UpgradeErrors.move b/crates/sui/src/unit_tests/fixtures/upgrade_errors/entry_linking_v2/sources/UpgradeErrors.move new file mode 100644 index 0000000000000..690e45c404cbf --- /dev/null +++ b/crates/sui/src/unit_tests/fixtures/upgrade_errors/entry_linking_v2/sources/UpgradeErrors.move @@ -0,0 +1,9 @@ +// Copyright (c) Mysten Labs, Inc. +// SPDX-License-Identifier: Apache-2.0 + +/// Module: UpgradeErrors + +#[allow(unused_field)] +module upgrades::upgrades { + fun entry_to_be_removed() {} +} diff --git a/crates/sui/src/unit_tests/fixtures/upgrade_errors/friend_linking_v1/Move.toml b/crates/sui/src/unit_tests/fixtures/upgrade_errors/friend_linking_v1/Move.toml new file mode 100644 index 0000000000000..39cc2752ab46d --- /dev/null +++ b/crates/sui/src/unit_tests/fixtures/upgrade_errors/friend_linking_v1/Move.toml @@ -0,0 +1,6 @@ +[package] +name = "upgrades" +edition = "2024.beta" # edition = "legacy" to use legacy (pre-2024) Move + +[addresses] +upgrades = "0x0" diff --git a/crates/sui/src/unit_tests/fixtures/upgrade_errors/friend_linking_v1/sources/UpgradeErrors.move b/crates/sui/src/unit_tests/fixtures/upgrade_errors/friend_linking_v1/sources/UpgradeErrors.move new file mode 100644 index 0000000000000..77beb53e4df95 --- /dev/null +++ b/crates/sui/src/unit_tests/fixtures/upgrade_errors/friend_linking_v1/sources/UpgradeErrors.move @@ -0,0 +1,15 @@ +// Copyright (c) Mysten Labs, Inc. +// SPDX-License-Identifier: Apache-2.0 + +/// Module: UpgradeErrors + +#[allow(unused_field)] +module upgrades::upgrades { + fun call_friend() { + upgrades::upgrades_friend::friend_to_be_dropped(); + } +} + +module upgrades::upgrades_friend { + public(package) fun friend_to_be_dropped() {} +} \ No newline at end of file diff --git a/crates/sui/src/unit_tests/fixtures/upgrade_errors/friend_linking_v2/Move.toml b/crates/sui/src/unit_tests/fixtures/upgrade_errors/friend_linking_v2/Move.toml new file mode 100644 index 0000000000000..39cc2752ab46d --- /dev/null +++ b/crates/sui/src/unit_tests/fixtures/upgrade_errors/friend_linking_v2/Move.toml @@ -0,0 +1,6 @@ +[package] +name = "upgrades" +edition = "2024.beta" # edition = "legacy" to use legacy (pre-2024) Move + +[addresses] +upgrades = "0x0" diff --git a/crates/sui/src/unit_tests/fixtures/upgrade_errors/friend_linking_v2/sources/UpgradeErrors.move b/crates/sui/src/unit_tests/fixtures/upgrade_errors/friend_linking_v2/sources/UpgradeErrors.move new file mode 100644 index 0000000000000..f412c97e05f3b --- /dev/null +++ b/crates/sui/src/unit_tests/fixtures/upgrade_errors/friend_linking_v2/sources/UpgradeErrors.move @@ -0,0 +1,12 @@ +// Copyright (c) Mysten Labs, Inc. +// SPDX-License-Identifier: Apache-2.0 + +/// Module: UpgradeErrors + +#[allow(unused_field)] +module upgrades::upgrades { + fun call_friend() {} +} + + +module upgrades::upgrades_friend {} diff --git a/crates/sui/src/unit_tests/fixtures/upgrade_errors/struct_missing_v1/Move.toml b/crates/sui/src/unit_tests/fixtures/upgrade_errors/struct_missing_v1/Move.toml new file mode 100644 index 0000000000000..39cc2752ab46d --- /dev/null +++ b/crates/sui/src/unit_tests/fixtures/upgrade_errors/struct_missing_v1/Move.toml @@ -0,0 +1,6 @@ +[package] +name = "upgrades" +edition = "2024.beta" # edition = "legacy" to use legacy (pre-2024) Move + +[addresses] +upgrades = "0x0" diff --git a/crates/sui/src/unit_tests/fixtures/upgrade_errors/struct_missing_v1/sources/UpgradeErrors.move b/crates/sui/src/unit_tests/fixtures/upgrade_errors/struct_missing_v1/sources/UpgradeErrors.move new file mode 100644 index 0000000000000..70db96559ad50 --- /dev/null +++ b/crates/sui/src/unit_tests/fixtures/upgrade_errors/struct_missing_v1/sources/UpgradeErrors.move @@ -0,0 +1,13 @@ +// Copyright (c) Mysten Labs, Inc. +// SPDX-License-Identifier: Apache-2.0 + +/// Module: UpgradeErrors + +#[allow(unused_field)] +module upgrades::upgrades { + // struct missing + public struct StructToBeRemoved { + b: u64 + } +} + diff --git a/crates/sui/src/unit_tests/fixtures/upgrade_errors/struct_missing_v2/Move.toml b/crates/sui/src/unit_tests/fixtures/upgrade_errors/struct_missing_v2/Move.toml new file mode 100644 index 0000000000000..70f14c105d35c --- /dev/null +++ b/crates/sui/src/unit_tests/fixtures/upgrade_errors/struct_missing_v2/Move.toml @@ -0,0 +1,6 @@ +[package] +name = "upgrades" +edition = "2024.beta" # edition = "legacy" to use legacy (pre-2024) Move + +[addresses] +upgrades = "0x0" \ No newline at end of file diff --git a/crates/sui/src/unit_tests/fixtures/upgrade_errors/struct_missing_v2/sources/UpgradeErrors.move b/crates/sui/src/unit_tests/fixtures/upgrade_errors/struct_missing_v2/sources/UpgradeErrors.move new file mode 100644 index 0000000000000..542e2edca5b60 --- /dev/null +++ b/crates/sui/src/unit_tests/fixtures/upgrade_errors/struct_missing_v2/sources/UpgradeErrors.move @@ -0,0 +1,11 @@ +// Copyright (c) Mysten Labs, Inc. +// SPDX-License-Identifier: Apache-2.0 + +/// Module: UpgradeErrors + +#[allow(unused_field)] +module upgrades::upgrades { + // struct missing + // public struct StructToBeRemoved {} +} + diff --git a/crates/sui/src/unit_tests/snapshots/sui__upgrade_compatibility__upgrade_compatibility_tests__all_fail.snap b/crates/sui/src/unit_tests/snapshots/sui__upgrade_compatibility__upgrade_compatibility_tests__all_fail.snap new file mode 100644 index 0000000000000..f75df286256ed --- /dev/null +++ b/crates/sui/src/unit_tests/snapshots/sui__upgrade_compatibility__upgrade_compatibility_tests__all_fail.snap @@ -0,0 +1,23 @@ +--- +source: crates/sui/src/unit_tests/upgrade_compatibility_tests.rs +expression: err.to_string() +--- +Upgrade compatibility check failed with the following errors: +- StructAbilityMismatch { name: Identifier("StructAbilityMismatchAdd"), old_struct: Struct { abilities: [], type_parameters: [], fields: [Field { name: Identifier("dummy_field"), type_: Bool }] }, new_struct: Struct { abilities: [Copy, ], type_parameters: [], fields: [Field { name: Identifier("dummy_field"), type_: Bool }] } } +- StructAbilityMismatch { name: Identifier("StructAbilityMismatchChange"), old_struct: Struct { abilities: [Copy, ], type_parameters: [], fields: [Field { name: Identifier("dummy_field"), type_: Bool }] }, new_struct: Struct { abilities: [Drop, ], type_parameters: [], fields: [Field { name: Identifier("dummy_field"), type_: Bool }] } } +- StructAbilityMismatch { name: Identifier("StructAbilityMismatchRemove"), old_struct: Struct { abilities: [Copy, ], type_parameters: [], fields: [Field { name: Identifier("dummy_field"), type_: Bool }] }, new_struct: Struct { abilities: [], type_parameters: [], fields: [Field { name: Identifier("dummy_field"), type_: Bool }] } } +- StructFieldMismatch { name: Identifier("StructFieldMismatchAdd"), old_struct: Struct { abilities: [], type_parameters: [], fields: [Field { name: Identifier("a"), type_: U64 }, Field { name: Identifier("b"), type_: U64 }] }, new_struct: Struct { abilities: [], type_parameters: [], fields: [Field { name: Identifier("a"), type_: U64 }, Field { name: Identifier("b"), type_: U64 }, Field { name: Identifier("c"), type_: U64 }] } } +- StructFieldMismatch { name: Identifier("StructFieldMismatchChange"), old_struct: Struct { abilities: [], type_parameters: [], fields: [Field { name: Identifier("a"), type_: U64 }, Field { name: Identifier("b"), type_: U64 }] }, new_struct: Struct { abilities: [], type_parameters: [], fields: [Field { name: Identifier("a"), type_: U64 }, Field { name: Identifier("b"), type_: U8 }] } } +- StructFieldMismatch { name: Identifier("StructFieldMismatchRemove"), old_struct: Struct { abilities: [], type_parameters: [], fields: [Field { name: Identifier("a"), type_: U64 }, Field { name: Identifier("b"), type_: U64 }] }, new_struct: Struct { abilities: [], type_parameters: [], fields: [Field { name: Identifier("a"), type_: U64 }] } } +- StructMissing { name: Identifier("StructToBeRemoved"), old_struct: Struct { abilities: [], type_parameters: [], fields: [Field { name: Identifier("b"), type_: U64 }] } } +- StructTypeParamMismatch { name: Identifier("StructTypeParamMismatch"), old_struct: Struct { abilities: [], type_parameters: [DatatypeTyParameter { constraints: [], is_phantom: false }, DatatypeTyParameter { constraints: [], is_phantom: false }], fields: [Field { name: Identifier("a"), type_: TypeParameter(0) }] }, new_struct: Struct { abilities: [], type_parameters: [DatatypeTyParameter { constraints: [], is_phantom: false }], fields: [Field { name: Identifier("a"), type_: TypeParameter(0) }] } } +- EnumAbilityMismatch { name: Identifier("EnumAbilityMismatchAdd"), old_enum: Enum { abilities: [], type_parameters: [], variants: [Variant { name: Identifier("A"), fields: [] }] }, new_enum: Enum { abilities: [Copy, ], type_parameters: [], variants: [Variant { name: Identifier("A"), fields: [] }] } } +- EnumAbilityMismatch { name: Identifier("EnumAbilityMismatchChange"), old_enum: Enum { abilities: [Copy, ], type_parameters: [], variants: [Variant { name: Identifier("A"), fields: [] }] }, new_enum: Enum { abilities: [Drop, ], type_parameters: [], variants: [Variant { name: Identifier("A"), fields: [] }] } } +- EnumAbilityMismatch { name: Identifier("EnumAbilityMismatchRemove"), old_enum: Enum { abilities: [Copy, ], type_parameters: [], variants: [Variant { name: Identifier("A"), fields: [] }] }, new_enum: Enum { abilities: [], type_parameters: [], variants: [Variant { name: Identifier("A"), fields: [] }] } } +- EnumNewVariant { name: Identifier("EnumNewVariant"), old_enum: Enum { abilities: [], type_parameters: [], variants: [Variant { name: Identifier("A"), fields: [] }, Variant { name: Identifier("B"), fields: [] }, Variant { name: Identifier("C"), fields: [] }] }, new_enum: Enum { abilities: [], type_parameters: [], variants: [Variant { name: Identifier("A"), fields: [] }, Variant { name: Identifier("B"), fields: [] }, Variant { name: Identifier("C"), fields: [] }, Variant { name: Identifier("D"), fields: [] }] } } +- EnumMissing { name: Identifier("EnumToBeRemoved"), old_enum: Enum { abilities: [], type_parameters: [], variants: [Variant { name: Identifier("A"), fields: [] }, Variant { name: Identifier("B"), fields: [] }] } } +- EnumVariantMissing { name: Identifier("EnumVariantMissing"), old_enum: Enum { abilities: [], type_parameters: [], variants: [Variant { name: Identifier("A"), fields: [] }, Variant { name: Identifier("B"), fields: [] }] }, tag: 1 } +- FunctionSignatureMismatch { name: Identifier("function_add_arg"), old_function: Function { visibility: Public, is_entry: false, type_parameters: [], parameters: [], return_: [], code: [Ret] }, new_function: Function { visibility: Public, is_entry: false, type_parameters: [], parameters: [U64], return_: [], code: [Ret] } } +- FunctionSignatureMismatch { name: Identifier("function_change_arg"), old_function: Function { visibility: Public, is_entry: false, type_parameters: [], parameters: [U64], return_: [], code: [Ret] }, new_function: Function { visibility: Public, is_entry: false, type_parameters: [], parameters: [U8], return_: [], code: [Ret] } } +- FunctionSignatureMismatch { name: Identifier("function_remove_arg"), old_function: Function { visibility: Public, is_entry: false, type_parameters: [], parameters: [U64], return_: [], code: [Ret] }, new_function: Function { visibility: Public, is_entry: false, type_parameters: [], parameters: [], return_: [], code: [Ret] } } +- FunctionLostPublicVisibility { name: Identifier("function_to_have_public_removed"), old_function: Function { visibility: Public, is_entry: false, type_parameters: [], parameters: [], return_: [], code: [Ret] } } diff --git a/crates/sui/src/unit_tests/snapshots/sui__upgrade_compatibility__upgrade_compatibility_tests__struct_missing.snap b/crates/sui/src/unit_tests/snapshots/sui__upgrade_compatibility__upgrade_compatibility_tests__struct_missing.snap new file mode 100644 index 0000000000000..faa586a1414ad --- /dev/null +++ b/crates/sui/src/unit_tests/snapshots/sui__upgrade_compatibility__upgrade_compatibility_tests__struct_missing.snap @@ -0,0 +1,6 @@ +--- +source: crates/sui/src/unit_tests/upgrade_compatibility_tests.rs +expression: err.to_string() +--- +Upgrade compatibility check failed with the following errors: +- StructMissing { name: Identifier("StructToBeRemoved"), old_struct: Struct { abilities: [], type_parameters: [], fields: [Field { name: Identifier("b"), type_: U64 }] } } diff --git a/crates/sui/src/unit_tests/upgrade_compatibility_tests.rs b/crates/sui/src/unit_tests/upgrade_compatibility_tests.rs new file mode 100644 index 0000000000000..1f0e38eae9995 --- /dev/null +++ b/crates/sui/src/unit_tests/upgrade_compatibility_tests.rs @@ -0,0 +1,65 @@ +// Copyright (c) Mysten Labs, Inc. +// SPDX-License-Identifier: Apache-2.0 + +use crate::upgrade_compatibility::compare_packages; +use insta::assert_snapshot; +use move_binary_format::CompiledModule; +use std::path::PathBuf; +use sui_move_build::BuildConfig; + +#[test] +fn test_all_fail() { + let (pkg_v1, pkg_v2) = get_packages("all"); + + let result = compare_packages(pkg_v1, pkg_v2); + assert!(result.is_err()); + let err = result.unwrap_err(); + + assert_snapshot!(err.to_string()); +} + +#[test] +fn test_struct_missing() { + let (pkg_v1, pkg_v2) = get_packages("struct_missing"); + let result = compare_packages(pkg_v1, pkg_v2); + + assert!(result.is_err()); + let err = result.unwrap_err(); + assert_snapshot!(err.to_string()); +} + +#[test] +fn test_friend_link_ok() { + let (pkg_v1, pkg_v2) = get_packages("friend_linking"); + // upgrade compatibility ignores friend linking + assert!(compare_packages(pkg_v1, pkg_v2).is_ok()); +} + +#[test] +fn test_entry_linking_ok() { + let (pkg_v1, pkg_v2) = get_packages("entry_linking"); + // upgrade compatibility ignores entry linking + assert!(compare_packages(pkg_v1, pkg_v2).is_ok()); +} + +fn get_packages(name: &str) -> (Vec, Vec) { + let mut path: PathBuf = PathBuf::from(env!("CARGO_MANIFEST_DIR")); + path.push("src/unit_tests/fixtures/upgrade_errors/"); + path.push(format!("{}_v1", name)); + + let pkg_v1 = BuildConfig::new_for_testing() + .build(&path) + .unwrap() + .into_modules(); + + let mut path: PathBuf = PathBuf::from(env!("CARGO_MANIFEST_DIR")); + path.push("src/unit_tests/fixtures/upgrade_errors/"); + path.push(format!("{}_v2", name)); + + let pkg_v2 = BuildConfig::new_for_testing() + .build(&path) + .unwrap() + .into_modules(); + + (pkg_v1, pkg_v2) +} diff --git a/crates/sui/src/upgrade_compatibility.rs b/crates/sui/src/upgrade_compatibility.rs new file mode 100644 index 0000000000000..d2a9eaf39813a --- /dev/null +++ b/crates/sui/src/upgrade_compatibility.rs @@ -0,0 +1,466 @@ +// Copyright (c) Mysten Labs, Inc. +// SPDX-License-Identifier: Apache-2.0 + +#[path = "unit_tests/upgrade_compatibility_tests.rs"] +#[cfg(test)] +mod upgrade_compatibility_tests; + +use anyhow::{anyhow, Context, Error}; +use std::collections::{BTreeSet, HashMap}; +use thiserror::Error; + +use move_binary_format::{ + compatibility::Compatibility, + compatibility_mode::CompatibilityMode, + file_format::Visibility, + normalized::{Enum, Function, Module, Struct}, + CompiledModule, +}; +use move_core_types::{ + account_address::AccountAddress, + identifier::{IdentStr, Identifier}, + language_storage::ModuleId, +}; +use sui_json_rpc_types::{SuiObjectDataOptions, SuiRawData}; +use sui_protocol_config::ProtocolConfig; +use sui_sdk::SuiClient; +use sui_types::{base_types::ObjectID, execution_config_utils::to_binary_config}; + +/// Errors that can occur during upgrade compatibility checks. +/// one-to-one related to the underlying trait functions see: [`CompatibilityMode`] +#[derive(Debug)] +pub(crate) enum UpgradeCompatibilityModeError { + StructMissing { + name: Identifier, + old_struct: Struct, + }, + StructAbilityMismatch { + name: Identifier, + old_struct: Struct, + new_struct: Struct, + }, + StructTypeParamMismatch { + name: Identifier, + old_struct: Struct, + new_struct: Struct, + }, + StructFieldMismatch { + name: Identifier, + old_struct: Struct, + new_struct: Struct, + }, + EnumMissing { + name: Identifier, + old_enum: Enum, + }, + EnumAbilityMismatch { + name: Identifier, + old_enum: Enum, + new_enum: Enum, + }, + EnumTypeParamMismatch { + name: Identifier, + old_enum: Enum, + new_enum: Enum, + }, + EnumNewVariant { + name: Identifier, + old_enum: Enum, + new_enum: Enum, + }, + EnumVariantMissing { + name: Identifier, + old_enum: Enum, + tag: usize, + }, + EnumVariantMismatch { + name: Identifier, + old_enum: Enum, + new_enum: Enum, + tag: usize, + }, + FunctionMissingPublic { + name: Identifier, + old_function: Function, + }, + FunctionMissingFriend { + name: Identifier, + old_function: Function, + }, + FunctionMissingEntry { + name: Identifier, + old_function: Function, + }, + FunctionSignatureMismatch { + name: Identifier, + old_function: Function, + new_function: Function, + }, + FunctionLostPublicVisibility { + name: Identifier, + old_function: Function, + }, + FunctionLostFriendVisibility { + name: Identifier, + old_function: Function, + }, + FunctionEntryCompatibility { + name: Identifier, + old_function: Function, + new_function: Function, + }, + FriendModuleMissing(BTreeSet, BTreeSet), +} + +impl UpgradeCompatibilityModeError { + fn breaks_compatibility(&self, compatability: &Compatibility) -> bool { + match self { + UpgradeCompatibilityModeError::StructAbilityMismatch { .. } + | UpgradeCompatibilityModeError::StructTypeParamMismatch { .. } + | UpgradeCompatibilityModeError::EnumAbilityMismatch { .. } + | UpgradeCompatibilityModeError::EnumTypeParamMismatch { .. } + | UpgradeCompatibilityModeError::FunctionMissingPublic { .. } + | UpgradeCompatibilityModeError::FunctionLostPublicVisibility { .. } => { + compatability.check_datatype_and_pub_function_linking + } + + UpgradeCompatibilityModeError::StructFieldMismatch { .. } + | UpgradeCompatibilityModeError::EnumVariantMissing { .. } + | UpgradeCompatibilityModeError::EnumVariantMismatch { .. } => { + compatability.check_datatype_layout + } + + UpgradeCompatibilityModeError::StructMissing { .. } + | UpgradeCompatibilityModeError::EnumMissing { .. } => { + compatability.check_datatype_and_pub_function_linking + || compatability.check_datatype_layout + } + + UpgradeCompatibilityModeError::FunctionSignatureMismatch { old_function, .. } => { + if old_function.visibility == Visibility::Public { + return compatability.check_datatype_and_pub_function_linking; + } else if old_function.visibility == Visibility::Friend { + return compatability.check_friend_linking; + } + if old_function.is_entry { + compatability.check_private_entry_linking + } else { + false + } + } + + UpgradeCompatibilityModeError::FunctionMissingFriend { .. } + | UpgradeCompatibilityModeError::FunctionLostFriendVisibility { .. } + | UpgradeCompatibilityModeError::FriendModuleMissing(_, _) => { + compatability.check_friend_linking + } + + UpgradeCompatibilityModeError::FunctionMissingEntry { .. } + | UpgradeCompatibilityModeError::FunctionEntryCompatibility { .. } => { + compatability.check_private_entry_linking + } + UpgradeCompatibilityModeError::EnumNewVariant { .. } => { + compatability.disallow_new_variants + } + } + } +} + +/// A compatibility mode that collects errors as a vector of enums which describe the error causes +#[derive(Default)] +pub(crate) struct CliCompatibilityMode { + errors: Vec, +} + +impl CompatibilityMode for CliCompatibilityMode { + type Error = anyhow::Error; + // ignored, address is not populated pre-tx + fn module_id_mismatch( + &mut self, + _old_addr: &AccountAddress, + _old_name: &IdentStr, + _new_addr: &AccountAddress, + _new_name: &IdentStr, + ) { + } + + fn struct_missing(&mut self, name: &Identifier, old_struct: &Struct) { + self.errors + .push(UpgradeCompatibilityModeError::StructMissing { + name: name.clone(), + old_struct: old_struct.clone(), + }); + } + + fn struct_ability_mismatch( + &mut self, + name: &Identifier, + old_struct: &Struct, + new_struct: &Struct, + ) { + self.errors + .push(UpgradeCompatibilityModeError::StructAbilityMismatch { + name: name.clone(), + old_struct: old_struct.clone(), + new_struct: new_struct.clone(), + }); + } + + fn struct_type_param_mismatch( + &mut self, + name: &Identifier, + old_struct: &Struct, + new_struct: &Struct, + ) { + self.errors + .push(UpgradeCompatibilityModeError::StructTypeParamMismatch { + name: name.clone(), + old_struct: old_struct.clone(), + new_struct: new_struct.clone(), + }); + } + + fn struct_field_mismatch( + &mut self, + name: &Identifier, + old_struct: &Struct, + new_struct: &Struct, + ) { + self.errors + .push(UpgradeCompatibilityModeError::StructFieldMismatch { + name: name.clone(), + old_struct: old_struct.clone(), + new_struct: new_struct.clone(), + }); + } + + fn enum_missing(&mut self, name: &Identifier, old_enum: &Enum) { + self.errors + .push(UpgradeCompatibilityModeError::EnumMissing { + name: name.clone(), + old_enum: old_enum.clone(), + }); + } + + fn enum_ability_mismatch(&mut self, name: &Identifier, old_enum: &Enum, new_enum: &Enum) { + self.errors + .push(UpgradeCompatibilityModeError::EnumAbilityMismatch { + name: name.clone(), + old_enum: old_enum.clone(), + new_enum: new_enum.clone(), + }); + } + + fn enum_type_param_mismatch(&mut self, name: &Identifier, old_enum: &Enum, new_enum: &Enum) { + self.errors + .push(UpgradeCompatibilityModeError::EnumTypeParamMismatch { + name: name.clone(), + old_enum: old_enum.clone(), + new_enum: new_enum.clone(), + }); + } + + fn enum_new_variant(&mut self, name: &Identifier, old_enum: &Enum, new_enum: &Enum) { + self.errors + .push(UpgradeCompatibilityModeError::EnumNewVariant { + name: name.clone(), + old_enum: old_enum.clone(), + new_enum: new_enum.clone(), + }); + } + + fn enum_variant_missing(&mut self, name: &Identifier, old_enum: &Enum, tag: usize) { + self.errors + .push(UpgradeCompatibilityModeError::EnumVariantMissing { + name: name.clone(), + old_enum: old_enum.clone(), + tag, + }); + } + + fn enum_variant_mismatch( + &mut self, + name: &Identifier, + old_enum: &Enum, + new_enum: &Enum, + variant_idx: usize, + ) { + self.errors + .push(UpgradeCompatibilityModeError::EnumVariantMismatch { + name: name.clone(), + old_enum: old_enum.clone(), + new_enum: new_enum.clone(), + tag: variant_idx, + }); + } + + fn function_missing_public(&mut self, name: &Identifier, old_function: &Function) { + self.errors + .push(UpgradeCompatibilityModeError::FunctionMissingPublic { + name: name.clone(), + old_function: old_function.clone(), + }); + } + + fn function_missing_friend(&mut self, name: &Identifier, old_function: &Function) { + self.errors + .push(UpgradeCompatibilityModeError::FunctionMissingFriend { + name: name.clone(), + old_function: old_function.clone(), + }); + } + + fn function_missing_entry(&mut self, name: &Identifier, old_function: &Function) { + self.errors + .push(UpgradeCompatibilityModeError::FunctionMissingEntry { + name: name.clone(), + old_function: old_function.clone(), + }); + } + + fn function_signature_mismatch( + &mut self, + name: &Identifier, + old_function: &Function, + new_function: &Function, + ) { + self.errors + .push(UpgradeCompatibilityModeError::FunctionSignatureMismatch { + name: name.clone(), + old_function: old_function.clone(), + new_function: new_function.clone(), + }); + } + + fn function_lost_public_visibility(&mut self, name: &Identifier, old_function: &Function) { + self.errors.push( + UpgradeCompatibilityModeError::FunctionLostPublicVisibility { + name: name.clone(), + old_function: old_function.clone(), + }, + ); + } + + fn function_lost_friend_visibility(&mut self, name: &Identifier, old_function: &Function) { + self.errors.push( + UpgradeCompatibilityModeError::FunctionLostFriendVisibility { + name: name.clone(), + old_function: old_function.clone(), + }, + ); + } + + fn function_entry_compatibility( + &mut self, + name: &Identifier, + old_function: &Function, + new_function: &Function, + ) { + self.errors + .push(UpgradeCompatibilityModeError::FunctionEntryCompatibility { + name: name.clone(), + old_function: old_function.clone(), + new_function: new_function.clone(), + }); + } + + fn friend_module_missing( + &mut self, + old_modules: BTreeSet, + new_modules: BTreeSet, + ) { + self.errors + .push(UpgradeCompatibilityModeError::FriendModuleMissing( + old_modules.clone(), + new_modules.clone(), + )); + } + + fn finish(&self, compatability: &Compatibility) -> Result<(), Self::Error> { + let errors: Vec = self + .errors + .iter() + .filter(|e| e.breaks_compatibility(compatability)) + .map(|e| format!("- {:?}", e)) + .collect(); + + if !errors.is_empty() { + return Err(anyhow!( + "Upgrade compatibility check failed with the following errors:\n{}", + errors.join("\n") + )); + } + Ok(()) + } +} + +/// Check the upgrade compatibility of a new package with an existing on-chain package. +pub(crate) async fn check_compatibility( + client: &SuiClient, + package_id: ObjectID, + compiled_modules: &[Vec], + protocol_config: ProtocolConfig, +) -> Result<(), Error> { + let new_modules = compiled_modules + .iter() + .map(|b| CompiledModule::deserialize_with_config(b, &to_binary_config(&protocol_config))) + .collect::, _>>() + .context("Unable to to deserialize compiled module")?; + + let existing_obj_read = client + .read_api() + .get_object_with_options(package_id, SuiObjectDataOptions::new().with_bcs()) + .await + .context("Unable to get existing package")?; + + let existing_obj = existing_obj_read + .into_object() + .context("Unable to get existing package")? + .bcs + .ok_or_else(|| anyhow!("Unable to read object"))?; + + let existing_package = match existing_obj { + SuiRawData::Package(pkg) => Ok(pkg), + SuiRawData::MoveObject(_) => Err(anyhow!("Object found when package expected")), + }?; + + let existing_modules = existing_package + .module_map + .iter() + .map(|m| CompiledModule::deserialize_with_config(m.1, &to_binary_config(&protocol_config))) + .collect::, _>>() + .context("Unable to get existing package")?; + + compare_packages(existing_modules, new_modules) +} + +fn compare_packages( + existing_modules: Vec, + new_modules: Vec, +) -> Result<(), Error> { + // create a map from the new modules + let new_modules_map: HashMap = new_modules + .iter() + .map(|m| (m.self_id().name().to_owned(), m.clone())) + .collect(); + + // for each existing find the new one run compatibility check + for existing_module in existing_modules { + let name = existing_module.self_id().name().to_owned(); + + // find the new module with the same name + match new_modules_map.get(&name) { + Some(new_module) => { + Compatibility::upgrade_check().check_with_mode::( + &Module::new(&existing_module), + &Module::new(new_module), + )?; + } + None => { + Err(anyhow!("Module {} is missing from the package", name))?; + } + } + } + + Ok(()) +} diff --git a/external-crates/move/crates/move-binary-format/src/compatibility.rs b/external-crates/move/crates/move-binary-format/src/compatibility.rs index aab2606ca936e..0b4a7054d90da 100644 --- a/external-crates/move/crates/move-binary-format/src/compatibility.rs +++ b/external-crates/move/crates/move-binary-format/src/compatibility.rs @@ -87,6 +87,19 @@ impl Compatibility { } } + pub fn upgrade_check() -> Self { + Self { + check_datatype_and_pub_function_linking: true, + check_datatype_layout: true, + check_friend_linking: false, + check_private_entry_linking: false, + disallowed_new_abilities: AbilitySet::ALL, + disallow_change_datatype_type_params: true, + // We disallow adding new variants to enums for now + disallow_new_variants: true, + } + } + pub fn need_check_compat(&self) -> bool { self != &Self::no_check() } diff --git a/sui-execution/latest/sui-adapter/src/programmable_transactions/execution.rs b/sui-execution/latest/sui-adapter/src/programmable_transactions/execution.rs index 341937c2bc165..12f144fbb04cf 100644 --- a/sui-execution/latest/sui-adapter/src/programmable_transactions/execution.rs +++ b/sui-execution/latest/sui-adapter/src/programmable_transactions/execution.rs @@ -686,16 +686,7 @@ mod checked { UpgradePolicy::Additive => InclusionCheck::Subset.check(cur_module, new_module), UpgradePolicy::DepOnly => InclusionCheck::Equal.check(cur_module, new_module), UpgradePolicy::Compatible => { - let compatibility = Compatibility { - check_datatype_and_pub_function_linking: true, - check_datatype_layout: true, - check_friend_linking: false, - check_private_entry_linking: false, - disallowed_new_abilities: AbilitySet::ALL, - disallow_change_datatype_type_params: true, - // We disallow adding new variants to enums for now - disallow_new_variants: true, - }; + let compatibility = Compatibility::upgrade_check(); compatibility.check(cur_module, new_module) }