Add networkId logic to superfluid subgraphs #1896
Conversation
WalkthroughThe pull request introduces network-specific enhancements to the SuperFluid adapter and related services. The primary modification involves adding a Changes
Possibly related PRs
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
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (6)
src/adapters/superFluid/superFluidAdapter.ts (4)
7-14: Mismatch in naming constants vs. actual chain references
The constants mention "Polygon" while the URLs point to Optimism. This can be confusing for future maintainers.Consider renaming them to something like
superFluidGraphqlOptimismUrlandsuperFluidGraphqlOptimismStagingUrlfor clarity.
23-30: Consider externalizing subgraph URLs
Storing these URLs in code is fine for now, but if additional networks or environments are added, you might consider loading them from configuration to ensure easy maintenance.
186-186: Validate typed responses
While the approach to fetchaccountTokenSnapshotsis correct, the method still returns untyped data (any). Consider defining an interface for typed responses to catch potential runtime issues.Also applies to: 188-188
233-233: Unify repeated flow query logic
getFlowByReceiverSenderFlowRateandgetFlowByTxHashshare similar code for constructing subgraph queries. Consider extracting common logic to maintain consistency and avoid duplication.Also applies to: 236-236, 237-237
src/adapters/superFluid/superFluidMockAdapter.ts (1)
98-98: Safe to ignore networkId
Returning a random string in the mock is acceptable. Ensure whenever your tests rely onnetworkId, you update the mock accordingly.test/graphqlQueries.ts (1)
2075-2092: Query looks good, but consider adding pagination or filter arguments to the query definition
The$networkId,$page, and$limitparameters are declared but not used within the query body. If you plan to implement pagination or additional filtering later, make sure to pass these parameters intorecurringDonationEligibleProjectsor remove them to avoid confusion.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
src/adapters/superFluid/superFluidAdapter.ts(4 hunks)src/adapters/superFluid/superFluidAdapterInterface.ts(1 hunks)src/adapters/superFluid/superFluidMockAdapter.ts(2 hunks)src/services/chains/evm/draftRecurringDonationService.ts(1 hunks)src/services/cronJobs/checkUserSuperTokenBalancesQueue.ts(1 hunks)src/services/recurringDonationService.ts(1 hunks)test/graphqlQueries.ts(1 hunks)
🔇 Additional comments (6)
src/adapters/superFluid/superFluidAdapter.ts (2)
16-21: Network-specific mapping looks solid
This map is comprehensive for the listed chains. Remember to add an entry or fallback whenever a new network is introduced.
207-207: Consistent usage of subgraph URLs
Including networkId to retrieve the correct subgraph URL is a good approach.
Also applies to: 210-210
src/adapters/superFluid/superFluidMockAdapter.ts (1)
88-88: Mock ignores new parameter
Including networkId in _params is fine for a mock. Just remain aware that if future logic depends on networkId, the mock will need to reflect that.
src/services/cronJobs/checkUserSuperTokenBalancesQueue.ts (1)
111-111: Good use of networkId for accountBalance
Passing the correct networkId ensures subgraph calls go to the right endpoint. The updated signature is properly used here.
src/services/chains/evm/draftRecurringDonationService.ts (1)
50-50: Consistent inclusion of networkId
Including networkId in getFlowParams aligns with the newly updated adapter methods, ensuring the correct subgraph is queried. Nicely done!
src/services/recurringDonationService.ts (1)
396-396: Validate networkId before calling getFlowByTxHash
While passing networkId is correct for ensuring the subgraph is queried for the right network, consider verifying that recurringDonation.networkId is valid and not null/undefined to prevent unexpected behavior or errors when awaiting the external call.
| @@ -20,12 +20,13 @@ export interface SuperFluidAdapterInterface { | |||
| currency: string; | |||
| recurringDonationTxHash: string; | |||
| }): Promise<any>; | |||
| accountBalance(accountId: string): Promise<any>; | |||
| accountBalance(accountId: string, networkId: number): Promise<any>; | |||
There was a problem hiding this comment.
Interface missing networkId in one of the method definitions
The getFlowByReceiverSenderFlowRate method in the adapter now accepts networkId, but the interface doesn't match. This can lead to inconsistent usage.
Apply this diff to keep the interface in sync:
export interface SuperFluidAdapterInterface {
...
getFlowByReceiverSenderFlowRate(params: {
receiver: string;
sender: string;
flowRate: string;
timestamp_gt: number;
+ networkId: number;
}): Promise<FlowUpdatedEvent | undefined>;
}Also applies to: 29-29
|
@CarlosQ96 it fails somewhere :( |
There was a problem hiding this comment.
Actionable comments posted: 3
🔭 Outside diff range comments (1)
src/adapters/superFluid/superFluidAdapter.ts (1)
Based on the analysis of the error handling patterns in the codebase, particularly in the
superFluidAdapter.tsfile and other network request handlers, I can provide the final response:Inconsistent error handling in network requests needs attention
The error handling across network requests shows inconsistencies that need to be addressed:
accountBalance()method swallows the error and returns undefinedgetFlowByTxHash()andgetFlowByReceiverSenderFlowRate()rethrow the error after logging- No validation of response data before accessing nested properties (e.g.,
response.data.data.account?.accountTokenSnapshots)Recommendations:
- Add proper error handling with specific error types for different failure scenarios
- Validate response data before accessing nested properties
- Be consistent with error propagation - either handle errors at this layer or propagate them
- Consider adding retry logic for transient network failures
🔗 Analysis chain
Line range hint
186-258: Verify error handling across network requestsGiven the user comment "it fails somewhere :(", we should verify the error handling across all network requests.
Run the following script to analyze error handling patterns:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check error handling patterns in network requests # Look for axios error handling patterns rg -A 5 "catch \(e\)" --type ts # Look for potential network-related error handling ast-grep --pattern 'try { $$$ await axios.$$$ $$$ } catch (e) { $$$ }'Length of output: 71181
🧰 Tools
🪛 GitHub Check: test
[failure] 242-242:
'networkId' is assigned a value but never used🪛 eslint
[error] 242-242: 'networkId' is assigned a value but never used.
(@typescript-eslint/no-unused-vars)
🧹 Nitpick comments (1)
src/adapters/superFluid/superFluidAdapter.ts (1)
24-30: Consider graceful fallback for getSubgraphUrlThe current implementation throws an error if no URL is found. Consider adding a fallback mechanism or default URL for better resilience.
Example implementation:
const getSubgraphUrl = (networkId: number) => { const url = subgraphUrlMap[networkId]; if (!url) { - throw new Error(`No subgraph URL found for network ID: ${networkId}`); + logger.warn(`No specific subgraph URL found for network ID: ${networkId}, using default`); + return superFluidGraphqlOptimismUrl; // or another suitable default } return url; };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/adapters/superFluid/superFluidAdapter.ts(4 hunks)src/adapters/superFluid/superFluidAdapterInterface.ts(1 hunks)
🧰 Additional context used
🪛 GitHub Check: test
src/adapters/superFluid/superFluidAdapter.ts
[failure] 211-211:
'networkId' is assigned a value but never used
[failure] 242-242:
'networkId' is assigned a value but never used
🪛 eslint
src/adapters/superFluid/superFluidAdapter.ts
[error] 211-211: 'networkId' is assigned a value but never used.
(@typescript-eslint/no-unused-vars)
[error] 242-242: 'networkId' is assigned a value but never used.
(@typescript-eslint/no-unused-vars)
🔇 Additional comments (5)
src/adapters/superFluid/superFluidAdapterInterface.ts (3)
23-23: LGTM: accountBalance signature updated correctly
The networkId parameter has been properly added to the accountBalance method signature.
29-29: LGTM: getFlowByTxHash signature updated correctly
The networkId parameter has been properly added to the getFlowByTxHash method parameters.
36-36: LGTM: getFlowByReceiverSenderFlowRate signature updated correctly
The networkId parameter has been properly added to the getFlowByReceiverSenderFlowRate method parameters.
src/adapters/superFluid/superFluidAdapter.ts (2)
16-21: Verify BASE_SEPOLIA configuration
BASE_MAINNET and BASE_SEPOLIA are using the same URL. This might be incorrect as typically testnet and mainnet use different subgraph endpoints.
Please verify if this is intentional or if BASE_SEPOLIA should use a different URL.
186-188: LGTM: accountBalance implementation
The networkId parameter is properly used to get the correct subgraph URL.
| const superFluidGraphqlPolygonUrl = | ||
| 'https://subgraph-endpoints.superfluid.dev/optimism-mainnet/protocol-v1'; | ||
| const superFluidGraphqlStagingUrl = | ||
| const superFluidGraphqlPolygonStagingUrl = | ||
| 'https://optimism-sepolia.subgraph.x.superfluid.dev'; | ||
| const superFluidGraphBaseUrl = | ||
| 'https://subgraph-endpoints.superfluid.dev/base-mainnet/protocol-v1'; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Fix misleading constant names
The constant names suggest Polygon network but they're actually for Optimism network. This could lead to confusion and maintenance issues.
Apply this diff to fix the naming:
-const superFluidGraphqlPolygonUrl =
+const superFluidGraphqlOptimismUrl =
'https://subgraph-endpoints.superfluid.dev/optimism-mainnet/protocol-v1';
-const superFluidGraphqlPolygonStagingUrl =
+const superFluidGraphqlOptimismSepoliaUrl =
'https://optimism-sepolia.subgraph.x.superfluid.dev';📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const superFluidGraphqlPolygonUrl = | |
| 'https://subgraph-endpoints.superfluid.dev/optimism-mainnet/protocol-v1'; | |
| const superFluidGraphqlStagingUrl = | |
| const superFluidGraphqlPolygonStagingUrl = | |
| 'https://optimism-sepolia.subgraph.x.superfluid.dev'; | |
| const superFluidGraphBaseUrl = | |
| 'https://subgraph-endpoints.superfluid.dev/base-mainnet/protocol-v1'; | |
| const superFluidGraphqlOptimismUrl = | |
| 'https://subgraph-endpoints.superfluid.dev/optimism-mainnet/protocol-v1'; | |
| const superFluidGraphqlOptimismSepoliaUrl = | |
| 'https://optimism-sepolia.subgraph.x.superfluid.dev'; | |
| const superFluidGraphBaseUrl = | |
| 'https://subgraph-endpoints.superfluid.dev/base-mainnet/protocol-v1'; |
| const subgraphUrl = getSubgraphUrl(params.networkId); | ||
| const { networkId, ...whereParams } = params; | ||
|
|
||
| const response = await axios.post(subgraphUrl, { | ||
| query: getFlowsQuery, | ||
| variables: { | ||
| where: params, | ||
| where: whereParams, |
There was a problem hiding this comment.
Fix unused networkId variable
The destructured networkId variable is not used. The linter correctly flagged this issue.
Apply this diff to fix the issue:
- const { networkId, ...whereParams } = params;
+ const { networkId: _, ...whereParams } = params;This change explicitly indicates that we're intentionally not using the networkId after destructuring.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const subgraphUrl = getSubgraphUrl(params.networkId); | |
| const { networkId, ...whereParams } = params; | |
| const response = await axios.post(subgraphUrl, { | |
| query: getFlowsQuery, | |
| variables: { | |
| where: params, | |
| where: whereParams, | |
| const subgraphUrl = getSubgraphUrl(params.networkId); | |
| const { networkId: _, ...whereParams } = params; | |
| const response = await axios.post(subgraphUrl, { | |
| query: getFlowsQuery, | |
| variables: { | |
| where: whereParams, |
🧰 Tools
🪛 GitHub Check: test
[failure] 211-211:
'networkId' is assigned a value but never used
🪛 eslint
[error] 211-211: 'networkId' is assigned a value but never used.
(@typescript-eslint/no-unused-vars)
| const subgraphUrl = getSubgraphUrl(params.networkId); | ||
|
|
||
| logger.debug('getFlowByReceiverSenderFlowRate has been called', params); | ||
|
|
||
| const { networkId, ...whereParams } = params; | ||
|
|
||
| const response = await axios.post(subgraphUrl, { | ||
| query: getFlowsQuery, | ||
| variables: { | ||
| where: params, | ||
| where: whereParams, |
There was a problem hiding this comment.
Fix unused networkId variable and improve logging
Similar to the previous issue, the networkId variable is unused. Also, the debug log could be more informative.
Apply this diff to fix these issues:
const subgraphUrl = getSubgraphUrl(params.networkId);
- logger.debug('getFlowByReceiverSenderFlowRate has been called', params);
+ logger.debug('getFlowByReceiverSenderFlowRate called with params:', {
+ ...params,
+ subgraphUrl
+ });
- const { networkId, ...whereParams } = params;
+ const { networkId: _, ...whereParams } = params;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const subgraphUrl = getSubgraphUrl(params.networkId); | |
| logger.debug('getFlowByReceiverSenderFlowRate has been called', params); | |
| const { networkId, ...whereParams } = params; | |
| const response = await axios.post(subgraphUrl, { | |
| query: getFlowsQuery, | |
| variables: { | |
| where: params, | |
| where: whereParams, | |
| const subgraphUrl = getSubgraphUrl(params.networkId); | |
| logger.debug('getFlowByReceiverSenderFlowRate called with params:', { | |
| ...params, | |
| subgraphUrl | |
| }); | |
| const { networkId: _, ...whereParams } = params; | |
| const response = await axios.post(subgraphUrl, { | |
| query: getFlowsQuery, | |
| variables: { | |
| where: whereParams, |
🧰 Tools
🪛 GitHub Check: test
[failure] 242-242:
'networkId' is assigned a value but never used
🪛 eslint
[error] 242-242: 'networkId' is assigned a value but never used.
(@typescript-eslint/no-unused-vars)
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/adapters/superFluid/superFluidAdapter.ts (1)
9-14:⚠️ Potential issueFix misleading constant names
The constant names suggest Polygon network but they're actually for Optimism network. This could lead to confusion and maintenance issues.
Apply this diff to fix the naming:
-const superFluidGraphqlPolygonUrl = +const superFluidGraphqlOptimismUrl = 'https://subgraph-endpoints.superfluid.dev/optimism-mainnet/protocol-v1'; -const superFluidGraphqlPolygonStagingUrl = +const superFluidGraphqlOptimismSepoliaUrl = 'https://optimism-sepolia.subgraph.x.superfluid.dev';
🧹 Nitpick comments (3)
src/adapters/superFluid/superFluidAdapter.ts (3)
186-188: Enhance error loggingConsider including networkId in error logs and properly typing the error for better debugging.
Apply this diff:
async accountBalance(accountId: string, networkId: number) { try { const subgraphUrl = getSubgraphUrl(networkId); const response = await axios.post(subgraphUrl, { query: accountQuery, variables: { id: accountId?.toLowerCase(), }, }); return response.data.data.account?.accountTokenSnapshots; } catch (e) { - logger.error('superFluidAdaptor.accountBalance error', e); + logger.error('superFluidAdaptor.accountBalance error', { + error: e instanceof Error ? e.message : String(e), + accountId, + networkId, + }); } }
211-216: Simplify parameter handling and improve error loggingThe parameter handling can be simplified using destructuring, and error logging can be enhanced.
Apply this diff:
- const whereParams = { - receiver: params.receiver, - sender: params.sender, - flowRate: params.flowRate, - transactionHash: params.transactionHash, - }; + const { networkId, ...whereParams } = params;Also enhance error logging:
} catch (e) { - logger.error('getFlowByReceiverSenderFlowRate error', e); + logger.error('getFlowByTxHash error', { + error: e instanceof Error ? e.message : String(e), + params, + }); throw e; }
247-252: Enhance parameter handling and loggingSimilar to getFlowByTxHash, parameter handling can be simplified and logging can be improved.
Apply this diff:
- logger.debug('getFlowByReceiverSenderFlowRate has been called', params); + logger.debug('getFlowByReceiverSenderFlowRate called', { + params, + subgraphUrl, + }); - const whereParams = { - receiver: params.receiver, - sender: params.sender, - flowRate: params.flowRate, - timestamp_gt: params.timestamp_gt, - }; + const { networkId, ...whereParams } = params;Also enhance error logging:
} catch (e) { - logger.error('getFlowByReceiverSenderFlowRate error', e); + logger.error('getFlowByReceiverSenderFlowRate error', { + error: e instanceof Error ? e.message : String(e), + params, + }); throw e; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/adapters/superFluid/superFluidAdapter.ts(4 hunks)
🔇 Additional comments (2)
src/adapters/superFluid/superFluidAdapter.ts (2)
23-30: LGTM! Well-implemented helper function
The function properly handles network ID validation with clear error messaging.
16-21: Verify network ID handling across the codebase
Given the comment "it fails somewhere", let's verify that all calls to these methods provide the correct network ID.
Run this script to check network ID usage:
✅ Verification successful
Let me gather more information about the network ID usage in these specific files and the parameters being passed.
Network ID handling is properly implemented and consistent
Based on the verification results, all SuperFluid adapter methods correctly handle network IDs:
- The interface (
superFluidAdapterInterface.ts) properly defines networkId as a required parameter for all methods - The implementation (
superFluidAdapter.ts) correctly uses networkId to get the appropriate subgraph URL - All service calls properly pass the networkId:
checkUserSuperTokenBalancesQueue.tspassesrecurringDonation.networkIddraftRecurringDonationService.tsincludes networkId ingetFlowParamsrecurringDonationService.tspasses networkId in all adapter calls
The "it fails somewhere" comment appears unrelated to network ID handling as the implementation is consistent and complete across the codebase.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if networkId is consistently passed to SuperFluid adapter methods
# Find all instances where SuperFluid adapter methods are called
echo "Checking SuperFluid adapter method calls..."
rg -A 2 "superFluidAdapter\.(accountBalance|getFlowByTxHash|getFlowByReceiverSenderFlowRate)"
# Find potential places where networkId might be undefined
echo "Checking for potential undefined networkId..."
ast-grep --pattern 'superFluidAdapter.$_($$$ networkId: $_? $$$)'
Length of output: 1285
Script:
#!/bin/bash
# Check the implementation of these methods and their parameters
echo "Checking superFluidAdapter implementation and parameters..."
rg -A 10 "getFlowByTxHash|accountBalance|getFlowByReceiverSenderFlowRate" src/adapters/superFluid/
# Check the context of the calls to verify networkId passing
echo "Checking context of superFluidAdapter calls..."
rg -B 5 -A 5 "superFluidAdapter\." src/services/recurringDonationService.ts src/services/cronJobs/checkUserSuperTokenBalancesQueue.ts src/services/chains/evm/draftRecurringDonationService.ts
# Check the getFlowParams construction
echo "Checking getFlowParams construction..."
rg "getFlowParams" src/services/chains/evm/draftRecurringDonationService.ts -B 5 -A 2
Length of output: 13206
* add networkId logic to superfluid subgraphs * remove networkId from api call to superfluid * fix eslint
Summary by CodeRabbit
networkIdparameter for improved network context handling.