Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
90 changes: 89 additions & 1 deletion soroban-env-common/env.json
Original file line number Diff line number Diff line change
Expand Up @@ -1708,6 +1708,94 @@
"return": "Void",
"docs": "Extend the contract instance and/or corresponding code entry TTL to be up to `extend_to` ledgers, where TTL is defined as `entry_live_until_ledger_seq - current_ledger_seq`. `extension_scope` defines whether contract instance, code, or both will be extended. The TTL extension only actually happens if it is at least `min_extension`, otherwise this function is a no-op. The amount of extension ledgers will not exceed `max_extension` ledgers. If attempting to extend an entry past the maximum allowed value (defined as the current ledger + `max_entry_ttl` - 1), its new `live_until_ledger_seq` will be clamped to the max.",
"min_supported_protocol": 26
},
{
"export": "h",
"name": "try_get_contract_data",
"args": [
{
"name": "k",
"type": "Val"
},
{
"name": "t",
"type": "StorageType"
},
{
"name": "has_pos",
"type": "U32Val"
}
],
"return": "Val",
"docs": "Return the value stored for a contract data key and write whether the value is present into linear memory at has_pos.",
"min_supported_protocol": 28
},
{
"export": "i",
"name": "has_external_contract_data",
"args": [
{
"name": "contract",
"type": "AddressObject"
},
{
"name": "k",
"type": "Val"
},
{
"name": "t",
"type": "StorageType"
}
],
"return": "Bool",
"docs": "Return whether the provided contract stores a value at the provided contract data key. This read-only operation records the target contract data key in the transaction footprint.",
"min_supported_protocol": 28
},
{
"export": "j",
"name": "get_external_contract_data",
"args": [
{
"name": "contract",
"type": "AddressObject"
},
{
"name": "k",
"type": "Val"
},
{
"name": "t",
"type": "StorageType"
}
],
"return": "Val",
"docs": "Return the value stored by the provided contract at the provided contract data key. This read-only operation records the target contract data key in the transaction footprint.",
"min_supported_protocol": 28
},
{
"export": "k",
"name": "try_get_external_contract_data",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This interface is rather awkward and inconsistent with the existing storage functions. I think we should either:

  • get rid of this function - has and get have us covered on the SDK-side for the try_ function implementation
  • if there is a good evidence that dedicated try_ function saves us a lot of instructions vs has + get, then we should return an 'option' value from the function instead of writing the output to linear memory. But I really think just dropping this is easier and not significantly more expensive.

"args": [
{
"name": "contract",
"type": "AddressObject"
},
{
"name": "k",
"type": "Val"
},
{
"name": "t",
"type": "StorageType"
},
{
"name": "has_pos",
"type": "U32Val"
}
],
"return": "Val",
"docs": "Return the value stored by the provided contract at the provided contract data key and write whether the value is present into linear memory at has_pos. This preserves stored Void values by reporting presence separately.",
"min_supported_protocol": 28
}
]
},
Expand Down Expand Up @@ -2954,4 +3042,4 @@
]
}
]
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ use super::storage_types::AllowanceValue;
pub(crate) fn read_allowance(e: &Host, from: Address, spender: Address) -> Result<i128, HostError> {
let key = DataKey::Allowance(AllowanceDataKey { from, spender });
if let Some(allowance) =
e.try_get_contract_data(key.try_into_val(e)?, StorageType::Temporary)?
e.try_get_contract_data_value(key.try_into_val(e)?, StorageType::Temporary)?
{
let val: AllowanceValue = allowance.try_into_val(e)?;
if val.live_until_ledger < e.get_ledger_sequence()?.into() {
Expand Down Expand Up @@ -66,7 +66,7 @@ pub(crate) fn write_allowance(
// If an allowance didn't exist, then the previous live_until ledger will be None.
let allowance_with_live_until_option: Option<(AllowanceValue, Option<u32>)> =
if let Some(allowance) =
e.try_get_contract_data(key.try_into_val(e)?, StorageType::Temporary)?
e.try_get_contract_data_value(key.try_into_val(e)?, StorageType::Temporary)?
{
let mut updated_allowance: AllowanceValue = allowance.try_into_val(e)?;
updated_allowance.amount = amount;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ pub(crate) fn read_balance(e: &Host, addr: Address) -> Result<i128, HostError> {
ScAddress::Contract(_) => {
let key = DataKey::Balance(addr);
if let Some(raw_balance) =
e.try_get_contract_data(key.try_into_val(e)?, StorageType::Persistent)?
e.try_get_contract_data_value(key.try_into_val(e)?, StorageType::Persistent)?
{
e.extend_contract_data_ttl(
key.try_into_val(e)?,
Expand Down Expand Up @@ -120,7 +120,7 @@ pub(crate) fn receive_balance(e: &Host, addr: Address, amount: i128) -> Result<(
ScAddress::Contract(id) => {
let key = DataKey::Balance(addr.metered_clone(e)?);
let mut balance = if let Some(raw_balance) =
e.try_get_contract_data(key.try_into_val(e)?, StorageType::Persistent)?
e.try_get_contract_data_value(key.try_into_val(e)?, StorageType::Persistent)?
{
raw_balance.try_into_val(e)?
} else {
Expand Down Expand Up @@ -174,7 +174,7 @@ pub(crate) fn spend_balance_no_authorization_check(
// this can be used to clawback when deauthorized.
let key = DataKey::Balance(addr.metered_clone(e)?);
if let Some(raw_balance) =
e.try_get_contract_data(key.try_into_val(e)?, StorageType::Persistent)?
e.try_get_contract_data_value(key.try_into_val(e)?, StorageType::Persistent)?
{
let mut balance: BalanceValue = raw_balance.try_into_val(e)?;
if balance.amount < amount {
Expand Down Expand Up @@ -236,7 +236,7 @@ pub(crate) fn is_authorized(e: &Host, addr: Address) -> Result<bool, HostError>
ScAddress::Contract(_) => {
let key = DataKey::Balance(addr);
if let Some(raw_balance) =
e.try_get_contract_data(key.try_into_val(e)?, StorageType::Persistent)?
e.try_get_contract_data_value(key.try_into_val(e)?, StorageType::Persistent)?
{
let balance: BalanceValue = raw_balance.try_into_val(e)?;
Ok(balance.authorized)
Expand Down Expand Up @@ -272,7 +272,7 @@ pub(crate) fn write_authorization(
ScAddress::Contract(id) => {
let key = DataKey::Balance(addr.metered_clone(e)?);
if let Some(raw_balance) =
e.try_get_contract_data(key.try_into_val(e)?, StorageType::Persistent)?
e.try_get_contract_data_value(key.try_into_val(e)?, StorageType::Persistent)?
{
let mut balance: BalanceValue = raw_balance.try_into_val(e)?;
balance.authorized = authorize;
Expand Down Expand Up @@ -340,7 +340,7 @@ pub(crate) fn check_clawbackable(e: &Host, addr: Address) -> Result<(), HostErro
ScAddress::Contract(_) => {
let key = DataKey::Balance(addr);
if let Some(raw_balance) =
e.try_get_contract_data(key.try_into_val(e)?, StorageType::Persistent)?
e.try_get_contract_data_value(key.try_into_val(e)?, StorageType::Persistent)?
{
let balance: BalanceValue = raw_balance.try_into_val(e)?;
if !balance.clawback {
Expand Down
78 changes: 72 additions & 6 deletions soroban-env-host/src/builtin_contracts/storage_utils.rs
Original file line number Diff line number Diff line change
@@ -1,15 +1,81 @@
use crate::{Env, Host, HostError, StorageType, Val};
use crate::{
storage::InstanceStorageMap,
xdr::{LedgerEntryData, ScErrorCode, ScErrorType},
AddressObject, Host, HostError, StorageType, Val,
};

impl Host {
pub(crate) fn try_get_contract_data(
fn contract_data_entry_value(&self, entry: &LedgerEntryData) -> Result<Val, HostError> {
match entry {
LedgerEntryData::ContractData(e) => self.to_valid_host_val(&e.val),
_ => Err(self.err(
ScErrorType::Storage,
ScErrorCode::InternalError,
"expected contract data ledger entry",
&[],
)),
}
}

pub(crate) fn try_get_contract_data_value(
&self,
k: Val,
t: StorageType,
) -> Result<Option<Val>, HostError> {
if self.has_contract_data(k, t)?.into() {
Ok(Some(self.get_contract_data(k, t)?))
} else {
Ok(None)
match t {
StorageType::Temporary | StorageType::Persistent => {
let key = self.storage_key_from_val(k, t.try_into()?)?;
let entry = self
.try_borrow_storage_mut()?
.try_get(&key, self, Some(k))?;
entry
.map(|entry| self.contract_data_entry_value(&entry.data))
.transpose()
}
StorageType::Instance => {
self.with_instance_storage(|s| s.map.get(&k, self).map(|v| v.copied()))
}
}
}

pub(crate) fn try_get_external_contract_data_value(
&self,
contract: AddressObject,
k: Val,
t: StorageType,
) -> Result<Option<Val>, HostError> {
match t {
StorageType::Temporary | StorageType::Persistent => {
let key = self.storage_key_from_address_val(contract, k, t.try_into()?)?;
let entry = self
.try_borrow_storage_mut()?
.try_get(&key, self, Some(k))?;
entry
.map(|entry| self.contract_data_entry_value(&entry.data))
.transpose()
}
StorageType::Instance => self.try_get_external_instance_data_value(contract, k),
}
}
Comment on lines +20 to +59

fn try_get_external_instance_data_value(
&self,
contract: AddressObject,
k: Val,
) -> Result<Option<Val>, HostError> {
let contract_id = self.contract_id_from_address(contract)?;
let instance_key = self.contract_instance_ledger_key(&contract_id)?;
let entry = self
.try_borrow_storage_mut()?
.try_get(&instance_key, self, None)?;
Comment on lines +68 to +70

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Read active instance storage for external instance reads

When has/get_external_contract_data is used with StorageType::Instance for a contract that is currently on the call stack (including the current contract's own address), this reads the instance ledger entry instead of the frame's in-memory Context.storage. Instance storage writes are only persisted on frame exit, so a contract that does put_instance(k, v) and then reads get_external(self, k) in the same invocation will see the old ledger value or None rather than the just-written value. The external instance path should consult the active frame's instance storage when the target contract is active.

Useful? React with 👍 / 👎.

let Some(entry) = entry else {
return Ok(None);
};
let instance = self.extract_contract_instance_from_ledger_entry(&entry)?;
let instance_storage = InstanceStorageMap::from_instance_xdr(&instance, self)?;
instance_storage
.map
.get(&k, self)
.map(|value| value.copied())
}
}
72 changes: 72 additions & 0 deletions soroban-env-host/src/host.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2275,6 +2275,78 @@ impl VmCallerEnv for Host {
}
}

// Notes on metering: covered by components
fn try_get_contract_data(
&self,
vmcaller: &mut VmCaller<Host>,
k: Val,
t: StorageType,
has_pos: U32Val,
) -> Result<Val, HostError> {
let MemFnArgs { vm, pos, .. } = self.get_mem_fn_args(has_pos, U32Val::from(1))?;
let value = self.try_get_contract_data_value(k, t)?;
let has = Val::from_bool(value.is_some()).to_val();
self.metered_vm_write_vals_to_linear_memory(vmcaller, &vm, pos, &[has], |v| {
Ok(u64::to_le_bytes(
self.absolute_to_relative(*v)?.get_payload(),
))
})?;
Ok(value.unwrap_or_else(|| Val::VOID.to_val()))
}

// Notes on metering: covered by components
fn has_external_contract_data(
&self,
_vmcaller: &mut VmCaller<Host>,
contract: AddressObject,
k: Val,
t: StorageType,
) -> Result<Bool, HostError> {
Ok(Val::from_bool(
self.try_get_external_contract_data_value(contract, k, t)?
.is_some(),
))
}

// Notes on metering: covered by components
fn get_external_contract_data(
&self,
_vmcaller: &mut VmCaller<Host>,
contract: AddressObject,
k: Val,
t: StorageType,
) -> Result<Val, HostError> {
self.try_get_external_contract_data_value(contract, k, t)?
.ok_or_else(|| {
self.err(
ScErrorType::Storage,
ScErrorCode::MissingValue,
"key is missing from external contract storage",
&[contract.to_val(), k],
)
})
}

// Notes on metering: covered by components
fn try_get_external_contract_data(
&self,
vmcaller: &mut VmCaller<Host>,
contract: AddressObject,
k: Val,
t: StorageType,
has_pos: U32Val,
) -> Result<Val, HostError> {
let MemFnArgs { vm, pos, .. } = self.get_mem_fn_args(has_pos, U32Val::from(1))?;
let value = self.try_get_external_contract_data_value(contract, k, t)?;
let has = Val::from_bool(value.is_some()).to_val();
self.metered_vm_write_vals_to_linear_memory(vmcaller, &vm, pos, &[has], |v| {
Ok(u64::to_le_bytes(
self.absolute_to_relative(*v)?.get_payload(),
))
})?;
Ok(value.unwrap_or_else(|| Val::VOID.to_val()))
}

// Notes on metering: covered by components
fn del_contract_data(
&self,
Expand Down
15 changes: 15 additions & 0 deletions soroban-env-host/src/host/conversion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,21 @@ impl Host {
self.storage_key_from_scval(key_scval, durability)
}

/// Converts a [`Val`] to an [`ScVal`] and combines it with the provided
/// contract address to produce a [`Key`] for accessing another contract's
/// ledger storage.
// Notes on metering: covered by components.
pub(crate) fn storage_key_from_address_val(
&self,
contract: AddressObject,
k: Val,
durability: ContractDataDurability,
) -> Result<Rc<LedgerKey>, HostError> {
let contract_id = self.contract_id_from_address(contract)?;
let key_scval = self.from_host_val_for_storage(k)?;
self.storage_key_for_address(ScAddress::Contract(contract_id), key_scval, durability)
}

/// Converts a binary search result into a u64. `res` is `Some(index)` if
/// the value was found at `index`, or `Err(index)` if the value was not
/// found and would've needed to be inserted at `index`. Returns a
Expand Down
15 changes: 7 additions & 8 deletions soroban-env-host/src/host/data_helper.rs
Original file line number Diff line number Diff line change
Expand Up @@ -514,14 +514,13 @@ impl Host {
) -> Result<(), HostError> {
let durability: ContractDataDurability = t.try_into()?;
let key = self.storage_key_from_val(k, durability)?;
// Currently the storage stores the whole ledger entries, while this
// operation might only modify the internal `ScVal` value. Thus we
// need to only overwrite the value in case if there is already an
// existing ledger entry value for the key in the storage.
if self.try_borrow_storage_mut()?.has(&key, self, Some(k))? {
let (current, live_until_ledger) = self
.try_borrow_storage_mut()?
.get_with_live_until_ledger(&key, self, Some(k))?;
// Storage stores whole ledger entries, while this only changes the
// internal ScVal when the entry already exists.
let existing = {
self.try_borrow_storage_mut()?
.try_get_full(&key, self, Some(k))?
};
if let Some((current, live_until_ledger)) = existing {
let mut current = (*current).metered_clone(self)?;
match current.data {
LedgerEntryData::ContractData(ref mut entry) => {
Expand Down
Loading