From 21bf272f18c9e84ad8a91a993fc41a617c43b4ab Mon Sep 17 00:00:00 2001 From: buffalu <85544055+buffalu@users.noreply.github.com> Date: Tue, 30 May 2023 16:26:06 -0500 Subject: [PATCH] Backport #309 (#312) Co-authored-by: segfaultdoctor <17258903+segfaultdoc@users.noreply.github.com> --- Cargo.lock | 12 --- core/Cargo.toml | 1 - core/src/bundle_account_locker.rs | 5 +- core/src/bundle_sanitizer.rs | 107 ++++++++++++------------ core/src/bundle_stage.rs | 116 +++++++++++++++------------ core/src/packet_bundle.rs | 4 +- core/src/proxy/block_engine_stage.rs | 3 +- programs/bpf/Cargo.lock | 12 --- sdk/Cargo.toml | 2 - sdk/src/bundle/sanitized.rs | 15 +++- 10 files changed, 136 insertions(+), 141 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 825b6dc754..66a9a9c518 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5383,7 +5383,6 @@ dependencies = [ "tonic 0.5.2", "tonic-build 0.5.2", "trees", - "uuid", ] [[package]] @@ -6461,7 +6460,6 @@ dependencies = [ "thiserror", "tiny-bip39", "uriparse", - "uuid", "wasm-bindgen", ] @@ -8079,16 +8077,6 @@ version = "0.1.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7cf7d77f457ef8dfa11e4cd5933c5ddb5dc52a94664071951219a97710f0a32b" -[[package]] -name = "uuid" -version = "1.3.3" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "345444e32442451b267fc254ae85a209c64be56d2890e601a0c37ff0c3c5ecd2" -dependencies = [ - "getrandom 0.2.3", - "rand 0.8.5", -] - [[package]] name = "vcpkg" version = "0.2.15" diff --git a/core/Cargo.toml b/core/Cargo.toml index 1f20295bcc..ec2fae4536 100644 --- a/core/Cargo.toml +++ b/core/Cargo.toml @@ -79,7 +79,6 @@ tokio = { version = "~1.14.1", features = ["full"] } tokio-stream = "0.1.8" tonic = { version = "0.5.2", features = ["tls"] } trees = "0.4.2" -uuid = { version = "1.0.0", features = ["v4", "fast-rng"] } [dev-dependencies] matches = "0.1.9" diff --git a/core/src/bundle_account_locker.rs b/core/src/bundle_account_locker.rs index 955a4dd170..166ca5dd4c 100644 --- a/core/src/bundle_account_locker.rs +++ b/core/src/bundle_account_locker.rs @@ -234,7 +234,6 @@ mod tests { system_transaction::transfer, transaction::VersionedTransaction, }, std::{collections::HashSet, sync::Arc}, - uuid::Uuid, }; #[test] @@ -266,11 +265,11 @@ mod tests { let packet_bundle0 = PacketBundle { batch: PacketBatch::new(vec![Packet::from_data(None, &tx0).unwrap()]), - uuid: Uuid::new_v4(), + bundle_id: tx0.signatures[0].to_string(), }; let packet_bundle1 = PacketBundle { batch: PacketBatch::new(vec![Packet::from_data(None, &tx1).unwrap()]), - uuid: Uuid::new_v4(), + bundle_id: tx1.signatures[0].to_string(), }; let mut transaction_errors = TransactionErrorMetrics::default(); diff --git a/core/src/bundle_sanitizer.rs b/core/src/bundle_sanitizer.rs index 620646a071..c561146a34 100644 --- a/core/src/bundle_sanitizer.rs +++ b/core/src/bundle_sanitizer.rs @@ -122,7 +122,7 @@ pub fn get_sanitized_bundle( Ok(SanitizedBundle { transactions, - uuid: packet_bundle.uuid, + bundle_id: packet_bundle.bundle_id.clone(), }) } @@ -167,16 +167,16 @@ mod tests { transaction_error_metrics::TransactionErrorMetrics, }, solana_sdk::{ + bundle::sanitized::derive_bundle_id, hash::Hash, instruction::Instruction, packet::Packet, pubkey::Pubkey, signature::{Keypair, Signer}, system_transaction::transfer, - transaction::{SanitizedTransaction, Transaction, VersionedTransaction}, + transaction::{Transaction, VersionedTransaction}, }, std::{collections::HashSet, sync::Arc}, - uuid::Uuid, }; #[test] @@ -198,10 +198,12 @@ mod tests { genesis_config.hash(), )); let packet = Packet::from_data(None, &tx).unwrap(); + let tx_signature = tx.signatures[0]; + let bundle_id = derive_bundle_id(&[tx]); let packet_bundle = PacketBundle { batch: PacketBatch::new(vec![packet]), - uuid: Uuid::new_v4(), + bundle_id, }; let mut transaction_errors = TransactionErrorMetrics::default(); @@ -214,10 +216,7 @@ mod tests { ) .unwrap(); assert_eq!(sanitized_bundle.transactions.len(), 1); - assert_eq!( - sanitized_bundle.transactions[0].signature(), - &tx.signatures[0] - ); + assert_eq!(sanitized_bundle.transactions[0].signature(), &tx_signature); } #[test] @@ -239,10 +238,11 @@ mod tests { genesis_config.hash(), )); let packet = Packet::from_data(None, &tx).unwrap(); + let bundle_id = derive_bundle_id(&[tx]); let packet_bundle = PacketBundle { batch: PacketBatch::new(vec![packet]), - uuid: Uuid::new_v4(), + bundle_id, }; let consensus_accounts_cache = HashSet::from([kp.pubkey()]); @@ -276,11 +276,12 @@ mod tests { genesis_config.hash(), )); let packet = Packet::from_data(None, &tx).unwrap(); + let bundle_id = derive_bundle_id(&[tx]); // bundle with a duplicate transaction let packet_bundle = PacketBundle { batch: PacketBatch::new(vec![packet.clone(), packet]), - uuid: Uuid::new_v4(), + bundle_id, }; // fails to pop because bundle it locks the same transaction twice @@ -310,10 +311,11 @@ mod tests { let tx = VersionedTransaction::from(transfer(&mint_keypair, &kp.pubkey(), 1, Hash::default())); let packet = Packet::from_data(None, &tx).unwrap(); + let bundle_id = derive_bundle_id(&[tx]); let packet_bundle = PacketBundle { batch: PacketBatch::new(vec![packet.clone(), packet]), - uuid: Uuid::new_v4(), + bundle_id, }; // fails to pop because bundle has bad blockhash @@ -347,10 +349,11 @@ mod tests { genesis_config.hash(), )); let packet = Packet::from_data(None, &tx).unwrap(); + let bundle_id = derive_bundle_id(&[tx]); let packet_bundle = PacketBundle { batch: PacketBatch::new(vec![packet.clone()]), - uuid: Uuid::new_v4(), + bundle_id: bundle_id.clone(), }; let mut transaction_errors = TransactionErrorMetrics::default(); @@ -376,7 +379,7 @@ mod tests { // try to process the same one again shall fail let packet_bundle = PacketBundle { batch: PacketBatch::new(vec![packet]), - uuid: Uuid::new_v4(), + bundle_id, }; assert!(get_sanitized_bundle( @@ -406,24 +409,23 @@ mod tests { }); let kp = Keypair::new(); - let tx = - SanitizedTransaction::try_from_legacy_transaction(Transaction::new_signed_with_payer( - &[Instruction::new_with_bytes( - tip_manager.tip_payment_program_id(), - &[0], - vec![], - )], - Some(&kp.pubkey()), - &[&kp], - genesis_config.hash(), - )) - .unwrap(); - - let packet = Packet::from_data(None, &tx.to_versioned_transaction()).unwrap(); + let tx = VersionedTransaction::from(Transaction::new_signed_with_payer( + &[Instruction::new_with_bytes( + tip_manager.tip_payment_program_id(), + &[0], + vec![], + )], + Some(&kp.pubkey()), + &[&kp], + genesis_config.hash(), + )); + tx.sanitize(false).unwrap(); + let packet = Packet::from_data(None, &tx).unwrap(); + let bundle_id = derive_bundle_id(&[tx]); let packet_bundle = PacketBundle { batch: PacketBatch::new(vec![packet]), - uuid: Uuid::new_v4(), + bundle_id, }; // fails to pop because bundle mentions tip program @@ -445,20 +447,19 @@ mod tests { let bank = Arc::new(Bank::new_no_wallclock_throttle_for_tests(&genesis_config)); let kp = Keypair::new(); - let tx = - SanitizedTransaction::try_from_legacy_transaction(Transaction::new_signed_with_payer( - &[create_lookup_table(kp.pubkey(), kp.pubkey(), bank.slot()).0], - Some(&kp.pubkey()), - &[&kp], - genesis_config.hash(), - )) - .unwrap(); - - let packet = Packet::from_data(None, &tx.to_versioned_transaction()).unwrap(); + let tx = VersionedTransaction::from(Transaction::new_signed_with_payer( + &[create_lookup_table(kp.pubkey(), kp.pubkey(), bank.slot()).0], + Some(&kp.pubkey()), + &[&kp], + genesis_config.hash(), + )); + tx.sanitize(false).unwrap(); + let packet = Packet::from_data(None, &tx).unwrap(); + let bundle_id = derive_bundle_id(&[tx]); let packet_bundle = PacketBundle { batch: PacketBatch::new(vec![packet]), - uuid: Uuid::new_v4(), + bundle_id, }; let mut transaction_errors = TransactionErrorMetrics::default(); @@ -480,7 +481,7 @@ mod tests { let packet_bundle = PacketBundle { batch: PacketBatch::new(vec![]), - uuid: Uuid::new_v4(), + bundle_id: String::default(), }; // fails to pop because empty bundle let mut transaction_errors = TransactionErrorMetrics::default(); @@ -506,18 +507,20 @@ mod tests { let kp = Keypair::new(); - let packets = (0..MAX_PACKETS_PER_BUNDLE + 1).map(|i| { - let tx = VersionedTransaction::from(transfer( - &mint_keypair, - &kp.pubkey(), - i as u64, - genesis_config.hash(), - )); - Packet::from_data(None, &tx).unwrap() - }); + let txs = (0..MAX_PACKETS_PER_BUNDLE + 1) + .map(|i| { + VersionedTransaction::from(transfer( + &mint_keypair, + &kp.pubkey(), + i as u64, + genesis_config.hash(), + )) + }) + .collect::>(); + let packets = txs.iter().map(|tx| Packet::from_data(None, tx).unwrap()); let packet_bundle = PacketBundle { batch: PacketBatch::new(packets.collect()), - uuid: Uuid::new_v4(), + bundle_id: derive_bundle_id(&txs), }; // fails to pop because too many packets in a bundle let mut transaction_errors = TransactionErrorMetrics::default(); @@ -554,7 +557,7 @@ mod tests { let packet_bundle = PacketBundle { batch: PacketBatch::new(vec![packet]), - uuid: Uuid::new_v4(), + bundle_id: derive_bundle_id(&[tx]), }; // fails to pop because one of the packets is marked as discard @@ -599,7 +602,7 @@ mod tests { let packet_bundle = PacketBundle { batch: PacketBatch::new(vec![packet]), - uuid: Uuid::new_v4(), + bundle_id: derive_bundle_id(&[tx]), }; let mut transaction_errors = TransactionErrorMetrics::default(); assert!(get_sanitized_bundle( diff --git a/core/src/bundle_stage.rs b/core/src/bundle_stage.rs index 86861d1df9..3b709984bc 100644 --- a/core/src/bundle_stage.rs +++ b/core/src/bundle_stage.rs @@ -63,7 +63,6 @@ use { thread::{self, Builder, JoinHandle}, time::{Duration, Instant}, }, - uuid::Uuid, }; const MAX_BUNDLE_RETRY_DURATION: Duration = Duration::from_millis(10); @@ -406,8 +405,8 @@ impl BundleStage { &bank_start.working_bank, ); warn!( - "bundle dropped, qos rate limit. uuid: {} bundle_cost: {}, block_cost: {}", - sanitized_bundle.uuid, + "bundle dropped, qos rate limit. bundle_id: {} bundle_cost: {}, block_cost: {}", + sanitized_bundle.bundle_id, tx_costs.iter().map(|c| c.sum()).sum::(), &bank_start .working_bank @@ -1162,7 +1161,7 @@ impl BundleStage { tip_manager, cluster_info, )?, - uuid: Uuid::default(), + bundle_id: String::default(), }; if !initialize_tip_accounts_bundle.transactions.is_empty() { debug!("initialize tip account"); @@ -1244,7 +1243,7 @@ impl BundleStage { let change_tip_receiver_bundle = SanitizedBundle { transactions: vec![change_tip_receiver_tx], - uuid: Uuid::default(), + bundle_id: String::default(), }; let locked_change_tip_receiver_bundle = bundle_account_locker .prepare_locked_bundle(&change_tip_receiver_bundle, &bank_start.working_bank) @@ -1689,8 +1688,11 @@ mod tests { solana_perf::packet::PacketBatch, solana_poh::poh_recorder::create_test_recorder, solana_sdk::{ - bundle::error::BundleExecutionError::{ - ExceedsCostModel, PohMaxHeightError, TransactionFailure, + bundle::{ + error::BundleExecutionError::{ + ExceedsCostModel, PohMaxHeightError, TransactionFailure, + }, + sanitized::derive_bundle_id, }, compute_budget::ComputeBudgetInstruction, genesis_config::GenesisConfig, @@ -1707,7 +1709,6 @@ mod tests { }, }, std::{collections::HashSet, sync::atomic::Ordering}, - uuid::Uuid, }; const TEST_MAX_RETRY_DURATION: Duration = Duration::from_millis(500); @@ -1871,14 +1872,19 @@ mod tests { let ix_mint_a = system_instruction::transfer(&mint_keypair.pubkey(), &kp_a.pubkey(), 1); let ix_mint_b = system_instruction::transfer(&mint_keypair.pubkey(), &kp_b.pubkey(), 1); let message = Message::new(&[ix_mint_a, ix_mint_b], Some(&mint_keypair.pubkey())); - let tx = Transaction::new(&[&mint_keypair], message, genesis_config.hash()); - let packet = Packet::from_data(None, tx).unwrap(); + let tx = VersionedTransaction::from(Transaction::new( + &[&mint_keypair], + message, + genesis_config.hash(), + )); + let packet = Packet::from_data(None, &tx).unwrap(); + let bundle_id = derive_bundle_id(&[tx]); ( genesis_config, PacketBundle { batch: PacketBatch::new(vec![packet]), - uuid: Uuid::new_v4(), + bundle_id, }, ) } @@ -1900,12 +1906,16 @@ mod tests { ], Some(&mint_keypair.pubkey()), ); - let tx = Transaction::new(&[&mint_keypair], message, genesis_config.hash()); - let packet = Packet::from_data(None, tx).unwrap(); + let tx = VersionedTransaction::from(Transaction::new( + &[&mint_keypair], + message, + genesis_config.hash(), + )); + let packet = Packet::from_data(None, &tx).unwrap(); let bundle = PacketBundle { batch: PacketBatch::new(vec![packet]), - uuid: Uuid::new_v4(), + bundle_id: derive_bundle_id(&[tx]), }; assert_eq!( test_single_bundle(genesis_config, bundle, Some(vec![LowComputeBudget])), @@ -1924,21 +1934,18 @@ mod tests { let kp_a = Keypair::new(); let kp_nonce = Keypair::new(); let kp_nonce_authority = Keypair::new(); - let packet = Packet::from_data( - None, - system_transaction::nonced_transfer( - &mint_keypair, - &kp_a.pubkey(), - 1, - &kp_nonce.pubkey(), - &kp_nonce_authority, - genesis_config.hash(), - ), - ) - .unwrap(); + let tx = VersionedTransaction::from(system_transaction::nonced_transfer( + &mint_keypair, + &kp_a.pubkey(), + 1, + &kp_nonce.pubkey(), + &kp_nonce_authority, + genesis_config.hash(), + )); + let packet = Packet::from_data(None, &tx).unwrap(); let bundle = PacketBundle { batch: PacketBatch::new(vec![packet]), - uuid: Uuid::new_v4(), + bundle_id: derive_bundle_id(&[tx]), }; assert_eq!( @@ -1960,19 +1967,22 @@ mod tests { let kp_a = Keypair::new(); let kp_b = Keypair::new(); - let successful_packet = Packet::from_data( - None, - system_transaction::transfer(&mint_keypair, &kp_b.pubkey(), 1, genesis_config.hash()), - ) - .unwrap(); - let failed_packet = Packet::from_data( - None, - system_transaction::transfer(&kp_a, &kp_b.pubkey(), 1, genesis_config.hash()), - ) - .unwrap(); + + let successful_tx = VersionedTransaction::from(transfer( + &mint_keypair, + &kp_b.pubkey(), + 1, + genesis_config.hash(), + )); + let failed_tx = + VersionedTransaction::from(transfer(&kp_a, &kp_b.pubkey(), 1, genesis_config.hash())); + + let successful_packet = Packet::from_data(None, &successful_tx).unwrap(); + let failed_packet = Packet::from_data(None, &failed_tx).unwrap(); + let bundle = PacketBundle { batch: PacketBatch::new(vec![successful_packet, failed_packet]), - uuid: Uuid::new_v4(), + bundle_id: derive_bundle_id(&[successful_tx, failed_tx]), }; assert_eq!( @@ -1991,14 +2001,12 @@ mod tests { let kp_a = Keypair::new(); let kp_b = Keypair::new(); - let packet = Packet::from_data( - None, - system_transaction::transfer(&kp_a, &kp_b.pubkey(), 1, genesis_config.hash()), - ) - .unwrap(); + let tx = + VersionedTransaction::from(transfer(&kp_a, &kp_b.pubkey(), 1, genesis_config.hash())); + let packet = Packet::from_data(None, &tx).unwrap(); let bundle = PacketBundle { batch: PacketBatch::new(vec![packet]), - uuid: Uuid::new_v4(), + bundle_id: derive_bundle_id(&[tx]), }; assert_eq!( @@ -2018,14 +2026,16 @@ mod tests { genesis_config.ticks_per_slot = 1; // Reduce ticks so that POH fails let kp_b = Keypair::new(); - let packet = Packet::from_data( - None, - system_transaction::transfer(&mint_keypair, &kp_b.pubkey(), 1, genesis_config.hash()), - ) - .unwrap(); + let tx = VersionedTransaction::from(transfer( + &mint_keypair, + &kp_b.pubkey(), + 1, + genesis_config.hash(), + )); + let packet = Packet::from_data(None, &tx).unwrap(); let bundle = PacketBundle { batch: PacketBatch::new(vec![packet]), - uuid: Uuid::new_v4(), + bundle_id: derive_bundle_id(&[tx]), }; assert_eq!( test_single_bundle(genesis_config, bundle, None), @@ -2093,10 +2103,10 @@ mod tests { // push and pop tx0 let bundle = PacketBundle { - batch: PacketBatch::new(vec![Packet::from_data(None, tx0).unwrap()]), - uuid: Uuid::new_v4(), + batch: PacketBatch::new(vec![Packet::from_data(None, &tx0).unwrap()]), + bundle_id: derive_bundle_id(&[tx0]), }; - info!("test_bundle_max_retries uuid: {:?}", bundle.uuid); + info!("test_bundle_max_retries uuid: {:?}", bundle.bundle_id); let sanitized_bundle = get_sanitized_bundle( &bundle, diff --git a/core/src/packet_bundle.rs b/core/src/packet_bundle.rs index f5a6a7ac6d..2158f37414 100644 --- a/core/src/packet_bundle.rs +++ b/core/src/packet_bundle.rs @@ -1,7 +1,7 @@ -use {solana_perf::packet::PacketBatch, uuid::Uuid}; +use solana_perf::packet::PacketBatch; #[derive(Clone, Debug)] pub struct PacketBundle { pub batch: PacketBatch, - pub uuid: Uuid, + pub bundle_id: String, } diff --git a/core/src/proxy/block_engine_stage.rs b/core/src/proxy/block_engine_stage.rs index 5a1067b2a0..1bea7c6ad4 100644 --- a/core/src/proxy/block_engine_stage.rs +++ b/core/src/proxy/block_engine_stage.rs @@ -41,7 +41,6 @@ use { transport::{Channel, Endpoint}, Status, Streaming, }, - uuid::Uuid, }; const CONNECTION_TIMEOUT_S: u64 = 10; @@ -478,7 +477,7 @@ impl BlockEngineStage { .map(proto_packet_to_packet) .collect(), ), - uuid: Uuid::from_str(&bundle.uuid).ok()?, + bundle_id: bundle.uuid, }) }) .collect(); diff --git a/programs/bpf/Cargo.lock b/programs/bpf/Cargo.lock index 83964b3403..519611a9e1 100644 --- a/programs/bpf/Cargo.lock +++ b/programs/bpf/Cargo.lock @@ -5096,7 +5096,6 @@ dependencies = [ "tonic 0.5.2", "tonic-build 0.5.2", "trees", - "uuid", ] [[package]] @@ -5870,7 +5869,6 @@ dependencies = [ "solana-sdk-macro 1.14.19", "thiserror", "uriparse", - "uuid", "wasm-bindgen", ] @@ -7278,16 +7276,6 @@ version = "0.7.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "05e42f7c18b8f902290b009cde6d651262f956c98bc51bca4cd1d511c9cd85c7" -[[package]] -name = "uuid" -version = "1.3.3" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "345444e32442451b267fc254ae85a209c64be56d2890e601a0c37ff0c3c5ecd2" -dependencies = [ - "getrandom 0.2.4", - "rand 0.8.5", -] - [[package]] name = "valuable" version = "0.1.0" diff --git a/sdk/Cargo.toml b/sdk/Cargo.toml index e13f73d12d..ba43e7a496 100644 --- a/sdk/Cargo.toml +++ b/sdk/Cargo.toml @@ -34,7 +34,6 @@ full = [ "libsecp256k1", "sha3", "digest", - "uuid" ] [dependencies] @@ -80,7 +79,6 @@ solana-program = { path = "program", version = "=1.14.19" } solana-sdk-macro = { path = "macro", version = "=1.14.19" } thiserror = "1.0" uriparse = "0.6.4" -uuid = { version = "1.0.0", features = ["v4", "fast-rng"], optional = true } wasm-bindgen = "0.2" [target.'cfg(target_arch = "wasm32")'.dependencies] diff --git a/sdk/src/bundle/sanitized.rs b/sdk/src/bundle/sanitized.rs index b0133a40a7..3c4ec77cef 100644 --- a/sdk/src/bundle/sanitized.rs +++ b/sdk/src/bundle/sanitized.rs @@ -1,9 +1,20 @@ #![cfg(feature = "full")] -use {solana_sdk::transaction::SanitizedTransaction, uuid::Uuid}; +use { + crate::transaction::VersionedTransaction, + itertools::Itertools, + sha2::{Digest, Sha256}, + solana_sdk::transaction::SanitizedTransaction, +}; #[derive(Clone, Debug)] pub struct SanitizedBundle { pub transactions: Vec, - pub uuid: Uuid, + pub bundle_id: String, +} + +pub fn derive_bundle_id(transactions: &[VersionedTransaction]) -> String { + let mut hasher = Sha256::new(); + hasher.update(transactions.iter().map(|tx| tx.signatures[0]).join(",")); + format!("{:x}", hasher.finalize()) }