Skip to content
This repository has been archived by the owner on Aug 2, 2022. It is now read-only.

Add cleos helper command to add eosio.code to permission #6116

Merged
merged 1 commit into from
Oct 24, 2018

Conversation

conr2d
Copy link
Contributor

@conr2d conr2d commented Oct 23, 2018

Change Description

Fixes #6072
This PR adds --add-code and --remove-code options to cleos set account permission command so as to add or remove eosio.code permission conveniently to specified permission authority.

Documentation Additions

Following examples can be added to documentation.

  1. Add eosio@eosio.code to eosio@active
    cleos set account permission eosio active --add-code

  2. Remove eosio@eosio.code from eosio@active
    cleos set account permission eosio active --remove-code

  3. Add eosio.token@eosio.code to eosio@owner
    cleos set account permission eosio owner eosio.token --add-code -p eosio@owner

@conr2d conr2d force-pushed the code_permission branch 3 times, most recently from c605bb7 to a435f0d Compare October 24, 2018 08:27
@heifner heifner requested a review from arhag October 24, 2018 13:35
Copy link
Contributor

@arhag arhag left a comment

Choose a reason for hiding this comment

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

Nice job. There are three issues I discovered though and I have suggested changes to fix them.

First, it should be possible to add and remove null@eosio.code to an authority. The way you are checking for the null string of the authority to maintain the old behavior of deleting a permission regardless of whether --add-code or --remove-code are specified was preventing that from being possible.

Second, the blockchain rules enforce certain validation rules on authorities provided in updateauth. One of those rules is that the permission_levels specified in the accounts vector of an authority must be in ascending order with no repeats (similar rules apply for keys and waits as well). So, if you just add a new code permission to the end of the accounts vector, it will sometimes fail. My suggested code changes should insert the new code permission into the correct spot.

Third, an authority can consist entirely of waits (meaning no entries in keys and accounts). This is perhaps not a wise move because it means any account can satisfy that authority if they just delay their transaction for long enough. But it is technically a valid authority, and so that must be considered by this cleos command to make sure to not incorrectly delete a permission that won't actually have an empty authority after remove the code permission.

programs/cleos/main.cpp Show resolved Hide resolved
programs/cleos/main.cpp Outdated Show resolved Hide resolved
programs/cleos/main.cpp Outdated Show resolved Hide resolved
programs/cleos/main.cpp Outdated Show resolved Hide resolved
programs/cleos/main.cpp Outdated Show resolved Hide resolved
programs/cleos/main.cpp Outdated Show resolved Hide resolved
programs/cleos/main.cpp Outdated Show resolved Hide resolved
programs/cleos/main.cpp Outdated Show resolved Hide resolved
@conr2d
Copy link
Contributor Author

conr2d commented Oct 24, 2018

@arhag I applied all of your suggestions. Thanks for your kind review. :)

Copy link
Contributor

@arhag arhag left a comment

Choose a reason for hiding this comment

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

Thanks!

For future reference, please just add a new commit of the requested changes rather than force pushing a replacement commit since it makes it easier to review the changes.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants