Skip to content

improve: Use inventory equivalence remapping in BundleDataApproxClient, InventoryClient, TokenClient#3109

Merged
nicholaspai merged 7 commits intomasterfrom
npai/client-bundle-inventory
Mar 26, 2026
Merged

improve: Use inventory equivalence remapping in BundleDataApproxClient, InventoryClient, TokenClient#3109
nicholaspai merged 7 commits intomasterfrom
npai/client-bundle-inventory

Conversation

@nicholaspai
Copy link
Copy Markdown
Member

Summary

Updates the core inventory clients to use the new utility functions from #3108 (split from #3073).

  • BundleDataApproxClient: Returns values in L1 token decimals, groups deposits/refunds across inventory-equivalent L2 tokens, restores requireExecution param for deposit/refund lookback decoupling, fixes refund decimal conversion from origin token decimals
  • InventoryClient: Uses getInventoryEquivalentL1TokenAddress, getInventoryBalanceContributorTokens, getLatestRunningBalances; adds protected getTokenInfo for test mocking
  • TokenClient: Uses getInventoryBalanceContributorTokens for L2 token discovery
  • Tests: Alias config for USDC, L1-decimal refunds, MockInventoryClient.getTokenInfo override

Upstream dependencies

Downstream PRs

  • Monitor simplification (depends on this + adapter PR)

Test plan

  • BundleDataApproxClient (9 tests pass)
  • InventoryClient.InventoryRebalance (15 tests pass)
  • InventoryClient.RefundChain (41 tests pass)

🤖 Generated with Claude Code

nicholaspai and others added 2 commits March 25, 2026 11:36
…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>
correspondingProposedRootBundle,
chainId,
this.chainIdList
this.protocolChainIdIndices
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Note: this is a very important bug fix! Previously the chainIdList is not equal to the canonical chain ID indices list used on-chain by the proposer so the last validated end blocks are totally wrong for most chains

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 67fccda325

ℹ️ 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".

Comment on lines 230 to +232
for (const chainId of this.chainIdList) {
const repaymentToken = this.getRemoteTokenForL1Token(l1Token, chainId);
if (!repaymentToken) {
continue;
}
const { decimals: l2TokenDecimals } = this.hubPoolClient.getTokenInfoForAddress(repaymentToken, chainId);
const refundAmount = this.getUpcomingRefunds(chainId, l1Token, this.relayer);
const convertedRefundAmount = sdkUtils.ConvertDecimals(l2TokenDecimals, l1TokenDecimals)(refundAmount);
totalRefundsPerChain[chainId] = convertedRefundAmount;
totalRefundsPerChain[chainId] = refundAmount;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Invalidate pre-upgrade cache before using L1-unit refunds

getCumulativeBalanceWithApproximateUpcomingRefunds now treats getUpcomingRefunds as already normalized to L1 decimals, but cache import remains schema-compatible with older persisted bundleDataState. In src/relayer/index.ts, cached inventory is imported on startup and initialization is skipped when present, so pre-change cache entries (stored in repayment-token decimals) are interpreted with the new units. On tokens where repayment-token decimals differ from L1 decimals, this can mis-scale balances by large factors and drive incorrect refund-chain / inventory decisions until cache is rebuilt. Please add cache versioning/invalidation or migrate imported values.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@codex can you review the impact to BSC inventory decisions? Refund-chain selection mistakes don't have much impact in the short term.

@nicholaspai
Copy link
Copy Markdown
Member Author

@codex please review all unresolved threads

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1fecd57fd4

ℹ️ 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".

Base automatically changed from npai/utils-token-binance-bn to master March 25, 2026 21:13
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: aa34ea2408

ℹ️ 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".

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 7dc0dc0bff

ℹ️ 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".

@nicholaspai nicholaspai merged commit ce39568 into master Mar 26, 2026
4 checks passed
@nicholaspai nicholaspai deleted the npai/client-bundle-inventory branch March 26, 2026 15: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.

2 participants