Skip to content
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

[x/programs] All host functions (that get linked) should follow the same two argument calling convention #862

Closed
richardpringle opened this issue Apr 25, 2024 · 4 comments
Assignees

Comments

@richardpringle
Copy link
Contributor

richardpringle commented Apr 25, 2024

Similarly to #638, we want to serialize all the arguments into a single Vec<u8>. Further, we need to make sure that the bytes aren't dropped before they're read by the host.

I'm going to propose that all host functions (on the Rust side) have a safe wrapper and that their actual definition is written with a macro instead of being called directly.

Right now, the code looks something like this:

#[link(wasm_import_module = "state")]
extern "C" {
    #[link_name = "put"]
    fn _put(caller: i64, key: i64, value: i64) -> i64;
}

pub unsafe fn put_bytes<V>(caller: &Program, key: &Key, value: &V) -> Result<(), Error>
where
    V: BorshSerialize,
{
    let value_bytes = borsh::to_vec(value).map_err(|_| Error::Serialization)?;
    let caller = to_host_ptr(caller.id())?;
    let value = to_host_ptr(&value_bytes)?;
    let key = to_host_ptr(key)?;

    match unsafe { _put(caller, key, value) } {
        0 => Ok(()),
        _ => Err(Error::Write),
    }
}

But this functionality should look something like this:

pub fn put_bytes<V>(caller: &Program, key: &Key, value: &V) -> Result<(), Error>
where
    V: BorshSerialize,
{
    call_host_fn! {
        wasm_import_module = "state";
        link_name = "put";
        args = (caller, key, value);
    }
}

Here the macro would actually define the external function. Alternatively, if there is no module, there should be another pattern to patch the following:

pub fn put_bytes<V>(caller: &Program, key: &Key, value: &V) -> Result<(), Error>
where
    V: BorshSerialize,
{
    call_host_fn!("put", (caller, key, value))
}

Inside the macro, the actual name of the function inside the extern block doesn't matter; it's only the link name that does something.

Could also fix #849 while fixing this.

@richardpringle
Copy link
Contributor Author

Actually assigned to @iFrostizz

@richardpringle richardpringle changed the title [x/programs] All host functions (that get linked) should can the same two argument calling convention [x/programs] All host functions (that get linked) should follow the same two argument calling convention Apr 25, 2024
@iFrostizz
Copy link
Collaborator

TODO: after merging #872, we should serialize the parameters being (ptr_offset, data_len)* with Borsh and just pass one of these CPointer which will simplify the API a lot.

@richardpringle
Copy link
Contributor Author

TODO: after merging #872, we should serialize the parameters being (ptr_offset, data_len)* with Borsh and just pass one of these CPointer which will simplify the API a lot.

done

@richardpringle
Copy link
Contributor Author

Going to close this as #900 essentially achieves everything we want. I think we want to make one more minor change in that we want to generically call functions with context + n args (including the trivial case)... but I don't think we need an issue tracking that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

No branches or pull requests

2 participants