Skip to content

feat(1748): first draft of saucerswap plugin#1754

Open
mmyslblocky wants to merge 3 commits intomainfrom
feat/1748-adr-for-saucerswap
Open

feat(1748): first draft of saucerswap plugin#1754
mmyslblocky wants to merge 3 commits intomainfrom
feat/1748-adr-for-saucerswap

Conversation

@mmyslblocky
Copy link
Copy Markdown
Contributor

Issue #1748

Signed-off-by: mmyslblocky <michal.myslinski@blockydevs.com>
@mmyslblocky mmyslblocky requested a review from a team as a code owner April 1, 2026 20:22
@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 1, 2026

Unit Test Results

1 100 tests  ±0   1 100 ✅ ±0   1m 39s ⏱️ -10s
  113 suites ±0       0 💤 ±0 
    1 files   ±0       0 ❌ ±0 

Results for commit 166abf3. ± Comparison against base commit 31a7ea4.

♻️ This comment has been updated with latest results.

@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 1, 2026

Integration Test Results

37 tests  ±0   35 ✅ ±0   3m 51s ⏱️ ±0s
21 suites ±0    2 💤 ±0 
 1 files   ±0    0 ❌ ±0 

Results for commit 166abf3. ± Comparison against base commit 31a7ea4.

♻️ This comment has been updated with latest results.

[SupportedNetwork.MAINNET]: {
routerContractId: '0.0.3045981',
factoryContractId: '0.0.2895920', // UniswapV2Factory
whbarTokenId: '0.0.1456986',
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do you need this?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and should we not have some definitions set in the config of the default contracts https://docs.saucerswap.finance/developerx/contract-deployments ? Probably I should also add the possiblity to override the defualt values here, but I think the default values could be set here

Comment on lines +141 to +150
This detection affects which Solidity function is called:

| Operation | Both tokens are HTS | One token is HBAR |
| ------------------ | -------------------------- | -------------------------------------------------- |
| Create pool | `addLiquidityNewPool` | `addLiquidityETHNewPool` |
| Deposit | `addLiquidity` | `addLiquidityETH` |
| Withdraw | `removeLiquidity` | `removeLiquidityETH` |
| Swap (exact input) | `swapExactTokensForTokens` | `swapExactETHForTokens` or `swapExactTokensForETH` |
| Buy (exact output) | `swapTokensForExactTokens` | `swapETHForExactTokens` or `swapTokensForExactETH` |

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think for now we just need:

  • swapExactETHForTokens or swapExactTokensForETH
  • swapETHForExactTokens or swapTokensForExactETH

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think for monitoring and trading you are right that we need only those probably, but maybe also we should add these creation of liquidity pool, deposit and withtdraw also? Just to have more manageable plugin for saucerswap?

Comment on lines +431 to +437
const functionParameters = new ContractFunctionParameters()
.addUint256(params.amountIn)
.addUint256(params.amountOutMin)
.addAddressArray(path)
.addAddress(params.recipientEvm)
.addUint256(deadline);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why don't you use: the abi + encodeFunctionData from ethers?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but is that necessary. I think for erc-20 and erc-721 we did it through the contract parameters defined in that way. One place where we defined the parameters through ethers + abi was for mirror node call, because we didn't want query fees present. Could we leave like that?


```ts
async buildTransaction(args, params): Promise<BuildTransactionResult> {
const path = [params.fromEvmAddress, params.toEvmAddress];
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

don't you also need the pool fees?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think in V1 we do not need to inlcude the pool fees as it always is 0.3%, but I am not quite sure about it. Can provide more detail in your comment?

Signed-off-by: mmyslblocky <michal.myslinski@blockydevs.com>
Signed-off-by: mmyslblocky <michal.myslinski@blockydevs.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants