Skip to content
Open
Show file tree
Hide file tree
Changes from all 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
8 changes: 4 additions & 4 deletions src/deposit-address/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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).
Expand Down Expand Up @@ -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:<botIdentifier>` — set of `erc20Transfer.transactionHash` for successfully executed deposits.
- `deposit-address:withdrawn-deposit-keys:<botIdentifier>` — set of `depositKey` (`depositAddress:transactionHash`) for successfully executed refund withdraws.
- `deposit-address:executed:<botIdentifier>` — set of `depositKey` for successfully executed deposits.
- `deposit-address:withdrawn-deposit-keys:<botIdentifier>` — set of `depositKey` for successfully executed refund withdraws.
- `deposit-address:skipped-withdraw-keys:<botIdentifier>` — 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:<botIdentifier>` — 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.

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