From 509b13714e8cf9d3b73c305cb53b9c05662a2454 Mon Sep 17 00:00:00 2001 From: Dmytro Kozhevin Date: Fri, 21 Jul 2023 20:16:08 -0400 Subject: [PATCH] Autobump fixes & max expiration ledger enforcement. --- soroban-env-host/src/auth.rs | 12 +-- soroban-env-host/src/host.rs | 9 +- soroban-env-host/src/host/frame.rs | 3 + .../src/host/ledger_info_helper.rs | 42 ++++++++- .../src/native_contract/token/allowance.rs | 6 +- soroban-env-host/src/storage.rs | 89 +++++++++++++------ 6 files changed, 115 insertions(+), 46 deletions(-) diff --git a/soroban-env-host/src/auth.rs b/soroban-env-host/src/auth.rs index 5cac27961..5cb999f57 100644 --- a/soroban-env-host/src/auth.rs +++ b/soroban-env-host/src/auth.rs @@ -1465,8 +1465,7 @@ impl AccountAuthorizationTracker { } self.need_nonce = false; if let Some((nonce, expiration_ledger)) = &self.nonce { - let (ledger_seq, max_entry_expiration) = - host.with_ledger_info(|li| Ok((li.sequence_number, li.max_entry_expiration)))?; + let ledger_seq = host.with_ledger_info(|li| Ok(li.sequence_number))?; if ledger_seq > *expiration_ledger { return Err(host.err( ScErrorType::Auth, @@ -1478,17 +1477,14 @@ impl AccountAuthorizationTracker { ], )); } - if *expiration_ledger - > ledger_seq - .saturating_sub(1) - .saturating_add(max_entry_expiration) - { + let max_expiration_ledger = host.max_expiration_ledger()?; + if *expiration_ledger > max_expiration_ledger { return Err(host.err( ScErrorType::Auth, ScErrorCode::InvalidInput, "signature expiration is too late", &[ - (ledger_seq + max_entry_expiration).try_into_val(host)?, + max_expiration_ledger.try_into_val(host)?, expiration_ledger.try_into_val(host)?, ], )); diff --git a/soroban-env-host/src/host.rs b/soroban-env-host/src/host.rs index 84f561915..97011f4dc 100644 --- a/soroban-env-host/src/host.rs +++ b/soroban-env-host/src/host.rs @@ -35,7 +35,7 @@ pub(crate) mod declared_size; pub(crate) mod error; pub(crate) mod frame; pub(crate) mod invoker_type; -mod ledger_info_helper; +pub(crate) mod ledger_info_helper; mod mem_helper; pub(crate) mod metered_clone; pub(crate) mod metered_map; @@ -431,7 +431,6 @@ impl Host { /// caller as a tuple wrapped in `Ok(...)`. pub fn try_finish(self) -> Result<(Storage, Budget, Events), HostError> { let events = self.try_borrow_events()?.externalize(&self)?; - self.maybe_autobump_expiration()?; Rc::try_unwrap(self.0) .map(|host_impl| { let storage = host_impl.storage.into_inner(); @@ -888,7 +887,11 @@ impl Host { LedgerKey::ContractData(_) | LedgerKey::ContractCode(_) => { if self.try_borrow_storage_mut()?.has(key, self.budget_ref())? { self.try_borrow_storage_mut()? - .bump(self, key.clone(), autobump_ledgers)?; + .bump_relative_to_entry_expiration( + self, + key.clone(), + autobump_ledgers, + )?; } } _ => (), diff --git a/soroban-env-host/src/host/frame.rs b/soroban-env-host/src/host/frame.rs index df711eded..2ed4bd3ed 100644 --- a/soroban-env-host/src/host/frame.rs +++ b/soroban-env-host/src/host/frame.rs @@ -653,6 +653,9 @@ impl Host { // Notes on metering: covered by the called components. fn invoke_function_raw(&self, hf: HostFunction) -> Result { + // Autobump expiration ledger of the footprint, in case if autobump is + // enabled. + self.maybe_autobump_expiration()?; let hf_type = hf.discriminant(); match hf { HostFunction::InvokeContract(invoke_args) => { diff --git a/soroban-env-host/src/host/ledger_info_helper.rs b/soroban-env-host/src/host/ledger_info_helper.rs index 2d8b1fc95..e19c1479a 100644 --- a/soroban-env-host/src/host/ledger_info_helper.rs +++ b/soroban-env-host/src/host/ledger_info_helper.rs @@ -1,4 +1,4 @@ -use soroban_env_common::xdr::ContractDataDurability; +use soroban_env_common::xdr::{ContractDataDurability, LedgerEntry, LedgerEntryData, LedgerKey}; use crate::{Host, HostError, LedgerInfo}; @@ -16,6 +16,44 @@ impl Host { self.with_ledger_info(|li: &LedgerInfo| Ok(li.min_persistent_entry_expiration))? } }; - Ok(ledger_seq.saturating_add(min_expiration).saturating_sub(1)) + Ok(ledger_seq.saturating_add(min_expiration.saturating_sub(1))) + } + + pub(crate) fn max_expiration_ledger(&self) -> Result { + self.with_ledger_info(|li| { + Ok(li + .sequence_number + // Entry can live for at most max_entry_expiration ledgers from + // now, counting the current one. + .saturating_add(li.max_entry_expiration.saturating_sub(1))) + }) + } +} + +pub fn get_entry_expiration(entry: &LedgerEntry) -> Option { + match &entry.data { + LedgerEntryData::ContractData(d) => Some(d.expiration_ledger_seq), + LedgerEntryData::ContractCode(c) => Some(c.expiration_ledger_seq), + _ => None, + } +} + +pub fn set_entry_expiration(entry: &mut LedgerEntry, new_expiration: u32) { + match &mut entry.data { + LedgerEntryData::ContractData(data) => { + data.expiration_ledger_seq = new_expiration; + } + LedgerEntryData::ContractCode(code) => { + code.expiration_ledger_seq = new_expiration; + } + _ => (), + } +} + +pub fn get_key_durability(key: &LedgerKey) -> Option { + match &key { + LedgerKey::ContractData(d) => Some(d.durability), + LedgerKey::ContractCode(_) => Some(ContractDataDurability::Persistent), + _ => None, } } diff --git a/soroban-env-host/src/native_contract/token/allowance.rs b/soroban-env-host/src/native_contract/token/allowance.rs index a35abdabc..de047e3ea 100644 --- a/soroban-env-host/src/native_contract/token/allowance.rs +++ b/soroban-env-host/src/native_contract/token/allowance.rs @@ -37,11 +37,7 @@ pub fn write_allowance( // validates the expiration and then returns the ledger seq // The expiration can be less than ledger seq if clearing an allowance let ledger_seq = e.with_ledger_info(|li| { - if expiration - > li.sequence_number - .saturating_add(li.max_entry_expiration) - .saturating_sub(1) - { + if expiration > e.max_expiration_ledger()? { Err(err!( e, ContractError::AllowanceError, diff --git a/soroban-env-host/src/storage.rs b/soroban-env-host/src/storage.rs index 0cb967b26..f41aa4f20 100644 --- a/soroban-env-host/src/storage.rs +++ b/soroban-env-host/src/storage.rs @@ -7,12 +7,14 @@ //! - [Env::put_contract_data](crate::Env::put_contract_data) //! - [Env::del_contract_data](crate::Env::del_contract_data) +use std::cmp::min; use std::rc::Rc; -use soroban_env_common::xdr::{LedgerEntryData, ScErrorCode, ScErrorType}; +use soroban_env_common::xdr::{ScErrorCode, ScErrorType}; use soroban_env_common::{Compare, Val}; use crate::budget::Budget; +use crate::host::ledger_info_helper::{get_entry_expiration, set_entry_expiration}; use crate::host::metered_clone::MeteredClone; use crate::xdr::{LedgerEntry, LedgerKey}; use crate::Host; @@ -38,7 +40,7 @@ impl InstanceStorageMap { /// A helper type used by [Footprint] to designate which ways /// a given [LedgerKey] is accessed, or is allowed to be accessed, /// in a given transaction. -#[derive(Clone, Debug, Eq, PartialEq, Ord, PartialOrd)] +#[derive(Clone, Copy, Debug, Eq, PartialEq, Ord, PartialOrd)] pub enum AccessType { /// When in [FootprintMode::Recording], indicates that the [LedgerKey] is only read. /// When in [FootprintMode::Enforcing], indicates that the [LedgerKey] is only _allowed_ to be read. @@ -322,39 +324,70 @@ impl Storage { bump_by_ledgers: u32, ) -> Result<(), HostError> { let _span = tracy_span!("bump key"); - let new_expiration = host.with_ledger_info(|li| { - Ok(li - .sequence_number - .saturating_add(bump_by_ledgers) - .min(li.max_entry_expiration)) + let new_expiration = host + .with_ledger_info(|li| Ok(li.sequence_number.saturating_add(bump_by_ledgers)))? + .min(host.max_expiration_ledger()?); + // Bumping deleted/non-existing/out-of-footprint entries will result in + // an error. + let old_entry = self.get(&key, host.budget_ref())?; + let old_expiration = get_entry_expiration(old_entry.as_ref()).ok_or_else(|| { + host.err( + ScErrorType::Storage, + ScErrorCode::InternalError, + "trying to bump non-expirable entry", + &[], + ) })?; + + if new_expiration > old_expiration { + let mut new_entry = (*old_entry).metered_clone(host.budget_ref())?; + set_entry_expiration(&mut new_entry, new_expiration); + self.map = self + .map + .insert(key, Some(Rc::new(new_entry)), host.budget_ref())?; + } + Ok(()) + } + + /// Bumps `key` to live for at least `bump_by_ledgers` from the current + /// expiration of the entry. + /// + /// This should only be used for internal autobumps. + /// + /// This operation is only defined within a host as it relies on ledger + /// state. + /// + /// This is always considered a 'read-only' operation, even though it might + /// modify the internal storage state. + pub(crate) fn bump_relative_to_entry_expiration( + &mut self, + host: &Host, + key: Rc, + bump_by_ledgers: u32, + ) -> Result<(), HostError> { + let _span = tracy_span!("bump key relative"); // Bumping deleted/non-existing/out-of-footprint entries will result in // an error. let old_entry = self.get(&key, host.budget_ref())?; - let old_expiration = match &old_entry.data { - LedgerEntryData::ContractData(d) => d.expiration_ledger_seq, - LedgerEntryData::ContractCode(c) => c.expiration_ledger_seq, - _ => { - return Err(host.err( - ScErrorType::Storage, - ScErrorCode::InternalError, - "trying to bump unexpected ledger entry type", - &[], - )); - } - }; + let old_expiration = get_entry_expiration(old_entry.as_ref()).ok_or_else(|| { + host.err( + ScErrorType::Storage, + ScErrorCode::InternalError, + "trying to bump non-expirable entry", + &[], + ) + })?; + let new_expiration = min( + old_expiration.saturating_add(bump_by_ledgers), + host.max_expiration_ledger()?, + ); if new_expiration > old_expiration { let mut new_entry = (*old_entry).metered_clone(host.budget_ref())?; - match &mut new_entry.data { - LedgerEntryData::ContractData(data) => { - data.expiration_ledger_seq = new_expiration; - } - LedgerEntryData::ContractCode(code) => { - code.expiration_ledger_seq = new_expiration; - } - _ => (), - } + set_entry_expiration( + &mut new_entry, + old_expiration.saturating_add(bump_by_ledgers), + ); self.map = self .map .insert(key, Some(Rc::new(new_entry)), host.budget_ref())?;