Skip to content

Commit

Permalink
abandon the wait_till_connected test-only method
Browse files Browse the repository at this point in the history
this was used to make sure the background task have found a working connection by the time we start querying electrum requests. we don't need this now though. because if there are no working/maintained connections we fall back to using any/all other connections (and we establish the connection inline first)
  • Loading branch information
mariocynicys committed Sep 18, 2024
1 parent 18a430e commit b4bd969
Show file tree
Hide file tree
Showing 4 changed files with 13 additions and 51 deletions.
25 changes: 10 additions & 15 deletions mm2src/coins/utxo/rpc_clients/electrum_rpc/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ impl ElectrumClientImpl {

/// Create a new Electrum client instance.
/// This function initializes the connection manager and starts the connection process.
pub async fn try_new_arc(
pub fn try_new_arc(
client_settings: ElectrumClientSettings,
block_headers_storage: BlockHeaderStorage,
abortable_system: AbortableQueue,
Expand All @@ -157,7 +157,6 @@ impl ElectrumClientImpl {
client_impl
.connection_manager
.initialize(Arc::downgrade(&client_impl))
.await
.map_err(|e| e.to_string())?;

Ok(client_impl)
Expand Down Expand Up @@ -202,7 +201,7 @@ impl ElectrumClientImpl {
pub fn weak_spawner(&self) -> WeakSpawner { self.abortable_system.weak_spawner() }

#[cfg(test)]
pub async fn with_protocol_version(
pub fn with_protocol_version(
client_settings: ElectrumClientSettings,
block_headers_storage: BlockHeaderStorage,
abortable_system: AbortableQueue,
Expand All @@ -224,7 +223,6 @@ impl ElectrumClientImpl {
client_impl
.connection_manager
.initialize(Arc::downgrade(&client_impl))
.await
.map_err(|e| e.to_string())?;

Ok(client_impl)
Expand Down Expand Up @@ -271,23 +269,20 @@ impl JsonRpcMultiClient for ElectrumClient {

#[cfg_attr(test, mockable)]
impl ElectrumClient {
pub async fn try_new(
pub fn try_new(
client_settings: ElectrumClientSettings,
event_handlers: Vec<Box<SharableRpcTransportEventHandler>>,
block_headers_storage: BlockHeaderStorage,
abortable_system: AbortableQueue,
scripthash_notification_sender: Option<UnboundedSender<ScripthashNotification>>,
) -> Result<ElectrumClient, String> {
let client = ElectrumClient(
ElectrumClientImpl::try_new_arc(
client_settings,
block_headers_storage,
abortable_system,
event_handlers,
scripthash_notification_sender,
)
.await?,
);
let client = ElectrumClient(ElectrumClientImpl::try_new_arc(
client_settings,
block_headers_storage,
abortable_system,
event_handlers,
scripthash_notification_sender,
)?);

Ok(client)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ impl ConnectionManager {

/// Initializes the connection manager by connecting the electrum connections.
/// This must be called and only be called once to have a functioning connection manager.
pub async fn initialize(&self, weak_client: Weak<ElectrumClientImpl>) -> Result<(), ConnectionManagerErr> {
pub fn initialize(&self, weak_client: Weak<ElectrumClientImpl>) -> Result<(), ConnectionManagerErr> {
// Disallow reusing the same manager with another client.
if self.weak_client().read().unwrap().is_some() {
return Err(ConnectionManagerErr::AlreadyInitialized);
Expand All @@ -171,16 +171,6 @@ impl ConnectionManager {
electrum_client.weak_spawner().spawn(self.clone().ping_task());
}

// Tests fail right away if an electrum request fails (non-test builds retry endlessly).
// It will definitely fail if the connection manager is just initialized because the background
// task wouldn't have had enough time to establish at least one connection. So let's optimistically
// block until a connection is established.
#[cfg(test)]
self.wait_till_connected(5.)
.await
.map_err(|e| log!("Failed to connect to any electrum server: {e:?}"))
.ok();

Ok(())
}

Expand Down Expand Up @@ -521,25 +511,3 @@ impl ConnectionManager {
.and_then(|weak| weak.upgrade().map(ElectrumClient)) // None here = client was dropped.
}
}

// Test-only methods.
#[cfg(test)]
impl ConnectionManager {
async fn wait_till_connected(&self, timeout: f32) -> Result<(), String> {
use instant::Instant;

let start_time = Instant::now();
loop {
if !self.get_active_connections().await.is_empty() {
return Ok(());
}
Timer::sleep(0.5).await;
if start_time.elapsed().as_secs_f32() > timeout {
return Err(format!(
"Waited for {} seconds but the connection manager is still not connected",
timeout
));
}
}
}
}
1 change: 0 additions & 1 deletion mm2src/coins/utxo/utxo_builder/utxo_coin_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -616,7 +616,6 @@ pub trait UtxoCoinBuilderCommonOps {
abortable_system,
scripthash_notification_sender,
)
.await
.map_to_mm(UtxoCoinBuildError::Internal)
}

Expand Down
4 changes: 2 additions & 2 deletions mm2src/coins/utxo/utxo_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -477,13 +477,13 @@ fn test_wait_for_payment_spend_timeout_electrum() {
min_connected: 1,
max_connected: 1,
};
let client = block_on(ElectrumClient::try_new(
let client = ElectrumClient::try_new(
client_settings,
Default::default(),
block_headers_storage,
abortable_system,
None,
))
)
.expect("Expected electrum_client_impl constructed without a problem");
let client = UtxoRpcClientEnum::Electrum(client);
let coin = utxo_coin_for_test(client, None, false);
Expand Down

0 comments on commit b4bd969

Please sign in to comment.