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(generic bridges): use scroll's canonical bridge function #1708

Merged
merged 12 commits into from
Jul 30, 2024

Conversation

bmzig
Copy link
Contributor

@bmzig bmzig commented Jul 26, 2024

This fixes an improperly set method on Arbitrum (not sending any msg.value to the outboundTransfer function), as well as removes ScrollWethBridge in favor of the ScrollERC20Bridge. The latter is done since we can simply bridge WETH to Scroll via the gateway router, which can subsequently be wrapped/unwrapped, so a dedicated bridge to interface with an atomic depositor is not needed.


As an overview. We recently merged in the generic adapters PR #1524, which at the time of writing, is dormant in production, since the generic AdapterManager in src/adapter/ is only activated if we set RELAYER_USE_GENERIC_ADAPTER and/or MONITOR_USE_GENERIC_ADAPTER in the environment. Now that it is merged, instead of making an entirely new ScrollAdapter, we can make a wrapper for the generic BaseChainAdapter and begin integrating the new code into our current structure. Going forward, we should continue to replace the existing specialized adapters (e.g. ArbitrumAdapter, LineaAdapter, ZkSyncAdapter, etc.) with wrappers for BaseChainAdapter using the same format found here

Object.values(this.spokePoolClients)
. Once all the adapters are replaced with their generic counterparts, we should be able to remove everything in src/clients/bridges/ except for CrossChainTransferClient, which will remain unchanged.

@nicholaspai
Copy link
Member

(although this shouldn't be preferred since we would receive WETH, not ETH, on Scroll).

I'm confused by this. We should be happy to send WETH, because the bots should know how to unwrap WETH into ETH, right? This function should be run here before evaluating any WETH rebalances, allowing the bots to always assume it has enough ETH in its balance before evaluating WETH inventory sends.

From the inventory manager's perspective, ETH is treated as gas balance and WETH is an inventory balance

@bmzig bmzig marked this pull request as ready for review July 26, 2024 21:58
Copy link
Member

@nicholaspai nicholaspai left a comment

Choose a reason for hiding this comment

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

Couple questions, but nice to remove the Scroll(Atomic)WethBridge!

@bmzig
Copy link
Contributor Author

bmzig commented Jul 29, 2024

ERC20 transfer tracking is working now.

@bmzig bmzig requested a review from nicholaspai July 29, 2024 18:19
Signed-off-by: bennett <[email protected]>
@bmzig bmzig requested a review from pxrl July 29, 2024 19:07
Copy link
Contributor

@pxrl pxrl left a comment

Choose a reason for hiding this comment

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

I've tested this OK when rebalancing all 4 supported tokens. Cross-chain transfer tracking worked correctly. There are some items here that could be revisited for a post-merge cleanup, but for now this is OK to go in.

@pxrl pxrl merged commit 0be6231 into master Jul 30, 2024
4 checks passed
@pxrl pxrl deleted the bz/scrollAdapterFix branch July 30, 2024 10:25
ishantiw pushed a commit to LiskHQ/across-relayer that referenced this pull request Aug 8, 2024
…protocol#1708)

Fix Scroll cross-chain adapters:
 - Use the native ERC20 bridge for WETH, rather than the 
   AtomicWethDepositor.
 - Correct the gateways used for USDC transfer tracking.
 - Add Scroll tests.
 - Fix an issue in the new Arbitrum cross-chain adapter. This new 
   adapter is not active in prod yet.

---------

Signed-off-by: bennett <[email protected]>
ishantiw pushed a commit to LiskHQ/across-relayer that referenced this pull request Aug 8, 2024
…protocol#1708)

Fix Scroll cross-chain adapters:
 - Use the native ERC20 bridge for WETH, rather than the 
   AtomicWethDepositor.
 - Correct the gateways used for USDC transfer tracking.
 - Add Scroll tests.
 - Fix an issue in the new Arbitrum cross-chain adapter. This new 
   adapter is not active in prod yet.

---------

Signed-off-by: bennett <[email protected]>
sameersubudhi pushed a commit to LiskHQ/across-relayer that referenced this pull request Sep 6, 2024
…protocol#1708)

Fix Scroll cross-chain adapters:
 - Use the native ERC20 bridge for WETH, rather than the 
   AtomicWethDepositor.
 - Correct the gateways used for USDC transfer tracking.
 - Add Scroll tests.
 - Fix an issue in the new Arbitrum cross-chain adapter. This new 
   adapter is not active in prod yet.

---------

Signed-off-by: bennett <[email protected]>
sameersubudhi pushed a commit to LiskHQ/across-relayer that referenced this pull request Sep 10, 2024
…protocol#1708)

Fix Scroll cross-chain adapters:
 - Use the native ERC20 bridge for WETH, rather than the 
   AtomicWethDepositor.
 - Correct the gateways used for USDC transfer tracking.
 - Add Scroll tests.
 - Fix an issue in the new Arbitrum cross-chain adapter. This new 
   adapter is not active in prod yet.

---------

Signed-off-by: bennett <[email protected]>
sameersubudhi pushed a commit to LiskHQ/across-relayer that referenced this pull request Sep 10, 2024
…protocol#1708)

Fix Scroll cross-chain adapters:
 - Use the native ERC20 bridge for WETH, rather than the 
   AtomicWethDepositor.
 - Correct the gateways used for USDC transfer tracking.
 - Add Scroll tests.
 - Fix an issue in the new Arbitrum cross-chain adapter. This new 
   adapter is not active in prod yet.

---------

Signed-off-by: bennett <[email protected]>
sameersubudhi pushed a commit to LiskHQ/across-relayer that referenced this pull request Sep 10, 2024
…protocol#1708)

Fix Scroll cross-chain adapters:
 - Use the native ERC20 bridge for WETH, rather than the 
   AtomicWethDepositor.
 - Correct the gateways used for USDC transfer tracking.
 - Add Scroll tests.
 - Fix an issue in the new Arbitrum cross-chain adapter. This new 
   adapter is not active in prod yet.

---------

Signed-off-by: bennett <[email protected]>
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.

3 participants