Add function to calculate estimated user rewards based on fees [PFT-2621]#108
Add function to calculate estimated user rewards based on fees [PFT-2621]#108
Conversation
WalkthroughA new feature was introduced to estimate user rewards based on integrator fees within a specified time frame. This includes a public method in the Changes
Sequence Diagram(s)sequenceDiagram
participant Test
participant Subgraph
participant Helper as getEstimatedRewardsForUser (helper)
participant QueryGen as fetchIntegratorFeesWithTimestampQuery
participant SubgraphAPI
Test->>Subgraph: getEstimatedRewardsForUser(user, rewards, start, end)
Subgraph->>Helper: getEstimatedRewardsForUser(endpoint, user, rewards, start, end)
Helper->>QueryGen: fetchIntegratorFeesWithTimestampQuery(start, end)
Helper->>SubgraphAPI: GraphQL query (with generated query)
SubgraphAPI-->>Helper: Fee data
Helper-->>Subgraph: Calculated user rewards
Subgraph-->>Test: User rewards
Suggested reviewers
Poem
Tip ⚡️ Faster reviews with caching
Enjoy the performance boost—your workflow just got faster. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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: 5
🧹 Nitpick comments (7)
src/subgraph/scripts/subgraphQueries.ts (3)
4-31: Consider pagination for large datasetsThe query limits results to the first 1000 snxAccounts, which might not retrieve all relevant data if there are more than 1000 accounts in the system. This could lead to incomplete calculations of rewards.
Consider adding pagination support:
export const fetchIntegratorFeesWithTimestampQuery = ( startTimeStamp: number, // unix timestamp endTimeStamp: number, //unix timestamp + skip: number = 0 ) => gql`{ snxAccounts( first: 1000 + skip: ${skip} where: { type: PERP, totalOrdersCount_gt: 0 }) { id integratorFeesGenerated accountId owner { id } orders( where: { status: SETTLED, createdTimestamp_gt: ${startTimeStamp}, createdTimestamp_lt: ${endTimeStamp} } ) { id referralFees collectedFees } } } `;
21-22: Consider inclusive timestamp boundariesUsing strictly greater than (
_gt) and strictly less than (_lt) operators might exclude orders created exactly at the start or end timestamps.Consider using inclusive boundaries for either the start or end timestamp if needed:
orders( where: { status: SETTLED, - createdTimestamp_gt: ${startTimeStamp}, - createdTimestamp_lt: ${endTimeStamp} + createdTimestamp_gte: ${startTimeStamp}, + createdTimestamp_lt: ${endTimeStamp} }
8-30: Add sorting for consistent resultsThe query doesn't specify a sorting order, which might lead to inconsistent results across multiple calls when paginating or analyzing the data.
Add an orderBy parameter to ensure consistent results:
gql`{ snxAccounts( first: 1000 - where: { type: PERP, totalOrdersCount_gt: 0 }) { + where: { type: PERP, totalOrdersCount_gt: 0 } + orderBy: id + orderDirection: asc) { id integratorFeesGenerated accountId owner { id } orders( where: { status: SETTLED, createdTimestamp_gt: ${startTimeStamp}, createdTimestamp_lt: ${endTimeStamp} } + orderBy: createdTimestamp + orderDirection: asc ) {src/subgraph/scripts/index.ts (1)
34-40: Streamline number conversionThe code first converts to string and then to Number, which is unnecessary and less efficient.
Simplify the conversion:
account.orders.forEach((order) => { const formattedFees = new Decimal(order.referralFees).div(WAD); if (account.owner.id == userAddress) { - userFeesForPeriod += Number(formattedFees.toString()); + userFeesForPeriod += formattedFees.toNumber(); } - totalFeesForPeriod += Number(formattedFees.toString()); + totalFeesForPeriod += formattedFees.toNumber(); });test/subgraph-tests/index.test.ts (2)
46-47: Remove or limit console.log statements in testsConsole logs in tests can clutter the test output, making it harder to identify issues.
Consider removing the console.log or conditionally enabling it only when debugging:
- console.log('User rewards : ', userRewards); + // Only log when debugging is enabled + if (process.env.DEBUG) { + console.log('User rewards : ', userRewards); + }
47-47: Enhance test assertions for reward calculationsThe test only checks if the reward is greater than zero, which doesn't thoroughly validate the calculation logic.
Add more specific assertions to validate the reward calculation:
- expect(userRewards).toBeGreaterThan(0); + // Basic validation + expect(userRewards).toBeGreaterThan(0); + expect(userRewards).toBeLessThanOrEqual(totalRewards); + + // Store a snapshot of the calculation for regression testing + expect(userRewards).toMatchSnapshot();src/subgraph/index.ts (1)
301-304: Use a more specific section nameThe "MISC" section label is generic and doesn't clearly indicate the feature's purpose.
Rename the section to better reflect its purpose:
- //////////////////////////////////////////////////////////////// - /////////////////////// MISC ///////////////////////////// - //////////////////////////////////////////////////////////////// + //////////////////////////////////////////////////////////////// + ///////////////// USER REWARDS ////////////////////////// + ////////////////////////////////////////////////////////////////
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/subgraph/index.ts(2 hunks)src/subgraph/scripts/index.ts(1 hunks)src/subgraph/scripts/subgraphQueries.ts(1 hunks)test/subgraph-tests/index.test.ts(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
test/subgraph-tests/index.test.ts (3)
src/index.ts (1)
ParifiSdk(15-43)test/index.ts (1)
getParifiSdkInstanceForTesting(5-33)test/common/constants.ts (1)
TEST_USER_1(8-8)
src/subgraph/index.ts (1)
src/subgraph/scripts/index.ts (1)
getEstimatedRewardsForUser(6-45)
🪛 GitHub Actions: Test contracts sdk
test/subgraph-tests/index.test.ts
[error] 26-26: Test failed: GraphQL 403 Forbidden error when fetching any valid query data in src/subgraph/index.ts at line 80.
[error] 40-40: Test failed: GraphQL 403 Forbidden error when fetching user estimated rewards for a period of time in src/subgraph/scripts/index.ts at line 25.
| it('should return user estimated rewards for a period of time', async () => { | ||
| if (!parifiSdk) throw new Error('Parifi SDK not initialized'); | ||
|
|
||
| const totalRewards = 50000; // 50k esPRF | ||
| const userAddress = TEST_USER_1; | ||
|
|
||
| // Default to one week before current time | ||
| const startTimestamp = Math.floor((Date.now() - 7 * 24 * 60 * 60 * 1000) / 1000); | ||
| const endTimestamp = Math.floor(Date.now() / 1000); | ||
|
|
||
| const userRewards = await parifiSdk.subgraph.getEstimatedRewardsForUser( | ||
| userAddress, | ||
| totalRewards, | ||
| startTimestamp, | ||
| endTimestamp, | ||
| ); | ||
| console.log('User rewards : ', userRewards); | ||
| expect(userRewards).toBeGreaterThan(0); | ||
| }); |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Fix the 403 Forbidden error in test
The pipeline failures show a 403 Forbidden error when accessing the subgraph, which means authentication is failing.
The test is failing with GraphQL 403 Forbidden errors. This might be due to:
- Missing authentication tokens in the test environment
- Incorrect subgraph endpoint URL
- IP restrictions on the subgraph endpoint
Verify the authentication setup and ensure environment variables are correctly configured. You might need to update the test to use the correct credentials:
beforeAll(async () => {
parifiSdk = await getParifiSdkInstanceForTesting();
+ // Verify that the subgraph endpoint and authentication are correctly configured
+ console.log(`Using subgraph endpoint: ${process.env.SUBGRAPH_ENDPOINT}`);
});🏁 Script executed:
#!/bin/bash
# Check if SUBGRAPH_ENDPOINT is set in the environment
echo "Checking environment configuration for tests:"
if [ -f ".env" ]; then
echo "Found .env file:"
grep -v "PASSWORD" .env | grep "SUBGRAPH\|RPC"
else
echo "No .env file found"
fi
# Check for authentication-related files or configs
echo "Checking for authentication configurations:"
find . -type f -name "*.json" -size -1024k -exec grep -l "token\|auth\|key\|credential" {} \;Length of output: 412
Ensure SUBGRAPH_ENDPOINT and auth are set for subgraph tests
Tests in test/subgraph-tests/index.test.ts are making real GraphQL calls, but there is no .env (and no SUBGRAPH_ENDPOINT or credentials) in the sandbox. The verification script confirms:
- No
.envfile found - No auth tokens or keys in JSON configs
To fix:
- Add a
.env.test(or.env) containing at minimum:SUBGRAPH_ENDPOINT=https://your-subgraph-endpoint SUBGRAPH_API_KEY=your-api-key # if your subgraph requires a token - Update CI to load those variables (or export them directly in the pipeline).
- In your test setup, explicitly load and validate env vars before using the SDK:
import dotenv from 'dotenv'; dotenv.config({ path: '.env.test' }); // or '.env' beforeAll(async () => { const endpoint = process.env.SUBGRAPH_ENDPOINT; if (!endpoint) { throw new Error('SUBGRAPH_ENDPOINT is not defined'); } parifiSdk = await getParifiSdkInstanceForTesting({ subgraphEndpoint: endpoint, apiKey: process.env.SUBGRAPH_API_KEY, }); });
- (Optional) To avoid network calls in unit tests, consider mocking the subgraph (e.g. with nock or MSW) and asserting on stubbed responses.
🧰 Tools
🪛 GitHub Actions: Test contracts sdk
[error] 40-40: Test failed: GraphQL 403 Forbidden error when fetching user estimated rewards for a period of time in src/subgraph/scripts/index.ts at line 25.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
2 participants
Summary by CodeRabbit