Skip to content

Commit

Permalink
[move-compiler] Improve interactions with Sui mode and interface files (
Browse files Browse the repository at this point in the history
#20196)

## Description 

- Sui mode used only named addresses for type comparison. This is not
sufficient for interface files that might be using only the numerical
address.
- Switched `is` to use only the numerical address. We might want to
evaluate differently if we start using it more outside of Sui mode

## Test plan 

- Ran tests 

---

## Release notes

Check each box that your changes affect. If none of the boxes relate to
your changes, release notes aren't required.

For each box you select, include information after the relevant heading
that describes the impact of your changes that a user might notice and
any actions they must take to implement updates.

- [ ] Protocol: 
- [ ] Nodes (Validators and Full nodes): 
- [ ] Indexer: 
- [ ] JSON-RPC: 
- [ ] GraphQL: 
- [ ] CLI: 
- [ ] Rust SDK:
- [ ] REST API:
  • Loading branch information
tnowacki authored Nov 12, 2024
1 parent 7b61e1f commit ec6d016
Show file tree
Hide file tree
Showing 25 changed files with 296 additions and 218 deletions.
10 changes: 1 addition & 9 deletions crates/sui-types/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ pub mod utils;
macro_rules! built_in_ids {
($($addr:ident / $id:ident = $init:expr);* $(;)?) => {
$(
pub const $addr: AccountAddress = builtin_address($init);
pub const $addr: AccountAddress = AccountAddress::from_suffix($init);
pub const $id: ObjectID = ObjectID::from_address($addr);
)*
}
Expand Down Expand Up @@ -133,14 +133,6 @@ pub const SUI_SYSTEM_STATE_OBJECT_SHARED_VERSION: SequenceNumber = OBJECT_START_
pub const SUI_CLOCK_OBJECT_SHARED_VERSION: SequenceNumber = OBJECT_START_VERSION;
pub const SUI_AUTHENTICATOR_STATE_OBJECT_SHARED_VERSION: SequenceNumber = OBJECT_START_VERSION;

const fn builtin_address(suffix: u16) -> AccountAddress {
let mut addr = [0u8; AccountAddress::LENGTH];
let [hi, lo] = suffix.to_be_bytes();
addr[AccountAddress::LENGTH - 2] = hi;
addr[AccountAddress::LENGTH - 1] = lo;
AccountAddress::new(addr)
}

pub fn sui_framework_address_concat_string(suffix: &str) -> String {
format!("{}{suffix}", SUI_FRAMEWORK_ADDRESS.to_hex_literal())
}
Expand Down
15 changes: 11 additions & 4 deletions external-crates/move/crates/move-compiler/src/cfgir/visitor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ use crate::{
parser::ast::{ConstantName, DatatypeName, FunctionName},
shared::CompilationEnv,
};
use move_core_types::account_address::AccountAddress;
use move_ir_types::location::*;
use move_proc_macros::growing_stack;

Expand Down Expand Up @@ -846,19 +847,25 @@ where
exp_satisfies_(e, &mut p)
}

pub fn calls_special_function(special: &[(&str, &str, &str)], cfg: &ImmForwardCFG) -> bool {
pub fn calls_special_function(
special: &[(AccountAddress, &str, &str)],
cfg: &ImmForwardCFG,
) -> bool {
cfg_satisfies(cfg, |_| true, |e| is_special_function(special, e))
}

pub fn calls_special_function_command(special: &[(&str, &str, &str)], cmd: &Command) -> bool {
pub fn calls_special_function_command(
special: &[(AccountAddress, &str, &str)],
cmd: &Command,
) -> bool {
command_satisfies(cmd, |_| true, |e| is_special_function(special, e))
}

pub fn calls_special_function_exp(special: &[(&str, &str, &str)], e: &Exp) -> bool {
pub fn calls_special_function_exp(special: &[(AccountAddress, &str, &str)], e: &Exp) -> bool {
exp_satisfies(e, |e| is_special_function(special, e))
}

fn is_special_function(special: &[(&str, &str, &str)], e: &Exp) -> bool {
fn is_special_function(special: &[(AccountAddress, &str, &str)], e: &Exp) -> bool {
use H::UnannotatedExp_ as E;
matches!(
&e.exp.value,
Expand Down
20 changes: 14 additions & 6 deletions external-crates/move/crates/move-compiler/src/expansion/ast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -657,12 +657,17 @@ impl Address {
}
}

pub fn is(&self, address: impl AsRef<str>) -> bool {
pub fn is<Addr>(&self, address: &Addr) -> bool
where
NumericalAddress: PartialEq<Addr>,
{
self.numerical_value().is_some_and(|sp!(_, v)| v == address)
}

pub fn numerical_value(&self) -> Option<&Spanned<NumericalAddress>> {
match self {
Self::Numerical { name: Some(n), .. } | Self::NamedUnassigned(n) => {
n.value.as_str() == address.as_ref()
}
Self::Numerical { name: None, .. } => false,
Self::Numerical { value, .. } => Some(value),
Self::NamedUnassigned(_) => None,
}
}
}
Expand All @@ -672,7 +677,10 @@ impl ModuleIdent_ {
Self { address, module }
}

pub fn is(&self, address: impl AsRef<str>, module: impl AsRef<str>) -> bool {
pub fn is<Addr>(&self, address: &Addr, module: impl AsRef<str>) -> bool
where
NumericalAddress: PartialEq<Addr>,
{
let Self {
address: a,
module: m,
Expand Down
46 changes: 28 additions & 18 deletions external-crates/move/crates/move-compiler/src/hlir/ast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -582,12 +582,10 @@ impl UnannotatedExp_ {
}

impl TypeName_ {
pub fn is(
&self,
address: impl AsRef<str>,
module: impl AsRef<str>,
name: impl AsRef<str>,
) -> bool {
pub fn is<Addr>(&self, address: &Addr, module: impl AsRef<str>, name: impl AsRef<str>) -> bool
where
NumericalAddress: PartialEq<Addr>,
{
match self {
TypeName_::Builtin(_) => false,
TypeName_::ModuleType(mident, n) => {
Expand Down Expand Up @@ -677,12 +675,15 @@ impl BaseType_ {
}
}

pub fn is_apply(
pub fn is_apply<Addr>(
&self,
address: impl AsRef<str>,
address: &Addr,
module: impl AsRef<str>,
name: impl AsRef<str>,
) -> Option<(&AbilitySet, &TypeName, &[BaseType])> {
) -> Option<(&AbilitySet, &TypeName, &[BaseType])>
where
NumericalAddress: PartialEq<Addr>,
{
match self {
Self::Apply(abs, n, tys) if n.value.is(address, module, name) => Some((abs, n, tys)),
_ => None,
Expand Down Expand Up @@ -734,12 +735,15 @@ impl SingleType_ {
}
}

pub fn is_apply(
pub fn is_apply<Addr>(
&self,
address: impl AsRef<str>,
address: &Addr,
module: impl AsRef<str>,
name: impl AsRef<str>,
) -> Option<(&AbilitySet, &TypeName, &[BaseType])> {
) -> Option<(&AbilitySet, &TypeName, &[BaseType])>
where
NumericalAddress: PartialEq<Addr>,
{
match self {
Self::Ref(_, b) | Self::Base(b) => b.value.is_apply(address, module, name),
}
Expand Down Expand Up @@ -810,12 +814,15 @@ impl Type_ {
sp(loc, t_)
}

pub fn is_apply(
pub fn is_apply<Addr>(
&self,
address: impl AsRef<str>,
address: &Addr,
module: impl AsRef<str>,
name: impl AsRef<str>,
) -> Option<(&AbilitySet, &TypeName, &[BaseType])> {
) -> Option<(&AbilitySet, &TypeName, &[BaseType])>
where
NumericalAddress: PartialEq<Addr>,
{
match self {
Type_::Unit => None,
Type_::Single(t) => t.value.is_apply(address, module, name),
Expand Down Expand Up @@ -859,12 +866,15 @@ impl TName for BlockLabel {
}

impl ModuleCall {
pub fn is(
pub fn is<Addr>(
&self,
address: impl AsRef<str>,
address: &Addr,
module: impl AsRef<str>,
function: impl AsRef<str>,
) -> bool {
) -> bool
where
NumericalAddress: PartialEq<Addr>,
{
let Self {
module: sp!(_, mident),
name: f,
Expand Down
20 changes: 8 additions & 12 deletions external-crates/move/crates/move-compiler/src/naming/ast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -758,12 +758,10 @@ impl BuiltinFunction_ {
}

impl TypeName_ {
pub fn is(
&self,
address: impl AsRef<str>,
module: impl AsRef<str>,
name: impl AsRef<str>,
) -> bool {
pub fn is<Addr>(&self, address: &Addr, module: impl AsRef<str>, name: impl AsRef<str>) -> bool
where
NumericalAddress: PartialEq<Addr>,
{
match self {
TypeName_::Builtin(_) | TypeName_::Multiple(_) => false,
TypeName_::ModuleType(mident, n) => {
Expand Down Expand Up @@ -895,12 +893,10 @@ impl Type_ {
}
}

pub fn is(
&self,
address: impl AsRef<str>,
module: impl AsRef<str>,
name: impl AsRef<str>,
) -> bool {
pub fn is<Addr>(&self, address: &Addr, module: impl AsRef<str>, name: impl AsRef<str>) -> bool
where
NumericalAddress: PartialEq<Addr>,
{
self.type_name()
.is_some_and(|tn| tn.value.is(address, module, name))
}
Expand Down
51 changes: 25 additions & 26 deletions external-crates/move/crates/move-compiler/src/sui_mode/id_leak.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,6 @@
// Copyright (c) The Move Contributors
// SPDX-License-Identifier: Apache-2.0

use move_ir_types::location::*;
use move_symbol_pool::Symbol;

use crate::{
cfgir::{
absint::JoinResult,
Expand All @@ -21,42 +18,44 @@ use crate::{
hlir::ast::{self as H, Exp, Label, ModuleCall, SingleType, Type, Type_, Var},
parser::ast::Ability_,
shared::{program_info::TypingProgramInfo, Identifier},
sui_mode::{OBJECT_NEW, TEST_SCENARIO_MODULE_NAME, TS_NEW_OBJECT},
sui_mode::{
AUTHENTICATOR_STATE_CREATE, AUTHENTICATOR_STATE_MODULE_NAME, BRIDGE_ADDR_VALUE,
BRIDGE_CREATE, BRIDGE_MODULE_NAME, CLOCK_MODULE_NAME, DENY_LIST_CREATE,
DENY_LIST_MODULE_NAME, ID_LEAK_DIAG, OBJECT_MODULE_NAME, OBJECT_NEW,
OBJECT_NEW_UID_FROM_HASH, RANDOMNESS_MODULE_NAME, RANDOMNESS_STATE_CREATE, SUI_ADDR_NAME,
SUI_ADDR_VALUE, SUI_CLOCK_CREATE, SUI_SYSTEM_ADDR_VALUE, SUI_SYSTEM_CREATE,
SUI_SYSTEM_MODULE_NAME, TEST_SCENARIO_MODULE_NAME, TS_NEW_OBJECT, UID_TYPE_NAME,
},
};
use move_core_types::account_address::AccountAddress;
use move_ir_types::location::*;
use move_symbol_pool::Symbol;
use std::collections::BTreeMap;

use super::{
AUTHENTICATOR_STATE_CREATE, AUTHENTICATOR_STATE_MODULE_NAME, BRIDGE_ADDR_NAME, BRIDGE_CREATE,
BRIDGE_MODULE_NAME, CLOCK_MODULE_NAME, DENY_LIST_CREATE, DENY_LIST_MODULE_NAME, ID_LEAK_DIAG,
OBJECT_MODULE_NAME, OBJECT_NEW_UID_FROM_HASH, RANDOMNESS_MODULE_NAME, RANDOMNESS_STATE_CREATE,
SUI_ADDR_NAME, SUI_CLOCK_CREATE, SUI_SYSTEM_ADDR_NAME, SUI_SYSTEM_CREATE,
SUI_SYSTEM_MODULE_NAME, UID_TYPE_NAME,
};

pub const FRESH_ID_FUNCTIONS: &[(Symbol, Symbol, Symbol)] = &[
(SUI_ADDR_NAME, OBJECT_MODULE_NAME, OBJECT_NEW),
(SUI_ADDR_NAME, OBJECT_MODULE_NAME, OBJECT_NEW_UID_FROM_HASH),
(SUI_ADDR_NAME, TEST_SCENARIO_MODULE_NAME, TS_NEW_OBJECT),
pub const FRESH_ID_FUNCTIONS: &[(AccountAddress, Symbol, Symbol)] = &[
(SUI_ADDR_VALUE, OBJECT_MODULE_NAME, OBJECT_NEW),
(SUI_ADDR_VALUE, OBJECT_MODULE_NAME, OBJECT_NEW_UID_FROM_HASH),
(SUI_ADDR_VALUE, TEST_SCENARIO_MODULE_NAME, TS_NEW_OBJECT),
];
pub const FUNCTIONS_TO_SKIP: &[(Symbol, Symbol, Symbol)] = &[
pub const FUNCTIONS_TO_SKIP: &[(AccountAddress, Symbol, Symbol)] = &[
(
SUI_SYSTEM_ADDR_NAME,
SUI_SYSTEM_ADDR_VALUE,
SUI_SYSTEM_MODULE_NAME,
SUI_SYSTEM_CREATE,
),
(SUI_ADDR_NAME, CLOCK_MODULE_NAME, SUI_CLOCK_CREATE),
(SUI_ADDR_VALUE, CLOCK_MODULE_NAME, SUI_CLOCK_CREATE),
(
SUI_ADDR_NAME,
SUI_ADDR_VALUE,
AUTHENTICATOR_STATE_MODULE_NAME,
AUTHENTICATOR_STATE_CREATE,
),
(
SUI_ADDR_NAME,
SUI_ADDR_VALUE,
RANDOMNESS_MODULE_NAME,
RANDOMNESS_STATE_CREATE,
),
(SUI_ADDR_NAME, DENY_LIST_MODULE_NAME, DENY_LIST_CREATE),
(BRIDGE_ADDR_NAME, BRIDGE_MODULE_NAME, BRIDGE_CREATE),
(SUI_ADDR_VALUE, DENY_LIST_MODULE_NAME, DENY_LIST_CREATE),
(BRIDGE_ADDR_VALUE, BRIDGE_MODULE_NAME, BRIDGE_CREATE),
];

//**************************************************************************************************
Expand Down Expand Up @@ -119,7 +118,7 @@ impl SimpleAbsIntConstructor for IDLeakVerifier {
if let MemberName::Function(n) = &context.member {
let should_skip = FUNCTIONS_TO_SKIP
.iter()
.any(|to_skip| module.value.is(to_skip.0, to_skip.1) && n.value == to_skip.2);
.any(|to_skip| module.value.is(&to_skip.0, to_skip.1) && n.value == to_skip.2);
if should_skip {
return None;
}
Expand Down Expand Up @@ -223,7 +222,7 @@ impl<'a> SimpleAbsInt for IDLeakVerifierAI<'a> {
) -> Option<Vec<Value>> {
if FRESH_ID_FUNCTIONS
.iter()
.any(|makes_fresh| f.is(makes_fresh.0, makes_fresh.1, makes_fresh.2))
.any(|makes_fresh| f.is(&makes_fresh.0, makes_fresh.1, makes_fresh.2))
{
return Some(vec![Value::FreshID(*loc)]);
}
Expand All @@ -236,7 +235,7 @@ impl<'a> SimpleAbsInt for IDLeakVerifierAI<'a> {
}

fn value_for_ty(loc: &Loc, sp!(_, t): &SingleType) -> Value {
if t.is_apply(SUI_ADDR_NAME, OBJECT_MODULE_NAME, UID_TYPE_NAME)
if t.is_apply(&SUI_ADDR_VALUE, OBJECT_MODULE_NAME, UID_TYPE_NAME)
.is_some()
{
Value::NotFresh(*loc)
Expand Down
10 changes: 7 additions & 3 deletions external-crates/move/crates/move-compiler/src/sui_mode/info.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ use crate::{
unique_map::UniqueMap,
},
sui_mode::{
OBJECT_MODULE_NAME, SUI_ADDR_NAME, TRANSFER_FUNCTION_NAME, TRANSFER_MODULE_NAME,
OBJECT_MODULE_NAME, SUI_ADDR_VALUE, TRANSFER_FUNCTION_NAME, TRANSFER_MODULE_NAME,
UID_TYPE_NAME,
},
typing::{ast as T, visitor::TypingVisitorContext},
Expand Down Expand Up @@ -131,7 +131,7 @@ fn all_uid_holders(info: &TypingProgramInfo) -> BTreeMap<(ModuleIdent, DatatypeN
N::Type_::Ref(_, inner) => visit_ty(info, visited, uid_holders, inner),

N::Type_::Apply(_, sp!(_, tn_), _)
if tn_.is(SUI_ADDR_NAME, OBJECT_MODULE_NAME, UID_TYPE_NAME) =>
if tn_.is(&SUI_ADDR_VALUE, OBJECT_MODULE_NAME, UID_TYPE_NAME) =>
{
Some(UIDHolder::IsUID)
}
Expand Down Expand Up @@ -285,7 +285,11 @@ fn add_private_transfers(
let E::ModuleCall(call) = &e.exp.value else {
return false;
};
if !call.is(SUI_ADDR_NAME, TRANSFER_MODULE_NAME, TRANSFER_FUNCTION_NAME) {
if !call.is(
&SUI_ADDR_VALUE,
TRANSFER_MODULE_NAME,
TRANSFER_FUNCTION_NAME,
) {
return false;
}
let [sp!(_, ty)] = call.type_arguments.as_slice() else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,13 @@ use crate::{
expansion::ast::ModuleIdent,
naming::ast as N,
parser::ast::DatatypeName,
sui_mode::SUI_ADDR_VALUE,
typing::{ast as T, visitor::simple_visitor},
};

use super::{
LinterDiagnosticCategory, LinterDiagnosticCode, COIN_MOD_NAME, COIN_STRUCT_NAME,
LINT_WARNING_PREFIX, SUI_PKG_NAME,
LINT_WARNING_PREFIX,
};

const COIN_FIELD_DIAG: DiagnosticInfo = custom(
Expand Down Expand Up @@ -62,7 +63,7 @@ fn is_field_coin_type(sp!(_, t): &N::Type) -> bool {
T::Ref(_, inner_t) => is_field_coin_type(inner_t),
T::Apply(_, tname, _) => {
let sp!(_, tname) = tname;
tname.is(SUI_PKG_NAME, COIN_MOD_NAME, COIN_STRUCT_NAME)
tname.is(&SUI_ADDR_VALUE, COIN_MOD_NAME, COIN_STRUCT_NAME)
}
T::Unit | T::Param(_) | T::Var(_) | T::Anything | T::UnresolvedError | T::Fun(_, _) => {
false
Expand Down
Loading

0 comments on commit ec6d016

Please sign in to comment.