Modify TokenRegistry contract

[max-sidorov]

Simplify registration to one tx instead of two and prevent registering the same token twice.

[msieczko]

I'd rename RegisteredToken -> TokenRegistered similarly as here #676 (review). Other than that LGTM.

[msieczko]

LGTM. @jacque006 could you take a look as well and merge the PR?

[jacque006]

Couple of things worth considering with these changes:

• I believe it was a past intent to in some way limit the registration of tokens to either deploy time or some sort of access control (see tokenRegistry in Access control for contracts #584). For example, anyone might be able to request a token registration but only an owner/admin could finalize the registration.
• With these changes (and even before), anyone can register a token to a Hubble network. This could be nice as it allows anyone to onboard at token at their own purview/cost, but also means more scammy/less reputable tokens can also be registered which could have impacts on the reputation of the network. If having token registration fully open is a concern, I would at the very least recommend adding @openzepplin/contracts's Ownable to the TokenRegistry and adding the onlyOwner modifier to the registerToken.

I have no strong opinion on either your changes or the access control schemes mentioned above as they all have their tradeoffs. Since you are the most active devs I will leave it at your discretion. Current changes look good 👍 assuming the above has been considered.

[msieczko]

Hey @jacque006, thanks for your answer 👍

Current changes look good 👍 assuming the above has been considered.

I agree that we should to protect TokenRegistry (and SpokeRegistry similarly), so that possibly-malicious tokens are not added arbitrarily. Optimally I think it should be the responsibility of a DAO to accept/reject token registrations.

We're not there yet though, so this PR is just meant to simplify the token registration to a single tx - it is open to anyone now and will still be. The second, more important change in this PR is preventing repeated registration of a token contract - currently finaliseRegistration can be called multiple times for the same token.

I suggest merging this PR now and tackling the issues that you mention in the future.