-
Notifications
You must be signed in to change notification settings - Fork 4
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: compatibility with rxtx #48
Conversation
# Conflicts: # crates/types/src/subgraph.rs
WalkthroughThis pull request updates multiple dependency declarations and import statements across various Cargo.toml and Rust source files. Versions for key dependencies are bumped, new dependencies are added, and some are renamed or removed. Additionally, a GraphQL schema initialization now enables introspection, the HTTP playground endpoint is removed, and several subgraph-related types have been moved from one module to another. A local subgraph module is commented out or entirely removed from the codebase. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Schema
Client->>Schema: Call new_dynamic_schema(subgraph_index)
Schema->>Schema: Execute new_with_info(subgraph_index)
Schema->>Schema: Call enable_introspection()
Schema-->>Client: Return initialized GqlDynamicSchema
Possibly related PRs
Poem
Tip ⚡🧪 Multi-step agentic review comment chat (experimental)
✨ Finishing Touches
🪧 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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🔭 Outside diff range comments (2)
crates/cli/src/http/mod.rs (2)
76-77
:⚠️ Potential issue"/playground" endpoint removed but still referenced elsewhere.
The
/playground
route has been removed from the HTTP server configuration, but on line 242 there's still code sending a playground URL to users. This will lead to a broken experience as users will be directed to a non-existent endpoint.- let _ = sender.send("http://127.0.0.1:8900/playground".into()); + let _ = sender.send("http://127.0.0.1:8900/graphiql".into());
242-242
:⚠️ Potential issueFix outdated playground URL reference.
This line is sending a URL for the "/playground" endpoint, but that endpoint has been removed from the server setup. Users will be directed to a non-existent endpoint.
- let _ = sender.send("http://127.0.0.1:8900/playground".into()); + let _ = sender.send("http://127.0.0.1:8900/graphiql".into());
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
Cargo.lock
is excluded by!**/*.lock
📒 Files selected for processing (16)
Cargo.toml
(2 hunks)crates/cli/src/http/mod.rs
(1 hunks)crates/core/Cargo.toml
(1 hunks)crates/core/src/lib.rs
(1 hunks)crates/core/src/rpc/admin.rs
(1 hunks)crates/gql/Cargo.toml
(1 hunks)crates/gql/src/lib.rs
(2 hunks)crates/gql/src/subscription.rs
(1 hunks)crates/gql/src/types/mod.rs
(1 hunks)crates/gql/src/types/schema.rs
(1 hunks)crates/subgraph/Cargo.toml
(1 hunks)crates/subgraph/src/lib.rs
(1 hunks)crates/types/Cargo.toml
(1 hunks)crates/types/src/lib.rs
(1 hunks)crates/types/src/subgraph.rs
(0 hunks)crates/types/src/types.rs
(1 hunks)
💤 Files with no reviewable changes (1)
- crates/types/src/subgraph.rs
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: run_cargo_checks
- GitHub Check: build
🔇 Additional comments (18)
crates/core/Cargo.toml (1)
74-74
: Added dependency to support rxtx compatibility.The addition of
txtx-addon-network-svm-types
as a workspace dependency aligns with the PR objective of ensuring compatibility with rxtx. This appears to be a supporting type library for the SVM network addon.crates/types/src/lib.rs (1)
4-4
:❓ Verification inconclusive
Commented out the subgraph module.
The subgraph module has been commented out, likely as part of the move to rxtx compatibility. This change appears intentional, but it's important to verify that all code previously depending on this module has been updated accordingly.
🏁 Script executed:
#!/bin/bash # Check for any imports of the subgraph module that might now be broken rg -l "use (surfpool_types::)?subgraph::" --type rustLength of output: 55
Verify Subgraph Dependencies
The change in
crates/types/src/lib.rs
—commenting out the subgraph module—appears intentional. The automated search for lines matchinguse (surfpool_types::)?subgraph::
produced no output, suggesting there are no direct imports in the codebase. However, the lack of output is low-confidence evidence. Please perform a manual check to ensure that any dependencies on the subgraph module (possibly indirect or in less obvious patterns) have been correctly updated in line with the rxtx compatibility move.
- Confirm that no other parts of the repository reference the subgraph module.
- Verify any indirect dependencies that might still expect the subgraph functionality.
crates/gql/Cargo.toml (1)
27-27
: Added dependency for rxtx compatibility.The addition of
txtx-addon-network-svm-types
as a workspace dependency is consistent with the changes in other Cargo.toml files, ensuring that GraphQL functionality can work with the rxtx system.crates/subgraph/Cargo.toml (1)
18-18
: Added dependency to support rxtx compatibility.Including
txtx-addon-network-svm-types
as a workspace dependency in the subgraph crate is consistent with the PR's objective. Note that this crate now depends on bothtxtx-addon-network-svm
andtxtx-addon-network-svm-types
, which appears to be the intended structure.crates/core/src/rpc/admin.rs (1)
11-11
: Import path updated correctly.The import for
PluginConfig
has been updated fromsurfpool_types::subgraph
totxtx_addon_network_svm_types::subgraph
, consistent with the PR's objective of updating dependencies for rxtx compatibility.crates/core/src/lib.rs (1)
23-24
: Import structure looks good.The changes maintain imports from
surfpool_types
for the necessary types while correctly moving thePluginConfig
import totxtx_addon_network_svm_types::subgraph
, aligning with the changes made in other files.crates/gql/src/types/schema.rs (1)
7-7
: Import path updated correctly.The import for
IndexedSubgraphField
andSVM_PUBKEY
has been updated to usetxtx_addon_network_svm_types
, consistent with the dependency changes throughout the PR.crates/types/Cargo.toml (2)
31-31
: Dependency added correctly.The addition of
txtx-addon-network-svm-types
as a workspace dependency correctly supports the import changes made across the codebase.
15-33
:✅ Verification successful
Verify dependency versions compatibility.
The PR adds
txtx-addon-network-svm-types
while removinganchor-lang-idl
. Make sure that all types previously used fromanchor-lang-idl
are now available through the new dependency or other existing dependencies.
🏁 Script executed:
#!/bin/bash # Check if there are any remaining imports of anchor-lang-idl in the codebase echo "Checking for remaining imports of anchor-lang-idl..." rg "use\s+anchor.lang.idl" || echo "No imports found."Length of output: 188
Dependency compatibility verified.
The verification confirms that there are no remaining imports of
anchor-lang-idl
in the codebase. With the removal complete and the addition oftxtx-addon-network-svm-types
, please ensure that all functionality previously provided byanchor-lang-idl
is now correctly sourced from the new dependency or other existing ones.
- Action Items:
- Verify that types and functionality previously provided by
anchor-lang-idl
are available throughtxtx-addon-network-svm-types
or other dependencies.- Double-check integration tests (if any) to ensure runtime compatibility.
crates/gql/src/subscription.rs (1)
16-16
: Good practice: Marking unused parameter with underscoreRenaming
context
to_context
follows Rust's convention for indicating intentionally unused parameters, which helps suppress compiler warnings about unused variables.crates/types/src/types.rs (1)
13-13
: Import source changed for SubgraphRequestThe
SubgraphRequest
type is now imported from the externaltxtx_addon_network_svm_types
crate instead of from within the local crate. This change aligns with the PR objective to ensure compatibility with rxtx.crates/gql/src/types/mod.rs (1)
18-18
: Import source changed for SvmValue and SVM_PUBKEYThe
SvmValue
andSVM_PUBKEY
imports have been migrated fromsurfpool_types::subgraph
totxtx_addon_network_svm_types
. This is consistent with the other import changes in this PR, consolidating these types in the new dependency.crates/subgraph/src/lib.rs (1)
10-12
: Import sources updated while maintaining functionalityThe imports for
IndexedSubgraphSourceType
andSubgraphRequest
have been moved fromsurfpool_types::subgraph
totxtx_addon_network_svm_types::subgraph
, whileSchemaDataSourcingEvent
andSubgraphPluginConfig
remain insurfpool_types
. This split approach maintains compatibility during the transition to the new dependency structure.crates/gql/src/lib.rs (2)
7-7
: Import cleaned up to remove unused type.The
SubgraphDataEntryUpdate
type has been removed from imports, which is consistent with it not being used in the file. This is a good cleanup.
42-51
: GraphQL introspection enabled.The function now enables introspection for the GraphQL schema, which will allow clients to discover the schema structure. This is a good improvement for API discoverability and client tooling support.
Cargo.toml (3)
45-45
: Added bincode feature to solana-transaction.The
bincode
feature has been added to the solana-transaction dependency, which will enable serialization/deserialization using the bincode format. This is likely to support new functionality.
71-76
:❓ Verification inconclusive
Updated txtx dependency versions.
Multiple changes have been made to the txtx dependencies:
- txtx-core updated from 0.2.5 to 0.2.7
- txtx-addon-network-svm downgraded from 0.1.9 to 0.1.8
- Added new dependency txtx-addon-network-svm-types at 0.1.0
- Added "wasm" feature to txtx-addon-kit
The downgrade of txtx-addon-network-svm from 0.1.9 to 0.1.8 is unusual and might indicate a compatibility issue.
🏁 Script executed:
#!/bin/bash # Check for compatibility issues in changelogs or commit history curl -s https://github.com/txtx/txtx/releases/tag/v0.1.8 || echo "Repository or release not accessible" curl -s https://github.com/txtx/txtx/releases/tag/v0.1.9 || echo "Repository or release not accessible"Length of output: 77921
Dependency Version Update Verification – Manual Check Recommended
The following Cargo.toml changes have been applied:
- txtx-core upgraded from 0.2.5 to 0.2.7.
- txtx-addon-network-svm was downgraded from 0.1.9 to 0.1.8.
- A new dependency, txtx-addon-network-svm-types at 0.1.0, has been added.
- The "wasm" feature has been enabled for txtx-addon-kit.
The downgrade for txtx-addon-network-svm is unusual and may indicate an underlying compatibility adjustment. A preliminary check via the release pages for versions v0.1.8 and v0.1.9 did not yield a clear indication of compatibility issues. However, the output from the automated verification did not provide sufficiently detailed information.
I recommend a manual verification of the changelog or related documentation in the txtx repository to ensure that this downgrade does not introduce unintended compatibility problems.
47-47
:❓ Verification inconclusive
Updated solana-transaction-status version.
The version of solana-transaction-status has been updated from 2.2.1 to 2.2.2. Make sure this version is compatible with the other Solana dependencies that are still at 2.2.1.
🏁 Script executed:
#!/bin/bash # Check for version compatibility information curl -s https://github.com/solana-labs/solana/releases/tag/v2.2.2 | grep -i "breaking change"Length of output: 95
Action: Verify Solana Dependency Compatibility
The update in Cargo.toml changes the
solana-transaction-status
version from 2.2.1 to 2.2.2. Automated retrieval of breaking change information from the release notes for v2.2.2 returned no output. Since no explicit compatibility data was found, please perform a manual verification to confirm that this updated version will work seamlessly with the other Solana dependencies that remain at 2.2.1.
- File: Cargo.toml (line 47)
- Snippet:
solana-transaction-status = { version = "2.2.2" }- Action Required: Manually verify compatibility between solana-transaction-status v2.2.2 and any other Solana dependencies still pinned to v2.2.1.
There was a problem hiding this 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
🧹 Nitpick comments (1)
crates/cli/src/http/mod.rs (1)
242-242
: URL reference updated, but still hardcodedThe URL has been updated to match the new GraphiQL endpoint, fixing the inconsistency noted in the previous review. However, the URL still contains a hardcoded port (8900).
Consider deriving the URL from configuration instead of hardcoding it:
- let _ = sender.send("http://127.0.0.1:8900/gql/console".into()); + let host = config.host.unwrap_or("127.0.0.1".to_string()); + let port = config.port.unwrap_or(8900); + let _ = sender.send(format!("http://{}:{}/gql/console", host, port).into());
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
Cargo.lock
is excluded by!**/*.lock
📒 Files selected for processing (8)
Cargo.toml
(2 hunks)crates/cli/src/http/mod.rs
(3 hunks)crates/core/Cargo.toml
(1 hunks)crates/core/src/lib.rs
(1 hunks)crates/core/src/rpc/admin.rs
(1 hunks)crates/gql/src/lib.rs
(2 hunks)crates/gql/src/subscription.rs
(1 hunks)crates/types/src/subgraph.rs
(0 hunks)
💤 Files with no reviewable changes (1)
- crates/types/src/subgraph.rs
🚧 Files skipped from review as they are similar to previous changes (6)
- crates/gql/src/subscription.rs
- crates/core/Cargo.toml
- crates/core/src/lib.rs
- crates/core/src/rpc/admin.rs
- Cargo.toml
- crates/gql/src/lib.rs
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: run_cargo_checks
- GitHub Check: build
🔇 Additional comments (2)
crates/cli/src/http/mod.rs (2)
10-10
: Removed playground_handler import correctlyThe removal of
playground_handler
from the imports aligns with the complete removal of the playground functionality from the codebase, as indicated in the PR summary.
76-76
: Endpoint path change is a breaking changeThe GraphiQL interface endpoint has been updated from
/graphiql
to/gql/console
. This is a breaking change that will affect any clients directly accessing the GraphiQL interface.Make sure this change is documented in the project's changelog or release notes to inform users about this breaking change.
Summary by CodeRabbit