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

Merge upstream changes and fix fork sync #53

Merged

Conversation

sameersubudhi
Copy link
Member

@sameersubudhi sameersubudhi commented Sep 6, 2024

What was the problem?

This PR resolves LISK-1054

How was it solved?

  • Merged upstream changes into current repo
  • Fixed fork sync config

How was it tested?

pxrl and others added 30 commits July 15, 2024 16:53
…ol#1663)

This single test file takes ~10 minutes to execute in CI and is by far
the longest running test. CI can be sped up by splitting part of this
test out into a separate file, such that they can be executed in
parallel. This reduces the runtime for this set of tests by
approximately 50%, and shaves ~3 minutes of the entire test run.

While here, apply a couple of minor type corrections.
across-protocol#1677)

Bumps [actions/dependency-review-action](https://github.com/actions/dependency-review-action) from 4.3.2 to 4.3.4.
- [Release notes](https://github.com/actions/dependency-review-action/releases)
- [Commits](actions/dependency-review-action@v4.3.2...v4.3.4)

---
updated-dependencies:
- dependency-name: actions/dependency-review-action
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [braces](https://github.com/micromatch/braces) from 3.0.2 to 3.0.3.
- [Changelog](https://github.com/micromatch/braces/blob/master/CHANGELOG.md)
- [Commits](micromatch/braces@3.0.2...3.0.3)

---
updated-dependencies:
- dependency-name: braces
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
There can be multiple tokens that map to the same L1 token (i.e. USDC,
USDC.e, USDbC). Deduplicate this array on instantiation to avoid issuing
duplicate requests.
…protocol#1561)

These defaults are quite old and were initially set conservatively. I've
tested locally w/ quorum 3 on the major providers and they all work
fine.
…col#1546)

DEFAULT_MIN_DEPOSIT_CONFIRMATIONS isn't actually used anymore, so drop
it, but migrate the chain finality defaults into
MIN_DEPOSIT_CONFIRMATIONS. Also, split out testnet chain IDs to be
dynamically generated based on predefined chains, but only when the 
relayer is not running on mainnet. This should make it a bit simpler to
update and maintain in future.
…1683)

Lite chains will have a tendency to creep over target and require manual
rebalancing. Additionally, taking voluntary repayment on a lite chain is
actually a net cost to the dataworker because it requires tokens to be
bridged to the lite chain, so it causes higher gas consumption for the 
deposit transactions on mainnet. Try to limit this by clamping the 
default overage buffer on lite chains to 1x, instead of the existing
default 1.5x.
…col#1679)

The relayer doesn't need to respect HubPool limits on these deposits.
Bumps [actions/checkout](https://github.com/actions/checkout) from 3 to 4.
- [Release notes](https://github.com/actions/checkout/releases)
- [Changelog](https://github.com/actions/checkout/blob/main/CHANGELOG.md)
- [Commits](actions/checkout@v3...v4)

---
updated-dependencies:
- dependency-name: actions/checkout
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
* feat(blast): add blast finalizer

Signed-off-by: bennett <[email protected]>

---------

Signed-off-by: bennett <[email protected]>
Co-authored-by: Paul <[email protected]>
…l#1685)

ContractAddresses pre-dates the existence of the external constants
repository, which now contains the "canonical" definitions for Across.
It's preferable to inherit the WETH definition from there because it
ensures that WETH addresses are consistent throughout the bot.
Additionally, it removes a step for adding a new chain.
…protocol#1686)

Only few chains can support chunk sizes of >50 in practice, and the
benefits of chunking so many are very limited (especially post-4844).
However, large chunk sizes have caused pain during times of high
network load (including causing stuck transactions on Linea), and they
impose an overhead when setting defaults for new chains. Instead, just
revert to a default of 50 and avoid having to mess with this in future.
…t chain choice (across-protocol#1680)

* feat(InventoryClient): Consider repayment amount in choosing repayment chain choice

The current algorithm for deciding which potential repayment chain choice to use does not take into account the `inputAmount` when deciding whether a chain's balance is under or over allocated.

This can lead to taking repayment on some chains that are currently under allocated but the fill is very large and so we swing very extreme to the over allocation status following the repayment

* Update InventoryClient.ts

* Update InventoryClient.ts

* Update InventoryClient.ts

* fix tests

* Remove inputAmount - outputAmount from chain balance calc
…ayment chain (across-protocol#1681)

Motivation is to reduce utilization by removing tokens from blast spoke pool that could be sent to the 14 day bridge. Should be merged in after across-protocol#1680 to reduce chances we build up a balance on Blast
* add scroll constants

Signed-off-by: bennett <[email protected]>

---------

Signed-off-by: bennett <[email protected]>
Co-authored-by: Paul <[email protected]>
A default of 3bps will result in the relayer rejecting almost every
fill.
This PR adds tests for the zkSync chain adapter. These tests apply to
the existing implementation, not the new one that is concurrently being
refined in another PR.
…s-protocol#1600)

* fix(Finalizer): Update to zksync-ethers instead of zksync-web3

This fixes an issue with the finalizer's use of the SDK's `isWithdrawalFinalized()` implementation, which ~seems to no longer work for withdrawals finalized after the v24 zksync upgrade~ no longer works.

 The reason is that the zksync-web3 SDK `isWithdrawalFinalized()` function no longer returns `true` when a withdrawal is finalized. This is counter intuitive but as you can see in the latest implementation (upgraded very recently, right before the v24 upgrade) of the L1ERC20Bridge, the Solidity comment explains how  this behavior was changed. The new way to check the withdrawal finalization status is to query the SharedBridge, which routes to the L1ERC20Bridge/Mailbox. The zksync-ethers SDK takes care of this for us.

## Implementation

This PR replaces zksync-web3 (deprecated) with zksync-ethers which nicely is a drop-in replacemet and loads the correct system contracts.

## DO NOT MERGE

Note, this PR should not go in until all ZkSync RPCs are confirmed to support the new upgrade and also all recent withdrawals are all post v24 upgrade. Currently, Quicknode and Alchemy's `zks_getBridgeContracts` do not return the correct `l1SharedDefaultBridge` value (returning it as `null` instead) which would break the `zksync-ethers` call.

```sh
> curl -X POST -H "Content-Type: application/json" \
--data '{"jsonrpc": "2.0", "id": 1, "method": "zks_getBridgeContracts", "params": [  ]}' \
"https://zksync-mainnet.g.alchemy.com/v2/<KEY>"

> {"jsonrpc":"2.0","id":1,"result":{"l1SharedDefaultBridge":null,"l2SharedDefaultBridge":"0x11f943b2c77b743ab90f4a0ae7d5a4e7fca3e102","l1Erc20DefaultBridge":"0x57891966931eb4bb6fb81430e6ce0a03aabde063","l2Erc20DefaultBridge":"0x11f943b2c77b743ab90f4a0ae7d5a4e7fca3e102","l1WethBridge":"0x0000000000000000000000000000000000000000","l2WethBridge":"0x0000000000000000000000000000000000000000"}}
```

* Update package.json

* Update yarn.lock

* remove ascii-table3

Signed-off-by: bennett <[email protected]>

* resync yarn.lock

Signed-off-by: bennett <[email protected]>

---------

Signed-off-by: bennett <[email protected]>
Co-authored-by: Paul <[email protected]>
Co-authored-by: bmzig <[email protected]>
Co-authored-by: bennett <[email protected]>
* chore: bump v20

Signed-off-by: james-a-morris <[email protected]>

* chore: update to v20

Signed-off-by: james-a-morris <[email protected]>

---------

Signed-off-by: james-a-morris <[email protected]>
Co-authored-by: Paul <[email protected]>
)

The ProfitClient currently assumes that the value of a fill can be
determined from the input token price * the output amount of the
output token. This is roughly correct for most Across deposits, but
falls over when the deposit includes an in-protocol swap to an output
token with a different number of decimals.

This was exposed on an Optimism deposit for inputToken USDC, outputToken
AERO on Base.

Deposit hash:
  0x4a3febcfb764467d9eae261b3a3938137db383289fe328050a2871eb407e1147
* improve(adapters): make adapters more generic

Signed-off-by: bennett <[email protected]>

This adds the `adapter` directory which contains all the code for the generic adapter upgrade to be rolled out. Upon this merge, it will still be dormant since it must be enabled via a config, which defaults to the legacy adapters.
---------

Signed-off-by: james-a-morris <[email protected]>
Signed-off-by: bennett <[email protected]>
Co-authored-by: Paul <[email protected]>
Co-authored-by: bmzig <[email protected]>
Co-authored-by: bennett <[email protected]>
Co-authored-by: nicholaspai <[email protected]>
…1698)

This is necessary to protect against attempting repeat fills on chains
where the transaction confirmation times can span multiple relayer
loops. Without this the relayer may try to fill the same deposit
multiple times. This was exposed by a third-party relayer running in
looping mode on mainnet.
…tocol#1697)

* fix(finalizer): Blast finalizer supports custom interface

The Blast OptimismPortal where `finalizeWithdrawalTransaction()` is called for OP stack L2->L1 withdrawals has a special interface. This PR handles the Blast case specially. Blast finalizations incur additional RPC requests to construct the correct parameters to make the call.

I've tested this PR out on production finalizations that work.

* Reuse addresses

---------

Co-authored-by: Paul <[email protected]>
james-a-morris and others added 17 commits August 28, 2024 15:55
* improve: remove reundant function definitions

Signed-off-by: james-a-morris <[email protected]>

* fix: formatting issue

Signed-off-by: james-a-morris <[email protected]>

* chore: linting errosr

Signed-off-by: james-a-morris <[email protected]>

---------

Signed-off-by: james-a-morris <[email protected]>
…l#1788)

The current ethers polling interval is 4 seconds. In case of waiting on
transaction confirmation, this probably imposes unnecessary delays in
the fast relayer when it has many fills to make.

This is done for test purposes to see whether relayer fill times improve
as a result. Subject to the results, this should be upstreamed to the 
SDK and made configurable, such that the fast relayer can opt-in to a
lower pollingInterval, and that it can tune the interval per chain.
…oss-protocol#1792)

* fix(BundleDataClient): Ignore slow fill requests for lite chains

Specifically, patch a couple of branches where we do not currently ignore slow fill requests:
- If a fill replaces a slow fill request for a deposit to or from a lite chain, we should never mark an excess slow fill amount to be returned to the Hub
- If an expired deposit refund supercedes a slow fill request we should never mark an excess slow fill amount to be returned to the Hub

because slow fill requests to or from lite chains are invalid

* Update BundleDataClient.ts
…ocol#1793)

If there are no deposits then it doesn't make sense to compute this.
Defer it in order to reduce log noise.
…#1794)

This is not used in prod and it introduces unnecessary branching logic,
so drop it to enable some subsequent refactoring.
…or manual intervention (across-protocol#1739)

* feat(validateRunningBalances): Add ability to log unexecuted leaves for manual intervention

Can use this script to log out unexecuted Refund leaves we can then use to manually execute

* Update validateRunningBalances.ts

* Update validateRunningBalances.ts
…ross-protocol#1790)

* improve(relayer): log warns based on time since last log

Signed-off-by: bennett <[email protected]>

---------

Signed-off-by: bennett <[email protected]>
Co-authored-by: Paul <[email protected]>
…tocol#1795)

SKIP_REBALANCING and SEND_REBALANCES are opposites, so there's no reason
to have support for both.
)

Canonical bridge approvals are only needed if rebalancing is enabled.
This is long overdue, but will be useful for testing of relayer
exclusivity.
…ross-protocol#1736)

* improve(adapters): Migrate Arbitrum One to generic adapter format

Signed-off-by: bennett <[email protected]>

---------

Signed-off-by: bennett <[email protected]>
…#1771)

As newer chains are added, we sometimes see that websockets are
occasionally dropped; potentially as a result of low activity on the
chain meaning that the websocket sits idle. This commit adds a simple
periodic keepalive that will be initiated by each listener process
towards each of its configured providers.
* chore: refactor provider utils to sdk

Signed-off-by: james-a-morris <[email protected]>

* chore: fix bug

Signed-off-by: james-a-morris <[email protected]>

* improve: remove unneeded packages

Signed-off-by: james-a-morris <[email protected]>

* chore: bump sdk

Signed-off-by: james-a-morris <[email protected]>

* chore: bump SDK to reflect changes

Signed-off-by: james-a-morris <[email protected]>

* chore: use local instance

Signed-off-by: james-a-morris <[email protected]>

* chore: re-migrate the timeout

Signed-off-by: james-a-morris <[email protected]>

---------

Signed-off-by: james-a-morris <[email protected]>
Co-authored-by: Paul <[email protected]>
@sameersubudhi sameersubudhi self-assigned this Sep 6, 2024
@sameersubudhi sameersubudhi force-pushed the LISK-1054-Merge-upstream-changes-and-fix-sync-fork branch from 811169d to ad96de6 Compare September 6, 2024 09:34
Copy link

@matjazv matjazv left a comment

Choose a reason for hiding this comment

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

🚀

@sameersubudhi sameersubudhi merged commit 14089d5 into master Sep 6, 2024
1 check passed
@sameersubudhi sameersubudhi deleted the LISK-1054-Merge-upstream-changes-and-fix-sync-fork branch September 6, 2024 12:25
sameersubudhi added a commit that referenced this pull request Sep 10, 2024
### What was the problem?

This PR resolves
[LISK-1054](https://onchaincollective.atlassian.net/browse/LISK-1054)

### How was it solved?

- [x] Merged upstream changes into current repo
- [x] Fixed fork sync config

### How was it tested?

- `yarn install --frozen-lockfile && yarn build && yarn relay --wallet
void --address <ADDRESS>`
- https://github.com/LiskHQ/across-relayer/actions/runs/10735103387
- https://github.com/LiskHQ/across-relayer/actions/runs/10735812996
-
LISK-1054-Merge-upstream-changes-and-fix-sync-fork...across-protocol:relayer:master


[LISK-1054]:
https://onchaincollective.atlassian.net/browse/LISK-1054?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ

---------

Signed-off-by: dependabot[bot] <[email protected]>
Signed-off-by: bennett <[email protected]>
Signed-off-by: james-a-morris <[email protected]>
Co-authored-by: Paul <[email protected]>
Co-authored-by: finaltrip <[email protected]>
Co-authored-by: omahs <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: bmzig <[email protected]>
Co-authored-by: nicholaspai <[email protected]>
Co-authored-by: James Morris, MS <[email protected]>
Co-authored-by: bennett <[email protected]>
Co-authored-by: Matt Rice <[email protected]>
Co-authored-by: Elias Rad <[email protected]>
sameersubudhi added a commit that referenced this pull request Sep 10, 2024
### What was the problem?

This PR resolves
[LISK-1054](https://onchaincollective.atlassian.net/browse/LISK-1054)

### How was it solved?

- [x] Merged upstream changes into current repo
- [x] Fixed fork sync config

### How was it tested?

- `yarn install --frozen-lockfile && yarn build && yarn relay --wallet
void --address <ADDRESS>`
- https://github.com/LiskHQ/across-relayer/actions/runs/10735103387
- https://github.com/LiskHQ/across-relayer/actions/runs/10735812996
-
LISK-1054-Merge-upstream-changes-and-fix-sync-fork...across-protocol:relayer:master


[LISK-1054]:
https://onchaincollective.atlassian.net/browse/LISK-1054?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ

---------

Signed-off-by: dependabot[bot] <[email protected]>
Signed-off-by: bennett <[email protected]>
Signed-off-by: james-a-morris <[email protected]>
Co-authored-by: Paul <[email protected]>
Co-authored-by: finaltrip <[email protected]>
Co-authored-by: omahs <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: bmzig <[email protected]>
Co-authored-by: nicholaspai <[email protected]>
Co-authored-by: James Morris, MS <[email protected]>
Co-authored-by: bennett <[email protected]>
Co-authored-by: Matt Rice <[email protected]>
Co-authored-by: Elias Rad <[email protected]>
sameersubudhi added a commit that referenced this pull request Sep 10, 2024
### What was the problem?

This PR resolves
[LISK-1054](https://onchaincollective.atlassian.net/browse/LISK-1054)

### How was it solved?

- [x] Merged upstream changes into current repo
- [x] Fixed fork sync config

### How was it tested?

- `yarn install --frozen-lockfile && yarn build && yarn relay --wallet
void --address <ADDRESS>`
- https://github.com/LiskHQ/across-relayer/actions/runs/10735103387
- https://github.com/LiskHQ/across-relayer/actions/runs/10735812996
-
LISK-1054-Merge-upstream-changes-and-fix-sync-fork...across-protocol:relayer:master


[LISK-1054]:
https://onchaincollective.atlassian.net/browse/LISK-1054?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ

---------

Signed-off-by: dependabot[bot] <[email protected]>
Signed-off-by: bennett <[email protected]>
Signed-off-by: james-a-morris <[email protected]>
Co-authored-by: Paul <[email protected]>
Co-authored-by: finaltrip <[email protected]>
Co-authored-by: omahs <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: bmzig <[email protected]>
Co-authored-by: nicholaspai <[email protected]>
Co-authored-by: James Morris, MS <[email protected]>
Co-authored-by: bennett <[email protected]>
Co-authored-by: Matt Rice <[email protected]>
Co-authored-by: Elias Rad <[email protected]>
@shuse2 shuse2 restored the LISK-1054-Merge-upstream-changes-and-fix-sync-fork branch September 10, 2024 14:52
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.