fix(indexer): dedupe deployment inserts by contract address, not just tx hash - #340
Merged
blockchain-maxis merged 2 commits intoSep 2, 2026
Conversation
… tx hash Contract.address is @unique, but the deployment worker's dedupe check only looked at deployTxHash. Once a profile can hold several linked wallets, the same contract can be reached from more than one wallet's scan path — each surfacing its own transaction hash — so a tx-hash-only guard lets a second wallet's scan attempt to insert a contract that's already recorded under a different wallet. Add a second dedupe check by address once the address is known (it isn't yet at the cheap tx-hash-only guard, which still runs first to avoid fetching the transaction at all when possible). The upsert itself was already address-keyed with an empty update, so it couldn't actually corrupt attribution — but nothing stopped a redundant write attempt, inflated contractsFound counts, or misleading "new contract" logs, and none of it was tested. Made the worker's store injectable (mirroring OperationsStore in operations.ts, defaulting to the real Prisma client so index.ts's call site is unchanged) specifically so this could be tested. Added deployment.test.ts, building real Soroban V3 result_meta_xdr fixtures via the SDK's own xdr builders so the dedupe is exercised through the actual contract-address extraction path, not a stubbed-out one.
|
@ibochielizabeth-spec Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
✅ Deploy Preview for stellar-signet ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
blockchain-maxis#334 landed the same DeploymentStore seam this branch introduced, from the other direction (making the worker testable for mid-life wallet pickup), so both files conflicted. deployment.ts: - keep main's doc comment for the seam, plus this branch's note on why the seam also covers the two-wallet dedupe case; - findFirst takes `{ deployTxHash } | { address }` — this branch's widening, which is what the address dedupe needs; - `store` stays a required parameter (main's index.ts passes prisma explicitly), dropping this branch's default; - keep the address dedupe itself, the point of the PR. deployment.test.ts: took this branch's file (it carries the three dedupe tests plus main's other three) and ported main's 'wallet linked between cycles' test onto its fakeHorizon, which needed pubkey tracking added. 63/63 indexer tests pass; typecheck and lint clean.
blockchain-maxis
added a commit
to escaprt/signet
that referenced
this pull request
Sep 2, 2026
This branch rewrites the deployment worker's single 200-op scan into a paginated backfill, while blockchain-maxis#334 and blockchain-maxis#340 changed that same scan on main. Merged so both survive rather than picking a side: - DeploymentStore gains this branch's wallet.update alongside main's widened contract.findFirst (`{ deployTxHash } | { address }`). - `store` stays a required parameter (index.ts passes prisma explicitly), dropping this branch's default. - blockchain-maxis#340's address dedup is re-added inside handleOperation — the new shared handler both the quick-check and backfill paths run through, so it now covers more history than the scan it was written for, not less. Without this the merge would have silently reverted that fix. deployment.test.ts: took this branch's file (its fixtures match the paginated worker) and ported main's coverage onto them — cross-wallet dedup, per-wallet attribution, the cheap tx-hash guard firing before any transaction fetch, non-create ops ignored, and mid-life wallet pickup. Its in-memory store's findFirst also had to learn the address key shape; as written it read where.deployTxHash unconditionally, so every address lookup would have matched whichever row had an undefined hash. Checked the ported dedup test actually bites: stubbing out the address check fails it (65 pass / 1 fail), so it is a real guard rather than a passing assertion. 66/66 indexer tests pass; typecheck and lint clean. The migration (20260830000000_wallet_deployment_backfill) sorts after main's 20260829000000_drop_redundant_unique_indexes, and adds two nullable columns, so it is safe on an existing database.
blockchain-maxis
added a commit
to escaprt/signet
that referenced
this pull request
Sep 2, 2026
Both blockchain-maxis#360 (linkDeployWallet) and blockchain-maxis#361 (per-wallet backfill) landed while this was open, and this PR extends exactly those two files. Everything here is additive rather than a choice between sides: - schema.prisma keeps both column sets — blockchain-maxis#361's deploymentCursor / deploymentBackfilledAt and this branch's indexRequestedAt. - account.ts keeps main's WalletSource typing and toWalletSource read guard, plus this branch's indexRequestedAt stamping on create and re-link. - deployment.ts keeps blockchain-maxis#361's paginated quickCheck/backfill split (this branch still had the old single 200-op scan, which would have reverted it) and adds this branch's indexRequestedAt clearing after each wallet, on success or failure. - DeploymentStore's update now accepts all three fields; findFirst keeps blockchain-maxis#340's widened key shape. - docs/INDEXER.md's worker table describes the paginated scan and the cleared request in one row rather than one replacing the other. deployment.test.ts: took main's file (its fixtures match the paginated worker) and added index-request coverage on top — cleared after a successful scan, cleared after a failed one, and untouched when never set. The middle case is the one worth having: leaving the flag set after a transient Horizon error would force short ticks for as long as Horizon stayed down. 69/69 indexer and 273/273 web tests pass; typecheck, lint and build clean. The new migration (20260830010000_wallet_index_request) sorts after blockchain-maxis#361's 20260830000000 and adds one nullable column.
blockchain-maxis
added a commit
to manchesternews98-jpg/signet
that referenced
this pull request
Sep 4, 2026
Closes blockchain-maxis#394. blockchain-maxis#279 — the issue this branch was opened against — is already fixed on main by blockchain-maxis#361, so the wholesale rewrite of deployment.ts is dropped: as authored it would have reverted the blockchain-maxis#340 address dedup, removed indexRequestedAt handling (blockchain-maxis#362/blockchain-maxis#334) and left two disagreeing backfill state machines on the Wallet table (blockchain-maxis#361). What is kept is the two ideas in it that main genuinely does not have, rebased as a small additive diff. **Forward catch-up.** Once a wallet is backfilled the worker checked a fixed newest-200 window each tick. That window is anchored to *now* rather than to how far the worker actually got, so anything that fell out of it between two ticks was never examined again — the backward walk is finished and does not revisit. A deploy script, a busy testnet key, or a long idle interval all reach it, and the symptom is invisible: the profile just quietly misses contracts, which is blockchain-maxis#279's complaint arriving by a different route. The new `Wallet.deploymentWatermark` records the newest operation actually examined, and the post-backfill check resumes forward from it. A wallet with no watermark yet reads one bounded newest-first page to establish one, which is the old behaviour for exactly one tick. **Per-page persistence.** `deploymentCursor` was written once at the end of a tick's walk, so a crash discarded up to MAX_PAGES_PER_TICK pages. Both cursor and watermark are now written after every page. On a wallet thousands of operations behind, that is the difference between making progress and never catching up. The docs claimed "Horizon is always newest-first, so nothing new can be missed", which is precisely the assumption that fails here; INDEXER.md now says why the obvious design is wrong instead. 5 tests added, 18 pass. Everything blockchain-maxis#361 established — the dedup, the indexRequestedAt clearing, the resumable backward walk, the explicit store seam — is untouched.
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.
closes #283
Summary
Contract.addressis@unique, but the deployment worker's dedupe check only looked atdeployTxHash(findFirst({ where: { deployTxHash: txHash } } })). Once a profile can hold several linked wallets, the same contract can be reached from more than one wallet's scan path — each surfacing its own transaction hash — so a tx-hash-only guard doesn't stop a second wallet's scan from attempting to record a contract that's already known under a different wallet.Changes
apps/indexer/src/workers/deployment.ts: added a second dedupe check byaddress, right after the address is extracted from the transaction (it isn't known yet at the cheap tx-hash-only guard, which still runs first and still short-circuits before fetching the transaction at all whenever possible). The existingupsert(where: { address },update: {}) was already safe against actually corrupting attribution — an empty update never rewriteswalletId/deployerPubkey— but nothing stopped a redundant write attempt, an inflatedcontractsFoundcount, or a misleading "new contract" log line on rediscovery, and none of this had any test coverage at all.DeploymentStoreinterface (mirroringOperationsStoreinoperations.ts), defaulting to the real Prisma client soindex.ts's call site (runDeploymentWorker(horizon, config)) is unchanged.apps/indexer/src/workers/deployment.test.ts(new): builds real, minimal Soroban V3result_meta_xdrfixtures using the SDK's ownxdrbuilders, so the dedupe is exercised through the actualextractContractAddressparsing path rather than a stubbed-out one. Covers:Verification
pnpm --filter @signet/indexer typecheck/lint/test— all pass (53/53 tests, 6 new).pnpm typecheck/pnpm test— unaffected, all pass.