Skip to content

Commit

Permalink
refactor(relayer): Remove filledRelays mapping (#1353)
Browse files Browse the repository at this point in the history
v3 has removed partial fills and the relayer now knows the onchain fill
status of each candidate deposit prior to considering possible fills, so
there's no need for additional internal tracking of the filled relays.
  • Loading branch information
pxrl authored Mar 25, 2024
1 parent c63c1c1 commit 5257f48
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 40 deletions.
23 changes: 1 addition & 22 deletions src/relayer/Relayer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,6 @@ const UNPROFITABLE_DEPOSIT_NOTICE_PERIOD = 60 * 60; // 1 hour
export class Relayer {
public readonly relayerAddress: string;

// Track by originChainId since depositId is issued on the origin chain.
// Key is in the form of "chainId-depositId".
private fullyFilledDeposits: { [key: string]: boolean } = {};

constructor(
relayerAddress: string,
readonly logger: winston.Logger,
Expand Down Expand Up @@ -393,21 +389,7 @@ export class Relayer {
}

fillRelay(deposit: V3Deposit, repaymentChainId: number, realizedLpFeePct: BigNumber, gasLimit?: BigNumber): void {
const { originChainId, depositId, outputToken, outputAmount } = deposit;
// Skip deposits that this relayer has already filled completely before to prevent double filling (which is a waste
// of gas as the second fill would fail).
// TODO: Handle the edge case scenario where the first fill failed due to transient errors and needs to be retried.
const fillKey = `${originChainId}-${depositId}`;
if (this.fullyFilledDeposits[fillKey]) {
this.logger.debug({
at: "Relayer",
message: "Skipping deposit already filled by this relayer.",
originChainId: deposit.originChainId,
depositId: deposit.depositId,
});
return;
}

const { outputToken, outputAmount } = deposit;
const { spokePoolClients, multiCallerClient } = this.clients;
this.logger.debug({ at: "Relayer", message: "Filling v3 deposit.", deposit, repaymentChainId, realizedLpFeePct });

Expand All @@ -434,9 +416,6 @@ export class Relayer {

// Decrement tokens in token client used in the fill. This ensures that we dont try and fill more than we have.
this.clients.tokenClient.decrementLocalBalance(deposit.destinationChainId, outputToken, outputAmount);

// All fills routed through `fillRelay()` will complete the relay.
this.fullyFilledDeposits[fillKey] = true;
}

protected async resolveRepaymentChain(
Expand Down
33 changes: 15 additions & 18 deletions test/Relayer.BasicFill.ts
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,7 @@ describe("Relayer: Check for Unfilled Deposits and Fill", async function () {
await relayerInstance.checkForUnfilledDepositsAndFill();
expect(lastSpyLogIncludes(spy, "Filling v3 deposit")).to.be.true;
expect(multiCallerClient.transactionCount()).to.equal(1); // One transaction, filling the one deposit.
await multiCallerClient.executeTxnQueues();

// The first fill is still pending but if we rerun the relayer loop, it shouldn't try to fill a second time.
await Promise.all([spokePoolClient_1.update(), spokePoolClient_2.update(), hubPoolClient.update()]);
Expand Down Expand Up @@ -343,10 +344,12 @@ describe("Relayer: Check for Unfilled Deposits and Fill", async function () {
});

it("Ignores exclusive deposits", async function () {
for (const exclusiveRelayer of [randomAddress(), relayerInstance.relayerAddress]) {
const currentTime = (await spokePool_2.getCurrentTime()).toNumber();
const exclusivityDeadline = currentTime + 60;
const currentTime = (await spokePool_2.getCurrentTime()).toNumber();
const exclusivityDeadline = currentTime + 7200;

// Make two deposits - one with the relayer as exclusiveRelayer, and one with a random address.
// Verify that the relayer can immediately fill the first deposit, and both after the exclusivity window.
for (const exclusiveRelayer of [randomAddress(), relayerInstance.relayerAddress]) {
await depositV3(
spokePool_1,
destinationChainId,
Expand All @@ -357,24 +360,18 @@ describe("Relayer: Check for Unfilled Deposits and Fill", async function () {
outputAmount.div(2),
{ exclusivityDeadline, exclusiveRelayer }
);
const relayerIsExclusive = relayerInstance.relayerAddress === exclusiveRelayer;
await updateAllClients();

await relayerInstance.checkForUnfilledDepositsAndFill();
expect(multiCallerClient.transactionCount()).to.equal(relayerIsExclusive ? 1 : 0);
}

if (relayerIsExclusive) {
continue;
}
await updateAllClients();
await relayerInstance.checkForUnfilledDepositsAndFill();
expect(multiCallerClient.transactionCount()).to.equal(1);

await spokePool_2.setCurrentTime(exclusivityDeadline + 1);
await updateAllClients();
await spokePool_2.setCurrentTime(exclusivityDeadline + 1);
await updateAllClients();

// Relayer can unconditionally fill after the exclusivityDeadline.
expect(multiCallerClient.transactionCount()).to.equal(0);
await relayerInstance.checkForUnfilledDepositsAndFill();
expect(multiCallerClient.transactionCount()).to.equal(1);
}
// Relayer can unconditionally fill after the exclusivityDeadline.
await relayerInstance.checkForUnfilledDepositsAndFill();
expect(multiCallerClient.transactionCount()).to.equal(2);
});

it("Ignores deposits older than min deposit confirmation threshold", async function () {
Expand Down

0 comments on commit 5257f48

Please sign in to comment.