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

Support for EXTCODEHASH #49

Merged
merged 2 commits into from
Jul 31, 2018
Merged

Support for EXTCODEHASH #49

merged 2 commits into from
Jul 31, 2018

Conversation

gumb0
Copy link
Member

@gumb0 gumb0 commented Jul 31, 2018

No description provided.

@gumb0 gumb0 mentioned this pull request Jul 31, 2018
Merged
axic
axic approved these changes Jul 31, 2018
View reviewed changes
Copy link
Member

@axic axic 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. Too bad this wasn't merged the same time as create2 so we didn't need to bump ABI again.

@gumb0
Copy link
Member Author

gumb0 commented Jul 31, 2018

@axic I've fixed the tests and updated the instrucrtions lib, you might want to take another look

@gumb0 gumb0 removed the in progress label Jul 31, 2018
@gumb0 gumb0 requested a review from chfast July 31, 2018 13:03
axic
axic reviewed Jul 31, 2018
View reviewed changes
examples/capi.c
@@ -16,13 +16,6 @@ struct evmc_uint256be balance(struct evmc_context* context, const struct evmc_ad
return ret;
}

struct evmc_address address(struct evmc_context* context)
Copy link
Member

Choose a reason for hiding this comment

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

Ah this wasn't used anymore?

Copy link
Member Author

Choose a reason for hiding this comment

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

Right.

@axic
Copy link
Member

axic commented Jul 31, 2018

It still looks good to me.

@gumb0 gumb0 merged commit 7559387 into master Jul 31, 2018
@gumb0 gumb0 deleted the extcodehash branch July 31, 2018 13:22
@axic
Copy link
Member

axic commented Jul 31, 2018 •
edited
Loading

@gumb0 but should have waited for a quick glimpse from @chfast 😉

@chfast
Copy link
Member

chfast commented Jul 31, 2018

Looks good. But we can drop account_exists() now.

@axic
Copy link
Member

axic commented Jul 31, 2018

I think it is fine to keep, because the hashing would be an extra unneeded step if account_exists is satisfactory.

@axic
Copy link
Member

axic commented Jul 31, 2018 •
edited
Loading

We could make it optional saying if it is not implemented, get_code_hash can be used instead. Probably a bad idea.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@axic axic axic approved these changes

@chfast chfast Awaiting requested review from chfast

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@gumb0 @axic @chfast