Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(Dataworker): Cannot fund DonationBox using MulticallerClient #1975

Open
wants to merge 12 commits into
base: master
Choose a base branch
from
Open
29 changes: 29 additions & 0 deletions src/clients/TransactionClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -121,4 +121,33 @@ export class TransactionClient {

return txnResponses;
}

async simulateAndSubmit(chainId: number, txns: AugmentedTransaction[]): Promise<TransactionResponse[]> {
const simulationResults = await this.simulate(txns);
const txnsToSubmit: AugmentedTransaction[] = [];
this.logger.debug({
at: "TransactionClient",
message: `Simulating ${txns.length} transactions before submitting the ones that pass simulation`,
});
for (let i = 0; i < simulationResults.length; i++) {
bmzig marked this conversation as resolved.
Show resolved Hide resolved
nicholaspai marked this conversation as resolved.
Show resolved Hide resolved
const { reason, succeed, transaction } = simulationResults[i];
const { contract: targetContract, ...txnRequestData } = transaction;
if (!succeed) {
this.logger.warn({
at: "TransactionClient",
message: `Failed to simulate calling ${transaction.method} on chain ${chainId}`,
reason,
contract: targetContract.address,
txnRequestData,
});
} else {
txnsToSubmit.push(txns[i]);
}
}
this.logger.debug({
at: "TransactionClient",
message: `Submitting ${txnsToSubmit.length} transactions that successfully simulated`,
});
return await this.submit(chainId, txnsToSubmit);
}
}
28 changes: 20 additions & 8 deletions src/dataworker/Dataworker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ import {
FillStatus,
} from "../interfaces";
import { DataworkerClients } from "./DataworkerClientHelper";
import { SpokePoolClient, BalanceAllocator, BundleDataClient } from "../clients";
import { SpokePoolClient, BalanceAllocator, BundleDataClient, TransactionClient } from "../clients";
import * as PoolRebalanceUtils from "./PoolRebalanceUtils";
import {
blockRangesAreInvalidForSpokeClients,
Expand Down Expand Up @@ -1690,13 +1690,25 @@ export class Dataworker {
value: requiredAmount,
});
} else {
this.clients.multiCallerClient.enqueueTransaction({
contract: new Contract(feeToken, ERC20.abi, signer),
chainId: hubPoolChainId,
method: "transfer",
args: [holder, requiredAmount],
message: `Loaded orbit gas token for message to ${getNetworkName(leaf.chainId)} 📨!`,
mrkdwn: `Root hash: ${tree.getHexRoot()}\nLeaf: ${leaf.leafId}\nChain: ${leaf.chainId}`,
// We can't use the multicaller client here because the feeToken is not guaranteed to be a Multicaller
// contract and this is a permissioned function where the msg.sender needs to be the
// feeToken balance owner.
nicholaspai marked this conversation as resolved.
Show resolved Hide resolved
const txnClient = new TransactionClient(this.logger);
const txnsToSubmit = [
{
contract: new Contract(feeToken, ERC20.abi, signer),
chainId: hubPoolChainId,
method: "transfer",
args: [holder, requiredAmount],
message: `Loaded orbit gas token for message to ${getNetworkName(leaf.chainId)} 📨!`,
mrkdwn: `Root hash: ${tree.getHexRoot()}\nLeaf: ${leaf.leafId}\nChain: ${leaf.chainId}`,
},
];
const result = (await txnClient.simulateAndSubmit(hubPoolChainId, txnsToSubmit))[0];
this.logger.debug({
at: "Dataworker#_executePoolRebalanceLeaves",
message: `Successfully funded ${requiredAmount.toString()} ${feeToken} tokens to ${holder}`,
hash: result.hash,
});
}
}
Expand Down
4 changes: 3 additions & 1 deletion src/finalizer/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -326,7 +326,9 @@ export async function finalize(

if (finalizations.length > 0) {
// @dev use multicaller client to execute batched txn to take advantage of its native txn simulation
// safety features
// safety features. This only works because we assume all finalizer transactions are
// unpermissioned (i.e. msg.sender can be anyone). If this is not true for any chain then we'd need to use
// the TransactionClient.
const multicallerClient = new MultiCallerClient(logger);
let txnHashLookup: Record<number, string[]> = {};
try {
Expand Down
21 changes: 13 additions & 8 deletions test/Dataworker.executePoolRebalanceUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -706,15 +706,20 @@ describe("Dataworker: Utilities to execute pool rebalance leaves", async functio
);
expect(result).to.equal(2);

// Should submit two transactions to load ETH for each leaf plus pool rebalance leaf execution.
expect(multiCallerClient.transactionCount()).to.equal(4);
// Should submit two pool rebalance leaf executions to multicaller client.
expect(multiCallerClient.transactionCount()).to.equal(2);
const queuedTransactions = multiCallerClient.getQueuedTransactions(hubPoolClient.chainId);
expect(queuedTransactions[0].method).to.equal("transfer");
expect(queuedTransactions[0].args).to.deep.equal([customGasTokenFunder, expectedFeeLeaf1]);
expect(queuedTransactions[1].method).to.equal("transfer");
expect(queuedTransactions[1].args).to.deep.equal([customGasTokenFunder, expectedFeeLeaf2]);
expect(queuedTransactions[2].method).to.equal("executeRootBundle");
expect(queuedTransactions[3].method).to.equal("executeRootBundle");
// expect(queuedTransactions[0].method).to.equal("transfer");
// expect(queuedTransactions[0].args).to.deep.equal([customGasTokenFunder, expectedFeeLeaf1]);
// expect(queuedTransactions[1].method).to.equal("transfer");
// expect(queuedTransactions[1].args).to.deep.equal([customGasTokenFunder, expectedFeeLeaf2]);
nicholaspai marked this conversation as resolved.
Show resolved Hide resolved
expect(queuedTransactions[0].method).to.equal("executeRootBundle");
expect(queuedTransactions[1].method).to.equal("executeRootBundle");

// Should also submit two transfer calls through the transaction client
const fundedLogs = spy.getCalls().filter((l) => l.lastArg.message.includes("Successfully funded"));
expect(fundedLogs[0].lastArg.message.includes(expectedFeeLeaf1.toString())).to.be.true;
expect(fundedLogs[1].lastArg.message.includes(expectedFeeLeaf2.toString())).to.be.true;
});
it("fails to fund custom gas token orbit leaf", async function () {
// Replicate custom gas token setups, but this time do not set a balance for the custom gas token funder.
Expand Down
32 changes: 32 additions & 0 deletions test/TransactionClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -142,4 +142,36 @@ describe("TransactionClient", async function () {
expect(txnResponse.gasLimit).to.equal(gasLimit.mul(idx + 1));
});
});

it("Simulate and Submit", async function () {
const chainId = chainIds[0];

// This transaction will fail to simulate:
const txns: AugmentedTransaction[] = [
{
chainId,
contract: { address } as Contract,
method,
args: [{ result: "Forced submission failure" }],
value: toBN(0),
mrkdwn: `Sample markdown string for chain ${chainId} transaction`,
},
];

// Should simulate all transactions and submit only the ones that pass simulation.
const successfulTxns = 2;
for (let i = 0; i < successfulTxns; i++) {
txns.push({
chainId,
contract: { address } as Contract,
method,
args: [{ result: txnClientPassResult }],
value: toBN(0),
mrkdwn: `Sample markdown string for chain ${chainId} transaction`,
});
}

const txnResponses = await txnClient.simulateAndSubmit(chainId, txns);
expect(txnResponses.length).to.equal(2);
});
});
Loading