Skip to content
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 2 additions & 3 deletions src/deposit-address-service/message.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
29 changes: 14 additions & 15 deletions src/deposit-address/DepositAddressHandler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -150,8 +150,8 @@ export class DepositAddressHandler {
/** Per chainId: set of deposit keys already executed (like gasless depositNonces). */
private observedExecutedDeposits: { [chainId: number]: Set<string> } = {};

/** Set of erc20Transfer.transactionHash for deposits successfully executed (persisted in Redis for handover). */
private executedDepositTxHashes: Set<string> = new Set();
/** Set of depositKeys for deposits successfully executed (persisted in Redis for handover). */
private executedDepositKeys: Set<string> = new Set();

/** Set of depositKeys for refund withdraws successfully executed (persisted in Redis for handover). */
private executedWithdrawKeys: Set<string> = new Set();
Expand Down Expand Up @@ -308,11 +308,11 @@ export class DepositAddressHandler {

throw err;
}
this.executedDepositTxHashes = new Set(arr);
this.executedDepositKeys = new Set(arr);
Comment thread
amateima marked this conversation as resolved.
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,
});
}

Expand Down Expand Up @@ -474,11 +474,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]) {
Expand Down Expand Up @@ -784,14 +783,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<void> {
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. */
Expand Down Expand Up @@ -1006,7 +1005,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)",
Expand Down Expand Up @@ -1143,7 +1142,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();
Comment thread
amateima marked this conversation as resolved.
}

Expand All @@ -1167,7 +1166,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)",
Expand Down Expand Up @@ -1294,7 +1293,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);
Expand Down
5 changes: 4 additions & 1 deletion src/utils/DepositAddressUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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}`;
}

/**
Expand Down
14 changes: 6 additions & 8 deletions test/DepositAddressHandler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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<string> }).refundOnlyDepositKeys.add(
`${message.depositAddress}:${message.erc20Transfer.transactionHash}`
);
(handler as unknown as { refundOnlyDepositKeys: Set<string> }).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);
Expand Down Expand Up @@ -549,7 +547,7 @@ describe("DepositAddressHandler._getExecuteTx terminal-code 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<DepositAddressExecuteResponse | undefined>;
Expand Down Expand Up @@ -628,7 +626,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<void>;
Expand Down Expand Up @@ -1009,7 +1007,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);
// Publish-first ordering: a Redis throw must not swallow the event.
Expand Down Expand Up @@ -1280,7 +1278,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<void>;
Expand Down
12 changes: 11 additions & 1 deletion test/DepositAddressUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 () {
Expand Down