-
-
Notifications
You must be signed in to change notification settings - Fork 246
Enhancements/evm and uniswap-like_connectors #540
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Enhancements/evm and uniswap-like_connectors #540
Conversation
…d Uniswap connectors - Set default slippage percentage to be fetched from respective configuration files in addLiquidity, executeSwap, and quoteSwap functions. - Remove redundant slippage percentage checks and ensure consistent handling across all swap and liquidity functions. - Clean up unused imports and commented-out code in schemas and routes.
- Removed the waitForTransactionWithTimeout utility function and replaced its usage with handleTransactionExecution across various routes and connectors. - Updated transaction confirmation logic to utilize the new handleExecuteQuoteTransactionConfirmation method in the 0x and Uniswap connectors. - Introduced transactionExecutionTimeoutMs configuration in chain templates for better timeout management. - Adjusted transaction status handling to ensure accurate reporting of transaction outcomes.
- Removed the waitForTransactionWithTimeout utility function and replaced its usage with handleTransactionExecution across various routes and connectors. - Updated transaction confirmation logic to utilize the new handleExecuteQuoteTransactionConfirmation method in the 0x and Uniswap connectors. - Introduced transactionExecutionTimeoutMs configuration in chain templates for better timeout management. - Adjusted transaction status handling to ensure accurate reporting of transaction outcomes.
|
thanks @vic-en. can you provide more details on this change (why it's needed, the solution implemented, etc):
@rapcmia @nikspz This PR should be slated for the following release (v.2.11) |
@fengtality
My solution:
|
|
@fengtality For Pool/Pair Address Derivation Inconsistency, my plan is to replace |
fengtality
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One change requested, but otherwise PGTM
src/chains/ethereum/ethereum.ts
Outdated
| return { | ||
| signature: '', | ||
| status: 0, // PENDING | ||
| status: -1, // PENDING |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use -1 for failed and 0 for pending.
This is what Hummingbot expects:
class TransactionStatus(Enum):
"""Transaction status constants for gateway operations."""
CONFIRMED = 1
PENDING = 0
FAILED = -1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you can also just use -1 and never 0 since I don't think we're handling a pending state
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I changed -1 to 0 and 0 to -1 to make it work with Hummingbot.
However, defaulting to -1 (i.e. FAILED) will cause issues on the client. Orders would be reported as failed and dropped from the in-flight tracker - https://github.com/hummingbot/hummingbot/blob/5f447fe56a777c84ab7a9e5e9879dc9e922b751b/hummingbot/connector/gateway/gateway_base.py#L591
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please use 0 for pending and -1 for failed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@fengtality I think I did already
Note that However, for the liquidity routes, they should all have Note that I'm working some improvements in #543. I'll ask QA to review both branches separately, and I'll handle any conflicts upon merge. |
|
commit 11a47f9 steps:
Actual:
|
|
@nikspz Here's the branch with the fix https://github.com/hummingbot/gateway/tree/enhancements/uniswap-like_connectors |
I got same error using enhancements/uniswap-like_connectors https://github.com/hummingbot/gateway.git src/connectors/pancakeswap/clmm-routes/addLiquidity.ts:65:27 - error TS2304: Cannot find name 'utils'. 65 const baseAmountRaw = utils.parseUnits( src/connectors/pancakeswap/clmm-routes/addLiquidity.ts:78:28 - error TS2304: Cannot find name 'utils'. 78 const quoteAmountRaw = utils.parseUnits( Found 2 errors in the same file, starting at: src/connectors/pancakeswap/clmm-routes/addLiquidity.ts:65 |
|
@vic-en could you please review |
…d Uniswap connectors - Set default slippage percentage to be fetched from respective configuration files in addLiquidity, executeSwap, and quoteSwap functions. - Remove redundant slippage percentage checks and ensure consistent handling across all swap and liquidity functions. - Clean up unused imports and commented-out code in schemas and routes.
- Removed the waitForTransactionWithTimeout utility function and replaced its usage with handleTransactionExecution across various routes and connectors. - Updated transaction confirmation logic to utilize the new handleExecuteQuoteTransactionConfirmation method in the 0x and Uniswap connectors. - Introduced transactionExecutionTimeoutMs configuration in chain templates for better timeout management. - Adjusted transaction status handling to ensure accurate reporting of transaction outcomes.
|
@nikspz It's updated now |
|
|
@nikspz please test the fixes that Victor made in this PR. Pancakeswap-SOL is not related to his changes.
|
nikspz
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Test performed:
- Pool/Pair Address Derivation Inconsistency: created tx successfully without using pool address and using pool address for uniswap/amm
- uniswap/amm
- POST /connectors/uniswap/amm/execute-swap with "poolAddress": "" empty field: ok ✅
- POST /connectors/uniswap/amm/execute-swap using "poolAddress": "0x88A43bbDF9D098eEC7bCEda4e2494615dfD9bB9C”: ok ✅
- uniswap/router
- POST /connectors/uniswap/router/execute-swap: ok ✅
- uniswap/amm
- Slippage Parameter Handling
- set connector config slippage
config/uniswap.ymlslippagePct: 2 ✅ -
- GET /connectors/uniswap/amm/quote-swap
- /connectors/uniswap/amm/swap
- "slippagePct": 3: success ✅
- Response: "slippagePct": 3
- "slippagePct": null: success ✅
- "slippagePct": 1: success ✅
- 2025-11-11 08:11:07 | info | Transaction sent: 0xc6027a115162a79e9a0eca8c6e54cd61bd7b89da6e8b2c2ab3222ac31069b9b0
- 2025-11-11 08:11:17 | warn | Transaction 0xc6027a115162a79e9a0eca8c6e54cd61bd7b89da6e8b2c2ab3222ac31069b9b0 is still pending after timeout
- https://basescan.org/tx/0xc6027a115162a79e9a0eca8c6e54cd61bd7b89da6e8b2c2ab3222ac31069b9b0
- "slippagePct": 3: success ✅
- GET /connectors/uniswap/amm/quote-liquidity: ok
- POST /connectors/uniswap/amm/add-liquidity
- "slippagePct": 5: ok
- "slippagePct": 1: ok
- set connector config slippage
- sqrtPriceLimit Not Enforced in CLMM Swap
-
GET /connectors/uniswap/clmm/quote-swap: ok
-
POST /connectors/uniswap/clmm/execute-swap: ok
- "slippagePct": 1: ok
- "slippagePct": null: ok
-
- Transaction Receipt Waiting Pattern
- default transactionExecutionTimeoutMs: 10000: ok ✅ (no timeout)
- transactionExecutionTimeoutMs: 1000:
- 2025-11-11 08:19:44 | info | Transaction sent: ✅0x74b70d95e29fc7aeea7d176f1f9825e21f0e945c5a81336dd55357653bb1f242
- 2025-11-11 08:19:45 | warn | Transaction 0x74b70d95e29fc7aeea7d176f1f9825e21f0e945c5a81336dd55357653bb1f242 is still pending after timeout ✅
- 2025-11-11 08:19:45 | error | Transaction failed on-chain. Receipt: {"transactionHash":"0x74b70d95e29fc7aeea7d176f1f9825e21f0e945c5a81336dd55357653bb1f242","blockHash":"","blockNumber":null,"transactionIndex":null,"from":"0x08940dc9B5a19FAb9319b77C61DDA7B8067E6843","to":"0x4752ba5DBc23f44D87826276BF6Fd6b1C372aD24","cumulativeGasUsed":{"type":"BigNumber","hex":"0x00"},"gasUsed":{"type":"BigNumber","hex":"0x00"},"contractAddress":null,"logs":[],"logsBloom":"","status":0,"effectiveGasPrice":{"type":"BigNumber","hex":"0x00"}}
- 2025-11-11 08:19:45 | error | Swap execution error: Transaction reverted on-chain. This could be due to slippage, insufficient funds, or other blockchain issues.
- 2025-11-11 08:19:48 | info | Transaction 0x74b70d95e29fc7aeea7d176f1f9825e21f0e945c5a81336dd55357653bb1f242 confirmed in block 38044320 ✅
- transactionExecutionTimeoutMs: 5000 ✅
- same behavior after 5s
- Pool/Pair Address Derivation Inconsistency: created tx successfully without using pool address and using pool address for uniswap/amm
Before submitting this PR, please make sure:
A description of the changes proposed in the pull request:
1. Slippage Parameter Inconsistency
2. Pool/Pair Address Derivation Inconsistency
3. sqrtPriceLimit Not Enforced in CLMM Swap
4. Transaction Receipt Waiting Pattern
tx.wait()for receipt and set status to 1 in responsestransactionExecutionTimeoutMsparameter in config for each evm chain.Tests performed by the developer:
WIP
Tips for QA testing: