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

Consensus protocol upgrade to forward setcode actions to the contract on the eosio account #6988

Closed
arhag opened this issue Mar 22, 2019 · 1 comment

Comments

Projects
None yet
1 participant
@arhag
Copy link
Contributor

commented Mar 22, 2019

Background

Normal execution of actions involve first calling any native handler that may be registered for the action and then calling the WebAssembly code if a contract is in place on the receiver account (see apply_context::exec_one). The condition a.code.size() > 0 (where a is the account_object of the receiver) is used in that code to check that a contract exists on the receiver and if not it will not attempt to execute WebAssembly code.

However, the existing code has additional restrictions that prevents the execution of WebAssembly code for a eosio::setcode action even when a contract exists. The full if statement to enter the part of the code that executes the contract is:

if( a.code.size() > 0 && !(act.account == config::system_account_name && act.name == N( setcode ) && receiver == config::system_account_name) ) {

Note that act is the currently executing action and config::system_account_name == N(eosio).

The original intention behind this condition, which is one we no longer find necessary and may even find undesirable to maintain, was to prevent the contract from executing if the eosio::setcode was setting a new contract on the eosio account. But due to a bug in the condition, it ended up preventing WebAssembly code from executing on any eosio::setcode actions, regardless of which account the eosio::setcode action was for.

Use cases are enabled by fixing the bug. For example, it enables operators of the network to deploy a system contract that is selective about the contracts it allows other accounts to deploy.

There is not as strong of a use case identified for allowing an eosio::setcode action that tries to set a contract on the eosio account to also be forwarded to execute on the contract deployed on eosio; but we haven't identified any harm in allowing that to occur as well. If it was to be allowed, there is a choice available between two ways of doing it. One option is for the old WebAssembly code to run in the context of the eosio::setcode action, essentially acting as a pre-check to the code that is to replace it. The other option is for the new WebAssembly code to run in the context of the eosio::setcode action. We have identified three arguments in favor of taking the latter approach:

  1. The code is currently structured to support easier implementation of the latter approach.
  2. Any subsequent actions that run because of the execution of the WebAssembly code running on eosio (whether before or after contract replacement) via the require_recipient or send_inline intrinsics would necessarily run in an environment in which the new contract was already deployed. So going with the latter approach provides more consistency to contract developers.
  3. There is an alternative mechanism already available to behave like a pre-check to changes to the contract deployed on eosio. A separate management account could be in charge of deploying changes to the eosio contract by providing a proxy action to setcode; any necessary pre-checks would be added to this contract. Then the eosio::setcode action could be linked for the eosio account to a custom permission that is unilaterally satisfiable by the eosio.code permission of the management account.

Consensus protocol upgrade feature

This protocol feature (codename: FORWARD_SETCODE) allows the contract WebAssembly code deployed on the eosio account to execute for eosio::setcode actions.

So when the eosio::setcode action is dispatched to the eosio receiver, first the native handler (see apply_eosio_newaccount) would run as it does currently. But then the WebAssembly code deployed on the eosio account would execute under the context of that action (assuming a contract was deployed on the eosio account).

This means that an eosio::setcode action that changes the contract on eosio would first change the contract to the new one, and then execute the new contract under the context of the eosio::setcode action. Given the recent changes of eosio.cdt v1.6.x, it means any contract compiled with recent versions of eosio.cdt that is to be deployed onto eosio should include a handler for the eosio::setcode action even if it is to do nothing; otherwise, the auto-generated dispatcher would reject the eosio::setcode action making it impossible to deploy that contract to eosio in the first place. (Note: the current eosio.system contract is properly designed to handle this, but the eosio.bios contract would need to be updated.)

The changes described above can be achieved, after the protocol feature foundations have been completed (see issues #6429 and #6431), by simply changing the if condition mentioned in the background section to instead be:

if( a.code.size() > 0 && ( control.is_builtin_activated( builtin_protocol_feature_t::forward_setcode ) || !(act.account == config::system_account_name && act.name == N( setcode ) && receiver == config::system_account_name) ) ) {

(The above change omits the other code changes needed to setup FORWARD_SETCODE as a new protocol feature in the first place.)

@arhag arhag added the CONSENSUS label Mar 22, 2019

arhag added a commit that referenced this issue Apr 11, 2019

Added protocol_feature_tests/forward_setcode_test unit test to test t…
…he FORWARD_SETCODE protocol feature. #6988

Added a new test contract, reject_all, to enable the unit test.
@arhag

This comment has been minimized.

Copy link
Contributor Author

commented Apr 11, 2019

Resolved by #7109.

@arhag arhag closed this Apr 11, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.