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

Add support for EIP-1087: Net storage gas metering for the EVM #50

Closed
chfast opened this issue Aug 1, 2018 · 6 comments
Closed

Add support for EIP-1087: Net storage gas metering for the EVM #50

chfast opened this issue Aug 1, 2018 · 6 comments
Assignees

Comments

@chfast
Copy link
Member

chfast commented Aug 1, 2018

I propose to extend the function evmc_set_storage() by returning additional information to

enum evmc_storage_status evmc_set_storage(
    struct evmc_context* context,
    const struct evmc_address* address,
    const struct evmc_uint256be* key,
    const struct evmc_uint256be* value
);

The enum evmc_storage_status would contain the information what happened with the storage, like

  • unmodified,
  • modified,
  • added,
  • deleted,
  • modified_dirty.

The gas cost is going to be applied after the evmc_set_storage() is executed (is this a problem?).

This also avoids calling evmc_get_storage() first.

@chfast
Copy link
Member Author

chfast commented Aug 1, 2018

cc @Arachnid

@gumb0
Copy link
Member

gumb0 commented Aug 1, 2018

The EIP suggests different costs when the slot is modified for the first time in transaction vs subsequent modification in the same transaction, so I think this proposal doesn't cover this difference.

@chfast chfast closed this as completed Aug 1, 2018
@chfast chfast reopened this Aug 1, 2018
@chfast
Copy link
Member Author

chfast commented Aug 1, 2018

The EIP suggessts different costs when the slot is modified for the first time in transaction vs subsequent modification in the same transaction, so I think this proposal doesn't cover this difference.

You are right. I didn't wanted to make it so much detailed. I meant "include any distinctive storage status need to implement Constantinople".

I added modified_dirty status which means "a dirty storage slot was modified".

@gumb0
Copy link
Member

gumb0 commented Aug 1, 2018

Sufficient statuses I think would be

  • unmodified
  • added_clear // modified for the first time the slot that was zero
  • modified_clear // modified for the first time the slot that was non-zero
  • modified_dirty

(Refunds for deleting are handled outside of VM in transaction finalization)

@chriseth chriseth removed their assignment Aug 1, 2018
@chfast chfast assigned chfast and unassigned axic, holiman, chfast and gumb0 Aug 8, 2018
@chfast
Copy link
Member Author

chfast commented Aug 8, 2018

I will work on this.

@chfast
Copy link
Member Author

chfast commented Aug 9, 2018

I'm closing it as partly done in #52. The list would have to extended for EIP-1087, but nothing complicated is expected.

@chfast chfast closed this as completed Aug 9, 2018
@chfast chfast removed the in progress label Aug 9, 2018
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

No branches or pull requests

5 participants