Skip to content
This repository has been archived by the owner on Mar 3, 2024. It is now read-only.

Design of BigInt.toHex() and BigInt.toHexString() #20

Closed
fewwwww opened this issue Jun 23, 2023 · 1 comment · Fixed by #38
Closed

Design of BigInt.toHex() and BigInt.toHexString() #20

fewwwww opened this issue Jun 23, 2023 · 1 comment · Fixed by #38

Comments

@fewwwww
Copy link
Collaborator

fewwwww commented Jun 23, 2023

For The Graph's graph-ts original implementation, BigInt.toHex() and BigInt.toHexString() behaves the same.

toHex(): string {
  return typeConversion.bigIntToHex(this);
}

toHexString(): string {
  return typeConversion.bigIntToHex(this);
}

However, in my thoughts toHex() should return a string that looks like deadbeef while toHexString() should return a string that looks like 0xdeadbeef.

So the implementation should be updated into:

toHex(): string {
  return typeConversion.bigIntToHex(this);
}

toHexString(): string {
  return "0x" + typeConversion.bigIntToHex(this);
}

Additionally, toHexString() is not tracked by The Graph's AssemblyScript API docs.

This issue is pending The Graph's opinion.

@fewwwww fewwwww mentioned this issue Jul 11, 2023
@fewwwww fewwwww linked a pull request Jul 11, 2023 that will close this issue
@fewwwww fewwwww closed this as completed Jul 11, 2023
@fewwwww
Copy link
Collaborator Author

fewwwww commented Jul 11, 2023

Discussed in the original graph-tooling/graph-ts repo (graphprotocol/graph-tooling#1389), a backward-compatible version is provided.

toHexString(prefix: string = ''): string {
  if (prefix !== '') {
    return prefix + typeConversion.bigIntToHex(this)
  }
  return typeConversion.bigIntToHex(this);
}

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant