runtime-sdk: Better handle nonces in the future

Author: kostko

runtime-sdk/src/context.rs

@@ -36,11 +36,13 @@ pub enum Mode {
     ExecuteTx,
     CheckTx,
     SimulateTx,
+    PreScheduleTx,
 }
 
 const MODE_CHECK_TX: &str = "check_tx";
 const MODE_EXECUTE_TX: &str = "execute_tx";
 const MODE_SIMULATE_TX: &str = "simulate_tx";
+const MODE_PRE_SCHEDULE_TX: &str = "pre_schedule_tx";
 
 impl fmt::Display for Mode {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
@@ -54,6 +56,7 @@ impl From<&Mode> for &'static str {
             Mode::CheckTx => MODE_CHECK_TX,
             Mode::ExecuteTx => MODE_EXECUTE_TX,
             Mode::SimulateTx => MODE_SIMULATE_TX,
+            Mode::PreScheduleTx => MODE_PRE_SCHEDULE_TX,
         }
     }
 }
@@ -89,7 +92,12 @@ pub trait Context {
 
     /// Whether the transaction is just being checked for validity.
     fn is_check_only(&self) -> bool {
-        self.mode() == Mode::CheckTx
+        self.mode() == Mode::CheckTx || self.mode() == Mode::PreScheduleTx
+    }
+
+    /// Whether the transaction is just being checked for validity before being scheduled.
+    fn is_pre_schedule(&self) -> bool {
+        self.mode() == Mode::PreScheduleTx
     }
 
     /// Whether the transaction is just being simulated.
@@ -122,7 +130,7 @@ pub trait Context {
                     .unwrap_or_default()
             }
             // When just checking a transaction, we always want to be fast and skip contracts.
-            Mode::CheckTx => false,
+            Mode::CheckTx | Mode::PreScheduleTx => false,
         }
     }
 

runtime-sdk/src/dispatcher.rs

@@ -115,6 +115,8 @@ pub struct DispatchOptions<'a> {
     pub tx_index: usize,
     /// Optionally only allow methods for which the provided authorizer closure returns true.
     pub method_authorizer: Option<&'a dyn Fn(&str) -> bool>,
+    /// Optionally skip authentication.
+    pub skip_authentication: bool,
 }
 
 /// The runtime dispatcher.
@@ -244,8 +246,10 @@ impl<R: Runtime> Dispatcher<R> {
         opts: &DispatchOptions<'_>,
     ) -> Result<DispatchResult, Error> {
         // Run pre-processing hooks.
-        if let Err(err) = R::Modules::authenticate_tx(ctx, &tx) {
-            return Ok(err.into_call_result().into());
+        if !opts.skip_authentication {
+            if let Err(err) = R::Modules::authenticate_tx(ctx, &tx) {
+                return Ok(err.into_call_result().into());
+            }
         }
         let tx_auth_info = tx.auth_info.clone();
         let is_read_only = tx.call.read_only;
@@ -656,21 +660,50 @@ impl<R: Runtime + Send + Sync> transaction::dispatcher::Dispatcher for Dispatche
                         }
 
                         // Determine the current transaction index.
-                        let index = new_batch.len();
+                        let tx_index = new_batch.len();
 
                         // First run the transaction in check tx mode in a separate subcontext. If
-                        // that fails, skip and reject transaction.
-                        let check_result = ctx.with_child(Mode::CheckTx, |mut ctx| {
-                            Self::dispatch_tx(&mut ctx, tx_size, tx.clone(), index)
-                        })?;
-                        if let module::CallResult::Failed { .. } = check_result.result {
-                            // Skip and reject transaction.
-                            tx_reject_hashes.push(Hash::digest_bytes(&raw_tx));
+                        // that fails, skip and (sometimes) reject transaction.
+                        let skip =
+                            ctx.with_child(Mode::PreScheduleTx, |mut ctx| -> Result<_, Error> {
+                                // First authenticate the transaction to get any nonce related errors.
+                                match R::Modules::authenticate_tx(&mut ctx, &tx) {
+                                    Err(modules::core::Error::FutureNonce) => {
+                                        // Only skip transaction as it may become valid in the future.
+                                        return Ok(true);
+                                    }
+                                    Err(_) => {
+                                        // Skip and reject the transaction.
+                                    }
+                                    Ok(_) => {
+                                        // Run additional checks on the transaction.
+                                        let check_result = Self::dispatch_tx_opts(
+                                            &mut ctx,
+                                            tx.clone(),
+                                            &DispatchOptions {
+                                                tx_size,
+                                                tx_index,
+                                                skip_authentication: true, // Already done.
+                                                ..Default::default()
+                                            },
+                                        )?;
+                                        if check_result.result.is_success() {
+                                            // Checks successful, execute transaction as usual.
+                                            return Ok(false);
+                                        }
+                                    }
+                                }
+
+                                // Skip and reject the transaction.
+                                tx_reject_hashes.push(Hash::digest_bytes(&raw_tx));
+                                Ok(true)
+                            })?;
+                        if skip {
                             continue;
                         }
 
                         new_batch.push(raw_tx);
-                        results.push(Self::execute_tx(ctx, tx_size, tx, index)?);
+                        results.push(Self::execute_tx(ctx, tx_size, tx, tx_index)?);
                     }
 
                     // If there's more room in the block and we got the maximum number of

runtime-sdk/src/modules/accounts/mod.rs

@@ -1,5 +1,6 @@
 //! Accounts module.
 use std::{
+    cmp::Ordering,
     collections::{BTreeMap, BTreeSet},
     convert::TryInto,
 };
@@ -16,7 +17,9 @@ use crate::{
     modules,
     modules::core::{Error as CoreError, API as _},
     runtime::Runtime,
-    sdk_derive, storage,
+    sdk_derive,
+    sender::SenderMeta,
+    storage,
     storage::Prefix,
     types::{
         address::{Address, SignatureAddressSpec},
@@ -32,6 +35,10 @@ pub mod types;
 /// Unique module name.
 const MODULE_NAME: &str = "accounts";
 
+/// Maximum delta that the transaction nonce can be in the future from the current nonce to still
+/// be accepted during transaction checks.
+const MAX_CHECK_NONCE_FUTURE_DELTA: u64 = 10;
+
 /// Errors emitted by the accounts module.
 #[derive(Error, Debug, oasis_runtime_sdk_macros::Error)]
 pub enum Error {
@@ -230,7 +237,7 @@ pub trait API {
     fn check_signer_nonces<C: Context>(
         ctx: &mut C,
         tx_auth_info: &AuthInfo,
-    ) -> Result<Option<Address>, modules::core::Error>;
+    ) -> Result<Address, modules::core::Error>;
 
     /// Update transaction signer account nonces.
     fn update_signer_nonces<C: Context>(
@@ -623,31 +630,73 @@ impl API for Module {
     fn check_signer_nonces<C: Context>(
         ctx: &mut C,
         auth_info: &AuthInfo,
-    ) -> Result<Option<Address>, modules::core::Error> {
+    ) -> Result<Address, modules::core::Error> {
+        let is_pre_schedule = ctx.is_pre_schedule();
+        let is_check_only = ctx.is_check_only();
+
         // TODO: Optimize the check/update pair so that the accounts are
         // fetched only once.
         let params = Self::params(ctx.runtime_state());
         // Fetch information about each signer.
         let mut store = storage::PrefixStore::new(ctx.runtime_state(), &MODULE_NAME);
         let accounts =
             storage::TypedStore::new(storage::PrefixStore::new(&mut store, &state::ACCOUNTS));
-        let mut payer = None;
+        let mut sender = None;
         for si in auth_info.signer_info.iter() {
             let address = si.address_spec.address();
             let account: types::Account = accounts.get(&address).unwrap_or_default();
-            if account.nonce != si.nonce {
-                // Reject unles nonce checking is disabled.
-                if !params.debug_disable_nonce_check {
+
+            // First signer pays for the fees and is considered the sender.
+            if sender.is_none() {
+                sender = Some(SenderMeta {
+                    address,
+                    tx_nonce: si.nonce,
+                    state_nonce: account.nonce,
+                });
+            }
+
+            // When nonce checking is disabled, skip the rest of the checks.
+            if params.debug_disable_nonce_check {
+                continue;
+            }
+
+            // Check signer nonce against the corresponding account nonce.
+            match si.nonce.cmp(&account.nonce) {
+                Ordering::Less => {
+                    // In the past and will never become valid, reject.
                     return Err(modules::core::Error::InvalidNonce);
                 }
-            }
+                Ordering::Equal => {} // Ok.
+                Ordering::Greater => {
+                    // If too much in the future, reject.
+                    if si.nonce - account.nonce > MAX_CHECK_NONCE_FUTURE_DELTA {
+                        return Err(modules::core::Error::InvalidNonce);
+                    }
 
-            // First signer pays for the fees.
-            if payer.is_none() {
-                payer = Some(address);
+                    // If in the future and this is before scheduling, reject with separate error
+                    // that will make the scheduler skip the transaction.
+                    if is_pre_schedule {
+                        return Err(modules::core::Error::FutureNonce);
+                    }
+
+                    // If in the future and this is during execution, reject.
+                    if !is_check_only {
+                        return Err(modules::core::Error::InvalidNonce);
+                    }
+
+                    // If in the future and this is during checks, accept.
+                }
             }
         }
-        Ok(payer)
+
+        // Configure the sender.
+        let sender = sender.expect("at least one signer is always present");
+        let sender_address = sender.address;
+        if is_check_only {
+            <C::Runtime as Runtime>::Core::set_sender_meta(ctx, sender);
+        }
+
+        Ok(sender_address)
     }
 
     fn update_signer_nonces<C: Context>(
@@ -869,8 +918,6 @@ impl module::TransactionHandler for Module {
 
         // Charge the specified amount of fees.
         if !tx.auth_info.fee.amount.amount().is_zero() {
-            let payer = payer.expect("at least one signer is always present");
-
             if ctx.is_check_only() {
                 // Do not update balances during transaction checks. In case of checks, only do it
                 // after all the other checks have already passed as otherwise retrying the
@@ -918,7 +965,6 @@ impl module::TransactionHandler for Module {
 
         // Update payer balance.
         let payer = Self::check_signer_nonces(ctx, tx_auth_info).unwrap(); // Already checked.
-        let payer = payer.unwrap(); // Already checked.
         let amount = &tx_auth_info.fee.amount;
         Self::sub_amount(ctx.runtime_state(), payer, amount).unwrap(); // Already checked.
 

runtime-sdk/src/modules/core/mod.rs

@@ -130,6 +130,10 @@ pub enum Error {
     #[sdk_error(code = 25)]
     ReadOnlyTransaction,
 
+    #[error("future nonce")]
+    #[sdk_error(code = 26)]
+    FutureNonce,
+
     #[error("{0}")]
     #[sdk_error(transparent)]
     TxSimulationFailed(#[from] TxSimulationFailure),