thehubbleproject / hubble-contracts

Hubble optimistic rollup
https://thehubbleproject.github.io/docs/
MIT License
133 stars 29 forks source link

Modify TokenRegistry contract #677

Closed max-sidorov closed 2 years ago

max-sidorov commented 2 years ago

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

jacque006 commented 2 years ago

Couple of things worth considering with these changes:

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 :+1: assuming the above has been considered.

msieczko commented 2 years ago

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.