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

Add --create option to evmc run #566

Merged
merged 2 commits into from
Mar 22, 2021
Merged

Add --create option to evmc run #566

merged 2 commits into from
Mar 22, 2021

Conversation

chfast
Copy link
Member

@chfast chfast commented Nov 5, 2020 •
edited
Loading 

> bin/evmc run -h
Execute EVM bytecode
Usage: bin/evmc run [OPTIONS] code

Positionals:
  code TEXT REQUIRED          Hex-encoded bytecode

Options:
  -h,--help                   Print this help message and exit
  --gas INT:INT in [0 - 1000000000]=1000000
                              Execution gas limit
  --rev ENUM=7                EVM revision
  --input TEXT                Hex-encoded input bytes
  --create                    Create new contract out of the code and then execute this contract with the input

@chfast chfast mentioned this pull request Nov 5, 2020
Merged
Base automatically changed from example_push32 to master February 15, 2021 15:49
@chfast chfast force-pushed the tools_run_create branch 3 times, most recently from b7d820a to ecf6ae1 Compare February 15, 2021 17:48
@chfast chfast requested review from axic and gumb0 February 15, 2021 17:51
@chfast chfast marked this pull request as ready for review February 15, 2021 17:52
@chfast chfast force-pushed the tools_run_create branch 2 times, most recently from 590721d to 8a61e92 Compare February 16, 2021 10:37
gumb0
gumb0 reviewed Feb 16, 2021
View reviewed changes
tools/commands/run.cpp Outdated

const auto deploy_result = vm.execute(host, rev, deploy_msg, code.data(), code.size());
if (deploy_result.status_code != EVMC_SUCCESS)
return 1;
Copy link
Member

Choose a reason for hiding this comment

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

Perhaps output a message in this case.

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.

@codecov-io
Copy link

Codecov Report

Merging #566 (d2645f1) into master (b606331) will increase coverage by 0.28%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #566      +/-   ##
==========================================
+ Coverage   93.54%   93.82%   +0.28%     
==========================================
  Files          23       23              
  Lines        3422     3497      +75     
==========================================
+ Hits         3201     3281      +80     
+ Misses        221      216       -5

axic
axic reviewed Mar 8, 2021
View reviewed changes
tools/commands/run.cpp Outdated
@@ -11,24 +11,62 @@ namespace evmc
{
namespace cmd
{
namespace
{
constexpr auto deploy_address = 0x000000000000000000000000000000000c9ea7ed_address;
Copy link
Member

Choose a reason for hiding this comment

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

Why does this address has so many leading zeroes?

Copy link
Member Author

Choose a reason for hiding this comment

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

Changed.

axic
axic reviewed Mar 8, 2021
View reviewed changes
tools/commands/run.cpp Outdated
return 1;

auto& deployed_account = host.accounts[deploy_address];
deployed_account.code = bytes(deploy_result.output_data, deploy_result.output_size);
Copy link
Member

Choose a reason for hiding this comment

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

Why isn't this done in MockedHost?

Copy link
Member Author

Choose a reason for hiding this comment

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

MockedHost does not have any call/create logic, it just records the passed message.

axic
axic approved these changes Mar 8, 2021
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.

Okay for me, but left some questions.

@chfast chfast force-pushed the tools_run_create branch 2 times, most recently from 2eac3cd to f8a39a1 Compare March 8, 2021 14:14
gumb0
gumb0 reviewed Mar 9, 2021
View reviewed changes
test/unittests/tool_commands_test.cpp Outdated
auto vm = evmc::VM{evmc_create_example_vm()};
std::ostringstream out;

const auto exit_code = cmd::run(vm, EVMC_PETERSBURG, 0, "FE", "", true, out);
Copy link
Member

Choose a reason for hiding this comment

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

Nit: other hex strings are lowercase in this file.

Suggested change
const auto exit_code = cmd::run(vm, EVMC_PETERSBURG, 0, "FE", "", true, out);
const auto exit_code = cmd::run(vm, EVMC_PETERSBURG, 0, "fe", "", true, out);

@chfast chfast force-pushed the tools_run_create branch from f8a39a1 to ed4dddf Compare March 22, 2021 10:20
@chfast chfast force-pushed the tools_run_create branch from ed4dddf to c8c90e0 Compare March 22, 2021 10:55
@chfast chfast force-pushed the tools_run_create branch from c8c90e0 to 6330289 Compare March 22, 2021 11:00
@chfast chfast merged commit bfe3cf1 into master Mar 22, 2021
@chfast chfast deleted the tools_run_create branch March 22, 2021 11:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gumb0 gumb0 gumb0 approved these changes

@axic axic axic approved these changes

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

Successfully merging this pull request may close these issues.
4 participants
@chfast @codecov-io @axic @gumb0