Skip to content

Commit

Permalink
[Move] Implemented one-type witness checking (MystenLabs#3771)
Browse files Browse the repository at this point in the history
* [Move] Implemented one-type witness checking

* Added missing files

* Adjusted number of object created in init functions

* Renamed char(acteristic) type to one-time witness

* Changed location of one-time witness checking function

* All coins now use one-time witness

* Added a new line

* Updated test output

* Adjusted tests

* Updated snapshot test file

* Removed check to make the test more robust

* Updated snapshot test file

* one-timeness check moved to Coin (MystenLabs#3894)

* Removed a function that could compromise one-time witness safety and adjusted tests

Co-authored-by: Damir Shamanaev <[email protected]>
  • Loading branch information
awelc and damirka authored Aug 11, 2022
1 parent 5487eae commit a523c40
Show file tree
Hide file tree
Showing 39 changed files with 193 additions and 134 deletions.

Large diffs are not rendered by default.

10 changes: 3 additions & 7 deletions crates/sui-core/src/unit_tests/gas_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -406,7 +406,9 @@ async fn test_move_call_gas() -> SuiResult {
expected_gas_balance,
);

// Mimic the gas charge behavior and cross check the result with above.
// Mimic the gas charge behavior and cross check the result with above. Do not include
// computation cost calculation as it would require hard-coding a constant representing VM
// execution cost which is quite fragile.
let mut gas_status = SuiGasStatus::new_with_budget(GAS_VALUE_FOR_TESTING, 1, 1);
gas_status.charge_min_tx_gas()?;
let package_object = authority_state
Expand All @@ -417,11 +419,6 @@ async fn test_move_call_gas() -> SuiResult {
package_object.object_size_for_gas_metering() + gas_object.object_size_for_gas_metering(),
)?;
let gas_used_before_vm_exec = gas_status.summary(true).gas_used();
// The gas cost to execute the function in Move VM.
// Hard code it here since it's difficult to mock that in test.
// If a new native move module/function is modified, this value may need to be increased due to the increase of sui framework package
const MOVE_VM_EXEC_COST: u64 = 16006;
gas_status.charge_vm_exec_test_only(MOVE_VM_EXEC_COST)?;
let created_object = authority_state
.get_object(&effects.created[0].0 .0)
.await?
Expand All @@ -434,7 +431,6 @@ async fn test_move_call_gas() -> SuiResult {
)?;

let new_cost = gas_status.summary(true);
assert_eq!(gas_cost.computation_cost, new_cost.computation_cost,);
assert_eq!(gas_cost.storage_cost, new_cost.storage_cost);
// This is the total amount of storage cost paid. We will use this
// to check if we get back the same amount of rebate latter.
Expand Down
2 changes: 1 addition & 1 deletion crates/sui-framework/sources/balance.move
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ module sui::balance {
}

/// Create a new supply for type T.
public fun create_supply<T: drop>(_witness: T): Supply<T> {
public fun create_supply<T: drop>(_: T): Supply<T> {
Supply { value: 0 }
}

Expand Down
11 changes: 6 additions & 5 deletions crates/sui-framework/sources/coin.move
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@ module sui::coin {
use sui::tx_context::{Self, TxContext};
use std::vector;

/// For when a type passed to create_supply is not a one-time witness.
const EBadWitness: u64 = 0;

/// A coin of type `T` worth `value`. Transferable and storable
struct Coin<phantom T> has key, store {
id: UID,
Expand All @@ -28,11 +31,6 @@ module sui::coin {
balance::supply_value(&cap.total_supply)
}

/// Wrap a `Supply` into a transferable `TreasuryCap`.
public fun treasury_from_supply<T>(total_supply: Supply<T>, ctx: &mut TxContext): TreasuryCap<T> {
TreasuryCap { id: object::new(ctx), total_supply }
}

/// Unwrap `TreasuryCap` getting the `Supply`.
public fun treasury_into_supply<T>(treasury: TreasuryCap<T>): Supply<T> {
let TreasuryCap { id, total_supply } = treasury;
Expand Down Expand Up @@ -153,6 +151,9 @@ module sui::coin {
witness: T,
ctx: &mut TxContext
): TreasuryCap<T> {
// Make sure there's only one instance of the type T
assert!(sui::types::is_one_time_witness(&witness), EBadWitness);

TreasuryCap {
id: object::new(ctx),
total_supply: balance::create_supply(witness)
Expand Down
11 changes: 11 additions & 0 deletions crates/sui-framework/sources/types.move
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
// Copyright (c) 2022, Mysten Labs, Inc.
// SPDX-License-Identifier: Apache-2.0

/// Sui types helpers and utilities
module sui::types {
// === one-time witness ===

/// Tests if the argument type is a one-time witness, that is a type with only one instantiation
/// across the entire code base.
public native fun is_one_time_witness<T: drop>(_: &T): bool;
}
2 changes: 2 additions & 0 deletions crates/sui-framework/src/natives/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ mod object;
mod test_scenario;
mod transfer;
mod tx_context;
mod types;

use move_binary_format::errors::PartialVMError;
use move_core_types::{account_address::AccountAddress, identifier::Identifier};
Expand Down Expand Up @@ -64,6 +65,7 @@ pub fn all_natives(
"new_signer_from_address",
tx_context::new_signer_from_address,
),
("types", "is_one_time_witness", types::is_one_time_witness),
];
SUI_NATIVES
.iter()
Expand Down
40 changes: 40 additions & 0 deletions crates/sui-framework/src/natives/types.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
// Copyright (c) 2022, Mysten Labs, Inc.
// SPDX-License-Identifier: Apache-2.0

use move_binary_format::errors::PartialVMResult;
use move_core_types::language_storage::TypeTag;
use move_vm_runtime::native_functions::NativeContext;
use move_vm_types::{
gas_schedule::NativeCostIndex,
loaded_data::runtime_types::Type,
natives::function::{native_gas, NativeResult},
values::Value,
};
use smallvec::smallvec;
use std::collections::VecDeque;

pub fn is_one_time_witness(
context: &mut NativeContext,
mut ty_args: Vec<Type>,
args: VecDeque<Value>,
) -> PartialVMResult<NativeResult> {
debug_assert!(ty_args.len() == 1);
debug_assert!(args.len() == 1);

// unwrap safe because the interface of native function guarantees it.
let type_tag = context.type_to_type_tag(&ty_args.pop().unwrap())?;

// TODO: what should the cost of this be?
let cost = native_gas(context.cost_table(), NativeCostIndex::LENGTH, 1);

// If a struct type has the same name as the module that defines it but capitalized, it means
// that it's a characteristic type (which is one way of implementing a one-time witness
// type). This is checked in the char_type validator pass (a type with this type of name that
// does not have all properties required of a characteristic type will cause a validator error).
Ok(NativeResult::ok(
cost,
smallvec![Value::bool(
matches!(type_tag, TypeTag::Struct(struct_tag) if struct_tag.name.to_string() == struct_tag.module.to_string().to_ascii_uppercase())
)],
))
}

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,4 @@ processed 2 tasks

task 1 'publish'. lines 8-20:
Error: Transaction Effects Status: Sui Move Bytecode Verification Error. Please run the Sui Move Verifier for more information.
Execution Error: ExecutionError: ExecutionError { inner: ExecutionErrorInner { kind: SuiMoveVerificationError, source: Some("characteristic type _::m::M is instantiated in the _::m::pack function and must never be") } }
Execution Error: ExecutionError: ExecutionError { inner: ExecutionErrorInner { kind: SuiMoveVerificationError, source: Some("one-time witness type _::m::M is instantiated in the _::m::pack function and must never be") } }
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,4 @@ processed 2 tasks

task 1 'publish'. lines 8-20:
Error: Transaction Effects Status: Sui Move Bytecode Verification Error. Please run the Sui Move Verifier for more information.
Execution Error: ExecutionError: ExecutionError { inner: ExecutionErrorInner { kind: SuiMoveVerificationError, source: Some("characteristic type candidate _::m::M must have a single ability: drop") } }
Execution Error: ExecutionError: ExecutionError { inner: ExecutionErrorInner { kind: SuiMoveVerificationError, source: Some("one-time witness type candidate _::m::M must have a single ability: drop") } }
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,4 @@ processed 2 tasks

task 1 'publish'. lines 8-16:
Error: Transaction Effects Status: Sui Move Bytecode Verification Error. Please run the Sui Move Verifier for more information.
Execution Error: ExecutionError: ExecutionError { inner: ExecutionErrorInner { kind: SuiMoveVerificationError, source: Some("characteristic type candidate _::m::M must have a single ability: drop") } }
Execution Error: ExecutionError: ExecutionError { inner: ExecutionErrorInner { kind: SuiMoveVerificationError, source: Some("one-time witness type candidate _::m::M must have a single ability: drop") } }
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// Copyright (c) 2022, Mysten Labs, Inc.
// SPDX-License-Identifier: Apache-2.0

// invalid, char type has no drop ability
// invalid, one-time witness type has no drop ability

//# init --addresses test=0x0

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,4 @@ processed 2 tasks

task 1 'publish'. lines 8-15:
Error: Transaction Effects Status: Sui Move Bytecode Verification Error. Please run the Sui Move Verifier for more information.
Execution Error: ExecutionError: ExecutionError { inner: ExecutionErrorInner { kind: SuiMoveVerificationError, source: Some("init function of a module containing characteristic type candidate must have _::m::M as the first parameter") } }
Execution Error: ExecutionError: ExecutionError { inner: ExecutionErrorInner { kind: SuiMoveVerificationError, source: Some("init function of a module containing one-time witness type candidate must have _::m::M as the first parameter") } }
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// Copyright (c) 2022, Mysten Labs, Inc.
// SPDX-License-Identifier: Apache-2.0

// invalid, no char type parameter in init
// invalid, no one-time witness type parameter in init

//# init --addresses test=0x0

Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// Copyright (c) 2022, Mysten Labs, Inc.
// SPDX-License-Identifier: Apache-2.0

// invalid, characteristic type candidate used in a different module
// invalid, one-time witness type candidate used in a different module

//# init --addresses test1=0x0 test2=0x0

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,4 @@ processed 2 tasks

task 1 'publish'. lines 8-15:
Error: Transaction Effects Status: Sui Move Bytecode Verification Error. Please run the Sui Move Verifier for more information.
Execution Error: ExecutionError: ExecutionError { inner: ExecutionErrorInner { kind: SuiMoveVerificationError, source: Some("characteristic type candidate _::m::M must have a single bool field only (or no fields)") } }
Execution Error: ExecutionError: ExecutionError { inner: ExecutionErrorInner { kind: SuiMoveVerificationError, source: Some("one-time witness type candidate _::m::M must have a single bool field only (or no fields)") } }
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,4 @@ processed 2 tasks

task 1 'publish'. lines 8-17:
Error: Transaction Effects Status: Sui Move Bytecode Verification Error. Please run the Sui Move Verifier for more information.
Execution Error: ExecutionError: ExecutionError { inner: ExecutionErrorInner { kind: SuiMoveVerificationError, source: Some("init function of a module containing characteristic type candidate must have _::m::M as the first parameter") } }
Execution Error: ExecutionError: ExecutionError { inner: ExecutionErrorInner { kind: SuiMoveVerificationError, source: Some("init function of a module containing one-time witness type candidate must have _::m::M as the first parameter") } }
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
// Copyright (c) 2022, Mysten Labs, Inc.
// SPDX-License-Identifier: Apache-2.0

// invalid, wrong one-time witness type name

//# init --addresses test=0x0

//# publish
module test::m {

struct OneTimeWitness has drop { }

fun init(_: OneTimeWitness, _ctx: &mut sui::tx_context::TxContext) {
}
}
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// Copyright (c) 2022, Mysten Labs, Inc.
// SPDX-License-Identifier: Apache-2.0

// invalid, wrong characteristic type name format
// invalid, wrong one-time witness type name format

//# init --addresses test=0x0

Expand Down
8 changes: 4 additions & 4 deletions crates/sui-verifier/src/entry_points_verifier.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ use crate::{format_signature_token, resolve_struct, verification_failure, INIT_F
/// - The function must have `Visibility::Private`
/// - The function can have at most two parameters:
/// - mandatory &mut TxContext (see `is_tx_context`) in the last position
/// - optional characteristic type (see char_type verifier pass) passed by value in the first
/// - optional one-time witness type (see one_time_witness verifier pass) passed by value in the first
/// position
///
/// For transaction entry points
Expand Down Expand Up @@ -132,9 +132,9 @@ fn verify_init_function(module: &CompiledModule, fdef: &FunctionDefinition) -> R
}

// Checking only the last (and possibly the only) parameter here. If there are two parameters,
// then the first parameter must be of a characteristic type and must be passed by value. This
// is checked by the verifier pass handling characteristic types (char_type) - please see the
// description of this pass for additional details.
// then the first parameter must be of a one-time witness type and must be passed by value. This
// is checked by the verifier for pass one-time witness value (one_time_witness_verifier) -
// please see the description of this pass for additional details.
if is_tx_context(view, &parameters[parameters.len() - 1]) {
Ok(())
} else {
Expand Down
2 changes: 1 addition & 1 deletion crates/sui-verifier/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,10 @@

pub mod verifier;

pub mod char_type_verifier;
pub mod entry_points_verifier;
pub mod global_storage_access_verifier;
pub mod id_leak_verifier;
pub mod one_time_witness_verifier;
pub mod private_generics;
pub mod struct_with_key_verifier;

Expand Down
Loading

0 comments on commit a523c40

Please sign in to comment.