refactor: Simplify Monitor balance reporting and delete TokenTransferClient#3111
refactor: Simplify Monitor balance reporting and delete TokenTransferClient#3111nicholaspai merged 14 commits intomasterfrom
Conversation
…inance deposit matching, and floatToBN fix - TokenUtils: getInventoryEquivalentL1TokenAddress, getInventoryBalanceContributorTokens, isL2OnlyEquivalentToken - RunningBalanceUtils: getLatestRunningBalances extracted for shared use - BinanceUtils: getOutstandingBinanceDeposits for cross-L2 deposit matching, export BinanceDeposit type - BNUtils: Fix floatToBN MAX_SAFE_INTEGER overflow via string manipulation Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…t, InventoryClient, TokenClient - BundleDataApproxClient: returns values in L1 token decimals, groups deposits/refunds across inventory-equivalent L2 tokens, restores requireExecution param, fixes refund decimal conversion - InventoryClient: uses getInventoryEquivalentL1TokenAddress, getInventoryBalanceContributorTokens, getLatestRunningBalances; adds protected getTokenInfo for test mocking - TokenClient: uses getInventoryBalanceContributorTokens for L2 token discovery - Test updates: alias config for USDC, L1-decimal refunds, MockInventoryClient getTokenInfo override Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- BinanceCEXBridge: rewrite getL2PendingWithdrawalAmount to correctly compute outstanding deposits when finalizer batches deposits from multiple L2s into single L1 withdrawal. Uses getOutstandingBinanceDeposits with Redis-cached deposit sender lookups. - BaseChainAdapter: use getInventoryEquivalentL1TokenAddress, skip zero-amount outstanding transfers in getOutstandingCrossChainTransfers - AdapterManager: minor cleanup - MockLineaEvents: accept amount parameter for BridgingInitiatedV2/BridgingFinalizedV2 - Generic adapter tests: updated for zero-amount skip behavior Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…Client - Monitor.reportRelayerBalances: rewritten to use getInventoryBalanceContributorTokens for per-chain L2 token discovery, batch-fetch all balances in parallel, produce aligned table per token. Adds protected getTokenInfo for test mocking. Fixes pending rebalance double-count by only applying to canonical L2 token. - MonitorConfig: removes L2Token interface, l2OnlyTokens, L2_ONLY_TOKENS env var - MonitorClientHelper: removes TokenTransferClient construction - Delete TokenTransferClient (no longer needed) - Delete unused BalanceType/RelayerBalanceReport/TokenTransfer interfaces Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1227d02411
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (isDefined(this.clients.rebalancerClient)) { | ||
| pendingRebalances = await this.clients.rebalancerClient.getPendingRebalances(); |
There was a problem hiding this comment.
Guard pending rebalance fetch to keep monitor loop alive
This call now runs without any error handling, so a transient failure inside rebalancerClient.getPendingRebalances() (for example from an external bridge/CEX adapter) will throw out of reportRelayerBalances and abort the entire monitor iteration. The previous flow handled this path defensively and continued with zero pending rebalances, so this is a resilience regression that can suppress unrelated monitoring checks.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
This shouldn't error and if it does we want to know
|
@codex re review |
Summary
Testing
|
- MonitorClientHelper: Should filter out duplicate spoke pool address strings passed into AdapterManager - AdapterManager: should filter out hub/spoke pool addresses from monitored addresses passed to BaseChainAdapter if the token doesn't have a pool rebalance route to that chain. - BaseChainAdapter: computes outstanding l1->l2 transfer amount using net amount approach rather than marking off events one by one, which better supports adapters with Binance which don't have this 1:1 relationship assumption
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fe5e120254
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
src/adapter/BaseChainAdapter.ts
Outdated
| const totalAmount = totalDepositedAmount.sub(totalFinalizedAmount); | ||
| let remainingUnfinalizedAmount = totalAmount; | ||
| if (remainingUnfinalizedAmount.gt(0)) { | ||
| for (const depositEvent of depositInitiatedEvents) { |
There was a problem hiding this comment.
Exclude ignored bridge tx refs when attributing outstanding hashes
The net outstanding amount is computed from deposits filtered by ignoredPendingBridgeTxnRefs, but the tx-hash attribution loop still iterates the unfiltered depositInitiatedEvents. In cases where ignored bridge txs are present, those ignored refs can be recorded as outstanding (and consume remainingUnfinalizedAmount) while real outstanding txs are omitted, which breaks diagnostics and any alerting that relies on depositTxHashes.
Useful? React with 👍 / 👎.
Summary
Testing
|
Summary
Major simplification of the Monitor's balance reporting and spoke pool running balance logic:
TokenTransferClient: Removed entirely — it was only used by Monitor and added complexity by querying ERC20 Transfer events per-chain. Balance reporting now uses direct on-chain balance queries instead.reportRelayerBalances(): Replaced the old multi-pass approach (initializeBalanceReports→updateCurrentRelayerBalances→updateLatestAndFutureRelayerRefunds→ helpers for cross-chain transfers, L2 withdrawals, pending rebalances) with a single streamlined loop that:getInventoryBalanceContributorTokens()to discover L2 tokens per (l1Token, chainId) paircheckSpokePoolRunningBalances()→reportSpokePoolRunningBalances(): Replaced the old approach that reconstructed pool rebalance roots and computed slow fill amounts from scratch with a call to the newgetLatestRunningBalances()utility, producing a clean per-token table.L2_ONLY_TOKENSconfig: TheL2Tokeninterface,l2OnlyTokensconfig parsing, and related reporting are deleted — L2-only tokens are now handled naturally viagetInventoryBalanceContributorTokens.BalanceType,RelayerBalanceReport,RelayerBalanceTable,RelayerBalanceCell,TokenTransfer,TransfersByChain,TransfersByTokens— all deleted.BaseChainAdaptermonitored addresses: Changed fromAddress[]to{ [l1Token: string]: Address[] }so adapters only query bridge events for addresses relevant to each token's pool rebalance route. This prevents unnecessary queries for hub/spoke pool addresses on chains without a route for a given token.AdapterManageraddress filtering: Hub/spoke pool addresses are now excluded from monitored addresses for tokens that don't have a pool rebalance route on that chain.BinanceCEXBridgedecimal handling: Finalization events now use L2 token decimals instead of L1 decimals, fixing incorrect amount conversions.BaseChainAdapter: Changed from per-event amount matching (which broke when L1/L2 decimals differed) to net-amount approach: sum all deposits, subtract all finalized amounts (with decimal conversion), then attribute the net outstanding amount to the newest unfinalized events.MonitorClientHelper: Prevents duplicate cross-chain transfer queries when multiple chains share the same spoke pool address.AdapterManagerso cross-chain transfer queries include relayer bridges.Test plan
test/Monitor.ts— removed tests for deleted methods (initializeBalanceReports,updateCurrentRelayerBalances,updateLatestAndFutureRelayerRefunds), simplified test monitor class toMockMonitorthat only overridesgetTokenInfofor test tokens🤖 Generated with Claude Code