Paro/rfq indexer updates suggs #3243
Conversation
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
WalkthroughThe pull request introduces several enhancements to the RFQ Indexer API, including modifications to the README for improved clarity, the addition of new controllers for handling disputes and pending transactions, and updates to existing queries and types. New environment variables are documented, and the database schema is expanded to accommodate dispute events. The changes also include the integration of GraphQL queries and resolvers for better data retrieval related to disputes, alongside updates to routing for new endpoints. Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
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
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #3243 +/- ##
============================================
Coverage ? 90.43902%
============================================
Files ? 54
Lines ? 1025
Branches ? 82
============================================
Hits ? 927
Misses ? 95
Partials ? 3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
![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 (14)
packages/rfq-indexer/api/src/controllers/disputesController.ts (3)
7-13: Function declaration and query construction look solid.The asynchronous function is correctly typed for Express, and the query is well-structured using a CTE. The use of
qDisputeswithactiveOnly: truealigns with the controller's purpose.Consider adding input validation for any query parameters that might be passed to the controller, even if not used in this implementation. This would improve robustness and prepare for future extensions.
15-17: Query execution and result processing are handled well.The asynchronous query execution and the use of
nest_resultsfor processing are implemented correctly.Consider adding a type annotation for the
disputesvariable to enhance code clarity and maintainability. For example:const disputes: DisputeType[] = nest_results(results)Replace
DisputeTypewith the appropriate type for your dispute objects.
23-26: Error handling is present but could be enhanced.The current implementation catches errors and provides a generic response, which is good for basic error handling.
Consider the following improvements to make error handling more robust:
- Use a logging library (e.g., Winston) instead of
console.errorfor better log management.- Include more context in the error log, such as request parameters.
- Implement different error responses based on error types (e.g., database errors vs. other types of errors).
Example enhancement:
} catch (error) { logger.error('Error fetching active disputes:', { error, requestParams: req.params }); if (error instanceof DatabaseError) { res.status(503).json({ message: 'Database service unavailable' }); } else { res.status(500).json({ message: 'Internal server error' }); } }packages/rfq-indexer/api/src/queries/disputesQueries.ts (1)
4-19: LGTM: Query construction is logically sound.The query is well-constructed, effectively filtering out stale or invalid disputes using a left join. The field selection and aliasing are appropriate.
Consider adding a comment explaining the purpose of the left join for improved readability. For example:
// Left join with BridgeProofProvidedEvents to filter out disputes that have been resolved // by a proof provided after the dispute was raisedpackages/rfq-indexer/api/src/controllers/conflictingProofsController.ts (1)
47-47: LGTM! Consider adding error details to the log.The updated error message accurately reflects the change in the query to only consider active proofs. This provides more precise error logging.
Consider including more details about the error in the log message. You could modify the line as follows:
- console.error('Error fetching active conflicting proofs:', error) + console.error('Error fetching active conflicting proofs:', error.message, '\n', error.stack)This will provide more context for debugging if an error occurs.
packages/rfq-indexer/api/README.md (2)
10-32: Great improvements to the documentation!The addition of Swagger documentation information and the updates to the API call descriptions enhance the README's usefulness. The correction of URL paths in the examples is also a good catch.
To further improve the documentation:
- Consider adding a brief explanation of what Swagger is and its benefits for users who might not be familiar with it.
- For consistency and better syntax highlighting, specify the language for the fenced code blocks in the curl examples.
Here's an example of how to specify the language for the curl examples:
- ``` + ```bash curl http://localhost:3001/api/pending-transactions/missing-relay ```Apply this change to all curl examples in the README.
🧰 Tools
🪛 Markdownlint
17-17: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
24-24: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
31-31: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
38-41: Excellent addition of environment variables!The inclusion of environment variables is crucial for developers. The NODE_ENV description is clear and helpful.
To enhance the documentation further:
- Consider providing more details about the DATABASE_URL, such as the expected format or an example.
Here's a suggestion to improve the DATABASE_URL description:
- - **DATABASE_URL**: PostgreSQL connection URL for the ponder index. + - **DATABASE_URL**: PostgreSQL connection URL for the ponder index. Format: `postgresql://[user[:password]@][netloc][:port][/dbname][?param1=value1&...]` + Example: `postgresql://myuser:mypassword@localhost:5432/mydb`This addition will help developers quickly understand the expected format and provide a clear example.
packages/rfq-indexer/api/src/graphql/types/events.graphql (1)
83-91: LGTM: BridgeProofDisputedEvent addition with suggestionThe addition of the
BridgeProofDisputedEventtype is consistent with the PR objective and follows the structure of other event types. However, consider adding the following fields for consistency with other event types:
relayer: String!to: String!token: String!amount: BigInt!amountFormatted: String!These additional fields might provide more context about the disputed proof, if applicable to the dispute event.
packages/rfq-indexer/api/src/routes/disputesRoute.ts (2)
7-62: OpenAPI documentation is comprehensive, consider adding pagination.The OpenAPI documentation is well-structured and provides clear information about the endpoint's functionality and response schemas. This is excellent for API consumers.
However, consider the following suggestions:
- Add information about query parameters if any are supported (e.g., filtering options).
- Consider implementing and documenting pagination for the disputes list, as it could potentially be a large dataset.
If you decide to implement pagination, you could add something like this to the documentation:
parameters: - in: query name: page schema: type: integer minimum: 1 default: 1 description: The page number for pagination - in: query name: limit schema: type: integer minimum: 1 maximum: 100 default: 20 description: The number of items per pageAnd update the response schema to include pagination metadata:
schema: type: object properties: data: type: array items: $ref: '#/components/schemas/Dispute' pagination: type: object properties: currentPage: type: integer totalPages: type: integer totalItems: type: integer
63-65: Route definition is correct; consider adding error handling.The route definition and export are implemented correctly. The use of a separate controller (disputesController) promotes clean code organization.
Consider adding error handling middleware specific to this route. This can help catch and format errors consistently. For example:
import { ErrorRequestHandler } from 'express'; const errorHandler: ErrorRequestHandler = (err, req, res, next) => { console.error(err.stack); res.status(500).json({ message: 'An unexpected error occurred' }); }; router.get('/', disputesController); router.use(errorHandler); export default router;This ensures that any errors thrown in the disputesController are caught and returned in a consistent format.
packages/rfq-indexer/api/src/graphql/queries/queries.graphql (1)
122-122: LGTM: New query added, but consider adding a deadline parameterThe new
pendingTransactionsMissingRelayExceedDeadlinequery is a good addition and aligns with the PR objectives. It correctly returns an array ofPendingTransactionMissingRelayobjects.However, consider adding an optional deadline parameter to make the query more flexible. This would allow clients to specify different deadlines as needed.
Example:
pendingTransactionsMissingRelayExceedDeadline(deadline: Int): [PendingTransactionMissingRelay!]!This change would make the query more versatile while maintaining backwards compatibility.
packages/rfq-indexer/api/src/routes/pendingTransactionsRoute.ts (1)
150-190: New route added successfully, with minor improvements neededThe new '/exceed-deadline' route has been successfully implemented with comprehensive OpenAPI documentation. Great job on providing detailed response schemas and descriptions.
However, there are a couple of minor points to address:
There's a typo in the 404 response description:
"No pending transactionst that exceed the deadline found" should be "No pending transactions that exceed the deadline found"The indentation of the route definition (lines 187-190) could be improved for consistency with other routes in the file. Consider aligning it with the existing route definitions.
Here's a suggested fix for the indentation:
-router.get( - '/exceed-deadline', - pendingTransactionsMissingRelayExceedDeadlineController -) +router.get('/exceed-deadline', pendingTransactionsMissingRelayExceedDeadlineController)packages/rfq-indexer/api/src/queries/proofsQueries.ts (1)
21-22: Enhancement: Add comments to explain the filtering logicConsider adding a comment above the
if (activeOnly)block to explain that you're filtering out proofs that have been disputed. This will improve code readability and help others understand the purpose of this condition.packages/rfq-indexer/api/src/graphql/resolvers.ts (1)
91-104: Addition ofqDisputesfunction is consistent; consider including additional relevant fieldsThe
qDisputesfunction is correctly defined and follows the established pattern of the other query functions. However, consider including additional fields that might be relevant for dispute events, such asrelayerordisputer, if these columns exist in theBridgeProofDisputedEventstable. Including these fields could provide more context about the disputes and may be useful for downstream processing or client applications.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (16)
- packages/rfq-indexer/api/README.md (1 hunks)
- packages/rfq-indexer/api/src/controllers/conflictingProofsController.ts (2 hunks)
- packages/rfq-indexer/api/src/controllers/disputesController.ts (1 hunks)
- packages/rfq-indexer/api/src/controllers/pendingTransactionsController.ts (4 hunks)
- packages/rfq-indexer/api/src/controllers/transactionIdController.ts (2 hunks)
- packages/rfq-indexer/api/src/db/index.ts (2 hunks)
- packages/rfq-indexer/api/src/graphql/queries/queries.graphql (1 hunks)
- packages/rfq-indexer/api/src/graphql/resolvers.ts (3 hunks)
- packages/rfq-indexer/api/src/graphql/types/events.graphql (5 hunks)
- packages/rfq-indexer/api/src/queries/disputesQueries.ts (1 hunks)
- packages/rfq-indexer/api/src/queries/index.ts (1 hunks)
- packages/rfq-indexer/api/src/queries/proofsQueries.ts (1 hunks)
- packages/rfq-indexer/api/src/routes/disputesRoute.ts (1 hunks)
- packages/rfq-indexer/api/src/routes/index.ts (2 hunks)
- packages/rfq-indexer/api/src/routes/pendingTransactionsRoute.ts (2 hunks)
- packages/rfq-indexer/api/src/types/index.ts (1 hunks)
🧰 Additional context used
🪛 Markdownlint
packages/rfq-indexer/api/README.md
17-17: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
24-24: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
31-31: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
🔇 Additional comments (32)
packages/rfq-indexer/api/src/routes/index.ts (3)
8-8: LGTM: Import statement for disputesRouteThe import statement for
disputesRouteis correctly placed and follows the existing pattern for route imports.
17-17: LGTM: New route for disputesThe new route for disputes is correctly implemented and follows the existing pattern for route definitions. The path '/disputes' is consistent with the naming convention used for other routes.
8-8: Verify implementation of disputesRouteThe import and usage of
disputesRoutelook correct. To ensure completeness, please verify that thedisputesRouteis properly implemented in its own file (./disputesRoute.ts).Run the following script to check the implementation of
disputesRoute:Also applies to: 17-17
✅ Verification successful
disputesRoute is properly implemented and imported
- Verified that
disputesRoute.tsexists inpackages/rfq-indexer/api/src/routes/.- Confirmed that
disputesRouteis correctly imported and used inindex.ts.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the implementation of disputesRoute # Test: Check if disputesRoute.ts exists and contains a router definition rg --type typescript -A 5 'const.*Router\(\)' ./disputesRoute.ts # Test: Check for typical route handler patterns rg --type typescript 'router\.(get|post|put|delete)' ./disputesRoute.tsLength of output: 210
Script:
#!/bin/bash # Description: Verify the implementation of disputesRoute with corrected type # Test: Check if disputesRoute.ts exists and contains a router definition rg --type ts -A 5 'const.*Router\(\)' ./disputesRoute.ts # Test: Check for typical route handler patterns rg --type ts 'router\.(get|post|put|delete)' ./disputesRoute.tsLength of output: 242
Script:
#!/bin/bash # Description: Search for disputesRoute implementation across the codebase # Search for files named disputesRoute.ts fd 'disputesRoute\.ts$' # If not found, search for any files containing 'disputesRoute' rg 'disputesRoute' --type tsLength of output: 280
packages/rfq-indexer/api/src/db/index.ts (1)
10-10: LGTM! New event type added consistently.The addition of
BridgeProofDisputedEventsto both the import statement and theDatabaseinterface is consistent with the existing pattern for other event types. This change appropriately extends the database schema to include disputed events.To ensure the new type is properly defined, please run the following script:
This will help confirm that the
BridgeProofDisputedEventsinterface is correctly defined in the types file.Also applies to: 25-25
✅ Verification successful
Verification Successful:
BridgeProofDisputedEventsDefined Correctly.The
BridgeProofDisputedEventsinterface is properly defined in the types file (packages/rfq-indexer/api/src/types/index.ts), ensuring consistency and correctness in the database schema for disputed events.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the definition of BridgeProofDisputedEvents in the types file # Test: Check if BridgeProofDisputedEvents is defined in the types file rg --type typescript -A 5 'export interface BridgeProofDisputedEvents' ../typesLength of output: 116
Script:
#!/bin/bash # Description: Verify the definition of BridgeProofDisputedEvents in the types file # Search for BridgeProofDisputedEvents interface definition without specifying file type fd --extension ts src/types | xargs rg 'export interface BridgeProofDisputedEvents'Length of output: 174
packages/rfq-indexer/api/src/controllers/disputesController.ts (2)
1-5: Imports look good and are well-organized.The necessary dependencies are imported correctly, including Express types, database connection, queries, and utility functions. This setup provides a solid foundation for the controller's functionality.
18-22: Response handling is appropriate and consistent.The code correctly differentiates between the presence and absence of active disputes, providing appropriate responses in both cases. The use of status 200 for "no active disputes" is correct, as this is not an error condition.
packages/rfq-indexer/api/src/queries/disputesQueries.ts (5)
1-1: LGTM: Import statement is correct.The import of
dbfrom '../db' is appropriate for the database operations performed in this file.
3-3: LGTM: Well-structured function signature.The function signature for
qDisputesis well-designed. It uses an object parameter with a default value, which enhances readability and allows for easy extension of parameters in the future if needed.
21-23: LGTM: Correct implementation of activeOnly filter.The
activeOnlyfilter is correctly implemented. When true, it effectively filters out disputes that have corresponding proofs by checking if 'BridgeProofProvidedEvents.transactionId' is null.
25-26: LGTM: Appropriate return statement.The function correctly returns the constructed query, allowing for further modifications if needed before execution.
1-26: LGTM: Well-implemented dispute query function.The
qDisputesfunction is well-implemented and aligns with the PR objectives. It effectively queries dispute events, filters out stale or invalid disputes, and provides flexibility with theactiveOnlyparameter. The code is clean, readable, and maintainable.To ensure the function is being used correctly throughout the codebase, let's run a quick check:
✅ Verification successful
Verified:
qDisputesfunction is correctly imported and used in the codebase.
packages/rfq-indexer/api/src/graphql/resolvers.tspackages/rfq-indexer/api/src/controllers/transactionIdController.tspackages/rfq-indexer/api/src/controllers/disputesController.ts🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of qDisputes function # Test: Search for imports and usage of qDisputes echo "Checking imports and usage of qDisputes:" rg --type typescript "import.*qDisputes|qDisputes\s*\("Length of output: 183
Script:
#!/bin/bash # Description: Re-verify the usage of qDisputes function without specifying file type echo "Re-checking imports and usage of qDisputes across all files:" rg "import.*qDisputes|qDisputes\s*\("Length of output: 846
packages/rfq-indexer/api/src/controllers/conflictingProofsController.ts (2)
44-44: LGTM! Improved response clarity.The updated message accurately reflects the change in the query to only consider active proofs. This provides clearer feedback to the API consumer.
16-16: LGTM! Verify impact on query results and performance.The change to filter for active proofs aligns with the PR objectives. However, ensure that this modification doesn't negatively impact query performance or unexpectedly alter the results.
To verify the impact, run the following script:
packages/rfq-indexer/api/README.md (2)
Line range hint
44-47: LGTM: Important Scripts sectionThe "Important Scripts" section remains clear and informative, providing developers with the necessary commands to run the API in different modes.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~45-~45: Loose punctuation mark.
Context: ...## Important Scripts -yarn dev:local: Runs the API in development mode using ...(UNLIKELY_OPENING_PUNCTUATION)
🪛 Markdownlint
17-17: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
24-24: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
31-31: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
Line range hint
1-47: Excellent improvements to the README!The changes made to this README file have significantly enhanced its clarity, organization, and usefulness. Key improvements include:
- Addition of Swagger documentation information.
- Updated and corrected API call descriptions and examples.
- New section on environment variables.
- Consistent formatting throughout the document.
These updates align well with the PR objectives of updating documentation and comments. The README now provides a more comprehensive guide for developers working with the RFQ Indexer API.
Great job on improving the documentation!
🧰 Tools
🪛 LanguageTool
[uncategorized] ~45-~45: Loose punctuation mark.
Context: ...## Important Scripts -yarn dev:local: Runs the API in development mode using ...(UNLIKELY_OPENING_PUNCTUATION)
🪛 Markdownlint
17-17: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
24-24: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
31-31: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
packages/rfq-indexer/api/src/graphql/types/events.graphql (2)
Line range hint
1-1: LGTM: BigInt scalar type additionThe addition of the
BigIntscalar type is appropriate for handling large integer values, particularly forblockNumberfields across various event types.
Line range hint
1-91: Overall changes align with PR objectivesThe additions to this file, including the
BigIntscalar type and theBridgeProofDisputedEventtype, are consistent with the PR objective of adding dispute events. The file maintains a clear and organized structure, with the new event type following the established pattern of other event types.packages/rfq-indexer/api/src/routes/disputesRoute.ts (1)
1-5: LGTM: Imports and router setup look good.The imports and router initialization are correctly implemented. The separation of the controller logic into a separate file (disputesController) is a good practice for maintaining clean and modular code.
packages/rfq-indexer/api/src/graphql/queries/queries.graphql (2)
115-118: LGTM: NewDisputedRelaytype is well-structuredThe new
DisputedRelaytype is correctly defined and aligns well with the PR objective of adding dispute events. It appropriately uses existing types and ensures that all necessary information (transaction and proof) is included for each disputed relay.
129-129: LGTM: NewdisputedRelaysquery addedThe new
disputedRelaysquery is a great addition that complements the newly addedDisputedRelaytype. It correctly returns an array ofDisputedRelayobjects and uses appropriate non-nullable types. This query aligns well with the PR objectives of adding dispute events to endpoints.packages/rfq-indexer/api/src/types/index.ts (2)
84-92: LGTM: New interface for dispute events addedThe new
BridgeProofDisputedEventsinterface is well-structured and consistent with other event interfaces in the file. It includes all necessary properties for tracking dispute events, which aligns with the PR objective of adding dispute events to endpoints.
100-100: LGTM: EventType updated to include 'DISPUTE'The addition of 'DISPUTE' to the
EventTypetype is consistent with the newBridgeProofDisputedEventsinterface and allows for proper categorization of dispute events in the system.packages/rfq-indexer/api/src/routes/pendingTransactionsRoute.ts (2)
6-7: LGTM: Import statement updated correctlyThe import statement has been properly updated to include the new
pendingTransactionsMissingRelayExceedDeadlineController. This addition is consistent with the new route being added and follows the existing naming convention for controllers in this file.
Line range hint
1-191: Summary: New route successfully implemented, aligning with PR objectivesThe changes in this file successfully implement a new route for retrieving transactions that have exceeded a deadline. This addition aligns well with the PR objectives of enhancing dispute-related functionality.
The implementation is solid, with only minor suggestions for improvement (a typo fix and indentation adjustment). Overall, these changes contribute positively to the RFQ Indexer API's capabilities for handling pending transactions and potential disputes.
packages/rfq-indexer/api/src/queries/proofsQueries.ts (1)
9-10: Verify the logical operator in the join conditionThe join condition uses a greater-than operator (
'>') to compareblockTimestampfields:.onRef('BridgeProofDisputedEvents.blockTimestamp', '>', 'BridgeProofProvidedEvents.blockTimestamp')Ensure that this operator accurately reflects the intended logic for your application. If the goal is to join disputed events that occurred after the proof was provided, then this is correct. Otherwise, consider whether a different operator (e.g.,
'<'or an equality check) might be more appropriate.packages/rfq-indexer/api/src/controllers/transactionIdController.ts (4)
4-4: Import 'qDisputes' to access dispute queries.Adding
qDisputesto the import statement ensures that the controller has access to dispute-related queries.
16-17: Verify the 'activeOnly' parameters for proofs and disputes.Setting
activeOnly: falsefor proofs includes all proofs, even those invalidated or replaced by a dispute. Conversely,activeOnly: truefor disputes includes only active disputes, excluding those invalidated or replaced by a proof. Please confirm that this behavior aligns with the intended functionality.
25-25: Include disputes in the combined query with correct join conditions.Adding a left join with
'disputes'on'transactionId_deposit'and'transactionId_dispute'integrates dispute data into the transaction results, ensuring comprehensive transaction details.
31-31: Select all fields from 'disputes' to enrich response data.Including all fields from
'disputes'in the final selection ensures that the response contains complete dispute information related to the transaction.packages/rfq-indexer/api/src/controllers/pendingTransactionsController.ts (1)
24-24: Verify that theqProofsfunction accepts theactiveOnlyparameterThe code calls
qProofs({ activeOnly: true }). Please ensure that theqProofsfunction is defined to accept theactiveOnlyparameter and handles it correctly.Run the following script to confirm that
qProofssupports theactiveOnlyparameter:Also applies to: 65-65
packages/rfq-indexer/api/src/graphql/resolvers.ts (2)
237-249: IncludingBridgeProofDisputedEventsineventsresolverThe addition of the
unionAllblock forBridgeProofDisputedEventscorrectly incorporates dispute events into theeventsresolver. This ensures that dispute events are now retrievable through theeventsquery.
496-518: Verify thatdisputedRelaysresolver correctly associates proofs with disputesThe
disputedRelaysresolver fetches proofs and their associated disputes by joining ontransactionId_proofandtransactionId_dispute. Please verify that this join accurately reflects the relationship between proofs and disputes in your data model. If a dispute could be associated with a different identifier, or if multiple disputes can relate to a single proof, consider adjusting the join conditions accordingly.
| @@ -1,5 +1,6 @@ | |||
| export { qClaims } from './claimsQueries' | |||
| export { qDeposits } from './depositsQueries' | |||
| export { qDisputes } from './disputesQueries' | |||
There was a problem hiding this comment.
💡 Codebase verification
qDisputes does not use the expected GraphQL query structure.
The qDisputes export in disputesQueries.ts is correctly defined and exported. However, it does not utilize the gql template literal typically associated with GraphQL queries. If qDisputes is intended to be a GraphQL query, please update its implementation to use the gql syntax. Otherwise, ensure that its current database query structure aligns with the project's architecture.
🔗 Analysis chain
LGTM! The new export for dispute queries is well-integrated.
The addition of the qDisputes export from the disputesQueries module is consistent with the existing pattern and maintains alphabetical order. This change aligns well with the PR objective of adding dispute events to endpoints.
To ensure the completeness of this change, please verify the following:
This script will help ensure that the disputesQueries module exists and has the expected basic structure.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the existence and basic structure of the disputesQueries module
# Test 1: Check if the disputesQueries.ts file exists
if [ -f "packages/rfq-indexer/api/src/queries/disputesQueries.ts" ]; then
echo "disputesQueries.ts file exists."
else
echo "Error: disputesQueries.ts file not found."
exit 1
fi
# Test 2: Check if qDisputes is exported from disputesQueries.ts
if grep -q "export const qDisputes" "packages/rfq-indexer/api/src/queries/disputesQueries.ts"; then
echo "qDisputes is exported from disputesQueries.ts"
else
echo "Warning: qDisputes export not found in disputesQueries.ts"
fi
# Test 3: Check for basic query structure (assuming it's a GraphQL query)
if grep -q "const qDisputes = gql\`" "packages/rfq-indexer/api/src/queries/disputesQueries.ts"; then
echo "qDisputes appears to have a GraphQL query structure"
else
echo "Warning: qDisputes may not have the expected GraphQL query structure"
fi
Length of output: 571
Script:
#!/bin/bash
# Description: Retrieve the definition of qDisputes from disputesQueries.ts
# Use ast-grep to find the definition of qDisputes
ast-grep --lang typescript --pattern 'const qDisputes = $_' 'packages/rfq-indexer/api/src/queries/disputesQueries.ts'
Length of output: 2657
| export interface BridgeProofDisputedEvents { | ||
| id: ColumnType<string> | ||
| transactionId: ColumnType<string> | ||
| blockNumber: ColumnType<bigint> | ||
| blockTimestamp: ColumnType<number> | ||
| transactionHash: ColumnType<string> | ||
| chainId: ColumnType<number> | ||
| chain: ColumnType<string> | ||
| } | ||
| // Add any other shared types used across the API | ||
| export type EventType = | ||
| | 'REQUEST' | ||
| | 'RELAYED' | ||
| | 'PROOF_PROVIDED' | ||
| | 'DEPOSIT_REFUNDED' | ||
| | 'DEPOSIT_CLAIMED' | ||
|
|
||
| | 'DISPUTE' |
There was a problem hiding this comment.
💡 Codebase verification
Dispute events not fully integrated into services
While the BridgeProofDisputedEvents interface and the 'DISPUTE' event type have been added, there is no evidence of dispute event handling in the services layer. Please ensure that the service components are updated to process and handle dispute events appropriately.
🔗 Analysis chain
Verify complete integration of dispute events
The addition of the BridgeProofDisputedEvents interface and the 'DISPUTE' event type are good first steps in supporting dispute events. To ensure full integration, please verify that other parts of the codebase (e.g., controllers, services, database schema) have been updated to handle these new dispute events.
Regenerate dispute event handling checks without using --type
The previous script failed to recognize the typescript file type. Please run the updated scripts below to search for dispute-related code in .ts and .tsx files within the controllers, services, and database directories.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for dispute event handling in other parts of the codebase
# Test: Search for dispute-related code in controllers
echo "Checking controllers for dispute handling:"
rg --type typescript -i "dispute" packages/rfq-indexer/api/src/controllers
# Test: Search for dispute-related code in services
echo "Checking services for dispute handling:"
rg --type typescript -i "dispute" packages/rfq-indexer/api/src/services
# Test: Check for database schema updates related to disputes
echo "Checking database schema for dispute-related changes:"
rg --type typescript -i "dispute" packages/rfq-indexer/api/src/db
Length of output: 618
Script:
#!/bin/bash
# Description: Check for dispute event handling in other parts of the codebase without using --type
# Test: Search for dispute-related code in controllers
echo "Checking controllers for dispute handling:"
rg -i "dispute" packages/rfq-indexer/api/src/controllers --glob "*.ts" --glob "*.tsx"
# Test: Search for dispute-related code in services
echo "Checking services for dispute handling:"
rg -i "dispute" packages/rfq-indexer/api/src/services --glob "*.ts" --glob "*.tsx"
# Test: Check for database schema updates related to disputes
echo "Checking database schema for dispute-related changes:"
rg -i "dispute" packages/rfq-indexer/api/src/db --glob "*.ts" --glob "*.tsx"
Length of output: 2807
| @@ -1,15 +1,25 @@ | |||
| import { db } from '../db' | |||
|
|
|||
| // typical fields to return for a BridgeProofProvided event when it is joined to a BridgeRequest | |||
| export const qProofs = () => { | |||
| return db | |||
| export const qProofs = ({ activeOnly}: { activeOnly: boolean} = { activeOnly: false}) => { | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Suggestion: Simplify function signature for better readability
The function signature can be simplified by making activeOnly optional with a default value directly in the parameter destructuring. This improves readability and follows common TypeScript conventions.
Apply this diff to simplify the function signature:
-export const qProofs = ({ activeOnly}: { activeOnly: boolean} = { activeOnly: false}) => {
+export const qProofs = ({ activeOnly = false }: { activeOnly?: boolean } = {}) => {📝 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.
| export const qProofs = ({ activeOnly}: { activeOnly: boolean} = { activeOnly: false}) => { | |
| export const qProofs = ({ activeOnly = false }: { activeOnly?: boolean } = {}) => { |
| console.error('Error fetching pending transactions missing relay:', error) | ||
| res.status(500).json({ message: 'Internal server error' }) | ||
| } |
There was a problem hiding this comment.
Update error messages to reflect the correct controller context
In the pendingTransactionsMissingRelayExceedDeadlineController, both the console error message and the 404 response message are the same as those in pendingTransactionsMissingRelayController. This could cause confusion during debugging or when clients receive responses.
Update the error messages to accurately reflect the context of the new controller.
Apply this diff to correct the error messages:
// In the catch block of pendingTransactionsMissingRelayExceedDeadlineController
- console.error('Error fetching pending transactions missing relay:', error)
+ console.error('Error fetching pending transactions exceeding relay deadline:', error)
// In the 404 response of pendingTransactionsMissingRelayExceedDeadlineController
- res
- .status(404)
- .json({ message: 'No pending transactions missing relay found' })
+ res
+ .status(404)
+ .json({ message: 'No pending transactions missing relay exceeding deadline found' })Also applies to: 165-167
| .where('blockTimestamp_deposit', '>', sevenDaysAgo) | ||
|
|
||
| const results = await query.execute() | ||
| const nestedResults = nest_results(results) | ||
|
|
||
| if (nestedResults && nestedResults.length > 0) { | ||
| res.json(nestedResults) | ||
| } else { | ||
| res | ||
| .status(404) | ||
| .json({ message: 'No pending transactions missing relay found' }) | ||
| } | ||
| } catch (error) { | ||
| console.error('Error fetching pending transactions missing relay:', error) | ||
| res.status(500).json({ message: 'Internal server error' }) | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Refactor controllers to eliminate code duplication
The controllers pendingTransactionsMissingRelayController and pendingTransactionsMissingRelayExceedDeadlineController share similar logic, differing only in the time filter for blockTimestamp_deposit. Consider refactoring to reduce duplication by creating a reusable function or helper that accepts the time condition as a parameter.
Here's how you might refactor the code:
- Extract a helper function:
const getPendingTransactionsMissingRelay = async (
timeOperator: '>' | '<=',
timestamp: number
) => {
return await db
.with('deposits', () => qDeposits())
.with('relays', () => qRelays())
.with('refunds', () => qRefunds())
.with(
'combined',
(qb) =>
qb
.selectFrom('deposits')
.selectAll('deposits')
.leftJoin('relays', 'transactionId_deposit', 'transactionId_relay')
.leftJoin('refunds', 'transactionId_deposit', 'transactionId_refund')
.where('transactionId_relay', 'is', null) // not relayed
.where('transactionId_refund', 'is', null) // not refunded
)
.selectFrom('combined')
.selectAll()
.orderBy('blockTimestamp_deposit', 'desc')
.where('blockTimestamp_deposit', timeOperator, timestamp)
.execute()
}- Update the controllers to use the helper function:
export const pendingTransactionsMissingRelayController = async (
req: Request,
res: Response
) => {
try {
const results = await getPendingTransactionsMissingRelay('>', sevenDaysAgo)
const nestedResults = nest_results(results)
if (nestedResults && nestedResults.length > 0) {
res.json(nestedResults)
} else {
res
.status(404)
.json({ message: 'No pending transactions missing relay found' })
}
} catch (error) {
console.error('Error fetching pending transactions missing relay:', error)
res.status(500).json({ message: 'Internal server error' })
}
}
export const pendingTransactionsMissingRelayExceedDeadlineController = async (
req: Request,
res: Response
) => {
try {
const results = await getPendingTransactionsMissingRelay('<=', sevenDaysAgo)
const nestedResults = nest_results(results)
if (nestedResults && nestedResults.length > 0) {
res.json(nestedResults)
} else {
res
.status(404)
.json({
message:
'No pending transactions missing relay exceeding deadline found',
})
}
} catch (error) {
console.error(
'Error fetching pending transactions exceeding relay deadline:',
error
)
res.status(500).json({ message: 'Internal server error' })
}
}This refactoring enhances maintainability by centralizing shared logic.
Also applies to: 140-167
adding dispute events to endpoints - updating some docs/commments
Summary by CodeRabbit
Release Notes
New Features
Improvements
Bug Fixes
Types and Schema Updates