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

Support eth_getBalance in mapping #5202

Merged
merged 4 commits into from
Feb 20, 2024
Merged

Conversation

incrypto32
Copy link
Member

Closes #895

@incrypto32 incrypto32 force-pushed the incrypto32/eth-get-balance branch from d5b71d3 to 6fee9e2 Compare February 13, 2024 06:32
@incrypto32 incrypto32 marked this pull request as ready for review February 13, 2024 06:33
@incrypto32 incrypto32 self-assigned this Feb 13, 2024
@incrypto32 incrypto32 force-pushed the incrypto32/eth-get-balance branch 2 times, most recently from f1249d3 to ad9dd45 Compare February 14, 2024 10:57
eth_adapter: &EthereumAdapter,
ctx: HostFnCtx<'_>,
wasm_ptr: u32,
) -> Result<AscPtr<AscBigInt>, HostExportError> {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we need to charge gas for this call? I am not sure about the mechanics/reasoning of gas, but it feels wrong to make this free.

Copy link
Member Author

Choose a reason for hiding this comment

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

Right, this shouldn't be gas free. @leoyvens how would we determine a good gas value for eth_getBalance?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I suppose we can start with charging the same we charge for eth_call.

@lutter
Copy link
Collaborator

lutter commented Feb 14, 2024

For the failing unit tests, if you rebase onto latest master, the tests should pass.

@lutter
Copy link
Collaborator

lutter commented Feb 14, 2024

Oh, one more thing: do we need a new spec or API version because this adds a host fn?

@incrypto32 incrypto32 force-pushed the incrypto32/eth-get-balance branch from 193a93c to 4232b32 Compare February 19, 2024 15:51
@incrypto32 incrypto32 force-pushed the incrypto32/eth-get-balance branch from 4232b32 to eb46cec Compare February 19, 2024 16:25
@incrypto32 incrypto32 force-pushed the incrypto32/eth-get-balance branch from 84c073b to 6ecc831 Compare February 20, 2024 03:14
@incrypto32 incrypto32 merged commit d4b6291 into master Feb 20, 2024
7 checks passed
@incrypto32 incrypto32 deleted the incrypto32/eth-get-balance branch February 20, 2024 12:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support for calling Ether Balances in mappings
3 participants