Skip to content
This repository has been archived by the owner on Jan 5, 2019. It is now read-only.

Update gas cost #224

Closed
wants to merge 1 commit into from
Closed

Update gas cost #224

wants to merge 1 commit into from

Conversation

hugo-dc
Copy link
Member

@hugo-dc hugo-dc commented Apr 12, 2018 •
edited by axic
Loading

Updated gas costs (based on ethereumjs-vm costs).

Fixes #164.

@hugo-dc hugo-dc requested a review from axic April 12, 2018 19:44
axic
axic reviewed Apr 13, 2018
View reviewed changes
opcodes.js
0xa1: ['LOG', 375, 3, 0],
0xa2: ['LOG', 375, 4, 0],
0xa3: ['LOG', 375, 5, 0],
0xa4: ['LOG', 375, 6, 0],
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if it is properly charging for the data too?

Copy link
Member

Choose a reason for hiding this comment

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

Can somebody check this?

Copy link
Member

@lrettig lrettig May 27, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

As discussed just now, evm2wasm does not charge for the data. Hera does it here: https://github.com/ewasm/hera/blob/master/src/eei.cpp#L503.

@lrettig lrettig force-pushed the update-gas-cost branch 3 times, most recently from 3cb059f to 6e62404 Compare May 25, 2018 23:00
@lrettig lrettig force-pushed the update-gas-cost branch from 6e62404 to 957994d Compare May 27, 2018 18:50
@lrettig
Copy link
Member

lrettig commented May 27, 2018

I tried running this against the tests in https://github.com/ethereum/tests/tree/develop/src/GeneralStateTestsFiller/stLogTests but they all depend on call which is broken pending #246

@lrettig
Copy link
Member

lrettig commented May 27, 2018 •
edited
Loading

Both evm2wasm and hera are currently metering for the interface methods. We need to turn off one or the other. evm2wasm does not have complete gas metering logic for dynamic metering for opcodes like LOG, so in the short term we should probably continue to do metering in hera, which means disabling metering for interface opcodes in evm2wasm.

@axic added a quick fix for this in #265.

@axic
Copy link
Member

axic commented Aug 1, 2018

I think we should close this as it may take a while doing this properly.

@axic axic closed this Aug 2, 2018
@axic axic deleted the update-gas-cost branch August 2, 2018 02:35
@axic axic removed the in progress label Aug 2, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@axic axic axic left review comments

@lrettig lrettig lrettig left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@hugo-dc @lrettig @axic