From 51980fd7c0acec532166e1c1db1748607470e5f7 Mon Sep 17 00:00:00 2001 From: hal3e Date: Wed, 23 Aug 2023 10:05:22 +0200 Subject: [PATCH] pr comments --- packages/fuels-accounts/src/lib.rs | 2 +- packages/fuels-accounts/src/provider.rs | 45 +++--- packages/fuels-core/src/types.rs | 2 +- .../src/types/transaction_builders.rs | 5 +- .../src/types/wrappers/transaction.rs | 129 ++++++++++++------ .../types/wrappers/transaction_response.rs | 17 +-- packages/fuels-programs/src/contract.rs | 5 +- packages/fuels-programs/src/script_calls.rs | 2 +- 8 files changed, 117 insertions(+), 90 deletions(-) diff --git a/packages/fuels-accounts/src/lib.rs b/packages/fuels-accounts/src/lib.rs index 9a1c04047..e7fd4db0d 100644 --- a/packages/fuels-accounts/src/lib.rs +++ b/packages/fuels-accounts/src/lib.rs @@ -399,7 +399,7 @@ mod tests { let tx_signature = Signature::from_bytes(bytes); // Sign the transaction manually - let message = Message::from_bytes(*tx.id()); + let message = Message::from_bytes(*tx.id(0.into())); let signature = Signature::sign(&wallet.private_key, &message); // Check if the signatures are the same diff --git a/packages/fuels-accounts/src/provider.rs b/packages/fuels-accounts/src/provider.rs index cd9851648..ab5681327 100644 --- a/packages/fuels-accounts/src/provider.rs +++ b/packages/fuels-accounts/src/provider.rs @@ -440,29 +440,18 @@ impl Provider { &self, tx_id: &TxId, ) -> ProviderResult> { - Ok(self.client.transaction(tx_id).await?.map(|fuel_tr| { - TransactionResponse::from_fuel_response(fuel_tr, self.consensus_parameters) - })) + Ok(self.client.transaction(tx_id).await?.map(Into::into)) } - // - Get transaction(s) pub async fn get_transactions( &self, request: PaginationRequest, ) -> ProviderResult> { let pr = self.client.transactions(request).await?; - let results = pr - .results - .into_iter() - .map(|fuel_tr| { - TransactionResponse::from_fuel_response(fuel_tr, self.consensus_parameters) - }) - .collect(); - Ok(PaginatedResult { cursor: pr.cursor, - results, + results: pr.results.into_iter().map(Into::into).collect(), has_next_page: pr.has_next_page, has_previous_page: pr.has_previous_page, }) @@ -479,17 +468,9 @@ impl Provider { .transactions_by_owner(&owner.into(), request) .await?; - let results = pr - .results - .into_iter() - .map(|fuel_tr| { - TransactionResponse::from_fuel_response(fuel_tr, self.consensus_parameters) - }) - .collect(); - Ok(PaginatedResult { cursor: pr.cursor, - results, + results: pr.results.into_iter().map(Into::into).collect(), has_next_page: pr.has_next_page, has_previous_page: pr.has_previous_page, }) @@ -546,27 +527,35 @@ impl Provider { let tolerance = tolerance.unwrap_or(DEFAULT_GAS_ESTIMATION_TOLERANCE); // Remove limits from an existing Transaction for accurate gas estimation - let new_tx = tx.to_dry_run_tx(0, MAX_GAS_PER_TX); + let dry_run_tx = Self::generate_dry_run_tx(tx.clone()); let gas_used = self - .get_gas_used_with_tolerance(new_tx.clone(), tolerance) + .get_gas_used_with_tolerance(dry_run_tx.clone(), tolerance) .await?; // Update the tx with estimated gas_used and correct gas price to calculate the total_fee - let new_tx = new_tx.to_dry_run_tx(gas_price, gas_used); + let dry_run_tx = dry_run_tx + .with_gas_price(gas_price) + .with_gas_limit(gas_used); - let transaction_fee = new_tx - .fee_checked_from_tx() + let transaction_fee = dry_run_tx + .fee_checked_from_tx(&self.consensus_parameters) .expect("Error calculating TransactionFee"); Ok(TransactionCost { min_gas_price, gas_price, gas_used, - metered_bytes_size: new_tx.metered_bytes_size() as u64, + metered_bytes_size: dry_run_tx.metered_bytes_size() as u64, total_fee: transaction_fee.max_fee(), }) } + // Remove limits from an existing Transaction to get an accurate gas estimation + fn generate_dry_run_tx(tx: T) -> T { + // Simulate the contract call with MAX_GAS_PER_TX to get the complete gas_used + tx.clone().with_gas_limit(MAX_GAS_PER_TX).with_gas_price(0) + } + // Increase estimated gas by the provided tolerance async fn get_gas_used_with_tolerance( &self, diff --git a/packages/fuels-core/src/types.rs b/packages/fuels-core/src/types.rs index a29db7ad1..5cd15ab8d 100644 --- a/packages/fuels-core/src/types.rs +++ b/packages/fuels-core/src/types.rs @@ -2,7 +2,7 @@ use std::fmt; pub use fuel_tx::{Address, AssetId, ContractId, TxPointer, UtxoId}; use fuel_types::bytes::padded_len; -pub use fuel_types::{MessageId, Nonce}; +pub use fuel_types::{ChainId, MessageId, Nonce}; pub use crate::types::{core::*, wrappers::*}; use crate::types::{ diff --git a/packages/fuels-core/src/types/transaction_builders.rs b/packages/fuels-core/src/types/transaction_builders.rs index abb61f990..33fb895c2 100644 --- a/packages/fuels-core/src/types/transaction_builders.rs +++ b/packages/fuels-core/src/types/transaction_builders.rs @@ -88,10 +88,7 @@ macro_rules! impl_tx_trait { tx.estimate_predicates(&consensus_parameters, &GasCosts::default())?; }; - Ok($tx_ty { - tx, - consensus_parameters - }) + Ok($tx_ty { tx }) } fn add_unresolved_signature(&mut self, owner: Bech32Address, secret_key: SecretKey) { diff --git a/packages/fuels-core/src/types/wrappers/transaction.rs b/packages/fuels-core/src/types/wrappers/transaction.rs index 63a77a4e7..31451e5ed 100644 --- a/packages/fuels-core/src/types/wrappers/transaction.rs +++ b/packages/fuels-core/src/types/wrappers/transaction.rs @@ -8,6 +8,7 @@ use fuel_tx::{ Salt as FuelSalt, Script, StorageSlot, Transaction as FuelTransaction, TransactionFee, UniqueIdentifier, Witness, }; +use fuel_types::ChainId; use crate::{ constants::{DEFAULT_GAS_LIMIT, DEFAULT_GAS_PRICE, DEFAULT_MATURITY}, @@ -67,24 +68,33 @@ pub enum TransactionType { } pub trait Transaction: Into + Clone { - fn fee_checked_from_tx(&self) -> Option; + fn fee_checked_from_tx( + &self, + consensus_parameters: &ConsensusParameters, + ) -> Option; fn check_without_signatures( &self, block_height: u32, - parameters: &ConsensusParameters, + consensus_parameters: &ConsensusParameters, ) -> Result<()>; - fn to_dry_run_tx(self, gas_price: u64, gas_limit: u64) -> Self; - - fn id(&self) -> Bytes32; + fn id(&self, chain_id: ChainId) -> Bytes32; fn maturity(&self) -> u32; + fn with_maturity(self, maturity: u32) -> Self; + fn gas_price(&self) -> u64; + fn with_gas_price(self, gas_price: u64) -> Self; + fn gas_limit(&self) -> u64; + fn with_gas_limit(self, gas_limit: u64) -> Self; + + fn with_tx_params(self, tx_params: TxParameters) -> Self; + fn metered_bytes_size(&self) -> usize; fn inputs(&self) -> &Vec; @@ -107,39 +117,35 @@ impl From for FuelTransaction { } impl Transaction for TransactionType { - fn fee_checked_from_tx(&self) -> Option { + fn fee_checked_from_tx( + &self, + consensus_parameters: &ConsensusParameters, + ) -> Option { match self { - TransactionType::Script(tx) => tx.fee_checked_from_tx(), - TransactionType::Create(tx) => tx.fee_checked_from_tx(), + TransactionType::Script(tx) => tx.fee_checked_from_tx(consensus_parameters), + TransactionType::Create(tx) => tx.fee_checked_from_tx(consensus_parameters), } } fn check_without_signatures( &self, block_height: u32, - parameters: &ConsensusParameters, + consensus_parameters: &ConsensusParameters, ) -> Result<()> { - match self { - TransactionType::Script(tx) => tx.check_without_signatures(block_height, parameters), - TransactionType::Create(tx) => tx.check_without_signatures(block_height, parameters), - } - } - - fn to_dry_run_tx(self, gas_price: u64, gas_limit: u64) -> Self { match self { TransactionType::Script(tx) => { - TransactionType::Script(tx.to_dry_run_tx(gas_price, gas_limit)) + tx.check_without_signatures(block_height, consensus_parameters) } TransactionType::Create(tx) => { - TransactionType::Create(tx.to_dry_run_tx(gas_price, gas_limit)) + tx.check_without_signatures(block_height, consensus_parameters) } } } - fn id(&self) -> Bytes32 { + fn id(&self, chain_id: ChainId) -> Bytes32 { match self { - TransactionType::Script(tx) => tx.id(), - TransactionType::Create(tx) => tx.id(), + TransactionType::Script(tx) => tx.id(chain_id), + TransactionType::Create(tx) => tx.id(chain_id), } } @@ -150,6 +156,13 @@ impl Transaction for TransactionType { } } + fn with_maturity(self, maturity: u32) -> Self { + match self { + TransactionType::Script(tx) => TransactionType::Script(tx.with_maturity(maturity)), + TransactionType::Create(tx) => TransactionType::Create(tx.with_maturity(maturity)), + } + } + fn gas_price(&self) -> u64 { match self { TransactionType::Script(tx) => tx.gas_price(), @@ -157,6 +170,13 @@ impl Transaction for TransactionType { } } + fn with_gas_price(self, gas_price: u64) -> Self { + match self { + TransactionType::Script(tx) => TransactionType::Script(tx.with_gas_price(gas_price)), + TransactionType::Create(tx) => TransactionType::Create(tx.with_gas_price(gas_price)), + } + } + fn gas_limit(&self) -> u64 { match self { TransactionType::Script(tx) => tx.gas_limit(), @@ -164,6 +184,20 @@ impl Transaction for TransactionType { } } + fn with_gas_limit(self, gas_limit: u64) -> Self { + match self { + TransactionType::Script(tx) => TransactionType::Script(tx.with_gas_limit(gas_limit)), + TransactionType::Create(tx) => TransactionType::Create(tx.with_gas_limit(gas_limit)), + } + } + + fn with_tx_params(self, tx_params: TxParameters) -> Self { + match self { + TransactionType::Script(tx) => TransactionType::Script(tx.with_tx_params(tx_params)), + TransactionType::Create(tx) => TransactionType::Create(tx.with_tx_params(tx_params)), + } + } + fn metered_bytes_size(&self) -> usize { match self { TransactionType::Script(tx) => tx.metered_bytes_size(), @@ -205,7 +239,6 @@ macro_rules! impl_tx_wrapper { #[derive(Debug, Clone)] pub struct $wrapper { pub(crate) tx: $wrapped, - pub(crate) consensus_parameters: ConsensusParameters, } impl From<$wrapper> for $wrapped { @@ -220,53 +253,67 @@ macro_rules! impl_tx_wrapper { } } - impl $wrapper { - pub fn from_fuel_tx(tx: $wrapped, consensus_parameters: ConsensusParameters) -> Self { - $wrapper { - tx, - consensus_parameters, - } + impl From<$wrapped> for $wrapper { + fn from(tx: $wrapped) -> Self { + $wrapper { tx } } } impl Transaction for $wrapper { - fn fee_checked_from_tx(&self) -> Option { - TransactionFee::checked_from_tx(&self.consensus_parameters, &self.tx) + fn fee_checked_from_tx( + &self, + consensus_parameters: &ConsensusParameters, + ) -> Option { + TransactionFee::checked_from_tx(consensus_parameters, &self.tx) } fn check_without_signatures( &self, block_height: u32, - parameters: &ConsensusParameters, + consensus_parameters: &ConsensusParameters, ) -> Result<()> { Ok(self .tx - .check_without_signatures(block_height.into(), parameters)?) - } - - fn to_dry_run_tx(mut self, gas_price: u64, gas_limit: u64) -> Self { - *self.tx.gas_price_mut() = gas_price; - *self.tx.gas_limit_mut() = gas_limit; - - self + .check_without_signatures(block_height.into(), consensus_parameters)?) } - fn id(&self) -> Bytes32 { - self.tx.id(&self.consensus_parameters.chain_id.into()) + fn id(&self, chain_id: ChainId) -> Bytes32 { + self.tx.id(&chain_id) } fn maturity(&self) -> u32 { (*self.tx.maturity()).into() } + fn with_maturity(mut self, maturity: u32) -> Self { + *self.tx.maturity_mut() = maturity.into(); + self + } + fn gas_price(&self) -> u64 { *self.tx.gas_price() } + fn with_gas_price(mut self, gas_price: u64) -> Self { + *self.tx.gas_price_mut() = gas_price; + self + } + fn gas_limit(&self) -> u64 { *self.tx.gas_limit() } + fn with_gas_limit(mut self, gas_limit: u64) -> Self { + *self.tx.gas_limit_mut() = gas_limit; + self + } + + fn with_tx_params(self, tx_params: TxParameters) -> Self { + self.with_gas_limit(tx_params.gas_limit) + .with_gas_price(tx_params.gas_price) + .with_maturity(tx_params.maturity) + } + fn metered_bytes_size(&self) -> usize { self.tx.metered_bytes_size() } diff --git a/packages/fuels-core/src/types/wrappers/transaction_response.rs b/packages/fuels-core/src/types/wrappers/transaction_response.rs index c6aae9c19..9b3a6e592 100644 --- a/packages/fuels-core/src/types/wrappers/transaction_response.rs +++ b/packages/fuels-core/src/types/wrappers/transaction_response.rs @@ -6,7 +6,7 @@ use chrono::{DateTime, NaiveDateTime, Utc}; use fuel_core_client::client::types::{ TransactionResponse as ClientTransactionResponse, TransactionStatus as ClientTransactionStatus, }; -use fuel_tx::{ConsensusParameters, Transaction}; +use fuel_tx::Transaction; use fuel_types::Bytes32; use crate::types::transaction::{CreateTransaction, ScriptTransaction, TransactionType}; @@ -27,11 +27,8 @@ pub enum TransactionStatus { SqueezedOut(), } -impl TransactionResponse { - pub fn from_fuel_response( - client_response: ClientTransactionResponse, - consensus_parameters: ConsensusParameters, - ) -> Self { +impl From for TransactionResponse { + fn from(client_response: ClientTransactionResponse) -> Self { let block_id = match &client_response.status { ClientTransactionStatus::Submitted { .. } | ClientTransactionStatus::SqueezedOut { .. } => None, @@ -53,12 +50,8 @@ impl TransactionResponse { }; let transaction = match client_response.transaction { - Transaction::Script(tx) => { - TransactionType::Script(ScriptTransaction::from_fuel_tx(tx, consensus_parameters)) - } - Transaction::Create(tx) => { - TransactionType::Create(CreateTransaction::from_fuel_tx(tx, consensus_parameters)) - } + Transaction::Script(tx) => TransactionType::Script(ScriptTransaction::from(tx)), + Transaction::Create(tx) => TransactionType::Create(CreateTransaction::from(tx)), Transaction::Mint(_) => unimplemented!(), }; diff --git a/packages/fuels-programs/src/contract.rs b/packages/fuels-programs/src/contract.rs index b7dbdbde9..5f444aec5 100644 --- a/packages/fuels-programs/src/contract.rs +++ b/packages/fuels-programs/src/contract.rs @@ -506,7 +506,7 @@ where let tx = self.build_tx().await?; let provider = self.account.try_provider()?; - self.cached_tx_id = Some(tx.id()); + self.cached_tx_id = Some(tx.id(provider.chain_id())); let receipts = if simulate { provider.checked_dry_run(tx).await? @@ -747,7 +747,8 @@ impl MultiContractCallHandler { ) -> Result> { let tx = self.build_tx().await?; let provider = self.account.try_provider()?; - self.cached_tx_id = Some(tx.id()); + + self.cached_tx_id = Some(tx.id(provider.chain_id())); let receipts = if simulate { provider.checked_dry_run(tx).await? diff --git a/packages/fuels-programs/src/script_calls.rs b/packages/fuels-programs/src/script_calls.rs index bbf607bad..aedf7c010 100644 --- a/packages/fuels-programs/src/script_calls.rs +++ b/packages/fuels-programs/src/script_calls.rs @@ -223,7 +223,7 @@ where /// The other field of [`FuelCallResponse`], `receipts`, contains the receipts of the transaction. async fn call_or_simulate(&mut self, simulate: bool) -> Result> { let tx = self.build_tx().await?; - self.cached_tx_id = Some(tx.id()); + self.cached_tx_id = Some(tx.id(self.provider.chain_id())); let receipts = if simulate { self.provider.checked_dry_run(tx).await?