fix(ui): make the transactions payload signature cover the time range - #262
Merged
Merged
Conversation
`renderTransactions()` holds the only "is the cached payload still valid?" guard: it compares page, page size and the payload signature. The signature hashed `txTable.filters` (column filters) only, while the list and trend URLs are rendered from a *second* piece of state — the time range (`txRange` / `txCustomStart` / `txCustomEnd`). A range-only change was therefore invisible to the guard, so each of the three range handlers paired the render with an explicit `loadTransactions()`. That patch double-fetches whenever the guard also fires, and `#tx-range` resets the page *before* rendering: from page >= 2, `loadedPage !== page` makes the guard fetch once and return, and then the explicit call fetches the same URL again. Two byte-identical list requests plus two trend requests for a single change of one select. On page 1 only one request goes out, which is why it stayed hidden. Make the signature cover every input that changes the request body: * `txFilterSig()` -> `txQuerySig()`, now hashing the column filters *and* the three range state values. It must read the state values — deriving it from `txRangeParams()` would embed a `now - window` timestamp, changing on every call and turning the guard into a per-render request storm. * `txTable.loadedFilterSig` -> `loadedQuerySig`. * New `reloadTransactions()`: the single reload trigger (resetting the page and reloading are one action). All four controls — type tabs, range quick pick, custom start, custom end — go through it, and the explicit `loadTransactions()` calls are gone from the wiring, so a control change costs at most one request (its own fetch stays guarded by an empty slot, for the first visit and the retry path). * New static gate `state_gate::the_transaction_payload_has_one_signature_and_one_reload_trigger` pins that shape in CI (there is no JS runner there). It judges comment-stripped bodies, and matches the loader by identifier — the two symbols are prefixes of each other (`reloadTransactions()` ends with `loadTransactions()`), so a substring test would red every fixed tree. The sibling control (type tabs) was already fixed in C2112 for exactly this reason; this closes the range half. Tests: `cargo test` 289 -> 290; `cargo fmt --check` clean; clippy reports only the pre-existing `protocol.rs:662`. Verified with `tmp/c2146_probe.js` (jsdom, real scripts, logged `fetch`): 10/10 on this tree, and exactly the `A1`/`B1` axis legs red on the unfixed tree, where the two list requests are byte-identical.
Owner
Author
|
Self-review (author, What the diff does, in three parts
Evidence
Honest limits
|
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.
Summary
The transactions view decides "is the cached payload still valid?" in exactly one
place — the guard in
renderTransactions(), which compares page / page size / thepayload signature. The signature hashed
txTable.filters(column filters) only,while the list and trend URLs are rendered from a second piece of state: the time
range (
txRange/txCustomStart/txCustomEnd). A range-only change wastherefore invisible to the guard, so each of the three range handlers paired the
render with an explicit
loadTransactions().That patch double-fetches whenever the guard also fires, and
#tx-rangeresets thepage before rendering — so from page ≥ 2,
loadedPage !== pagemakes the guardfetch once and return, and then the explicit call fetches the same URL again: two
byte-identical list requests plus two trend requests for one change of one select.
On page 1 only one request goes out, which is why this stayed hidden until now.
Changes
txFilterSig()→txQuerySig(): the payload signature now covers everyinput that changes the request body — the column filters and the three range
state values. It must read the state values; deriving it from
txRangeParams()would embed a
now − windowtimestamp that differs on every call, making thesignature change constantly and turning the guard into a per-render request storm
(worse than the original defect — see the probe numbers below).
txTable.loadedFilterSig→loadedQuerySig(guarded at the guard, written by theloader).
reloadTransactions()— the single reload trigger: resetting the page andreloading are one action. All four controls go through it (type tabs, range quick
pick, custom start, custom end); the explicit
loadTransactions()calls are gonefrom the wiring, so a control change costs at most one request. Its own fetch
stays guarded by an empty slot (first visit / retry path, where the renderer only
draws the degraded state).
state_gate::the_transaction_payload_has_one_signature_and_one_reload_triggerwith four rules, each with its own teeth: the signature reads the range state and
is not derived from
txRangeParams(; the control-wiring function calls no loaderdirectly; the "resets the page + fetches" function is unique and shared by the four
controls; and its explicit fetch stays guarded.
ui/README.mdconvention section;ui/index.htmlcache-bustapp.js?v=…-15.The sibling control (type tabs) was already fixed this way in C2112 for the same
reason — this closes the range half.
Related Issue
None.
Tests
cargo test— 289 → 290 passed, 0 failedcargo fmt --checkcleansrc/state_gate.rs), with a self-verifying extractorA/B (both instruments, run locally)
Static gate (
tmp/c2147_gate_ab.py, mutatingui/js/app.jsin place andrestoring it byte-exactly, md5 verified each leg) — 7 mutation legs, each lighting
up exactly the intended rule and nothing else:
m0m1txRange"m2cols + txRangeParams()(the tempting fix)txRange"m2btxRangeParams()txRangeParams("m3loadTransactions()loadTransactions()itself"m4m5On the unfixed tree (
git show HEAD:ui/js/app.js) the gate is red for the rightreason (
txFilterSigdoes not readtxRange).Runtime probe (
tmp/c2146_probe.js, jsdom booting the realui/index.htmlplusthe four real scripts, stubbing and logging every
fetch, driving the realcontrols) — 10 legs:
A1/B1axis legs red, and the two list requests arebyte-identical; every control leg green on both trees
txRangeParams()) is rejected bythe probe: the request count grows leg over leg (3→3→2→4→5) — a self-feeding storm
Checklist
fix/…)