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

remove eip-2200-advance transition #11624

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

q9f
Copy link
Member

@q9f q9f commented Apr 11, 2020

context ref #11347 ref #11474

@q9f
Copy link
Member Author

q9f commented Apr 11, 2020

ok this is good to go.

@@ -396,7 +396,7 @@ fn calculate_eip1283_sstore_gas<Gas: evm::CostType>(schedule: &Schedule, origina
if current == new {
// 1. If current value equals new value (this is a no-op), `SSTORE_DIRTY_GAS`
// (or if not set, `SLOAD_GAS`) is deducted.
schedule.sstore_dirty_gas.unwrap_or(schedule.sload_gas)
schedule.sload_gas
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think the comments need to be updated here and below, it might be easier to just revert #11347

Copy link
Collaborator

@niklasad1 niklasad1 Apr 14, 2020

Choose a reason for hiding this comment

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

I don't think we can revert it entirely because we still need the builtins, no?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Good point, we can revert this commit only openethereum/openethereum@10077a7

Copy link
Member Author

Choose a reason for hiding this comment

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

Not everything in that commit is inaccurate. Maybe I'll just manually rewrite the comments in question.

Copy link
Collaborator

Choose a reason for hiding this comment

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

This still needs to be addressed ^^

@q9f q9f closed this Apr 14, 2020
@q9f q9f deleted the q9-eip2200-advance branch April 14, 2020 22:09
@q9f q9f restored the q9-eip2200-advance branch April 15, 2020 09:21
@q9f q9f reopened this Apr 15, 2020
@ordian ordian added the A5-grumble 🔥 Pull request has minor issues that must be addressed before merging. label Apr 17, 2020
@vorot93
Copy link

vorot93 commented Jun 6, 2020

Is this still relevant?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A5-grumble 🔥 Pull request has minor issues that must be addressed before merging.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants