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

Update gas cost #224

Closed
hugo-dc wants to merge 1 commit into from
Closed

Update gas cost #224

hugo-dc 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

@axic axic Apr 13, 2018

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

@axic axic May 25, 2018

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

2 more reviewers

@axic axic axic left review comments

@lrettig lrettig lrettig left review comments

Reviewers whose approvals may not affect merge requirements

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