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(TransactionUtils): Use sdk-v2 gasPriceOracle #742

Merged
merged 8 commits into from
Aug 14, 2023

Conversation

pxrl
Copy link
Contributor

@pxrl pxrl commented Jun 16, 2023

The gasPriceOracle was recently upgraded to support Polygon's gas station.

As a nice side-effect, we can remove node-fetch from our list of dependencies.

Fixes ACX-1434.

The gasPriceOracle was recently upgraded to support Polygon's gas
station.

As a nice side-effect, we can remove node-fetch from our list of
dependencies.
Base automatically changed from pxrl/runningBalances to master June 16, 2023 15:32
@pxrl pxrl marked this pull request as ready for review August 14, 2023 13:52
@linear
Copy link

linear bot commented Aug 14, 2023

ACX-1434 Test: relayer gas estimates for zkSync are sky high

As seen below, the relayer sets a max fee of $12.65 and has $12.41 refunded. The relayer is fortunately not over-spending due to the refund, but with profitability enabled, this will make almost all fills seem unprofitable.

We can disable the profitability logic on zkSync entirely, or we can optimistically migrate to using the gasPriceOracle from sdk-v2: https://github.com/across-protocol/relayer-v2/pull/742

The mispricing issue does not seem to be present in the FE, which uses the gasPriceOracle directly. So we might resolve the issue simply by migrating.

https://explorer.zksync.io/tx/0xc155229e19f1633fc0dfb9b94f525810280d5a52e0ebbb8676f6fa171950e45d

2023-08-14-154955_914x216_scrot.png

FE:

2023-08-14-155453_608x752_scrot.png

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.

I think this is WiP so I'll hold off.

src/utils/TransactionUtils.ts Show resolved Hide resolved
james-a-morris and others added 2 commits August 14, 2023 21:35
* feat: ⚡ convert BaseAdapter to abstract class

This change converts the BaseAdapter class to an abstract class. This way, we can add common function signatures as abstract functions which need to be implemented in order for the child class to inherit from BaseAdapter.

* feat: scaffold of ZKSyncAdapter

* improve: add additional scaffolding

* improve: zksync usage in relayer adapter

* improve: add zksync

* improve: add zksync changes

* improve: add documentation

* Back out zksync import

* fix

* Implement zksync wrap eth func

* improve: Centralise hardcoded Hub Chain ID

* lint

* Fix address comparison

* Simplify calls to wrapEthIfAboveThreshold

* Condense setL1TokenApprovals

* improve: Remove additional chain ID hardcoding

Also, reconstruct the zkSync provider from the existing SpokePool
provider URL.

* Fix previous

* Be optimistic about case compatibility

* nit: return valid result

* Add getters for L2 bridge and messaging contracts

* improve: resolve rpc utils to ZKSync

Co-authored-by: Paul <108695806+pxrl@users.noreply.github.com>

* improve: add filtration to l1TokenEvents

* improve: package upgrade

* feat: add contract addresses

* improve: resolve all finalization events

* chore: use alternative fn call

* feat: add required addresses

* chore: add docs

* docs: add comments

* improve: use supported chains

* chore: docs

* chore: remove async remnant

* chore: remove unneeded fn

* improve: remove zksync wallet ref

* fix: remove testing condition

* feat: consolidate supported logic (#870)

* feat: consolidate supported logic

* chore: include optimism

* improve: set adapter to remove duplicates

* feat: add helpful util

* chore: remove address option in supported symbols

* chore: conform supported tokens

* chore: remove comment and enable weth explicitly

* improve: make `isSupportedToken` verbose

* improve: revert logic for weth

* improve: make uppercase_snake case

* Update src/clients/bridges/BaseAdapter.ts

Co-authored-by: nicholaspai <9457025+nicholaspai@users.noreply.github.com>

---------

Co-authored-by: nicholaspai <9457025+nicholaspai@users.noreply.github.com>

* Fix contracts

* Use atomic weth depositor

* feat: Add ZkSync function to AtomicWethDepositor

* Update src/clients/bridges/ArbitrumAdapter.ts

* Update AtomicWethDepositor.sol

* Update AtomicWethDepositor.sol

* implement getCrossChainTransfers

* proposed redesign

Signed-off-by: Matt Rice <matthewcrice32@gmail.com>

* slight refactor

Signed-off-by: Matt Rice <matthewcrice32@gmail.com>

* improve: make supported check more robust

* improve: rename fn

* WIP

* fix tests

* Fix

* Update ZKSyncAdapter.ts

* Refactor and fix tests

* Update BaseAdapter.ts

* Update atomic depositor address

* James comments

* WIP

* Update yarn.lock

---------

Signed-off-by: Matt Rice <matthewcrice32@gmail.com>
Co-authored-by: Paul <108695806+pxrl@users.noreply.github.com>
Co-authored-by: nicholaspai <npai.nyc@gmail.com>
Co-authored-by: nicholaspai <9457025+nicholaspai@users.noreply.github.com>
Co-authored-by: Matt Rice <matthewcrice32@gmail.com>
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.

LGTM

pxrl and others added 2 commits August 14, 2023 22:41
Co-authored-by: James Morris, MS <96435344+james-a-morris@users.noreply.github.com>
@pxrl pxrl merged commit d35c4a4 into master Aug 14, 2023
2 checks passed
@pxrl pxrl deleted the pxrl/gasPriceOracle branch August 14, 2023 21:31
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