Support for deployments from anchor cli #39
Conversation
WalkthroughThis pull request refines the command-line and RPC interfaces. In the CLI module, it removes the redundant alias Changes
Possibly related PRs
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
CodeRabbit Configuration File (
|
crates/core/src/rpc/accounts_data.rs
Outdated
| .await; | ||
|
|
||
| // Save missing fetched accounts | ||
| let fetched_account = |
There was a problem hiding this comment.
Should we first load what we have locally, then fetch from the remote the ones we don't have, then merge?
There was a problem hiding this comment.
It costs the same for most RPC providers but I fixed that nonetheless, it's more efficient
crates/core/src/rpc/mod.rs
Outdated
| // println!("Processing took: {:?}", start.elapsed()); | ||
| res | ||
| }))) | ||
| println!("Processing request {:?}", request); |
![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 (1)
crates/core/src/rpc/accounts_data.rs (1)
150-157: Consider verifying pubkeys lazily.While collecting all pubkey verification results before proceeding ensures early error detection, it may be inefficient for large input sets where only a few keys need remote fetching. Consider verifying keys lazily as needed.
- let pubkeys = match pubkeys_str - .iter() - .map(|s| verify_pubkey(s)) - .collect::<Result<Vec<_>>>() - { - Ok(p) => p, - Err(e) => return Box::pin(future::err(e.into())), - }; + let mut pubkeys = Vec::with_capacity(pubkeys_str.len()); + for s in pubkeys_str.iter() { + match verify_pubkey(s) { + Ok(pk) => pubkeys.push(pk), + Err(e) => return Box::pin(future::err(e.into())), + } + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
crates/core/src/rpc/accounts_data.rs(4 hunks)crates/core/src/rpc/mod.rs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- crates/core/src/rpc/mod.rs
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: build
- GitHub Check: run_cargo_checks
🔇 Additional comments (3)
crates/core/src/rpc/accounts_data.rs (3)
1-1: LGTM! Import changes align with the new implementation.The removal of
join_alland addition ofPubkeyimports reflect the shift from parallel to batch account fetching.Also applies to: 6-6, 10-10, 18-18
42-48: LGTM! Parameter rename improves clarity.The rename from
pubkeytopubkey_strsbetter indicates that the parameter is a collection of string-formatted public keys.
177-211: LGTM! Efficient implementation with proper caching.The new implementation:
- Efficiently identifies missing accounts
- Uses batch fetching instead of parallel individual fetches
- Properly caches fetched accounts
- Handles errors robustly
|
Addressed feedbacks in the last commit @lgalabru |
This PR adds features and fixes to enable deploying and testing programs.
However, Anchor by default uses websockets and TPU client, which are not available in Surfpool yet. So deploying requires running
solana program deploy -u localhost target/deploy/surfpool_test.so --use-rpcto do everything through the RPC, and testing programs works (transactions are correctely received and processed by Surfpool) but Anchor tries to stream program logs through websockets and so it currently gets no response.Summary by CodeRabbit
New Features
Bug Fixes
Chores