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 option to preload system contracts from file before execution #289

Merged
merged 1 commit into from
Aug 28, 2018

Conversation

jakelang
Copy link
Member

@jakelang jakelang commented Aug 3, 2018

Closes #288
Syntax: --evmc sys:<address or alias>=<filepath>
Hex address must be formatted with a leading 0x and must be 40 characters long (all nibbles must be explicitly specified, including leading zeroes).

@axic
Copy link
Member

axic commented Aug 10, 2018

I think as milestone 1, this should just replace the code in hera_execute depending on what is the msg.destination.

So it is only two pieces of code added here:

  • parsing the replacement list from CLI & loading the code
  • using that in hera_execute

Later we can think about how to deploy and replace more.

In milestone 1 it will be possible to:

  • --evmc metering=true --evmc sys:sentinel=mysentinel.wasm
  • --evmc evm2wasm=true --evmc sys:evm2wasm=myevm2wasm.wasm

@jakelang
Copy link
Member Author

Rebased.

@jakelang
Copy link
Member Author

@axic updated this to override the code at runtime, without CREATEing system contracts

@jakelang jakelang force-pushed the syscontract-cli branch 3 times, most recently from 604fcca to 9149571 Compare August 16, 2018 17:07
src/hera.cpp Outdated
#endif

//Check the "sys:" syntax by comparing substring
if (evmc_option_raw.length() < 4 || evmc_option_raw.substr(0, 4).compare("sys:") != 0) {
Copy link
Member

Choose a reason for hiding this comment

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

Canno you just use evmc_option_raw.find("sys:") == 0 here?

@codecov-io
Copy link

codecov-io commented Aug 16, 2018

Codecov Report

Merging #289 into master will decrease coverage by 9.71%.
The diff coverage is 17.7%.

@@            Coverage Diff             @@
##           master     #289      +/-   ##
==========================================
- Coverage   72.06%   62.34%   -9.72%     
==========================================
  Files           8        5       -3     
  Lines         970      964       -6     
  Branches      118      139      +21     
==========================================
- Hits          699      601      -98     
- Misses        241      331      +90     
- Partials       30       32       +2

src/hera.cpp Outdated
@@ -61,6 +61,7 @@ struct hera_instance : evmc_instance {
hera_wasm_engine wasm_engine = hera_wasm_engine::binaryen;
hera_evm_mode evm_mode = hera_evm_mode::reject;
bool metering = false;
vector<pair<evmc_address, string>> contract_preload_list;
Copy link
Member

Choose a reason for hiding this comment

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

Why not use a map here?

src/hera.cpp Outdated
#endif

//Check the "sys:" syntax by comparing substring
if (evmc_option_raw.length() < 4 || evmc_option_raw.find("sys:") != 0) {
Copy link
Member

Choose a reason for hiding this comment

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

I think there isn't a need to check the length if using find.

return pair<evmc_address, bool>(ret, true);
}

pair<evmc_address, bool> parse_preload_addr(const char *name)
Copy link
Member

Choose a reason for hiding this comment

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

I think we need a helper which does the entire string parsing and returns a pair<address, filename>, where if either of them is empty that means it failed. This should not do alias resolution.

Copy link
Member

Choose a reason for hiding this comment

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

address is a string and can contain an alias or a hex string.

@jakelang jakelang force-pushed the syscontract-cli branch 4 times, most recently from bc65111 to 09558f1 Compare August 21, 2018 17:00
src/hera.cpp Outdated
@@ -64,6 +64,7 @@ struct hera_instance : evmc_instance {
hera_wasm_engine wasm_engine = hera_wasm_engine::binaryen;
hera_evm_mode evm_mode = hera_evm_mode::reject;
bool metering = false;
vector<pair<evmc_address, string>> contract_preload_list;
Copy link
Member

Choose a reason for hiding this comment

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

I think this should be vector<pair<evmc_address, vector<uint8_t>>> so that the at runtime it doesn't need to be loaded every time.

@axic axic force-pushed the syscontract-cli branch 3 times, most recently from b7db68a to 435fcf6 Compare August 26, 2018 23:31
@axic axic self-assigned this Aug 28, 2018
@axic axic changed the title [WIP] Add option to preload system contracts from file before execution Add option to preload system contracts from file before execution Aug 28, 2018
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