fix(api,k6): ride out chain read-after-write races that flake the staging deploy gate - #595
Open
GTC6244 wants to merge 1 commit into
Open
fix(api,k6): ride out chain read-after-write races that flake the staging deploy gate#595GTC6244 wants to merge 1 commit into
GTC6244 wants to merge 1 commit into
Conversation
…ging deploy gate Three consecutive Deploy Staging runs (and the Jul 20 one) were killed by k6-correctness flakes that all reduce to reads racing just-mined writes on a load-balanced RPC: - POST /new_account 500 "Simulation failed: NoAccountAccess": the newAccount receipt proves the account exists, but send_transaction dry-runs the follow-up registerWalletDerivation via eth_call, which the provider can serve from a node that hasn't executed that block yet. That stale-read revert is the only way NoAccountAccess can occur at this call site, so retry it (4 attempts, 250ms exponential backoff); everything else still fails immediately. This is a real user-facing 500, not just a test flake. - integration.spec.ts sent update_action_metadata a hash of 0x0 when the list_actions read immediately after add_action/add_action_to_group was stale. Poll both list reads (bounded, 5 attempts / 2s) until the action is visible with a non-zero id, and fail explicitly if it never appears instead of forwarding 0x0. The third failure mode (every request EOF ~6 min after the cold box booted, run 29882923105) is not addressed here: the box passed wait-for-api, smoke, attestation and openapi at minute 2 and was unreachable by minute 7, and the ping-pong rollback correctly restored the domain to the old instance. Runs 1 and 2 stopped their cold box before minute 5, so they can't rule a dies-after-boot defect in or out; the next deploy is the second data point. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR mitigates staging deploy flakes (and a user-facing POST /new_account failure mode) caused by RPC read-after-write propagation lag by adding bounded retries/polling around immediately-following reads/simulations after on-chain writes.
Changes:
- Add bounded retry + exponential backoff in
new_accountforregisterWalletDerivationwhen the dry-run hits a transientNoAccountAccessdue to stale RPC reads. - Update k6 correctness integration to poll
list_actionsuntil the newly-added action is visible with a non-zero id before proceeding. - Document the fix in
CHANGELOG.md.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| lit-api-server/src/core/account_management.rs | Retries registerWalletDerivation on transient NoAccountAccess to ride out RPC stale-read windows after newAccount. |
| k6/correctness/integration.spec.ts | Polls list_actions until the newly added action appears with a usable (non-zero) id to avoid forwarding 0x0. |
| CHANGELOG.md | Adds a release note describing the POST /new_account mitigation. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+238
to
+240
| ) { | ||
| sleep(2); | ||
| listActionsAccountRes = client.listActions( |
Comment on lines
+273
to
+275
| ) { | ||
| sleep(2); | ||
| listActionsRes = client.listActions( |
Comment on lines
+71
to
+74
| - `POST /new_account` no longer intermittently returns a 500 | ||
| (`NoAccountAccess`) when the RPC provider serves the follow-up wallet | ||
| registration's dry-run from a node that hasn't executed the just-mined | ||
| `newAccount` block; the propagation window is now retried with backoff. |
Comment on lines
+140
to
+144
| let mut attempt = 0u32; | ||
| loop { | ||
| attempt += 1; | ||
| match accounts::register_wallet_derivation( | ||
| signer_pool.clone(), |
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.
HOLD
I think I may have merged a second branch into
mainbefore the first one completed - this also matches the results of the investigation... and by Occum's Razor ...Why
Three consecutive
Deploy Stagingruns on main were rolled back by k6-correctness failures, leaving staging stuck on2c972ff3(v1.1.10-58):POST /new_account→ 500Simulation failed: NoAccountAccess(1 of 4 iterations)POST /update_action_metadata→ 400Cannot update action with hash 0x0The Jul 20 run (pre-dating all three commits) failed with the identical
NoAccountAccesserror, so failures 1–2 are chronic flakes, not regressions from those commits. Both reduce to the same root cause: reads racing just-mined writes on the load-balanced RPC.What
new_account500 (real user-facing bug, not just a gate flake): thenewAccountreceipt proves the account exists, butsend_transactiondry-runs the follow-upregisterWalletDerivationviaeth_call, which the provider can serve from a node that hasn't executed that block yet — revertingNoAccountAccessfor an account that was just mined.newAccountitself never revertsNoAccountAccess(checked WritesFacet.sol), so at this call site the revert can only be the stale-read window. Retry it (4 attempts, 250 ms exponential backoff); any other error still fails immediately.hash 0x0400:integration.spec.tstookhashed_cidfrom alist_actionsread issued immediately afteradd_action/add_action_to_group; a stale read returned the action without a usable id and the spec forwarded0x0. Both list reads now poll (bounded: 5 attempts / 2 s) until the action is visible with a non-zero id, and the spec fails explicitly if it never appears.Not addressed: the EOF in run 29882923105
The cold box (
chipotle-next-rep-sa6xj, prod2) passed wait-for-api/attestation/openapi/smoke at minute ~2 after boot and was fully unreachable by minute ~7. Runs 1–2 stopped their cold box before minute 5, so they can't rule a dies-after-boot defect in or out. The ping-pong rollback worked as designed (domain restored to...-rep-w4hdl, bad box stopped). The next deploy is the second data point — if it EOFs again ~6 min after boot, the new build has a genuine post-boot crash and the box's phala logs need a look before it's stopped.Testing
cargo +1.91 check/cargo +1.91 test --lib account_management/cargo +1.91 fmton lit-api-serverk6 inspect k6/correctness/integration.spec.tsparses clean🤖 Generated with Claude Code