Skip to content

Commit

Permalink
[fastx framework] Maintain object sequence numbers across unwrapping
Browse files Browse the repository at this point in the history
Trying approach (1) to addressing #98. In particular, this:

- Extends the Move `ID` type to include a `version`. This allows all Move objects to carry their verson, and thus persist it across wrapping and unwrapping. But having it live inside `ID` saves us from having to rewrite the `ID`-related bytecode verifier passes or ask the programmer to write `struct S has key { id: ID, version: Version, ... }` to declare a FastX object. In fact, there are no changes from the programmer's perspective except that they can now read an object's version inside Move if they wish to.
- Removed `version` from the Rust object types, since it now lives in the Move-managed `contents` field. Added a variety of helper functions for reading and writing the `version from Rust.
- Changed the adapter to understand the new location of `version`. This actually simplifies the adapter code quite a bit.
- Added a test that demonstrates that sequence numbers are maintained correctly across wrapping and unwrapping.

There is one change introduced here that is important enough to mention separately *object versions now begin at 1*. That is, if a tranaction creates and then transfers an object `X`, the version of `X` in the transaction effects will be 1, not 0. The reasons why this change is needed are somewhat subtle.
- This change happens because the adapter increments the sequence number of every transferred object.
- You could imagine asking the adapter to instead check whether a transferred object was created by the current transaction, passed as an input, or unwrapped + only incrementing the sequence number in the second two cases. However, if sequence numbers of freshly created objects started at 0, the adapter would actually not be able to tell the difference between a freshly created object `O1` (will have seq 0) and an object `O2` that was created (will have seq 0), then subsquently wrapped (will still have seq 0), and later unwrapped (will still have seq 0!).
- Another solution to this problem would be incrementing the sequence number of all objects passed as inputs to the transaction before beginning execution. This would allow freshly created objects to start at seq 0, but it would be slightly odd in that the programmer would pass in an object with seq `S`, but would see `S+1 if they try to read the sequence number from Move during the transaction. It seems most intuitive to maintain the invariant that the object you put into the transaction is exactly what you will read inside the transaction. In addition, that would require us to do the "created or transferred/unwrapped" special-casing described above, which makes the adapter a bit more complicated.
  • Loading branch information
sblackshear committed Jan 10, 2022
1 parent b15f328 commit e7b2ecb
Show file tree
Hide file tree
Showing 19 changed files with 445 additions and 192 deletions.
4 changes: 2 additions & 2 deletions fastpay/src/bench.rs
Original file line number Diff line number Diff line change
Expand Up @@ -133,15 +133,15 @@ impl ClientServerBenchmark {
let object_id: ObjectID = ObjectID::random();

let object = Object::with_id_owner_for_testing(object_id, keypair.0);
assert!(object.next_sequence_number == SequenceNumber::from(0));
assert!(object.version() == SequenceNumber::from(0));
let object_ref = object.to_object_reference();
state.init_order_lock(object_ref).await;
state.insert_object(object).await;
account_objects.push((keypair.0, object_ref, keypair.1));

let gas_object_id = ObjectID::random();
let gas_object = Object::with_id_owner_for_testing(gas_object_id, keypair.0);
assert!(gas_object.next_sequence_number == SequenceNumber::from(0));
assert!(gas_object.version() == SequenceNumber::from(0));
let gas_object_ref = gas_object.to_object_reference();
state.init_order_lock(gas_object_ref).await;
state.insert_object(gas_object).await;
Expand Down
19 changes: 10 additions & 9 deletions fastpay_core/src/authority.rs
Original file line number Diff line number Diff line change
Expand Up @@ -103,10 +103,10 @@ impl AuthorityState {

// Check that the seq number is the same
fp_ensure!(
object.next_sequence_number == sequence_number,
object.version() == sequence_number,
FastPayError::UnexpectedSequenceNumber {
object_id,
expected_sequence: object.next_sequence_number,
expected_sequence: object.version(),
received_sequence: sequence_number
}
);
Expand Down Expand Up @@ -180,7 +180,7 @@ impl AuthorityState {
.await
.map_err(|_| FastPayError::ObjectNotFound)?;

let input_sequence_number = input_object.next_sequence_number;
let input_sequence_number = input_object.version();

// Check that the current object is exactly the right version.
if input_sequence_number < input_seq {
Expand All @@ -204,6 +204,10 @@ impl AuthorityState {
let mut temporary_store = AuthorityTemporaryStore::new(self, &inputs);
let _status = match order.kind {
OrderKind::Transfer(t) => {
debug_assert!(
inputs.len() == 2,
"Expecting two inputs: gas and object to be transferred"
);
// unwraps here are safe because we built `inputs`
let mut gas_object = inputs.pop().unwrap();
deduct_gas(
Expand All @@ -212,14 +216,11 @@ impl AuthorityState {
)?;
temporary_store.write_object(gas_object);

let mut output_object = inputs[0].clone();
output_object.next_sequence_number =
output_object.next_sequence_number.increment()?;

let mut output_object = inputs.pop().unwrap();
output_object.transfer(match t.recipient {
Address::Primary(_) => FastPayAddress::default(),
Address::FastPay(addr) => addr,
});
})?;
temporary_store.write_object(output_object);
Ok(())
}
Expand Down Expand Up @@ -354,7 +355,7 @@ impl AuthorityState {
Ok(ObjectInfoResponse {
object_id: object.id(),
owner: object.owner,
next_sequence_number: object.next_sequence_number,
next_sequence_number: object.version(),
requested_certificate,
pending_confirmation: lock,
requested_received_transfers: Vec::new(),
Expand Down
2 changes: 1 addition & 1 deletion fastpay_core/src/authority/authority_store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ impl AuthorityStore {
.objects
.iter()
.filter(|(_, object)| object.owner == account)
.map(|(id, object)| (id, object.next_sequence_number, object.digest()))
.map(|(id, object)| (id, object.version(), object.digest()))
.collect())
}

Expand Down
2 changes: 1 addition & 1 deletion fastpay_core/src/authority/temporary_store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,7 @@ impl ResourceResolver for AuthorityTemporaryStore {
Data::Move(m) => {
assert!(struct_tag == &m.type_, "Invariant violation: ill-typed object in storage or bad object request from caller\
");
Ok(Some(m.contents.clone()))
Ok(Some(m.contents().to_vec()))
}
other => unimplemented!(
"Bad object lookup: expected Move object, but got {:?}",
Expand Down
95 changes: 45 additions & 50 deletions fastpay_core/src/unit_tests/authority_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -113,17 +113,17 @@ fn test_handle_transfer_order_bad_sequence_number() {
let mut sequence_number_state = authority_state;
let sequence_number_state_sender_account =
sequence_number_state.objects.get_mut(&object_id).unwrap();
sequence_number_state_sender_account.next_sequence_number =
sequence_number_state_sender_account.version() =
sequence_number_state_sender_account
.next_sequence_number
.version()
.increment()
.unwrap();
assert!(sequence_number_state
.handle_transfer_order(transfer_order)
.is_err());
let object = sequence_number_state.objects.get(&object_id).unwrap();
assert!(sequence_number_state.get_order_lock(object.id, object.next_sequence_number).unwrap().is_none());
assert!(sequence_number_state.get_order_lock(object.id, object.version()).unwrap().is_none());
}
*/

Expand Down Expand Up @@ -192,7 +192,8 @@ async fn test_handle_transfer_zero_balance() {

// Create a gas object with 0 balance.
let gas_object_id = ObjectID::random();
let gas_object = Object::with_id_owner_gas_for_testing(gas_object_id, sender, 0);
let gas_object =
Object::with_id_owner_gas_for_testing(gas_object_id, SequenceNumber::new(), sender, 0);
authority_state
.init_order_lock((gas_object_id, 0.into(), gas_object.digest()))
.await;
Expand Down Expand Up @@ -251,7 +252,7 @@ fn check_gas_object(
expected_balance: u64,
expected_sequence_number: SequenceNumber,
) {
assert_eq!(gas_object.next_sequence_number, expected_sequence_number);
assert_eq!(gas_object.version(), expected_sequence_number);
let new_balance = get_gas_balance(gas_object).unwrap();
assert_eq!(new_balance, expected_balance);
}
Expand Down Expand Up @@ -302,9 +303,9 @@ async fn test_publish_module_no_dependencies_ok() {
let (sender, sender_key) = get_key_pair();
let gas_payment_object_id = ObjectID::random();
let gas_balance = MAX_GAS;
let gas_seq = SequenceNumber::new();
let gas_payment_object =
Object::with_id_owner_gas_for_testing(gas_payment_object_id, sender, gas_balance);
let gas_seq = gas_payment_object.next_sequence_number;
Object::with_id_owner_gas_for_testing(gas_payment_object_id, gas_seq, sender, gas_balance);
let gas_payment_object_ref = gas_payment_object.to_object_reference();
let mut authority = init_state_with_objects(vec![gas_payment_object]).await;

Expand Down Expand Up @@ -340,8 +341,12 @@ async fn test_publish_module_insufficient_gas() {
let (sender, sender_key) = get_key_pair();
let gas_payment_object_id = ObjectID::random();
let gas_balance = 9;
let gas_payment_object =
Object::with_id_owner_gas_for_testing(gas_payment_object_id, sender, gas_balance);
let gas_payment_object = Object::with_id_owner_gas_for_testing(
gas_payment_object_id,
SequenceNumber::new(),
sender,
gas_balance,
);
let gas_payment_object_ref = gas_payment_object.to_object_reference();
let authority = init_state_with_objects(vec![gas_payment_object]).await;

Expand All @@ -361,9 +366,9 @@ async fn test_handle_move_order() {
let (sender, sender_key) = get_key_pair();
let gas_payment_object_id = ObjectID::random();
let gas_balance = MAX_GAS;
let gas_seq = SequenceNumber::new();
let gas_payment_object =
Object::with_id_owner_gas_for_testing(gas_payment_object_id, sender, gas_balance);
let gas_seq = gas_payment_object.next_sequence_number;
Object::with_id_owner_gas_for_testing(gas_payment_object_id, gas_seq, sender, gas_balance);
let gas_payment_object_ref = gas_payment_object.to_object_reference();
// find the function Object::create and call it to create a new object
let genesis = genesis::GENESIS.lock().unwrap();
Expand Down Expand Up @@ -404,9 +409,10 @@ async fn test_handle_move_order() {
MAX_GAS,
&sender_key,
);
// 27 is for bytecode execution, 24 is for object creation.

// 36 is for bytecode execution, 24 is for object creation.
// If the number changes, we want to verify that the change is intended.
let gas_cost = 27 + 24;
let gas_cost = 36 + 24;
let res = send_and_confirm_order(&mut authority_state, order)
.await
.unwrap();
Expand All @@ -424,7 +430,7 @@ async fn test_handle_move_order() {
.unwrap();
assert_eq!(created_obj.owner, sender,);
assert_eq!(created_obj.id(), created_object_id);
assert_eq!(created_obj.next_sequence_number, SequenceNumber::new());
assert_eq!(created_obj.version(), SequenceNumber::from(1));

// Check that gas is properly deducted.
let gas_payment_object = authority_state
Expand Down Expand Up @@ -546,7 +552,7 @@ async fn test_handle_confirmation_order_bad_sequence_number() {
let old_seq_num;
{
let old_account = authority_state.object_state(&object_id).await.unwrap();
old_seq_num = old_account.next_sequence_number;
old_seq_num = old_account.version();
}

let certified_transfer_order = init_certified_transfer_order(
Expand All @@ -561,8 +567,10 @@ async fn test_handle_confirmation_order_bad_sequence_number() {
// Increment the sequence number
{
let mut sender_object = authority_state.object_state(&object_id).await.unwrap();
sender_object.next_sequence_number =
sender_object.next_sequence_number.increment().unwrap();
let o = sender_object.data.try_as_move_mut().unwrap();
let old_contents = o.contents().to_vec();
// update object contents, which will increment the sequence number
o.update_contents(old_contents).unwrap();
authority_state.insert_object(sender_object).await;
}

Expand All @@ -575,10 +583,7 @@ async fn test_handle_confirmation_order_bad_sequence_number() {

// Check that the new object is the one recorded.
let new_account = authority_state.object_state(&object_id).await.unwrap();
assert_eq!(
old_seq_num.increment().unwrap(),
new_account.next_sequence_number
);
assert_eq!(old_seq_num.increment().unwrap(), new_account.version());

// No recipient object was created.
assert!(authority_state
Expand Down Expand Up @@ -609,13 +614,9 @@ async fn test_handle_confirmation_order_exceed_balance() {
.await
.is_ok());
let new_account = authority_state.object_state(&object_id).await.unwrap();
assert_eq!(SequenceNumber::from(1), new_account.next_sequence_number);
assert_eq!(SequenceNumber::from(1), new_account.version());
assert!(authority_state
.parent(&(
object_id,
new_account.next_sequence_number,
new_account.digest()
))
.parent(&(object_id, new_account.version(), new_account.digest()))
.await
.is_some());
}
Expand Down Expand Up @@ -646,15 +647,12 @@ async fn test_handle_confirmation_order_receiver_balance_overflow() {
.await
.is_ok());
let new_sender_account = authority_state.object_state(&object_id).await.unwrap();
assert_eq!(
SequenceNumber::from(1),
new_sender_account.next_sequence_number
);
assert_eq!(SequenceNumber::from(1), new_sender_account.version());

assert!(authority_state
.parent(&(
object_id,
new_sender_account.next_sequence_number,
new_sender_account.version(),
new_sender_account.digest()
))
.await
Expand Down Expand Up @@ -682,10 +680,10 @@ async fn test_handle_confirmation_order_receiver_equal_sender() {
.await
.is_ok());
let account = authority_state.object_state(&object_id).await.unwrap();
assert_eq!(SequenceNumber::from(1), account.next_sequence_number);
assert_eq!(SequenceNumber::from(1), account.version());

assert!(authority_state
.parent(&(object_id, account.next_sequence_number, account.digest()))
.parent(&(object_id, account.version(), account.digest()))
.await
.is_some());
}
Expand All @@ -700,7 +698,12 @@ async fn test_handle_confirmation_order_gas() {

// Create a gas object with insufficient balance.
let gas_object_id = ObjectID::random();
let gas_object = Object::with_id_owner_gas_for_testing(gas_object_id, sender, gas);
let gas_object = Object::with_id_owner_gas_for_testing(
gas_object_id,
SequenceNumber::new(),
sender,
gas,
);
authority_state
.init_order_lock((gas_object_id, 0.into(), gas_object.digest()))
.await;
Expand All @@ -720,10 +723,8 @@ async fn test_handle_confirmation_order_gas() {
.await
};
let result = run_test_with_gas(10).await;
assert!(result
.unwrap_err()
.to_string()
.contains("Gas balance is 10, not enough to pay 12"));
let err_string = result.unwrap_err().to_string();
assert!(err_string.contains("Gas balance is 10, not enough to pay 16"));
let result = run_test_with_gas(20).await;
assert!(result.is_ok());
}
Expand All @@ -746,7 +747,7 @@ async fn test_handle_confirmation_order_ok() {
);

let old_account = authority_state.object_state(&object_id).await.unwrap();
let mut next_sequence_number = old_account.next_sequence_number;
let mut next_sequence_number = old_account.version();
next_sequence_number = next_sequence_number.increment().unwrap();

let info = authority_state
Expand All @@ -757,16 +758,12 @@ async fn test_handle_confirmation_order_ok() {

let new_account = authority_state.object_state(&object_id).await.unwrap();
assert_eq!(recipient, new_account.owner);
assert_eq!(next_sequence_number, new_account.next_sequence_number);
assert_eq!(next_sequence_number, new_account.version());
assert_eq!(None, info.signed_order);
assert_eq!(
{
let refx = authority_state
.parent(&(
object_id,
new_account.next_sequence_number,
new_account.digest(),
))
.parent(&(object_id, new_account.version(), new_account.digest()))
.await
.unwrap();
authority_state.read_certificate(&refx).await.unwrap()
Expand Down Expand Up @@ -867,8 +864,7 @@ async fn test_authority_persist() {
// Create an object
let recipient = dbg_addr(2);
let object_id = ObjectID::random();
let mut obj = Object::with_id_for_testing(object_id);
obj.transfer(recipient);
let obj = Object::with_id_owner_for_testing(object_id, recipient);

// Store an object
authority
Expand Down Expand Up @@ -920,8 +916,7 @@ async fn init_state_with_ids<I: IntoIterator<Item = (FastPayAddress, ObjectID)>>
) -> AuthorityState {
let state = init_state();
for (address, object_id) in objects {
let mut obj = Object::with_id_for_testing(object_id);
obj.transfer(address);
let obj = Object::with_id_owner_for_testing(object_id, address);
state
.init_order_lock((object_id, 0.into(), obj.digest()))
.await;
Expand Down
3 changes: 1 addition & 2 deletions fastpay_core/src/unit_tests/client_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -158,8 +158,7 @@ async fn fund_account<I: IntoIterator<Item = Vec<ObjectID>>>(
) {
for (authority, object_ids) in authorities.into_iter().zip(object_ids.into_iter()) {
for object_id in object_ids {
let mut object = Object::with_id_for_testing(object_id);
object.transfer(client.address);
let object = Object::with_id_owner_for_testing(object_id, client.address);
let client_ref = authority.0.as_ref().try_lock().unwrap();

client_ref
Expand Down
Loading

0 comments on commit e7b2ecb

Please sign in to comment.