Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bench tps tx account limits #32281

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

2 changes: 2 additions & 0 deletions bench-tps/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,13 @@ edition = { workspace = true }
[dependencies]
clap = { workspace = true }
crossbeam-channel = { workspace = true }
itertools = { workspace = true }
log = { workspace = true }
rand = { workspace = true }
rayon = { workspace = true }
serde_json = { workspace = true }
serde_yaml = { workspace = true }
solana-address-lookup-table-program = { workspace = true }
solana-clap-utils = { workspace = true }
solana-cli-config = { workspace = true }
solana-client = { workspace = true }
Expand Down
118 changes: 118 additions & 0 deletions bench-tps/src/address_table_lookup.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,118 @@
use {
crate::bench_tps_client::*,
itertools::Itertools,
log::*,
solana_address_lookup_table_program::{
instruction::{create_lookup_table, extend_lookup_table},
state::AddressLookupTable,
},
solana_sdk::{
commitment_config::CommitmentConfig,
hash::Hash,
pubkey::Pubkey,
signature::{Keypair, Signer},
slot_history::Slot,
transaction::Transaction,
},
std::{sync::Arc, thread::sleep, time::Duration},
};

/// To create a lookup-table account via `client`, and extend it with `num_addresses`.
pub fn create_address_lookup_table_account<T: 'static + BenchTpsClient + Send + Sync + ?Sized>(
client: Arc<T>,
funding_key: &Keypair,
num_addresses: usize,
) -> Result<Pubkey> {
let (transaction, lookup_table_address) = build_create_lookup_table_tx(
funding_key,
client.get_slot().unwrap_or(0),
client.get_latest_blockhash().unwrap(),
);
send_and_confirm_transaction(client.clone(), transaction, &lookup_table_address);
tao-stones marked this conversation as resolved.
Show resolved Hide resolved

// Due to legacy transaction's size limits, the number of addresses to be added by one
// `extend_lookup_table` transaction is also limited. If `num_addresses` is greater
// than NUMBER_OF_ADDRESSES_PER_EXTEND, multiple extending are required.
const NUMBER_OF_ADDRESSES_PER_EXTEND: usize = 16;

let mut i: usize = 0;
while i < num_addresses {
let extend_num_addresses = NUMBER_OF_ADDRESSES_PER_EXTEND.min(num_addresses - i);
i += extend_num_addresses;

let transaction = build_extend_lookup_table_tx(
&lookup_table_address,
funding_key,
extend_num_addresses,
client.get_latest_blockhash().unwrap(),
);
send_and_confirm_transaction(client.clone(), transaction, &lookup_table_address);
}

Ok(lookup_table_address)
}

fn build_create_lookup_table_tx(
funding_key: &Keypair,
recent_slot: Slot,
recent_blockhash: Hash,
) -> (Transaction, Pubkey) {
let (create_lookup_table_ix, lookup_table_address) = create_lookup_table(
funding_key.pubkey(), // authority
funding_key.pubkey(), // payer
recent_slot, // slot
);

(
Transaction::new_signed_with_payer(
&[create_lookup_table_ix],
Some(&funding_key.pubkey()),
&[funding_key],
recent_blockhash,
),
lookup_table_address,
)
}

fn build_extend_lookup_table_tx(
lookup_table_address: &Pubkey,
funding_key: &Keypair,
num_addresses: usize,
recent_blockhash: Hash,
) -> Transaction {
// generates random addresses to populate lookup table account
let addresses = (0..num_addresses)
.map(|_| Pubkey::new_unique())
.collect_vec();
let extend_lookup_table_ix = extend_lookup_table(
*lookup_table_address,
funding_key.pubkey(), // authority
Some(funding_key.pubkey()), // payer
addresses,
);

Transaction::new_signed_with_payer(
&[extend_lookup_table_ix],
Some(&funding_key.pubkey()),
&[funding_key],
recent_blockhash,
)
}

fn send_and_confirm_transaction<T: 'static + BenchTpsClient + Send + Sync + ?Sized>(
client: Arc<T>,
transaction: Transaction,
lookup_table_address: &Pubkey,
) {
let _tx_sig = client.send_transaction(transaction).unwrap();
Copy link
Contributor

Choose a reason for hiding this comment

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

I feel like this might fail often enough if we use testnet or similar. In this case we need to implement some error handling strategy (try again for example)


// Sleep a few slots to allow transactions to process
sleep(Duration::from_secs(1));
Copy link
Contributor

Choose a reason for hiding this comment

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

there's gotta be a better way than this, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

perhaps. Didn't feel this is more important than funding txs where it also sleeps for a sec between sending batch of transactions and verifying them (send_batch.rs: Ln227). Just just copied it ;)

Can use a timeout loop to call get_account_with_commiment() with ms sleep in between to reduce overall wait time. Don't fee that makes meaningful difference tho.


// confirm tx
let lookup_table_account = client
Copy link
Contributor

Choose a reason for hiding this comment

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

If we believe that verifying that accounts has been added properly is necessary, I would do this on the level up: request accounts from ALT and check that at least the number of accounts as expected.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The original intention is to first confirm the ALT account was created before sending extend transactions, not so much about the number of accounts were added to the ATL. so it just did account lookup, which is sufficient imo.

.get_account_with_commitment(lookup_table_address, CommitmentConfig::processed())
.unwrap();
let lookup_table = AddressLookupTable::deserialize(&lookup_table_account.data).unwrap();
Copy link
Contributor

Choose a reason for hiding this comment

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

I have a mixed feeling about that part, I think it would be more efficient to extend ALTs and after that do the check in a separate loop. Another thing is that the check requires trace and also using unwrap. My experience with testnet is that all transactions from time to time fail or go to the block much later than has been sent.

trace!("lookup table: {:?}", lookup_table);
}
Loading