Skip to content

Address attestation QA findings - #6247

Merged
j0ntz merged 4 commits into
developfrom
jon/attestation-qa-findings
Oct 10, 2026
Merged

j0ntz merged 4 commits into
developfrom
jon/attestation-qa-findings

Conversation

@j0ntz

@j0ntz j0ntz commented Oct 7, 2026 •

Copy link
Copy Markdown
Contributor

Description

Asana task

Fixes the failures QA reported against the attestation and key-delivery test builds.

  • No clock warning on some launches. The wrong-clock warning ran only during an attestation handshake. A launch with no handshake never warned. A 401 on the signed key fetch now feeds its Date header to the same check.
  • Paybis and Simplex missing from buy and sell. A key fetch sent before the attestation token arrived cached the lower key tier for the next launch. A token that arrives after such a fetch triggers a refetch and cache write, tried again on the next token if it fails. The cache records whether its request carried a token, and an unattested answer does not replace an attested entry younger than three days.
  • No exchange rates with the device clock ahead. The rates server stamps current rates with its own clock. The app compared that stamp to the device clock, and a clock more than an hour ahead filed every current rate as historical. Rate age is judged against the server's Date header.
  • Export Logs failing with "status 400" under a wrong clock. The logs server refuses logs dated more than five minutes off its clock. The app recognizes that refusal and tells the user to fix the device clock.

The rates and Export Logs defects predate the attestation work and behave the same on master. QA found them while testing the clock warning.

Testing

  • Unit tests: keysStore, keysServer, attestation, ExchangeRateActions (getServerNow), and a new LogActions suite.
  • Android emulator, debug build, clock set two days ahead:
    • Cold launch with the handshake warning silenced locally: the refused key fetch raised the warning.
    • Clock changed while the app was alive: the warning appeared at the next handshake attempt.
    • Logged in with the clock ahead: the wallet list shows fiat values.
    • Settings > Export Logs > Send to Edge: the alert shows the clock message.
  • The late-token refetch was driven with a stand-in token handed to the handler locally. No made-up token was sent to a server. With the network off the refetch failed and the cache stayed as it was. The next delivery refetched and wrote the cache.
  • Cache hold, same emulator, cache entry seeded on disk, no token on any launch:
    • Attested entry one day old: served and left in place across two launches.
    • Attested entry four days old, an entry without the attested field, and an entry stamped four days in the future: each replaced by the unattested answer.

Not covered

  • The emulator cannot pass hardware attestation, and the hardware key tier was never delivered on device. Paybis and Simplex need a recheck on a phone. The listener ending its wait after an attested answer is cached has unit tests only.
  • With the clock ahead, the 24h change reads 0%: the rates server does not answer a request for a future date.
  • A clock changed mid-session warns at the next handshake. On a phone with a live token that is the next token refresh.
  • The region picker, reported in the same QA round, is not changed by this PR, and CountryListModal.tsx is identical to master. With no saved region it lists the phone's region setting first and waits for a pick. It has never selected a country automatically, so the QA step that expected that was corrected.

CHANGELOG

Does this branch warrant an entry to the CHANGELOG?

  • Yes
  • No

Dependencies

none

Requirements

If you have made any visual changes to the GUI. Make sure you have:

  • Tested on iOS device
  • Tested on Android device
  • Tested on small-screen device (iPod Touch)
  • Tested on large-screen device (tablet)

Test evidence

3e760df
Refresh cached keys when a token arrives late
2026-10-07
🪓 silenced the two handshake clock checks locally so only the refused key fetch could raise the warning

1. 🪓 clock warning from refused key fetch

2. rates shown with clock two days ahead

3. export logs clock message with clock ahead

4. clock warning in live session after clock jump

Note

Medium Risk
Changes boot-time key caching and partner plugin availability, plus shared clock-skew UX; mistakes could affect buy/sell providers or first-launch key tiers, though runtime keys are not swapped mid-session.

Overview
Fixes QA issues around wrong device clocks and slow or late device attestation affecting partner keys, rates, and support tooling.

Attestation / remote keys: When the initial key fetch runs without a token, the app now subscribes for a late attestation token and refetches once, caching the attested payload for the next launch without hot-swapping running keys. Cache entries record attested and fetchedAt; unattested answers no longer overwrite a recent attested cache (3-day hold). A 401 on the signed key fetch passes the response Date into the existing clock-skew warning so launches that never run a handshake still alert the user.

Exchange rates: Current vs historical rates are aged against the rates server clock from the Date header (getServerNow), so a device clock set far ahead no longer hides all fiat values.

Export logs: A 400 with body Time Out of Sync surfaces a dedicated LogsClockError message instead of a generic status error.

Docs (CONFIG_KEYS_ARCHITECTURE.md), types, locales, and unit tests cover the new cache rules, errors, and behaviors.

Reviewed by Cursor Bugbot for commit a211ee9. Bugbot is set up for automated code reviews on this repo. Configure here.

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@peachbits peachbits left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Non-blocking: the CHANGELOG entries for the first three commits all landed in the last one, and the enUS.json string landed in the rates commit while its en_US.ts source is in the logs commit.

Comment thread src/util/keysStore.ts Outdated
Comment thread src/util/keysStore.ts
Comment thread docs/CONFIG_KEYS_ARCHITECTURE.md Outdated

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Claude Code Review

Claude Code Review is paused for this repository. To reconnect it, an admin of this repository's GitHub organization (or the account owner, for personal repositories) who can also manage your Claude organization's Code Review settings needs to re-link GitHub in Code Review settings. This is a one-time step.

Tip: disable this comment in your organization's Code Review settings.

@j0ntz

j0ntz commented Oct 9, 2026

Copy link
Copy Markdown
Contributor Author

Not changed in this push. Moving the CHANGELOG lines and the enUS.json string means rewriting the three commits already reviewed, so this round only adds 54bb4d9 on top of the last one. The final tree is the same either way.

@j0ntz
j0ntz force-pushed the jon/attestation-qa-findings branch 2 times, most recently from 4221be7 to acbab39 Compare October 9, 2026 22:36

@peachbits peachbits left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All three threads check out at acbab39. One note for landing: autosquash drops the fixup's message, so the squashed commit would keep the original body, which still says it fetches "once more on the first token" and describes the per-launch flag. Reword that body when squashing.

j0ntz added 4 commits October 9, 2026 18:04
The clock warning only fired from the attestation handshake, so a launch
that ran no handshake (attestation unsupported, or in failure backoff)
never showed it, while the signed key request failed with a 401 on every
launch the device clock was wrong.

Carry the status and the HTTP Date header of a non-OK key response on a
typed error, and feed a 401's Date header into the same once-per-day
warning. An error response is not served from an HTTP cache, so its Date
header is a current reading of the server clock.
The rates server stamps each current rate with its own clock. The app
compared that stamp against the device clock to decide whether a rate
was current or historical, so a device clock set more than an hour
ahead filed every current rate as yesterday's rate. Cached rates then
expired and no fiat value could be shown. Read the server's time from
the Date response header and compare against that, falling back to the
device clock when the header is missing or unreadable.
The logs server refuses any log dated more than five minutes from its
own clock and answers 400 with the body 'Time Out of Sync'. The app
dropped that body and showed only the status code, which reads as a
server fault. Recognize the refusal and tell the user to fix the
device clock instead.
A signed key request that goes out before the attestation handshake has
a token is answered from the default layer, which has no Paybis or
Simplex keys. That answer was written to the cache and nothing fetched
again, so the next launch warm-started without those providers. QA hit
this on launches where the handshake had to re-enroll its key or was
rate limited and only succeeded on a retry.

When a request goes out with no token, subscribe to the attestation
engine and fetch again once a token arrives, writing the answer to the
cache for the next launch. The subscription stays until a token-carrying
answer is cached, so a refetch that fails is tried again at the next
token. One refetch runs at a time. The running keys are not swapped.

The cache records whether its request carried a token, and an answer to
a request without one no longer replaces an attested entry younger than
3 days. A launch that misses the attestation budget on a device that
still attests (slow handshake, rate limit) used to cache the default
payload at once, so closing the app before the retry landed still cost
the next launch its partner keys. The same check covers two requests in
flight at once.

The architecture doc describes the late-token rules as built.
@j0ntz

j0ntz commented Oct 10, 2026

Copy link
Copy Markdown
Contributor Author

Reworded the squashed commit body to describe the cache-side token check and the retained late-token subscription, and dropped the per-launch flag wording.

@j0ntz
j0ntz force-pushed the jon/attestation-qa-findings branch from acbab39 to a211ee9 Compare October 10, 2026 01:05
@j0ntz
j0ntz enabled auto-merge October 10, 2026 01:06
@j0ntz
j0ntz merged commit 10cd2e2 into develop Oct 10, 2026
7 checks passed
@j0ntz
j0ntz deleted the jon/attestation-qa-findings branch October 10, 2026 01:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants