Changing node domain makes previous domain unavailable

[John Henderson]

Summary:

When set-node is called to change the domain of an existing operator, then the previous domain can no longer be used.

Explanation of root cause:

The OCN Registry smart contract keeps a private variable that tracks the domain which have been used:

mapping(string => bool) private uniqueDomains;

When a set-node is called, there is a check that the domain isn't true in the uniqueDomains mapping:

// check for domain uniqueness
require(uniqueDomains[domain] == false, "Domain name already registered.");
uniqueDomains[domain] = true;

So when set-node is called, both the previous domain and the new domain are true in the uniqueDomains mapping

This is the relevant code of delete-node:

string memory domain = nodeOf[operator];
require(bytes(domain).length > 0, "Cannot delete node that does not exist.");// delete node and log
uniqueDomains[domain] = false;

So first, it is getting the current domain of the operator. And then it is setting that to false in the uniqueDomains mapping
So only the current domain is "released" from the uniqueDomains mapping. The previous domain remains set to true in the uniqueDomains and is therefore unusable

[Adam Staveley]

As a medium term solution (aside from the documentation you have written - thanks btw!), we might consider the following alternatives on the TS/Kt library/CLI level:

• “deprecate” current set-node command and create a new command that:

  • checks if the command is a create or update: if create it simply calls set-node; if update it calls delete-node before set-node.

• modify set-node to check if the command is create or update: if create continue as normal; if update return an error message that telling the user to first run delete-node.

‌

[John Henderson]

I think we should, at a minimum, do the second option to make set-node safe to use. IMO, just deprecating set-node isn’t enough as I think the issue would still be relatively likely to occur.

What do you think of modifying set-node to call delete-node before set-node if it is an update (which was your suggestion, just doing it for set-node instead of for a new command)? I suppose one issue might be that it could be seen as deceitful to be executing two transactions when the user only initiated one command. An implementation of this approach is here: https://bitbucket.org/shareandcharge/ocn-registry/pull-requests/20/fix-setnode-8-calling-delete-if-already

[John Henderson]

Guard in ocn-registry cli added by PR #26

[John Henderson]

Root cause is in ocn-registry smart contract and there are no plans to update registry at the moment. Guard in cli should prevent most cases.