Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(rfq-indexer): add request column to BridgeRequested for refunds #3287

Merged
merged 3 commits into from
Oct 16, 2024

Conversation

golangisfun123
Copy link
Collaborator

@golangisfun123 golangisfun123 commented Oct 14, 2024

Description
A clear and concise description of the features you're adding in this pull request.

Additional context
Add any other context about the problem you're solving.

Metadata

  • Fixes #[Link to Issue]

Summary by CodeRabbit

  • New Features

    • Added a new field, request, to the deposit events, enhancing the information retrieved for bridge requests.
    • Expanded the GraphQL schema to include the request field in the BridgeRequestEvent type.
  • Bug Fixes

    • Ensured type consistency by explicitly converting originChainId and destChainId to numbers in the event data structure.

dwasse and others added 2 commits October 14, 2024 10:18
* Feat: add quoteParams helper for test

* Feat: add MaxQuoteAmount to relconfig

* Feat: use MaxQuoteAmount

* Feat: handle MaxQuoteAmount in quoter test

* Replace: MaxQuoteAmount -> MaxRelayAmount

* Feat: shouldProcess() returns false if max relay amount exceeded

* Feat: add test for MaxRelayAmount
Copy link
Contributor

coderabbitai bot commented Oct 14, 2024

Walkthrough

The changes in this pull request involve the addition of a new field, 'BridgeRequestEvents.request', across multiple files related to the qDeposits function and the BridgeRequestEvents structure. This new field enhances the data retrieved for deposit events in the GraphQL resolvers, database queries, and type definitions. The modifications also include updates to the event handling logic for FastBridgeV2 events, ensuring that the new property is captured during event processing. The overall structure and logic of existing functionalities remain unchanged.

Changes

File Path Change Summary
.../graphql/resolvers.ts Added field 'BridgeRequestEvents.request' in qDeposits function.
.../queries/depositsQueries.ts Added selection field 'BridgeRequestEvents.request' in qDeposits function.
.../types/index.ts Added property request: ColumnType<string> in BridgeRequestEvents interface.
.../ponder.schema.ts Added field request: p.string() in BridgeRequestEvents table schema.
.../index/src/index.ts Added property request in event handler for FastBridgeV2:BridgeRequested.

Possibly related PRs

  • RFQ-Indexer Adding events #3227: This PR introduces the "bridgedispute" event to the indexer, which is directly related to the changes made in the main PR regarding the BridgeRequestEvents and the handling of disputes.
  • RFQ-Indexer DB updates #3239: This PR updates the BridgeProofDisputedEvents table schema and modifies event handling logic, which aligns with the main PR's focus on expanding the BridgeRequestEvents functionality.
  • Paro/rfq indexer updates suggs #3243: This PR enhances functionality related to dispute events, which is directly relevant to the main PR's changes involving the addition of the request field in the context of bridge events.

Suggested labels

size/m, M-docs

Suggested reviewers

  • trajan0x
  • parodime
  • ChiTimesChi
  • bigboydiamonds

🐇 In the fields where bridges sway,
A new request has come to play.
With data rich, our queries grow,
In events of bridges, we now know!
So hop along, let’s celebrate,
For changes made, we elevate! 🎉


📜 Recent review details

Configuration used: .coderabbit.yaml
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between d84403e and a714503.

📒 Files selected for processing (1)
  • packages/rfq-indexer/api/src/graphql/types/events.graphql (1 hunks)
🧰 Additional context used
🔇 Additional comments (1)
packages/rfq-indexer/api/src/graphql/types/events.graphql (1)

20-20: New field request added to BridgeRequestEvent

The addition of the request: String! field to the BridgeRequestEvent type is consistent with the PR objectives to enhance the functionality for refunds. This non-nullable field will ensure that all bridge request events include this information.

Consider the following points:

  1. Ensure that all resolvers and data sources are updated to provide this new field.
  2. Update any existing queries that fetch BridgeRequestEvent to include this new field if necessary.
  3. Consider adding a comment or description to the field to clarify its purpose and expected content.

To ensure consistency across the codebase, let's verify the usage of this new field:


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?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@golangisfun123 golangisfun123 changed the title feat(rfq-indexer): add request column toBridgeRequested for refunds feat(rfq-indexer): add request column to BridgeRequested for refunds Oct 14, 2024
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Outside diff range and nitpick comments (1)
packages/rfq-indexer/indexer/src/index.ts (1)

19-19: LGTM! Consider adding type checking for the request property.

The addition of the request property to the BridgeRequested event handler aligns with the PR objective. The implementation correctly extracts the property from the event args and includes it in the database entry.

To enhance type safety, consider adding a type check or assertion for the request property. For example:

request: request as string, // Assuming request is expected to be a string

This will help catch potential type mismatches early in the development process.

Also applies to: 37-37

📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between 93f9886 and d84403e.

📒 Files selected for processing (5)
  • packages/rfq-indexer/api/src/graphql/resolvers.ts (1 hunks)
  • packages/rfq-indexer/api/src/queries/depositsQueries.ts (1 hunks)
  • packages/rfq-indexer/api/src/types/index.ts (1 hunks)
  • packages/rfq-indexer/indexer/ponder.schema.ts (1 hunks)
  • packages/rfq-indexer/indexer/src/index.ts (2 hunks)
🧰 Additional context used
🔇 Additional comments (5)
packages/rfq-indexer/api/src/queries/depositsQueries.ts (1)

20-20: LGTM! Verify impact on dependent code.

The addition of 'BridgeRequestEvents.request' to the selection is correct and aligns with the PR objectives. This change expands the data retrieved by the query, which is likely the intended behavior for handling refunds.

To ensure this change doesn't cause any issues in dependent code, please run the following script:

This script will help identify any code that might need to be updated to handle the new request field in the query results.

packages/rfq-indexer/indexer/ponder.schema.ts (1)

8-8: LGTM! Verify usage in related code.

The addition of the request field to the BridgeRequestEvents table is implemented correctly and aligns with the PR objective. The field type (string) is appropriate for storing request-related information.

To ensure consistency across the codebase, please verify that all code interacting with BridgeRequestEvents is updated to handle this new field. Run the following script to identify potential areas that may need updates:

This script will help identify areas of the codebase that might need to be updated to accommodate the new request field.

✅ Verification successful

Verification Complete: No Issues Found

The usage of the request field in BridgeRequestEvents has been correctly updated across the codebase. All relevant TypeScript files, including GraphQL resolvers and queries, are handling the new field appropriately.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Find usages of BridgeRequestEvents that might need updating

# Search for BridgeRequestEvents usage in TypeScript/JavaScript files
echo "Searching for BridgeRequestEvents usage:"
rg --type-add 'ts:*.{ts,js}' --type ts 'BridgeRequestEvents' -C 3

# Search for GraphQL queries/mutations that might need updating
echo "\nSearching for GraphQL queries/mutations involving BridgeRequestEvents:"
rg --type-add 'gql:*.{graphql,gql}' --type gql 'BridgeRequestEvents' -C 3

Length of output: 12483

packages/rfq-indexer/api/src/types/index.ts (1)

13-13: LGTM! Verify usage across the codebase.

The addition of the request property to the BridgeRequestEvents interface is appropriate and consistent with the existing structure. The type ColumnType<string> aligns with other string fields in the interface.

To ensure this change is properly integrated, please run the following script to check for any places that might need updating:

This will help identify any areas of the codebase that might need to be updated to accommodate the new request field.

✅ Verification successful

Verified: The request property in BridgeRequestEvents is correctly integrated and consistently used across the codebase. No issues found.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for usage of BridgeRequestEvents that might need updating

# Search for BridgeRequestEvents usage
echo "Searching for BridgeRequestEvents usage:"
rg --type typescript -A 5 "BridgeRequestEvents"

# Search for database operations related to bridge_request_events table
echo "Searching for database operations on bridge_request_events:"
rg --type typescript -A 5 "bridge_request_events"

Length of output: 387


Script:

#!/bin/bash
# Description: Re-check for usage of BridgeRequestEvents that might need updating without specifying file type

# Search for BridgeRequestEvents usage in .ts and .tsx files
echo "Searching for BridgeRequestEvents usage in TypeScript files:"
rg "BridgeRequestEvents" --glob "*.ts" --glob "*.tsx" -A 5

# Search for database operations related to bridge_request_events table in .ts and .tsx files
echo "Searching for database operations on bridge_request_events in TypeScript files:"
rg "bridge_request_events" --glob "*.ts" --glob "*.tsx" -A 5

Length of output: 11580

packages/rfq-indexer/indexer/src/index.ts (1)

Line range hint 1-238: Overall changes look good and are well-integrated.

The addition of the request property to the BridgeRequested event handler is the only change in this file. The implementation is consistent with the existing code style and doesn't introduce any breaking changes to other event handlers.

To ensure the request property is used correctly throughout the codebase, run the following script:

This will help identify any places where the new property might need to be added or where there might be pending work related to this change.

✅ Verification successful

Verification Successful: 'request' property is used correctly

  • References to BridgeRequestEvents.request found in:

    • packages/rfq-indexer/api/src/queries/depositsQueries.ts
    • packages/rfq-indexer/api/src/graphql/resolvers.ts
  • No TODOs or FIXMEs related to the request property were found.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the usage of the new 'request' property in related files.

# Test: Search for references to 'BridgeRequestEvents' and 'request' property
rg --type typescript -e 'BridgeRequestEvents.*request' -e 'FastBridgeV2:BridgeRequested.*request'

# Test: Check for any TODOs or FIXMEs related to the new property
rg --type typescript -e 'TODO.*request' -e 'FIXME.*request'

Length of output: 231


Script:

#!/bin/bash
# Description: Verify the usage of the new 'request' property in related files.

# Test: Search for references to 'BridgeRequestEvents' and 'request' property in TypeScript files
rg -g "*.ts" -g "*.tsx" -e 'BridgeRequestEvents.*request' -e 'FastBridgeV2:BridgeRequested.*request'

# Test: Check for any TODOs or FIXMEs related to the new property in TypeScript files
rg -g "*.ts" -g "*.tsx" -e 'TODO.*request' -e 'FIXME.*request'

Length of output: 347

packages/rfq-indexer/api/src/graphql/resolvers.ts (1)

23-23: New field 'request' added to BridgeRequestEvents query

The addition of the 'BridgeRequestEvents.request' field to the qDeposits function aligns with the PR objective. This change will allow the retrieval of the 'request' data for refunds in bridge requests.

Please ensure the following:

  1. Update any relevant GraphQL type definitions or database schemas to include this new 'request' field.
  2. Verify if the nest_results function needs to be modified to properly handle this new field in the query results.

To help with verification, you can run the following script to check for any type definitions that might need updating:

Copy link

cloudflare-workers-and-pages bot commented Oct 14, 2024

Deploying sanguine-fe with  Cloudflare Pages  Cloudflare Pages

Latest commit: a714503
Status: ✅  Deploy successful!
Preview URL: https://3feae47d.sanguine-fe.pages.dev
Branch Preview URL: https://add-request.sanguine-fe.pages.dev

View logs

Copy link

codecov bot commented Oct 14, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 35.14833%. Comparing base (8724af9) to head (a714503).
Report is 12 commits behind head on feat/explorer-w.

Additional details and impacted files
@@                    Coverage Diff                     @@
##           feat/explorer-w       #3287          +/-   ##
==========================================================
- Coverage         90.44834%   35.14833%   -55.30001%     
==========================================================
  Files                   54          77          +23     
  Lines                 1026        2663        +1637     
  Branches                82          82                  
==========================================================
+ Hits                   928         936           +8     
- Misses                  95        1722        +1627     
- Partials                 3           5           +2     
Flag Coverage Δ
packages 90.44834% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@golangisfun123 golangisfun123 changed the base branch from master to feat/explorer-w October 15, 2024 15:05
Copy link
Collaborator

@Defi-Moses Defi-Moses left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@golangisfun123 golangisfun123 merged commit 6b4c78e into feat/explorer-w Oct 16, 2024
33 of 36 checks passed
@golangisfun123 golangisfun123 deleted the add-request branch October 16, 2024 15:12
Defi-Moses added a commit that referenced this pull request Oct 29, 2024
* explorer UI updated

* rfq-indexer update

* explorer backend update

* [goreleaser] trigger explorer version bump

* rfq indexer with the right contracts

* [goreleaser] adding catch

* response error fixes and wld decimals

* adding address

* feat(rfq-indexer): add `request` column to `BridgeRequested` for refunds (#3287)

* feat(rfq-relayer): add MaxRelayAmount (#3259)

* Feat: add quoteParams helper for test

* Feat: add MaxQuoteAmount to relconfig

* Feat: use MaxQuoteAmount

* Feat: handle MaxQuoteAmount in quoter test

* Replace: MaxQuoteAmount -> MaxRelayAmount

* Feat: shouldProcess() returns false if max relay amount exceeded

* Feat: add test for MaxRelayAmount

* add request field for refunds

* adding to events typing

---------

Co-authored-by: dwasse <wassermandaniel8@gmail.com>
Co-authored-by: defi-moses <jakedinero@protonmail.com>

* fix api docs

* linting fixes

* fixing irrelavent files

---------

Co-authored-by: vro <168573323+golangisfun123@users.noreply.github.com>
Co-authored-by: dwasse <wassermandaniel8@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants