From 8c422c2fd60b934312c5223346c5a2895b699481 Mon Sep 17 00:00:00 2001 From: amateima Date: Tue, 25 Aug 2026 20:45:11 +0300 Subject: [PATCH 1/2] fix(deposit-address): include logIndex in dedup keys so sibling transfers in one tx all sweep One transaction can carry multiple transfers to the same deposit address, but both dedup guards collapsed them into one: getDepositKey was depositAddress:txHash and the Redis-persisted executed set keyed on the bare txHash, so only the first transfer was swept and the rest were silently skipped until the indexer window closed. getDepositKey is now depositAddress:txHash:logIndex, which fixes every set keyed on it (in-flight deposit/withdraw locks, executed/skipped/ refund-only withdraw sets) on both v1 and v3 paths. The executed-deposits set now stores depositKeys instead of bare txHashes, letting the prune loop reuse the depositKey set built from indexer messages. Old-format Redis entries self-prune on the first poll; rows still inside the indexer window fall through to the existing on-chain balance checks. Co-Authored-By: Claude Fable 5 --- src/deposit-address-service/message.ts | 5 ++-- src/deposit-address/DepositAddressHandler.ts | 29 ++++++++++---------- src/utils/DepositAddressUtils.ts | 5 +++- test/DepositAddressHandler.ts | 14 ++++------ test/DepositAddressUtils.ts | 12 +++++++- 5 files changed, 37 insertions(+), 28 deletions(-) diff --git a/src/deposit-address-service/message.ts b/src/deposit-address-service/message.ts index 5b9e206212..e91295149f 100644 --- a/src/deposit-address-service/message.ts +++ b/src/deposit-address-service/message.ts @@ -75,9 +75,8 @@ export interface ParsedTransfer { /** * The indexer row's durable identity, and the tuple `DepositAddressExecutionConsumer` already keys - * lookups on. Deliberately finer than the polling bot's `getDepositKey`, which is - * `depositAddress:transactionHash` and so collides when one transaction makes two transfers to the same - * address. + * lookups on. Same granularity as the polling bot's `getDepositKey` + * (`depositAddress:transactionHash:logIndex`), but chain-qualified instead of address-qualified. * * Normalised, because the same transfer must always produce the same id: `chainId` arrives as a string, * and hash casing varies. No prefix normalisation — format is consistent per chain (EVM `0x`, Tron diff --git a/src/deposit-address/DepositAddressHandler.ts b/src/deposit-address/DepositAddressHandler.ts index b7a80638ed..e69b03ed3e 100644 --- a/src/deposit-address/DepositAddressHandler.ts +++ b/src/deposit-address/DepositAddressHandler.ts @@ -142,8 +142,8 @@ export class DepositAddressHandler { /** Per chainId: set of deposit keys already executed (like gasless depositNonces). */ private observedExecutedDeposits: { [chainId: number]: Set } = {}; - /** Set of erc20Transfer.transactionHash for deposits successfully executed (persisted in Redis for handover). */ - private executedDepositTxHashes: Set = new Set(); + /** Set of depositKeys for deposits successfully executed (persisted in Redis for handover). */ + private executedDepositKeys: Set = new Set(); /** Set of depositKeys for refund withdraws successfully executed (persisted in Redis for handover). */ private executedWithdrawKeys: Set = new Set(); @@ -300,11 +300,11 @@ export class DepositAddressHandler { throw err; } - this.executedDepositTxHashes = new Set(arr); + this.executedDepositKeys = new Set(arr); this.logger.debug({ at: "DepositAddressHandler#_loadExecutedDepositsFromRedis", - message: "Loaded executed deposit tx hashes from Redis", - count: this.executedDepositTxHashes.size, + message: "Loaded executed deposit keys from Redis", + count: this.executedDepositKeys.size, }); } @@ -466,11 +466,10 @@ export class DepositAddressHandler { // We want to remove all executed deposits from the in-memory set if they are not returned by the indexer. // This is because the indexer will stop sending the deposit once it has been "expired" (internal TTL). // So there is no point of keeping them in Redis after Indexer API stops returning them. - const refTxHashesFromIndexer = new Set(depositMessages.map((m) => m.erc20Transfer.transactionHash)); const depositKeysFromIndexer = new Set(depositMessages.map((m) => getDepositKey(m))); - for (const tx of [...this.executedDepositTxHashes]) { - if (!refTxHashesFromIndexer.has(tx)) { - this.executedDepositTxHashes.delete(tx); + for (const key of [...this.executedDepositKeys]) { + if (!depositKeysFromIndexer.has(key)) { + this.executedDepositKeys.delete(key); } } for (const key of [...this.executedWithdrawKeys]) { @@ -776,14 +775,14 @@ export class DepositAddressHandler { } /** - * Overwrites Redis key with the full executedDepositTxHashes set (single SET; value is JSON array). + * Overwrites Redis key with the full executedDepositKeys set (single SET; value is JSON array). * Called at start of each poll (after filtering) and after each successful execute. */ private async _persistExecutedDepositsRedis(): Promise { assert(isDefined(this.redisCache), "DepositAddressHandler: redisCache accessed before initialize()"); const { redisCache } = this; const redisKey = this.getExecutedDepositsRedisKey(); - await redisCache.set(redisKey, JSON.stringify([...this.executedDepositTxHashes])); + await redisCache.set(redisKey, JSON.stringify([...this.executedDepositKeys])); } /** Same pattern as `_persistExecutedDepositsRedis` but for refund-withdraw deposit keys. */ @@ -998,7 +997,7 @@ export class DepositAddressHandler { } // Skip if a previous instance (or this one) already executed this deposit (persisted in Redis). - if (this.executedDepositTxHashes.has(refTxHash)) { + if (this.executedDepositKeys.has(depositKey)) { this.logger.debug({ at: "DepositAddressHandler#initiateDeposit", message: "Skipping already executed deposit (found in Redis)", @@ -1135,7 +1134,7 @@ export class DepositAddressHandler { } // Persist full set to Redis immediately so handover cannot miss this execute. - this.executedDepositTxHashes.add(refTxHash); + this.executedDepositKeys.add(depositKey); await this._persistExecutedDepositsRedis(); } @@ -1159,7 +1158,7 @@ export class DepositAddressHandler { } // Skip if a previous instance (or this one) already executed this deposit (persisted in Redis). - if (this.executedDepositTxHashes.has(refTxHash)) { + if (this.executedDepositKeys.has(depositKey)) { this.logger.debug({ at: "DepositAddressHandler#initiateDepositV3", message: "Skipping already executed deposit (found in Redis)", @@ -1286,7 +1285,7 @@ export class DepositAddressHandler { // The execute is on-chain; keep the in-flight lock and persist to Redis immediately so // handover cannot miss this execute. - this.executedDepositTxHashes.add(refTxHash); + this.executedDepositKeys.add(depositKey); executeCommitted = true; await this._persistExecutedDepositsRedis(); await this._publishDepositExecuted(depositReceipt, depositMessage); diff --git a/src/utils/DepositAddressUtils.ts b/src/utils/DepositAddressUtils.ts index 71c6b97570..4aa7c179da 100644 --- a/src/utils/DepositAddressUtils.ts +++ b/src/utils/DepositAddressUtils.ts @@ -99,9 +99,12 @@ export function isNativeTokenSentinel(token: string): boolean { /** * Returns a unique key for a deposit so we can track if it was already executed (e.g. in observedExecutedDeposits). * Accepts any message version — the key only depends on the shared deposit-address/transfer envelope. + * logIndex disambiguates multiple transfers to the same address within one transaction, which would + * otherwise collide and leave all but one unswept. */ export function getDepositKey(depositMessage: AnyDepositAddressMessage): string { - return `${depositMessage.depositAddress}:${depositMessage.erc20Transfer.transactionHash}`; + const { transactionHash, logIndex } = depositMessage.erc20Transfer; + return `${depositMessage.depositAddress}:${transactionHash}:${logIndex}`; } /** diff --git a/test/DepositAddressHandler.ts b/test/DepositAddressHandler.ts index 6aa1b83258..47a33ed180 100644 --- a/test/DepositAddressHandler.ts +++ b/test/DepositAddressHandler.ts @@ -17,7 +17,7 @@ import { AcrossApiHttpError, DepositAddressExecuteResponse, DepositAddressSignWi import { DepositAddressHandler } from "../src/deposit-address/DepositAddressHandler"; import { DepositAddressHandlerConfig } from "../src/deposit-address/DepositAddressHandlerConfig"; import { ERC20_TRANSFER_TOPIC } from "../src/deposit-address/withdrawPayload"; -import { NATIVE_TOKEN_SENTINEL_ADDRESS } from "../src/utils/DepositAddressUtils"; +import { getDepositKey, NATIVE_TOKEN_SENTINEL_ADDRESS } from "../src/utils/DepositAddressUtils"; // EIP-55 checksummed: the handler round-trips the signer through `toAddressType().toNative()`, // which returns the checksummed form, so an un-checksummed literal fails the request-shape compares. @@ -400,9 +400,7 @@ describe("DepositAddressHandler.processExecution v3 routing", function () { it("routes a v3 correct_transfer marked refund-only to the v3 withdraw path", async function () { const message = depositMessageV3(); - (handler as unknown as { refundOnlyDepositKeys: Set }).refundOnlyDepositKeys.add( - `${message.depositAddress}:${message.erc20Transfer.transactionHash}` - ); + (handler as unknown as { refundOnlyDepositKeys: Set }).refundOnlyDepositKeys.add(getDepositKey(message)); await (handler as unknown as Internals).processExecution(message); expect(withdrawV3Stub.calledOnceWithExactly(message)).to.equal(true); expect(v3Stub.notCalled).to.equal(true); @@ -549,7 +547,7 @@ describe("DepositAddressHandler._getExecuteTx below-minimum handling", function let executeStub: sinon.SinonStub; let redisSetStub: sinon.SinonStub; let warnStub: sinon.SinonStub; - const depositKey = `${DEPOSIT_ADDRESS}:${"0x" + "3".repeat(64)}`; + const depositKey = getDepositKey(depositMessageV3()); type Internals = { _getExecuteTx: (m: DepositAddressMessageV3) => Promise; @@ -616,7 +614,7 @@ describe("DepositAddressHandler.initiateDepositV3 below-minimum refund fallback" let withdrawV3Stub: sinon.SinonStub; let warnStub: sinon.SinonStub; const originChainId = 42161; - const depositKey = `${DEPOSIT_ADDRESS}:${"0x" + "3".repeat(64)}`; + const depositKey = getDepositKey(depositMessageV3()); type Internals = { initiateDepositV3: (m: DepositAddressMessageV3) => Promise; @@ -990,7 +988,7 @@ describe("DepositAddressHandler._getSignedWithdrawV3", function () { const result = await internals()._getSignedWithdrawV3(message, v3WithdrawLeaf); expect(result).to.equal(undefined); expect(signWithdrawStub.callCount).to.equal(1); // no retries on a terminal 422 - const depositKey = `${DEPOSIT_ADDRESS}:${message.erc20Transfer.transactionHash}`; + const depositKey = getDepositKey(message); expect(internals().terminallySkippedWithdrawKeys.has(depositKey)).to.equal(true); expect(redisSetStub.calledOnce).to.equal(true); }); @@ -1247,7 +1245,7 @@ describe("DepositAddressHandler._publishDepositExecuted", function () { describe("DepositAddressHandler refund-only key persistence", function () { let handler: DepositAddressHandler; let redisGetStub: sinon.SinonStub; - const depositKey = `${DEPOSIT_ADDRESS}:${"0x" + "3".repeat(64)}`; + const depositKey = getDepositKey(depositMessageV3()); type Internals = { _loadRefundOnlyKeysFromRedis: () => Promise; diff --git a/test/DepositAddressUtils.ts b/test/DepositAddressUtils.ts index 3280bb9f2d..9bca115957 100644 --- a/test/DepositAddressUtils.ts +++ b/test/DepositAddressUtils.ts @@ -161,9 +161,19 @@ describe("DepositAddressUtils", function () { const raw = tronOriginIndexerMessage(); const normalized = normalizeDepositAddressMessage(raw); - expect(getDepositKey(normalized)).to.equal(`${normalized.depositAddress}:${raw.erc20Transfer.transactionHash}`); + expect(getDepositKey(normalized)).to.equal( + `${normalized.depositAddress}:${raw.erc20Transfer.transactionHash}:${raw.erc20Transfer.logIndex}` + ); expect(getDepositKey(normalized)).to.not.equal(getDepositKey(raw)); }); + + it("getDepositKey distinguishes multiple transfers within one transaction by logIndex", function () { + const first = tronOriginIndexerMessage(); + const second = tronOriginIndexerMessage(); + second.erc20Transfer.logIndex = first.erc20Transfer.logIndex + 1; + + expect(getDepositKey(second)).to.not.equal(getDepositKey(first)); + }); }); describe("isNativeTokenSentinel", function () { From 035fa355964ee2f548ce711660cacde9981a552a Mon Sep 17 00:00:00 2001 From: amateima Date: Wed, 26 Aug 2026 01:02:07 +0300 Subject: [PATCH 2/2] docs(deposit-address): update Redis persistence contract for composite depositKey Co-Authored-By: Claude Fable 5 --- src/deposit-address/README.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/deposit-address/README.md b/src/deposit-address/README.md index 06ff53f9ea..45faea4aa8 100644 --- a/src/deposit-address/README.md +++ b/src/deposit-address/README.md @@ -110,7 +110,7 @@ the production rollout). The flow (`initiateDepositV3`): 1. Filter on `relayerOriginChains` and dedup against the same Redis/in-memory sets as v1 (the dedup - scheme is keyed on `erc20Transfer.transactionHash` / `depositKey`, shared across versions). + scheme is keyed on `depositKey`, shared across versions). 2. Skip when `depositAddressNamespace` / `refundAddress.namespace` don't match the origin chain's family (`evm` ⇒ EVM chains, `tron` ⇒ Tron; other families, and cross-family anomalies like a `tron` namespace on an EVM chainId, are skipped with a warn). @@ -246,10 +246,10 @@ neither validates nor skips: the deposit address is already deployed from explic ## Redis persistence -Four sets persist across runs so handover does not double-spend, double-refund, re-attempt a terminally-skipped refund, or re-execute a transfer already known to be unexecutable: +Four sets persist across runs so handover does not double-spend, double-refund, re-attempt a terminally-skipped refund, or re-execute a transfer already known to be unexecutable. All are keyed on `depositKey` (`depositAddress:transactionHash:logIndex` — logIndex disambiguates multiple transfers to the same address in one tx): -- `deposit-address:executed:` — set of `erc20Transfer.transactionHash` for successfully executed deposits. -- `deposit-address:withdrawn-deposit-keys:` — set of `depositKey` (`depositAddress:transactionHash`) for successfully executed refund withdraws. +- `deposit-address:executed:` — set of `depositKey` for successfully executed deposits. +- `deposit-address:withdrawn-deposit-keys:` — set of `depositKey` for successfully executed refund withdraws. - `deposit-address:skipped-withdraw-keys:` — set of `depositKey` for v3 refund withdraws that failed terminally: a quote-api 422 (`GAS_EXCEEDS_REFUND` / `UNPRICEABLE_REFUND_TOKEN`), or a balance read that reverted because the token is not a conforming ERC-20; never re-attempted. - `deposit-address:refund-only-deposit-keys:` — set of `depositKey` for v3 correct-transfers the execute endpoint rejected terminally (`AMOUNT_BELOW_MINIMUM` / `AMOUNT_TEMPORARILY_UNSWEEPABLE`); never executed again, routed to the refund-withdraw path instead.