Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 13 additions & 1 deletion forester-utils/src/rpc_pool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ pub enum PoolError {

pub struct SolanaConnectionManager<R: Rpc + 'static> {
url: String,
photon_url: Option<String>,
commitment: CommitmentConfig,
// TODO: implement Rpc for SolanaConnectionManager and rate limit requests.
_rpc_rate_limiter: Option<RateLimiter>,
Expand All @@ -36,12 +37,14 @@ pub struct SolanaConnectionManager<R: Rpc + 'static> {
impl<R: Rpc + 'static> SolanaConnectionManager<R> {
pub fn new(
url: String,
photon_url: Option<String>,
commitment: CommitmentConfig,
rpc_rate_limiter: Option<RateLimiter>,
send_tx_rate_limiter: Option<RateLimiter>,
) -> Self {
Self {
url,
photon_url,
commitment,
_rpc_rate_limiter: rpc_rate_limiter,
_send_tx_rate_limiter: send_tx_rate_limiter,
Expand All @@ -58,8 +61,8 @@ impl<R: Rpc + 'static> bb8::ManageConnection for SolanaConnectionManager<R> {
async fn connect(&self) -> Result<Self::Connection, Self::Error> {
let config = LightClientConfig {
url: self.url.to_string(),
photon_url: self.photon_url.clone(),
commitment_config: Some(self.commitment),
with_indexer: false,
fetch_active_tree: false,
};

Expand All @@ -86,6 +89,8 @@ pub struct SolanaRpcPool<R: Rpc + 'static> {
#[derive(Debug)]
pub struct SolanaRpcPoolBuilder<R: Rpc> {
url: Option<String>,
photon_url: Option<String>,

commitment: Option<CommitmentConfig>,

max_size: u32,
Expand All @@ -110,6 +115,7 @@ impl<R: Rpc> SolanaRpcPoolBuilder<R> {
pub fn new() -> Self {
Self {
url: None,
photon_url: None,
commitment: None,
max_size: 50,
connection_timeout_secs: 15,
Expand All @@ -128,6 +134,11 @@ impl<R: Rpc> SolanaRpcPoolBuilder<R> {
self
}

pub fn photon_url(mut self, url: Option<String>) -> Self {
self.photon_url = url;
self
}

pub fn commitment(mut self, commitment: CommitmentConfig) -> Self {
self.commitment = Some(commitment);
self
Expand Down Expand Up @@ -183,6 +194,7 @@ impl<R: Rpc> SolanaRpcPoolBuilder<R> {

let manager = SolanaConnectionManager::new(
url,
self.photon_url,
commitment,
self.rpc_rate_limiter,
self.send_tx_rate_limiter,
Expand Down
8 changes: 3 additions & 5 deletions forester/src/epoch_manager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -475,10 +475,9 @@ impl<R: Rpc, I: Indexer + IndexerType<R> + 'static> EpochManager<R, I> {
) -> Result<ForesterEpochInfo> {
let rpc = LightClient::new(LightClientConfig {
url: self.config.external_services.rpc_url.to_string(),
photon_url: self.config.external_services.indexer_url.clone(),
commitment_config: None,
fetch_active_tree: false,

with_indexer: false,
})
.await
.unwrap();
Expand Down Expand Up @@ -545,10 +544,9 @@ impl<R: Rpc, I: Indexer + IndexerType<R> + 'static> EpochManager<R, I> {
info!("Registering for epoch: {}", epoch);
let mut rpc = LightClient::new(LightClientConfig {
url: self.config.external_services.rpc_url.to_string(),
photon_url: None,
commitment_config: None,
fetch_active_tree: false,

with_indexer: false,
})
.await
.unwrap();
Expand Down Expand Up @@ -1198,9 +1196,9 @@ impl<R: Rpc, I: Indexer + IndexerType<R> + 'static> EpochManager<R, I> {
info!("Reporting work");
let mut rpc = LightClient::new(LightClientConfig {
url: self.config.external_services.rpc_url.to_string(),
photon_url: None,
commitment_config: None,
fetch_active_tree: false,
with_indexer: false,
})
.await
.unwrap();
Expand Down
2 changes: 1 addition & 1 deletion forester/src/forester_status.rs
Original file line number Diff line number Diff line change
Expand Up @@ -175,9 +175,9 @@ pub async fn fetch_forester_status(args: &StatusArgs) {
debug!("RPC URL: {}", config.external_services.rpc_url);
let mut rpc = LightClient::new(LightClientConfig {
url: config.external_services.rpc_url.to_string(),
photon_url: None,
commitment_config: None,
fetch_active_tree: false,
with_indexer: false,
})
.await
.unwrap();
Comment on lines 176 to 183
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Use configured photon_url and avoid panic on LightClient build

The ForesterConfig already carries external_services.photon_url; passing None disables the indexer even when the user enabled it in the config. At the same time, unwrap() will abort the status command on any RPC hiccup.

-    let mut rpc = LightClient::new(LightClientConfig {
-        url: config.external_services.rpc_url.to_string(),
-        photon_url: None,
+    let mut rpc = LightClient::new(LightClientConfig {
+        url: config.external_services.rpc_url.to_string(),
+        photon_url: config.external_services.photon_url.clone(),
         commitment_config: None,
         fetch_active_tree: false,
-    })
-    .await
-    .unwrap();
+    })
+    .await
+    .context("failed to create LightClient")?;

This preserves the new configuration surface and improves robustness.

📝 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.

Suggested change
let mut rpc = LightClient::new(LightClientConfig {
url: config.external_services.rpc_url.to_string(),
photon_url: None,
commitment_config: None,
fetch_active_tree: false,
with_indexer: false,
})
.await
.unwrap();
let mut rpc = LightClient::new(LightClientConfig {
url: config.external_services.rpc_url.to_string(),
photon_url: config.external_services.photon_url.clone(),
commitment_config: None,
fetch_active_tree: false,
})
.await
.context("failed to create LightClient")?;
🤖 Prompt for AI Agents
In forester/src/forester_status.rs around lines 176 to 183, replace the
hardcoded None for photon_url with the configured photon_url from
external_services to respect user settings. Also, instead of using unwrap() on
the LightClient::new call, handle the Result properly to avoid panics on RPC
failures, for example by using match or if let to gracefully handle errors and
improve robustness.

Expand Down
4 changes: 2 additions & 2 deletions forester/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,10 +53,9 @@ pub async fn run_queue_info(
) {
let mut rpc = LightClient::new(LightClientConfig {
url: config.external_services.rpc_url.to_string(),
photon_url: None,
commitment_config: None,
fetch_active_tree: false,

with_indexer: false,
})
.await
.unwrap();
Expand Down Expand Up @@ -101,6 +100,7 @@ pub async fn run_pipeline<R: Rpc, I: Indexer + IndexerType<R> + 'static>(
) -> Result<()> {
let mut builder = SolanaRpcPoolBuilder::<R>::default()
.url(config.external_services.rpc_url.to_string())
.photon_url(config.external_services.indexer_url.clone())
.commitment(CommitmentConfig::confirmed())
.max_size(config.rpc_pool_config.max_size)
.connection_timeout_secs(config.rpc_pool_config.connection_timeout_secs)
Expand Down
13 changes: 3 additions & 10 deletions forester/tests/address_v2_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use light_batched_merkle_tree::{
use light_client::{
indexer::{photon_indexer::PhotonIndexer, AddressWithTree},
local_test_validator::{LightValidatorConfig, ProverConfig},
rpc::{client::RpcUrl, merkle_tree::MerkleTreeExt, LightClient, LightClientConfig, Rpc},
rpc::{merkle_tree::MerkleTreeExt, LightClient, LightClientConfig, Rpc},
};
use light_compressed_account::{
address::derive_address,
Expand All @@ -32,7 +32,7 @@ use light_test_utils::{
use rand::{prelude::StdRng, Rng, SeedableRng};
use serial_test::serial;
use solana_program::{native_token::LAMPORTS_PER_SOL, pubkey::Pubkey};
use solana_sdk::{commitment_config::CommitmentConfig, signature::Keypair, signer::Signer};
use solana_sdk::{signature::Keypair, signer::Signer};
use tokio::sync::{mpsc, oneshot, Mutex};

use crate::test_utils::{forester_config, init};
Expand Down Expand Up @@ -71,14 +71,7 @@ async fn test_create_v2_address() {
config.derivation_pubkey = env.protocol.forester.pubkey();
config.general_config = GeneralConfig::test_address_v2();

let mut rpc = LightClient::new(LightClientConfig {
url: RpcUrl::Localnet.to_string(),
commitment_config: Some(CommitmentConfig::processed()),
fetch_active_tree: false,
with_indexer: true,
})
.await
.unwrap();
let mut rpc = LightClient::new(LightClientConfig::local()).await.unwrap();
rpc.payer = env.protocol.forester.insecure_clone();

ensure_sufficient_balance(
Expand Down
2 changes: 1 addition & 1 deletion forester/tests/batched_address_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,9 +65,9 @@ async fn test_address_batched() {
let commitment_config = CommitmentConfig::confirmed();
let mut rpc = LightClient::new(LightClientConfig {
url: RpcUrl::Localnet.to_string(),
photon_url: None,
commitment_config: Some(commitment_config),
fetch_active_tree: false,
with_indexer: false,
})
.await
.unwrap();
Expand Down
12 changes: 2 additions & 10 deletions forester/tests/batched_state_async_indexer_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ use light_client::{
GetCompressedTokenAccountsByOwnerOrDelegateOptions, Indexer,
},
local_test_validator::{LightValidatorConfig, ProverConfig},
rpc::{client::RpcUrl, LightClient, LightClientConfig, Rpc},
rpc::{LightClient, LightClientConfig, Rpc},
};
use light_compressed_account::{
address::derive_address_legacy,
Expand All @@ -38,7 +38,6 @@ use rand::{prelude::SliceRandom, rngs::StdRng, Rng, SeedableRng};
use serial_test::serial;
use solana_program::{native_token::LAMPORTS_PER_SOL, pubkey::Pubkey};
use solana_sdk::{
commitment_config::CommitmentConfig,
signature::{Keypair, Signature},
signer::Signer,
};
Expand Down Expand Up @@ -222,14 +221,7 @@ async fn test_state_indexer_async_batched() {
// ─────────────────────────────────────────────────────────────────────────────

async fn setup_rpc_connection(forester: &Keypair) -> LightClient {
let mut rpc = LightClient::new(LightClientConfig {
url: RpcUrl::Localnet.to_string(),
commitment_config: Some(CommitmentConfig::processed()),
fetch_active_tree: false,
with_indexer: true,
})
.await
.unwrap();
let mut rpc = LightClient::new(LightClientConfig::local()).await.unwrap();
rpc.payer = forester.insecure_clone();
rpc
}
Expand Down
12 changes: 2 additions & 10 deletions forester/tests/batched_state_indexer_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ use light_batched_merkle_tree::{
use light_client::{
indexer::{photon_indexer::PhotonIndexer, Indexer, IndexerRpcConfig, RetryConfig},
local_test_validator::{LightValidatorConfig, ProverConfig},
rpc::{client::RpcUrl, LightClient, LightClientConfig, Rpc},
rpc::{LightClient, LightClientConfig, Rpc},
};
use light_compressed_account::TreeType;
use light_program_test::{accounts::test_accounts::TestAccounts, indexer::TestIndexer};
Expand Down Expand Up @@ -61,15 +61,7 @@ async fn test_state_indexer_batched() {
.await
.unwrap();

let commitment_config = CommitmentConfig::confirmed();
let mut rpc = LightClient::new(LightClientConfig {
url: RpcUrl::Localnet.to_string(),
fetch_active_tree: false,
commitment_config: Some(commitment_config),
with_indexer: true,
})
.await
.unwrap();
let mut rpc = LightClient::new(LightClientConfig::local()).await.unwrap();
rpc.payer = forester_keypair.insecure_clone();

rpc.airdrop_lamports(&forester_keypair.pubkey(), LAMPORTS_PER_SOL * 100_000)
Expand Down
2 changes: 1 addition & 1 deletion forester/tests/batched_state_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,9 +69,9 @@ async fn test_state_batched() {
let commitment_config = CommitmentConfig::confirmed();
let mut rpc = LightClient::new(LightClientConfig {
url: RpcUrl::Localnet.to_string(),
photon_url: None,
commitment_config: Some(commitment_config),
fetch_active_tree: false,
with_indexer: false,
})
.await
.unwrap();
Expand Down
13 changes: 4 additions & 9 deletions forester/tests/e2e_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,14 +60,9 @@ async fn test_epoch_monitor_with_2_foresters() {
.await
.unwrap();

let mut rpc = LightClient::new(LightClientConfig {
url: RpcUrl::Localnet.to_string(),
commitment_config: Some(CommitmentConfig::confirmed()),
fetch_active_tree: false,
with_indexer: false,
})
.await
.unwrap();
let mut rpc = LightClient::new(LightClientConfig::local_no_indexer())
.await
.unwrap();
rpc.payer = forester_keypair1.insecure_clone();

// Airdrop to both foresters and governance authority
Expand Down Expand Up @@ -410,9 +405,9 @@ async fn test_epoch_double_registration() {

let mut rpc = LightClient::new(LightClientConfig {
url: RpcUrl::Localnet.to_string(),
photon_url: None,
commitment_config: Some(CommitmentConfig::confirmed()),
fetch_active_tree: false,
with_indexer: false,
})
.await
.unwrap();
Expand Down
12 changes: 2 additions & 10 deletions forester/tests/priority_fee_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,8 @@ use forester::{
ForesterConfig,
};
use light_client::rpc::{LightClient, LightClientConfig, Rpc};
use light_test_utils::RpcUrl;
use reqwest::Url;
use solana_sdk::{commitment_config::CommitmentConfig, signature::Signer};
use solana_sdk::signature::Signer;

use crate::test_utils::init;
mod test_utils;
Expand Down Expand Up @@ -79,14 +78,7 @@ async fn test_priority_fee_request() {
let config = ForesterConfig::new_for_start(&args).expect("Failed to create config");

// Setup RPC connection using config
let mut rpc = LightClient::new(LightClientConfig {
url: RpcUrl::Localnet.to_string(),
commitment_config: Some(CommitmentConfig::confirmed()),
fetch_active_tree: false,
with_indexer: false,
})
.await
.unwrap();
let mut rpc = LightClient::new(LightClientConfig::local()).await.unwrap();
rpc.payer = config.payer_keypair.insecure_clone();

let account_keys = vec![config.payer_keypair.pubkey()];
Expand Down
2 changes: 1 addition & 1 deletion program-tests/utils/src/setup_accounts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ pub async fn setup_accounts(keypairs: TestKeypairs, url: RpcUrl) -> Result<TestA
let mut rpc = light_client::rpc::LightClient::new(LightClientConfig {
commitment_config: Some(CommitmentConfig::confirmed()),
url: url.to_string(),
with_indexer: false,
photon_url: None,
fetch_active_tree: false,
})
.await
Expand Down
17 changes: 2 additions & 15 deletions sdk-libs/client/src/rpc/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -110,21 +110,8 @@ impl LightClient {
let client = RpcClient::new_with_commitment(config.url.to_string(), commitment_config);
let retry_config = retry_config.unwrap_or_default();

let indexer = if config.with_indexer {
if config.url == RpcUrl::Localnet.to_string() {
Some(PhotonIndexer::new(
"http://127.0.0.1:8784".to_string(),
None,
))
} else {
Some(PhotonIndexer::new(
config.url.to_string(),
None, // TODO: test that this is not required
))
}
} else {
None
};
let indexer = config.photon_url.map(|path| PhotonIndexer::new(path, None));

let mut new = Self {
Comment on lines 111 to 115
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Photon API key silently dropped

PhotonIndexer::new(path, None) ignores any API key that may be required for production use. If the key is available but not forwarded, authenticated requests will fail.

- let indexer = config.photon_url.map(|path| PhotonIndexer::new(path, None));
+ // Propagate the optional API key once LightClientConfig is extended
+ let indexer = config
+     .photon_url
+     .as_ref()
+     .map(|path| PhotonIndexer::new(path.clone(), config.photon_api_key.clone()));

This will need:

  1. Adding photon_api_key: Option<String> to LightClientConfig.
  2. Populating it in the builders/tests that already carry the key.

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In sdk-libs/client/src/rpc/client.rs around lines 111 to 115, the PhotonIndexer
is created without passing the API key, causing it to be silently dropped and
leading to failed authenticated requests. To fix this, add a photon_api_key
field of type Option<String> to the LightClientConfig struct, update all
builders and tests that instantiate LightClientConfig to populate this new field
with the API key if available, and modify the PhotonIndexer::new call to pass
the API key instead of None so that authenticated requests can succeed.

client,
payer,
Expand Down
14 changes: 7 additions & 7 deletions sdk-libs/client/src/rpc/rpc_trait.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,24 +27,24 @@ use crate::{
pub struct LightClientConfig {
pub url: String,
pub commitment_config: Option<CommitmentConfig>,
pub with_indexer: bool,
pub photon_url: Option<String>,
pub fetch_active_tree: bool,
}

impl LightClientConfig {
pub fn new(url: String) -> Self {
pub fn new(url: String, photon_url: Option<String>) -> Self {
Self {
url,
photon_url,
commitment_config: Some(CommitmentConfig::confirmed()),
with_indexer: true,
fetch_active_tree: true,
}
}
pub fn local_no_indexer() -> Self {
Self {
url: RpcUrl::Localnet.to_string(),
commitment_config: Some(CommitmentConfig::confirmed()),
with_indexer: false,
photon_url: None,
fetch_active_tree: false,
}
}
Expand All @@ -53,16 +53,16 @@ impl LightClientConfig {
Self {
url: RpcUrl::Localnet.to_string(),
commitment_config: Some(CommitmentConfig::confirmed()),
with_indexer: true,
photon_url: Some("http://127.0.0.1:8784".to_string()),
fetch_active_tree: false,
}
}

pub fn devnet() -> Self {
pub fn devnet(photon_url: Option<String>) -> Self {
Self {
url: RpcUrl::Devnet.to_string(),
photon_url,
commitment_config: Some(CommitmentConfig::confirmed()),
with_indexer: true,
fetch_active_tree: true,
}
}
Expand Down
2 changes: 1 addition & 1 deletion xtask/src/create_batch_address_tree.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,9 @@ pub async fn create_batch_address_tree(options: Options) -> anyhow::Result<()> {
};
let mut rpc = LightClient::new(LightClientConfig {
url: rpc_url,
photon_url: None,
commitment_config: None,
fetch_active_tree: false,
with_indexer: false,
})
.await
.unwrap();
Expand Down
Loading