search

status-im / ens-usernames

DApp to register usernames for Status Network
MIT License
19 stars 11 forks source link

Replace setState() with dynamic getState() #101

Closed 3esmit closed 4 years ago

3esmit commented 4 years ago

Fixes #100

Activation of contract now is signaled through a boolean, which is changed only once after it's activated.

Moved state is not assumed to happen only though the call of UsernameRegistrar.moveRegistry(address), instead it looks up the ownership of the name. This allows the use of BaseRegistrarImplementation.reclaim(uint256,address) (owner of .eth ENS name) to operate migrations, where the ownership can be directly transferred to a new version of UsernameRegistrar, reducing the migration time and cost.

There will be a slight overhead in gas cost for every operation which reads getState(), which won't be noticed by users due it's minimal impact.

The old registrar MUST still use the old 'UsernameRegistrar.moveRegistry(address)' to mark the it as Moved, so it will operate as it was designed.

3esmit commented 4 years ago

While I don't fully understand the context yet, I've commented on a potential bug. The name change is a bit confusing (removing a setter setState() to replace with a getter getState(); ideally we have both).

There is no setState because it was replaced by a dynamic lookup. If the contract don't owns the domain anymore, then it considers it moved.

This is also done by the ENS Domains base registrar, see here https://github.com/ensdomains/ethregistrar/blob/master/contracts/BaseRegistrarImplementation.sol#L31