Skip to content

Commit 71fa975

Browse files
authored
Add test utilities to compensate hiding the storage interface. (#1556)
### What Add test utilities to compensate hiding the storage interface. Also do a small passing-by cleanup for the storage function names. ### Why There generally should not be a need to expose the Storage implementation to the env users; the interface is pretty complex and we want to be able to change it (as we did e.g. for the error handling). The test utilities make sure the Storage remains mostly encapsulated in host, while SDK gets the necessary tools to read/write the arbitrary ledger entries (which is mostly useful for SAC setup - maybe eventually this should be moved to host as well, as this is protocol level logic). ### Known limitations N/A
1 parent a532072 commit 71fa975

File tree

13 files changed

+142
-157
lines changed

13 files changed

+142
-157
lines changed

soroban-env-host/src/auth.rs

Lines changed: 14 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -2226,22 +2226,19 @@ impl Host {
22262226
let live_until_ledger = live_until_ledger
22272227
.max(self.get_min_live_until_ledger(xdr::ContractDataDurability::Temporary)?);
22282228
self.with_mut_storage(|storage| {
2229-
if storage
2230-
.has_with_host(&nonce_key, self, None)
2231-
.map_err(|err| {
2232-
if err.error.is_type(ScErrorType::Storage)
2233-
&& err.error.is_code(ScErrorCode::ExceededLimit)
2234-
{
2235-
return self.err(
2236-
ScErrorType::Storage,
2237-
ScErrorCode::ExceededLimit,
2238-
"trying to access nonce outside of footprint for address",
2239-
&[address.to_val()],
2240-
);
2241-
}
2242-
err
2243-
})?
2244-
{
2229+
if storage.has(&nonce_key, self, None).map_err(|err| {
2230+
if err.error.is_type(ScErrorType::Storage)
2231+
&& err.error.is_code(ScErrorCode::ExceededLimit)
2232+
{
2233+
return self.err(
2234+
ScErrorType::Storage,
2235+
ScErrorCode::ExceededLimit,
2236+
"trying to access nonce outside of footprint for address",
2237+
&[address.to_val()],
2238+
);
2239+
}
2240+
err
2241+
})? {
22452242
return Err(self.err(
22462243
ScErrorType::Auth,
22472244
ScErrorCode::ExistingValue,
@@ -2261,7 +2258,7 @@ impl Host {
22612258
data,
22622259
ext: LedgerEntryExt::V0,
22632260
};
2264-
storage.put_with_host(
2261+
storage.put(
22652262
&nonce_key,
22662263
&Rc::metered_new(entry, self)?,
22672264
Some(live_until_ledger),

soroban-env-host/src/builtin_contracts/stellar_asset_contract/balance.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -453,7 +453,7 @@ fn transfer_account_balance(
453453
if new_balance >= min_balance && new_balance <= max_balance {
454454
ae.balance = new_balance;
455455
le = Host::modify_ledger_entry_data(host, &le, LedgerEntryData::Account(ae))?;
456-
storage.put_with_host(&lk, &le, None, &host, None)
456+
storage.put(&lk, &le, None, &host, None)
457457
} else {
458458
Err(err!(
459459
host,
@@ -535,7 +535,7 @@ fn transfer_trustline_balance(
535535
if new_balance >= min_balance && new_balance <= max_balance {
536536
tl.balance = new_balance;
537537
le = Host::modify_ledger_entry_data(host, &le, LedgerEntryData::Trustline(tl))?;
538-
storage.put_with_host(&lk, &le, None, &host, None)
538+
storage.put(&lk, &le, None, &host, None)
539539
} else {
540540
Err(err!(
541541
host,
@@ -805,7 +805,7 @@ fn set_trustline_authorization(
805805
tl.flags |= TrustLineFlags::AuthorizedToMaintainLiabilitiesFlag as u32;
806806
}
807807
le = Host::modify_ledger_entry_data(host, &le, LedgerEntryData::Trustline(tl))?;
808-
storage.put_with_host(&lk, &le, None, &host, None)
808+
storage.put(&lk, &le, None, &host, None)
809809
})
810810
}
811811

soroban-env-host/src/e2e_invoke.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -652,7 +652,7 @@ pub fn invoke_host_function_in_recording_mode(
652652
base_prng_seed: [u8; 32],
653653
diagnostic_events: &mut Vec<DiagnosticEvent>,
654654
) -> Result<InvokeHostFunctionRecordingModeResult, HostError> {
655-
let storage = Storage::with_recording_footprint(ledger_snapshot.clone())?;
655+
let storage = Storage::with_recording_footprint(ledger_snapshot.clone());
656656
let host = Host::with_storage_and_budget(storage, budget.clone());
657657
let is_recording_auth = matches!(auth_mode, RecordingInvocationAuthMode::Recording(_));
658658
let ledger_seq = ledger_info.sequence_number;

soroban-env-host/src/host.rs

Lines changed: 35 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -2095,8 +2095,7 @@ impl VmCallerEnv for Host {
20952095
let res = match t {
20962096
StorageType::Temporary | StorageType::Persistent => {
20972097
let key = self.storage_key_from_val(k, t.try_into()?)?;
2098-
self.try_borrow_storage_mut()?
2099-
.has_with_host(&key, self, Some(k))?
2098+
self.try_borrow_storage_mut()?.has(&key, self, Some(k))?
21002099
}
21012100
StorageType::Instance => {
21022101
self.with_instance_storage(|s| Ok(s.map.get(&k, self)?.is_some()))?
@@ -2116,9 +2115,7 @@ impl VmCallerEnv for Host {
21162115
match t {
21172116
StorageType::Temporary | StorageType::Persistent => {
21182117
let key = self.storage_key_from_val(k, t.try_into()?)?;
2119-
let entry = self
2120-
.try_borrow_storage_mut()?
2121-
.get_with_host(&key, self, Some(k))?;
2118+
let entry = self.try_borrow_storage_mut()?.get(&key, self, Some(k))?;
21222119
match &entry.data {
21232120
LedgerEntryData::ContractData(e) => Ok(self.to_valid_host_val(&e.val)?),
21242121
_ => Err(self.err(
@@ -2155,8 +2152,7 @@ impl VmCallerEnv for Host {
21552152
match t {
21562153
StorageType::Temporary | StorageType::Persistent => {
21572154
let key = self.storage_key_from_val(k, t.try_into()?)?;
2158-
self.try_borrow_storage_mut()?
2159-
.del_with_host(&key, self, Some(k))?;
2155+
self.try_borrow_storage_mut()?.del(&key, self, Some(k))?;
21602156
}
21612157
StorageType::Instance => {
21622158
self.with_mut_instance_storage(|s| {
@@ -3367,44 +3363,40 @@ impl VmCallerEnv for Host {
33673363
address: AddressObject,
33683364
) -> Result<Val, Self::Error> {
33693365
let sc_address = self.scaddress_from_address(address)?;
3370-
let maybe_executable =
3371-
match sc_address {
3372-
ScAddress::Account(account_id) => {
3373-
let key = self.to_account_key(account_id)?;
3374-
if self
3375-
.try_borrow_storage_mut()?
3376-
.has_with_host(&key, &self, None)?
3377-
{
3378-
Some(AddressExecutable::Account)
3379-
} else {
3380-
None
3381-
}
3382-
}
3383-
ScAddress::Contract(id) => {
3384-
let storage_key = self.contract_instance_ledger_key(&id)?;
3385-
let maybe_instance_entry = self
3386-
.try_borrow_storage_mut()?
3387-
.try_get_full_with_host(&storage_key, self, None)?;
3388-
if let Some((instance_entry, _ttl)) = maybe_instance_entry {
3389-
let instance =
3390-
self.extract_contract_instance_from_ledger_entry(&instance_entry)?;
3391-
Some(AddressExecutable::from_contract_executable_xdr(
3392-
&self,
3393-
&instance.executable,
3394-
)?)
3395-
} else {
3396-
None
3397-
}
3366+
let maybe_executable = match sc_address {
3367+
ScAddress::Account(account_id) => {
3368+
let key = self.to_account_key(account_id)?;
3369+
if self.try_borrow_storage_mut()?.has(&key, &self, None)? {
3370+
Some(AddressExecutable::Account)
3371+
} else {
3372+
None
33983373
}
3399-
_ => {
3400-
return Err(self.err(
3401-
ScErrorType::Object,
3402-
ScErrorCode::InternalError,
3403-
"Unexpected Address variant in get_address_executable",
3404-
&[address.into()],
3405-
));
3374+
}
3375+
ScAddress::Contract(id) => {
3376+
let storage_key = self.contract_instance_ledger_key(&id)?;
3377+
let maybe_instance_entry =
3378+
self.try_borrow_storage_mut()?
3379+
.try_get_full(&storage_key, self, None)?;
3380+
if let Some((instance_entry, _ttl)) = maybe_instance_entry {
3381+
let instance =
3382+
self.extract_contract_instance_from_ledger_entry(&instance_entry)?;
3383+
Some(AddressExecutable::from_contract_executable_xdr(
3384+
&self,
3385+
&instance.executable,
3386+
)?)
3387+
} else {
3388+
None
34063389
}
3407-
};
3390+
}
3391+
_ => {
3392+
return Err(self.err(
3393+
ScErrorType::Object,
3394+
ScErrorCode::InternalError,
3395+
"Unexpected Address variant in get_address_executable",
3396+
&[address.into()],
3397+
));
3398+
}
3399+
};
34083400
if let Some(exec) = maybe_executable {
34093401
Val::try_from_val(self, &exec)
34103402
} else {

soroban-env-host/src/host/data_helper.rs

Lines changed: 40 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ use crate::{
1919
};
2020

2121
impl Host {
22-
pub fn with_mut_storage<F, U>(&self, f: F) -> Result<U, HostError>
22+
pub(crate) fn with_mut_storage<F, U>(&self, f: F) -> Result<U, HostError>
2323
where
2424
F: FnOnce(&mut Storage) -> Result<U, HostError>,
2525
{
@@ -115,9 +115,7 @@ impl Host {
115115
&self,
116116
key: &Rc<LedgerKey>,
117117
) -> Result<ScContractInstance, HostError> {
118-
let entry = self
119-
.try_borrow_storage_mut()?
120-
.get_with_host(key, self, None)?;
118+
let entry = self.try_borrow_storage_mut()?.get(key, self, None)?;
121119
self.extract_contract_instance_from_ledger_entry(&entry)
122120
}
123121

@@ -137,11 +135,7 @@ impl Host {
137135
wasm_hash: &Hash,
138136
) -> Result<(BytesM, VersionedContractCodeCostInputs), HostError> {
139137
let key = self.contract_code_ledger_key(wasm_hash)?;
140-
match &self
141-
.try_borrow_storage_mut()?
142-
.get_with_host(&key, self, None)?
143-
.data
144-
{
138+
match &self.try_borrow_storage_mut()?.get(&key, self, None)?.data {
145139
LedgerEntryData::ContractCode(e) => {
146140
let code = e.code.metered_clone(self)?;
147141
let costs = match &e.ext {
@@ -167,8 +161,7 @@ impl Host {
167161

168162
pub(crate) fn wasm_exists(&self, wasm_hash: &Hash) -> Result<bool, HostError> {
169163
let key = self.contract_code_ledger_key(wasm_hash)?;
170-
self.try_borrow_storage_mut()?
171-
.has_with_host(&key, self, None)
164+
self.try_borrow_storage_mut()?.has(&key, self, None)
172165
}
173166

174167
// Stores the contract instance specified with its parts (executable and
@@ -184,10 +177,7 @@ impl Host {
184177
contract_id: ContractId,
185178
key: &Rc<LedgerKey>,
186179
) -> Result<(), HostError> {
187-
if self
188-
.try_borrow_storage_mut()?
189-
.has_with_host(key, self, None)?
190-
{
180+
if self.try_borrow_storage_mut()?.has(key, self, None)? {
191181
let (current, live_until_ledger) = self
192182
.try_borrow_storage_mut()?
193183
.get_with_live_until_ledger(key, self, None)?;
@@ -218,7 +208,7 @@ impl Host {
218208
));
219209
}
220210

221-
self.try_borrow_storage_mut()?.put_with_host(
211+
self.try_borrow_storage_mut()?.put(
222212
&key,
223213
&Rc::metered_new(current, self)?,
224214
live_until_ledger,
@@ -243,7 +233,7 @@ impl Host {
243233
durability: ContractDataDurability::Persistent,
244234
ext: ExtensionPoint::V0,
245235
};
246-
self.try_borrow_storage_mut()?.put_with_host(
236+
self.try_borrow_storage_mut()?.put(
247237
key,
248238
&Host::new_contract_data(self, data)?,
249239
Some(self.get_min_live_until_ledger(ContractDataDurability::Persistent)?),
@@ -305,17 +295,15 @@ impl Host {
305295
// notes on metering: `get` from storage is covered. Rest are free.
306296
pub(crate) fn load_account(&self, account_id: AccountId) -> Result<AccountEntry, HostError> {
307297
let acc = self.to_account_key(account_id)?;
308-
self.with_mut_storage(
309-
|storage| match &storage.get_with_host(&acc, self, None)?.data {
310-
LedgerEntryData::Account(ae) => ae.metered_clone(self),
311-
e => Err(err!(
312-
self,
313-
(ScErrorType::Storage, ScErrorCode::InternalError),
314-
"ledger entry is not account",
315-
e.name()
316-
)),
317-
},
318-
)
298+
self.with_mut_storage(|storage| match &storage.get(&acc, self, None)?.data {
299+
LedgerEntryData::Account(ae) => ae.metered_clone(self),
300+
e => Err(err!(
301+
self,
302+
(ScErrorType::Storage, ScErrorCode::InternalError),
303+
"ledger entry is not account",
304+
e.name()
305+
)),
306+
})
319307
}
320308

321309
pub(crate) fn to_account_key(&self, account_id: AccountId) -> Result<Rc<LedgerKey>, HostError> {
@@ -485,10 +473,7 @@ impl Host {
485473
// operation might only modify the internal `ScVal` value. Thus we
486474
// need to only overwrite the value in case if there is already an
487475
// existing ledger entry value for the key in the storage.
488-
if self
489-
.try_borrow_storage_mut()?
490-
.has_with_host(&key, self, Some(k))?
491-
{
476+
if self.try_borrow_storage_mut()?.has(&key, self, Some(k))? {
492477
let (current, live_until_ledger) = self
493478
.try_borrow_storage_mut()?
494479
.get_with_live_until_ledger(&key, self, Some(k))?;
@@ -506,7 +491,7 @@ impl Host {
506491
));
507492
}
508493
}
509-
self.try_borrow_storage_mut()?.put_with_host(
494+
self.try_borrow_storage_mut()?.put(
510495
&key,
511496
&Rc::metered_new(current, self)?,
512497
live_until_ledger,
@@ -521,7 +506,7 @@ impl Host {
521506
durability,
522507
ext: ExtensionPoint::V0,
523508
};
524-
self.try_borrow_storage_mut()?.put_with_host(
509+
self.try_borrow_storage_mut()?.put(
525510
&key,
526511
&Host::new_contract_data(self, data)?,
527512
Some(self.get_min_live_until_ledger(durability)?),
@@ -537,20 +522,36 @@ impl Host {
537522
#[cfg(any(test, feature = "testutils"))]
538523
use crate::crypto;
539524
#[cfg(any(test, feature = "testutils"))]
540-
use crate::storage::{AccessType, Footprint};
525+
use crate::storage::{AccessType, EntryWithLiveUntil, Footprint};
541526

542527
#[cfg(any(test, feature = "testutils"))]
543528
impl Host {
544-
// Writes an arbitrary ledger entry to storage.
529+
/// Writes an arbitrary ledger entry to storage.
545530
pub fn add_ledger_entry(
546531
&self,
547532
key: &Rc<LedgerKey>,
548533
val: &Rc<soroban_env_common::xdr::LedgerEntry>,
549534
live_until_ledger: Option<u32>,
550535
) -> Result<(), HostError> {
551-
self.with_mut_storage(|storage| {
552-
storage.put_with_host(key, val, live_until_ledger, self, None)
553-
})
536+
self.with_mut_storage(|storage| storage.put(key, val, live_until_ledger, self, None))
537+
}
538+
539+
/// Reads an arbitrary ledger entry from the storage.
540+
///
541+
/// Returns `None` if the entry does not exist.
542+
pub fn get_ledger_entry(
543+
&self,
544+
key: &Rc<LedgerKey>,
545+
) -> Result<Option<EntryWithLiveUntil>, HostError> {
546+
self.with_mut_storage(|storage| storage.try_get_full(key, self, None))
547+
}
548+
549+
/// Returns all the ledger entries stored in the storage as key-value pairs.
550+
#[allow(clippy::type_complexity)]
551+
pub fn get_stored_entries(
552+
&self,
553+
) -> Result<Vec<(Rc<LedgerKey>, Option<EntryWithLiveUntil>)>, HostError> {
554+
self.with_mut_storage(|storage| Ok(storage.map.map.clone()))
554555
}
555556

556557
// Performs the necessary setup to access the provided ledger key/entry in

soroban-env-host/src/host/frame.rs

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -697,10 +697,7 @@ impl Host {
697697
// checking the cache: this seems like overkill but it
698698
// provides some future-proofing, see below.
699699
let wasm_key = self.contract_code_ledger_key(wasm_hash)?;
700-
if self
701-
.try_borrow_storage_mut()?
702-
.has_with_host(&wasm_key, self, None)?
703-
{
700+
if self.try_borrow_storage_mut()?.has(&wasm_key, self, None)? {
704701
if let Some(parsed_module) = cache.get_module(wasm_hash)? {
705702
return Vm::from_parsed_module_and_wasmi_linker(
706703
self,

soroban-env-host/src/host/invocation_metering.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -323,7 +323,7 @@ impl InvocationMeter {
323323
let mut entry_size = 0;
324324
let mut new_entry_size_for_rent = 0;
325325
let mut entry_live_until_ledger = None;
326-
let maybe_entry = curr_storage.try_get_full_with_host(key, host, None)?;
326+
let maybe_entry = curr_storage.try_get_full(key, host, None)?;
327327
if let Some((entry, entry_live_until)) = maybe_entry {
328328
let mut buf = Vec::<u8>::new();
329329
metered_write_xdr(host.budget_ref(), entry.as_ref(), &mut buf)?;

0 commit comments

Comments
 (0)