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

improve(NetworkUtils): add chainIsL1Function #682

Merged
merged 3 commits into from
Jul 15, 2024
Merged

improve(NetworkUtils): add chainIsL1Function #682

merged 3 commits into from
Jul 15, 2024

Conversation

bmzig
Copy link
Contributor

@bmzig bmzig commented Jul 11, 2024

As we begin to introduce tryMulticall into the relayer, the MulticallerClient will need to be aware of whether it is calling a spoke pool or hub pool contract (to determine whether it can even call tryMulticall). One approach is to first check if the contract's chain ID we are calling is one which has a hub pool implementation. It would be ideal to introduce this code into the sdk, where there already exists a set of functions which performs similar checks with our other chain IDs.

Signed-off-by: bennett <bennett@umaproject.org>
@pxrl
Copy link
Contributor

pxrl commented Jul 12, 2024

One approach is to first check if the contract's chain ID we are calling is one which has a hub pool implementation.

wdyt about just checking for the existence of tryMulticall in the contract interface? I think something like that is still needed (or alternatively a check to verify that the target address is a SpokePool), and in the case of mainnet we still need to distinguish between Hub and Spoke, so I'm not sure how knowing whether the chain is mainnet is helpful.

@bmzig
Copy link
Contributor Author

bmzig commented Jul 12, 2024

One approach is to first check if the contract's chain ID we are calling is one which has a hub pool implementation.

wdyt about just checking for the existence of tryMulticall in the contract interface? I think something like that is still needed (or alternatively a check to verify that the target address is a SpokePool), and in the case of mainnet we still need to distinguish between Hub and Spoke, so I'm not sure how knowing whether the chain is mainnet is helpful.

The issue with this is that we get the contract interface from dist/typechain/factories/contracts in the contracts repo, so this will falsely claim that every spoke pool has tryMulticall (since the ABIs cached are of the most up-to-date contract). The other option is to fetch the deployed bytecode and look for tryMulticall function signature bytes in the code, but since this is asynchronous (and the context in which we want to check whether a contract has tryMulticall is synchronous), it would require some extra refactoring on the relayer + an RPC call (this also assumes that the bytes in tryMulticall's function selector do not appear anywhere else in the contract).

I am adding a function to determine if a contract is a hub pool or spoke pool, but this will be in across-protocol/relayer#1466.

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.

We should include the version bump before merging.

src/utils/NetworkUtils.ts Show resolved Hide resolved
Signed-off-by: bennett <bennett@umaproject.org>
Signed-off-by: bennett <bennett@umaproject.org>
@bmzig
Copy link
Contributor Author

bmzig commented Jul 15, 2024

We should include the version bump before merging.

I don't need this in now, so I'm actually going to just wait until this gets merged in, so that version bump will include this as well. #683

@bmzig bmzig merged commit 56eda28 into master Jul 15, 2024
4 checks passed
@bmzig bmzig deleted the bz/chainIsL1 branch July 15, 2024 15:44
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