diff --git a/fastpay_core/src/authority.rs b/fastpay_core/src/authority.rs index f3785f142722d..d0df24b74cff7 100644 --- a/fastpay_core/src/authority.rs +++ b/fastpay_core/src/authority.rs @@ -114,8 +114,9 @@ impl AuthorityState { FastPayError::IncorrectSigner ); + // The last object in input_objects is always the gas payment object. + // TODO: Add get_gas_object_ref() to Order once Transfer order also has gas object. if idx + 1 == input_object_cnt { - // The last object in input_objects is always the gas payment object. check_gas_requirement(&order, &object)?; } @@ -211,7 +212,7 @@ impl AuthorityState { c.type_arguments, inputs, c.pure_arguments, - Some(c.gas_budget), + c.gas_budget, gas_object, tx_ctx, ) { diff --git a/fastpay_core/src/authority/temporary_store.rs b/fastpay_core/src/authority/temporary_store.rs index 583cc6e4feb32..5c04b2b5c0c5f 100644 --- a/fastpay_core/src/authority/temporary_store.rs +++ b/fastpay_core/src/authority/temporary_store.rs @@ -75,6 +75,19 @@ impl AuthorityTemporaryStore { }, "Object both written and deleted." ); + + // Check all mutable inputs are either written or deleted + debug_assert!( + { + use std::collections::HashSet; + let mut used = HashSet::new(); + self.written.iter().all(|elt| used.insert(elt.0)); + self.deleted.iter().all(|elt| used.insert(elt.0)); + + self.active_inputs.iter().all(|elt| !used.insert(elt.0)) + }, + "Mutable input neither written nor deleted." + ); } } diff --git a/fastpay_core/src/unit_tests/authority_tests.rs b/fastpay_core/src/unit_tests/authority_tests.rs index 29c75d3f6cdb7..fce153d20b63c 100644 --- a/fastpay_core/src/unit_tests/authority_tests.rs +++ b/fastpay_core/src/unit_tests/authority_tests.rs @@ -7,7 +7,7 @@ use fastx_adapter::genesis; #[cfg(test)] use fastx_types::{ base_types::dbg_addr, - gas::{calculate_module_publish_gas, get_gas_balance}, + gas::{calculate_module_publish_cost, get_gas_balance}, }; use move_binary_format::{ file_format::{self, AddressIdentifierIndex, IdentifierIndex, ModuleHandle}, @@ -199,6 +199,7 @@ fn make_dependent_module(m: &CompiledModule) -> CompiledModule { dependent_module } +#[cfg(test)] fn check_gas_object( gas_object: &Object, expected_balance: u64, @@ -262,7 +263,7 @@ async fn test_publish_module_no_dependencies_ok() { let mut module_bytes = Vec::new(); module.serialize(&mut module_bytes).unwrap(); let module_bytes = vec![module_bytes]; - let gas_cost = calculate_module_publish_gas(&module_bytes); + let gas_cost = calculate_module_publish_cost(&module_bytes); let order = Order::new_module(sender, gas_payment_object_ref, module_bytes, &sender_key); let module_object_id = TxContext::new(order.digest()).fresh_id(); let response = send_and_confirm_order(&mut authority, order).await.unwrap(); @@ -319,7 +320,7 @@ async fn test_handle_move_order() { let mut genesis_module_objects = genesis.objects.clone(); let module_object_ref = genesis_module_objects .iter() - .find_map(|o| match o.data.as_module() { + .find_map(|o| match o.data.try_as_module() { Some(m) => { if m.self_id().name() == ident_str!("ObjectBasics") { Some((*m.self_id().address(), SequenceNumber::new())) @@ -350,7 +351,7 @@ async fn test_handle_move_order() { MAX_GAS, &sender_key, ); - let gas_cost = 142; + let gas_cost = 142 + 24; // 142 is for bytecode execution, 24 is for object creation. let res = send_and_confirm_order(&mut authority_state, order) .await .unwrap(); @@ -392,7 +393,7 @@ async fn test_handle_move_order_insufficient_budget() { let mut genesis_module_objects = genesis.objects.clone(); let module_object_ref = genesis_module_objects .iter() - .find_map(|o| match o.data.as_module() { + .find_map(|o| match o.data.try_as_module() { Some(m) if m.self_id().name() == ident_str!("ObjectBasics") => { Some((*m.self_id().address(), SequenceNumber::new())) } diff --git a/fastx_programmability/adapter/src/adapter.rs b/fastx_programmability/adapter/src/adapter.rs index bdb01d90014f9..7c7cf3414cc13 100644 --- a/fastx_programmability/adapter/src/adapter.rs +++ b/fastx_programmability/adapter/src/adapter.rs @@ -10,7 +10,10 @@ use fastx_types::{ TX_CONTEXT_MODULE_NAME, TX_CONTEXT_STRUCT_NAME, }, error::{FastPayError, FastPayResult}, - gas::{calculate_module_publish_gas, deduct_gas}, + gas::{ + calculate_module_publish_cost, calculate_object_creation_cost, + calculate_object_deletion_refund, deduct_gas, + }, object::{Data, MoveObject, Object}, storage::Storage, }; @@ -47,8 +50,8 @@ pub fn execute + ModuleResolver, object_args: Vec, pure_args: Vec>, - gas_budget: Option, - gas_object: Object, + gas_budget: u64, + mut gas_object: Object, ctx: TxContext, ) -> Result<(), FastPayError> { let TypeCheckSuccess { @@ -69,7 +72,7 @@ pub fn execute + ModuleResolver + ModuleResolver 0 || gas_budget == None); let mutable_refs = mutable_ref_objects .into_iter() .zip(mutable_ref_values.into_iter()); @@ -106,12 +107,20 @@ pub fn execute + ModuleResolver Err(FastPayError::AbortedExecution { - error: error.to_string(), - }), + ExecutionResult::Fail { error, gas_used } => { + // Need to deduct gas even if the execution failed. + deduct_gas(&mut gas_object, gas_used as i128)?; + state_view.write_object(gas_object); + // TODO: Keep track the gas deducted so that we could give them to participants. + + Err(FastPayError::AbortedExecution { + error: error.to_string(), + }) + } } } @@ -127,9 +136,10 @@ pub fn publish + ModuleResolver)>, events: Vec, mut gas_object: Object, - gas_used: u64, + mut gas_used: u64, + gas_budget: u64, ) -> Result<(), FastPayError> { for (mut obj, new_contents) in mutable_refs { match &mut obj.data { @@ -259,7 +270,9 @@ fn process_successful_execution< old_object.next_sequence_number = sequence_number; old_object } else { - Object::new_move(move_obj, recipient, SequenceNumber::new()) + let obj = Object::new_move(move_obj, recipient, SequenceNumber::new()); + gas_used += calculate_object_creation_cost(&obj); + obj }; new_object.owner = recipient; state_view.write_object(new_object); @@ -271,20 +284,29 @@ fn process_successful_execution< unimplemented!("Processing user events") } } + if gas_used > gas_budget { + return Err(FastPayError::InsufficientGas { + error: format!( + "Gas budget is {}, not enough to pay for cost {}", + gas_budget, gas_used + ), + }); + } // any object left in `by_value_objects` is an input passed by value that was not transferred or frozen. // this means that either the object was (1) deleted from the FastX system altogether, or // (2) wrapped inside another object that is in the FastX object pool // in either case, we want to delete it - for id in by_value_objects.keys() { + let mut gas_refund: u64 = 0; + for (id, object) in by_value_objects.iter() { state_view.delete_object(id); + gas_refund += calculate_object_deletion_refund(object); } - if gas_used > 0 { - // The fact that Move VM succeeded means that there is enough gas. - // Hence deduct_gas cannot fail. - deduct_gas(&mut gas_object, gas_used).unwrap(); - state_view.write_object(gas_object); - } + // TODO: In the current approach, we basically can use refund gas to pay for current transaction. + // Is this allowed? + deduct_gas(&mut gas_object, (gas_used as i128) - (gas_refund as i128))?; + state_view.write_object(gas_object); + // TODO: Keep track the gas deducted so that we could give them to participants. Ok(()) } @@ -392,7 +414,7 @@ fn resolve_and_type_check( let mut args = Vec::new(); let mut mutable_ref_objects = Vec::new(); let mut by_value_objects = BTreeMap::new(); - #[allow(unused_mut)] + #[cfg(debug_assertions)] let mut num_immutable_objects = 0; #[cfg(debug_assertions)] let num_objects = object_args.len(); diff --git a/fastx_programmability/adapter/src/unit_tests/adapter_tests.rs b/fastx_programmability/adapter/src/unit_tests/adapter_tests.rs index 84662356c4af4..1ce1ff0d2e758 100644 --- a/fastx_programmability/adapter/src/unit_tests/adapter_tests.rs +++ b/fastx_programmability/adapter/src/unit_tests/adapter_tests.rs @@ -40,7 +40,7 @@ impl InMemoryStorage { pub fn find_module(&self, name: &str) -> Option { let id = Identifier::new(name).unwrap(); for o in self.persistent.values() { - match o.data.as_module() { + match o.data.try_as_module() { Some(m) if m.self_id().name() == id.as_ident_str() => return Some(o.clone()), _ => (), } @@ -126,7 +126,7 @@ fn call( native_functions: &NativeFunctionTable, name: &str, gas_object: Object, - gas_budget: Option, + gas_budget: u64, object_args: Vec, pure_args: Vec>, ) -> FastPayResult { @@ -157,7 +157,11 @@ fn test_object_basics() { let mut storage = InMemoryStorage::new(genesis.objects.clone()); // 0. Create a gas object for gas payment. Note that we won't really use it because we won't be providing a gas budget. - let gas_object = Object::with_id_for_testing(ObjectID::random()); + let gas_object = Object::with_id_owner_gas_for_testing( + ObjectID::random(), + base_types::FastPayAddress::default(), + MAX_GAS, + ); storage.write_object(gas_object.clone()); storage.flush(); @@ -175,7 +179,7 @@ fn test_object_basics() { &native_functions, "create", gas_object.clone(), - None, + MAX_GAS, Vec::new(), pure_args, ) @@ -183,7 +187,7 @@ fn test_object_basics() { let created = storage.created(); assert_eq!(created.len(), 1); - assert!(storage.updated().is_empty()); + assert_eq!(storage.updated().len(), 1); // The gas object assert!(storage.deleted().is_empty()); let id1 = created .keys() @@ -204,14 +208,14 @@ fn test_object_basics() { &native_functions, "transfer", gas_object.clone(), - None, + MAX_GAS, vec![obj1.clone()], pure_args, ) .unwrap(); let updated = storage.updated(); - assert_eq!(updated.len(), 1); + assert_eq!(updated.len(), 2); assert!(storage.created().is_empty()); assert!(storage.deleted().is_empty()); storage.flush(); @@ -232,7 +236,7 @@ fn test_object_basics() { &native_functions, "create", gas_object.clone(), - None, + MAX_GAS, Vec::new(), pure_args, ) @@ -251,13 +255,13 @@ fn test_object_basics() { &native_functions, "update", gas_object.clone(), - None, + MAX_GAS, vec![obj1.clone(), obj2], Vec::new(), ) .unwrap(); let updated = storage.updated(); - assert_eq!(updated.len(), 1); + assert_eq!(updated.len(), 2); assert!(storage.created().is_empty()); assert!(storage.deleted().is_empty()); storage.flush(); @@ -274,7 +278,7 @@ fn test_object_basics() { &native_functions, "delete", gas_object, - None, + MAX_GAS, vec![obj1], Vec::new(), ) @@ -282,7 +286,7 @@ fn test_object_basics() { let deleted = storage.deleted(); assert_eq!(deleted.len(), 1); assert!(storage.created().is_empty()); - assert!(storage.updated().is_empty()); + assert_eq!(storage.updated().len(), 1); storage.flush(); assert!(storage.read_object(&id1).is_none()) } @@ -317,7 +321,7 @@ fn test_move_call_insufficient_gas() { &native_functions, "create", gas_object, - Some(50), // This budget is not enough to execute all bytecode. + 50, // This budget is not enough to execute all bytecode. Vec::new(), pure_args, ); diff --git a/fastx_types/src/gas.rs b/fastx_types/src/gas.rs index 044d599728647..b3f35b6c1fa76 100644 --- a/fastx_types/src/gas.rs +++ b/fastx_types/src/gas.rs @@ -29,7 +29,7 @@ pub fn check_gas_requirement(order: &Order, gas_object: &Object) -> FastPayResul Ok(()) } OrderKind::Publish(publish) => { - assert_eq!(publish.gas_payment.0, gas_object.id()); + debug_assert_eq!(publish.gas_payment.0, gas_object.id()); let balance = get_gas_balance(gas_object)?; ok_or_gas_error!( balance >= MIN_MOVE_PUBLISH_GAS, @@ -40,7 +40,7 @@ pub fn check_gas_requirement(order: &Order, gas_object: &Object) -> FastPayResul ) } OrderKind::Call(call) => { - assert_eq!(call.gas_payment.0, gas_object.id()); + debug_assert_eq!(call.gas_payment.0, gas_object.id()); ok_or_gas_error!( call.gas_budget >= MIN_MOVE_CALL_GAS, format!( @@ -60,15 +60,26 @@ pub fn check_gas_requirement(order: &Order, gas_object: &Object) -> FastPayResul } } -pub fn deduct_gas(gas_object: &mut Object, amount: u64) -> FastPayResult { +/// Subtract the gas balance of \p gas_object by \p amount. +/// \p amount being positive means gas deduction; \p amount being negative +/// means gas refund. +pub fn deduct_gas(gas_object: &mut Object, amount: i128) -> FastPayResult { let gas_coin = GasCoin::try_from(&*gas_object)?; - let balance = gas_coin.value(); + let balance = gas_coin.value() as i128; + let new_balance = balance - amount; ok_or_gas_error!( - balance >= amount, + new_balance >= 0, format!("Gas balance is {}, not enough to pay {}", balance, amount) )?; - let new_gas_coin = GasCoin::new(*gas_coin.id(), balance - amount); - gas_object.data.as_move_mut().unwrap().contents = bcs::to_bytes(&new_gas_coin).unwrap(); + ok_or_gas_error!( + new_balance <= u64::MAX as i128, + format!( + "Gas balance is {}, overflow after reclaiming {}", + balance, -amount + ) + )?; + let new_gas_coin = GasCoin::new(*gas_coin.id(), new_balance as u64); + gas_object.data.try_as_move_mut().unwrap().contents = bcs::to_bytes(&new_gas_coin).unwrap(); let sequence_number = gas_object.next_sequence_number.increment()?; gas_object.next_sequence_number = sequence_number; Ok(()) @@ -78,8 +89,18 @@ pub fn get_gas_balance(gas_object: &Object) -> FastPayResult { Ok(GasCoin::try_from(gas_object)?.value()) } -pub fn calculate_module_publish_gas(module_bytes: &[Vec]) -> u64 { +pub fn calculate_module_publish_cost(module_bytes: &[Vec]) -> u64 { // TODO: Figure out module publish gas formula. // Currently just use the size in bytes of the modules plus a default minimum. module_bytes.iter().map(|v| v.len() as u64).sum::() + MIN_MOVE_PUBLISH_GAS } + +pub fn calculate_object_creation_cost(object: &Object) -> u64 { + // TODO: Figure out object creation gas formula. + object.data.try_as_move().unwrap().contents.len() as u64 +} + +pub fn calculate_object_deletion_refund(object: &Object) -> u64 { + // TODO: Figure out object creation gas formula. + (object.data.try_as_move().unwrap().contents.len() / 2) as u64 +} diff --git a/fastx_types/src/object.rs b/fastx_types/src/object.rs index f79693914eade..64f4a25f68f2a 100644 --- a/fastx_types/src/object.rs +++ b/fastx_types/src/object.rs @@ -47,7 +47,7 @@ impl Data { } } - pub fn as_move(&self) -> Option<&MoveObject> { + pub fn try_as_move(&self) -> Option<&MoveObject> { use Data::*; match self { Move(m) => Some(m), @@ -55,7 +55,7 @@ impl Data { } } - pub fn as_move_mut(&mut self) -> Option<&mut MoveObject> { + pub fn try_as_move_mut(&mut self) -> Option<&mut MoveObject> { use Data::*; match self { Move(m) => Some(m), @@ -63,7 +63,7 @@ impl Data { } } - pub fn as_module(&self) -> Option { + pub fn try_as_module(&self) -> Option { use Data::*; match self { Move(_) => None,