feat: introduce accounts_data trait #6
Conversation
|
Warning Rate limit exceeded@lgalabru has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 21 minutes and 19 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (3)
WalkthroughThe changes introduce a new RPC trait, Changes
Sequence Diagram(s)sequenceDiagram
participant C as Client
participant S as RPC Server
participant A as AccountsData Delegate
C->>S: Send RPC request (e.g., get_account_info)
S->>A: Delegate the request
A-->>S: Return account data
S-->>C: Deliver JSON-RPC response
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
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: 0
🧹 Nitpick comments (2)
crates/core/src/rpc/accounts_data.rs (2)
38-84: Document public RPC trait methods.
Since these methods are user-facing, adding doc comments or examples would clarify how to consume each RPC endpoint and what to expect in terms of inputs, outputs, and potential error scenarios.
86-137: Provide partial implementation or stubs for unimplemented methods.
Relying solely onunimplemented!()might hinder integration testing. Consider returning dummy responses or partial results to facilitate incremental development and testing.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
crates/core/src/rpc/accounts_data.rs(1 hunks)crates/core/src/rpc/mod.rs(1 hunks)crates/core/src/simnet/mod.rs(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- crates/core/src/rpc/mod.rs
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
🔇 Additional comments (3)
crates/core/src/rpc/accounts_data.rs (1)
1-37: Imports look well-structured.
The comprehensive set of Solana and JSON-RPC imports is appropriate to support the RPC trait.crates/core/src/simnet/mod.rs (2)
16-16: New import aligns with new RPC trait.
ImportingAccountsDatais consistent with integrating the new account-related RPC endpoints.
68-68: RPC handler extension looks correct.
IntegratingSurfpoolAccountsDataRpcinto the server’s IoHandler ensures that account data methods are now exposed to clients.
lgalabru
force-pushed
the
feat/introduce-accounts-data-rpc
branch
from
04fb542 to
1d8b53f
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
crates/core/src/rpc/accounts_data.rs(1 hunks)crates/core/src/rpc/mod.rs(1 hunks)crates/core/src/simnet/mod.rs(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- crates/core/src/rpc/mod.rs
- crates/core/src/simnet/mod.rs
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
🔇 Additional comments (3)
crates/core/src/rpc/accounts_data.rs (3)
38-84: Well-structured trait definition following Solana's RPC conventions.The trait is well-designed with:
- Clear method signatures matching Solana's RPC API patterns
- Appropriate use of async BoxFuture for potentially long-running operations
- Good documentation for SPL Token-specific endpoints
12-16: Consider removing unused imports.Several imported types from
rpc_configappear to be unused in the current implementation:
RpcBlockConfigRpcBlocksConfigWrapperRpcEncodingConfigWrapperRpcEpochConfigRpcRequestAirdropConfigRpcSendTransactionConfigRpcSignatureStatusConfigRpcSignaturesForAddressConfigRpcSimulateTransactionConfigRpcTransactionConfig✅ Verification successful
Unused RPC configuration imports in
crates/core/src/rpc/accounts_data.rsThe search confirms that all the listed RPC configuration types (e.g.,
RpcBlockConfig,RpcBlocksConfigWrapper, etc.) are only mentioned in the import statement (lines 13–15) and aren’t used elsewhere in the file. Consider removing these unused imports to clean up the code.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify usage of potentially unused imports rg -l "Rpc(Block|Blocks|Encoding|Epoch|RequestAirdrop|SendTransaction|SignatureStatus|SignaturesForAddress|SimulateTransaction|Transaction)Config" --type rustLength of output: 225
Script:
#!/bin/bash # Check for direct usage of the imported types in crates/core/src/rpc/accounts_data.rs rg "RpcBlockConfig" --type rust crates/core/src/rpc/accounts_data.rsLength of output: 158
Script:
#!/bin/bash # Check for each imported type beyond the import block in crates/core/src/rpc/accounts_data.rs imports=( "RpcBlockConfig" "RpcBlocksConfigWrapper" "RpcEncodingConfigWrapper" "RpcEpochConfig" "RpcRequestAirdropConfig" "RpcSendTransactionConfig" "RpcSignatureStatusConfig" "RpcSignaturesForAddressConfig" "RpcSimulateTransactionConfig" "RpcTransactionConfig" ) for imp in "${imports[@]}"; do echo "Searching for $imp usage (excluding the import block)..." rg -n "$imp" --type rust crates/core/src/rpc/accounts_data.rs | grep -v "rpc_config::" doneLength of output: 4097
1-137: Add tests for the RPC interface.Consider adding unit tests to:
- Verify RPC method signatures match Solana's API
- Test error handling once implemented
- Ensure proper serialization/deserialization of requests and responses
| pub struct SurfpoolAccountsDataRpc; | ||
| impl AccountsData for SurfpoolAccountsDataRpc { | ||
| type Metadata = Option<RunloopContext>; | ||
|
|
||
| fn get_account_info( | ||
| &self, | ||
| meta: Self::Metadata, | ||
| pubkey_str: String, | ||
| config: Option<RpcAccountInfoConfig>, | ||
| ) -> BoxFuture<Result<RpcResponse<Option<UiAccount>>>> { | ||
| unimplemented!() | ||
| } | ||
|
|
||
| fn get_multiple_accounts( | ||
| &self, | ||
| meta: Self::Metadata, | ||
| pubkey_strs: Vec<String>, | ||
| config: Option<RpcAccountInfoConfig>, | ||
| ) -> BoxFuture<Result<RpcResponse<Vec<Option<UiAccount>>>>> { | ||
| unimplemented!() | ||
| } | ||
|
|
||
| fn get_block_commitment( | ||
| &self, | ||
| meta: Self::Metadata, | ||
| block: Slot, | ||
| ) -> Result<RpcBlockCommitment<BlockCommitmentArray>> { | ||
| unimplemented!() | ||
| } | ||
|
|
||
| // SPL Token-specific RPC endpoints | ||
| // See https://github.com/solana-labs/solana-program-library/releases/tag/token-v2.0.0 for | ||
| // program details | ||
|
|
||
| fn get_token_account_balance( | ||
| &self, | ||
| meta: Self::Metadata, | ||
| pubkey_str: String, | ||
| commitment: Option<CommitmentConfig>, | ||
| ) -> Result<RpcResponse<UiTokenAmount>> { | ||
| unimplemented!() | ||
| } | ||
|
|
||
| fn get_token_supply( | ||
| &self, | ||
| meta: Self::Metadata, | ||
| mint_str: String, | ||
| commitment: Option<CommitmentConfig>, | ||
| ) -> Result<RpcResponse<UiTokenAmount>> { | ||
| unimplemented!() | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Replace unimplemented!() with proper error handling.
Currently, all methods use unimplemented!() which will cause runtime panics. Consider:
- Adding TODOs with implementation plans
- Implementing basic error responses until full implementation is ready
Example implementation pattern:
fn get_account_info(
&self,
meta: Self::Metadata,
pubkey_str: String,
config: Option<RpcAccountInfoConfig>,
) -> BoxFuture<Result<RpcResponse<Option<UiAccount>>>> {
- unimplemented!()
+ // TODO: Implement account info retrieval
+ Box::pin(async move {
+ Err(Error::method_not_implemented())
+ })
}Apply similar pattern to other methods to provide proper error handling instead of panics.
📝 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.
| pub struct SurfpoolAccountsDataRpc; | |
| impl AccountsData for SurfpoolAccountsDataRpc { | |
| type Metadata = Option<RunloopContext>; | |
| fn get_account_info( | |
| &self, | |
| meta: Self::Metadata, | |
| pubkey_str: String, | |
| config: Option<RpcAccountInfoConfig>, | |
| ) -> BoxFuture<Result<RpcResponse<Option<UiAccount>>>> { | |
| unimplemented!() | |
| } | |
| fn get_multiple_accounts( | |
| &self, | |
| meta: Self::Metadata, | |
| pubkey_strs: Vec<String>, | |
| config: Option<RpcAccountInfoConfig>, | |
| ) -> BoxFuture<Result<RpcResponse<Vec<Option<UiAccount>>>>> { | |
| unimplemented!() | |
| } | |
| fn get_block_commitment( | |
| &self, | |
| meta: Self::Metadata, | |
| block: Slot, | |
| ) -> Result<RpcBlockCommitment<BlockCommitmentArray>> { | |
| unimplemented!() | |
| } | |
| // SPL Token-specific RPC endpoints | |
| // See https://github.com/solana-labs/solana-program-library/releases/tag/token-v2.0.0 for | |
| // program details | |
| fn get_token_account_balance( | |
| &self, | |
| meta: Self::Metadata, | |
| pubkey_str: String, | |
| commitment: Option<CommitmentConfig>, | |
| ) -> Result<RpcResponse<UiTokenAmount>> { | |
| unimplemented!() | |
| } | |
| fn get_token_supply( | |
| &self, | |
| meta: Self::Metadata, | |
| mint_str: String, | |
| commitment: Option<CommitmentConfig>, | |
| ) -> Result<RpcResponse<UiTokenAmount>> { | |
| unimplemented!() | |
| } | |
| } | |
| pub struct SurfpoolAccountsDataRpc; | |
| impl AccountsData for SurfpoolAccountsDataRpc { | |
| type Metadata = Option<RunloopContext>; | |
| fn get_account_info( | |
| &self, | |
| meta: Self::Metadata, | |
| pubkey_str: String, | |
| config: Option<RpcAccountInfoConfig>, | |
| ) -> BoxFuture<Result<RpcResponse<Option<UiAccount>>>> { | |
| // TODO: Implement account info retrieval | |
| Box::pin(async move { | |
| Err(Error::method_not_implemented()) | |
| }) | |
| } | |
| fn get_multiple_accounts( | |
| &self, | |
| meta: Self::Metadata, | |
| pubkey_strs: Vec<String>, | |
| config: Option<RpcAccountInfoConfig>, | |
| ) -> BoxFuture<Result<RpcResponse<Vec<Option<UiAccount>>>>> { | |
| unimplemented!() | |
| } | |
| fn get_block_commitment( | |
| &self, | |
| meta: Self::Metadata, | |
| block: Slot, | |
| ) -> Result<RpcBlockCommitment<BlockCommitmentArray>> { | |
| unimplemented!() | |
| } | |
| // SPL Token-specific RPC endpoints | |
| // See https://github.com/solana-labs/solana-program-library/releases/tag/token-v2.0.0 for | |
| // program details | |
| fn get_token_account_balance( | |
| &self, | |
| meta: Self::Metadata, | |
| pubkey_str: String, | |
| commitment: Option<CommitmentConfig>, | |
| ) -> Result<RpcResponse<UiTokenAmount>> { | |
| unimplemented!() | |
| } | |
| fn get_token_supply( | |
| &self, | |
| meta: Self::Metadata, | |
| mint_str: String, | |
| commitment: Option<CommitmentConfig>, | |
| ) -> Result<RpcResponse<UiTokenAmount>> { | |
| unimplemented!() | |
| } | |
| } |
lgalabru
force-pushed
the
feat/introduce-accounts-data-rpc
branch
from
1d8b53f to
55100e4
Compare
lgalabru
force-pushed
the
feat/introduce-accounts-data-rpc
branch
from
55100e4 to
206c6e1
Compare
Summary by CodeRabbit