feat: migrate to latest ponder #14
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughThe pull request includes updates to several files, primarily focusing on enhancing the schema management and database interaction methods. Key changes involve upgrading dependencies in Changes
Possibly related PRs
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🪧 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: 5
🧹 Outside diff range and nitpick comments (5)
ponder.schema.ts (2)
75-75: Specify default value forLMSeasonsarrayThe
LMSeasonsfield in theuserHistoryTableis defined ast.bigint().array().notNull(). To prevent potential issues with null references, consider specifying a default value, such as an empty array.Apply this diff to set a default empty array:
-export const userHistoryTable = onchainTable("UserHistory", (t) => ({ +export const userHistoryTable = onchainTable("UserHistory", (t) => ({ // ... - LMSeasons: t.bigint().array().notNull(), // If the array is empty, the user has not participated in any season + LMSeasons: t.bigint().array().notNull().default([]), // Default to empty array if not provided // ...
Duplicate enum values detected in
pointsSourceEnumThe verification revealed two issues:
- Several enum values appear twice in the schema, including
stratosphere_enrollment,dex_aggregator_swap, and various other entries- The
generateLMSeasonEnumfunction generates unique values with patternliquid_mining_first_wallet_season_${i}which are different from the duplicated values foundThe duplicates appear to be in the schema file itself, not from the generated values. Here are the duplicated entries:
vape_staking_first_walletvape_staking_first_depositstratosphere_enrollmentliquid_mining_twelve_seasonsliquid_mining_three_seasonsliquid_mining_six_seasonsliquid_mining_one_seasonliquid_mining_first_depositdex_aggregator_swapdex_aggregator_1k_swapsdex_aggregator_10k_swapsdex_aggregator_100k_swaps🔗 Analysis chain
Line range hint
39-55: Verify generated enum values forpointsSourceEnumThe
pointsSourceEnumincludes values generated bygenerateLMSeasonEnum(120). Ensure that the generated enum values are unique and correctly formatted, and that there are no duplicates or unintended values.Run the following script to list all enum values and check for duplicates:
Let me verify the implementation of
generateLMSeasonEnumto understand if it might be contributing to these duplicates.
Let me try a different approach to find the implementation of
generateLMSeasonEnum.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that `pointsSourceEnum` contains unique values # Extract enum values from the schema file grep -Po '(?<=")[^"]+(?=")' ponder.schema.ts | sort | uniq -c | sort -nr | awk '$1 > 1' # Expected: No output, indicating no duplicate enum valuesLength of output: 514
Script:
#!/bin/bash # Search for the implementation of generateLMSeasonEnum ast-grep --pattern 'function generateLMSeasonEnum' -A 20Length of output: 58
Script:
#!/bin/bash # Search for the implementation using ripgrep rg -A 10 "const generateLMSeasonEnum" # Also check if it might be defined as a regular function rg -A 10 "function generateLMSeasonEnum" # Look for any file that might contain this function fd generateLMSeasonEnumLength of output: 697
src/index.ts (1)
Line range hint
308-352: Ensure type consistency when comparingBigIntvaluesWhen comparing
usdValueOfTrade >= MINIMUM_POINTS, confirm that both variables are of the same typeBigIntto avoid unexpected behavior.Additionally, verify that arithmetic operations involving
BigIntare handled correctly throughout the code.src/helpers.ts (2)
Line range hint
117-154: Consider transaction safety and initialization improvements
- The check-then-insert pattern could lead to race conditions.
- The
LMSeasonsarray is initialized empty but its type isn't explicitly defined.Consider these improvements:
export async function getOrCreateUserData( context: Context, tokenId: bigint, address: string ): Promise<UserHistory> { const { db } = context; const { chainId } = context.network; + return await db.transaction(async (tx) => { + let userHistory = await tx.find(userHistoryTable, { + id: `${address}-${chainId}`, + }); + + const mainWallet = await getTokenIdOwner(tokenId, context); + + if (!userHistory) { + const LMSeasons: number[] = []; // Explicit type + userHistory = await tx.insert(userHistoryTable)... + await tx.insert(userDataTable)... + } + + return userHistory; + }); }
Line range hint
337-393: Improve transaction safety in token data updatesThe check-then-update pattern could lead to race conditions when multiple transactions attempt to update the same token data simultaneously.
Consider wrapping the operations in a transaction:
return await db.transaction(async (tx) => { let tokenIdData = await tx.find(tokenIdDataTable, ...); let tokenIdDataWeekly = await tx.find(tokenIdDataWeeklyTable, ...); if (!tokenIdData) { tokenIdData = await tx.insert(tokenIdDataTable)...; } if (!tokenIdDataWeekly) { tokenIdDataWeekly = await tx.insert(tokenIdDataWeeklyTable)...; } tokenIdData = await tx.update(tokenIdDataTable)...; tokenIdDataWeekly = await tx.update(tokenIdDataWeeklyTable)...; return tokenIdData; });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (6)
package.json(1 hunks)ponder-env.d.ts(2 hunks)ponder.schema.ts(3 hunks)src/helpers.ts(7 hunks)src/index.ts(6 hunks)src/types.ts(0 hunks)
💤 Files with no reviewable changes (1)
- src/types.ts
🔇 Additional comments (6)
ponder.schema.ts (1)
121-122: Consistent data types for chainId field
Similar to the previous comment, the chainId in tokenIdDataTable is defined as t.integer().notNull(). Ensure that all instances of chainId across different tables use the same data type for consistency.
src/index.ts (1)
256-264: Review logic for vapeStakingData retrieval and insertion
Using a fixed id when retrieving and inserting vapeStakingData may not reflect the data accurately for multiple users. Ensure that the logic aligns with the intended behavior of tracking individual user deposits.
package.json (2)
34-34: Verify Node.js version compatibility
The required Node.js version has been updated to >=20. Ensure that all deployment environments and developers' local setups are compatible with Node.js 20. Some hosting services may not yet support this version.
16-31: Check for breaking changes in updated dependencies
Several dependencies have been updated:
@ponder/corefrom0.4.13to0.8.0-next.1axiosfrom1.6.7to1.7.7viemfrom1.19.3to2.21.3typescriptfrom^5.2.2to^5.3.2
These updates may introduce breaking changes. Please review the release notes for each package and test the application thoroughly to ensure compatibility.
ponder-env.d.ts (1)
10-10: Confirm removal of Schema type does not affect other modules
The Schema type export has been removed. Verify that this type is not used elsewhere in the codebase, or update any imports accordingly to prevent type errors.
src/helpers.ts (1)
20-30: LGTM: Clean table imports for Ponder migration
The table imports are well-organized and follow a consistent naming convention, aligning with the migration to the latest Ponder version.
|
@mejiasd3v Yes for consistency |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (3)
ponder.schema.ts (3)
57-65: Consider adding foreign key constraints and indexes.While the table structure is correct, consider these performance and data integrity improvements:
- Add an index on
userHistoryIdanduserDataIdfor faster lookups- Add an index on
timestampfor time-based queries- Consider adding foreign key constraints to ensure referential integrity
71-90: Consider using a composite primary key for the UserHistory table.Since the ID is constructed from wallet address and chainId, consider using them as separate fields in a composite primary key for better querying capabilities.
export const userHistoryTable = onchainTable("UserHistory", (t) => ({ - id: t.text().primaryKey().notNull(), - chainId: t.integer().notNull(), + walletAddress: t.text().notNull(), + chainId: t.integer().notNull(), + id: t.primaryKey(["walletAddress", "chainId"]), // ... rest of the fields }));
142-147: Add validation for wallets array in WalletsPerTier table.Consider adding validation to ensure the wallets array contains unique addresses to prevent duplicates.
export const walletsPerTierTable = onchainTable("WalletsPerTier", (t) => ({ id: t.text().primaryKey().notNull(), - wallets: t.text().array().notNull(), + wallets: t.text().array().unique().notNull(), }));
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (4)
.gitignore(1 hunks)ponder-env.d.ts(0 hunks)ponder.schema.ts(3 hunks)src/helpers.ts(7 hunks)
💤 Files with no reviewable changes (1)
- ponder-env.d.ts
✅ Files skipped from review due to trivial changes (1)
- .gitignore
🔇 Additional comments (6)
ponder.schema.ts (2)
Line range hint 39-55: LGTM! Well-structured enum definition with comprehensive documentation.
The pointsSourceEnum is well-documented and includes all necessary point source types. The use of generateLMSeasonEnum for dynamic enum values is a good approach.
93-98: Standardize chainId data type across tables.
The chainId field in userDataTable is defined as integer(), which is consistent with most other tables. This is good for maintaining data type consistency across the schema.
src/helpers.ts (4)
20-30: LGTM! Well-organized imports from schema.
The imports are properly structured and aligned with the schema definitions.
79-100:
Address potential race conditions in handleChainFirstWallet.
The function should use a transaction to handle the race condition when checking and updating the first wallet status.
export const handleChainFirstWallet = async (
context: Context,
chainId: number,
userAddressLowerCase: string,
userData: UserHistory,
event: { block?: { timestamp: bigint } }
): Promise<UserHistory> => {
let { db } = context;
- let allProtocols = await db.find(allProtocolsTable, { id: "protocols" });
- if (!allProtocols) {
- allProtocols = await db.insert(allProtocolsTable).values({
- id: "protocols",
- firstWallet: userAddressLowerCase,
- });
- await db.insert(pointsTable).values({
- // ... points table values
- });
- userData = await db
- .update(userHistoryTable, { id: userData.id })
- .set((row) => ({
- chainFirstWallet: true,
- }));
- }
+ return await db.transaction(async (tx) => {
+ let allProtocols = await tx.find(allProtocolsTable, { id: "protocols" });
+ if (!allProtocols) {
+ allProtocols = await tx.insert(allProtocolsTable).values({
+ id: "protocols",
+ firstWallet: userAddressLowerCase,
+ });
+ await tx.insert(pointsTable).values({
+ // ... points table values
+ });
+ userData = await tx
+ .update(userHistoryTable, { id: userData.id })
+ .set((row) => ({
+ chainFirstWallet: true,
+ }));
+ }
+ return userData;
+ });
};419-423:
Fix array mutation in WalletsPerTier update.
The current implementation mutates the array before updating, which is unnecessary and could lead to inconsistencies.
- walletsPerTier.wallets.push(userAddress);
- walletsPerTier = await db
- .update(walletsPerTierTable, { id })
- .set((row) => ({
- wallets: row.wallets.concat(userAddress),
- }));
+ walletsPerTier = await db
+ .update(walletsPerTierTable, { id })
+ .set((row) => ({
+ wallets: [...new Set([...row.wallets, userAddress])],
+ }));377-384:
Use atomic updates for TokenIdData.
The current implementation might lead to race conditions when multiple updates occur simultaneously. Consider using atomic updates.
- tokenIdData = await db
- .update(tokenIdDataTable, { id: `${tokenId}-${chainId}` })
- .set((row) => ({
- pointsEarned: row.pointsEarned + pointsEarned,
- pointsClaimed: row.pointsClaimed + pointsClaimed,
- pointsSpent: row.pointsSpent + pointsSpent,
- lastUpdated: timestamp,
- }));
+ tokenIdData = await db.transaction(async (tx) => {
+ const current = await tx.find(tokenIdDataTable, {
+ id: `${tokenId}-${chainId}`
+ });
+ return await tx
+ .update(tokenIdDataTable, { id: `${tokenId}-${chainId}` })
+ .set({
+ pointsEarned: current.pointsEarned + pointsEarned,
+ pointsClaimed: current.pointsClaimed + pointsClaimed,
+ pointsSpent: current.pointsSpent + pointsSpent,
+ lastUpdated: timestamp,
+ });
+ });Likely invalid or redundant comment.
Summary by CodeRabbit
New Features
Bug Fixes
Chores
ponder-env.d.tsfile.ponder-env.d.tsto the.gitignorefile.maxBlockRangeproperty from network configurations of several contracts.