Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bench for Move based transfer with Gas #190

Merged
merged 12 commits into from
Jan 19, 2022
98 changes: 85 additions & 13 deletions fastpay/src/bench.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,12 @@
use bytes::Bytes;
use fastpay::{network, transport};
use fastpay_core::authority::*;
use fastx_types::FASTX_FRAMEWORK_ADDRESS;
use fastx_types::{base_types::*, committee::*, messages::*, object::Object, serialize::*};
use futures::stream::StreamExt;
use log::*;
use move_core_types::ident_str;
use rand::Rng;
use std::time::{Duration, Instant};
use structopt::StructOpt;
use tokio::runtime::Runtime;
Expand Down Expand Up @@ -67,6 +70,9 @@ struct ClientServerBenchmark {
/// Number of database cpus
#[structopt(long, default_value = "1")]
db_cpus: usize,
/// Use Move orders
#[structopt(long)]
use_move: bool,
}
#[derive(Debug, Clone, PartialEq, EnumString)]
enum BenchmarkType {
Expand Down Expand Up @@ -160,20 +166,30 @@ impl ClientServerBenchmark {
let keypair = get_key_pair();
let object_id: ObjectID = ObjectID::random();

let object = Object::with_id_owner_for_testing(object_id, keypair.0);
let object = if self.use_move {
Object::with_id_owner_gas_coin_object_for_testing(
ObjectID::random(),
SequenceNumber::new(),
keypair.0,
rand::thread_rng().gen_range(0..0xFFFFFF),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we use a seeded RNG here? Want to make sure we get the same values across different runs.

)
} else {
Object::with_id_owner_for_testing(object_id, keypair.0)
};

assert!(object.version() == SequenceNumber::from(0));
let object_ref = object.to_object_reference();
state.init_order_lock(object_ref).await;
account_objects.push((keypair.0, object.clone(), keypair.1));
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.version() == SequenceNumber::from(0));
let gas_object_ref = gas_object.to_object_reference();
state.init_order_lock(gas_object_ref).await;
gas_objects.push(gas_object.clone());
state.insert_object(gas_object).await;
gas_objects.push(gas_object_ref);
}
state
});
Expand All @@ -182,15 +198,47 @@ impl ClientServerBenchmark {
// Make one transaction per account (transfer order + confirmation).
let mut orders: Vec<Bytes> = Vec::new();
let mut next_recipient = get_key_pair().0;
for ((pubx, object_ref, secx), gas_payment) in account_objects.iter().zip(gas_objects) {
let transfer = Transfer {
object_ref: *object_ref,
sender: *pubx,
recipient: Address::FastPay(next_recipient),
gas_payment,
for ((account_addr, object, secret), gas_obj) in account_objects.iter().zip(gas_objects) {
let object_ref = object.to_object_reference();
let gas_object_ref = gas_obj.to_object_reference();

let order = if self.use_move {
// TODO: authority should not require seq# or digets for package in Move calls. Use dummy values
let framework_obj_ref = (
FASTX_FRAMEWORK_ADDRESS,
SequenceNumber::new(),
ObjectDigest::new([0; 32]),
);

Order::new_move_call(
*account_addr,
framework_obj_ref,
ident_str!("GAS").to_owned(),
ident_str!("transfer").to_owned(),
Vec::new(),
// Do I really need this TypeArg? Does not make a diff

//vec![move_core_types::language_storage::TypeTag::Struct(
//GasCoin::type_(),
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sblackshear specifying type_args did not make a difference here so I committed them.
However I reckon they might be useful when calling a generic fn

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We will need them eventually, but punting on them until we use Coin::transfer seems reasonable.

//)],
gas_object_ref,
vec![object_ref],
vec![bcs::to_bytes(&next_recipient.to_vec()).unwrap()],
1000,
secret,
)
} else {
let transfer = Transfer {
sender: *account_addr,
recipient: Address::FastPay(next_recipient),
object_ref,
gas_payment: gas_object_ref,
};
Order::new_transfer(transfer, secret)
};
next_recipient = *pubx;
let order = Order::new_transfer(transfer, secx);

// Set the next recipient to current
next_recipient = *account_addr;

// Serialize order
let serialized_order = serialize_order(&order);
Expand All @@ -210,10 +258,10 @@ impl ClientServerBenchmark {
let serialized_certificate = serialize_cert(&certificate);
assert!(!serialized_certificate.is_empty());

if self.benchmark_type != BenchmarkType::OrdersOnly {
if self.benchmark_type != BenchmarkType::CertsOnly {
orders.push(serialized_order.into());
}
if self.benchmark_type != BenchmarkType::CertsOnly {
if self.benchmark_type != BenchmarkType::OrdersOnly {
orders.push(serialized_certificate.into());
}
}
Expand Down Expand Up @@ -261,6 +309,30 @@ impl ClientServerBenchmark {

let responses = mass_client.run(orders, connections).concat().await;
info!("Received {} responses.", responses.len(),);
// Check the responses for errors
for resp in responses {
let reply_message = deserialize_message(&resp[..]);
match reply_message {
Ok(SerializedMessage::OrderResp(resp)) =>
oxade marked this conversation as resolved.
Show resolved Hide resolved
{
#[allow(clippy::collapsible_if)]
if resp.signed_effects.is_some() {
if resp.signed_effects.as_ref().unwrap().effects.status
!= ExecutionStatus::Success
{
info!(
"Execution Error {:?}",
resp.signed_effects.unwrap().effects.status
);
}
}
}
Err(err) => {
info!("Received Error {:?}", err);
}
_ => {}
};
}
} else {
// Use actual client core
let client = network::Client::new(
Expand Down
14 changes: 10 additions & 4 deletions fastpay_core/src/authority.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,8 @@ pub struct AuthorityState {
pub secret: KeyPair,

/// Move native functions that are available to invoke
native_functions: NativeFunctionTable,
_native_functions: NativeFunctionTable,
move_vm: Arc<adapter::MoveVM>,
/// The database
_database: Arc<AuthorityStore>,
}
Expand Down Expand Up @@ -239,8 +240,9 @@ impl AuthorityState {
let gas_object = inputs.pop().unwrap();
let package = inputs.pop().unwrap();
adapter::execute(
&self.move_vm,
&mut temporary_store,
self.native_functions.clone(),
self._native_functions.clone(),
package,
&c.module,
&c.function,
Expand Down Expand Up @@ -332,7 +334,9 @@ impl AuthorityState {
committee,
name,
secret,
native_functions,
_native_functions: native_functions.clone(),
move_vm: adapter::new_move_vm(native_functions)
.expect("We defined natives to not fail here"),
_database: store,
};

Expand All @@ -355,11 +359,13 @@ impl AuthorityState {
secret: KeyPair,
store: Arc<AuthorityStore>,
) -> Self {
let native_functions = NativeFunctionTable::new();
AuthorityState {
committee,
name,
secret,
native_functions: NativeFunctionTable::new(),
_native_functions: native_functions.clone(),
move_vm: adapter::new_move_vm(native_functions).expect("Only fails due to natives."),
_database: store,
}
}
Expand Down
15 changes: 12 additions & 3 deletions fastpay_core/src/unit_tests/authority_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -387,7 +387,8 @@ async fn test_handle_move_order() {

genesis_package_objects.push(gas_payment_object);
let mut authority_state = init_state_with_objects(genesis_package_objects).await;
authority_state.native_functions = native_functions;
authority_state._native_functions = native_functions.clone();
authority_state.move_vm = adapter::new_move_vm(native_functions).unwrap();

let function = ident_str!("create").to_owned();
let order = Order::new_move_call(
Expand All @@ -414,8 +415,15 @@ async fn test_handle_move_order() {

// Check that effects are reported
assert!(res.signed_effects.is_some());
res.signed_effects
.as_ref()
.unwrap()
.effects
.status
.as_ref()
.unwrap();
let mutated = res.signed_effects.unwrap().effects.mutated;
assert!(mutated.len() == 2);
assert_eq!(mutated.len(), 2);

let created_object_id = mutated[0].0; // res.object_id;
// check that order actually created an object with the expected ID, owner, sequence number
Expand Down Expand Up @@ -466,7 +474,8 @@ async fn test_handle_move_order_insufficient_budget() {

genesis_package_objects.push(gas_payment_object);
let mut authority_state = init_state_with_objects(genesis_package_objects).await;
authority_state.native_functions = native_functions;
authority_state._native_functions = native_functions.clone();
authority_state.move_vm = adapter::new_move_vm(native_functions).unwrap();

let function = ident_str!("create").to_owned();
let order = Order::new_move_call(
Expand Down
19 changes: 12 additions & 7 deletions fastx_programmability/adapter/src/adapter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,22 +30,29 @@ use move_core_types::{
language_storage::{ModuleId, StructTag, TypeTag},
resolver::{ModuleResolver, ResourceResolver},
};
use move_vm_runtime::{
move_vm::MoveVM, native_functions::NativeFunctionTable, session::ExecutionResult,
};
use std::{borrow::Borrow, collections::BTreeMap, convert::TryFrom, fmt::Debug};
use move_vm_runtime::{native_functions::NativeFunctionTable, session::ExecutionResult};
use std::{borrow::Borrow, collections::BTreeMap, convert::TryFrom, fmt::Debug, sync::Arc};

pub use move_vm_runtime::move_vm::MoveVM;

#[cfg(test)]
#[path = "unit_tests/adapter_tests.rs"]
mod adapter_tests;

pub fn new_move_vm(natives: NativeFunctionTable) -> Result<Arc<MoveVM>, FastPayError> {
Ok(Arc::new(
MoveVM::new(natives).map_err(|_| FastPayError::ExecutionInvariantViolation)?,
))
}

/// Execute `module::function<type_args>(object_args ++ pure_args)` as a call from `sender` with the given `gas_budget`.
/// Execution will read from/write to the store in `state_view`.
/// If `gas_budget` is None, runtime metering is disabled and execution may diverge.
#[allow(clippy::too_many_arguments)]
pub fn execute<E: Debug, S: ResourceResolver<Error = E> + ModuleResolver<Error = E> + Storage>(
vm: &MoveVM,
state_view: &mut S,
natives: NativeFunctionTable,
_natives: NativeFunctionTable,
package_object: Object,
module: &Identifier,
function: &Identifier,
Expand All @@ -71,8 +78,6 @@ pub fn execute<E: Debug, S: ResourceResolver<Error = E> + ModuleResolver<Error =
&ctx,
)?;

let vm = MoveVM::new(natives)
.expect("VM creation only fails if natives are invalid, and we created the natives");
// TODO: Update Move gas constants to reflect the gas fee on fastx.
let mut gas_status =
get_gas_status(Some(gas_budget)).map_err(|e| FastPayError::GasBudgetTooHigh {
Expand Down
2 changes: 2 additions & 0 deletions fastx_programmability/adapter/src/unit_tests/adapter_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,9 @@ fn call(
) -> FastPayResult {
let package = storage.find_package("ObjectBasics").unwrap();

let vm = adapter::new_move_vm(native_functions.clone()).expect("No errors");
adapter::execute(
&vm,
storage,
native_functions.clone(),
package,
Expand Down
7 changes: 7 additions & 0 deletions fastx_programmability/framework/sources/GAS.move
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
module FastX::GAS {
use FastX::Coin;
use FastX::Transfer;
use FastX::Address;
use FastX::TxContext::{Self, TxContext};

/// Name of the coin
Expand All @@ -16,4 +17,10 @@ module FastX::GAS {
let treasury_cap = Coin::create_currency(GAS{}, ctx);
Transfer::transfer(treasury_cap, TxContext::get_signer_address(ctx))
}

/// Transfer to a recipient
public fun transfer(c: Coin::Coin<GAS>, recipient: vector<u8>, _ctx: &mut TxContext) {
Coin::transfer(c, Address::new(recipient))
}

}
5 changes: 2 additions & 3 deletions fastx_types/src/coin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ use serde::{Deserialize, Serialize};

use crate::{
base_types::{ObjectID, SequenceNumber},
gas_coin::{GAS_MODULE_NAME, GAS_STRUCT_NAME},
id::ID,
FASTX_FRAMEWORK_ADDRESS,
};
Expand All @@ -33,8 +32,8 @@ impl Coin {
pub fn type_(type_param: StructTag) -> StructTag {
StructTag {
address: FASTX_FRAMEWORK_ADDRESS,
name: GAS_STRUCT_NAME.to_owned(),
module: GAS_MODULE_NAME.to_owned(),
name: COIN_STRUCT_NAME.to_owned(),
module: COIN_MODULE_NAME.to_owned(),
type_params: vec![TypeTag::Struct(type_param)],
}
}
Expand Down
Loading