Normalize labelhash for ENSRainbow Heal request#347
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
| ).rejects.toThrow("Invalid labelhash length 65 characters (expected 66)"); | ||
| labelByHash("0x0ca5d0b4ef1129e04bfe7d35ac9def2f4f91daeb202cbe6e613f1dd17b2da0"), | ||
| ).rejects.toThrow( | ||
| 'Error healing labelhash: "0x0ca5d0b4ef1129e04bfe7d35ac9def2f4f91daeb202cbe6e613f1dd17b2da0". Error (400): Invalid labelhash length: expected 32 bytes (64 hex chars), got 31 bytes: 0x0ca5d0b4ef1129e04bfe7d35ac9def2f4f91daeb202cbe6e613f1dd17b2da0.', |
There was a problem hiding this comment.
should we make these errors non redundant?
There was a problem hiding this comment.
Sorry I don't understand the question. Please share more details.
There was a problem hiding this comment.
E.g. labelhash is written twice in this error. I think it should be just:
Error (400): Invalid labelhash length: expected 32 bytes (64 hex chars), got 31 bytes: 0x0ca5d0b4ef1129e04bfe7d35ac9def2f4f91daeb202cbe6e613f1dd17b2da0.
lightwalker-eth
left a comment
lightwalker-eth
left a comment
There was a problem hiding this comment.
@djstrong Thanks for updates 👍 Reviewed and shared feedback
| count(): Promise<CountResponse>; | ||
|
|
||
| heal(labelhash: Labelhash): Promise<HealResponse>; | ||
| heal(labelhash: Labelhash | string): Promise<HealResponse>; |
There was a problem hiding this comment.
Please define a type for EncodedLabelhash
There was a problem hiding this comment.
I have created it for parseEncodedLabelhash but string needs to stay here because we accept unnormalized labelhashes.
| ).rejects.toThrow("Invalid labelhash length 65 characters (expected 66)"); | ||
| labelByHash("0x0ca5d0b4ef1129e04bfe7d35ac9def2f4f91daeb202cbe6e613f1dd17b2da0"), | ||
| ).rejects.toThrow( | ||
| 'Error healing labelhash: "0x0ca5d0b4ef1129e04bfe7d35ac9def2f4f91daeb202cbe6e613f1dd17b2da0". Error (400): Invalid labelhash length: expected 32 bytes (64 hex chars), got 31 bytes: 0x0ca5d0b4ef1129e04bfe7d35ac9def2f4f91daeb202cbe6e613f1dd17b2da0.', |
There was a problem hiding this comment.
Sorry I don't understand the question. Please share more details.
|
next steps: merge main & re-review |
| * @returns A normalized labelhash (a 0x-prefixed, lowercased, 64-character hex string) | ||
| * @throws {InvalidLabelhashError} If the input cannot be normalized to a valid labelhash | ||
| */ | ||
| export function parseLabelhash(maybeLabelhash: string): Labelhash { |
There was a problem hiding this comment.
imo we should refactor this entire file to just:
function parseLabelhashOrEncodedLabelhash(maybeLabelHash: string) {
// 1. ensure is Hex formatted
let hexLabelhash: Hex;
try {
// attempt to decode label hash with ensjs
// NOTE: decodeLabelhash will throw InvalidEncodedLabelError or return hex-looking string
hexLabelhash = decodeLabelhash(maybeLabelHash);
} catch {
// if not encoded label hash, prefix with 0x
hexLabelhash = maybeLabelHash.startsWith("0x")
? (maybeLabelHash as `0x${string}`)
: `0x${maybeLabelHash}`;
}
// 2. if not hex, throw
if (!isHex(hexLabelhash, { strict: true })) throw new InvalidLabelhashError();
// 3. ensure hex part is correctly sized, padding left
const normalizedHex = pad(hexLabelhash, { dir: "left", size: 32 });
if (size(normalizedHex) !== 32) throw new InvalidLabelhashError();
// 4. return lower case string
return normalizedHex.toLowerCase() as Labelhash;
}There was a problem hiding this comment.
if ensjs isn't already a dependency, we can keep parseEncodedLabelHash in place of ensjs#decodeLabelHash except make sure it returns 0x${maybeEncodedLabelhash.slice(1, -1)} not the raw hex characters
There was a problem hiding this comment.
and the logic can be separated into multiple functions if desired. but we shouldn't be re-implementing hex-related logic that's already available in viem
There was a problem hiding this comment.
We don't have ensjs.
pad(hexLabelhash, { dir: "left", size: 32 }) is not working correctly, e.g. it validates positively 0x00000.
There was a problem hiding this comment.
I have updated the code to use isHex
There was a problem hiding this comment.
Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.
Comments suppressed due to low confidence (2)
packages/ensrainbow-sdk/src/utils.ts:28
- Consider adding an explicit check for an empty hex string (after stripping the '0x' prefix) to provide a clearer error message when the input is empty.
if (!/^[0-9a-fA-F]*$/.test(hexPart)) {
apps/ensindexer/src/lib/graphnode-helpers.ts:40
- Ensure that the error message format is consistent with other modules to aid debugging and align with test expectations.
throw new Error(`Error (${healResponse.errorCode}): ${healResponse.error}.`);
…h to provide clearer feedback
…h to accurately reflect byte count
3 participants
#211