From 838b049b83057e143d40cefa2712e8d4a70f3ccf Mon Sep 17 00:00:00 2001 From: rooooooooob Date: Mon, 13 Dec 2021 02:17:54 +0300 Subject: [PATCH] Remove all cryptoxide calls from fake_full_tx() (#286) Profiling the tx builder showed that even after we removed the fake_key_root construction in #214 due to terrible performance in asm.js (#213) that the cryptoxide calls within `fake_full_tx()` were up a ridiculous amount of the runtime. I investigated this as we had a report that it was taking up to hundreds of milliseconds just to make a tx and I wanted to see if there would be any significant improvement by migrating to an idiomatic rust API as discussed in #276 but from a purely performance perspective in a release build (opt-level=3) this seemed to be pretty minor, even with the unnecessary cloning forced by this inside of the implementation. This will need to be slightly modified once #273 gets merged as there are some conflicts. Previous perf results from building 100,000 simple txs and serializing them: ``` - 53.64% [.] <&cryptoxide::curve25519::Fe as core::ops::arith::Mul>::mul - 53.63% 0xffffffffffffffff + 23.03% cardano_serialization_lib::tx_builder::min_fee - 21.32% cardano_serialization_lib::tx_builder::TransactionBuilder::add_change_if_needed - 15.21% cardano_serialization_lib::tx_builder::TransactionBuilder::fee_for_output cardano_serialization_lib::tx_builder::min_fee + cardano_serialization_lib::tx_builder::fake_full_tx + 6.11% cardano_serialization_lib::tx_builder::TransactionBuilder::min_fee (inlined) - 7.78% ser_lib_perf::main + 6.11% cardano_serialization_lib::tx_builder::TransactionBuilder::build + 1.67% cardano_serialization_lib::tx_builder::TransactionBuilder::add_change_if_needed + 1.46% std::rt::lang_start_internal - 19.40% [.] cryptoxide::curve25519::Fe::square - 0xffffffffffffffff - 8.34% cardano_serialization_lib::tx_builder::min_fee cardano_serialization_lib::tx_builder::TransactionBuilder::build cardano_serialization_lib::tx_builder::TransactionBuilder::build_and_size + cardano_serialization_lib::tx_builder::fake_full_tx - 7.41% cardano_serialization_lib::tx_builder::TransactionBuilder::add_change_if_needed + 5.58% cardano_serialization_lib::tx_builder::TransactionBuilder::fee_for_output + 1.83% cardano_serialization_lib::tx_builder::TransactionBuilder::min_fee (inlined) + 2.77% ser_lib_perf::main + 0.88% std::rt::lang_start_internal + 7.80% [.] cryptoxide::curve25519::GePrecomp::maybe_set + 5.13% [.] cryptoxide::curve25519::Fe::invert + 3.98% [.] <&cryptoxide::curve25519::GeP3 as core::ops::arith::Add<&cryptoxide::curve25519::GePrecomp>>::add + 2.84% [.] cryptoxide::curve25519::GePrecomp::select + 1.22% [.] cryptoxide::curve25519::ge_scalarmult_base + 1.00% [.] cryptoxide::sha2::impl512::reference::digest_block_u64 0.47% [.] cryptoxide::curve25519::GeP2::dbl 0.13% [.] cryptoxide::curve25519::sc_muladd 0.12% [.] cryptoxide::curve25519::Fe::to_bytes 0.12% [.] cryptoxide::curve25519::sc_reduce 0.10% [.] cbor_event::se::Serializer::write_type 0.07% [.] cardano_serialization_lib::tx_builder::fake_full_tx ``` which shows that almost the entire runtime was spent on cryptoxide calls which were only necessary for `fake_full_tx()`. Running it again afterwards there is no overwhelming bottleneck anymore and the remaining runtime was fairly distributed. (cherry picked from commit e0b33c6dc2f15b9383627924c0f12b850864d37b) --- rust/src/tx_builder.rs | 52 ++++++++++++++++++++++++++++++------------ 1 file changed, 38 insertions(+), 14 deletions(-) diff --git a/rust/src/tx_builder.rs b/rust/src/tx_builder.rs index 88439599..9c69f709 100644 --- a/rust/src/tx_builder.rs +++ b/rust/src/tx_builder.rs @@ -37,28 +37,52 @@ fn witness_keys_for_cert(cert_enum: &Certificate, keys: &mut BTreeSet Bip32PrivateKey { + Bip32PrivateKey::from_bytes( + &[0xb8, 0xf2, 0xbe, 0xce, 0x9b, 0xdf, 0xe2, 0xb0, 0x28, 0x2f, 0x5b, 0xad, 0x70, 0x55, 0x62, 0xac, 0x99, 0x6e, 0xfb, 0x6a, 0xf9, 0x6b, 0x64, 0x8f, + 0x44, 0x45, 0xec, 0x44, 0xf4, 0x7a, 0xd9, 0x5c, 0x10, 0xe3, 0xd7, 0x2f, 0x26, 0xed, 0x07, 0x54, 0x22, 0xa3, 0x6e, 0xd8, 0x58, 0x5c, 0x74, 0x5a, + 0x0e, 0x11, 0x50, 0xbc, 0xce, 0xba, 0x23, 0x57, 0xd0, 0x58, 0x63, 0x69, 0x91, 0xf3, 0x8a, 0x37, 0x91, 0xe2, 0x48, 0xde, 0x50, 0x9c, 0x07, 0x0d, + 0x81, 0x2a, 0xb2, 0xfd, 0xa5, 0x78, 0x60, 0xac, 0x87, 0x6b, 0xc4, 0x89, 0x19, 0x2c, 0x1e, 0xf4, 0xce, 0x25, 0x3c, 0x19, 0x7e, 0xe2, 0x19, 0xa4] + ).unwrap() +} + +fn fake_key_hash() -> Ed25519KeyHash { + Ed25519KeyHash::from( + [142, 239, 181, 120, 142, 135, 19, 200, 68, 223, 211, 43, 46, 145, 222, 30, 48, 159, 239, 255, 213, 85, 248, 39, 204, 158, 225, 100] + ) +} + +fn fake_raw_key_sig() -> Ed25519Signature { + Ed25519Signature::from_bytes( + vec![36, 248, 153, 211, 155, 23, 253, 93, 102, 193, 146, 196, 181, 13, 52, 62, 66, 247, 35, 91, 48, 80, 76, 138, 231, 97, 159, 147, 200, 40, 220, 109, 206, 69, 104, 221, 105, 23, 124, 85, 24, 40, 73, 45, 119, 122, 103, 39, 253, 102, 194, 251, 204, 189, 168, 194, 174, 237, 146, 3, 44, 153, 121, 10] + ).unwrap() +} + +fn fake_raw_key_public() -> PublicKey { + PublicKey::from_bytes( + &[207, 118, 57, 154, 33, 13, 232, 114, 14, 159, 168, 148, 228, 94, 65, 226, 154, 181, 37, 227, 11, 196, 2, 128, 28, 7, 98, 80, 209, 88, 91, 205] + ).unwrap() +} + + // tx_body must be the result of building from tx_builder // constructs the rest of the Transaction using fake witness data of the correct length // for use in calculating the size of the final Transaction fn fake_full_tx(tx_builder: &TransactionBuilder, body: TransactionBody) -> Result { - // bytes for Bip32: art forum devote street sure rather head chuckle guard poverty release quote oak craft enemy - let fake_key_root = Bip32PrivateKey::from_bytes( - &[0xb8, 0xf2, 0xbe, 0xce, 0x9b, 0xdf, 0xe2, 0xb0, 0x28, 0x2f, 0x5b, 0xad, 0x70, 0x55, 0x62, 0xac, 0x99, 0x6e, 0xfb, 0x6a, 0xf9, 0x6b, 0x64, 0x8f, - 0x44, 0x45, 0xec, 0x44, 0xf4, 0x7a, 0xd9, 0x5c, 0x10, 0xe3, 0xd7, 0x2f, 0x26, 0xed, 0x07, 0x54, 0x22, 0xa3, 0x6e, 0xd8, 0x58, 0x5c, 0x74, 0x5a, - 0x0e, 0x11, 0x50, 0xbc, 0xce, 0xba, 0x23, 0x57, 0xd0, 0x58, 0x63, 0x69, 0x91, 0xf3, 0x8a, 0x37, 0x91, 0xe2, 0x48, 0xde, 0x50, 0x9c, 0x07, 0x0d, - 0x81, 0x2a, 0xb2, 0xfd, 0xa5, 0x78, 0x60, 0xac, 0x87, 0x6b, 0xc4, 0x89, 0x19, 0x2c, 0x1e, 0xf4, 0xce, 0x25, 0x3c, 0x19, 0x7e, 0xe2, 0x19, 0xa4] - ).unwrap(); + let fake_key_root = fake_private_key(); + let fake_key_hash = fake_key_hash(); + let raw_key_public = fake_raw_key_public(); + let fake_sig = fake_raw_key_sig(); // recall: this includes keys for input, certs and withdrawals let vkeys = match tx_builder.input_types.vkeys.len() { 0 => None, x => { let mut result = Vkeywitnesses::new(); - let raw_key = fake_key_root.to_raw_key(); for _i in 0..x { result.add(&Vkeywitness::new( - &Vkey::new(&raw_key.to_public()), - &raw_key.sign([1u8; 100].as_ref()) + &Vkey::new(&raw_key_public), + &fake_sig )); } Some(result) @@ -78,7 +102,7 @@ fn fake_full_tx(tx_builder: &TransactionBuilder, body: TransactionBody) -> Resul for addr in &tx_builder.input_types.bootstraps { // picking icarus over daedalus for fake witness generation shouldn't matter result.add(&make_icarus_bootstrap_witness( - &hash_transaction(&body), + &TransactionHash::from([0u8; TransactionHash::BYTE_COUNT]), &ByronAddress::from_bytes(addr.clone()).unwrap(), &fake_key_root )); @@ -170,7 +194,7 @@ impl TransactionBuilder { }); self.input_types.bootstraps.insert(hash.to_bytes()); } - + pub fn add_input(&mut self, address: &Address, input: &TransactionInput, amount: &Value) { match &BaseAddress::from_address(address) { Some(addr) => { @@ -1291,7 +1315,7 @@ mod tests { ), &Value::new(&to_bignum(2_400_000)) ); - + tx_builder.set_ttl(1); let change_addr = ByronAddress::from_base58("Ae2tdPwUPEZGUEsuMAhvDcy94LKsZxDjCbgaiBBMgYpR8sKf96xJmit7Eho").unwrap(); @@ -1335,7 +1359,7 @@ mod tests { ), &input_value ); - + tx_builder.set_ttl(1); let change_addr = ByronAddress::from_base58("Ae2tdPwUPEZGUEsuMAhvDcy94LKsZxDjCbgaiBBMgYpR8sKf96xJmit7Eho").unwrap();