feat(core): improve hex utils

[phroi]

• I have read the Contributing Guidelines

[netlify]

✅ Deploy Preview for liveccc ready!

Name	Link
🔨 Latest commit	565c4bb
🔍 Latest deploy log	https://app.netlify.com/projects/liveccc/deploys/68c7c29e150a2700086ae541
😎 Deploy Preview	https://deploy-preview-268--liveccc.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.
Lighthouse	1 paths audited Performance: 38 (🔴 down 3 from production) Accessibility: 88 (no change from production) Best Practices: 92 (🔴 down 8 from production) SEO: 100 (no change from production) PWA: - View the detailed breakdown and full score reports

---

To edit notification comments on pull requests, go to your Netlify project configuration.

[netlify]

✅ Deploy Preview for apiccc ready!

Name	Link
🔨 Latest commit	565c4bb
🔍 Latest deploy log	https://app.netlify.com/projects/apiccc/deploys/68c7c29e16a6e50008782db9
😎 Deploy Preview	https://deploy-preview-268--apiccc.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.
Lighthouse	1 paths audited Performance: 87 (🟢 up 1 from production) Accessibility: 100 (no change from production) Best Practices: 100 (no change from production) SEO: 94 (no change from production) PWA: - View the detailed breakdown and full score reports

---

To edit notification comments on pull requests, go to your Netlify project configuration.

[netlify]

✅ Deploy Preview for appccc ready!

Name	Link
🔨 Latest commit	565c4bb
🔍 Latest deploy log	https://app.netlify.com/projects/appccc/deploys/68c7c29e7c3a7b00084bbda4
😎 Deploy Preview	https://deploy-preview-268--appccc.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.
Lighthouse	1 paths audited Performance: 43 (🔴 down 2 from production) Accessibility: 89 (🟢 up 1 from production) Best Practices: 92 (🔴 down 8 from production) SEO: 100 (no change from production) PWA: - View the detailed breakdown and full score reports

---

To edit notification comments on pull requests, go to your Netlify project configuration.

[gemini-code-assist]

Summary of Changes

Hello @phroi, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request introduces a significant enhancement to the core hexFrom utility, allowing it to convert Entity objects and bigint values into hexadecimal strings. This change improves the flexibility and usability of the hex conversion module by supporting more data types directly. Additionally, a new isHex function has been added to provide reliable validation for hexadecimal string formats.

Highlights

• Expanded hexFrom Functionality: The hexFrom utility function has been significantly enhanced to accept Entity objects and bigint values as input, in addition to the existing HexLike types. This broadens the utility of the function for various data representations.
• New isHex Validation Utility: A new isHex function has been introduced to provide robust validation for hexadecimal strings. This function ensures that a string adheres to the Hex format, checking for the '0x' prefix, even length, and valid hexadecimal characters.
• Improved Type Handling and Conversion Logic: The internal logic of hexFrom now includes specific handling for bigint values, converting them directly to their hexadecimal string representation. For Entity objects, the function leverages their toBytes() method to obtain a byte array before conversion.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in issue comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[gemini-code-assist]

Code Review

This pull request extends the hexFrom utility to support conversion from Entity and bigint types. A new isHex helper function is also introduced to validate hex strings. The changes are generally good, but I've found a significant issue with the bigint conversion logic in hexFrom. It can produce malformed or inconsistent hex strings, especially for values that result in an odd number of hex digits and for negative numbers. My review includes suggestions to fix this to ensure the function is robust and its behavior is consistent.

[netlify]

✅ Deploy Preview for docsccc ready!

Name	Link
🔨 Latest commit	565c4bb
🔍 Latest deploy log	https://app.netlify.com/projects/docsccc/deploys/68c7c29e30c19e00086d3f3e
😎 Deploy Preview	https://deploy-preview-268--docsccc.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.
Lighthouse	1 paths audited Performance: 66 (🟢 up 19 from production) Accessibility: 88 (no change from production) Best Practices: 92 (no change from production) SEO: 92 (no change from production) PWA: - View the detailed breakdown and full score reports

---

To edit notification comments on pull requests, go to your Netlify project configuration.

[phroi]

My main point is that a bigint should always encode to the exact same Hex given the same encoding.

I wonder if the following holds true for all v in bigint:

numToHex(v) === `0x${bytesTo(numBeToBytes(v), "hex")}`

In the upcoming days I'll get back to this issue: I'll write some tests and maybe I'll integrate the benchmark too, to check if the hex string pass-thru makes sense or not.

Phroi %117

[Hanssen0]

My main point is that a bigint should always encode to the exact same Hex given the same encoding.

I wonder if the following holds true for all v in bigint:

numToHex(v) === `0x${bytesTo(numBeToBytes(v), "hex")}`

I don't think they are the same. Bytes is always more strict than Hex.

An obvious example is negative numbers, -0xa is valid, while we don't have the negative sign for Bytes, so we have to represent the negative number by other ways (usually two's complement). That's why we have both numToHex and numToBytes.

Bytes is always aligned to 8 bits, while Hex doesn't have to.

[phroi]

An obvious example is negative numbers, -0xa is valid

/**
 * Represents a hexadecimal string prefixed with "0x".
 * @public
 */
export type Hex = `0x${string}`;

Nope, the definition of ccc.Hex doesn't allow for -0xa. Additionally, numToHex doesn't handle negative numbers correctly:

console.log(numToHex(-1n)) // Prints: '0x-1'

While 0x-1 would be allowed by ccc.Hex, 0x-1 is not a valid hexadecimal literal in any common language nor notation.

we have to represent the negative number by other ways (usually two's complement).

Agreed, the standard way of handling negative numbers is to use two's complement with numLeToBytes/numBeToBytes and then serialize them to ccc.Hex, so 0x${bytesTo(numBeToBytes(v, bytes), "hex")}.

That's why we have both numToHex and numToBytes.

A couple issues:

• For some x, hexFrom(numToHex(x)) != numToHex(x) comes as a surprise to devs.
• numToHex is not well defined for negative numbers.

Phroi %32

[Hanssen0]

An obvious example is negative numbers, -0xa is valid

/**
 * Represents a hexadecimal string prefixed with "0x".
 * @public
 */
export type Hex = `0x${string}`;

Nope, the definition of ccc.Hex doesn't allow for -0xa. Additionally, numToHex doesn't handle negative numbers correctly:

console.log(numToHex(-1n)) // Prints: '0x-1'

While 0x-1 would be allowed by ccc.Hex, 0x-1 is not a valid hexadecimal literal in any common language nor notation.

• numToHex is not well defined for negative numbers.

Yes. I consider these to be all bugs due to a lack of support for negative numbers.

we have to represent the negative number by other ways (usually two's complement).

Agreed, the standard way of handling negative numbers is to use two's complement with numLeToBytes/numBeToBytes and then serialize them to ccc.Hex, so 0x${bytesTo(numBeToBytes(v, bytes), "hex")}.

That's why we have both numToHex and numToBytes.

A couple issues:

• For some x, numToHex(hexFrom(x)) != hexFrom(x) comes as a surprise to devs.

The first idea that came into my mind was "So we should invoke numToHex in hexFrom". But soon I realized the reason why I didn't add ccc.Num or ccc.NumLike to ccc.HexLike or ccc.BytesLike - circular reference. Using bigint is a good idea to bypass this restriction, but I'm afraid that it didn't work for Entity.

I'm unsure which environments will be impacted by circular references, because all the apps we've built are fine with that. But I received some reports of:

Error: Cannot access 'Entity' before initialization

and fixed by #257 .

[Hanssen0]

Yes. I consider these to be all bugs due to a lack of support for negative numbers.

Edit: Though, maybe we should forbid -0x1 by restricting this in numToHex / hexFrom. It's mathematically / theoretically correct that -a or -0xa is valid, but that also makes the type system over-complicated. It will be annoying if we need to deal with the negative sign while working on Script.args (or maybe won't as long as devs don't manipulate the hex string directly, but after bytesFrom).

[phroi]

Too sleepy! Tomorrow I'll think about it, but I'll not be able to code for a while, night night 🤗

[phroi]

In these days I thought about the issue and in my opinion we need to update numToHex from:

export function numToHex(val: NumLike): Hex {
  return `0x${numFrom(val).toString(16)}`;
}

to:

export function numToHex(val: NumLike): Hex {
  const v = numFrom(val);
  if (v < ccc.Zero) {
    throw new Error("Negative number detected, please use numBeToBytes")
  }
  let h = v.toString(16);
  return h.length % 2 === 0 ? `0x${h}` : `0x0${h}`;
}

@Hanssen0 please do not hate me too much 🤣

[Hanssen0]

In these days I thought about the issue and in my opinion we need to update numToHex from:

export function numToHex(val: NumLike): Hex {
  return `0x${numFrom(val).toString(16)}`;
}

to:

export function numToHex(val: NumLike): Hex {
  const v = numFrom(val);
  if (v < ccc.Zero) {
    throw new Error("Negative number detected, please use numBeToBytes")
  }
  let h = v.toString(16);
  return h.length % 2 === 0 ? `0x${h}` : `0x0${h}`;
}

@Hanssen0 please do not hate me too much 🤣

It's exactly what I'm thinking, that's the best we can do so far. In the current CCC, Hex and Bytes are used interchangeably. I doubt that's a good idea, as I don't think manipulating hex strings is better than manipulating bytes.

• bytes.length is better than hex.length / 2 - 1.
• bytes[0] is better than hex.slice(2, 4).
• bytes[1] = 2 is better than ${hex.slice(0, 4)}02${hex.slice(6)}

Readability of console.log(bytes) should be able to be solved by overriding toString.

In the next major version, let's let them separate a little more. I'm not sure what will be left to be Hex, though (maybe some RPC interfaces), mostly should be replaced by Num or Bytes.

[phroi]

It's exactly what I'm thinking, that's the best we can do so far. In the current CCC, Hex and Bytes are used interchangeably.

Should I open a new PR for this or bump this one as major/minor?

I don't think manipulating hex strings is better than manipulating bytes.

• bytes.length is better than hex.length / 2 - 1.
• bytes[0] is better than hex.slice(2, 4).
• bytes[1] = 2 is better than ${hex.slice(0, 4)}02${hex.slice(6)}

There are drawbacks too:

1. Mutability, which is undesirable most of the time
2. Cannot be used directly as keys in Map/Objects
3. No automatic interning by V8
4. Slow comparison compared to interned strings

[Hanssen0]

It's exactly what I'm thinking, that's the best we can do so far. In the current CCC, Hex and Bytes are used interchangeably.

Should I open a new PR for this or bump this one as major/minor?

We shouldn't do that right now. Let's wait until there are actual needs.

I don't think manipulating hex strings is better than manipulating bytes.

• bytes.length is better than hex.length / 2 - 1.
• bytes[0] is better than hex.slice(2, 4).
• bytes[1] = 2 is better than ${hex.slice(0, 4)}02${hex.slice(6)}

There are drawbacks too:

1. Mutability, which is undesirable most of the time
2. Cannot be used directly as keys in Map/Objects
3. No automatic interning by V8
4. Slow comparison compared to interned strings

Agree. That is worth more discussion. Maybe bytesFrom(hex) is just better.

[phroi]

We shouldn't do that right now. Let's wait until there are actual needs.

The only need is gonna ever appear is when devs stumble in this in a form of a difficult-to-trace bug.

That said, if we modify the behavior of numToHex new bugs may appear in people code, which is undesirable.

That's why I'm also open to deprecate the numToHex and encourage people to move over to numBeToBytes, possibly optimizing it if achievable.

@Hanssen0 what would you say about deprecating numToHex?

[Hanssen0]

We shouldn't do that right now. Let's wait until there are actual needs.

The only need is gonna ever appear is when devs stumble in this in a form of a difficult-to-trace bug.

That said, if we modify the behavior of numToHex new bugs may appear in people code, which is undesirable.

That's why I'm also open to deprecate the numToHex and encourage people to move over to numBeToBytes, possibly optimizing it if achievable.

@Hanssen0 what would you say about deprecating numToHex?

I think I may have misunderstood what you meant, so let me rephrase it.

1. I think we shouldn't change Hex to Bytes right now.
2. Modification for numToHex is fine, it should be a patch change since it doesn't introduce API changes, and can be done in this PR.
3. I don't think we should deprecate numToHex.

[phroi]

The first idea that came into my mind was "So we should invoke numToHex in hexFrom". But soon I realized the reason why I didn't add ccc.Num or ccc.NumLike to ccc.HexLike or ccc.BytesLike - circular reference. Using bigint is a good idea to bypass this restriction, but I'm afraid that it didn't work for Entity.

I'm unsure which environments will be impacted by circular references, because all the apps we've built are fine with that. But I received some reports of:

Error: Cannot access 'Entity' before initialization

and fixed by #257 .

I was wondering:

• Exactly which circular references we want to avoid? (file<->file, type<->type...)
• How can I check automatically for circular references?

Phroi %3

[Hanssen0]

I was wondering:

• Exactly which circular references we want to avoid? (file<->file, type<->type...)

Good question. It seems that only implementation references will cause problems, while type references won't, e.g.

ccc/packages/core/src/ckb/transaction.ts

Line 2 in 698df9a

import type { ClientCollectableSearchKeyFilterLike } from "../client/clientTypes.advanced.js";

• How can I check automatically for circular references?

I have no idea, otherwise I would fix that sooner.

[phroi]

Let me express some thoughts:

bigint <-> ccc.Hex is troublesome as it's not bijective:

• numToHex zero padding thing
• Negatives cannot be directly represented
• numFrom(0x) === 0n but also numFrom(0x00) === 0n

So I was thinking about not adding bigint to hexFrom conversion, I'd rather have devs take one second more to think about the encoding they need to use.

Let me ask again, should I still change numToHex behavior?

About Entity, I was thinking, maybe I can try add Entity to BytesLike?
And so HexLike since type HexLike = BytesLike.

Phroi %13

[Hanssen0]

Let me express some thoughts:

bigint <-> ccc.Hex is troublesome as it's not bijective:

• numToHex zero padding thing
• Negatives cannot be directly represented
• numFrom(0x) === 0n but also numFrom(0x00) === 0n

So I was thinking about not adding bigint to hexFrom conversion, I'd rather have devs take one second more to think about the encoding they need to use.

Let me ask again, should I still change numToHex behavior?

If you like, I think we should fix numToHex (to something similar to numToBytes).

About Entity, I was thinking, maybe I can try add Entity to BytesLike? And so HexLike since type HexLike = BytesLike.

Of course, as long as it doesn't introduce a circular reference problem (by importing only type).

[changeset-bot]

🦋 Changeset detected

Latest commit: 565c4bb

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 18 packages

Name	Type
@ckb-ccc/core	Patch
@ckb-ccc/eip6963	Patch
@ckb-ccc/joy-id	Patch
@ckb-ccc/lumos-patches	Patch
@ckb-ccc/nip07	Patch
@ckb-ccc/okx	Patch
@ckb-ccc/rei	Patch
@ckb-ccc/shell	Patch
@ckb-ccc/spore	Patch
@ckb-ccc/ssri	Patch
@ckb-ccc/udt	Patch
@ckb-ccc/uni-sat	Patch
@ckb-ccc/utxo-global	Patch
@ckb-ccc/xverse	Patch
@ckb-ccc/ccc	Patch
ckb-ccc	Patch
@ckb-ccc/connector	Patch
@ckb-ccc/connector-react	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[phroi]

/gemini review

[gemini-code-assist]

Code Review

This pull request introduces valuable improvements to the hex utility functions. The new isHex function provides robust validation for hex strings, and its use in hexFrom is a good performance optimization. The changes in numToHex to enforce hex normalization and add a check for negative values make the function more reliable. I've provided a couple of suggestions to further refine the code: one to improve conciseness in isHex using a regular expression, and another to reduce module dependencies in numToHex. Overall, these are solid enhancements.

[phroi]

I restricted myself to the minimal stuff that made sense. I tried more things, but Circular references were tricky to avoid.

@Hanssen0 what's your take on these minimal changes?

Love & Peace, Phroi %73

[Hanssen0]

I restricted myself to the minimal stuff that made sense. I tried more things, but Circular references were tricky to avoid.

@Hanssen0 what's your take on these minimal changes?

LGTM

I think we should be able to safely import type { Entity } here without causing circular references. I hope that we can find a way to write some unit tests for this. I'm not sure if https://github.com/ckb-devrel/ccc/tree/master/packages/tests helps.

[phroi]

I think we should be able to safely import type { Entity } here without causing circular references

Wouldn't it make more sense to have Entity as additional ByteLike option?

Isssue is that Entity.eq uses internally bytesEq.

Not sure it's worth the additional hassle, in the end we are just trying to transform:

hexFrom(e.toBytes())

into:

hexFrom(e)

I hope that we can find a way to write some unit tests for this

How about the benchmark that I used for testing Script.eq speedup improvement? Would that work for testing for circular references?

Love & Peace, Phroi %41

[phroi]

/gemini review

[phroi]

What if instead I added on Entity a toHex method?

[phroi]

Doing the Epoch utils implementation and came to my mind, this wouldn't add circular deps

[phroi]

Maybe I'll just add it there, cause I'd like to use it

[Hanssen0]

LGTM

[Hanssen0]

Wouldn't it make more sense to have Entity as additional ByteLike option?

Sure

Isssue is that Entity.eq uses internally bytesEq.

Not sure it's worth the additional hassle, in the end we are just trying to transform:

hexFrom(e.toBytes())

into:

hexFrom(e)

I think this will make developers happier. It is actually quite annoying to write a call once before and once after.

What if instead I added on Entity a toHex method?

I like both, and I'm fine with both. After all, adding them both won't cause any problems. You can decide whichever approach you prefer, as long as it doesn't cause any problems.

Thanks for your contribution! I think the current code is good, and feel free to let me know once you have the parts you want merged ready.

[phroi]

What if instead I added on Entity a toHex method?

I like both, and I'm fine with both. After all, adding them both won't cause any problems. You can decide whichever approach you prefer, as long as it doesn't cause any problems.

Added toHex in #314

Phroi %10