Superfluid Base Support #1893
Superfluid Base Support #1893
Conversation
CarlosQ96
changed the title
WalkthroughThis pull request introduces enhancements to several files related to recurring donations and token management. The changes primarily focus on expanding super token definitions, improving network ID handling, and adding a new method to retrieve eligible recurring donation projects. The modifications span across multiple services and resolvers, introducing more flexible token and network-specific logic while maintaining the existing code structure. Changes
Possibly related PRs
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
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 (8)
src/services/cronJobs/checkUserSuperTokenBalancesQueue.ts (1)
120-133: Simplify redundant assignment ofsuperTokenDataArraySince
superTokenDataArrayis already initialized tosuperTokens, reassigning it in theelse ifblock is redundant. You can simplify the code by removing the redundant assignment.Apply this diff to simplify the code:
let superTokenDataArray = superTokens; if ( recurringDonation.networkId === NETWORK_IDS.BASE_SEPOLIA || recurringDonation.networkId === NETWORK_IDS.BASE_MAINNET ) { superTokenDataArray = superTokensBase; - } else if ( - recurringDonation.networkId === NETWORK_IDS.OPTIMISM_SEPOLIA || - recurringDonation.networkId === NETWORK_IDS.OPTIMISTIC - ) { - superTokenDataArray = superTokens; }src/resolvers/recurringDonationResolver.test.ts (7)
32-35: Test suite needs additional test cases.While the basic happy path is covered, consider adding test cases for:
- Error scenarios (e.g., invalid network IDs)
- Edge cases (e.g., projects with no anchor contracts)
- Pagination and filtering capabilities
71-148: Consider improving test data setup and assertions.The test case could be improved in several ways:
- Use descriptive variable names instead of generic ones (e.g.,
optimisticAnchorContractinstead ofanchorContractAddress)- Add assertions for the
isActivefield of anchor contracts- Consider testing the ordering of returned anchor contracts
Here's a suggested refactor:
- const anchorContractAddress = await addNewAnchorAddress({ + const optimisticAnchorContract = await addNewAnchorAddress({ project, owner: projectOwner, creator: projectOwner, address: generateRandomEtheriumAddress(), networkId: NETWORK_IDS.OPTIMISTIC, txHash: generateRandomEvmTxHash(), }); - const anchorContractAddressBASE = await addNewAnchorAddress({ + const baseAnchorContract = await addNewAnchorAddress({ project, owner: projectOwner, creator: projectOwner, address: generateRandomEtheriumAddress(), networkId: NETWORK_IDS.BASE_MAINNET, txHash: generateRandomEvmTxHash(), }); // Add assertions for isActive field + assert.isTrue(optimisticContract.isActive, 'Optimistic contract should be active'); + assert.isTrue(baseContract.isActive, 'BASE contract should be active');
149-149: Add validation test cases for recurring donation creation.Consider adding test cases for:
- Invalid currency validation
- Invalid flowRate validation (negative values, zero, extremely large values)
- Network-specific validations for Superfluid tokens
Line range hint
1893-1893: Add concurrency test cases for recurring donation updates.Consider adding test cases for:
- Concurrent updates to the same donation
- Partial updates (updating only specific fields)
- State transition validations (e.g., can't archive an active donation)
Line range hint
2071-2071: Enhance query test coverage.Consider adding test cases for:
- Complex filter combinations (e.g., multiple networks + status + date range)
- Edge cases in sorting (null values, same values)
- Performance testing with large result sets
Line range hint
2489-2489: Add comprehensive status transition test cases.Consider adding test cases for:
- All possible status transitions (create a transition matrix)
- Invalid status transitions
- Status updates with invalid permissions
Line range hint
1-31: Consider adding test documentation and improving error assertions.
- Add JSDoc comments for each test suite describing:
- Purpose of the test suite
- Test scenarios covered
- Expected behaviors
- Use consistent error message assertions:
- Create constants for expected error messages
- Add type checking for error responses
Example documentation:
/** * Test suite for recurring donation eligible projects * * Scenarios covered: * - Fetching projects with anchor contracts * - Multi-network support (OPTIMISTIC, BASE_MAINNET) * - Error cases * * Expected behavior: * - Returns projects with active anchor contracts * - Includes network-specific contract addresses * - Properly handles error cases */
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/provider.ts(1 hunks)src/resolvers/recurringDonationResolver.test.ts(2 hunks)src/resolvers/recurringDonationResolver.ts(8 hunks)src/services/cronJobs/checkUserSuperTokenBalancesQueue.ts(2 hunks)src/services/recurringDonationService.ts(1 hunks)
🔇 Additional comments (4)
src/resolvers/recurringDonationResolver.ts (1)
59-75: Proper definition of RecurringDonationEligibleProject class
The new RecurringDonationEligibleProject class is correctly defined with the necessary fields and appropriate decorators.
src/provider.ts (2)
45-46: Addition of 'DEGENx' mapping in superTokensToToken
The mapping for DEGENx to DEGEN has been correctly added to superTokensToToken.
49-114: New superTokensBase array correctly defined
The superTokensBase array has been properly defined with the necessary token details, including underlying tokens and their properties.
src/services/recurringDonationService.ts (1)
124-129: Verify the necessity of changing networkId from BASE_SEPOLIA to BASE_MAINNET
Ensure that overwriting networkId from BASE_SEPOLIA to BASE_MAINNET is appropriate and does not affect the logic elsewhere in the function or other parts of the code that rely on the original networkId value.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
src/resolvers/recurringDonationResolver.ts (4)
493-497: Consider extracting common filtering logicThe network and token filtering logic is duplicated across multiple queries. Consider extracting this into a reusable helper method to improve maintainability.
Example implementation:
private addNetworkAndTokenFilters( query: SelectQueryBuilder<RecurringDonation>, networkId?: number, filteredTokens?: string[] ): void { if (filteredTokens?.length > 0) { query.andWhere(`recurringDonation.currency IN (:...filteredTokens)`, { filteredTokens, }); } if (networkId) { query.andWhere(`recurringDonation.networkId = :networkId`, { networkId, }); } }This helper method can then be used in all queries:
this.addNetworkAndTokenFilters(query, networkId, filteredTokens);Also applies to: 527-531, 665-669
760-763: Add validation for pagination parametersConsider adding validation for
pageandlimitparameters to ensure they are positive numbers and thatlimithas a reasonable maximum value.@Min(1) @Field(_type => Int, { nullable: true, defaultValue: 1 }) page: number = 1; @Min(1) @Max(100) @Field(_type => Int, { nullable: true, defaultValue: 50 }) limit: number = 50;
797-799: Improve error message specificityThe current error message is too generic. Consider providing more specific error information while still maintaining security.
- throw new Error( - `Error fetching eligible projects for donation: ${error}`, - ); + throw new Error( + 'Failed to fetch eligible projects. Please try again later or contact support if the issue persists.' + );
777-795: Improve SQL query formatting for better readabilityThe SQL query could be better formatted for readability. Also, consider adding a comment explaining the query's purpose.
+ // Fetch GivBack eligible projects with active anchor contracts + // Returns project details along with their associated anchor contract information return await Project.getRepository().query(` SELECT project.id, project.slug, project.title, array_agg(json_build_object( 'address', anchor_contract_address.address, 'networkId', anchor_contract_address."networkId", 'isActive', anchor_contract_address."isActive" )) as "anchorContracts" FROM project INNER JOIN anchor_contract_address ON project.id = anchor_contract_address."projectId" WHERE project."isGivbackEligible" = true AND anchor_contract_address."isActive" = true ${networkFilter} GROUP BY project.id, project.slug, project.title OFFSET $1 LIMIT $2 `, queryParams);🧰 Tools
🪛 eslint
[error] 777-777: Insert
⏎········(prettier/prettier)
[error] 795-795: Replace
·queryParamswith⏎········queryParams,⏎······(prettier/prettier)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/resolvers/recurringDonationResolver.ts(8 hunks)
🧰 Additional context used
🪛 eslint
src/resolvers/recurringDonationResolver.ts
[error] 777-777: Insert ⏎········
(prettier/prettier)
[error] 795-795: Replace ·queryParams with ⏎········queryParams,⏎······
(prettier/prettier)
🔇 Additional comments (2)
src/resolvers/recurringDonationResolver.ts (2)
61-74: LGTM: Well-structured class definition
The RecurringDonationEligibleProject class is properly defined with appropriate TypeGraphQL decorators and clear field definitions.
188-189: LGTM: Clean addition of networkId field
The networkId field is properly added to UserRecurringDonationsArgs with correct typing and decoration.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
CarlosQ96
force-pushed
the
feature_base_support_for_streams
branch
from
364cfbc to
7b62770
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/resolvers/recurringDonationResolver.ts (1)
757-804: Consider adding pagination metadata to the response.The
recurringDonationEligibleProjectsquery implementation looks good with proper SQL injection prevention using parameterized queries. However, consider enhancing the response to include pagination metadata (total count, hasNextPage, etc.) to help clients implement pagination controls effectively.Apply this diff to add pagination metadata:
@ObjectType() +class RecurringDonationEligibleProjectsResponse { + @Field(_type => [RecurringDonationEligibleProject]) + projects: RecurringDonationEligibleProject[]; + + @Field(_type => Int) + totalCount: number; + + @Field(_type => Boolean) + hasNextPage: boolean; +} @Query(_return => [RecurringDonationEligibleProject], { nullable: true }) async recurringDonationEligibleProjects( @Arg('networkId', { nullable: true }) networkId?: number, @Arg('page', _type => Int, { nullable: true, defaultValue: 1 }) page: number = 1, @Arg('limit', _type => Int, { nullable: true, defaultValue: 50 }) limit: number = 50, -): Promise<RecurringDonationEligibleProject[]> { +): Promise<RecurringDonationEligibleProjectsResponse> { try { const offset = (page - 1) * limit; const queryParams = [offset, limit]; let networkFilter = ''; let paramIndex = 3; if (networkId) { networkFilter = `AND anchor_contract_address."networkId" = $${paramIndex}`; queryParams.push(networkId); paramIndex++; } + // Get total count + const [{ count }] = await Project.getRepository().query( + ` + SELECT COUNT(DISTINCT project.id) as count + FROM project + INNER JOIN anchor_contract_address ON project.id = anchor_contract_address."projectId" + WHERE project."isGivbackEligible" = true + AND anchor_contract_address."isActive" = true + ${networkFilter} + `, + networkId ? [networkId] : [], + ); + const projects = await Project.getRepository().query( ` SELECT project.id, project.slug, project.title, array_agg(json_build_object( 'address', anchor_contract_address.address, 'networkId', anchor_contract_address."networkId", 'isActive', anchor_contract_address."isActive" )) as "anchorContracts" FROM project INNER JOIN anchor_contract_address ON project.id = anchor_contract_address."projectId" WHERE project."isGivbackEligible" = true AND anchor_contract_address."isActive" = true ${networkFilter} GROUP BY project.id, project.slug, project.title OFFSET $1 LIMIT $2 `, queryParams, ); + return { + projects, + totalCount: parseInt(count), + hasNextPage: parseInt(count) > offset + limit, + }; } catch (error) { throw new Error( `Error fetching eligible projects for donation: ${error}`, ); } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
migration/data/seedTokens.ts(1 hunks)src/resolvers/recurringDonationResolver.ts(8 hunks)src/services/chains/index.test.ts(1 hunks)
🔇 Additional comments (8)
migration/data/seedTokens.ts (2)
1698-1707: LGTM! Token configuration is well-structured and complete.
The new cbBTC token entry follows the established pattern and includes all required fields with appropriate values. The configuration is properly documented with the Basescan URL reference.
1702-1705: Verify token contract address and CoinGecko ID.
Let's verify the token configuration against external sources.
✅ Verification successful
Token configuration verified successfully
The verification confirms:
- CoinGecko ID "coinbase-wrapped-btc" exists and is valid
- Token address format is correct (0xcbB7C0000aB88B473b1f5aFd9ef808440eed33Bf)
- Token symbol on CoinGecko matches (cbBTC)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify token contract address and CoinGecko ID
# Check if the token exists on CoinGecko
echo "Verifying CoinGecko ID..."
curl -s "https://api.coingecko.com/api/v3/coins/coinbase-wrapped-btc" | jq '.id,.symbol'
# Note: We would typically verify the contract address on Basescan,
# but since we're in a sandbox environment, we'll just check the address format
echo "Verifying address format..."
if [[ "0xcbB7C0000aB88B473b1f5aFd9ef808440eed33Bf" =~ ^0x[a-fA-F0-9]{40}$ ]]; then
echo "Address format is valid"
else
echo "Invalid address format"
fi
Length of output: 374
src/resolvers/recurringDonationResolver.ts (5)
61-74: LGTM! The new class definition is well-structured.
The RecurringDonationEligibleProject class is properly decorated with @ObjectType() and has appropriate field decorators. The class structure aligns well with the PR's objective of supporting Superfluid functionality.
187-189: LGTM! The networkId field addition is appropriate.
The addition of the optional networkId field to UserRecurringDonationsArgs is well-decorated and aligns with the filtering requirements.
434-439: LGTM! The filteredTokens parameter addition is appropriate.
The addition of the filteredTokens parameter with proper defaults and type decorations enhances the filtering capabilities.
493-497: LGTM! The token filtering implementation is correct.
The implementation of token filtering using a parameterized SQL query is secure and efficient.
527-531: LGTM! The network filtering implementation is correct.
The implementation of network filtering using a parameterized SQL query is secure and efficient.
src/services/chains/index.test.ts (1)
600-617: Verify if ZKEVM Cardano test cases should remain commented out.
The test cases for ZKEVM Cardano transactions have been commented out. Please verify if this is intentional and if it impacts the test coverage for the ZKEVM Cardano network support.
Run the following script to check test coverage for ZKEVM Cardano related code:
* add includeUnlisted to FilterProjectQueryInputParams * return proper projects * add recipient address to streams when nonexistent (#1890) * Superfluid Base Support (#1893) * finish project endpoint for superfluid to read * add filters for network and tokens for recurringdonations * fix verification logic and emails for base network streams * Update src/resolvers/recurringDonationResolver.ts Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * comment network test and add cbBTC to seeds --------- Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> --------- Co-authored-by: Cherik <[email protected]> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Related to: #1879. (superfluid endpoints)
Related to: #1883 (Filters and account balance)
Related to: #1878 (verifying donations)
Summary by CodeRabbit
New Features
RecurringDonationEligibleProjectclass for managing donation eligibility.Bug Fixes
Tests
Chores