Skip to content

Commit

Permalink
Remove all cryptoxide calls from fake_full_tx()
Browse files Browse the repository at this point in the history
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<W>::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.
  • Loading branch information
rooooooooob committed Dec 9, 2021
1 parent f64e952 commit 4cd3f9e
Showing 1 changed file with 20 additions and 6 deletions.
26 changes: 20 additions & 6 deletions rust/src/tx_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,28 +47,42 @@ fn fake_private_key() -> Bip32PrivateKey {
}

fn fake_key_hash() -> Ed25519KeyHash {
Ed25519KeyHash::from_bytes(
vec![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]
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<Transaction, JsError> {
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)
Expand All @@ -88,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
));
Expand Down

0 comments on commit 4cd3f9e

Please sign in to comment.