performance: bounded replay store, synchronous JS SHA-256 miner, accurate cost docs - #4
Merged
Merged
Conversation
…tible), synchronous JS SHA-256 miner, accurate docs
9 tasks
…gation (CI flake)
Merged
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.
Stacked on #3 (architecture), which is on top of #2 (security). Fixes the performance findings from the review in #1 (P1, P2, P3) and closes S5/S7, which live in the same worker file.
Changes
P1 — bounded replay store (
includes/class-cardea-core.php,uninstall.php): per-comment replay state was one pair ofwp_optionsrows per comment (2 rows each:cardea_used_<sig>+ its timeout). Now a singlecardea_usedoption holding entries{signature, time, window}, capped at 1024 with oldest-first eviction and prune-on-write (each stored entry keeps its own window, so window changes are honored per entry). Loadedautoload=no;delete_option('cardea_used')added touninstall.phpalongside the legacy transient sweep for upgrades.P2 + S5 + S7 — synchronous JS SHA-256 miner (
assets/js/pow-worker.js): replaced thecrypto.subtlebatched loop (1000 digests perPromise.allround,crypto.getRandomValues-free but TLS/secure-context-bound, 100M counter cap that could silently give up) with a compact FIPS 180-4 SHA-256 implemented in JS (32-bit ops, incremental, ~250 lines).crypto.subtledependency → mining works on non-secure (plain HTTP) contexts too (fixes S5), with zero per-digest Promise overhead.hash('sha256', $challengeString . $solution)— the client mines exactly that (tested against PHP-side verification semantics).node:cryptoacross block boundaries).P3 — difficulty guidance (
includes/class-cardea-admin.php): the difficulty field description now carries expected solve times (≈0.1s @3, 1-2s @4, 10-30s @5 on a typical laptop CPU).Tests:
tests/js/pow-worker.test.jsnow imports the real worker (module export underjsdom) instead of mirroring the logic: FIPS KATs,node:cryptocross-checks (lengths 0-1024 incl. 55-char production-shaped challenges, block boundaries 63/64/65/119/120), difficulty check, mining at d=1..4 with server-side verification semantics, determinism, and the workerpostMessageinterface.WP 7.1 compatibility found in-flight: WordPress 7.1 has removed
set_option()(verified viafunction_existsinside a live WP 7.1 Playground:get_option/update_option/add_option/delete_optionexist,set_optiondoes not). The store therefore writes viaupdate_option()(the current API; same 3-arg signature, present in all supported WP versions).Pushback (per #1)
challenge . counteras sent) — no benefit, regression risk. The real win is removing per-digest Promise/batch overhead with a synchronous miner, which is what this does.wp-crondependency, no per-token rows, bounded memory, per-entry window semantics preserved.Benchmark (Node/V8, 55-char challenge)
crypto.subtlebatched path (as-is)≈2.1× faster end-to-end in Node; browser numbers expected to trend the same direction (and the new miner is the only option on plain-HTTP sites). Browser measurements will be added in follow-up if useful.
Test evidence
Full suite green (Docker,
make test+make lint):node:cryptocross-checks)