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.

Sign up for GitHub

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

Jump to bottom

eip160 change byte cost for exp op #346

Merged
merged 3 commits into from
Aug 14, 2018
Merged

eip160 change byte cost for exp op #346

merged 3 commits into from
Aug 14, 2018

Conversation

ayrat555
Copy link
Member

@ayrat555 ayrat555 commented Aug 14, 2018 •
edited
Loading

Changes:

  • add EIP158 config
  • implement EIP160 - change gas cost for exp op
  • run EIP158 state tests
  • run EIP158 blockchain tests

@ayrat555 ayrat555 added this to the Simple node (MVP) milestone Aug 14, 2018
@ghost ghost assigned ayrat555 Aug 14, 2018
@ghost ghost added the status: in progress label Aug 14, 2018
@ayrat555 ayrat555 mentioned this pull request Aug 14, 2018
Closed
4 tasks
germsvel
germsvel approved these changes Aug 14, 2018
View reviewed changes
Copy link
Contributor

@germsvel germsvel left a comment

Choose a reason for hiding this comment

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

Looks good to me. Just one comment on style, but feel free to ignore if you don't think it makes sense.

apps/evm/lib/evm/configuration/eip158.ex
alias EVM.Configuration

@spec contract_creation_cost(Configuration.t()) :: integer()
def contract_creation_cost(config),
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we might be able to use defdelegate for a lot of these functions that are simply calling the fallback_config. Don't think it changes things much in practice, but just a thought if you feel like it'd be nicer.

Copy link
Member Author

Choose a reason for hiding this comment

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

@germsvel Can you give a simple example? It looks like defdelegate is used to delegate to a different module

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmmm, yeah seems you're right. I thought there was another way to use defdelegate but doesn't seem like it makes sense here.

@ayrat555 ayrat555 merged commit 714436c into master Aug 14, 2018
@ghost ghost removed the status: in progress label Aug 14, 2018
@ayrat555 ayrat555 deleted the ab-eip160 branch August 14, 2018 13:34
@germsvel germsvel mentioned this pull request Oct 2, 2018
Closed
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@germsvel germsvel germsvel approved these changes

@masonforest masonforest Awaiting requested review from masonforest

Assignees

@ayrat555 ayrat555

Labels
blockchain evm
Projects
None yet
Milestone
Simple node (MVP)
Development

Successfully merging this pull request may close these issues.
2 participants
@ayrat555 @germsvel