Repository navigation
feat(ecurrency): ledger → eVault sync script that verifies against each eVault - #1201
Conversation
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configuration
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e2cec7350b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| async function expectedPayload(row: Ledger): Promise<Record<string, unknown>> { | ||
| const currencyGlobalId = await adapter.mappingDb.getGlobalId(row.currencyId); | ||
| return { | ||
| currencyId: currencyGlobalId ?? row.currencyId, |
There was a problem hiding this comment.
When the currency mapping is absent or stale—the same mapping-database failure mode this repair tool is intended to tolerate—this fallback writes the local PostgreSQL UUID as currencyId. The live adapter instead resolves this field through the mapping store, so the repaired ledger no longer references the eVault currency; because isCopyOf accepts a matching hash without checking currencyId, the verification pass nevertheless reports it as synced. Resolve and verify the currency envelope mapping, or fail the row rather than persisting a local ID.
Useful? React with 👍 / 👎.
| const t = Date.parse(String(a)); | ||
| if (Number.isNaN(t)) return false; | ||
| const diff = Math.abs(t - b.getTime()); | ||
| return diff === 0 || (diff % (15 * 60_000) === 0 && diff <= 14 * 3_600_000); |
There was a problem hiding this comment.
For hashless records, this treats any timestamps separated by a multiple of 15 minutes up to 14 hours as identical, not merely timestamps affected by a known timezone offset. If an account returns to the same balance after repeated same-value transactions—for example, equivalent debits at 10:00 and 10:15—the same hashless envelope can be selected as the update target for both rows; --apply then overwrites that envelope twice instead of creating the missing copy, corrupting the first copy before the final verification fails. Matching should use an explicit timezone conversion and additional immutable identity fields rather than this broad interval test.
Useful? React with 👍 / 👎.
| const flag = (name: string) => args.find((a) => a.startsWith(`--${name}=`))?.split("=")[1]; | ||
| const APPLY = args.includes("--apply"); | ||
| const ONLY = flag("only") ? normalizeEname(flag("only") as string) : null; | ||
| const SETTLE_MS = Number(flag("settle") ?? 120) * 1000; |
There was a problem hiding this comment.
If an operator supplies a malformed value such as --settle=120s, Number(...) produces NaN, the cutoff becomes an invalid date, and every row.createdAt <= cutoff comparison is false. The script consequently checks zero rows and exits successfully with a 0/0 report, which can make an automated repair job falsely appear complete; validate that the value is finite and nonnegative before querying.
Useful? React with 👍 / 👎.
| if (ename) parties.add(ename); | ||
| else unnamed.add(`${type}:${id}`); | ||
| } | ||
| if (parties.size === 0) noParties++; |
There was a problem hiding this comment.
CurrencyService.mintCurrency creates each minted credit with the group's accountId and accountType but without sender or receiver fields, so this branch classifies every mint as having no parties and never adds it to byVault. As a result, neither check nor apply mode verifies or repairs minted-supply entries in the group's eVault, and the script can exit successfully while those ledger rows remain absent; non-initialization rows without explicit parties need to fall back to their ledger account owner.
Useful? React with 👍 / 👎.
| } catch (error) { | ||
| if (attempt >= 5) throw error; | ||
| await sleep(1000 * 2 ** attempt); | ||
| continue; |
There was a problem hiding this comment.
This generic retry also wraps storeMetaEnvelope, whose identifier is generated by the eVault. If the eVault commits a missing-row write but the response is lost or the client times out, this catch retries the mutation and creates a second envelope for the same ledger row. The post-write verification then succeeds because at least one copy matches, and report.duplicates is not recomputed after writes, so the run can exit 0 without even reporting the duplicate it created; retries for stores need an idempotency key or an intervening read instead.
Useful? React with 👍 / 👎.
| if (current) await adapter.mappingDb.deleteMapping(localId); | ||
| await adapter.mappingDb.storeMapping({ localId, globalId }); | ||
| } catch (error) { | ||
| console.warn(`[SYNC] mapping ${localId}: ${(error as Error).message}`); |
There was a problem hiding this comment.
If SQLite is locked, unwritable, or otherwise rejects storeMapping, this block only logs a warning; when an old mapping existed it has already been deleted on the preceding line. Vault verification does not inspect the mapping database, so --apply can still exit 0 while the adapter mapping remains stale or absent, contrary to the repair's stated guarantee and leaving subsequent live updates directed at the wrong envelope. Propagate the error into the vault report or verify the mapping before declaring success.
Useful? React with 👍 / 👎.
1 participant
Why
The live ledger sync is fire-and-forget: each row is pushed 3s after insert and again after its hash is saved, failures are only logged, and a restart inside that window drops it. On the network today 590 of 786 eCurrency ledger copies carry no hash, and 40 transfers exist in only one party's vault.
What
pnpm sync:ledger(platforms/ecurrency/api/src/scripts/sync-ledger.ts):/platforms/certification(asVITE_ECURRENCY_BASE_URL, like the live adapter).--applycreates missing copies with the live adapter's payload and acl, updates outdated ones in place, repoints the adapter mapping, then re-reads every vault and exits 1 if anything is still unsynced.--settle(default 120s) are skipped to avoid racing the live sync.--only=@enamelimits to one vault.Testing
Ran against real eCurrency migrations on Postgres plus a mock Registry/eVault seeded with missing, outdated, duplicate and synced copies, a mapping pointing at a non-existent envelope, a 429, a 500, and a vault that silently drops one write:
--applywrote 11, updated 2, caught the dropped write on re-read, exited 1; second run fixed it and exited 0. The bad mapping was repointed.Notes
skipWebhooksis only honoured for emover).