From fda64c803950438b3c2954e44500fb7cc48906a8 Mon Sep 17 00:00:00 2001 From: Svyatoslav Nikolsky Date: Thu, 16 Jul 2020 12:58:46 +0300 Subject: [PATCH] Exchange pallet benchmarks (#158) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * exchange benchmarks: framework * updated comment about tx size Co-authored-by: Tomasz Drwięga --- bin/node/runtime/Cargo.toml | 15 +- bin/node/runtime/src/exchange.rs | 82 +++++++---- bin/node/runtime/src/lib.rs | 45 +++++- modules/currency-exchange/src/benchmarking.rs | 134 ++++++++++++++++++ modules/currency-exchange/src/lib.rs | 9 +- modules/ethereum/src/benchmarking.rs | 1 - modules/ethereum/src/lib.rs | 4 +- modules/ethereum/src/test_utils.rs | 29 +++- primitives/ethereum-poa/Cargo.toml | 1 + primitives/ethereum-poa/src/lib.rs | 6 +- 10 files changed, 281 insertions(+), 45 deletions(-) create mode 100644 modules/currency-exchange/src/benchmarking.rs diff --git a/bin/node/runtime/Cargo.toml b/bin/node/runtime/Cargo.toml index 7d6d6f009ce..fbb9099d5ab 100644 --- a/bin/node/runtime/Cargo.toml +++ b/bin/node/runtime/Cargo.toml @@ -16,6 +16,12 @@ version = "1.3.1" default-features = false features = ["derive"] +[dependencies.libsecp256k1] +optional = true +version = "0.3.4" +default-features = false +features = ["hmac"] + [dependencies.serde] version = "1.0.114" optional = true @@ -200,10 +206,15 @@ tag = 'v2.0.0-rc4' default-features = false git = "https://github.com/paritytech/substrate/" +[dev-dependencies.libsecp256k1] +version = "0.3.4" +default-features = false +features = ["hmac"] + [dev-dependencies.sp-bridge-eth-poa] version = "0.1.0" default-features = false -features = ["test-helpers"] +features = ["std", "test-helpers"] path = "../../../primitives/ethereum-poa" [build-dependencies.wasm-builder-runner] @@ -250,7 +261,9 @@ runtime-benchmarks = [ "frame-benchmarking", "frame-support/runtime-benchmarks", "frame-system/runtime-benchmarks", + "libsecp256k1", "pallet-bridge-currency-exchange/runtime-benchmarks", "pallet-bridge-eth-poa/runtime-benchmarks", + "sp-bridge-eth-poa/test-helpers", "sp-runtime/runtime-benchmarks", ] diff --git a/bin/node/runtime/src/exchange.rs b/bin/node/runtime/src/exchange.rs index 8a627e8a2c8..3e9ab8a609d 100644 --- a/bin/node/runtime/src/exchange.rs +++ b/bin/node/runtime/src/exchange.rs @@ -145,6 +145,57 @@ impl MaybeLockFundsTransaction for EthTransaction { } } +/// Prepares everything required to bench claim of funds locked by given transaction. +#[cfg(feature = "runtime-benchmarks")] +pub(crate) fn prepare_environment_for_claim( + transactions: &[RawTransaction], +) -> sp_bridge_eth_poa::H256 { + use pallet_bridge_eth_poa::{ + test_utils::{insert_header, validator_utils::validator, HeaderBuilder}, + BridgeStorage, Storage, + }; + use sp_bridge_eth_poa::compute_merkle_root; + + let mut storage = BridgeStorage::::new(); + let header = HeaderBuilder::with_parent_number_on_runtime::(0) + .with_transactions_root(compute_merkle_root(transactions.iter())) + .sign_by(&validator(0)); + let header_id = header.compute_id(); + insert_header(&mut storage, header); + storage.finalize_and_prune_headers(Some(header_id), 0); + + header_id.hash +} + +/// Prepare signed ethereum lock-funds transaction. +#[cfg(any(feature = "runtime-benchmarks", test))] +pub(crate) fn prepare_ethereum_transaction( + recipient: &crate::AccountId, + editor: impl Fn(&mut sp_bridge_eth_poa::UnsignedTransaction), +) -> Vec { + use sp_bridge_eth_poa::signatures::SignTransaction; + + // prepare tx for OpenEthereum private dev chain: + // chain id is 0x11 + // sender secret is 0x4d5db4107d237df6a3d58ee5f70ae63d73d7658d4026f2eefd2f204c81682cb7 + let chain_id = 0x11; + let signer = secp256k1::SecretKey::parse(&hex!( + "4d5db4107d237df6a3d58ee5f70ae63d73d7658d4026f2eefd2f204c81682cb7" + )) + .unwrap(); + let recipient_raw: &[u8; 32] = recipient.as_ref(); + let mut eth_tx = sp_bridge_eth_poa::UnsignedTransaction { + nonce: 0.into(), + to: Some(LOCK_FUNDS_ADDRESS.into()), + value: 100.into(), + gas: 100_000.into(), + gas_price: 100_000.into(), + payload: recipient_raw.to_vec(), + }; + editor(&mut eth_tx); + eth_tx.sign_by(&signer.into(), Some(chain_id)) +} + #[cfg(test)] mod tests { use super::*; @@ -158,33 +209,10 @@ mod tests { hex!("1cbd2d43530a44705ad088af313e18f80b53ef16b36177cd4b77b846f2a5f07c").into() } - fn prepare_ethereum_transaction(editor: impl Fn(&mut UnsignedTransaction)) -> Vec { - // prepare tx for OpenEthereum private dev chain: - // chain id is 0x11 - // sender secret is 0x4d5db4107d237df6a3d58ee5f70ae63d73d7658d4026f2eefd2f204c81682cb7 - let chain_id = 0x11_u64; - let signer = SecretKey::parse(&hex!( - "4d5db4107d237df6a3d58ee5f70ae63d73d7658d4026f2eefd2f204c81682cb7" - )) - .unwrap(); - let ferdie_id = ferdie(); - let ferdie_raw: &[u8; 32] = ferdie_id.as_ref(); - let mut eth_tx = UnsignedTransaction { - nonce: 0.into(), - to: Some(LOCK_FUNDS_ADDRESS.into()), - value: 100.into(), - gas: 100_000.into(), - gas_price: 100_000.into(), - payload: ferdie_raw.to_vec(), - }; - editor(&mut eth_tx); - eth_tx.sign_by(&signer, Some(chain_id)) - } - #[test] fn valid_transaction_accepted() { assert_eq!( - EthTransaction::parse(&prepare_ethereum_transaction(|_| {})), + EthTransaction::parse(&prepare_ethereum_transaction(&ferdie(), |_| {})), Ok(LockFundsTransaction { id: EthereumTransactionTag { account: hex!("00a329c0648769a73afac7f9381e08fb43dbea72"), @@ -207,7 +235,7 @@ mod tests { #[test] fn transaction_with_invalid_peer_recipient_rejected() { assert_eq!( - EthTransaction::parse(&prepare_ethereum_transaction(|tx| { + EthTransaction::parse(&prepare_ethereum_transaction(&ferdie(), |tx| { tx.to = None; })), Err(ExchangeError::InvalidTransaction), @@ -217,7 +245,7 @@ mod tests { #[test] fn transaction_with_invalid_recipient_rejected() { assert_eq!( - EthTransaction::parse(&prepare_ethereum_transaction(|tx| { + EthTransaction::parse(&prepare_ethereum_transaction(&ferdie(), |tx| { tx.payload.clear(); })), Err(ExchangeError::InvalidRecipient), @@ -227,7 +255,7 @@ mod tests { #[test] fn transaction_with_invalid_amount_rejected() { assert_eq!( - EthTransaction::parse(&prepare_ethereum_transaction(|tx| { + EthTransaction::parse(&prepare_ethereum_transaction(&ferdie(), |tx| { tx.value = sp_core::U256::from(u128::max_value()) + sp_core::U256::from(1); })), Err(ExchangeError::InvalidAmount), diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index 8ed1e5fbef4..6d2c5eb7869 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -641,9 +641,52 @@ impl_runtime_apis! { ) -> Result, sp_runtime::RuntimeString> { use frame_benchmarking::{Benchmarking, BenchmarkBatch, add_benchmark}; let mut batches = Vec::::new(); - let params = (&pallet, &benchmark, &lowest_range_values, &highest_range_values, &steps, repeat); + + let whitelist: Vec> = vec![]; + let params = (&pallet, &benchmark, &lowest_range_values, &highest_range_values, &steps, repeat, &whitelist); + + use pallet_bridge_currency_exchange::benchmarking::{ + Module as BridgeCurrencyExchangeBench, + Trait as BridgeCurrencyExchangeTrait, + ProofParams as BridgeCurrencyExchangeProofParams, + }; + + impl BridgeCurrencyExchangeTrait for Runtime { + fn make_proof( + proof_params: BridgeCurrencyExchangeProofParams, + ) -> crate::exchange::EthereumTransactionInclusionProof { + use sp_currency_exchange::DepositInto; + + if proof_params.recipient_exists { + ::DepositInto::deposit_into( + proof_params.recipient.clone(), + ExistentialDeposit::get(), + ).unwrap(); + } + + let transaction = crate::exchange::prepare_ethereum_transaction( + &proof_params.recipient, + |tx| { + // our runtime only supports transactions where data is exactly 32 bytes long + // (receiver key) + // => we are ignoring `transaction_size_factor` here + tx.value = (ExistentialDeposit::get() * 10).into(); + }, + ); + let transactions = sp_std::iter::repeat(transaction.clone()) + .take(1 + proof_params.proof_size_factor as usize) + .collect::>(); + let block_hash = crate::exchange::prepare_environment_for_claim::(&transactions); + crate::exchange::EthereumTransactionInclusionProof { + block: block_hash, + index: 0, + proof: transactions, + } + } + } add_benchmark!(params, batches, b"bridge-eth-poa", BridgeEthPoA); + add_benchmark!(params, batches, b"bridge-currency-exchange", BridgeCurrencyExchangeBench::); if batches.is_empty() { return Err("Benchmark not found for this pallet.".into()) } Ok(batches) diff --git a/modules/currency-exchange/src/benchmarking.rs b/modules/currency-exchange/src/benchmarking.rs new file mode 100644 index 00000000000..dc032720ce3 --- /dev/null +++ b/modules/currency-exchange/src/benchmarking.rs @@ -0,0 +1,134 @@ +// Copyright 2019-2020 Parity Technologies (UK) Ltd. +// This file is part of Parity Bridges Common. + +// Parity Bridges Common is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. + +// Parity Bridges Common is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License +// along with Parity Bridges Common. If not, see . + +//! Exchange module complexity is mostly determined by callbacks, defined by runtime. +//! So we are giving runtime opportunity to prepare environment and construct proof +//! before invoking module calls. + +use super::{Blockchain, Call, Module as CurrencyExchangeModule, Trait as CurrencyExchangeTrait}; +use sp_std::prelude::*; + +use frame_benchmarking::{account, benchmarks}; +use frame_system::RawOrigin; + +const SEED: u32 = 0; +const WORST_TX_SIZE_FACTOR: u32 = 1000; +const WORST_PROOF_SIZE_FACTOR: u32 = 1000; + +/// Module we're benchmarking here. +pub struct Module(CurrencyExchangeModule); + +/// Proof benchmarking parameters. +pub struct ProofParams { + /// Funds recipient. + pub recipient: Recipient, + /// When true, recipient must exists before import. + pub recipient_exists: bool, + /// When 0, transaction should have minimal possible size. When this value has non-zero value n, + /// transaction size should be (if possible) near to MIN_SIZE + n * SIZE_FACTOR. + pub transaction_size_factor: u32, + /// When 0, proof should have minimal possible size. When this value has non-zero value n, + /// proof size should be (if possible) near to MIN_SIZE + n * SIZE_FACTOR. + pub proof_size_factor: u32, +} + +/// Trait that must be implemented by runtime. +pub trait Trait: CurrencyExchangeTrait { + /// Prepare proof for importing exchange transaction. + fn make_proof( + proof_params: ProofParams, + ) -> <::PeerBlockchain as Blockchain>::TransactionInclusionProof; +} + +benchmarks! { + _ { } + + // Benchmark `import_peer_transaction` extrinsic with the best possible conditions: + // * Proof is the transaction itself. + // * Transaction has minimal size. + // * Recipient account exists. + import_peer_transaction_best_case { + let i in 1..100; + + let recipient: T::AccountId = account("recipient", i, SEED); + let proof = T::make_proof(ProofParams { + recipient: recipient.clone(), + recipient_exists: true, + transaction_size_factor: 0, + proof_size_factor: 0, + }); + }: import_peer_transaction(RawOrigin::Signed(recipient), proof) + + // Benchmark `import_peer_transaction` extrinsic when recipient account does not exists. + import_peer_transaction_when_recipient_does_not_exists { + let i in 1..100; + + let recipient: T::AccountId = account("recipient", i, SEED); + let proof = T::make_proof(ProofParams { + recipient: recipient.clone(), + recipient_exists: false, + transaction_size_factor: 0, + proof_size_factor: 0, + }); + }: import_peer_transaction(RawOrigin::Signed(recipient), proof) + + // Benchmark `import_peer_transaction` when transaction size increases. + import_peer_transaction_when_transaction_size_increases { + let i in 1..100; + let n in 1..WORST_TX_SIZE_FACTOR; + + let recipient: T::AccountId = account("recipient", i, SEED); + let proof = T::make_proof(ProofParams { + recipient: recipient.clone(), + recipient_exists: true, + transaction_size_factor: n, + proof_size_factor: 0, + }); + }: import_peer_transaction(RawOrigin::Signed(recipient), proof) + + // Benchmark `import_peer_transaction` when proof size increases. + import_peer_transaction_when_proof_size_increases { + let i in 1..100; + let n in 1..WORST_PROOF_SIZE_FACTOR; + + let recipient: T::AccountId = account("recipient", i, SEED); + let proof = T::make_proof(ProofParams { + recipient: recipient.clone(), + recipient_exists: true, + transaction_size_factor: 0, + proof_size_factor: n, + }); + }: import_peer_transaction(RawOrigin::Signed(recipient), proof) + + // Benchmark `import_peer_transaction` extrinsic with the worst possible conditions: + // * Proof is large. + // * Transaction has large size. + // * Recipient account does not exists. + import_peer_transaction_worst_case { + let i in 1..100; + let m in WORST_TX_SIZE_FACTOR..WORST_TX_SIZE_FACTOR+1; + let n in WORST_PROOF_SIZE_FACTOR..WORST_PROOF_SIZE_FACTOR+1; + + let recipient: T::AccountId = account("recipient", i, SEED); + let proof = T::make_proof(ProofParams { + recipient: recipient.clone(), + recipient_exists: false, + transaction_size_factor: m, + proof_size_factor: n, + }); + }: import_peer_transaction(RawOrigin::Signed(recipient), proof) + +} diff --git a/modules/currency-exchange/src/lib.rs b/modules/currency-exchange/src/lib.rs index 959538b7465..c6ca356e00f 100644 --- a/modules/currency-exchange/src/lib.rs +++ b/modules/currency-exchange/src/lib.rs @@ -24,6 +24,9 @@ use sp_currency_exchange::{ }; use sp_runtime::DispatchResult; +#[cfg(feature = "runtime-benchmarks")] +pub mod benchmarking; + /// Called when transaction is submitted to the exchange module. pub trait OnTransactionSubmitted { /// Called when valid transaction is submitted and accepted by the module. @@ -229,7 +232,7 @@ mod tests { fn parse(tx: &Self::Transaction) -> sp_currency_exchange::Result { match tx.id { - INVALID_TRANSACTION_ID => Err(sp_currency_exchange::Error::InvalidTransaction), + INVALID_TRANSACTION_ID => Err(ExchangeError::InvalidTransaction), _ => Ok(tx.clone()), } } @@ -243,7 +246,7 @@ mod tests { fn map(peer_recipient: Self::PeerRecipient) -> sp_currency_exchange::Result { match peer_recipient { - UNKNOWN_RECIPIENT_ID => Err(sp_currency_exchange::Error::FailedToMapRecipients), + UNKNOWN_RECIPIENT_ID => Err(ExchangeError::FailedToMapRecipients), _ => Ok(peer_recipient * 10), } } @@ -257,7 +260,7 @@ mod tests { fn convert(amount: Self::SourceAmount) -> sp_currency_exchange::Result { match amount { - INVALID_AMOUNT => Err(sp_currency_exchange::Error::FailedToConvertCurrency), + INVALID_AMOUNT => Err(ExchangeError::FailedToConvertCurrency), _ => Ok(amount * 10), } } diff --git a/modules/ethereum/src/benchmarking.rs b/modules/ethereum/src/benchmarking.rs index d31d4103912..1f0a7fbe96f 100644 --- a/modules/ethereum/src/benchmarking.rs +++ b/modules/ethereum/src/benchmarking.rs @@ -48,7 +48,6 @@ benchmarks! { header }, ); - }: import_unsigned_header(RawOrigin::None, header, None) verify { let storage = BridgeStorage::::new(); diff --git a/modules/ethereum/src/lib.rs b/modules/ethereum/src/lib.rs index db54613b8e7..b77f436301d 100644 --- a/modules/ethereum/src/lib.rs +++ b/modules/ethereum/src/lib.rs @@ -44,7 +44,7 @@ mod benchmarking; mod mock; #[cfg(any(feature = "runtime-benchmarks", test))] -mod test_utils; +pub mod test_utils; /// Maximal number of blocks we're pruning in single import call. const MAX_BLOCKS_TO_PRUNE_IN_SINGLE_IMPORT: u64 = 8; @@ -547,7 +547,7 @@ impl frame_support::unsigned::ValidateUnsigned for Module { /// Runtime bridge storage. #[derive(Default)] -struct BridgeStorage(sp_std::marker::PhantomData); +pub struct BridgeStorage(sp_std::marker::PhantomData); impl BridgeStorage { /// Create new BridgeStorage. diff --git a/modules/ethereum/src/test_utils.rs b/modules/ethereum/src/test_utils.rs index a4c51e4ac68..9084783d680 100644 --- a/modules/ethereum/src/test_utils.rs +++ b/modules/ethereum/src/test_utils.rs @@ -27,7 +27,7 @@ use crate::finality::FinalityVotes; use crate::validators::CHANGE_EVENT_HASH; use crate::verification::calculate_score; -use crate::{HeaderToImport, Storage}; +use crate::{HeaderToImport, Storage, Trait}; use primitives::{ rlp_encode, @@ -60,25 +60,34 @@ impl HeaderBuilder { } } - /// Creates default header on top of parent with given hash. + /// Creates default header on top of test parent with given hash. #[cfg(test)] pub fn with_parent_hash(parent_hash: H256) -> Self { - use crate::mock::TestRuntime; + Self::with_parent_hash_on_runtime::(parent_hash) + } + + /// Creates default header on top of test parent with given number. First parent is selected. + #[cfg(test)] + pub fn with_parent_number(parent_number: u64) -> Self { + Self::with_parent_number_on_runtime::(parent_number) + } + + /// Creates default header on top of parent with given hash. + pub fn with_parent_hash_on_runtime(parent_hash: H256) -> Self { use crate::Headers; use frame_support::StorageMap; - let parent_header = Headers::::get(&parent_hash).unwrap().header; + let parent_header = Headers::::get(&parent_hash).unwrap().header; Self::with_parent(&parent_header) } /// Creates default header on top of parent with given number. First parent is selected. - #[cfg(test)] - pub fn with_parent_number(parent_number: u64) -> Self { + pub fn with_parent_number_on_runtime(parent_number: u64) -> Self { use crate::HeadersByNumber; use frame_support::StorageMap; let parent_hash = HeadersByNumber::get(parent_number).unwrap()[0].clone(); - Self::with_parent_hash(parent_hash) + Self::with_parent_hash_on_runtime::(parent_hash) } /// Creates default header on top of non-existent parent. @@ -185,6 +194,12 @@ impl HeaderBuilder { self } + /// Update transactions root field of this header. + pub fn with_transactions_root(mut self, transactions_root: H256) -> Self { + self.header.transactions_root = transactions_root; + self + } + /// Signs header by given author. pub fn sign_by(self, author: &SecretKey) -> Header { self.header.sign_by(author) diff --git a/primitives/ethereum-poa/Cargo.toml b/primitives/ethereum-poa/Cargo.toml index bf1d835197f..232bdf21bf6 100644 --- a/primitives/ethereum-poa/Cargo.toml +++ b/primitives/ethereum-poa/Cargo.toml @@ -54,6 +54,7 @@ features = ["hmac"] [dev-dependencies] hex-literal = "0.2" +libsecp256k1 = { version = "0.3.4", default-features = false, features = ["hmac"] } [features] default = ["std"] diff --git a/primitives/ethereum-poa/src/lib.rs b/primitives/ethereum-poa/src/lib.rs index c0cf4da9d59..fa47912f39f 100644 --- a/primitives/ethereum-poa/src/lib.rs +++ b/primitives/ethereum-poa/src/lib.rs @@ -117,9 +117,9 @@ pub struct UnsignedTransaction { pub gas: U256, /// Transaction destination address. None if it is contract creation transaction. pub to: Option
, - /// Transaction value. + /// Value. pub value: U256, - /// Transaction payload. + /// Associated data. pub payload: Bytes, } @@ -432,7 +432,7 @@ impl std::fmt::Debug for Bloom { } /// Decode Ethereum transaction. -pub fn transaction_decode(raw_tx: &[u8]) -> Result { +pub fn transaction_decode(raw_tx: &[u8]) -> Result { // parse transaction fields let unsigned = UnsignedTransaction::decode(raw_tx)?; let tx_rlp = Rlp::new(raw_tx);