From eece5d273ef8732fd3eeb12150d6cb574cfb4f78 Mon Sep 17 00:00:00 2001 From: Greg Nazario Date: Wed, 9 Nov 2022 11:25:06 -0800 Subject: [PATCH] [cli] Cleanup Simulation in CLI API Errors for simulation are fixed, so we use these now. Additionally, gas estimation and associated TODOs are cleaned up. --- crates/aptos-rest-client/src/lib.rs | 47 ++++++++------ crates/aptos/src/common/types.rs | 95 ++++------------------------- 2 files changed, 41 insertions(+), 101 deletions(-) diff --git a/crates/aptos-rest-client/src/lib.rs b/crates/aptos-rest-client/src/lib.rs index 16bf496f4c3c3..e6a48850ffd3e 100644 --- a/crates/aptos-rest-client/src/lib.rs +++ b/crates/aptos-rest-client/src/lib.rs @@ -24,9 +24,9 @@ use anyhow::{anyhow, Result}; use aptos_api_types::{ deserialize_from_string, mime_types::{BCS, BCS_SIGNED_TRANSACTION as BCS_CONTENT_TYPE}, - AptosError, BcsBlock, Block, Bytecode, ExplainVMStatus, GasEstimation, HexEncodedBytes, - IndexResponse, MoveModuleId, TransactionData, TransactionOnChainData, - TransactionsBatchSubmissionResult, UserTransaction, VersionedEvent, + AptosError, BcsBlock, Block, GasEstimation, HexEncodedBytes, IndexResponse, MoveModuleId, + TransactionData, TransactionOnChainData, TransactionsBatchSubmissionResult, UserTransaction, + VersionedEvent, }; use aptos_crypto::HashValue; use aptos_logger::{debug, info, sample, sample::SampleRate}; @@ -36,16 +36,13 @@ use aptos_types::{ contract_event::EventWithVersion, transaction::SignedTransaction, }; -use futures::executor::block_on; -use move_binary_format::CompiledModule; -use move_core_types::language_storage::{ModuleId, StructTag}; +use move_core_types::language_storage::StructTag; use reqwest::header::ACCEPT; use reqwest::{header::CONTENT_TYPE, Client as ReqwestClient, StatusCode}; use serde::{de::DeserializeOwned, Deserialize, Serialize}; use serde_json::{json, Value}; use std::collections::BTreeMap; use std::future::Future; -use std::rc::Rc; use std::time::Duration; use tokio::time::Instant; use url::Url; @@ -326,6 +323,30 @@ impl Client { self.json(response).await } + pub async fn simulate_with_gas_estimation( + &self, + txn: &SignedTransaction, + estimate_max_gas_amount: bool, + estimate_max_gas_unit_price: bool, + ) -> AptosResult>> { + let txn_payload = bcs::to_bytes(txn)?; + + let url = self.build_path(&format!( + "transactions/simulate?estimate_max_gas_amount={}&estimate_gas_unit_price={}", + estimate_max_gas_amount, estimate_max_gas_unit_price + ))?; + + let response = self + .inner + .post(url) + .header(CONTENT_TYPE, BCS_CONTENT_TYPE) + .body(txn_payload) + .send() + .await?; + + self.json(response).await + } + pub async fn simulate_bcs( &self, txn: &SignedTransaction, @@ -1550,15 +1571,3 @@ enum WaitForTransactionResult { Pending(State), Success(Response), } - -impl ExplainVMStatus for Client { - // TODO: Add some caching - fn get_module_bytecode(&self, module_id: &ModuleId) -> Result> { - let bytes = - block_on(self.get_account_module_bcs(*module_id.address(), module_id.name().as_str()))? - .into_inner(); - - let compiled_module = CompiledModule::deserialize(bytes.as_ref())?; - Ok(Rc::new(compiled_module) as Rc) - } -} diff --git a/crates/aptos/src/common/types.rs b/crates/aptos/src/common/types.rs index 612319113bb1b..6140b26f675e2 100644 --- a/crates/aptos/src/common/types.rs +++ b/crates/aptos/src/common/types.rs @@ -19,7 +19,7 @@ use aptos_crypto::{ }; use aptos_global_constants::adjust_gas_headroom; use aptos_keygen::KeyGen; -use aptos_rest_client::aptos_api_types::{ExplainVMStatus, HashValue, UserTransaction}; +use aptos_rest_client::aptos_api_types::HashValue; use aptos_rest_client::error::RestError; use aptos_rest_client::{Client, Transaction}; use aptos_sdk::{transaction_builder::TransactionFactory, types::LocalAccount}; @@ -45,7 +45,6 @@ use std::{ }; use thiserror::Error; -const MAX_POSSIBLE_GAS_UNITS: u64 = 1_000_000; pub const DEFAULT_PROFILE: &str = "default"; /// A common result to be returned to users @@ -1341,33 +1340,28 @@ impl TransactionOptions { sender_key.public_key(), Ed25519Signature::try_from([0u8; 64].as_ref()).unwrap(), ); - // TODO: Cleanup to use the gas price estimation here - let simulated_txn = client - .simulate_bcs_with_gas_estimation(&signed_transaction, true, false) + + let txns = client + .simulate_with_gas_estimation(&signed_transaction, true, false) .await? .into_inner(); + let simulated_txn = txns.first().unwrap(); // Check if the transaction will pass, if it doesn't then fail - // TODO: Add move resolver so we can explain the VM status with a proper error map - let status = simulated_txn.info.status(); - if !status.is_success() { - let status = client.explain_vm_status(status); - return Err(CliError::SimulationError(status)); + if !simulated_txn.info.success { + return Err(CliError::SimulationError( + simulated_txn.info.vm_status.clone(), + )); } // Take the gas used and use a headroom factor on it - let adjusted_max_gas = adjust_gas_headroom( - simulated_txn.info.gas_used(), - simulated_txn - .transaction - .as_signed_user_txn() - .expect("Should be signed user transaction") - .max_gas_amount(), - ); + let gas_used = simulated_txn.info.gas_used.0; + let adjusted_max_gas = + adjust_gas_headroom(gas_used, simulated_txn.request.max_gas_amount.0); // Ask if you want to accept the estimate amount let upper_cost_bound = adjusted_max_gas * gas_unit_price; - let lower_cost_bound = simulated_txn.info.gas_used() * gas_unit_price; + let lower_cost_bound = gas_used * gas_unit_price; let message = format!( "Do you want to submit a transaction for a range of [{} - {}] Octas at a gas unit price of {} Octas?", lower_cost_bound, @@ -1392,69 +1386,6 @@ impl TransactionOptions { Ok(response.into_inner()) } - pub async fn simulate_transaction( - &self, - payload: TransactionPayload, - gas_price: Option, - amount_transfer: Option, - ) -> CliTypedResult { - let client = self.rest_client()?; - let (sender_key, sender_address) = self.get_key_and_address()?; - - // Get sequence number for account - let sequence_number = get_sequence_number(&client, sender_address).await?; - - // Estimate gas price if necessary - let gas_price = if let Some(gas_price) = gas_price { - gas_price - } else { - self.estimate_gas_price().await? - }; - // Simulate transaction - // To get my known possible max gas, I need to get my current balance - let account_balance = client - .get_account_balance(sender_address) - .await? - .into_inner() - .coin - .value - .0; - - let max_possible_gas = if gas_price == 0 { - MAX_POSSIBLE_GAS_UNITS - } else if let Some(amount) = amount_transfer { - std::cmp::min( - account_balance - .saturating_sub(amount) - .saturating_div(gas_price), - MAX_POSSIBLE_GAS_UNITS, - ) - } else { - std::cmp::min( - account_balance.saturating_div(gas_price), - MAX_POSSIBLE_GAS_UNITS, - ) - }; - - let transaction_factory = TransactionFactory::new(chain_id(&client).await?) - .with_gas_unit_price(gas_price) - .with_max_gas_amount(max_possible_gas); - - let unsigned_transaction = transaction_factory - .payload(payload) - .sender(sender_address) - .sequence_number(sequence_number) - .build(); - - let signed_transaction = SignedTransaction::new( - unsigned_transaction, - sender_key.public_key(), - Ed25519Signature::try_from([0u8; 64].as_ref()).unwrap(), - ); - let txns = client.simulate(&signed_transaction).await?.into_inner(); - Ok(txns.first().unwrap().clone()) - } - pub async fn estimate_gas_price(&self) -> CliTypedResult { let client = self.rest_client()?; client