Skip to content

Commit

Permalink
[move][test_scenario] Fix bug in test_scenario where we weren't ret…
Browse files Browse the repository at this point in the history
…urning allocated tickets (#17991)

## Description 

Fixes a bug where we weren't returning allocated receiving tickets in a
transaction at the end of the transaction. This fixes the bug, and adds
tests to make sure allocated tickets are properly handled at the end of
a test scenario transaction.

## Test plan 

Added new tests to test the failing behavior and made sure they pass.
  • Loading branch information
tzakian authored and suiwombat committed Sep 16, 2024
1 parent 963c2eb commit ce985fc
Show file tree
Hide file tree
Showing 4 changed files with 178 additions and 59 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,6 @@
module sui::test_scenario_tests {
use sui::test_scenario;

const EIdBytesMismatch: u64 = 0;
const EValueMismatch: u64 = 1;

public struct Object has key, store {
id: UID,
value: u64,
Expand Down Expand Up @@ -196,8 +193,8 @@ module sui::test_scenario_tests {
assert!(scenario.has_most_recent_for_sender<Object>());
let received_obj = scenario.take_from_sender<Object>();
let Object { id: received_id, value } = received_obj;
assert!(received_id.to_inner() == id_bytes, EIdBytesMismatch);
assert!(value == 100, EValueMismatch);
assert!(received_id.to_inner() == id_bytes);
assert!(value == 100);
received_id.delete();
};
// check that the object is no longer accessible after deletion
Expand Down Expand Up @@ -228,7 +225,7 @@ module sui::test_scenario_tests {
};
scenario.next_tx(sender);
let ids = scenario.ids_for_sender<Object>();
assert!(ids == vector[id1, id2, id3], EValueMismatch);
assert!(ids == vector[id1, id2, id3]);
scenario.end();
}

Expand All @@ -255,9 +252,9 @@ module sui::test_scenario_tests {
let obj1 = scenario.take_from_sender_by_id<Object>(id1);
let obj3 = scenario.take_from_sender_by_id<Object>(id3);
let obj2 = scenario.take_from_sender_by_id<Object>(id2);
assert!(obj1.value == 10, EValueMismatch);
assert!(obj2.value == 20, EValueMismatch);
assert!(obj3.value == 30, EValueMismatch);
assert!(obj1.value == 10);
assert!(obj2.value == 20);
assert!(obj3.value == 30);
scenario.return_to_sender(obj1);
scenario.return_to_sender(obj2);
scenario.return_to_sender(obj3);
Expand Down Expand Up @@ -303,9 +300,9 @@ module sui::test_scenario_tests {
let obj1 = scenario.take_shared_by_id<Object>(id1);
let obj3 = scenario.take_shared_by_id<Object>(id3);
let obj2 = scenario.take_shared_by_id<Object>(id2);
assert!(obj1.value == 10, EValueMismatch);
assert!(obj2.value == 20, EValueMismatch);
assert!(obj3.value == 30, EValueMismatch);
assert!(obj1.value == 10);
assert!(obj2.value == 20);
assert!(obj3.value == 30);
test_scenario::return_shared(obj1);
test_scenario::return_shared(obj2);
test_scenario::return_shared(obj3);
Expand All @@ -326,7 +323,7 @@ module sui::test_scenario_tests {
{
assert!(test_scenario::has_most_recent_shared<Object>());
let obj1 = scenario.take_shared<Object>();
assert!(obj1.value == 10, EValueMismatch);
assert!(obj1.value == 10);
test_scenario::return_shared(obj1);
};
scenario.end();
Expand All @@ -345,7 +342,7 @@ module sui::test_scenario_tests {
{
assert!(test_scenario::has_most_recent_shared<Object>());
let obj1 = scenario.take_shared<Object>();
assert!(obj1.value == 10, EValueMismatch);
assert!(obj1.value == 10);
let Object { id, value: _ } = obj1;
id.delete();
};
Expand Down Expand Up @@ -375,9 +372,9 @@ module sui::test_scenario_tests {
let obj1 = scenario.take_immutable_by_id<Object>(id1);
let obj3 = scenario.take_immutable_by_id<Object>(id3);
let obj2 = scenario.take_immutable_by_id<Object>(id2);
assert!(obj1.value == 10, EValueMismatch);
assert!(obj2.value == 20, EValueMismatch);
assert!(obj3.value == 30, EValueMismatch);
assert!(obj1.value == 10);
assert!(obj2.value == 20);
assert!(obj3.value == 30);
test_scenario::return_immutable(obj1);
test_scenario::return_immutable(obj2);
test_scenario::return_immutable(obj3);
Expand All @@ -398,7 +395,7 @@ module sui::test_scenario_tests {
{
assert!(test_scenario::has_most_recent_immutable<Object>());
let obj1 = scenario.take_immutable<Object>();
assert!(obj1.value == 10, EValueMismatch);
assert!(obj1.value == 10);
test_scenario::return_immutable(obj1);
};
scenario.end();
Expand Down Expand Up @@ -433,9 +430,9 @@ module sui::test_scenario_tests {
let t3 = test_scenario::receiving_ticket_by_id<Object>(id3);
let obj2 = transfer::receive(&mut parent.id, t2);
let obj3 = transfer::receive(&mut parent.id, t3);
assert!(parent.value == 10, EValueMismatch);
assert!(obj2.value == 20, EValueMismatch);
assert!(obj3.value == 30, EValueMismatch);
assert!(parent.value == 10);
assert!(obj2.value == 20);
assert!(obj3.value == 30);
scenario.return_to_sender(parent);
transfer::public_transfer(obj2, id1_addr);
transfer::public_transfer(obj3, id1_addr)
Expand Down Expand Up @@ -464,8 +461,86 @@ module sui::test_scenario_tests {
let mut parent = scenario.take_from_sender_by_id<Object>(id1);
let t2 = test_scenario::most_recent_receiving_ticket<Object>(&id1);
let obj2 = transfer::receive(&mut parent.id, t2);
assert!(parent.value == 10, EValueMismatch);
assert!(obj2.value == 20, EValueMismatch);
assert!(parent.value == 10);
assert!(obj2.value == 20);
scenario.return_to_sender(parent);
transfer::public_transfer(obj2, id1_addr);
};
scenario.end();
}

// Make sure that we properly handle the case where we receive an object
// and don't need to deallocate the receiving ticket and underlying object
// at the end of the transaction.
#[test]
fun test_receive_object_multiple_in_row() {
let sender = @0x0;
let mut scenario = test_scenario::begin(sender);
let uid1 = scenario.new_object();
let uid2 = scenario.new_object();
let id1 = uid1.uid_to_inner();
let id1_addr = uid1.to_address();
{
let obj1 = Object { id: uid1, value: 10 };
let obj2 = Object { id: uid2, value: 20 };
transfer::public_transfer(obj1, sender);
transfer::public_transfer(obj2, id1_addr);
};
scenario.next_tx(sender);
{
let mut parent: Object = scenario.take_from_sender_by_id(id1);
let t2: transfer::Receiving<Object> = test_scenario::most_recent_receiving_ticket(&id1);
let obj2 = transfer::receive(&mut parent.id, t2);
assert!(parent.value == 10);
assert!(obj2.value == 20);
scenario.return_to_sender(parent);
transfer::public_transfer(obj2, id1_addr);
};
scenario.next_tx(sender);
{
let mut parent: Object = scenario.take_from_sender_by_id(id1);
let t2: transfer::Receiving<Object> = test_scenario::most_recent_receiving_ticket(&id1);
let obj2 = transfer::receive(&mut parent.id, t2);
assert!(parent.value == 10);
assert!(obj2.value == 20);
scenario.return_to_sender(parent);
transfer::public_transfer(obj2, id1_addr);
};
scenario.end();
}

// Make sure that we properly handle the case where we don't receive an
// object after allocating a ticket, and then receiving it in the next
// transaction.
#[test]
fun test_no_receive_object_then_use_next_tx() {
let sender = @0x0;
let mut scenario = test_scenario::begin(sender);
let uid1 = scenario.new_object();
let uid2 = scenario.new_object();
let id1 = uid1.uid_to_inner();
let id1_addr = uid1.to_address();
{
let obj1 = Object { id: uid1, value: 10 };
let obj2 = Object { id: uid2, value: 20 };
transfer::public_transfer(obj1, sender);
transfer::public_transfer(obj2, id1_addr);
};
scenario.next_tx(sender);
{
// allocate a receiving ticket in this transaction, but don't use it or return it.
test_scenario::most_recent_receiving_ticket<Object>(&id1);
};
scenario.next_tx(sender);
{
let mut parent: Object = scenario.take_from_sender_by_id(id1);
// Get the receiving ticket that was allocated in the previous
// transaction, again. If we failed to return unused receiving
// tickets at the end of the transaction above this will fail.
let t2: transfer::Receiving<Object> = test_scenario::most_recent_receiving_ticket(&id1);
let obj2 = transfer::receive(&mut parent.id, t2);
assert!(parent.value == 10);
assert!(obj2.value == 20);
scenario.return_to_sender(parent);
transfer::public_transfer(obj2, id1_addr);
};
Expand Down Expand Up @@ -566,9 +641,9 @@ module sui::test_scenario_tests {
let t3 = test_scenario::receiving_ticket_by_id<Object>(id3);
let obj2 = transfer::receive(&mut parent.id, t2);
let obj3 = transfer::receive(&mut parent.id, t3);
assert!(parent.value == 10, EValueMismatch);
assert!(obj2.value == 20, EValueMismatch);
assert!(obj3.value == 30, EValueMismatch);
assert!(parent.value == 10);
assert!(obj2.value == 20);
assert!(obj3.value == 30);
scenario.return_to_sender(parent);
transfer::public_transfer(obj2, id1_addr);
transfer::public_transfer(obj3, id1_addr)
Expand Down Expand Up @@ -609,9 +684,9 @@ module sui::test_scenario_tests {
let t3 = test_scenario::receiving_ticket_by_id<Object>(id3);
let mut obj2 = transfer::receive(&mut parent.id, t2);
let obj3 = transfer::receive(&mut parent.id, t3);
assert!(parent.value == 10, EValueMismatch);
assert!(obj2.value == 20, EValueMismatch);
assert!(obj3.value == 30, EValueMismatch);
assert!(parent.value == 10);
assert!(obj2.value == 20);
assert!(obj3.value == 30);
obj2.value = 42;
scenario.return_to_sender(parent);
transfer::public_transfer(obj2, sender);
Expand All @@ -620,7 +695,7 @@ module sui::test_scenario_tests {
scenario.next_tx(sender);
{
let obj = scenario.take_from_sender_by_id<Object>(id2);
assert!(obj.value == 42, EValueMismatch);
assert!(obj.value == 42);
scenario.return_to_sender(obj);
};
scenario.end();
Expand Down
12 changes: 5 additions & 7 deletions crates/sui-move/src/unit_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,7 @@ use move_package::BuildConfig;
use move_unit_test::{extensions::set_extension_hook, UnitTestingConfig};
use move_vm_runtime::native_extensions::NativeContextExtensions;
use once_cell::sync::Lazy;
use std::{
collections::BTreeMap,
path::Path,
sync::{Arc, RwLock},
};
use std::{cell::RefCell, collections::BTreeMap, path::Path, sync::Arc};
use sui_move_build::decorate_warnings;
use sui_move_natives::test_scenario::InMemoryTestStore;
use sui_move_natives::{object_runtime::ObjectRuntime, NativesCostTable};
Expand Down Expand Up @@ -58,8 +54,10 @@ impl Test {
}
}

static TEST_STORE_INNER: Lazy<RwLock<InMemoryStorage>> =
Lazy::new(|| RwLock::new(InMemoryStorage::default()));
// Create a separate test store per-thread.
thread_local! {
static TEST_STORE_INNER: RefCell<InMemoryStorage> = RefCell::new(InMemoryStorage::default());
}

static TEST_STORE: Lazy<InMemoryTestStore> = Lazy::new(|| InMemoryTestStore(&TEST_STORE_INNER));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,10 @@
use crate::{
interpreter::Interpreter, loader::Resolver, native_extensions::NativeContextExtensions,
};
use move_binary_format::errors::{ExecutionState, PartialVMError, PartialVMResult};
use move_binary_format::{
errors::{ExecutionState, PartialVMError, PartialVMResult},
file_format::AbilitySet,
};
use move_core_types::{
account_address::AccountAddress,
annotated_value as A,
Expand Down Expand Up @@ -155,6 +158,10 @@ impl<'a, 'b> NativeContext<'a, 'b> {
}
}

pub fn type_to_abilities(&self, ty: &Type) -> PartialVMResult<AbilitySet> {
self.resolver.loader().abilities(ty)
}

pub fn extensions(&self) -> &NativeContextExtensions<'b> {
self.extensions
}
Expand Down
Loading

0 comments on commit ce985fc

Please sign in to comment.