Roles 0.1.0/decenthats contract #2102
Conversation
✅ Deploy Preview for decent-interface-dev ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
adamgall
force-pushed
the
roles-0.1.0/decenthats-contract
branch
from
8ee7f6a to
ef46722
Compare
| hatsProtocol: '0x3bc1A0Ad72417f2d411118085256fC53CBdDd137', | ||
| }, | ||
| constants: { | ||
| ha75Address: '0x0000000000000000000000000000000000004a75', |
There was a problem hiding this comment.
Is it really reasonable to put it on network config if that's a constant for each network? Is there any reason to assume it would be different for some network in the future? Just don't wanna duplicate stuff if there's no reason in foreseeable future
There was a problem hiding this comment.
Yeah true, i'm sure they'll be the same in all networks for the forseeable future.
Where are you suggesting the constant is defined?
| @@ -47,6 +47,11 @@ export type NetworkConfig = { | |||
| erc721FreezeVotingMasterCopy: string; | |||
| votesERC20WrapperMasterCopy: string; | |||
| keyValuePairs: string; | |||
| decentHatsMasterCopy: string; | |||
| hatsProtocol: string; | |||
There was a problem hiding this comment.
Why not specifying this as Address type given we're putting that as a constant string? From what I'm getting it won't require getAddress call on network config nor as const
There was a problem hiding this comment.
Yeah really just to keep things consistent. On the remove typechain branch these are typed properly.
| hatsProtocol: string; | ||
| }; | ||
| constants: { | ||
| ha75Address: string; |
There was a problem hiding this comment.
Same here (if we wanna keep it) - why not specifying this as Address type?
Closes #2099