Skip to content

Commit 6f0699d

Browse files
tzakiantzakian
authored
[upgrade-compatibility] Disallow adding key ability to types during package upgrades (#11167)
Adds a new protocol version (7) and adds a check to make sure that a `key` ability cannot be added to a type during an upgrade. This also updates the ability checker so that we can pass it an `AbilitySet` of abilities that we don't want to allow to be added to pre-existing types.
1 parent 9ed6e46 commit 6f0699d

File tree

3 files changed

+45
-15
lines changed

3 files changed

+45
-15
lines changed

‎move-binary-format/src/compatibility.rs

Lines changed: 23 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,8 @@ pub struct Compatibility {
3131
pub check_friend_linking: bool,
3232
/// if false, treat `entry` as `private` when `check_struct_and_pub_function_linking`.
3333
pub check_private_entry_linking: bool,
34+
/// The set of abilities that cannot be added to an already exisiting type.
35+
pub disallowed_new_abilities: AbilitySet,
3436
}
3537

3638
impl Default for Compatibility {
@@ -40,6 +42,7 @@ impl Default for Compatibility {
4042
check_struct_layout: true,
4143
check_friend_linking: true,
4244
check_private_entry_linking: true,
45+
disallowed_new_abilities: AbilitySet::EMPTY,
4346
}
4447
}
4548
}
@@ -55,6 +58,7 @@ impl Compatibility {
5558
check_struct_layout: false,
5659
check_friend_linking: false,
5760
check_private_entry_linking: false,
61+
disallowed_new_abilities: AbilitySet::EMPTY,
5862
}
5963
}
6064

@@ -85,12 +89,14 @@ impl Compatibility {
8589
break;
8690
};
8791

88-
if !struct_abilities_compatibile(old_struct.abilities, new_struct.abilities)
89-
|| !struct_type_parameters_compatibile(
90-
&old_struct.type_parameters,
91-
&new_struct.type_parameters,
92-
)
93-
{
92+
if !struct_abilities_compatibile(
93+
self.disallowed_new_abilities,
94+
old_struct.abilities,
95+
new_struct.abilities,
96+
) || !struct_type_parameters_compatibile(
97+
&old_struct.type_parameters,
98+
&new_struct.type_parameters,
99+
) {
94100
struct_and_function_linking = false;
95101
}
96102
if new_struct.fields != old_struct.fields {
@@ -209,9 +215,18 @@ impl Compatibility {
209215
}
210216

211217
// When upgrading, the new abilities must be a superset of the old abilities.
212-
// Adding an ability is fine, but removing an ability could cause existing usages to fail.
213-
fn struct_abilities_compatibile(old_abilities: AbilitySet, new_abilities: AbilitySet) -> bool {
218+
// Adding an ability is fine as long as it's not in the disallowed_new_abilities,
219+
// but removing an ability could cause existing usages to fail.
220+
fn struct_abilities_compatibile(
221+
disallowed_new_abilities: AbilitySet,
222+
old_abilities: AbilitySet,
223+
new_abilities: AbilitySet,
224+
) -> bool {
214225
old_abilities.is_subset(new_abilities)
226+
&& disallowed_new_abilities.into_iter().all(|ability| {
227+
// If the new abilities have the ability the old ones must have it to
228+
!new_abilities.has_ability(ability) || old_abilities.has_ability(ability)
229+
})
215230
}
216231

217232
// When upgrading, the new type parameters must be the same length, and the new type parameter

‎move-binary-format/src/unit_tests/compatibility_tests.rs

Lines changed: 20 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -518,6 +518,7 @@ fn private_entry_signature_change_allowed() {
518518
check_struct_layout: true,
519519
check_friend_linking: true,
520520
check_private_entry_linking: false,
521+
disallowed_new_abilities: AbilitySet::EMPTY,
521522
}
522523
.check(&module, &updated_module)
523524
.is_ok());
@@ -528,6 +529,7 @@ fn private_entry_signature_change_allowed() {
528529
check_struct_layout: true,
529530
check_friend_linking: true,
530531
check_private_entry_linking: false,
532+
disallowed_new_abilities: AbilitySet::EMPTY,
531533
}
532534
.check(&updated_module, &module)
533535
.is_ok());
@@ -614,6 +616,7 @@ fn entry_fun_compat_tests() {
614616
check_struct_layout: true,
615617
check_friend_linking: true,
616618
check_private_entry_linking: false,
619+
disallowed_new_abilities: AbilitySet::EMPTY,
617620
}
618621
.check(prev, new)
619622
.is_ok());
@@ -631,6 +634,7 @@ fn entry_fun_compat_tests() {
631634
check_struct_layout: true,
632635
check_friend_linking: false,
633636
check_private_entry_linking: false,
637+
disallowed_new_abilities: AbilitySet::EMPTY,
634638
}
635639
.check(prev, new)
636640
.is_ok());
@@ -640,6 +644,7 @@ fn entry_fun_compat_tests() {
640644
check_struct_layout: true,
641645
check_friend_linking: true,
642646
check_private_entry_linking: false,
647+
disallowed_new_abilities: AbilitySet::EMPTY,
643648
}
644649
.check(prev, new)
645650
.is_err());
@@ -651,6 +656,7 @@ fn entry_fun_compat_tests() {
651656
check_struct_layout: true,
652657
check_friend_linking: true,
653658
check_private_entry_linking: false,
659+
disallowed_new_abilities: AbilitySet::EMPTY,
654660
}
655661
.check(prev, new)
656662
.is_err());
@@ -673,7 +679,8 @@ fn public_entry_signature_change_disallowed() {
673679
check_struct_and_pub_function_linking: true,
674680
check_struct_layout: true,
675681
check_friend_linking: true,
676-
check_private_entry_linking: false
682+
check_private_entry_linking: false,
683+
disallowed_new_abilities: AbilitySet::EMPTY,
677684
}
678685
.check(&module, &updated_module)
679686
.is_err());
@@ -682,7 +689,8 @@ fn public_entry_signature_change_disallowed() {
682689
check_struct_and_pub_function_linking: true,
683690
check_struct_layout: true,
684691
check_friend_linking: true,
685-
check_private_entry_linking: false
692+
check_private_entry_linking: false,
693+
disallowed_new_abilities: AbilitySet::EMPTY,
686694
}
687695
.check(&updated_module, &module)
688696
.is_err());
@@ -691,7 +699,8 @@ fn public_entry_signature_change_disallowed() {
691699
check_struct_and_pub_function_linking: true,
692700
check_struct_layout: true,
693701
check_friend_linking: true,
694-
check_private_entry_linking: true
702+
check_private_entry_linking: true,
703+
disallowed_new_abilities: AbilitySet::EMPTY,
695704
}
696705
.check(&module, &updated_module)
697706
.is_err());
@@ -712,7 +721,8 @@ fn friend_entry_signature_change_allowed() {
712721
check_struct_and_pub_function_linking: true,
713722
check_struct_layout: true,
714723
check_friend_linking: false,
715-
check_private_entry_linking: false
724+
check_private_entry_linking: false,
725+
disallowed_new_abilities: AbilitySet::EMPTY,
716726
}
717727
.check(&module, &updated_module)
718728
.is_ok());
@@ -721,7 +731,8 @@ fn friend_entry_signature_change_allowed() {
721731
check_struct_and_pub_function_linking: true,
722732
check_struct_layout: true,
723733
check_friend_linking: true,
724-
check_private_entry_linking: false
734+
check_private_entry_linking: false,
735+
disallowed_new_abilities: AbilitySet::EMPTY,
725736
}
726737
.check(&module, &updated_module)
727738
.is_err());
@@ -730,7 +741,8 @@ fn friend_entry_signature_change_allowed() {
730741
check_struct_and_pub_function_linking: true,
731742
check_struct_layout: true,
732743
check_friend_linking: false,
733-
check_private_entry_linking: true
744+
check_private_entry_linking: true,
745+
disallowed_new_abilities: AbilitySet::EMPTY,
734746
}
735747
.check(&module, &updated_module)
736748
.is_err());
@@ -739,7 +751,8 @@ fn friend_entry_signature_change_allowed() {
739751
check_struct_and_pub_function_linking: true,
740752
check_struct_layout: true,
741753
check_friend_linking: true,
742-
check_private_entry_linking: true
754+
check_private_entry_linking: true,
755+
disallowed_new_abilities: AbilitySet::EMPTY,
743756
}
744757
.check(&module, &updated_module)
745758
.is_err());

‎tools/move-cli/src/sandbox/utils/mod.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -350,6 +350,7 @@ pub(crate) fn explain_publish_error(
350350
check_struct_layout: true,
351351
check_friend_linking: false,
352352
check_private_entry_linking: true,
353+
disallowed_new_abilities: AbilitySet::EMPTY,
353354
})
354355
.check(&old_api, &new_api)
355356
.is_err()
@@ -362,6 +363,7 @@ pub(crate) fn explain_publish_error(
362363
check_struct_layout: false,
363364
check_friend_linking: false,
364365
check_private_entry_linking: true,
366+
disallowed_new_abilities: AbilitySet::EMPTY,
365367
})
366368
.check(&old_api, &new_api)
367369
.is_err()

0 commit comments

Comments
 (0)