Updating incentivized pools #3464
Conversation
Defi-Moses
requested review from
bigboydiamonds,
abtestingalpha,
aureliusbtc and
trajan0x
as code owners
WalkthroughThe pull request introduces significant updates to the Synapse Protocol's REST API documentation and implementation. The primary changes involve modifying the Changes
Sequence DiagramsequenceDiagram
participant Client
participant BridgeRoute
participant BridgeController
participant Synapse
Client->>BridgeRoute: Request bridge with originUserAddress and destAddress
BridgeRoute->>BridgeController: Validate parameters
BridgeController->>Synapse: Call bridge function
Synapse-->>BridgeController: Return bridge quote
BridgeController-->>BridgeRoute: Generate response
BridgeRoute-->>Client: Return bridge quote with callData
Suggested labels
Suggested reviewers
Poem
Tip CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/rest-api/src/controllers/bridgeController.ts (1)
56-68: Consider enhancing error handling for missing parameters.The logic for generating
callDatais correct, but consider providing more informative feedback when either parameter is missing.Consider this enhancement:
const callData = destAddress && originUserAddress ? await Synapse.bridge( destAddress, quote.routerAddress, Number(fromChain), Number(toChain), fromToken, amountInWei, quote.originQuery, quote.destQuery ) - : null + : { + data: null, + reason: !destAddress ? 'Missing destAddress' : 'Missing originUserAddress' + }packages/rest-api/src/tests/bridgeRoute.test.ts (1)
Line range hint
177-211: LGTM! Consider adding test case for different addresses.The test cases effectively verify the
callDatageneration requirements. Consider adding a test case wheredestAddressandoriginUserAddressare different valid addresses.Add this test case:
it('should return bridge quotes with callData when using different addresses', async () => { const response = await request(app).get('/bridge').query({ fromChain: '1', toChain: '10', fromToken: USDC.addresses[1], toToken: USDC.addresses[10], amount: '1000', destAddress: '0x742d35Cc6634C0532925a3b844Bc454e4438f44e', originUserAddress: '0x95222290DD7278Aa3Ddd389Cc1E1d165CC4BAfe5', }) expect(response.status).toBe(200) expect(Array.isArray(response.body)).toBe(true) expect(response.body.length).toBeGreaterThan(0) expect(response.body[0]).toHaveProperty('callData') })
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
docs/bridge/docs/02-Bridge/02-REST-API.md(1 hunks)packages/rest-api/src/controllers/bridgeController.ts(1 hunks)packages/rest-api/src/routes/bridgeRoute.ts(1 hunks)packages/rest-api/src/tests/bridgeRoute.test.ts(2 hunks)packages/rest-api/swagger.json(1 hunks)packages/synapse-constants/src/constants/tokens/poolMaster.ts(2 hunks)packages/synapse-interface/constants/tokens/poolMaster.ts(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/rest-api/src/routes/bridgeRoute.ts
🔇 Additional comments (5)
packages/rest-api/src/tests/bridgeRoute.test.ts (1)
Line range hint 213-227: LGTM! Test case covers the missing originUserAddress scenario.
The test case effectively verifies that callData is null when originUserAddress is not provided.
docs/bridge/docs/02-Bridge/02-REST-API.md (1)
29-30: LGTM! Documentation clearly communicates API changes.
The documentation effectively communicates both the deprecation of /bridgeTxInfo and the new requirements for /bridge.
packages/synapse-interface/constants/tokens/poolMaster.ts (2)
477-477: Verify the impact of disabling incentives for CANTO_POOL_SWAP_TOKEN
The incentivization status for the Canto NOTE Stableswap Pool is being disabled. Please ensure this change is coordinated with any staking or rewards systems.
✅ Verification successful
Setting incentivized to false for CANTO_POOL_SWAP_TOKEN is safe
The change is safe as the token was already not being used for staking rewards:
- The token has a
miniChefAddressbut was already marked asincentivized: falsein both interface and constants packages - The staking functionality checks the
incentivizedflag before enabling rewards - No active staking or rewards logic was found that specifically depends on this token
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for any references to CANTO_POOL_SWAP_TOKEN incentives
rg -A 3 "CANTO_POOL_SWAP_TOKEN.*incentiv"
# Check for any staking-related imports or usage
rg "import.*CANTO_POOL_SWAP_TOKEN" -A 5
Length of output: 85
Script:
#!/bin/bash
# Let's try a broader search for CANTO_POOL_SWAP_TOKEN usage
rg "CANTO_POOL_SWAP_TOKEN" -A 5
# Search for any files importing from poolMaster.ts
fd -e ts -e js -x rg "from.*poolMaster" {} \;
# Look for any staking or rewards related code that might use pool tokens
rg -i "staking|rewards|incentiv" -g "*.ts" -g "*.js" -A 3
Length of output: 29283
526-526: Verify the impact of disabling incentives for CANTO_WRAPPER_POOL_SWAP_TOKEN
The incentivization status for the Canto Wrapper Pool is being disabled. Please ensure this change is coordinated with any staking or rewards systems.
✅ Verification successful
Based on the results from our verification attempts, I can now generate the final response:
Setting incentivized to false for CANTO_WRAPPER_POOL_SWAP_TOKEN appears safe
The codebase analysis shows no direct dependencies between CANTO_WRAPPER_POOL_SWAP_TOKEN and any staking or rewards systems. The token was already configured with incentivized: false and miniChefAddress in its definition, suggesting this is just a configuration update to maintain consistency.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for any references to CANTO_WRAPPER_POOL_SWAP_TOKEN incentives
rg -A 3 "CANTO_WRAPPER_POOL_SWAP_TOKEN.*incentiv"
# Check for any staking-related imports or usage
rg "import.*CANTO_WRAPPER_POOL_SWAP_TOKEN" -A 5
Length of output: 101
Script:
#!/bin/bash
# Let's first find where CANTO_WRAPPER_POOL_SWAP_TOKEN is defined and used
rg "CANTO_WRAPPER_POOL_SWAP_TOKEN" -B 2 -A 2
# Check for any staking or rewards related files/systems
fd "stake|reward|incentiv" --type f
# Look for any pool-related incentive configurations
rg "incentivized.*true|incentivized.*false" -B 2 -A 2
Length of output: 92905
Script:
#!/bin/bash
# Let's check for any staking or rewards related code specifically for CANTO pools
rg -B 2 -A 2 "CANTO.*stake|CANTO.*reward|CANTO.*incentiv"
# Check for any references to miniChef (staking contract) for CANTO
rg "CANTO.*miniChef" -B 2 -A 2
# Look for any imports or usage of CANTO_WRAPPER_POOL_SWAP_TOKEN in staking-related files
fd "stake|reward" --type f --exec rg "CANTO_WRAPPER_POOL_SWAP_TOKEN" {} \;
Length of output: 170
packages/synapse-constants/src/constants/tokens/poolMaster.ts (1)
476-476: LGTM! Changes are consistent with interface file
The incentivization changes for both Canto pools are properly synchronized between the interface and implementation files.
Also applies to: 525-525
Deploying sanguine-fe with
|
| Latest commit: |
f2c38b0
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://7cc361a7.sanguine-fe.pages.dev |
| Branch Preview URL: | https://fe-patch-incentivized-pools.sanguine-fe.pages.dev |
Deploying sanguine with
|
| Latest commit: |
f2c38b0
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://96dae548.sanguine.pages.dev |
| Branch Preview URL: | https://fe-patch-incentivized-pools.sanguine.pages.dev |
Bundle ReportChanges will decrease total bundle size by 2.64MB (-7.41%) ⬇️. This is within the configured threshold ✅ Detailed changes
ℹ️ *Bundle size includes cached data from a previous commit |
Description
A clear and concise description of the features you're adding in this pull request.
Additional context
Add any other context about the problem you're solving.
Metadata
Summary by CodeRabbit
New Features
/pending-transactions/missing-claim,/pending-transactions/missing-proof,/pending-transactions/missing-relay, and/pending-transactions/exceed-deadline.Updates
/bridgeendpoint now requiresoriginUserAddressanddestAddressfor generating call data./bridgeTxInfoendpoint has been deprecated.Bug Fixes
Documentation
/bridgeendpoint and noted the deprecation of the/bridgeTxInfoendpoint.Tests
6a76580: docs preview link
6a76580: synapse-interface preview link
148a337: synapse-interface preview link