Skip to content

Commit

Permalink
runtime-sdk: Better handle nonces in the future
Browse files Browse the repository at this point in the history
  • Loading branch information
kostko committed Oct 4, 2022
1 parent ab8445c commit ac4dcbb
Show file tree
Hide file tree
Showing 4 changed files with 119 additions and 28 deletions.
12 changes: 10 additions & 2 deletions runtime-sdk/src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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,
}
}
}
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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,
}
}

Expand Down
55 changes: 44 additions & 11 deletions runtime-sdk/src/dispatcher.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down
76 changes: 61 additions & 15 deletions runtime-sdk/src/modules/accounts/mod.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
//! Accounts module.
use std::{
cmp::Ordering,
collections::{BTreeMap, BTreeSet},
convert::TryInto,
};
Expand All @@ -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},
Expand All @@ -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 {
Expand Down Expand Up @@ -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>(
Expand Down Expand Up @@ -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>(
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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.

Expand Down
4 changes: 4 additions & 0 deletions runtime-sdk/src/modules/core/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down

0 comments on commit ac4dcbb

Please sign in to comment.