[WIP] Liquidity Bridge Integration #99
Conversation
…pdate envexample with addresses
merging downstream
merging downstream
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR Summary
This PR introduces significant changes to implement a liquidity bridge integration across the bridge UI, with major modifications to chain selection and transaction handling.
- Removal of chain combination validation in
chainselectormodal.tsxcould allow invalid chain pairs (BASE<->AVAIL), potentially causing failed transactions - New
AdvancedSettingscomponent has unused slippage state and non-persisted settings that reset on remount useLiquidityBridgehook has incomplete ERC20 to Avail bridging flow and hardcoded tx_index valuegetERC20AvailBalanceinuseEthWallet.tshas dependency array issues and lacks proper error handling- Downgrade of @polkadot/types from v11.3.1 to v10.13.1 in package.json requires careful testing for compatibility
22 file(s) reviewed, 38 comment(s)
Edit PR Review Bot Settings | Greptile
| </Dialog> | ||
| ); | ||
| <div className="flex items-center justify-center p-4 text-[#8B8EA3] text-sm border-t bg-[#1D2230] rounded-b-xl border-[#252A3C]"> | ||
| looking for the testnet bridge, <a href="https://turing.bridge.avail.so/" className="italic cursor-pointer ml-1 underline">click here</a> |
There was a problem hiding this comment.
syntax: sentence should be capitalized and use proper punctuation
| looking for the testnet bridge, <a href="https://turing.bridge.avail.so/" className="italic cursor-pointer ml-1 underline">click here</a> | |
| Looking for the testnet bridge? <a href="https://turing.bridge.avail.so/" className="italic cursor-pointer ml-1 underline">Click here</a> |
components/common/settings.tsx
Outdated
| const [autoClaim, setAutoClaim] = useState(true); | ||
| const [slippage, setSlippage] = useState('0.5'); |
There was a problem hiding this comment.
logic: These states should be moved to a global store (e.g. useCommonStore) since they affect bridge behavior and need to persist between component remounts
| <ArrowLeft | ||
| className="text-gray-400 hover:text-white hover:cursor-pointer" | ||
| onClick={() => setIsOpen(false)} | ||
| /> |
There was a problem hiding this comment.
logic: Redundant close handler - the Dialog already handles closing via onOpenChange. Remove the onClick handler to avoid potential race conditions
| <ArrowLeft | |
| className="text-gray-400 hover:text-white hover:cursor-pointer" | |
| onClick={() => setIsOpen(false)} | |
| /> | |
| <ArrowLeft | |
| className="text-gray-400 hover:text-white hover:cursor-pointer" | |
| /> |
components/common/settings.tsx
Outdated
| const AdvancedSettings = () => { | ||
| const [isOpen, setIsOpen] = useState(false); | ||
| const [autoClaim, setAutoClaim] = useState(true); | ||
| const [slippage, setSlippage] = useState('0.5'); |
There was a problem hiding this comment.
logic: slippage state is defined but never used in the UI or passed to bridge functions
| @@ -0,0 +1,53 @@ | |||
| import React, { useState } from 'react'; | |||
| import { ArrowLeft, Info, Settings } from 'lucide-react'; | |||
There was a problem hiding this comment.
style: Info icon is imported but never used in the component
| import { ArrowLeft, Info, Settings } from 'lucide-react'; | |
| import { ArrowLeft, Settings } from 'lucide-react'; |
services/pallet.ts
Outdated
| address: account.address, | ||
| }); | ||
|
|
||
| const verification = signatureVerify(u8aToHex(messageBytes), signature, account.address); |
There was a problem hiding this comment.
style: Message bytes are converted to hex twice - once for signing and once for verification. Consider reusing the hex value
| atomicAmount | ||
| ) | ||
| .signAndSend(account.address, options, (result: ISubmittableResult) => { | ||
| console.log(`Tx status: ${result.status}`); |
There was a problem hiding this comment.
style: Using console.log for transaction status. Should use Logger utility for consistency
| console.log(`Tx status: ${result.status}`); | |
| Logger.info(`Tx status: ${result.status}`); |
| sender_address: string; | ||
| tx_index: number; | ||
| block_hash: string; | ||
| eth_receiver_address: string; | ||
| amount: string; | ||
| } |
There was a problem hiding this comment.
style: fields match those used in useLiquidityBridge.ts, but consider adding validation for eth_receiver_address format since it's used for Ethereum addresses
| sender_address: string; | ||
| tx_index: number; | ||
| status: "InProgress" | "Completed" | "Failed"; |
There was a problem hiding this comment.
logic: status values don't align with TransactionStatus enum defined above. Consider reusing those values for consistency
| status: "InProgress" | "Completed" | "Failed"; | |
| status: TransactionStatus.INITIATED | TransactionStatus.BRIDGED | TransactionStatus.FAILED; |
| block_hash: string; | ||
| eth_receiver_address: string; | ||
| amount: string; |
There was a problem hiding this comment.
style: amount is defined as string but should probably use BigInt for consistency with ethBalance interface above
No description provided.