-
Notifications
You must be signed in to change notification settings - Fork 354
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
BackendApi improvements #1985
BackendApi improvements #1985
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.
Good stuff. Only thing I am unsure is the Send bound. To me the blanket impl of Send
and Sync
we have for Environment
seems more at fault here though.
packages/vm/src/backend.rs
Outdated
pub trait BackendApi: Copy + Clone + Send { | ||
fn canonical_address(&self, human: &str) -> BackendResult<Vec<u8>>; | ||
fn human_address(&self, canonical: &[u8]) -> BackendResult<String>; | ||
pub trait BackendApi: Clone { |
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.
Shouldn't this continue to require Send
?
Or maybe it would be better to add the bound to the Send
impl of Environment
?
Very good point. This is probably all a leftover from trying to get everything to compile during the Wasmer 3 upgrade. Will revert the Send change in here and only drop the Copy requirement, which is overly strict IMO. Could you have a look into those Send and Sync implementations of Environment and how to make this code more solid? |
ba31f2c
to
df69d76
Compare
addr_validate can be implemented in Go directly such that we do not need to get the canonical address through cgo into Rust and send it back to get the humanized address again.