search

ethpm / ethpm-spec

Ethereum Package Manager http://ethpm.github.io/ethpm-spec/
165 stars 30 forks source link

fix: spec name sizes exceed spec maximum of 255 #169

Closed fubuloubu closed 2 years ago

fubuloubu commented 2 years ago

Working with the ethPM specification, I noticed that the EIP specification maximum of 255 characters for certain identifiers was not being respected by the schema specification

I've attempted to update all of the relevant locations where this violation was noticed (namely when there was a pattern to detect the first character of an identifier separately from the remaining characters)

fubuloubu commented 2 years ago

Technically, only the PackageName is violating the specification, as mentioned the maximum number of characters for the name field in the manifest should be 255 characters, but then the regular expression used in the schema is ^[a-zA-Z][-_a-zA-Z0-9]{0,255}$ which will match a maximum of 256 characters.

All the rest of the fields do not specify a maximum for the field, relying on the regular expression being correct.

Another alternative outcome would be to correct the EIP if this field should be 256 characters maximum and not 255

njgheorghita commented 2 years ago

Good catch, I guess tests are only useful when they're written with the correct constraints. I lean towards using the written specification as the canonical source of truth for ethpm, so I'd rather not update the EIP to 256 characters, and merge this solution instead.