Merge pull request #2237 from dusk-network/moonlight-mempool

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<Vec<u8>, 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<Vec<u8>, 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<Vec<u8>, 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

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::{

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};

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<SpendingId> {
+        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<Self> for SpentTransaction {
 
 impl Eq for SpentTransaction {}
 
+pub enum SpendingId {
+    Nullifier([u8; 32]),
+    AccountNonce(bls::PublicKey, u64),
+}
+
+impl SpendingId {
+    pub fn to_bytes(&self) -> Vec<u8> {
+        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::*;

node/src/chain/acceptor.rs

@@ -564,8 +564,8 @@ impl<DB: database::DB, VM: vm::VMExecution, N: Network> Acceptor<N, DB, VM> {
                         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}")

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<bool>;
 
-    /// 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(

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()
     }

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,23 +192,18 @@ 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)? {
                             events.push(TransactionEvent::Removed(m_tx_id));
                         };
                     } else {
                         return Err(
-                            TxAcceptanceError::NullifierExistsInMempool.into(),
+                            TxAcceptanceError::SpendIdExistsInMempool.into()
                         );
                     }
                 }

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