Open
Conversation
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @romeoscript - I've reviewed your changes - here's some feedback:
- There are placeholder contract addresses and an incomplete ABI in react.ts—please replace these with the actual deployed MPB contract addresses and full ABI definitions before merging.
- The source/target chain ID mapping logic is repeated across hooks and components—consider extracting it into a shared utility to avoid duplication and keep mappings in one place.
- The MPBBridge component is quite large; splitting it into smaller sub-components (e.g. provider selector, chain toggler, amount form) will improve readability and maintainability.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- There are placeholder contract addresses and an incomplete ABI in react.ts—please replace these with the actual deployed MPB contract addresses and full ABI definitions before merging.
- The source/target chain ID mapping logic is repeated across hooks and components—consider extracting it into a shared utility to avoid duplication and keep mappings in one place.
- The MPBBridge component is quite large; splitting it into smaller sub-components (e.g. provider selector, chain toggler, amount form) will improve readability and maintainability.
## Individual Comments
### Comment 1
<location> `packages/sdk-v2/src/sdk/mpbridge/react.ts:55` </location>
<code_context>
+ );
+
+ // Basic validation - in real implementation this would check actual contract limits
+ const isValid = amount && BigNumber.from(amount).gt(0);
+ const reason = isValid ? "" : "Invalid amount";
+
</code_context>
<issue_to_address>
Validation logic may not handle non-numeric or negative input robustly.
Since `BigNumber.from(amount)` can throw on invalid input, add a try/catch or validate that `amount` is a valid number before this call to prevent runtime errors.
</issue_to_address>
### Comment 2
<location> `packages/sdk-v2/src/sdk/mpbridge/react.ts:107` </location>
<code_context>
+ transactionName: "MPBBridgeTransfer"
+ });
+
+ const bridgeRequestId = (bridgeTo.state?.receipt?.logs || [])
+ .filter(log => log.address === mpbContract?.address)
+ .map(log => mpbContract?.interface.parseLog(log))?.[0]?.args?.id;
+
+ // Poll target chain for bridge completion
</code_context>
<issue_to_address>
Bridge request ID extraction assumes log structure and may fail silently.
Add error handling or validation to detect missing or malformed logs, so failures don't go unnoticed.
</issue_to_address>
### Comment 3
<location> `packages/sdk-v2/src/sdk/mpbridge/react.ts:170` </location>
<code_context>
+ lock.current = true;
+
+ // Get fee estimate
+ void mpbContract
+ ?.estimateSendFee(
+ bridgeRequest.targetChainId,
+ bridgeRequest.target || account,
+ bridgeRequest.amount,
+ false, // payInLZToken
+ "0x" // adapterParams
+ )
+ .then(([nativeFee]) => {
+ // Send bridge transaction with fee
+ void bridgeTo.send(
+ bridgeRequest.targetChainId,
+ bridgeRequest.target || account,
+ bridgeRequest.amount,
+ "0x", // adapterParams
+ { value: nativeFee }
+ );
+ })
+ .catch(e => {
+ console.error("MPB Bridge error:", e);
+ throw e;
</code_context>
<issue_to_address>
Error handling in bridge request only logs to console.
Consider surfacing the error to the UI or caller so users are notified of bridge transaction failures, rather than only logging to the console and re-throwing.
</issue_to_address>
### Comment 4
<location> `packages/good-design/src/apps/bridge/mpbridge/MPBBridge.tsx:123` </location>
<code_context>
+ sourceChain
+ });
+
+ const hasBalance = Number(bridgeWeiAmount) <= Number(wei);
+ const isValidInput = isValid && hasBalance;
+ const reasonOf = reason || (!hasBalance && "Not enough balance") || "";
</code_context>
<issue_to_address>
Balance check may be inaccurate for large values due to Number precision.
Consider using `BigNumber` for comparing `bridgeWeiAmount` and `wei` to avoid precision errors with large values.
</issue_to_address>
### Comment 5
<location> `packages/good-design/src/apps/bridge/mpbridge/MPBBridgeTransactionCard.tsx:194` </location>
<code_context>
+ persistentScrollbar={true}
+ nestedScrollEnabled={true}
+ >
+ {transactions?.map((tx: BridgeTransaction, i: any) => (
+ <BridgeTransactionCard key={i} {...{ transaction: tx, onTxDetailsPress }} />
+ ))}
</code_context>
<issue_to_address>
Using array index as key in list rendering can cause rendering issues.
Prefer using a unique identifier like `tx.id` or `tx.transactionHash` as the key instead of the array index to avoid potential rendering bugs when the list changes.
Suggested implementation:
```typescript
{transactions?.map((tx: BridgeTransaction) => (
<BridgeTransactionCard key={tx.id ?? tx.transactionHash} {...{ transaction: tx, onTxDetailsPress }} />
))}
```
- Make sure that every `BridgeTransaction` object in the `transactions` array has a unique `id` or `transactionHash` property. If neither exists, you may need to add a unique identifier to each transaction.
- If you know for sure which property is always present and unique (e.g., `tx.id`), you can use just that property for the key.
</issue_to_address>
### Comment 6
<location> `packages/good-design/src/apps/bridge/mpbridge/MPBBridgeController.tsx:34` </location>
<code_context>
+
+ const expectedToReceive = input.sub(expectedFee);
+
+ return {
+ expectedFee,
+ expectedToReceive,
</code_context>
<issue_to_address>
Date fallback uses current time, which may mislead users.
Using `new Date()` as a fallback may imply the transaction just occurred. Instead, omit the date or display 'unknown' when the actual timestamp is missing.
</issue_to_address>
### Comment 7
<location> `packages/good-design/src/apps/bridge/mpbridge/MPBBridge.tsx:72` </location>
<code_context>
+ };
+};
+
+export const MPBBridge = ({
+ useCanMPBBridge,
+ onSetChain,
</code_context>
<issue_to_address>
Consider breaking the large component into smaller presentational components, custom hooks, and centralized chain metadata to improve maintainability.
```markdown
You’ve added a lot of view‐logic, state, effects, and switch/mapping code into one 370 LOC component. Breaking it into smaller pieces and centralizing your “chain ↔ metadata” mappings will make it far easier to maintain. For example:
1. Extract your chain metadata into a constant map instead of `switch`/`?:` chains everywhere:
```ts
// constants/chains.ts
export const CHAIN_CONFIG = {
fuse: { id: 122, label: 'G$ Fuse', colorKey: 'goodGrey.700' },
celo: { id: 42220, label: 'G$ Celo', colorKey: 'goodGrey.700' },
mainnet: { id: 1, label: 'G$ Mainnet', colorKey: 'goodGrey.700' },
} as const;
export type ChainKey = keyof typeof CHAIN_CONFIG;
```
2. Create a small presentational component for each chain card:
```tsx
// components/ChainCard.tsx
import { VStack, Text } from 'native-base';
import { GdAmount } from '../core/layout/BalanceGD';
import { CHAIN_CONFIG, ChainKey } from '../constants/chains';
type Props = {
chain: ChainKey;
balance: CurrencyValue;
active: boolean;
onToggle?: () => void;
};
export function ChainCard({ chain, balance, active, onToggle }: Props) {
const { label, colorKey } = CHAIN_CONFIG[chain];
return (
<VStack flex="1" minW="120px" onPress={onToggle}>
<Text color={active ? colorKey : 'goodGrey.400'}>{label}</Text>
<GdAmount
color={active ? colorKey : 'goodGrey.400'}
amount={balance}
withDefaultSuffix={false}
withFullBalance
fontSize="xs"
/>
</VStack>
);
}
```
3. Extract the repeated `useG$Balance` logic into a custom hook:
```ts
// hooks/useChainBalance.ts
import { useG$Balance } from '@gooddollar/web3sdk-v2';
import { CHAIN_CONFIG, ChainKey } from '../constants/chains';
export function useChainBalance(chain: ChainKey) {
const cfg = CHAIN_CONFIG[chain];
const { G$ } = useG$Balance(5, cfg.id);
return G$;
}
```
4. Move your side‐effect logic for `bridgeStatus` into a hook:
```ts
// hooks/useBridgeStatus.ts
import { useEffect, useState } from 'react';
import { useWizard } from 'react-use-wizard';
export function useBridgeStatus(
statusObj: { status?: string; errorMessage?: string } | undefined,
onSuccess: () => void,
onFailure: (err: Error) => void
) {
const [isBridging, setBridging] = useState(false);
const [message, setMessage] = useState('');
const { nextStep } = useWizard();
useEffect(() => {
const status = statusObj?.status || '';
const isSuccess = status === 'Success';
const isFailed = ['Fail', 'Exception'].includes(status);
const isActive = !isFailed && !isSuccess && ['Mining','PendingSignature','Success'].includes(status);
setBridging(isActive);
if (status === 'Mining') {
setMessage('Bridging in progress...');
void nextStep();
}
if (status === 'PendingSignature') {
setMessage('Waiting for signature...');
}
if (isSuccess) {
setMessage('Bridge completed successfully!');
setTimeout(() => setBridging(false), 3000);
onSuccess();
}
if (isFailed) {
setMessage('Bridge failed');
setTimeout(() => setBridging(false), 3000);
onFailure(new Error(statusObj?.errorMessage ?? 'Failed to bridge'));
}
}, [statusObj, onSuccess, onFailure, nextStep]);
return { isBridging, message };
}
```
5. In your main `MPBBridge` component, you’ll then just compose these smaller pieces:
```tsx
import { CHAIN_CONFIG, ChainKey } from '../constants/chains';
import { useChainBalance } from '../hooks/useChainBalance';
import { useBridgeStatus } from '../hooks/useBridgeStatus';
import { ChainCard } from '../components/ChainCard';
// ...
const MPBBridge = (props: MPBBridgeProps) => {
// state, hooks...
const fuseBal = useChainBalance('fuse');
const celoBal = useChainBalance('celo');
const mainnetBal = useChainBalance('mainnet');
const { isBridging, message } = useBridgeStatus(
bridgeStatus,
props.onBridgeSuccess,
props.onBridgeFailed
);
return (
<VStack>
{/* provider selector */}
{/* status box */}
<HStack>
{(['fuse','celo','mainnet'] as ChainKey[]).map(chain => (
<ChainCard
key={chain}
chain={chain}
balance={chain === 'fuse' ? fuseBal : chain === 'celo' ? celoBal : mainnetBal}
active={sourceChain === chain}
onToggle={() => setSourceChain(chain)}
/>
))}
</HStack>
{/* rest of form */}
</VStack>
);
};
```
— This keeps each file ≤ 100 LOC, centralizes your chain mappings, and separates view, hooks, and effects.〉
</issue_to_address>
### Comment 8
<location> `packages/sdk-v2/src/sdk/mpbridge/react.ts:27` </location>
<code_context>
+ [SupportedChains.MAINNET]: "0x0000000000000000000000000000000000000000" // Replace with actual address
+};
+
+export const useGetMPBContracts = () => {
+ return {
+ [SupportedChains.FUSE]: new Contract(MPB_CONTRACTS[SupportedChains.FUSE], MPBBridgeABI) as Contract,
</code_context>
<issue_to_address>
Consider centralizing all per-chain configuration into a single array and mapping over it to eliminate repetitive code.
Here’s one way to DRY‐out all of the “per‐chain” duplication by centralizing your chain-config and mapping over it:
```ts
// 1) Define a single source of chain truth
const CHAIN_CONFIG = [
{
id: SupportedChains.FUSE,
name: "fuse",
label: "Fuse",
target: SupportedChains.CELO,
provider: "axelar",
explorerBase: "https://axelarscan.io/fuse/tx",
fromBlock: -20000,
},
{
id: SupportedChains.CELO,
name: "celo",
label: "Celo",
target: SupportedChains.MAINNET,
provider: "layerzero",
explorerBase: "https://layerzeroscan.com/celo/tx",
fromBlock: -20000,
},
{
id: SupportedChains.MAINNET,
name: "ethereum",
label: "Mainnet",
target: SupportedChains.FUSE,
provider: "axelar",
explorerBase: "https://axelarscan.io/ethereum/tx",
fromBlock: -20000,
},
] as const;
```
```ts
// 2) useGetMPBContracts → build once from that array
export const useGetMPBContracts = () => {
return useMemo(() => {
return CHAIN_CONFIG.reduce<Record<number, Contract>>((map, cfg) => {
map[cfg.id] = new Contract(MPB_CONTRACTS[cfg.id], MPBBridgeABI);
return map;
}, {});
}, []);
};
```
```ts
// 3) useMPBBridgeHistory → single loop instead of three hooks
export const useMPBBridgeHistory = () => {
const contracts = useGetMPBContracts();
const refresh = useRefreshOrNever(5);
const allEvents = CHAIN_CONFIG.flatMap(cfg => {
const logHook = useLogs(
{ contract: contracts[cfg.id], event: "BridgeRequest", args: [] },
{ chainId: cfg.id, fromBlock: cfg.fromBlock, refresh }
);
return logHook?.value?.map(e => ({
...e,
amount: formatAmount(e.data.amount, 18),
sourceChain: cfg.label,
targetChain: CHAIN_CONFIG.find(c => c.id === cfg.target)!.label,
bridgeProvider: cfg.provider,
chainId: cfg.id,
})) || [];
});
return { historySorted: sortBy(allEvents, "blockNumber").reverse() };
};
```
```ts
// 4) explorer helpers → one generic
export const getExplorerLink = (
txHash: string,
chainId: SupportedChains,
type: "axelar" | "layerzero"
) => {
const cfg = CHAIN_CONFIG.find(c => c.id === chainId)!;
const base = type === "axelar"
? cfg.explorerBase.replace("layerzeroscan", "axelarscan")
: cfg.explorerBase.replace("axelarscan", "layerzeroscan");
return `${base}/${txHash}`;
};
```
Benefits:
- All chain-specific data lives in one array.
- No more copy/paste for each chain.
- Adding a new network just means appending CHAIN_CONFIG.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
107
to
109
| const bridgeRequestId = (bridgeTo.state?.receipt?.logs || []) | ||
| .filter(log => log.address === mpbContract?.address) | ||
| .map(log => mpbContract?.interface.parseLog(log))?.[0]?.args?.id; |
There was a problem hiding this comment.
issue (bug_risk): Bridge request ID extraction assumes log structure and may fail silently.
Add error handling or validation to detect missing or malformed logs, so failures don't go unnoticed.
| sourceChain | ||
| }); | ||
|
|
||
| const hasBalance = Number(bridgeWeiAmount) <= Number(wei); |
There was a problem hiding this comment.
issue (bug_risk): Balance check may be inaccurate for large values due to Number precision.
Consider using BigNumber for comparing bridgeWeiAmount and wei to avoid precision errors with large values.
packages/good-design/src/apps/bridge/mpbridge/MPBBridgeTransactionCard.tsx
Outdated
Show resolved
Hide resolved
packages/good-design/src/apps/bridge/mpbridge/MPBBridgeController.tsx
Outdated
Show resolved
Hide resolved
![korbit-ai[bot]](https://avatars.githubusercontent.com/in/322216?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review by Korbit AI
Korbit automatically attempts to detect when you fix issues in new commits.
| Category | Issue | Status |
|---|---|---|
 |
Unsanitized JSON Data Rendering ▹ view | |
 |
Invalid Default Ethereum Address ▹ view | |
|
Sensitive data exposure in console logs ▹ view | ✅ Fix detected |
|
Hardcoded Bridge Fees and Limits ▹ view | ✅ Fix detected |
|
Incorrect Transaction Timestamp Fallback ▹ view | ✅ Fix detected |
 |
Non-idiomatic state management pattern ▹ view | |
|
Non-unique React key in transactions map ▹ view | ✅ Fix detected |
|
Unused limit prop in transaction list ▹ view | |
|
Undefined Bridge Status Values ▹ view |
Files scanned
| File Path | Reviewed |
|---|---|
| packages/sdk-v2/src/sdk/mpbridge/index.ts | ✅ |
| packages/sdk-v2/src/sdk/index.ts | ✅ |
| packages/good-design/src/apps/bridge/mpbridge/index.ts | ✅ |
| packages/good-design/src/stories/apps/bridge/MPBBridgeController.stories.tsx | ✅ |
| packages/good-design/src/apps/bridge/mpbridge/types.ts | ✅ |
| packages/sdk-v2/src/stories/mpbridge/MPBBridge.stories.tsx | ✅ |
| packages/good-design/src/apps/bridge/mpbridge/MPBBridgeController.tsx | ✅ |
| packages/good-design/src/apps/bridge/mpbridge/MPBBridgeTransactionCard.tsx | ✅ |
| packages/sdk-v2/src/sdk/mpbridge/react.ts | ✅ |
| packages/good-design/src/apps/bridge/mpbridge/MPBBridge.tsx | ✅ |
Explore our documentation to understand the languages and file types we support and the files we ignore.
Check out our docs on how you can make Korbit work best for you and your team.
| } | ||
| }, | ||
| args: { | ||
| address: "0x0", |
There was a problem hiding this comment.
Invalid Default Ethereum Address 
Tell me more
What is the issue?
Using an invalid Ethereum address '0x0' as a default value which could cause the bridge functionality to fail.
Why this matters
Bridge operations will fail with this invalid address, causing potential transaction errors and a poor user experience.
Suggested change ∙ Feature Preview
Use a valid zero address format for Ethereum:
address: "0x0000000000000000000000000000000000000000"Provide feedback to improve future suggestions
💬 Looking for more details? Reply to this comment to chat with Korbit.
Comment on lines
27
to
30
| bridgeStatus?: { | ||
| status?: string; | ||
| errorMessage?: string; | ||
| }; |
There was a problem hiding this comment.
Undefined Bridge Status Values
Tell me more
What is the issue?
The status field using a string type without specific values could lead to inconsistent status handling across the bridge implementation.
Why this matters
Without an enumerated set of possible status values, different parts of the code might use inconsistent status strings, making it difficult to reliably track and handle bridge operations.
Suggested change ∙ Feature Preview
Define an enum for possible bridge statuses and use it in the interface:
export enum BridgeStatus {
IDLE = 'IDLE',
PENDING = 'PENDING',
COMPLETED = 'COMPLETED',
FAILED = 'FAILED'
}
// Update the status type
bridgeStatus?: {
status?: BridgeStatus;
errorMessage?: string;
};Provide feedback to improve future suggestions
💬 Looking for more details? Reply to this comment to chat with Korbit.
Comment on lines
+20
to
+21
| <pre>bridge data {JSON.stringify(bridgeInfo, null, 4)}</pre> | ||
| <pre>bridge history {JSON.stringify(bridgeHistory, null, 4)}</pre> |
There was a problem hiding this comment.
Unsanitized JSON Data Rendering 
Tell me more
What is the issue?
Direct rendering of JSON data without sanitization could expose sensitive bridge transaction information and potentially lead to XSS if the data contains malicious content.
Why this matters
Unsanitized JSON output could expose sensitive transaction details and enable cross-site scripting attacks if the data contains malicious HTML/JavaScript.
Suggested change ∙ Feature Preview
Use a safe JSON viewer component or sanitize the output:
import DOMPurify from 'dompurify';
<pre>{DOMPurify.sanitize(JSON.stringify(bridgeInfo, null, 4))}</pre>
<pre>{DOMPurify.sanitize(JSON.stringify(bridgeHistory, null, 4))}</pre>Provide feedback to improve future suggestions
💬 Looking for more details? Reply to this comment to chat with Korbit.
packages/good-design/src/apps/bridge/mpbridge/MPBBridgeTransactionCard.tsx
Outdated
Show resolved
Hide resolved
packages/good-design/src/apps/bridge/mpbridge/MPBBridgeTransactionCard.tsx
Show resolved
Hide resolved
packages/good-design/src/apps/bridge/mpbridge/MPBBridgeController.tsx
Outdated
Show resolved
Hide resolved
packages/good-design/src/apps/bridge/mpbridge/MPBBridgeController.tsx
Outdated
Show resolved
Hide resolved
Contributor
|
@romeoscript The AI has many good observations |
…fers - Add MPBBridge and MPBBridgeController components - Implement SDK hooks for bridge transactions (LayerZero/Axelar) - Support Fuse ↔ Celo ↔ Mainnet bridging - Add dynamic fee calculation and status tracking - Include explorer links and error handling
- Update minimum amount from 1 G$ to 1000 G$ in bridge data - Fix validation logic to accept amounts >= 1000 G$ - Ensure consistent amount display across all chains
- Scale bridge min/max limits (from 18 decimals) to source-chain decimals (Fuse/Celo/Mainnet) via useG - Compare amounts with ethers.BigNumber to avoid JS precision issues - Pass scaled limits into MPBBridge; fix balance check to use BigNumber - Add debug logs to trace validation inputs/outputs This fixes the disabled bridge button when entering 2000 G$ on Fuse (was compared against 18-decimal limits). Files: MPBBridgeController.tsx, MPBBridge.tsx
- Remove fake recentTransactions array that showed input amounts - Integrate useMPBBridgeHistory hook for real transaction data - Add loading/empty states and proper event listening - Match microbridge pattern for transaction history - Show actual bridge amounts, status, and timestamps from blockchain
- Fix tokenValue type error by converting tx.amount to CurrencyValue - Remove non-existent tx.address reference for contractAddress - Add proper type guards for feeString parsing in bridge logic - Fix fee routing logic to use correct route-specific fees - Clean up console logs and improve error handling - Fix ESLint formatting issues - Remove unused import in MPBBridgeController
- Change onBridgeStart from () => void to (sourceChain: string, targetChain: string) => Promise<void> - Resolves TypeScript error in MPBBridgeController where handler expects 2 parameters
L03TJ3
requested changes
Sep 8, 2025
packages/good-design/src/apps/bridge/mpbridge/release/deployment.json
Outdated
Show resolved
Hide resolved
| }; | ||
|
|
||
| // Get status icon based on transaction status | ||
| const getStatusIcon = () => { |
Collaborator
There was a problem hiding this comment.
All of these utilities, if done within the component should be done with a callback and a dependency on 'status'
Collaborator
There was a problem hiding this comment.
This was never applied, should still be done, and following the changes and structure you used last time in the MPBBridgeTransactionModal.
This gist was given as example: https://gist.github.com/L03TJ3/ed27182ca6af219f56192adc902fd126
packages/good-design/src/apps/bridge/mpbridge/MPBBridgeController.tsx
Outdated
Show resolved
Hide resolved
packages/good-design/src/apps/bridge/mpbridge/MPBBridgeController.tsx
Outdated
Show resolved
Hide resolved
- Replace two-step approve+bridgeTo with single transferAndCall - Remove hardcoded fees, use dynamic API endpoint - Fix React cleanup and TypeScript errors
- Single transaction instead of approve + bridgeTo - Dynamic fees from API endpoint, no hardcoded values - Fix log parsing and minimum amount validation - Improve error handling and React cleanup
- Move source chain balance to appear above amount input field - Move target chain balance to appear above expected output field - Remove balance display from chain selection dropdowns for cleaner UI - Position balances contextually where users need them most
- Split 773-line react.ts into 5 smaller files by responsibility - Extract types and constants into types.ts - Move API functions and utilities to api.ts - Separate React hooks into hooks.ts - Create dedicated history.ts for bridge tracking - Update main react.ts to re-export from modules - Fix unused import warnings - Improve code organization and maintainability
- Split 876-line MPBBridge.tsx into 10 smaller, focused components - Extract utility functions into utils.ts for chain handling and fee calculation - Move custom hooks into hooks.ts for better reusability - Create dedicated UI components: ChainSelector, BridgeProviderSelector, AmountInput, ExpectedOutput, FeeInformation, TransactionHistory, BridgingStatusBanner - Reduce main component from 876 to 330 lines (~60% reduction) - Improve code organization, maintainability, and testability - Update index.ts to export all new components and utilities
…e-contracts package - Import contract addresses and ABI from node_modules instead of copied files - Remove duplicated deployment.json and mpb.json from good-design package - Update types.ts to use proper ABI extraction from JSON structure - Ensure consistent contract data across the application
Author
|
its in progress @L03TJ3 |
… with success modal
Also includes KISS/DRY cleanup: - Move BridgeTransaction type to types.ts (single source) - Add shared capitalizeChain utility
L03TJ3
requested changes
Feb 6, 2026
Collaborator
L03TJ3
left a comment
L03TJ3
left a comment
There was a problem hiding this comment.
Amount input validation still not working
| @@ -37,30 +37,44 @@ export const useReadOnlySDK = (type: SdkTypes, requiredChainId?: number): Reques | |||
| return useSDK(true, type, requiredChainId); | |||
Collaborator
|
Make sure any changes you have locally are commited. @romeoscript |
L03TJ3
requested changes
Feb 8, 2026
Collaborator
L03TJ3
left a comment
L03TJ3
left a comment
There was a problem hiding this comment.
- Widget does not fit on a mobile screen
Comment on lines
185
to
190
| case "MPBBridge": | ||
| return new Contract( | ||
| this.contracts["MPBBridge"], | ||
| CONTRACT_TO_ABI["MPBBridge"].abi, | ||
| this.signer || this.provider | ||
| ); |
Collaborator
There was a problem hiding this comment.
if you look at deployment.json, the key for the MPBBRidge = MpbBridge
thats why in my test nothing works because it never loads the contract
| direction={["column", "column", "row"]} | ||
| alignContent="center" | ||
| alignItems="center" | ||
| justifyContent="center" |
Collaborator
There was a problem hiding this comment.
remove all commits and changes that are not related to your implementation of the MPBBridge
| } | ||
| }, [chainId]); | ||
|
|
||
| const bridgeData = useGetMPBBridgeData(sourceChain, "fuse", bridgeProvider, inputTransaction[0], account); |
Collaborator
There was a problem hiding this comment.
The whole flow of useGetMPBBridgeData needs to be refactored as it is very inefficient.
There is a dependency on 'inputTransction[0]/amount'. this means that every time I type a digit in the input field, I am doing network calls repeating fetching of fees (which are static)
Collaborator
There was a problem hiding this comment.
- we should fetch limits and fees upon first render. these values are static and never change
- what needs to be rechecked is 'canBridge' only, based on input amount.
To reduce network calls this can easily be done as validation when someone clicks on the 'bridge' button.
-- We can do initial validation against the fees: is the amount within bridge limits? can the user cover the fees calculated?
-- The fees should be cached after first render
-- After all the initiall validations have passed, we only call 'canBridge' after someone clicks on bridge before we actually submit the transaction confirmation to the wallet.
| }; | ||
|
|
||
| // Get status icon based on transaction status | ||
| const getStatusIcon = () => { |
L03TJ3
requested changes
Feb 17, 2026
| * Bridge configuration constants | ||
| */ | ||
| export const BRIDGE_CONSTANTS = { | ||
| DEFAULT_CHAIN_ID: 122, |
Comment on lines
105
to
107
| // Utility functions (DRY principle) | ||
| export const safeBigNumber = (value: string | number | undefined, fallback = "0"): ethers.BigNumber => { | ||
| return ethers.BigNumber.from(value || fallback); |
Collaborator
There was a problem hiding this comment.
No single line wrapper methods
| const source = src.toLowerCase() as ChainName; | ||
| const target = dst.toLowerCase() as ChainName; | ||
| if ( | ||
| (["celo", "fuse", "mainnet"] as string[]).includes(source) && |
Collaborator
There was a problem hiding this comment.
No hardcoding of supported chains
Collaborator
|
@romeoscript Whats happening here? we have not seen any updates or heard from you in over a week? |
…ards - Fix case mismatch: deployment.json uses 'MpbBridge' but SDK looked up 'MPBBridge' - Add deploymentKeyMap in getContract() to translate internal names to deployment keys - Add isLoading guard in useGetMPBBridgeData validation to prevent false errors during loading - Add bridgeDataLoading guard in MPBBridgeController useCanMPBBridge callback
… in useGetMPBBridgeData - Separate static data fetching (fees, limits, protocol fee) from amount-dependent bridge eligibility check to avoid redundant network calls on every keystroke - Remove unnecessary comments (numbered checks, DRY annotations, obvious helpers) - Remove empty if-blocks for rejected promise results - Fix unused variable lint warnings
- Remove canUserBridge state and validateBridgeEligibility callback - Remove amount-dependent useEffect that made network calls on every keystroke - Validation is now purely local against cached limits (no network calls) - canBridge is already checked at transaction time by useBridgeValidators
…rdcoded chains - Set DEFAULT_CHAIN_ID to 42220 (Celo) instead of 122 (Fuse) - Remove safeBigNumber single-line wrapper, inline ethers.BigNumber.from() - Replace hardcoded chain names array with Object.keys(CHAIN_CONFIG)
…ack for proper memoization
L03TJ3
requested changes
Feb 24, 2026
| getContract(contractName: string): Contract; | ||
| getContract(contractName: string) { | ||
| if (!this.contracts?.[contractName]) return; | ||
| // Map internal names to deployment.json keys where they differ |
Collaborator
There was a problem hiding this comment.
This is a bug fix for a bug introduced by yourself in this PR.
its not used in that many places and the naming should just be aligned everywhere that we call for mpbbridge contract.
| extended.relayEvent = first(celoExecuted[e.data.id]); | ||
|
|
||
| extended.amount = formatAmount(e.data.amount, 18); //amount is normalized to 18 decimals in the bridge | ||
| // Add source and target chain IDs for Fuse -> Celo bridge transactions |
Collaborator
There was a problem hiding this comment.
Unrelated changes not part of the mpb bridge solution
| @@ -113,7 +97,6 @@ export const MPBBridgeController: React.FC<IMPBBridgeControllerProps> = ({ onBri | |||
| useEffect(() => { | |||
Collaborator
There was a problem hiding this comment.
Why is this not communicated to the user?
| import { useReadOnlyProvider } from "../../../hooks"; | ||
| import { BRIDGE_CONSTANTS } from "../constants"; | ||
|
|
||
| export const useMPBG$TokenContract = (chainId?: number, readOnly = false): IGoodDollar | null => { |
Collaborator
There was a problem hiding this comment.
I dont understand the need for this hook.
what is this solving and why not just call: useGetContract("GoodDollar", true, "base", chainId)?
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Multi-Protocol Bridge (MPB) widget that enables seamless cross-chain token transfers for GoodDollar (G$) between Fuse, Celo, and Ethereum Mainnet using either Axelar or LayerZero protocols.
About # (link your issue here)
#203
How Has This Been Tested?
Please describe the tests that you ran to verify your changes.
Checklist:
Description by Korbit AI
What change is being made?
Integrate the MPBBridge widget and its associated components into the codebase, enabling cross-chain bridging of G$ tokens between Fuse, Celo, and Mainnet using LayerZero/Axelar.
Why are these changes being made?
These changes implement the Main Bridge (MPB) functionality to support token bridging across different blockchain networks, leveraging LayerZero and Axelar for secure cross-chain communication. This addition enhances the application's capabilities in handling cross-chain transactions, providing users with a seamless experience when transferring G$ tokens across supported networks.