lang: Allow CPI return values #1598
Conversation
seems like a general problem with |
Added this.
Yea I think this is OK. |
lang/syn/src/codegen/program/cpi.rs
Outdated
| "()" => (quote! {anchor_lang::Result<()> }, quote! { Ok(()) }), | ||
| _ => ( | ||
| quote! { anchor_lang::Result<Return::<#ret_type>> }, | ||
| quote! { Ok(Return::<#ret_type> { phantom: PhantomData }) } |
There was a problem hiding this comment.
use full path for Return and PhantomData to avoid naming clashes (in the line above there's a Return too)
|
|
||
| pub fn parse_return(method: &syn::ItemFn) -> ParseResult<IxReturn> { | ||
| match method.sig.output { | ||
| syn::ReturnType::Type(_, ref ty) => { |
There was a problem hiding this comment.
an error here would be nice that hardcodes (with .to_string) that the return type must be of the form Result<()> or Result<T>
the return code (and possibly even other code) depends on this exact return type and you can't use aliases like type MyResult = Result<()>
There was a problem hiding this comment.
There is still some tests returning ProgramResult, this handles return types that don't confirm to Result<()> or Result<T> by defaulting to assuming it returns the unit type, these might break I think? I can commit it anyway and let CI figure it out
There was a problem hiding this comment.
yea I cant think of anything that can actually go wrong with your code defaulting to no return type. so no need for that extra check i think
lang/syn/src/codegen/program/cpi.rs
Outdated
| pub fn get(&self) -> Result<T> { | ||
| let (_key, data) = anchor_lang::solana_program::program::get_return_data().unwrap(); | ||
| let value = T::try_from_slice(&data).unwrap(); | ||
| Ok(value) |
There was a problem hiding this comment.
looks like this could just return T instead of Result<T>
| @@ -0,0 +1,13 @@ | |||
| { | |||
There was a problem hiding this comment.
I think the way it works / what worked for me is that you dont have to specify the dependencies here. The package.json file in the tests dir already has them and this folder can pull them as long as its defined as a npm workspace member in the package.json in tests
There was a problem hiding this comment.
Yea that is right
This reverts commit 594cf4d.
|
LGTM! Is there anything left you want to add or can I merge? |
|
All good from my end, thanks! 🚀 |
|
thank you for the contribution! |
If an instruction returns a type then write the result with the Solana
set_return_datasyscall. If the CPI client determines an instruction returns something it'll read it withget_return_data.This shouldn't change anything for standard
Result<()>returns from instructions, but specifying a Result with a type will result in aset_return_datacall regardless of whether the call is made via CPI. I haven't figured out whether that is a good thing or a bad thing.Closes #20