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

tools: Add --input option to evmc run #564

Merged
merged 1 commit into from
Feb 15, 2021
Merged

tools: Add --input option to evmc run #564

merged 1 commit into from
Feb 15, 2021

Conversation

chfast
Copy link
Member

@chfast chfast commented Nov 3, 2020

No description provided.

@chfast chfast requested review from axic and gumb0 November 3, 2020 19:52

auto& run_cmd = *app.add_subcommand("run", "Execute EVM bytecode");
run_cmd.add_option("code", code_hex, "Hex-encoded bytecode")->required();
run_cmd.add_option("--vm", vm_config, "EVMC VM module")->required()->envname("EVMC_VM");
run_cmd.add_option("--gas", gas, "Execution gas limit", true)->check(CLI::Range(0, 1000000000));
run_cmd.add_option("--rev", rev, "EVM revision", true);
run_cmd.add_option("--input", input_hex, "Hex-encoded input bytes");
Copy link
Member

Choose a reason for hiding this comment

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

I know it is common to name this input, but I think technically --calldata is a better choice as it reflects what it does.

Copy link
Member

Choose a reason for hiding this comment

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

I'd go as far to propose we should rename it in the EVMC interface in with ABIv8.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think this is fine to rename it, but I see two issues:

  1. There is execution "output".
  2. In EVMC this will be msg.calldata_data.

@chfast chfast merged commit 897665c into master Feb 15, 2021
@chfast chfast deleted the tools_run_input branch February 15, 2021 16:44
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.

3 participants