refactor: replace custom hex utils with @noble/hashes#916
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
|
Claude finished @jbojcic1's task in 1m 17s —— View job Investigating sha256 replacement with @noble/hashes
AnalysisCurrent implementation ( export async function computeSHA256(message: string): Promise<string> {
const encoder = new TextEncoder();
const data = encoder.encode(message);
const hashBuffer = await crypto.subtle.digest('SHA-256', data);
const hashArray = Array.from(new Uint8Array(hashBuffer));
const hashHex = hashArray.map((b) => b.toString(16).padStart(2, '0')).join('');
return hashHex;
}@noble/hashes sha256(msg: Input): Uint8Array // Input = string | Uint8ArrayKey differences:
Proposed PlanYes, this is definitely doable. Here's how:
RecommendationOption A (wrapper) is lower risk — it keeps the same function name/import, just changes the signature from Option B (inline) is cleaner long-term but touches more import lines. Either way, it's a straightforward change — maybe ~30 minutes of work. Want me to implement it? |
@claude do you think it's a good idea to do this given that it would turn async action to sync? is it better to have it async (disregard query key case in |
|
Claude finished @jbojcic1's task in 27s —— View job Sync vs Async SHA-256: Analysis
Short answer: sync is the right call here. Switching to Here's why: 1. The inputs are tiny. Every usage in this codebase hashes short strings — quote IDs, token IDs, passwords. We're talking about hashing maybe 32–128 bytes. SHA-256 on inputs this small takes microseconds in pure JS. The overhead of creating a Promise, scheduling a microtask, and awaiting it is likely more expensive than the hash computation itself. 2. 3. Sync simplifies call sites. Removing 4. @noble/hashes is battle-tested. It's already a dependency (used by When would async matter? If you were hashing megabytes of data where blocking the main thread is a real concern. That's not happening here — and if it ever did, you'd want a streaming hash anyway, not TL;DR: For small inputs like IDs and passwords, sync |
|
@claude good, lets implement it then according to your plan. Just make that change as a new commit in claude/issue-626-20260304-1121 branch. |
|
Claude finished @jbojcic1's task in 2m 7s —— View job Replacing
|
| File | Change |
|---|---|
app/lib/sha256.ts |
Rewrote to use @noble/hashes — now sync, 4 lines |
app/features/shared/cashu.ts |
getTokenHash() return type Promise<string> → string |
app/features/shared/spark.ts |
computeSHA256(mnemonic) in queryKey now correctly returns a string instead of a Promise |
app/features/user/auth.ts |
Removed await from computeSHA256 call |
app/features/send/cashu-send-quote-repository.ts |
Replaced Promise.all with sync hash + awaited encrypt |
app/features/send/cashu-send-swap-service.ts |
Removed await from two getTokenHash calls |
app/features/receive/cashu-receive-quote-repository.ts |
Replaced Promise.all with sync hash + awaited encrypt |
app/features/receive/cashu-receive-quote-repository.server.ts |
Replaced Promise.all with sync hash + awaited encrypt |
app/features/receive/cashu-receive-swap-repository.ts |
Removed await from getTokenHash call |
Bonus fix: sparkWalletQueryOptions in spark.ts was putting a Promise<string> into the queryKey array (since computeSHA256 was async but wasn't awaited). Now it correctly puts a string.
|
@claude can you also rebase it on top of latest master so I can merge? |
|
Claude finished @jbojcic1's task in 14s —— View job Rebase requestI'm unable to perform git rebase operations — this is a limitation of my current capabilities (I can only create/push commits and delete files, not run git operations like rebase, merge, or reset). You can rebase locally with: git fetch origin master
git rebase origin/master
git push --force-with-leaseOr if you prefer, GitHub's "Update branch" button on the PR page can merge master into the branch (though that creates a merge commit rather than a rebase). You can find more information and potential workarounds in the FAQ. |
b811ec5 to
6063b16
Compare
@claude what exactly do I need to do to give you rebase permissions? Regarding github button, there is also a button to rebase but for some reason it doesn't create verified commits and then if I use that I can't merge because our repo requires verified commits. Not sure why github commits made that way are not verified but once done by you are done in github action and are verified. |
|
Claude finished @jbojcic1's task in 23s —— View job Rebase permissions & verified commits
On rebase permissions: Unfortunately, this isn't a permissions issue — it's a fundamental limitation of the Claude Code Action. I operate through the GitHub API (creating commits via the API), and the API doesn't expose git rebase as an operation. Rebase requires running actual git commands ( Your best option for this PR: Run locally: git fetch origin master
git checkout claude/issue-626-20260304-1121
git rebase origin/master
git push --force-with-leaseOn verified commits: The reason my commits are verified but GitHub's rebase button produces unverified ones is:
This is a known GitHub limitation. Your options:
|
Remove custom hex conversion functions from lib/utils and use bytesToHex/hexToBytes from @noble/hashes/utils instead — more robust and already a dependency. Closes #626 Co-authored-by: Josip Bojčić <jbojcic1@users.noreply.github.com>
6063b16 to
0313b7a
Compare
Remove custom
uint8ArrayToHexandhexToUint8Arrayfromlib/utilsand usebytesToHex/hexToBytesfrom@noble/hashes/utilsinstead.Closes #626
Generated with Claude Code