Skip to content

fix(ui): use full balance on MAX for non-native tokens - #1141

Merged
ted-palmer merged 2 commits into
mainfrom
ted/int2-1408-max-leaves-gas-reserve-for-erc-20-sales
Sep 30, 2026
Merged

ted-palmer merged 2 commits into
mainfrom
ted/int2-1408-max-leaves-gas-reserve-for-erc-20-sales

Conversation

@ted-palmer

Copy link
Copy Markdown
Collaborator

No description provided.

@vercel

vercel Bot commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
relay-sdk-demo Ready Ready Preview Sep 30, 2026 4:23pm UTC

Request Review

@github-actions

github-actions Bot commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @ted-palmer's task in 24s —— View job


📝 Docs Drift Check — No docs impact

This PR does not touch surfaces that require a relayprotocol/relay-docs update.

@github-actions

github-actions Bot commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @ted-palmer's task in 52s —— View job


Code Review — No issues found.

Reviewed 3 files, ~46 lines changed. The refactor extracts the buffer math into utils/maxAmount.ts and makes MAX use the full balance for non-native tokens.

Verified:

  • Correctness — getExecutionBuffer is preserved verbatim from the old inline version. For non-native tokens getMaxAmount returns the full balance; the execution + fee buffers only apply when isFromNative. feeBufferAmount is already 0n unless isFromNative (line 64), so isFromNative ? feeBufferAmount : 0n on line 82 is behavior-preserving and just makes intent explicit. Edge cases (balance <= 0n, buffer ≥ balance) return 0n correctly.
  • Consistency — isFromNative originates from SwapWidgetRenderer.tsx:473 and is threaded through to both PercentageButtons instances; the new signature is used at every call site.
  • Conventions — Import uses the .js extension (../constants/maxAmountBuffer.js) resolving to the .ts source, matching repo convention. No stray Tailwind/CSS concerns. Changeset is present, scoped to @relayprotocol/relay-kit-ui as a patch, and reads as a user-facing outcome.

Matches the PR's stated intent. Nice, focused change.

@greptile-apps

greptile-apps Bot commented Sep 29, 2026 •

Copy link
Copy Markdown

RetriggerConfidence Score: 5/5

[Medium risk] Changes how the MAX button calculates token amounts.

The PR appears safe to merge, though MAX behavior still lacks regression coverage.

Findings

  1. P2 MAX change lacks regression coverage ▶
Summary

The PR lets MAX use the full balance for non-native tokens while preserving native-token execution and gas reserves, and updates native-token detection to handle EVM address casing.

  • Extracts the MAX calculation into a utility and adds a patch changeset.
  • Greptile automatically discovered a related ticket that helped explain the purpose of this PR: ERC-20 MAX should use the full balance without changing native-token gas reserves.

Reviews (2) · Last reviewed commit: "pr feedback"

Comment thread packages/ui/src/components/common/PercentageButtons.tsx
feeBufferAmount: bigint = 0n
): bigint => {
if (balance <= 0n) return 0n
if (!isFromNative) return balance

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 MAX change lacks regression coverage

The new full-balance branch has no regression test. Without a test for the Base USDC sale and one confirming that native-token MAX keeps gas headroom, a later change could restore the original ERC-20 bug or break the native behavior without being caught. Greptile automatically discovered a related ticket stating that the Base USDC case should have a regression test, which informed this comment.

Source Used: Linear — MAX leaves gas reserve for ERC-20 sales

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

@github-actions

github-actions Bot commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @ted-palmer's task in 1m 15s —— View job


Code Review — No issues found.

Reviewed 4 files, ~46 lines changed. Checked for correctness, security, error handling, and repo guidelines compliance.

The refactor extracts the buffer math into packages/ui/src/utils/maxAmount.ts and makes MAX use the full balance for non-native tokens while preserving the execution + gas buffers for native tokens.

Verified:

  • Correctness — getExecutionBuffer is preserved verbatim from the old inline version. getMaxAmount returns the full balance for non-native, and balance - (executionBuffer + feeBuffer) (floored at 0n) for native. feeBufferAmount is only computed when isFromNative in handleMaxClick (line 64), so passing it through is behavior-preserving; isFromNative ? feeBufferAmount : 0n on line 82 just makes intent explicit. Edge cases (balance <= 0n, buffer ≥ balance) return 0n.
  • Native detection — SwapWidgetRenderer.tsx:473 swaps the raw === compare for addressesEqual(...), which is case-insensitive for EVM/hypevm (address.ts:164-165) and matches the pattern already used at line 394. This actually hardens native detection against address-casing differences.

On the Greptile findings

  • P1 (native MAX can lose its reserve via case-sensitive compare) — appears to be a false positive. This PR replaces the old case-sensitive fromToken?.address === fromChain?.currency?.address with addressesEqual, which lowercases both sides for EVM. For SVM/BVM/TVM the compare is a === b, which is correct since those address encodings are inherently case-sensitive. So a native token differing only in casing is now still classified as native and keeps its buffers.
  • P2 (no regression test) — accurate as a gap, but worth noting the packages/ui package currently has no test suite (no *.test.ts(x) files exist). Adding one for getMaxAmount would be net-positive but means standing up test infra rather than extending it — reasonable to treat as a separate task.

Focused, clean change that matches its stated intent.

@github-actions

github-actions Bot commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @ted-palmer's task in 45s —— View job


📝 Docs Drift Check — No docs impact

This PR does not touch surfaces that require a relayprotocol/relay-docs update.

@ted-palmer
ted-palmer merged commit 17b0ef2 into main Sep 30, 2026
8 of 9 checks passed
@ted-palmer
ted-palmer deleted the ted/int2-1408-max-leaves-gas-reserve-for-erc-20-sales branch September 30, 2026 17:10

This branch was successfully deployed

1 active deployment
Preview — 9eedd95c Deployed Sep 30, 2026 by vercel[bot]
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