diff --git a/rusk/src/lib/lib.rs b/rusk/src/lib/lib.rs index 5452819bb4..994235ff4a 100644 --- a/rusk/src/lib/lib.rs +++ b/rusk/src/lib/lib.rs @@ -11,7 +11,7 @@ use crate::error::Error; use std::path::{Path, PathBuf}; use std::pin::Pin; use std::sync::{mpsc, Arc, LazyLock}; -use std::{cmp, fs, io}; +use std::{fs, io}; pub mod chain; pub mod error; @@ -39,8 +39,9 @@ use rkyv::validation::validators::DefaultValidator; use rkyv::{Archive, Deserialize, Infallible, Serialize}; use rusk_abi::dusk::{dusk, Dusk}; use rusk_abi::{ - CallReceipt, CallTree, ContractId, Error as PiecrustError, Event, Session, - StandardBufSerializer, STAKE_CONTRACT, TRANSFER_CONTRACT, VM, + CallReceipt, ContractError, ContractId, Error as PiecrustError, Event, + RawResult, Session, StandardBufSerializer, STAKE_CONTRACT, + TRANSFER_CONTRACT, VM, }; use rusk_profile::to_rusk_state_id_path; use sha3::{Digest, Sha3_256}; @@ -131,73 +132,51 @@ impl Rusk { for unspent_tx in txs { let tx = unspent_tx.inner.clone(); - let receipt = execute(&mut session, &tx, block_gas_left); - - let (call_result, gas_spent) = match receipt.data { - // We're currently ignoring the successful result of a call - Ok(_) => { - for event in receipt.events { - update_hasher(&mut event_hasher, event); + match execute(&mut session, &tx) { + Ok(receipt) => { + let gas_spent = receipt.points_spent; + + // If the transaction went over the block gas limit we + // re-execute all spent transactions. We don't discard the + // transaction, since it is technically valid. + if gas_spent > block_gas_left { + session = rusk_abi::new_session( + &inner.vm, + current_commit, + block_height, + )?; + + for spent_tx in &spent_txs { + // We know these transactions were correctly + // executed before, so we don't bother checking. + let _ = + execute(&mut session, &spent_tx.inner.inner); + } + + continue; } - (None, receipt.points_spent) - } - // This transaction was given its own gas limit, so it should be - // included with the error. - Err(TxError::TxLimit { err, gas_spent }) => { + for event in receipt.events { update_hasher(&mut event_hasher, event); } - (Some(err), gas_spent) - } - // An unspendable transaction should be discarded - Err(TxError::Unspendable(_)) => { - discarded_txs.push(unspent_tx); - continue; - } - // A transaction that errors due to hitting the block gas limit - // is not included, but not dicarded either. - Err(TxError::BlockLimit(_)) => continue, - // A transaction that hit the block gas limit after execution - // leaves the transaction in a spent state, therefore - // re-execution is required. It also is not discarded. - Err(TxError::BlockLimitAfter(_)) => { - session = rusk_abi::new_session( - &inner.vm, - current_commit, - block_height, - )?; - let mut block_gas_left = block_gas_limit; - - for spent_tx in &spent_txs { - let receipt = execute( - &mut session, - &spent_tx.inner.inner, - block_gas_left, - ); - - // We know these transactions were either spent or - // erroring with `TxLimit` so we don't need to check. - block_gas_left -= receipt.points_spent; - } + block_gas_left -= gas_spent; + dusk_spent += gas_spent * tx.fee.gas_price; + spent_txs.push(SpentTransaction { + inner: unspent_tx.clone(), + gas_spent, + block_height, + // We're currently ignoring the result of successful + // calls + err: receipt.data.err().map(|e| format!("{e:?}")), + }); + } + Err(_) => { + // An unspendable transaction should be discarded + discarded_txs.push(unspent_tx); continue; } - }; - - block_gas_left -= gas_spent; - dusk_spent += gas_spent * tx.fee.gas_price; - - spent_txs.push(SpentTransaction { - inner: unspent_tx.clone(), - gas_spent, - block_height, - err: call_result.map(|e| format!("{e:?}")), - }); - - // Stop executing if there is no gas left for a normal transfer - if block_gas_left < GAS_PER_INPUT { - break; } } @@ -647,29 +626,12 @@ fn accept( for unspent_tx in txs { let tx = &unspent_tx.inner; - let receipt = execute(session, tx, block_gas_left); + let receipt = execute(session, tx)?; - let (call_result, gas_spent) = match receipt.data { - Ok(_) => { - for event in receipt.events { - update_hasher(&mut event_hasher, event); - } - (None, receipt.points_spent) - } - Err(TxError::TxLimit { err, gas_spent }) => { - for event in receipt.events { - update_hasher(&mut event_hasher, event); - } - (Some(err), gas_spent) - } - Err( - TxError::Unspendable(err) - | TxError::BlockLimit(err) - | TxError::BlockLimitAfter(err), - ) => { - return Err(err.into()); - } - }; + for event in receipt.events { + update_hasher(&mut event_hasher, event); + } + let gas_spent = receipt.points_spent; dusk_spent += gas_spent * tx.fee.gas_price; block_gas_left = block_gas_left @@ -680,8 +642,8 @@ fn accept( inner: unspent_tx.clone(), gas_spent, block_height, - - err: call_result.map(|e| format!("{e:?}")), + // We're currently ignoring the result of successful calls + err: receipt.data.err().map(|e| format!("{e:?}")), }); } @@ -699,121 +661,33 @@ fn accept( )) } -/// Executes a transaction, returning the result of the call and the gas spent. -/// The following steps are executed: -/// -/// 0. Pre-flight checks, i.e. the transaction gas limit must be at least the -/// same as what is minimally charged for a transaction of its type, and the -/// transaction must fit in the remaining block gas. +/// Executes a transaction, returning the receipt of the call and the gas spent. +/// The following steps are performed: /// -/// 1. Call the "spend" function on the transfer contract with unlimited gas. If -/// this fails, the transaction should be considered invalid, or unspendable, -/// and an error is returned. +/// 1. Call the "spend_and_execute" function on the transfer contract with +/// unlimited gas. If this fails, an error is returned. If an error is +/// returned the transaction should be considered unspendable/invalid, but no +/// re-execution of previous transactions is required. /// -/// 2. If the transaction includes a contract call, execute it with the gas -/// limit given in the transaction, or with the block gas remaining, -/// whichever is smallest. If this fails with an out of gas, two possible -/// things happen: -/// * We use the transaction gas limit and will treat this as any other -/// transaction. -/// * We used the block gas remaining and can't be sure of what to do. In -/// this case we return early with an [TxError::BlockLimitAfter], since -/// we are in a bad state, and can't be sure of what to do. -/// For any other transaction error we proceed to step 3. -/// -/// 3. Call the "refund" function on the transfer contract with unlimited gas. -/// The amount charged depends on if the transaction has executed a call or -/// not. If it has there are two cases: -/// * The call succeeded and the transaction will be charged for gas used -/// plus the amount charged by a transaction of its type. -/// * The call errored and the transaction will be charged the full gas -/// given. -/// If the transaction has not executed a call only be the amount charged for -/// a transaction of its type. +/// 2. Call the "refund" function on the transfer contract with unlimited gas. +/// The amount charged depends on the gas spent by the transaction, and the +/// optional contract call in step 1. fn execute( session: &mut Session, tx: &PhoenixTransaction, - block_gas_left: u64, -) -> CallReceipt>, TxError>> { - let gas_for_spend = spent_gas_per_input(tx.nullifiers.len()); - - let mut receipt = CallReceipt { - points_spent: 0, - points_limit: tx.fee.gas_limit, - events: vec![], - data: Ok(None), - call_tree: CallTree::default(), - }; - - // If the gas given is less than the amount the node charges per input, then - // the transaction is unspendable. - if tx.fee.gas_limit < gas_for_spend { - receipt.data = Err(TxError::Unspendable(PiecrustError::OutOfPoints)); - return receipt; - } - - // If the gas to spend is more than the amount remaining in a block, then - // the transaction can't be spent at this spot in the block. - if block_gas_left < gas_for_spend { - receipt.data = Err(TxError::BlockLimit(PiecrustError::OutOfPoints)); - return receipt; - } - - // Spend the transaction. If this errors the transaction is unspendable. - match session.call::<_, ()>(TRANSFER_CONTRACT, "spend", tx, u64::MAX) { - Ok(spend_receipt) => { - receipt.points_spent += gas_for_spend; - receipt.events.extend(spend_receipt.events); - } - Err(err) => { - receipt.data = Err(TxError::Unspendable(err)); - return receipt; - } - }; - - let block_gas_left = block_gas_left - gas_for_spend; - let tx_gas_left = tx.fee.gas_limit - gas_for_spend; - - if let Some((contract_id_bytes, fn_name, fn_data)) = &tx.call { - let contract_id = ContractId::from_bytes(*contract_id_bytes); - let gas_left = cmp::min(block_gas_left, tx_gas_left); +) -> Result>, PiecrustError> { + // Spend the inputs and execute the call. If this errors the transaction is + // unspendable. + let mut receipt = session.call::<_, Result>( + TRANSFER_CONTRACT, + "spend_and_execute", + tx, + tx.fee.gas_limit, + )?; - match session.call_raw(contract_id, fn_name, fn_data.clone(), gas_left) - { - Ok(r) => { - receipt.points_spent += r.points_spent; - receipt.events.extend(r.events); - receipt.data = Ok(Some(r.data)); - } - Err(err) => match err { - err @ PiecrustError::OutOfPoints => { - // If the transaction failed with an OUT_OF_GAS, and - // we're using the block gas remaining as a limit, then - // we can't be sure that the transaction would fail if - // it was given the full gas it gave as a limit. - if gas_left == block_gas_left { - receipt.data = Err(TxError::BlockLimitAfter(err)); - return receipt; - } else { - // Otherwise we should spend the maximum available gas - receipt.points_spent = tx.fee.gas_limit; - receipt.data = Err(TxError::TxLimit { - gas_spent: receipt.points_spent, - err, - }); - } - } - err => { - // On any other error we should spent the maximum - // available gas - receipt.points_spent = tx.fee.gas_limit; - receipt.data = Err(TxError::TxLimit { - gas_spent: receipt.points_spent, - err, - }) - } - }, - } + // Ensure all gas is consumed if there's an error in the contract call + if receipt.data.is_err() { + receipt.points_spent = receipt.points_limit; } // Refund the appropriate amount to the transaction. This call is guaranteed @@ -830,7 +704,7 @@ fn execute( receipt.events.extend(refund_receipt.events); - receipt + Ok(receipt) } fn update_hasher(hasher: &mut Sha3_256, event: Event) { @@ -839,29 +713,6 @@ fn update_hasher(hasher: &mut Sha3_256, event: Event) { hasher.update(event.data); } -/// The gas charged per input of a transaction. -pub const GAS_PER_INPUT: u64 = 1_000_000; - -/// The gas charged given the number of inputs of a transaction. -const fn spent_gas_per_input(n_inputs: usize) -> u64 { - n_inputs as u64 * GAS_PER_INPUT -} - -/// The error returned when executing a transaction. -enum TxError { - /// A transaction can't be spent. - Unspendable(PiecrustError), - /// The error was produced by executing the transaction's call with its own - /// given gas limit. - TxLimit { gas_spent: u64, err: PiecrustError }, - /// The error was produced by executing the transaction's call with the - /// remaining block gas limit. - BlockLimit(PiecrustError), - /// The error was produced by executing the transaction's call with the - /// remaining block gas limit, and after execution of a call. - BlockLimitAfter(PiecrustError), -} - fn reward_and_update_root( session: &mut Session, block_height: u64, diff --git a/rusk/tests/services/gas_behavior.rs b/rusk/tests/services/gas_behavior.rs index 61bf5189bd..7492593014 100644 --- a/rusk/tests/services/gas_behavior.rs +++ b/rusk/tests/services/gas_behavior.rs @@ -11,7 +11,7 @@ use std::sync::{Arc, RwLock}; use dusk_wallet_core::{self as wallet}; use rand::prelude::*; use rand::rngs::StdRng; -use rusk::{Result, Rusk, GAS_PER_INPUT}; +use rusk::{Result, Rusk}; use rusk_abi::TRANSFER_CONTRACT; use tempfile::tempdir; use tracing::info; @@ -21,11 +21,11 @@ use crate::common::state::{generator_procedure, new_state}; use crate::common::wallet::{TestProverClient, TestStateClient, TestStore}; const BLOCK_HEIGHT: u64 = 1; -const BLOCK_GAS_LIMIT: u64 = 1_000_000_000; +const BLOCK_GAS_LIMIT: u64 = 1_000_000_000_000; const INITIAL_BALANCE: u64 = 10_000_000_000; -const GAS_LIMIT_0: u64 = 2_000_000; -const GAS_LIMIT_1: u64 = 100_000_000; +const GAS_LIMIT_0: u64 = 7_000_000; +const GAS_LIMIT_1: u64 = 200_000_000; // Creates the Rusk initial state for the tests below fn initial_state>(dir: P) -> Result { @@ -73,14 +73,14 @@ fn make_transactions( let mut rng = StdRng::seed_from_u64(0xdead); - // The first transaction will be a `wallet.execute` to a contract that is - // not deployed. This will produce an error in call execution and should - // consume all the gas provided. + // The first transaction will be a `wallet.execute` to the transfer + // contract, querying for the root of the tree. This will be given too + // little gas to execute correctly and error, consuming all gas provided. let tx_0 = wallet .execute( &mut rng, - [0x42; 32].into(), - String::from("nonsense"), + TRANSFER_CONTRACT.to_bytes().into(), + String::from("root"), (), SENDER_INDEX_0, &refund_0, @@ -89,9 +89,9 @@ fn make_transactions( ) .expect("Making the transaction should succeed"); - // The second transaction will also be a `wallet.execute`, but this time to - // the transfer contract, querying for the root of the tree. This will be - // tested for gas cost. + // The second transaction will also be a `wallet.execute` to the transfer + // contract, querying for the root of the tree. This will be tested for + // gas cost. let tx_1 = wallet .execute( &mut rng, @@ -105,7 +105,7 @@ fn make_transactions( ) .expect("Making the transaction should succeed"); - generator_procedure( + let spent_transactions = generator_procedure( rusk, &[tx_0, tx_1], BLOCK_HEIGHT, @@ -114,31 +114,24 @@ fn make_transactions( ) .expect("generator procedure should succeed"); - let final_balance_0 = wallet - .get_balance(SENDER_INDEX_0) - .expect("Getting final balance should succeed") - .value; + let mut spent_transactions = spent_transactions.into_iter(); + let tx_0 = spent_transactions + .next() + .expect("There should be two spent transactions"); + let tx_1 = spent_transactions + .next() + .expect("There should be two spent transactions"); - let final_balance_1 = wallet - .get_balance(SENDER_INDEX_1) - .expect("Getting final balance should succeed") - .value; + assert!(tx_0.err.is_some(), "The first transaction should error"); + assert!(tx_1.err.is_none(), "The second transaction should succeed"); - // The first transaction should consume all gas given, while the second one - // should consume a little more due to the root query. assert_eq!( - final_balance_0, - initial_balance_0 - GAS_LIMIT_0, - "Transaction should consume all the gas" - ); - - assert!( - final_balance_1 < initial_balance_1 - GAS_PER_INPUT, - "Transaction should consume more gas than just for one input" + tx_0.gas_spent, GAS_LIMIT_0, + "Erroring transaction should consume all gas" ); assert!( - final_balance_1 > GAS_LIMIT_1, - "Transaction should consume less gas than all given" + tx_1.gas_spent < GAS_LIMIT_1, + "Successful transaction should consume less than provided" ); } diff --git a/rusk/tests/services/multi_transfer.rs b/rusk/tests/services/multi_transfer.rs index 9d454a1cfa..c0eb781929 100644 --- a/rusk/tests/services/multi_transfer.rs +++ b/rusk/tests/services/multi_transfer.rs @@ -25,8 +25,8 @@ use crate::common::wallet::{TestProverClient, TestStateClient, TestStore}; const BLOCK_HEIGHT: u64 = 1; // This is purposefully chosen to be low to trigger the discarding of a // perfectly good transaction. -const BLOCK_GAS_LIMIT: u64 = 2_500_000; -const GAS_LIMIT: u64 = 1_000_000; +const BLOCK_GAS_LIMIT: u64 = 24_000_000; +const GAS_LIMIT: u64 = 12_000_000; // Lowest value for a transfer const INITIAL_BALANCE: u64 = 10_000_000_000; // Creates the Rusk initial state for the tests below diff --git a/rusk/tests/services/unspendable.rs b/rusk/tests/services/unspendable.rs index 4ae5ec11c3..51f9d27f41 100644 --- a/rusk/tests/services/unspendable.rs +++ b/rusk/tests/services/unspendable.rs @@ -24,7 +24,7 @@ const BLOCK_HEIGHT: u64 = 1; const BLOCK_GAS_LIMIT: u64 = 1_000_000_000_000; const INITIAL_BALANCE: u64 = 10_000_000_000; -const GAS_LIMIT_0: u64 = 1_000_000; // Enough to spend, but OOG during ICC +const GAS_LIMIT_0: u64 = 20_000_000; // Enough to spend, but OOG during ICC const GAS_LIMIT_1: u64 = 1_000; // Not enough to spend const GAS_LIMIT_2: u64 = 200_000_000; // All ok