FIP-40 establish permissions, integrate register_address_on_domain. #297
Conversation
FIP-40 skeletal contract for fio.perms
FIP-40 fio.perms initial integration into contracts
FIP-40 flush out error handling for add perm
FIP-40 implement parameter validation and tests
FIP-40 implement action logic for add and remove permission
FIP-40 table name change to plural
FIP-40 alittle code reformatting.
FIP-40 integrate register address with permissions.
FIP-40 integrate transfer domain and burn expired.
contracts/fio.perms/fio.perms.cpp
Outdated
| auto domains_iter = domainsbyname.find(domainHash); | ||
|
|
||
| fio_400_assert(domains_iter != domainsbyname.end(), "domnotfound object_name", object_name, | ||
| "Invalid object name", |
There was a problem hiding this comment.
Since this error is specific to the domain object, I recommend we use a clearer error like "FIO Domain not found"
contracts/fio.perms/fio.perms.cpp
Outdated
| const uint32_t domain_expiration = get_time_plus_seconds(domains_iter->expiration, SECONDS30DAYS); | ||
|
|
||
| const uint32_t present_time = now(); | ||
| fio_400_assert(present_time <= domain_expiration, "domexpired object_name", object_name, "Invalid object name", |
There was a problem hiding this comment.
"Domain expired: unable to set permission"
contracts/fio.perms/fio.perms.cpp
Outdated
| ErrorInvalidObjectName); | ||
|
|
||
| //check domain owner is actor | ||
| fio_400_assert((actor.value == domains_iter->account), "actornotowner object_name", object_name, |
There was a problem hiding this comment.
"Only domain owner can set permissions" (or something like that)
contracts/fio.perms/fio.perms.cpp
Outdated
| "grantee account is invalid", ErrorInvalidGranteeAccount); | ||
|
|
||
| fio_400_assert((grantee_account.value != actor.value), "grantee_account", grantee_account.to_string(), | ||
| "grantee account is invalid", ErrorInvalidGranteeAccount); |
There was a problem hiding this comment.
Is this a check that ensures that the actor is not granting themselves permission? If so, we should have a clearer error like "Cannot grant permissions to self"
contracts/fio.perms/fio.perms.cpp
Outdated
|
|
||
| // error if object name is not * or is not in the domains table | ||
| fio_400_assert(object_name.size() > 0, "object_name", object_name, | ||
| "Object name is invalid", ErrorInvalidObjectName); |
There was a problem hiding this comment.
Same as above. Generic "object name invalid" errors should be replaced with messages that will make it clear what is wrong with the transaction.
FIP-40 code review action items
FIP-40 add getters
FIP-40 merge develop up to dev branch of fio.contracts
FIP-40 implement wildcard for domain name option
FIP-40 final prep for merge
FIP-40 final merge prep
FIP-40 perp for merge into develop
|
|
||
| namespace fioio { | ||
|
|
||
| class [[eosio::contract("FioPermissions")]] FioPermissions : public eosio::contract { |
There was a problem hiding this comment.
Recommend using the contract name (should be used for all new contracts moving forward):
class [[eosio::contract("fio.perms")]] FioPermissions : public eosio::contract {
This will help alleviate issues with the abigen
This FIP establishes the notion of permissions on the fio protocol, and integrates a new permission which is defined in FIP-40, register_address_on_domain
please see the project info on the wiki at
https://fioprotocol.atlassian.net/wiki/spaces/FD/pages/555253761/FIP-40+Register+addresses+on+private+domains+access+control