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

Implementation of EIP-2929 #1387

Merged
merged 20 commits into from
Sep 28, 2020
Merged

Implementation of EIP-2929 #1387

merged 20 commits into from
Sep 28, 2020

Conversation

shemnon
Copy link
Contributor

@shemnon shemnon commented Sep 21, 2020

PR description

  • store warmed addresses and slots in the MessageFrame
  • probably need to do a gas calculator refactor at some point

Signed-off-by: Danno Ferrin danno.ferrin@gmail.com

builds off of #1386

Changelog

Legacy State tests before constantinople were moved to a child
directory.  We still need to validate against those.  Generate and
execute those tests under a "LegacyX" class series.

Signed-off-by: Danno Ferrin <danno.ferrin@gmail.com>
* deprecate old yoloV1 options
* replace with  yoloV2 options
* clear the yolo bootnode
* expose experimental EIPs flag to EVM Tool

Signed-off-by: Danno Ferrin <danno.ferrin@gmail.com>
* store warmed addresses and slots in the MessageFrame
* probably need to do a gas calculator refactor at some point

Signed-off-by: Danno Ferrin <danno.ferrin@gmail.com>
Recipient and sender are (almost) always the same.  We want sender and
contract (which should be recipient, unless it's a create, when we
do want the contract)

Signed-off-by: Danno Ferrin <danno.ferrin@gmail.com>
Signed-off-by: Danno Ferrin <danno.ferrin@gmail.com>
Signed-off-by: Danno Ferrin <danno.ferrin@gmail.com>
Signed-off-by: Danno Ferrin <danno.ferrin@gmail.com>
* 0x0 is not a precompile.
* Call operations need to check against to, not value recipient (which
  can be null)

Signed-off-by: Danno Ferrin <danno.ferrin@gmail.com>
Signed-off-by: Danno Ferrin <danno.ferrin@gmail.com>
Signed-off-by: Danno Ferrin <danno.ferrin@gmail.com>
Signed-off-by: Danno Ferrin <danno.ferrin@gmail.com>
* keep order of test cases
* show all memory

Signed-off-by: Danno Ferrin <danno.ferrin@gmail.com>
Signed-off-by: Danno Ferrin <danno.ferrin@gmail.com>
Signed-off-by: Danno Ferrin <danno.ferrin@gmail.com>
Dear mocking frameworks, I hate you.
Also, reference tests: chillax on the memory load.

Signed-off-by: Danno Ferrin <danno.ferrin@gmail.com>
shemnon added a commit to shemnon/ethereum-tests that referenced this pull request Sep 24, 2020
Filled from hyperledger/besu#1387

Signed-off-by: Danno Ferrin <danno.ferrin@gmail.com>
Signed-off-by: Danno Ferrin <danno.ferrin@gmail.com>
Copy link
Contributor

@matkt matkt left a comment

Choose a reason for hiding this comment

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

LGTM,
It seems that there are also the modifications of this PR #1386. Otherwise it's ok

@RatanRSur
Copy link
Contributor

Should this have a changelog entry?

Signed-off-by: Danno Ferrin <danno.ferrin@gmail.com>
Signed-off-by: Danno Ferrin <danno.ferrin@gmail.com>
Signed-off-by: Danno Ferrin <danno.ferrin@gmail.com>
@shemnon
Copy link
Contributor Author

shemnon commented Sep 28, 2020

Changelog entry added.

final Address address = account.getAddress();
final Bytes32 key = frame.popStackItem();
final boolean slotIsWarm = frame.warmUpStorage(address, key);
final Optional<Gas> optionalCost = slotIsWarm ? warmCost : coldCost;
Copy link
Contributor

Choose a reason for hiding this comment

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

This might be related to a question I asked in a previous pr but I can't remember. Do these need to be optional everywhere? For example, isn't the warmCost and coldCost always present here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's a mix. We need the optional for the return value but we need the value during gas calculations. The gas cost is not optional within the method but on the return, for some errors we return an empty cost (such as when we can't know because of a stack underflow).

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, I see, could we push the optional wrapping to as late as possible? I think it makes it clearer for which sections it is guaranteed to be defined. The intent is a bit unclear when the warmCost and coldCost fields are Optional.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The optionals are re-used across requests. If we wrapped in the execute method it would result in object churn.

Signed-off-by: Danno Ferrin <danno.ferrin@gmail.com>
@shemnon shemnon merged commit 0f69337 into hyperledger:master Sep 28, 2020
@shemnon shemnon mentioned this pull request Sep 28, 2020
1 task
@timbeiko timbeiko mentioned this pull request Oct 30, 2020
5 tasks
@shemnon shemnon deleted the EIP2929 branch February 26, 2022 18:41
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.

None yet

3 participants