Skip to content
This repository has been archived by the owner on Nov 6, 2020. It is now read-only.

Balance queries implemented in WASM runtime #6639

Merged
merged 1 commit into from
Oct 4, 2017
Merged

Conversation

lexfrl
Copy link
Contributor

@lexfrl lexfrl commented Oct 4, 2017

No description provided.

@lexfrl lexfrl added A0-pleasereview 🤓 Pull request needs code review. M4-core ⛓ Core client code / Rust. labels Oct 4, 2017
@NikVolf NikVolf added A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. labels Oct 4, 2017
@NikVolf NikVolf changed the title Address balance extern provided for wasm runtime Balance queries implemented in WASM runtime Oct 4, 2017
let address = self.pop_address(&mut context)?;
let balance = self.ext.balance(&address).map_err(|_| UserTrap::BalanceQueryError)?;
let value: H256 = balance.into();
self.memory.set(return_ptr, &*value)?;
Copy link
Contributor

@rphmeier rphmeier Oct 4, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does there need to be a check that the return pointer is actually valid i.e. in allocated memory or on the stack?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how do you imagine such check?

Copy link
Contributor

@rphmeier rphmeier Oct 4, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i'm not sure, just not familiar with the intricacies of WASM. to me it looks like it just writes into whatever pointer it's given. obviously if the pointer is invalid then it should lead to an error in the WASM runtime, but it shouldn't cause a panic in self.memory.set. Are we sure it doesn't?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes - it doesn't panic - that's why memory.set returns Result<>

@svyatonik svyatonik merged commit 492da38 into master Oct 4, 2017
@svyatonik svyatonik deleted the wasm-ext-balance branch October 4, 2017 19:09
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A8-looksgood 🦄 Pull request is reviewed well. M4-core ⛓ Core client code / Rust.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants