Skip to content

Commit

Permalink
refactor: Simplify relayer partial fill handling (#1015)
Browse files Browse the repository at this point in the history
Checking for existing fills in the pending transaction queue is only                                                                                                                           
necessary because of some test cases that instrument the relayer in                                                                                                                            
specific ways that produce this outcome. This is now how the relayer                                                                                                                           
works in production.                                                                                                                                                                           
                                                                                                                                                                                               
Flushing the transaction queue at the start of each run allows for the                                                                                                                         
relayer to assume that it has no previously-enqueued fills.
  • Loading branch information
pxrl authored Oct 18, 2023
1 parent b345734 commit 035dbe6
Show file tree
Hide file tree
Showing 4 changed files with 11 additions and 23 deletions.
19 changes: 4 additions & 15 deletions src/relayer/Relayer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,9 @@ export class Relayer {
const { config } = this;
const { acrossApiClient, hubPoolClient, profitClient, spokePoolClients, tokenClient } = this.clients;

// Flush any pre-existing enqueued transactions that might not have been executed.
this.clients.multiCallerClient.clearTransactionQueue();

const unfilledDeposits = await this.getUnfilledDeposits();

// Sum the total unfilled deposit amount per origin chain and set a MDC for that chain.
Expand Down Expand Up @@ -516,21 +519,7 @@ export class Relayer {
const { depositId, originChainId, destinationChainId, transactionHash: depositHash } = deposit;
const { inventoryClient, profitClient } = this.clients;

// TODO: Consider adding some way for Relayer to delete transactions in Queue for fills for same deposit.
// This way the relayer could set a repayment chain ID for any fill that follows a 1 wei fill in the queue.
// This isn't implemented due to complexity because its a very rare case in production, because its very
// unlikely that a relayer could enqueue a 1 wei fill (lacking balance to fully fill it) for a deposit and
// then later on in the run have enough balance to fully fill it.
const fillsInQueueForSameDeposit = this.clients.multiCallerClient
.getQueuedTransactions(deposit.destinationChainId)
.some(({ method, args }) => {
return (
(method === "fillRelay" && args[9] === depositId && args[6] === originChainId) ||
(method === "fillRelayWithUpdatedDeposit" && args[11] === depositId && args[7] === originChainId)
);
});

if (!fillAmount.eq(deposit.amount) || fillsInQueueForSameDeposit) {
if (!fillAmount.eq(deposit.amount)) {
const originChain = getNetworkName(originChainId);
const destinationChain = getNetworkName(destinationChainId);
this.logger.debug({
Expand Down
4 changes: 1 addition & 3 deletions test/Relayer.BasicFill.ts
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,6 @@ describe("Relayer: Check for Unfilled Deposits and Fill", async function () {
tokenClient = new TokenClient(spyLogger, relayer.address, spokePoolClients, hubPoolClient);
profitClient = new MockProfitClient(spyLogger, hubPoolClient, spokePoolClients, []);
profitClient.setTokenPrice(l1Token.address, bnOne);
Object.values(spokePoolClients).map((spokePoolClient) => profitClient.setGasCost(spokePoolClient.chainId, bnOne));

relayerInstance = new Relayer(
relayer.address,
Expand Down Expand Up @@ -411,8 +410,7 @@ describe("Relayer: Check for Unfilled Deposits and Fill", async function () {
// 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()]);
await relayerInstance.checkForUnfilledDepositsAndFill();
// Only still 1 transaction.
expect(multiCallerClient.transactionCount()).to.equal(1); // no Transactions to send.
expect(multiCallerClient.transactionCount()).to.equal(0); // no new transactions were enqueued.
});

it("Respects configured relayer routes", async function () {
Expand Down
2 changes: 2 additions & 0 deletions test/Relayer.TokenShortfall.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { utils as sdkUtils } from "@across-protocol/sdk-v2";
import {
AcrossApiClient,
ConfigStoreClient,
Expand Down Expand Up @@ -109,6 +110,7 @@ describe("Relayer: Token balance shortfall", async function () {
const spokePoolClients = { [originChainId]: spokePoolClient_1, [destinationChainId]: spokePoolClient_2 };
tokenClient = new TokenClient(spyLogger, relayer.address, spokePoolClients, hubPoolClient);
profitClient = new MockProfitClient(spyLogger, hubPoolClient, spokePoolClients, []);
profitClient.setTokenPrice(l1Token.address, sdkUtils.bnOne);

relayerInstance = new Relayer(
relayer.address,
Expand Down
9 changes: 4 additions & 5 deletions test/mocks/MockProfitClient.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { utils as sdkUtils } from "@across-protocol/sdk-v2";
import { GAS_TOKEN_BY_CHAIN_ID, HubPoolClient, ProfitClient, WETH } from "../../src/clients";
import { SpokePoolClientsByChain } from "../../src/interfaces";
import { BigNumber, toBNWei, winston } from "../utils";
import { BigNumber, winston } from "../utils";

export class MockProfitClient extends ProfitClient {
constructor(
Expand All @@ -24,10 +24,9 @@ export class MockProfitClient extends ProfitClient {
);

// Some tests run against mocked chains, so hack in the necessary parts
const chainIds = [666, 1337];
chainIds.forEach((chainId) => {
GAS_TOKEN_BY_CHAIN_ID[chainId] = WETH;
this.setGasCost(chainId, toBNWei(chainId));
Object.values(spokePoolClients).map(({ chainId }) => {
this.setGasCost(chainId, sdkUtils.bnOne);
GAS_TOKEN_BY_CHAIN_ID[chainId] ??= WETH;
});
this.setTokenPrice(WETH, sdkUtils.bnOne);
}
Expand Down

0 comments on commit 035dbe6

Please sign in to comment.