feat: add dapp swap comparison button#417
Conversation
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| feeBips, | ||
| ethAmountDisplay: ethAmountInput || DEFAULT_ETH_AMOUNT, | ||
| feePercentageDisplay: | ||
| feePercentageInput || DEFAULT_FEE_PERCENTAGE.toString(), |
There was a problem hiding this comment.
Bug: UI shows invalid input; uses defaults for tx
The display values (ethAmountDisplay and feePercentageDisplay) can show invalid user input even though the actual transaction will use default values. When a user enters invalid input (e.g., "abc" for ETH amount or "200" for fee percentage), the display will show the invalid input, but getConfigValues() will use the default values for the actual transaction due to parseEthAmount()/parseFeePercentage() returning null for invalid input. This creates a mismatch between what the UI displays and what transaction will actually be sent.
| ethers.utils.parseEther(DEFAULT_ETH_AMOUNT).toHexString(); | ||
|
|
||
| const feeBips = | ||
| parseFeePercentage(feePercentageInput) || DEFAULT_FEE_PERCENTAGE * 100; |
There was a problem hiding this comment.
Bug: Zero-fee input misinterpreted as fifty percent
When a user enters 0 for the fee percentage, parseFeePercentage returns 0, which is falsy in JavaScript. The || operator then incorrectly falls back to DEFAULT_FEE_PERCENTAGE * 100 (5000 basis points), causing a 0% fee input to be treated as 50%. This prevents users from setting a zero fee for testing.
seaona
left a comment
seaona
left a comment
There was a problem hiding this comment.
Overall looks good! Just added a small comment but non blocking
| <div class="card full-width"> | ||
| <div class="card-body"> | ||
| <h4 class="card-title"> | ||
| Swap Comparison (High Fee Test - Mainnet only) |
There was a problem hiding this comment.
if this is intended to work on mainnet only, maybe we could simply display the component depending on which network we have selected, using globalContext.chainIdInt?
As at the moment, nothing prevents me to trigger the button if I'm on a different network, leading to unexpected results
cryptotavares
dismissed stale reviews from seaona, vinistevam, and OGPoyraz
via
138ec5f
|
|
||
| document.addEventListener('newChainIdInt', function (e) { | ||
| swapButton.disabled = e.detail.chainIdInt !== 1; | ||
| }); |
There was a problem hiding this comment.
Bug: Enable Swap Without User Connection Checks
The newChainIdInt event handler enables the swap button when chainIdInt === 1 without checking if the user is connected. This allows the button to be enabled when on mainnet but not connected, potentially causing transaction failures when clicked.
4 participants
Note
Adds a configurable ETH→USDC swap comparison UI that builds/sends Universal Router calldata, enables only on mainnet, and is integrated into the transactions section.
swapComparisonComponentto compare an ETH→USDC swap with configurable fee (amount, fee %, recipient), status, and tx hash display.newChainIdInt,globalConnectionChange, anddisableAndClearevents.executecalldata using V4 actions (SWAP_EXACT_IN_SINGLE,SETTLE_ALL,TAKE_PORTION,TAKE_ALL) with ethers encoding and input validation.eth_sendTransactiontoUNIVERSAL_ROUTERwith user-specified ETH value.components/transactions/index.jsand render insrc/index.jstransactions row.newChainIdIntinhandleNewChainto drive component enablement.Written by Cursor Bugbot for commit 138ec5f. This will update automatically on new commits. Configure here.