Conversation
…per call (#4527) * Serve the fiat list from a cache instead of rebuilding it per call GET /v1/fiat returns a parameterless, unauthenticated master-data list - the same 7,632 bytes for every caller - and production serves it 3,891 times in six hours (~10.8/min, peaking at 30). Median 19.3 ms, but p95 2,487 ms and 14.3% of calls over a second. Two of the three inputs were already cached: fiat and country both go through findCached. transactionSpecification does not - it extends BaseRepository, so every single call hit the database - and the whole list was mapped again on top. Cache the assembled DTO list rather than its parts, so neither the query nor the mapping repeats. Five minutes is not a free choice: it is CachedRepository's default list TTL, which the underlying fiat and country lists already use. Holding the response longer than its own building blocks stay valid would be wrong. The ETag does not cover this. A conditional request still has to build the full response before the tag can be compared, so a 304 saves bandwidth and no server time. * Move the fiat cache down to the repository instead of the response The previous commit cached the assembled response in the controller. Two problems with that, both found in review: The controller was the only one of 24 AsyncCache sites in the repo not sitting in a service, against CONTRIBUTING's thin-controller rule. Worse, it sat above an invalidation it could not see. FiatService.updatePrice() calls fiatRepo.invalidateCache() after every price update, hourly per fiat, and minVolume/maxVolume are derived from approxPriceChf. A response cache with its own clock would keep serving the old limits for up to five minutes after the price job explicitly asked for them to be dropped. Cache transactionSpecification instead - it was the only uncached input left, so it was the actual per-call database query. fiat and country already go through findCached, and now all three inputs behave the same way, so invalidateCache() keeps taking effect immediately. Switching that repository is safe to do: its only other consumer loads it once into a field of its own, and no save/update/insert/delete path for the table exists anywhere in the codebase. The test pins the part that could regress silently - that the controller reads through the cache rather than around it.
github-actions
Bot
requested review from
TaprootFreak and
davidleomay
as code owners
July 30, 2026 21:46
… EIP-712 domain (#4542) * fix(realunit): accept registration signatures over a chainId-extended EIP-712 domain The BitBox02 firmware warns on typed data whose EIP712Domain has no chainId, and over Bluetooth confirming that warning answers with a NACK instead of a signature (BitBoxSwiss/bitbox02-firmware#2019). iOS reaches the device only over Bluetooth, so BitBox users cannot complete a RealUnit registration at all. The app therefore signs hardware-wallet registrations over a chainId-extended domain, which verification has to accept. Express the accepted set as an enumerated list of variants (domain x field encoding) instead of nesting loops: the cross-product was never visible as a set, so widening or narrowing it was a loop side effect rather than a deliberate edit. Verification now walks that list and returns the variant it recovered under, which forwardRegistration names in the log — making the accepted mix observable, and separating a local resolution miss from an Aktionariat-side rejection, which are otherwise indistinguishable downstream. Backward compatible on its own: the legacy chainId-less domain is tried first, so every existing registration takes the same path to the same result. At most one variant can match, since the shapes differ in the signed digest. The forwarded payload is unchanged and carries no domain, so Aktionariat rebuilds it independently and must accept the extended variant as well. * refactor(realunit): write out the four accepted signature shapes instead of iterating The variant list replaced a nested loop with an array assembled by a conditional spread and then walked by a loop, plus four hand-written labels duplicating what domain and encoding already say. The attempts were still not visible as attempts. Write them out as four expressions instead. recoverRegistration performs one attempt and yields the resolved signature or nothing, so resolveRegistrationSignature is the four shapes chained with ??, readable top to bottom. The log name is derived from the variant rather than stored beside it, so the two cannot drift. Tests assert the four shapes one by one — each verifies, forwards the bytes it was signed over, and is named correctly in the log — replacing the assertion on the list of labels. * fix(realunit): build each candidate message once instead of per attempt Moving each attempt into its own function pushed per-request work into the per-attempt path: the signature was re-normalised four times, and the two candidate messages were built twice each, because both domains recover against the same UTF-8 and ASCII messages. The nested loop this replaced built each message once. Normalise the signature and build both messages once in the resolution, and let the attempt do only what is per-attempt: recover under one domain and compare to the claimed wallet. The four attempts stay written out. A test pins the reuse — it drives the variant that matches on the last attempt and asserts two message builds, so a per-attempt rebuild fails it (verified: the regression produces six). * refactor(realunit): resolve the signature with one guard clause per accepted shape The chained-?? version still carried machinery: an attempt closure returning result objects, an enum-keyed message map, and an undefined-domain guard inside the attempt because the extended domain may not exist. Replace it with four guard clauses, read top to bottom — one statement per accepted shape. Per-request work (signature normalisation, the two candidate messages) happens once above them; the only helper left is a one-line boolean predicate. The missing-chainId case becomes a structural early return between the legacy and extended blocks instead of a per-attempt check. No behavioural change; the message-reuse and four-shape tests pass unchanged. * docs(realunit): trim comments to essentials The rationale lives in the PR description; the code keeps one-liners for what is not derivable from the code itself (the BitBox chainId cause, the unreachable chainId guard, the forward-on-miss fallback). * fix(realunit): PascalCase enum values and cover the environment-derived chainId Enum values follow CONTRIBUTING.md (PascalCase strings, never lowercase); the enum is module-private and never serialised, so no contract changes. The extended domain's chainId is the only environment-dependent part of this change and nothing pinned it — a hardcoded 1 passed the whole suite. Add a DEV case asserting Sepolia resolves and Ethereum does not. * docs(realunit): state the exact condition under which both encodings coincide toBitboxAscii returns input unchanged for pure printable ASCII, not merely for data without diacritics — CJK or Cyrillic carries no diacritics yet transliterates to '?', so the two encodings differ there and the label is exact. * docs(realunit): restore 'printable' in the encoding-coincidence condition toBitboxAscii gates passthrough on /^[\x20-\x7E]*$/, so a tab or newline is pure ASCII yet still transliterates to '?'. The commit message and PR body already said printable; only the comment dropped it.
TaprootFreak
approved these changes
Jul 31, 2026
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
2 participants
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.
Automatic Release PR
This PR was automatically created after changes were pushed to develop.
Commits: 1 new commit(s)
Checklist