Add dm3 name registrar #1
Conversation
| // Event emitted when a name is removed | ||
| event NameRemoved(address indexed addr, string indexed name); | ||
|
|
||
| function initialize(bytes32 _parentNode) public initializer { |
There was a problem hiding this comment.
it is recommended to have a constructor that calls _disableInitializers() (or similar) to prevent logic contracts from being claimable at all (-> parity hack).
In the initializer, call initializers of all inherited contracts, like ownable (which needs the address of the new owner).
When using proxies without factories, there is a risk that someone can call the initializer of the new contract before you do. I suppose we can take that risk in this case, as a new proxy is not important yet. But calling initiliaze is crucial!!!
There was a problem hiding this comment.
would making the initializer restricted to the owner mitigate the issue?
There was a problem hiding this comment.
The proxy does not have an owner before it is initialized, so this is not possible.
Some hints:
- how to call the inherited initializers
- you can inherit UUPS upgradeable, and then have to override this function
- disable initializers in constructor
OZ v5 might have changed some of these approaches, so it would be wise to do read the updated docs.
| /// @param node The node to set the text for | ||
| /// @param key The key for the text | ||
| /// @param value The text to set | ||
| function setText( |
There was a problem hiding this comment.
The contract owner can register a name for a user, but not set a text record. Isn't that required to publish a profile? If so, maybe ownerRegister(name, address) should have a sibling ownerRegister(name, address, keys, values) or similar.
There was a problem hiding this comment.
The constructor is missing. Is this on purpose?
|
|
||
| function initialize(bytes32 _parentNode) public initializer { | ||
| PARENT_NODE = _parentNode; | ||
| __Ownable_init(); |
There was a problem hiding this comment.
Is the deployer being the owner desired behavior? If not, we could call a function to set the owner right after this init.
| expect(bobOwner).to.equal(bobSigner.address); | ||
| expect(bobName).to.equal('bob'); | ||
| }); | ||
| it('can use addr to retrieve address of node', async () => { |
No description provided.