-
Notifications
You must be signed in to change notification settings - Fork 970
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
Implement the InvokeHostFunctionOp part of CAP-52 #3450
Conversation
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.
Looks generally good! A few minor touchups.
src/rust/src/contract.rs
Outdated
/// Deserializes an [`xdr::HostFunction`] host function identifier, an [`xdr::ScVec`] XDR object of | ||
/// arguments, an [`xdr::Footprint`] and a sequence of [`xdr::LedgerEntry`] entries containing all | ||
/// the data the invocation intends to read. Then calls the host function with the specified | ||
/// arguments and serializes the [`xdr::ScVal`] return value |
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.
This comment is inaccurate, the return value of the host function call is thrown away. Instead the ReadWrite
set is serialized and returned.
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.
Good catch, fixed.
stellar/rs-stellar-contract-env
LGTM -- I leave the landing sequence to you, I think you might need to merge the other 2 first and then bump the Cargo.toml here to point to the post-merge revision ID of the env crate. |
r+ a68b53e |
r+ b1d22f7 |
Description
Implements the
InvokeHostFunctionOp
part of CAP-52, but does not include the host functions.Checklist
clang-format
v8.0.0 (viamake format
or the Visual Studio extension)