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 test block with EIP-2930 tx #1376

Merged
merged 8 commits into from
Apr 7, 2021
Merged

Add test block with EIP-2930 tx #1376

merged 8 commits into from
Apr 7, 2021

Conversation

fvictorio
Copy link
Member

  • Add a full-block test for a Ropsten block that includes an EIP-2930 tx.
  • Add optional properties to the RpcTransaction type
  • Implement EIP2929StateManager interface in ForkStateManager

@fvictorio fvictorio changed the title Fork berlin Add test block with EIP-2930 tx Apr 6, 2021
// the following methods are copied verbatim from
// DefaultStateManager

public isWarmedAddress(address: Buffer): boolean {
Copy link
Member

Choose a reason for hiding this comment

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

What about doing return DefaultStateManager.prototype.isWarmedAddress.call(this, address) instead? The same for the rest of the methods.

Copy link
Member Author

Choose a reason for hiding this comment

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

Good idea, I'll give it a try.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done. Notice that this adds an implicit dependency between those method calls and certain instance variables that have to exist (_accessedStorage and _accessedStorageReverted). tsc doesn't seem to catch those errors.

Copy link
Member

Choose a reason for hiding this comment

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

Maybe leave a comment? Either thing is dangerous in its own way

Copy link
Member Author

Choose a reason for hiding this comment

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

There is one:

  // used by the DefaultStateManager calls
  private _accessedStorage: Array<Map<string, Set<string>>> = [new Map()];
  private _accessedStorageReverted: Array<Map<string, Set<string>>> = [
    new Map(),
  ];

@alcuadrado
Copy link
Member

I left a few small comments, but LGTM

@alcuadrado alcuadrado merged commit 4566718 into master Apr 7, 2021
@fvictorio fvictorio deleted the fork-berlin branch April 7, 2021 21:06
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 18, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants