diff --git a/contracts/transfer/src/state.rs b/contracts/transfer/src/state.rs index 5631019231..d9644b2ff0 100644 --- a/contracts/transfer/src/state.rs +++ b/contracts/transfer/src/state.rs @@ -28,7 +28,7 @@ use execution_core::{ withdraw::{ Withdraw, WithdrawReceiver, WithdrawReplayToken, WithdrawSignature, }, - Transaction, TRANSFER_CONTRACT, + Transaction, PANIC_NONCE_NOT_READY, TRANSFER_CONTRACT, }, BlsScalar, ContractError, ContractId, }; @@ -304,44 +304,51 @@ impl TransferState { /// The top level transaction execution function. /// - /// Delegates to [`Self::spend_and_execute_phoenix`] and - /// [`Self::spend_and_execute_moonlight`], depending on if the transaction + /// This will emplace the deposit in the state, if it exists - making it + /// available for any contracts called. + /// + /// [`refund`] **must** be called if this function doesn't panic, otherwise + /// we will have an inconsistent state. + /// + /// It delegate the spending phase to [`Self::spend_phoenix`] and + /// [`Self::spend_moonlight`], depending on if the transaction /// uses the Phoenix or the Moonlight models, respectively. + /// + /// Finally executes the contract call if present. + /// + /// # Panics + /// Any failure while spending will result in a panic. The contract expects + /// the environment to roll back any change in state. + /// + /// [`refund`]: [`TransferState::refund`] pub fn spend_and_execute( &mut self, tx: Transaction, ) -> Result, ContractError> { + transitory::put_transaction(tx); + let tx = transitory::unwrap_tx(); match tx { - Transaction::Phoenix(tx) => self.spend_and_execute_phoenix(tx), - Transaction::Moonlight(tx) => self.spend_and_execute_moonlight(tx), + Transaction::Phoenix(tx) => self.spend_phoenix(tx), + Transaction::Moonlight(tx) => self.spend_moonlight(tx), + } + match tx.call() { + Some(call) => { + rusk_abi::call_raw(call.contract, &call.fn_name, &call.fn_args) + } + None => Ok(Vec::new()), } } /// Spends the inputs and creates the given UTXO within the given phoenix - /// transaction, and executes the contract call if present. It performs - /// all checks necessary to ensure the transaction is valid - hash - /// matches, anchor has been a root of the tree, proof checks out, - /// etc... - /// - /// This will emplace the deposit in the state, if it exists - making it - /// available for any contracts called. - /// - /// [`refund`] **must** be called if this function succeeds, otherwise we - /// will have an inconsistent state. + /// transaction. It performs all checks necessary to ensure the transaction + /// is valid - hash matches, anchor has been a root of the tree, proof + /// checks out, etc... /// /// # Panics /// Any failure in the checks performed in processing the transaction will /// result in a panic. The contract expects the environment to roll back any /// change in state. - /// - /// [`refund`]: [`TransferState::refund`] - fn spend_and_execute_phoenix( - &mut self, - tx: PhoenixTransaction, - ) -> Result, ContractError> { - transitory::put_transaction(tx); - let phoenix_tx = transitory::unwrap_phoenix_tx(); - + fn spend_phoenix(&mut self, phoenix_tx: &PhoenixTransaction) { if phoenix_tx.chain_id() != self.chain_id() { panic!("The tx must target the correct chain"); } @@ -368,40 +375,17 @@ impl TransferState { let block_height = rusk_abi::block_height(); self.tree .extend_notes(block_height, phoenix_tx.outputs().clone()); - - // perform contract call if present - let mut result = Ok(Vec::new()); - if let Some(call) = phoenix_tx.call() { - result = - rusk_abi::call_raw(call.contract, &call.fn_name, &call.fn_args); - } - - result } - /// Spends the amount available to the moonlight transaction, and executes - /// the contract call if present. It performs all checks necessary to ensure - /// the transaction is valid - signature check, available funds, etc... - /// - /// This will emplace the deposit in the state, if it exists - making it - /// available for any contracts called. - /// - /// [`refund`] **must** be called if this function succeeds, otherwise we - /// will have an inconsistent state. + /// Spends the amount available to the moonlight transaction. It performs + /// all checks necessary to ensure the transaction is valid - signature + /// check, available funds, etc... /// /// # Panics /// Any failure in the checks performed in processing the transaction will /// result in a panic. The contract expects the environment to roll back any /// change in state. - /// - /// [`refund`]: [`TransferState::refund`] - fn spend_and_execute_moonlight( - &mut self, - tx: MoonlightTransaction, - ) -> Result, ContractError> { - transitory::put_transaction(tx); - let moonlight_tx = transitory::unwrap_moonlight_tx(); - + fn spend_moonlight(&mut self, moonlight_tx: &MoonlightTransaction) { if moonlight_tx.chain_id() != self.chain_id() { panic!("The tx must target the correct chain"); } @@ -448,8 +432,11 @@ impl TransferState { // transactions. Since this number is so large, we also // skip overflow checks. let incremented_nonce = account.nonce + 1; - if moonlight_tx.nonce() != incremented_nonce { - panic!("Invalid nonce"); + if moonlight_tx.nonce() < incremented_nonce { + panic!("Already used nonce"); + } + if moonlight_tx.nonce() > incremented_nonce { + panic!("{PANIC_NONCE_NOT_READY}",); } account.balance -= total_value; @@ -471,15 +458,6 @@ impl TransferState { let account = self.accounts.entry(key).or_insert(EMPTY_ACCOUNT); account.balance += moonlight_tx.value(); } - - // perform contract call if present - let mut result = Ok(Vec::new()); - if let Some(call) = moonlight_tx.call() { - result = - rusk_abi::call_raw(call.contract, &call.fn_name, &call.fn_args); - } - - result } /// Refund the previously performed transaction, taking into account the diff --git a/execution-core/src/transfer.rs b/execution-core/src/transfer.rs index cf0abd05bc..b93e253f41 100644 --- a/execution-core/src/transfer.rs +++ b/execution-core/src/transfer.rs @@ -31,6 +31,9 @@ pub mod withdraw; /// ID of the genesis transfer contract pub const TRANSFER_CONTRACT: ContractId = crate::reserved(0x1); +/// Panic of "Nonce not ready to be used yet" +pub const PANIC_NONCE_NOT_READY: &str = "Nonce not ready to be used yet"; + use contract_exec::{ContractCall, ContractDeploy, ContractExec}; use moonlight::Transaction as MoonlightTransaction; use phoenix::{ diff --git a/node-data/src/ledger.rs b/node-data/src/ledger.rs index 37c9823170..1423fc1c0a 100644 --- a/node-data/src/ledger.rs +++ b/node-data/src/ledger.rs @@ -11,7 +11,7 @@ mod block; pub use block::{Block, BlockWithLabel, Hash, Label}; mod transaction; -pub use transaction::{SpentTransaction, Transaction}; +pub use transaction::{SpendingId, SpentTransaction, Transaction}; mod faults; pub use faults::{Fault, InvalidFault, Slash, SlashType}; diff --git a/node-data/src/ledger/transaction.rs b/node-data/src/ledger/transaction.rs index 1ea38619dc..701d731699 100644 --- a/node-data/src/ledger/transaction.rs +++ b/node-data/src/ledger/transaction.rs @@ -4,6 +4,8 @@ // // Copyright (c) DUSK NETWORK. All rights reserved. +use dusk_bytes::Serializable; +use execution_core::signatures::bls; use execution_core::transfer::Transaction as ProtocolTransaction; use execution_core::BlsScalar; use serde::Serialize; @@ -63,12 +65,17 @@ impl Transaction { self.inner.gas_price() } - pub fn to_nullifiers(&self) -> Vec<[u8; 32]> { - self.inner - .nullifiers() - .iter() - .map(|n| n.to_bytes()) - .collect() + pub fn to_spend_ids(&self) -> Vec { + match &self.inner { + ProtocolTransaction::Phoenix(p) => p + .nullifiers() + .iter() + .map(|n| SpendingId::Nullifier(n.to_bytes())) + .collect(), + ProtocolTransaction::Moonlight(m) => { + vec![SpendingId::AccountNonce(*m.from_account(), m.nonce())] + } + } } } @@ -90,6 +97,24 @@ impl PartialEq for SpentTransaction { impl Eq for SpentTransaction {} +pub enum SpendingId { + Nullifier([u8; 32]), + AccountNonce(bls::PublicKey, u64), +} + +impl SpendingId { + pub fn to_bytes(&self) -> Vec { + match self { + SpendingId::Nullifier(n) => n.to_vec(), + SpendingId::AccountNonce(account, nonce) => { + let mut id = account.to_bytes().to_vec(); + id.extend_from_slice(&nonce.to_le_bytes()); + id + } + } + } +} + #[cfg(any(feature = "faker", test))] pub mod faker { use super::*; diff --git a/node/src/chain/acceptor.rs b/node/src/chain/acceptor.rs index 0f9244a658..d287f35b59 100644 --- a/node/src/chain/acceptor.rs +++ b/node/src/chain/acceptor.rs @@ -564,8 +564,8 @@ impl Acceptor { events.push(TransactionEvent::Removed(tx_id).into()); } - let nullifiers = tx.to_nullifiers(); - for orphan_tx in t.get_txs_by_nullifiers(&nullifiers) { + let spend_ids = tx.to_spend_ids(); + for orphan_tx in t.get_txs_by_spendable_ids(&spend_ids) { let deleted = Mempool::delete_tx(t, orphan_tx) .map_err(|e| { warn!("Error while deleting orphan_tx: {e}") diff --git a/node/src/database.rs b/node/src/database.rs index 76a96f9df3..05d8838bf5 100644 --- a/node/src/database.rs +++ b/node/src/database.rs @@ -10,7 +10,7 @@ use std::path::Path; pub mod rocksdb; use anyhow::Result; -use node_data::ledger::{self, Fault, Label, SpentTransaction}; +use node_data::ledger::{self, Fault, Label, SpendingId, SpentTransaction}; use serde::{Deserialize, Serialize}; pub struct LightBlock { @@ -131,8 +131,8 @@ pub trait Mempool { /// Deletes a transaction from the mempool. fn delete_tx(&self, tx_id: [u8; 32]) -> Result; - /// Get transactions hash from the mempool, searching by nullifiers - fn get_txs_by_nullifiers(&self, n: &[[u8; 32]]) -> HashSet<[u8; 32]>; + /// Get transactions hash from the mempool, searching by spendable ids + fn get_txs_by_spendable_ids(&self, n: &[SpendingId]) -> HashSet<[u8; 32]>; /// Get an iterator over the mempool transactions sorted by gas price fn get_txs_sorted_by_fee( diff --git a/node/src/database/rocksdb.rs b/node/src/database/rocksdb.rs index 48c1a17749..08c7723444 100644 --- a/node/src/database/rocksdb.rs +++ b/node/src/database/rocksdb.rs @@ -10,7 +10,9 @@ use super::{ use anyhow::Result; use std::cell::RefCell; -use node_data::ledger::{self, Fault, Header, Label, SpentTransaction}; +use node_data::ledger::{ + self, Fault, Header, Label, SpendingId, SpentTransaction, +}; use node_data::Serializable; use crate::database::Mempool; @@ -750,11 +752,13 @@ impl<'db, DB: DBAccess> Mempool for DBTransaction<'db, DB> { Ok(false) } - fn get_txs_by_nullifiers(&self, n: &[[u8; 32]]) -> HashSet<[u8; 32]> { + fn get_txs_by_spendable_ids(&self, n: &[SpendingId]) -> HashSet<[u8; 32]> { n.iter() - .filter_map(|n| match self.snapshot.get_cf(self.nullifiers_cf, n) { - Ok(Some(tx_id)) => tx_id.try_into().ok(), - _ => None, + .filter_map(|n| { + match self.snapshot.get_cf(self.nullifiers_cf, n.to_bytes()) { + Ok(Some(tx_id)) => tx_id.try_into().ok(), + _ => None, + } }) .collect() } diff --git a/node/src/mempool.rs b/node/src/mempool.rs index 7ddf67184a..4ce401c604 100644 --- a/node/src/mempool.rs +++ b/node/src/mempool.rs @@ -31,8 +31,8 @@ enum TxAcceptanceError { AlreadyExistsInMempool, #[error("this transaction exists in the ledger")] AlreadyExistsInLedger, - #[error("this transaction's input(s) exists in the mempool")] - NullifierExistsInMempool, + #[error("this transaction's spendId exists in the mempool")] + SpendIdExistsInMempool, #[error("this transaction is invalid {0}")] VerificationFailed(String), #[error("Maximum count of transactions exceeded {0}")] @@ -192,15 +192,10 @@ impl MempoolSrv { // Try to add the transaction to the mempool db.read().await.update(|db| { - let nullifiers: Vec<_> = tx - .inner - .nullifiers() - .iter() - .map(|nullifier| nullifier.to_bytes()) - .collect(); - - // ensure nullifiers do not exist in the mempool - for m_tx_id in db.get_txs_by_nullifiers(&nullifiers) { + let spend_ids = tx.to_spend_ids(); + + // ensure spend_ids do not exist in the mempool + for m_tx_id in db.get_txs_by_spendable_ids(&spend_ids) { if let Some(m_tx) = db.get_tx(m_tx_id)? { if m_tx.inner.gas_price() < tx.inner.gas_price() { if db.delete_tx(m_tx_id)? { @@ -208,7 +203,7 @@ impl MempoolSrv { }; } else { return Err( - TxAcceptanceError::NullifierExistsInMempool.into(), + TxAcceptanceError::SpendIdExistsInMempool.into() ); } } diff --git a/rusk/src/lib/node/rusk.rs b/rusk/src/lib/node/rusk.rs index df65da6347..0d3495b975 100644 --- a/rusk/src/lib/node/rusk.rs +++ b/rusk/src/lib/node/rusk.rs @@ -9,6 +9,7 @@ use std::sync::{mpsc, Arc, LazyLock}; use std::time::{Duration, Instant}; use std::{fs, io}; +use execution_core::transfer::PANIC_NONCE_NOT_READY; use parking_lot::RwLock; use sha3::{Digest, Sha3_256}; use tokio::task; @@ -180,6 +181,16 @@ impl Rusk { err, }); } + Err(PiecrustError::Panic(val)) + if val == PANIC_NONCE_NOT_READY => + { + // If the transaction panic due to a not yet valid nonce, + // we should not discard the transactions since it can be + // included in future. + + // TODO: Try to process the transaction as soon as the + // nonce is unlocked + } Err(e) => { info!("discard tx {tx_id} due to {e:?}"); // An unspendable transaction should be discarded