Add xdc fork test for GenericDistributionHelper #287
Conversation
![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> `test/reserve/GenericDistributionHelper.e2e.test.ts:275-284` </location>
<code_context>
+ it("should correctly calculate swap amounts using XSWAP pools", async function () {
</code_context>
<issue_to_address>
**suggestion (testing):** Missing test for edge cases in calcGDToSell (e.g., zero input, very large input).
Add tests for calcGDToSell with zero and very large G$ inputs to verify correct handling of boundary conditions.
</issue_to_address>
### Comment 2
<location> `test/reserve/GenericDistributionHelper.e2e.test.ts:322-331` </location>
<code_context>
+ it("should handle swap with correct slippage protection", async function () {
</code_context>
<issue_to_address>
**suggestion (testing):** Missing test for slippage exceeding maxSlippage.
Add a test case where slippage exceeds maxSlippage and verify that the swap is reverted, confirming slippage protection works correctly.
</issue_to_address>
### Comment 3
<location> `test/reserve/GenericDistributionHelper.e2e.test.ts:343` </location>
<code_context>
const maxSlippage = feeSettings.maxSlippage;
</code_context>
<issue_to_address>
**suggestion (code-quality):** Prefer object destructuring when accessing and using properties. ([`use-object-destructuring`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/TypeScript/Default-Rules/use-object-destructuring))
```suggestion
const {maxSlippage} = feeSettings;
```
<br/><details><summary>Explanation</summary>Object destructuring can often remove an unnecessary temporary reference, as well as making your code more succinct.
From the [Airbnb Javascript Style Guide](https://airbnb.io/javascript/#destructuring--object)
</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.
sirpy
force-pushed
the
feat/fork-test/distribution-with-xswap
branch
from
71b4c84 to
d74faa9
Compare
Add xdc fork test for GenericDistributionHelper
🚨 Report Summary
For more details view the full report in OpenZeppelin Code Inspector |
…ibutionHelper E2E tests
…tributionHelper E2E tests
| minReceived: ethers.utils.formatEther(minReceived) | ||
| }); | ||
|
|
||
| expect(quoteWXDC).to.be.equal(minReceived); |
There was a problem hiding this comment.
why does the quote need to equal minreceived?
also see the AI comment that you arenot testing a case where slippage is larger
There was a problem hiding this comment.
because they are using:
Same input: amountToSell (100 G$)
Same path: G$ -> CUSD -> WXDC
Same oracle: STATIC_ORACLE.quoteAllAvailablePoolsWithTimePeriod
Same pools: both use the same G$/CUSD and CUSD/WXDC pools
I will add large slippage test
|
|
||
| before(async function () { | ||
| await networkHelpers.reset(XDC_RPC_URL); | ||
| const chainId = await ethers.provider.getNetwork().then(network => network.chainId); |
There was a problem hiding this comment.
it should use network.reset to always test on xdc
There was a problem hiding this comment.
It's not working with "hardhat test".
I tried with another hardhat version(2.28.3 - the one which is used mostly) but it still shows the same err (Error: Storage overrides are not supported for forked blocks yet. See NomicFoundation/edr#911)
I ran the tests against an XDC fork, and all tests passed.
There was a problem hiding this comment.
i've downgraded hardhat and it works.
There was a problem hiding this comment.
Yes, I thought the same
Thanks you!
Description
Added a GenericDistributionHelper XDC-fork test to verify that swapping G$ to XDC through xswap is working correctly.
About # (link your issue here)
How Has This Been Tested?
Please describe the tests that you ran to verify your changes.
Checklist: