Xdc mento swap #609
Open
Xdc mento swap #609
Conversation
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
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey there - I've reviewed your changes and they look great!
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `src/pages/gd/Swap/SwapCore/mentoReserve.tsx:61-64` </location>
<code_context>
- const inputAmountBig = ethers.utils.parseUnits(inputAmount || '0', swapPair.input.decimals)
- const outputAmountBig = ethers.utils.parseUnits(outputAmount || '0', swapPair.output.decimals)
+ const inputAmountBig = useMemo(
+ () => ethers.utils.parseUnits(inputAmount || '0', swapPair.input.decimals),
+ [inputAmount]
+ )
</code_context>
<issue_to_address>
**suggestion:** Consider including swapPair.input.decimals in useMemo dependencies.
If swapPair.input.decimals is dynamic, omitting it from the dependency array may result in inputAmountBig not updating correctly.
```suggestion
const inputAmountBig = useMemo(
() => ethers.utils.parseUnits(inputAmount || '0', swapPair.input.decimals),
[inputAmount, swapPair.input.decimals]
)
```
</issue_to_address>
### Comment 2
<location> `src/pages/gd/Swap/SwapCore/mentoReserve.tsx:80-89` </location>
<code_context>
+ const swapInput: [string, string, any, any] = useMemo(
</code_context>
<issue_to_address>
**suggestion:** swapInput useMemo dependencies may be incomplete.
If swapPair.output.address or other swapPair fields can change, add them to the dependency array to avoid stale values.
Suggested implementation:
```typescript
const swapInput: [string, string, any, any] = useMemo(
() => [
swapPair.input.address,
swapPair.output.address,
lastEdited?.field === 'input'
? {
input: inputAmountBig.toString(),
minAmountOut: outputAmountBig
.mul(10000 - +slippageTolerance.value * 100)
.div(10000)
.toString(),
],
[
swapPair.input.address,
swapPair.output.address,
swapPair.input, // if swapPair.input is an object and can change
swapPair.output, // if swapPair.output is an object and can change
lastEdited,
inputAmountBig,
outputAmountBig,
slippageTolerance.value,
]
);
```
If `swapPair.input` and `swapPair.output` are objects whose properties can change, consider using a deep comparison or including all relevant properties in the dependency array. Adjust the dependency array as needed based on how `swapPair` is structured and updated in your codebase.
</issue_to_address>
### Comment 3
<location> `src/pages/gd/Swap/SwapCore/mentoReserve.tsx:150-153` </location>
<code_context>
}, [swapMeta, lastEdited?.field, swapPair])
- const handleApprove = async () => {
+ const handleApprove = useCallback(async () => {
if (approved) return
const type = buying ? 'buy' : 'sell'
</code_context>
<issue_to_address>
**suggestion:** handleApprove useCallback dependencies may be incomplete.
If setApproving and sendData are not guaranteed to be stable, please add them to the dependency array to prevent stale references.
Suggested implementation:
```typescript
const handleApprove = useCallback(async () => {
if (approved) return
const type = buying ? 'buy' : 'sell'
try {
} finally {
setApproving(false)
}
}, [approved, buying, network, setApproving, sendData])
```
If `sendData` is not currently defined in this file, or if it is a prop or comes from context, you will need to ensure it is available in the scope and referenced correctly. If `setApproving` is a state setter from `useState`, it is stable, but including it in the dependency array is safe and future-proof.
</issue_to_address>
### Comment 4
<location> `src/hooks/useReserveToken.ts:6-15` </location>
<code_context>
+import { ethers } from 'ethers'
+import { useMemo } from 'react'
+
+export const useReserveToken = () => {
+ const { chainId, address, decimals, symbol, name } = useMentoReserveToken()
+ const result = useMemo(
+ () =>
+ new Token(
+ chainId || 42220,
+ address || ethers.constants.AddressZero,
+ decimals || 18,
+ symbol || 'USD',
+ name || 'USD'
+ ),
+ [chainId, address, decimals, symbol, name]
+ )
+ return result
+}
</code_context>
<issue_to_address>
**suggestion (bug_risk):** useReserveToken defaults may mask errors if upstream values are missing.
Defaulting critical values may hide upstream configuration issues. Please add warnings or error handling for missing values to ensure problems are surfaced early.
</issue_to_address>
### Comment 5
<location> `src/hooks/useReserveToken.ts:8-19` </location>
<code_context>
const result = useMemo(
() =>
new Token(
chainId || 42220,
address || ethers.constants.AddressZero,
decimals || 18,
symbol || 'USD',
name || 'USD'
),
[chainId, address, decimals, symbol, name]
)
return result
</code_context>
<issue_to_address>
**suggestion (code-quality):** Inline variable that is immediately returned ([`inline-immediately-returned-variable`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/TypeScript/Default-Rules/inline-immediately-returned-variable))
```suggestion
return useMemo(
() =>
new Token(
chainId || 42220,
address || ethers.constants.AddressZero,
decimals || 18,
symbol || 'USD',
name || 'USD'
),
[chainId, address, decimals, symbol, name]
);
```
<br/><details><summary>Explanation</summary>Something that we often see in people's code is assigning to a result variable
and then immediately returning it.
Returning the result directly shortens the code and removes an unnecessary
variable, reducing the mental load of reading the function.
Where intermediate variables can be useful is if they then get used as a
parameter or a condition, and the name can act like a comment on what the
variable represents. In the case where you're returning it from a function, the
function name is there to tell you what the result is, so the variable name
is unnecessary.
</details>
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
![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 |
|---|---|---|
 |
Invalid fallback values for reserve token creation ▹ view | |
 |
Unnecessary Variable Assignment ▹ view | |
|
Incomplete useMemo dependencies for inputAmountBig ▹ view |
Files scanned
| File Path | Reviewed |
|---|---|
| src/hooks/useReserveToken.ts | ✅ |
| src/hooks/useWeb3.tsx | ✅ |
| src/pages/gd/Swap/index.tsx | ✅ |
| src/connectors/index.ts | ✅ |
| src/components/SideBar.tsx | ✅ |
| src/pages/gd/Swap/SwapCore/mentoReserve.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.
Deploying goodprotocolui with
|
| Latest commit: |
3568f69
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://6dcf36a0.goodprotocolui.pages.dev |
| Branch Preview URL: | https://xdc-mento-swap.goodprotocolui.pages.dev |
Collaborator
the tokenbalance in appbar is not updated when swithcing chain @blueogin |
Collaborator
|
Swithcing chain should be look into. its also breaking claim-flow (especially switching to XDC). Starting on Celo > switching to XDC will make the claim button not clickable. |
Collaborator
|
@L03TJ3 |
Collaborator
Screencast.from.07-10-25.16.21.12.webm |
Collaborator
I changed goodprotocol version to |
…ated goodprotocol version to '2.0.34-beta.2'
Collaborator
|
I used yalc for updated sdk, sdk-v2, good-design and updated goodprotocol version to |
blueogin
requested changes
Oct 7, 2025
Contributor
Author
Collaborator
|
@sirpy yes, its (now) part of the xdc mento-swap pr on mono-repo: GoodDollar/GoodWeb3-Mono#252 got fixed as part of this PR: GoodDollar/GoodWeb3-Mono#253 |
L03TJ3
approved these changes
Nov 25, 2025
Collaborator
L03TJ3
left a comment
L03TJ3
left a comment
There was a problem hiding this comment.
Just needs to update yalced packages when mono-repo PR is merged
Collaborator
Collaborator
|
@L03TJ3 For example: |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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
Add swap on XDC via mento reserve
requires feat/xdc-mento-swap branch in goodweb3-mono
Description by Korbit AI
What change is being made?
Update navigation and network bindings to support GoodReserve on XDC, including renaming swap target, switching XDC RPC to Xinfin, and introducing a new useReserveToken hook to map Mento reserves to a Token.
Why are these changes being made?
Enable cross-chain (Celo + XDC) swapping via GoodReserve (Mento) by aligning routes, RPC endpoints, and token modeling; keep translations in sync with the new reserve wording and networks.