dappnode / toolkit

1 stars 1 forks source link

Move ENS checks to utils #24

Closed dsimog01 closed 1 year ago

dsimog01 commented 1 year ago

ENS names validation moved to utils and tests added:

The previous functions were marking strings like example*.dappnode.eth

dsimog01 commented 1 year ago
  1. ensureValidDnpName was only checking if the string ends with "dappnode.eth". This is not enough
  2. isEnsDomain is used in directory.ts, but also by the function ensureValidDnpName in file apmRepository.ts. The first check this function should perform is if the string passed corresponds to an ENS domain
  3. These are functions that make sense to be placed in a utils.ts file as they are generic checks/transformations for ENS domains
  4. Now both functions are properly tested with a bunch of valid and invalid ENS domains
dsimog01 commented 1 year ago

@pablomendezroyo I think that it is good to have exported isEnsDomain function as it is used also in the dappmanager, and we could remove duplicate code from there and import it from the toolkit that is already a dependency for the dappmanager

pablomendezroyo commented 1 year ago

Then it should be moved to types dependency and then use it here and in the dappmanager @dsimog01

dsimog01 commented 1 year ago

You are implementing testing for a dependency. Move the tests to the types repo

Fixed @pablomendezroyo