Skip to content

Commit

Permalink
reorder check back & rename attribute
Browse files Browse the repository at this point in the history
  • Loading branch information
georgemitenkov committed Mar 7, 2024
1 parent f0f08d6 commit a37da25
Show file tree
Hide file tree
Showing 12 changed files with 29 additions and 30 deletions.
3 changes: 1 addition & 2 deletions aptos-move/aptos-vm/src/aptos_vm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -746,8 +746,7 @@ impl AptosVM {
entry_fn.ty_args(),
)?;

// TODO(George): change the order once performance is confirmed.
if is_entry_function_unbiasable(resolver, session, entry_fn)? && is_friend_or_private {
if is_friend_or_private && is_entry_function_unbiasable(resolver, session, entry_fn)? {
let txn_context = session
.get_native_extensions()
.get_mut::<RandomnessContext>();
Expand Down
4 changes: 2 additions & 2 deletions aptos-move/aptos-vm/src/verifier/randomness.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use crate::move_vm_ext::{AptosMoveResolver, SessionExt};
use aptos_types::transaction::EntryFunction;
use move_binary_format::errors::VMResult;

/// Returns true if function has an annotation that it is unbiasable.
/// Returns true if function has an annotation that it uses randomness.
pub(crate) fn is_entry_function_unbiasable(
resolver: &impl AptosMoveResolver,
session: &mut SessionExt,
Expand All @@ -19,7 +19,7 @@ pub(crate) fn is_entry_function_unbiasable(
Ok(metadata
.fun_attributes
.get(entry_fn.function().as_str())
.map(|attrs| attrs.iter().any(|attr| attr.is_unbiasable()))
.map(|attrs| attrs.iter().any(|attr| attr.is_randomness()))
.unwrap_or(false))
} else {
Ok(false)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
module 0x1::invalid {
#[unbiasable]
#[randomness]
public entry fun foo() {
// Do nothing.
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
module 0x1::invalid {
#[unbiasable]
#[randomness]
public fun foo() {
// Do nothing.
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,12 @@ module 0x1::test {
// Do nothing.
}

#[unbiasable]
#[randomness]
entry fun ok_if_annotated_and_not_using_randomness() {
// Do nothing.
}

#[unbiasable]
#[randomness]
entry fun ok_if_annotated_and_using_randomness() {
let _ = randomness::u64_integer();
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
module 0x1::some_randapp {
use aptos_framework::randomness;

#[unbiasable]
#[randomness]
entry fun safe_private_entry_call() {
let _ = randomness::u64_integer();
}
Expand All @@ -10,7 +10,7 @@ module 0x1::some_randapp {
let _ = randomness::u64_integer();
}

#[unbiasable]
#[randomness]
public(friend) entry fun safe_friend_entry_call() {
let _ = randomness::u64_integer();
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
module 0x1::some_randapp {
use aptos_framework::randomness;

#[unbiasable]
#[randomness]
entry fun safe_private_entry_call() {
let _ = randomness::u64_integer();
}
Expand All @@ -10,7 +10,7 @@ module 0x1::some_randapp {
let _ = randomness::u64_integer();
}

#[unbiasable]
#[randomness]
public(friend) entry fun safe_friend_entry_call() {
let _ = randomness::u64_integer();
}
Expand Down
4 changes: 2 additions & 2 deletions aptos-move/framework/aptos-framework/doc/randomness.md
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,7 @@ Event emitted every time a public randomness API in this module is called.
<a id="0x1_randomness_E_API_USE_SUSCEPTIBLE_TO_TEST_AND_ABORT"></a>

Randomness APIs calls must originate from a private entry function with
<code>#[unbiasable]</code> annotation. Otherwise, test-and-abort attacks are possible.
<code>#[<a href="randomness.md#0x1_randomness">randomness</a>]</code> annotation. Otherwise, test-and-abort attacks are possible.


<pre><code><b>const</b> <a href="randomness.md#0x1_randomness_E_API_USE_SUSCEPTIBLE_TO_TEST_AND_ABORT">E_API_USE_SUSCEPTIBLE_TO_TEST_AND_ABORT</a>: u64 = 1;
Expand Down Expand Up @@ -973,7 +973,7 @@ Aborts with <code><a href="randomness.md#0x1_randomness_E_API_USE_SUSCEPTIBLE_TO
Called in each randomness generation function to ensure certain safety invariants, namely:
1. The transaction that led to the call of this function had a private (or friend) entry
function as its payload.
2. The entry function had <code>#[unbiasable]</code> annotation.
2. The entry function had <code>#[<a href="randomness.md#0x1_randomness">randomness</a>]</code> annotation.


<pre><code><b>fun</b> <a href="randomness.md#0x1_randomness_is_unbiasable">is_unbiasable</a>(): bool
Expand Down
4 changes: 2 additions & 2 deletions aptos-move/framework/aptos-framework/sources/randomness.move
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ module aptos_framework::randomness {
const DST: vector<u8> = b"APTOS_RANDOMNESS";

/// Randomness APIs calls must originate from a private entry function with
/// `#[unbiasable]` annotation. Otherwise, test-and-abort attacks are possible.
/// `#[randomness]` annotation. Otherwise, test-and-abort attacks are possible.
const E_API_USE_SUSCEPTIBLE_TO_TEST_AND_ABORT: u64 = 1;

const MAX_U256: u256 = 115792089237316195423570985008687907853269984665640564039457584007913129639935;
Expand Down Expand Up @@ -384,7 +384,7 @@ module aptos_framework::randomness {
/// Called in each randomness generation function to ensure certain safety invariants, namely:
/// 1. The transaction that led to the call of this function had a private (or friend) entry
/// function as its payload.
/// 2. The entry function had `#[unbiasable]` annotation.
/// 2. The entry function had `#[randomness]` annotation.
native fun is_unbiasable(): bool;

#[test]
Expand Down
12 changes: 6 additions & 6 deletions aptos-move/framework/src/extended_checks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,11 +37,11 @@ const INIT_MODULE_FUN: &str = "init_module";
const LEGACY_ENTRY_FUN_ATTRIBUTE: &str = "legacy_entry_fun";
const ERROR_PREFIX: &str = "E";
const EVENT_STRUCT_ATTRIBUTE: &str = "event";
const RANDOMNESS_ATTRIBUTE: &str = "randomness";
const RESOURCE_GROUP: &str = "resource_group";
const RESOURCE_GROUP_MEMBER: &str = "resource_group_member";
const RESOURCE_GROUP_NAME: &str = "group";
const RESOURCE_GROUP_SCOPE: &str = "scope";
const UNBIASABLE_ATTRIBUTE: &str = "unbiasable";
const VIEW_FUN_ATTRIBUTE: &str = "view";

// top-level attribute names, only.
Expand All @@ -52,7 +52,7 @@ pub fn get_all_attribute_names() -> &'static BTreeSet<String> {
RESOURCE_GROUP_MEMBER,
VIEW_FUN_ATTRIBUTE,
EVENT_STRUCT_ATTRIBUTE,
UNBIASABLE_ATTRIBUTE,
RANDOMNESS_ATTRIBUTE,
];

fn extended_attribute_names() -> BTreeSet<String> {
Expand Down Expand Up @@ -460,26 +460,26 @@ impl<'a> ExtendedChecker<'a> {
impl<'a> ExtendedChecker<'a> {
fn check_and_record_unbiasabale_entry_functions(&mut self, module: &ModuleEnv) {
for ref fun in module.get_functions() {
if !self.has_attribute(fun, UNBIASABLE_ATTRIBUTE) {
if !self.has_attribute(fun, RANDOMNESS_ATTRIBUTE) {
continue;
}

if !fun.is_entry() || fun.visibility().is_public() {
self.env.error(
&fun.get_id_loc(),
"only private or public(friend) entry functions can have #[unbiasable] attribute",
"only private or public(friend) entry functions can have #[randomness] attribute",
)
}

// Record functions which are unbiasable.
// Record functions which use randomness.
let module_id = self.get_runtime_module_id(module);
self.output
.entry(module_id)
.or_default()
.fun_attributes
.entry(fun.get_simple_name_string().to_string())
.or_default()
.push(KnownAttribute::unbiasable());
.push(KnownAttribute::randomness());
}
}
}
Expand Down
14 changes: 7 additions & 7 deletions aptos-move/framework/src/module_metadata.rs
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ pub enum KnownAttributeKind {
ResourceGroup = 2,
ResourceGroupMember = 3,
Event = 4,
Unbiasable = 5,
Randomness = 5,
}

impl KnownAttribute {
Expand Down Expand Up @@ -149,15 +149,15 @@ impl KnownAttribute {
self.kind == KnownAttributeKind::Event as u8
}

pub fn unbiasable() -> Self {
pub fn randomness() -> Self {
Self {
kind: KnownAttributeKind::Unbiasable as u8,
kind: KnownAttributeKind::Randomness as u8,
args: vec![],
}
}

pub fn is_unbiasable(&self) -> bool {
self.kind == KnownAttributeKind::Unbiasable as u8
pub fn is_randomness(&self) -> bool {
self.kind == KnownAttributeKind::Randomness as u8
}
}

Expand Down Expand Up @@ -359,7 +359,7 @@ pub fn is_valid_unbiasable_function(

Err(AttributeValidationError {
key: fun.to_string(),
attribute: KnownAttributeKind::Unbiasable as u8,
attribute: KnownAttributeKind::Randomness as u8,
})
}

Expand Down Expand Up @@ -448,7 +448,7 @@ pub fn verify_module_metadata(
for attr in attrs {
if attr.is_view_function() {
is_valid_view_function(&functions, fun)?;
} else if attr.is_unbiasable() {
} else if attr.is_randomness() {
is_valid_unbiasable_function(&functions, fun)?;
} else {
return Err(AttributeValidationError {
Expand Down
2 changes: 1 addition & 1 deletion aptos-move/framework/src/natives/randomness.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ pub struct RandomnessContext {
// blob is requested.
txn_local_state: Vec<u8>,
// True if the current transaction's payload was a public(friend) or
// private entry function, which also had `#[unbiasable]` annotation.
// private entry function, which also has `#[randomness]` annotation.
unbiasable: bool,
}

Expand Down

0 comments on commit a37da25

Please sign in to comment.