Add external read-only storage SDK API#1908
Open
mihaieremia wants to merge 2 commits into
Open
Conversation
mihaieremia
force-pushed
the
codex/external-storage-sdk
branch
from
45591db to
a728ad8
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds a feature-gated (“next”) API for read-only external contract storage reads, plus internal plumbing to make Storage::get use a single “try-get” path.
Changes:
- Introduces
nextCargo feature (propagated to macros/guest/host crates). - Adds
has_external/get_externalAPIs for Persistent/Temporary/Instance storage (feature-gated). - Adds wasm imports + internal helpers for
try_get_*and a compile-time typecheck test.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| soroban-sdk/src/storage.rs | Adds internal try_get_* helpers and public external read APIs under the next feature. |
| soroban-sdk/tests/external_storage_next.rs | Adds a compile-only typecheck ensuring the new external storage methods exist under next. |
| soroban-sdk/Cargo.toml | Adds the next feature and forwards it to dependent crates. |
| soroban-sdk-macros/Cargo.toml | Adds a next feature to forward to soroban-env-common/next. |
Comment on lines
+332
to
+341
| #[cfg(all(target_family = "wasm", feature = "next"))] | ||
| fn try_get_internal(&self, key: Val, storage_type: StorageType) -> Option<Val> { | ||
| #[link(wasm_import_module = "l")] | ||
| extern "C" { | ||
| #[link_name = "h"] | ||
| fn try_get_contract_data(k: Val, t: StorageType, has_pos: internal::U32Val) -> Val; | ||
| } | ||
|
|
||
| let mut has = Val::VOID.to_val(); | ||
| let has_pos = internal::U32Val::from((&mut has as *mut Val) as u32); |
Comment on lines
+422
to
+430
| #[cfg(all(not(target_family = "wasm"), feature = "next"))] | ||
| fn has_external_internal( | ||
| &self, | ||
| _contract: AddressObject, | ||
| _key: Val, | ||
| _storage_type: StorageType, | ||
| ) -> bool { | ||
| panic!("external storage reads require protocol 28 host support") | ||
| } |
Comment on lines
+460
to
+468
| #[cfg(all(not(target_family = "wasm"), feature = "next"))] | ||
| fn try_get_external_internal( | ||
| &self, | ||
| _contract: AddressObject, | ||
| _key: Val, | ||
| _storage_type: StorageType, | ||
| ) -> Option<Val> { | ||
| panic!("external storage reads require protocol 28 host support") | ||
| } |
Comment on lines
+492
to
+496
| /// Returns if the provided contract stores a persistent value under `key`. | ||
| /// | ||
| /// This is read-only and does not invoke the target contract. | ||
| #[cfg(feature = "next")] | ||
| pub fn has_external<K>(&self, contract: &Address, key: &K) -> bool |
|
This would basically allow us to not have the need to make getters for everything 👍 |
Contributor
|
@tupui once we standardized exporting storage types and layout. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Adds
nextSDK APIs for read-only access to another contract's storage:env.storage().persistent().has_external(&contract, &key)env.storage().persistent().get_external::<_, V>(&contract, &key)env.storage().temporary().has_external(&contract, &key)env.storage().temporary().get_external::<_, V>(&contract, &key)env.storage().instance().has_external(&contract, &key)env.storage().instance().get_external::<_, V>(&contract, &key)The WASM path imports the protocol-28 host ABI directly. Optional reads use
try_get_external_contract_dataso a storedVoidremains distinguishable from a missing value. Local storagegetalso uses the new optional host path undernextfor the same reason.This branch intentionally stays on the current upstream SDK/env/XDR package versions and only adds
nextfeature hooks. It is not an official protocol release/version bump.Why
This lets contracts read intentionally public state from another contract without a cross-contract view invocation.
For lending, this enables a cleaner split where a governance, market, or oracle registry contract owns audit-critical read-only state such as market configuration, oracle references, reserve factors, pause flags, and collateral factors. The controller contract can execute the lending flow and read those keys directly, which helps keep controller and pool WASMs smaller while preserving modular contracts that are easier to audit.
Expected benefits from the host PR benchmark are lower CPU, memory, and wall-time cost than invoking a target contract just to return the same stored value.
Known limitations
nextand are not for protocol <= 27 contracts.Validation
cargo check -p soroban-sdkcargo check -p soroban-sdk --features nextcargo test -p soroban-sdk --features next,testutils --test external_storage_nextcargo fmt -p soroban-sdk -p soroban-sdk-macros -- --checkgit diff --check