Skip to content

Commit

Permalink
Make BanksClient timeout after 10 seconds instead of 60 (solana-labs#…
Browse files Browse the repository at this point in the history
…30904)

* don't increase the deadline by 50 seconds

* add test demonstrating a timeout panic

* add back missing Duration import for tests

* cargo fmt

* clarify test comments

* clarify comment

* make timeout test finish in 1 second

* sort dependencies
  • Loading branch information
kevinheavey authored Mar 28, 2023
1 parent 8e910b4 commit 6f73aaf
Show file tree
Hide file tree
Showing 4 changed files with 108 additions and 8 deletions.
2 changes: 2 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

16 changes: 8 additions & 8 deletions banks-client/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ use {
serde_transport::tcp,
ClientMessage, Response, Transport,
},
tokio::{net::ToSocketAddrs, time::Duration},
tokio::net::ToSocketAddrs,
tokio_serde::formats::Bincode,
};

Expand Down Expand Up @@ -234,8 +234,7 @@ impl BanksClient {
transaction: Transaction,
commitment: CommitmentLevel,
) -> impl Future<Output = Result<(), BanksClientError>> + '_ {
let mut ctx = context::current();
ctx.deadline += Duration::from_secs(50);
let ctx = context::current();
self.process_transaction_with_commitment_and_context(ctx, transaction, commitment)
.map(|result| match result? {
None => Err(BanksClientError::ClientError(
Expand All @@ -251,8 +250,7 @@ impl BanksClient {
transaction: impl Into<VersionedTransaction>,
) -> impl Future<Output = Result<BanksTransactionResultWithMetadata, BanksClientError>> + '_
{
let mut ctx = context::current();
ctx.deadline += Duration::from_secs(50);
let ctx = context::current();
self.process_transaction_with_metadata_and_context(ctx, transaction.into())
}

Expand All @@ -263,8 +261,7 @@ impl BanksClient {
transaction: Transaction,
commitment: CommitmentLevel,
) -> impl Future<Output = Result<(), BanksClientError>> + '_ {
let mut ctx = context::current();
ctx.deadline += Duration::from_secs(50);
let ctx = context::current();
self.process_transaction_with_preflight_and_commitment_and_context(
ctx,
transaction,
Expand Down Expand Up @@ -540,7 +537,10 @@ mod tests {
solana_sdk::{message::Message, signature::Signer, system_instruction},
std::sync::{Arc, RwLock},
tarpc::transport,
tokio::{runtime::Runtime, time::sleep},
tokio::{
runtime::Runtime,
time::{sleep, Duration},
},
};

#[test]
Expand Down
2 changes: 2 additions & 0 deletions program-test/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -29,4 +29,6 @@ thiserror = { workspace = true }
tokio = { workspace = true, features = ["full"] }

[dev-dependencies]
futures = { workspace = true }
solana-stake-program = { workspace = true }
tarpc = { workspace = true }
96 changes: 96 additions & 0 deletions program-test/tests/timeout.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
use {
futures::future::{join_all, Future, FutureExt},
solana_banks_client::{BanksClient, BanksClientError},
solana_program_test::ProgramTest,
solana_sdk::{
commitment_config::CommitmentLevel, pubkey::Pubkey, signature::Signer,
transaction::Transaction,
},
std::time::{Duration, SystemTime},
tarpc::context::current,
};

fn one_second_from_now() -> SystemTime {
SystemTime::now() + Duration::from_secs(1)
}

// like the BanksClient method of the same name,
// but uses a context with a one-second deadline
fn process_transaction_with_commitment(
client: &mut BanksClient,
transaction: Transaction,
commitment: CommitmentLevel,
) -> impl Future<Output = Result<(), BanksClientError>> + '_ {
let mut ctx = current();
ctx.deadline = one_second_from_now();
client
.process_transaction_with_commitment_and_context(ctx, transaction, commitment)
.map(|result| match result? {
None => Err(BanksClientError::ClientError(
"invalid blockhash or fee-payer",
)),
Some(transaction_result) => Ok(transaction_result?),
})
}

// like the BanksClient method of the same name,
// but uses a context with a one-second deadline
async fn process_transactions_with_commitment(
client: &mut BanksClient,
transactions: Vec<Transaction>,
commitment: CommitmentLevel,
) -> Result<(), BanksClientError> {
let mut clients: Vec<_> = transactions.iter().map(|_| client.clone()).collect();
let futures = clients
.iter_mut()
.zip(transactions)
.map(|(client, transaction)| {
process_transaction_with_commitment(client, transaction, commitment)
});
let statuses = join_all(futures).await;
statuses.into_iter().collect() // Convert Vec<Result<_, _>> to Result<Vec<_>>
}

#[should_panic(expected = "RpcError(DeadlineExceeded")]
#[tokio::test]
async fn timeout() {
// example of a test that hangs. The reasons for this hanging are a separate issue:
// https://github.com/solana-labs/solana/issues/30527).
// We just want to observe the timeout behaviour here.
let mut pt = ProgramTest::default();
pt.prefer_bpf(true);
let context = pt.start_with_context().await;
let mut client = context.banks_client;
let payer = context.payer;
let receiver = Pubkey::new_unique();
// If you set num_txs to 1, the process_transactions call never hangs.
// If you set it to 2 it sometimes hangs.
// If you set it to 10 it seems to always hang.
// Based on the logs this test usually hangs after processing 3 or 4 transactions
let num_txs = 10;
let num_txs_u64 = num_txs as u64;
let transfer_lamports_base = 1_000_000u64;
let mut txs: Vec<Transaction> = Vec::with_capacity(num_txs);
for i in 0..num_txs {
let ixs = [solana_sdk::system_instruction::transfer(
&payer.pubkey(),
&receiver,
transfer_lamports_base + i as u64, // deduping the tx
)];
let msg = solana_sdk::message::Message::new_with_blockhash(
&ixs,
Some(&payer.pubkey()),
&context.last_blockhash,
);
let tx = Transaction::new(&[&payer], msg, context.last_blockhash);
txs.push(tx);
}
process_transactions_with_commitment(&mut client, txs, CommitmentLevel::default())
.await
.unwrap();
let balance_after = client.get_balance(receiver).await.unwrap();
assert_eq!(
balance_after,
num_txs_u64 * transfer_lamports_base + ((num_txs_u64 - 1) * num_txs_u64) / 2
);
}

0 comments on commit 6f73aaf

Please sign in to comment.