Skip to content

Commit

Permalink
fix(Dataworker): Account for total required netSendAmount when execut…
Browse files Browse the repository at this point in the history
…ing PoolRebalanceLeaves (#1933)

* fix(Dataworker): Update balanceAllocator properly when executing PoolRebalanceLeaves

## Context

The dataworker executor functionality is supposed to detect when to call `sync()` before executing L1 PoolRebalance and RelayerRefund leaves depending on the `liquidReserves` value of l1 tokens before executing those leaves.

We use pass around the `balanceAllocator` when simulating execution of the [PoolRebalanceLeaves](https://github.com/across-protocol/relayer/blob/3e9c1e4108568b39fe007f8fcd71721db4bbe090/src/dataworker/Dataworker.ts#L1491) and the [RelayerRefundLeaves](https://github.com/across-protocol/relayer/blob/3e9c1e4108568b39fe007f8fcd71721db4bbe090/src/dataworker/Dataworker.ts#L1512) in order to keep track of how many L1 tokens are withdrawn from and deposited to the HubPool following Hub-chain leaf executions.

This way, we can use the `balanceAllocator` in [this function](https://github.com/across-protocol/relayer/blob/3e9c1e4108568b39fe007f8fcd71721db4bbe090/src/dataworker/Dataworker.ts#L1531) to detect when we're not going to have enough funds in LP reserves to execute a leaf [here](https://github.com/across-protocol/relayer/blob/3e9c1e4108568b39fe007f8fcd71721db4bbe090/src/dataworker/Dataworker.ts#L1823).

## Problem

The problem is that when accounting for PoolRebalanceLeaf executions, we were ADDING not subtracting balance to the balanceAllocator's count of the hubpool's reserves. This means that if the current `liquidReserves` were good enough to cover execution of the Ethereum PoolRebalanceLeaf [here](https://github.com/across-protocol/relayer/blob/3e9c1e4108568b39fe007f8fcd71721db4bbe090/src/dataworker/Dataworker.ts#L1484), then the balance allocator would accidentally inflate the HubPool's balance [here](https://github.com/across-protocol/relayer/blob/3e9c1e4108568b39fe007f8fcd71721db4bbe090/src/dataworker/Dataworker.ts#L1488).

This function [here](https://github.com/across-protocol/relayer/blob/3e9c1e4108568b39fe007f8fcd71721db4bbe090/src/dataworker/Dataworker.ts#L1529C40-L1529C92) would then have issues. Within this function, if any individual PoolRebalanceLeaf's `netSendAmount` was less than its liquid reserves, then a `sync` would be skipped [here](https://github.com/across-protocol/relayer/blob/3e9c1e4108568b39fe007f8fcd71721db4bbe090/src/dataworker/Dataworker.ts#L1802). This [line](https://github.com/across-protocol/relayer/blob/3e9c1e4108568b39fe007f8fcd71721db4bbe090/src/dataworker/Dataworker.ts#L1822) would then artificially inflate the hub pool's balance in the balance allocator, leading to a much more down stream simulation error when the pool rebalance leaf execution fails for unknown reasons.

## Examples of problems

We saw a bundle of PoolRebalanceLeaves today fail to execute because three of the leaves, one Ethereum leaf and two non-Ethereum leaves, had a total `netSendAmount` greater than the HubPool's `liquidReserves`, but each individually had a `netSendAmount` < the `liquidReserves`. For example, the three leaves had `netSendAmounts` of:
- 40
- 90
- 70

While the hubPool's liquidReserves was 180:
- 40 + 90 + 70 = 200 > 180
- 40 < 180
- 90 < 180
- 70 < 180

If you take these numbers and run them through the `executePoolRebalanceLeaves` code above, you'll see how a PoolRebalanceLeaf execution was able to be submitted but then fail in simulation, without preceding the leaf executions with a `sync` transaction.

* fix issue

* Update Dataworker.executePoolRebalances.ts

* Add more test cases

* Update Dataworker.executePoolRebalances.ts

* Update Monitor.ts

* comment on tests

* make test better

* Add orbit-fee handling, remove balance allocator

* Fix balanceAllocator call in _executePoolRebalanceLeaves

* throw error if can't fund the DonationBox or loadEthForL2Calls call

* add lifecycle test

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

* Exit early if aggregate net send amount == 0

* Update Dataworker.executePoolRebalances.ts

* Update log in _updateExchangeRatesBeforeExecutingNonHubChainLeaves when skipping exchange rate update early

* Update Dataworker.ts

* Fund more AZERO whenever we're short

* remove hardcodes

* Improve logs about lookback window being too short

* Improve logs on funding orbit chain message

* Update Dataworker.customSpokePoolClients.ts

* Update index.ts

* Update index.ts

* Add invariant unit test

* Remove l1 tokens with 0 net send amounts from _updateOldExchangeRates

* Rename to amountWei

* Refactor blockRangesAreInvalid to internal helper func

* Squash feeData

* Update src/dataworker/Dataworker.ts

Co-authored-by: Paul <[email protected]>

* Update src/dataworker/Dataworker.ts

Co-authored-by: Paul <[email protected]>

* Update src/dataworker/Dataworker.ts

Co-authored-by: Paul <[email protected]>

* result

* Add unit testing about exiting early if leaves are already executed

* Add ability for some nonHubChain leaves to be executed even if they all cannot

* Skip mainnet leaf execution if we cannot execute instead of throwing

* Skip sync in _updateExchangeRatesBeforeExecutingNonHubChainLeaves if liquid reserves won't increase

* refactor block range pretty printing

* update comments

* Add assert error messages

* Add _getSpokeBalanceForL2Tokens helper and add to logs

* Re-add balance allocator

* Update Dataworker.executeRelayerRefunds.ts

* Update Dataworker.ts

* Remove canExecute return value from  _updateExchangeRatesBeforeExecutingHubChainLeaves

* Update Dataworker.executePoolRebalances.ts

* Update Dataworker.executePoolRebalances.ts

* Refactor error log

* Clean up logs

* Consider state of liquid reserves following eth pool rebalance leaf executions

* Improve tests

* Update name

* Add unit test, split executePoolRebalanceLeaf tests in two files to take advantage of parallel test runs in CI

* Remove SIMULATE_L1_EXECUTION

* Add test for amountToReturn

* add tests

* Add test about hub chain slow fill leaves

* Update BalanceAllocator.ts

Co-authored-by: Paul <[email protected]>

* Update Dataworker.ts

Co-authored-by: Paul <[email protected]>

* change blockRangesAreInvalidForSpokeClients to return list of chain ID's that are invalid; add DISABLED_CHAINS unit tests to BundleDataClient unit test files

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

---------

Signed-off-by: nicholaspai <[email protected]>
Co-authored-by: Paul <[email protected]>
  • Loading branch information
nicholaspai and pxrl authored Dec 16, 2024
1 parent 71c609d commit 79c6f70
Show file tree
Hide file tree
Showing 16 changed files with 2,302 additions and 848 deletions.
12 changes: 12 additions & 0 deletions src/clients/BalanceAllocator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,12 @@ export class BalanceAllocator {
return this.requestBalanceAllocations([{ chainId, tokens, holder, amount }]);
}

async getBalanceSubUsed(chainId: number, token: string, holder: string): Promise<BigNumber> {
const balance = await this.getBalance(chainId, token, holder);
const used = this.getUsed(chainId, token, holder);
return balance.sub(used);
}

async getBalance(chainId: number, token: string, holder: string): Promise<BigNumber> {
if (!this.balances?.[chainId]?.[token]?.[holder]) {
const balance = await this._queryBalance(chainId, token, holder);
Expand All @@ -114,6 +120,12 @@ export class BalanceAllocator {
return this.balances[chainId][token][holder];
}

testSetBalance(chainId: number, token: string, holder: string, balance: BigNumber): void {
this.balances[chainId] ??= {};
this.balances[chainId][token] ??= {};
this.balances[chainId][token][holder] = balance;
}

getUsed(chainId: number, token: string, holder: string): BigNumber {
if (!this.used?.[chainId]?.[token]?.[holder]) {
// Note: cannot use assign because it breaks the BigNumber object.
Expand Down
28 changes: 28 additions & 0 deletions src/common/Constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -611,3 +611,31 @@ export const DEFAULT_GAS_MULTIPLIER: { [chainId: number]: number } = {
};

export const CONSERVATIVE_BUNDLE_FREQUENCY_SECONDS = 3 * 60 * 60; // 3 hours is a safe assumption for the time

export const ARBITRUM_ORBIT_L1L2_MESSAGE_FEE_DATA: {
[chainId: number]: {
// Amount of tokens required to send a single message to the L2
amountWei: number;
// Multiple of the required amount above to send to the feePayer in case
// we are short funds. For example, if set to 10, then everytime we need to load more funds
// we'll send 10x the required amount.
amountMultipleToFund: number;
// Account that pays the fees on-chain that we will load more fee tokens into.
feePayer?: string;
// Token that the feePayer will pay the fees in.
feeToken?: string;
};
} = {
// Leave feePayer undefined if feePayer is HubPool.
// Leave feeToken undefined if feeToken is ETH.
[CHAIN_IDs.ARBITRUM]: {
amountWei: 0.02,
amountMultipleToFund: 1,
},
[CHAIN_IDs.ALEPH_ZERO]: {
amountWei: 0.49,
amountMultipleToFund: 20,
feePayer: "0x0d57392895Db5aF3280e9223323e20F3951E81B1", // DonationBox
feeToken: TOKEN_SYMBOLS_MAP.AZERO.addresses[CHAIN_IDs.MAINNET],
},
};
Loading

0 comments on commit 79c6f70

Please sign in to comment.