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

Conversation

nicholaspai
Copy link
Member

The MulticallerClient has logic that batches transactions together and attempts to multicall() them. This works normally with the HubPool and SpokePool which extend the Multicaller() interface.

However, we can't use this logic to make calls on ERC20's like we are doing to fund the DonationBox. Instead, we either need to update the MulticallerClient to handle cases where transactions are not multicall-able, or we can send these transactions to the TransactionClient.

In this PR, I added a new method simulateAndSubmit to the TransactionClient that the Dataworker uses to transfer custom gas tokens to the DonationBox.

The MulticallerClient has [logic](https://github.com/across-protocol/relayer/blob/40b0c7971a3548a0499e7fc5990d150437c527fb/src/clients/MultiCallerClient.ts#L361) that batches transactions together and attempts to `multicall()` them. This works normally with the `HubPool` and `SpokePool` which extend the `Multicaller()` interface.

However, we can't use this logic to make calls on ERC20's like we are doing to fund the `DonationBox`. Instead, we either need to update the `MulticallerClient` to handle cases where transactions are not multicall-able, or we can send these transactions to the `TransactionClient`.

In this PR, I added a new method `simulateAndSubmit` to the `TransactionClient` that the Dataworker uses to `transfer` custom gas tokens to the DonationBox.
melisaguevara
melisaguevara previously approved these changes Dec 30, 2024
Copy link
Contributor

@melisaguevara melisaguevara left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm!

test/Dataworker.executePoolRebalanceUtils.ts Outdated Show resolved Hide resolved
melisaguevara
melisaguevara previously approved these changes Dec 30, 2024
@nicholaspai nicholaspai requested a review from bmzig December 31, 2024 15:17
Copy link
Contributor

@bmzig bmzig left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One minor question.

@nicholaspai nicholaspai requested a review from bmzig December 31, 2024 19:39
bmzig
bmzig previously approved these changes Dec 31, 2024
Copy link
Contributor

@james-a-morris james-a-morris left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

one comment otherwise lgtm

src/clients/TransactionClient.ts Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants