hotfix(migrations): revert ce4307c SQL comment edits to restore sqlx hash - #125
Merged
Merged
Conversation
…hash After #117 merged to develop the dfxdev container started crash-looping with `Migrate(VersionMismatch(1))` and the deploy-dev smoke test returned 502 for ~5 min straight before the workflow failed. Root cause: commit ce4307c ("docs(migrations): replace remaining 'server' with 'node' in SQL comments") edited the comment lines in the already-applied migrations `0001_initial.sql` and `0003_pending_inscriptions.sql`. sqlx hashes the migration file content (comments included), so a deployed DB whose `_sqlx_migrations.checksum` reflects the pre-edit text refuses to boot with the post-edit binary. This is the exact same class of issue that PR #95 already had to hotfix (`13155c1 hotfix(migration): revert SQL comment edit to keep sqlx hash stable`) and that `feedback_sqlx_migration_hash` documents. Fix: restore both files to their pre-ce4307c byte-for-byte content. Pure cosmetic revert — the only difference is "node" → "server" in 6 lines of `--` comments. No schema, no logic, no data change. The container's first boot after this lands will match its existing `_sqlx_migrations` row and proceed past the migrate step. If the "node" / "server" vocabulary is eventually wanted in the migration prose, the right move is a NEW migration whose comments use the chosen vocabulary — the old ones must stay frozen for the checksum to match deployed databases.
This was referenced May 27, 2026
TaprootFreak
added a commit
that referenced
this pull request
May 28, 2026
…132) The previous fix (PR #129) attacked the symptom (wallet's local BIP-32 child-index counter desyncing from the server after a seed restore) by emitting `num_sends` from `/api/balance` so the wallet could hydrate its counter and derive `prev_commitment_pubkey` correctly. That works only when the wallet's deployed code actually syncs the counter — which the stale DEV deploy of `zk-coins/app` demonstrably did not, so `07-send.spec.ts::send-success` kept failing with `Interner Fehler: Vorheriger Public Key fehlt.` after PR #129 went live. Root cause is structural: making the client responsible for `prev_commitment_pubkey` puts a derivable lookup key on the client side that has to stay in lockstep with the server's persisted state across seed restores, app deploys, and TOCTOU windows between balance fetch and signing. Every desync surfaces as a 400. The class of bugs is not solvable by counter-syncing. This change moves the lookup to where the data lives. `Account` gains a `commitment_public_key: Option<PublicKey>` field set atomically with `proof` + `num_sends` inside `send_coins_inner`. The AccountUpdate branch reads it directly from the persisted account; the caller-supplied `prev_commitment_pubkey` is ignored. The 400 error string disappears from `map_send_coins_error` — it is unreachable as long as the field invariant (`proof.is_some() iff num_sends > 0 iff commitment_public_key.is_some()`) holds, which `send_coins_inner` is the only mutator of. Net result: a wallet that omits `prev_commitment_pubkey` entirely (or sends a stale one from a desynced counter) now succeeds. The deployed `zk-coins/app:beta` already on DEV stops 400ing without needing the app-side PR #125 to deploy first. Migration 0012 wipes `accounts` (same closed-test-env precedent as 0010 / 0011): the bincode shape is non-additive, and 0011 left post-#129 rows in the inconsistent `proof=Some, commitment_public_key=None` state that would panic the AccountUpdate branch's invariant `expect`. The legacy `SendCoinRequest::prev_commitment_pubkey` field stays on the wire so deployed wallets (including `app` PR #125, which still emits it) keep parsing. Drop it from the API once every published client has cycled off the contract. Regression coverage: - `account_node_tests::test_send_coins_second_send_succeeds_without_prev_commitment_pubkey` drives the AccountUpdate branch with `prev_commitment_pubkey = None` directly through `send_coins`. - `api_remote::second_send_succeeds_without_prev_commitment_pubkey_field` drives the same contract end-to-end against the live DEV server via the slim `ci:full` lane. - `account_node_tests::test_send_coins_twice_from_same_account_uses_update_account` pins the post-condition that all three coupled fields advance together (`proof.is_some()`, `num_sends == 2`, and `commitment_public_key == Some(pubkey_used_in_send_2)`). - The historical 400 mapping unit test (`map_send_coins_error_prev_commitment_pubkey_required_is_400`) becomes `map_send_coins_error_legacy_prev_commitment_pubkey_string_is_unmapped_500` — pinning that the string falls through to the catch-all 500 arm so any future regression that re-introduces it can't be silently re-mapped to 400 without also walking back the architecture.
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
1 participant
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.
What
Restore
0001_initial.sqland0003_pending_inscriptions.sqlto their pre-ce4307c byte-for-byte content. Pure-comment revert (6 lines), no schema / logic / data change.Why
dfxdev is currently crash-looping. After #117 merged to
develop, the newzkcoins/node:betaimage refuses to start with:…and the deploy-dev smoke test polled
dev-api.zkcoins.app/api/infofor ~5 min straight, getting 502 every time, before the workflow failed.Root cause: commit
ce4307c("docs(migrations): replace remaining 'server' with 'node' in SQL comments") edited the comment lines in two already-applied migrations. sqlx hashes the full migration file content (comments included), so a deployed DB whose_sqlx_migrations.checksumreflects the pre-edit text refuses to boot with the post-edit binary.This is the exact same class of issue that #95 already had to hotfix (
13155c1) and that thefeedback_sqlx_migration_hashmemory documents.Going forward
If the "node" / "server" vocabulary normalisation is eventually wanted in the migration prose, the right move is a NEW migration whose comments use the chosen vocabulary — the old ones must stay frozen for the checksum to match deployed databases. Editing the prose of already-applied migrations is never safe once a DB has applied them.
Urgency
dfxdev container is in
Restarting (1)loop. Merge ASAP to unblock the deploy.🤖 Generated as a hotfix after the failed Deploy DEV run on
develop8e0b691.