-
Notifications
You must be signed in to change notification settings - Fork 94
feat: add try_as_contract fn #1662
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: release/v25-preview
Are you sure you want to change the base?
Conversation
leighmcculloch
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some feedback inline.
soroban-sdk/src/env.rs
Outdated
| /// | ||
| /// Used to write or read contract data, or take other actions in tests for | ||
| /// setting up tests or asserting on internal state. | ||
| pub fn try_as_contract<T>(&self, id: &Address, f: impl FnOnce() -> T) -> Result<T, HostError> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For the return value I think this function should return an InvokeError and the same Result<Result<>, Result<>> structure for consistency with the try_invoke_contract function.
rs-soroban-sdk/soroban-sdk/src/env.rs
Lines 407 to 412 in 9415cee
| pub fn try_invoke_contract<T, E>( | |
| &self, | |
| contract_address: &Address, | |
| func: &crate::Symbol, | |
| args: Vec<Val>, | |
| ) -> Result<Result<T, T::Error>, Result<E, InvokeError>> |
Why: So the dev has the same error handling experience as for invoking. The as_contract fn is basically like calling invoke_contract on a contract function. Also, like the other try_ functions the case where T is the wrong type should be handable as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
*_as_contract extracts the result of the closure without converting it to a Val. Thus, I kept the Ok result as T, otherwise we need to convert it to a val and back which felt weird.
Good call on the error, updated it to match the try_invoke method and added tests.
What
Adds a
try_as_contractfn that gracefully returns errors liketry_invoke_contract.Why
Fixes #1434
Known limitations