Repository navigation
feat(hash): add setHashAsync to hash a field off the event loop - #110
Merged
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
⚠ sensitive: password hashing.
Why
A password write should not stall the server.
@HashedrunsscryptSyncinside the field setter, so every assignment blocks the event loop for about 150 ms (ANT-1932 measured 144 to 162 ms). The setter has to stay synchronous, so this adds an async way to set the field. Antony approved the API on 2026-11-10 (ANT-1960).What changes for a module author
await row.setHashAsync(field, value). It runs the scrypt run on the libuv thread pool, then assigns the finished hash through the field's own setter. It uses the same parameters (ln=15, r=8, p=3, 16-byte salt, 32-byte key) and the same stored format$scrypt$ln=15,r=8,p=3$<salt>$<key>. A hash written either way verifies withtestHashandtestHashAsync, and existing rows still verify.insertorupdate. A row instance goes through the model unconverted, so the hash is kept.testHash,testHashAsyncandisHashOutdatedare unchanged. Plain-datainsert({ password })andupdate(id, { password })still hash on the event loop. The docs say so and point tosetHashAsync.setHashAsyncbehaves like an assignment in these cases: on a field with analgorithm(a digest is fast), on a field with another modifier beside@Hashed, forundefined, and when given the stored hash back.flowchart LR A["row.setHashAsync(field, value)"] --> B{"@Hashed alone,<br/>scrypt, new value?"} B -- yes --> C["scrypt on the thread pool"] --> D["PrecomputedHash(stored)"] B -- no --> E["value as is"] D --> F["row[field] = ... (same setter)"] E --> F F --> G["HashModifier.lock: keeps a PrecomputedHash,<br/>otherwise hashes as before"]PrecomputedHashis private tohash.ts, and so islockAsync. Neither appears in the published declarations except asprivate lockAsync;. Data from a request or from the database cannot be an instance of that class, so it cannot skip hashing.Proof
The event-loop test fails on main and passes after
src/tests/modifiers/hash_async_set.test.tsmeasures the longest gap between 1 ms timers while a hash runs. A sync assignment calibrates it, and the async path must stay below half of that.On
origin/mainwith only the new test file added:Against a wrong implementation that is async but still hashes on the main thread (built
lockAsyncpatched to return the raw value):Against the first review round's code (empty value, and a
Localizedfield held floating):At head:
src/tests/hash_upgrade.test.tsadds two MongoDB cases:setHashAsyncfollowed by a whole-rowupdate, andsetHashAsyncfollowed byinsert. Both pass.Load, before and after
ANT-1932: 10 connections on a cheap route next to one serial signup stream, sync scrypt against async scrypt with the same parameters.
scryptSync, today)Reproduced on this branch's build: 16 cores,
autocannon -c 10 -d 10on anhttproute answeringok, with one serial loop ofnew Account()plus a@Hashedwrite in the same process, 3 rounds each.row.password = pwawait row.setHashAsync("password", pw)The sync row here is lower than ANT-1932's because this loop starts the next write as soon as the last one ends. It holds the main thread for about 76 × 150 ms of each 10 s run, which makes it a worst case. The async row stays within 6 % of no writes.
pnpm run checksVerify layer
An independent verifier ran
antelopejs-verifyonc925386. The result was## Verify: PASS+NOTESwithOne-way-door: format, contract. All 17 journeys in smoke, declarations and hashing passed. The format door is a false positive: an independentnode:cryptoscrypt reproduced the stored key. The contract door is the addedsetHashAsync, which ships as a new minor. The notes saidlockAsyncwas public, and the last commit makes it private. They also noted that no harness journey callssetHashAsyncyet. The test suite and a probe of the build covered it instead.Review
An independent reviewer (claude-fable-5-1) ran two rounds. Round 1 found three verified defects, all fixed in
c925386:Localizedfield held floating returned the internal wrapper.Encryptedmodifier under@Hashedre-hashed on the event loop.Round 2 had no verified findings.
Release
Needs a minor release,
0.3.0, because of the new public method. The release commit sets the version.Follow-ups
setHashAsynconce this is released (ANT-1960 Done means).@Hashedstores a hash of the wrapped value in both write paths, sotestHashfails for that field. It deserves its own issue.setHashAsyncjourney.