Skip to content
This repository has been archived by the owner on Nov 6, 2020. It is now read-only.

Implement caching for service transactions checker #10088

Merged
merged 41 commits into from
Mar 31, 2019
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
41 commits
Select commit Hold shift + click to select a range
234e582
Tx permission contract improvement
VladLupashevskyi Apr 13, 2018
da93f5c
Merge remote-tracking branch 'upstream/master'
VladLupashevskyi Jun 1, 2018
fbe439f
Merge branch 'master' of https://github.com/paritytech/parity
VladLupashevskyi Jun 2, 2018
7f56abe
Merge remote-tracking branch 'remotes/upstream/master'
VladLupashevskyi Jul 10, 2018
1f9c595
Merge branch 'master' of https://github.com/paritytech/parity
VladLupashevskyi Nov 14, 2018
5984a41
Merge branch 'master' of https://github.com/paritytech/parity
VladLupashevskyi Dec 19, 2018
16571bc
Take in account zero gas price certification when doing transact_cont…
VladLupashevskyi Dec 19, 2018
2bf7312
DRY in ServiceTransactionChecker
VladLupashevskyi Jan 9, 2019
33c769f
Fix typos and regroup mod
VladLupashevskyi Jan 9, 2019
fb74e9d
Merge branch 'master' of https://github.com/paritytech/parity
VladLupashevskyi Jan 9, 2019
b2c127d
Introduce CertifiedAddressesCache
VladLupashevskyi Jan 21, 2019
086cc5b
Introduce refresh_cache for CertifiedAddressesCache
VladLupashevskyi Jan 21, 2019
6cab68f
Add CertifiedAddressesCache read and write on checking
VladLupashevskyi Jan 21, 2019
1ccf28c
Refresh CertifiedAddressesCache on new imported block
VladLupashevskyi Jan 21, 2019
0f4df80
Merge branch 'master' of https://github.com/paritytech/parity
VladLupashevskyi Jan 21, 2019
cab0989
Separate ChainInfo trait and fix errors after merge
VladLupashevskyi Jan 21, 2019
da776e9
Do not fire an error when service txes contract does not exist
VladLupashevskyi Jan 21, 2019
557bd0c
WIP: Shared certified addresses cache between miner and client + use …
VladLupashevskyi Jan 22, 2019
cd38f3b
Refactor refresh_cache for ServiceTransactionChecker
VladLupashevskyi Jan 22, 2019
96e5bba
Refresh cache fixes
VladLupashevskyi Jan 22, 2019
1863d90
Add cache read in check_address + log when cache is used + improve code
VladLupashevskyi Jan 22, 2019
823f383
Remove ChainInfo from ServiceTransaction dependencies
VladLupashevskyi Jan 22, 2019
79131c6
DRY ServiceTransactionChecker
VladLupashevskyi Jan 22, 2019
51bbad3
Fix Client and Miner in tests
VladLupashevskyi Jan 23, 2019
06a4b2d
Fix node_filter test
VladLupashevskyi Jan 23, 2019
ec610a3
Fix Client::new in add_peer_with_private_config
VladLupashevskyi Jan 23, 2019
118002c
Merge branch 'master' of https://github.com/paritytech/parity
VladLupashevskyi Jan 26, 2019
6e73d1e
WIP: Separated ChainNotify from ethcore trait and implemented ChainNo…
VladLupashevskyi Jan 27, 2019
bffd73e
Fix watcher test
VladLupashevskyi Jan 28, 2019
4e7371d
Merge branch 'master' into master
VladLupashevskyi Jan 28, 2019
0a97ea2
Revert "Merge branch 'master' into master"
VladLupashevskyi Feb 5, 2019
aba3273
Revert "Fix watcher test"
VladLupashevskyi Feb 5, 2019
f383cec
Revert "WIP: Separated ChainNotify from ethcore trait and implemented…
VladLupashevskyi Feb 5, 2019
fbcf37b
Revert "Fix Client::new in add_peer_with_private_config"
VladLupashevskyi Feb 5, 2019
49cdf39
Revert "Fix node_filter test"
VladLupashevskyi Feb 5, 2019
8c2c152
Revert "Fix Client and Miner in tests"
VladLupashevskyi Feb 5, 2019
9c41cdc
Implement ServiceTransactionChecker in miner and delegate it to clien…
VladLupashevskyi Feb 5, 2019
93955a7
Merge master
VladLupashevskyi Feb 5, 2019
3415776
Code improvements
VladLupashevskyi Feb 7, 2019
497b951
Merge branch 'master' of https://github.com/paritytech/parity-ethereum
VladLupashevskyi Feb 7, 2019
04841c6
Merge branch 'master' of https://github.com/paritytech/parity-ethereum
VladLupashevskyi Feb 7, 2019
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
8 changes: 7 additions & 1 deletion ethcore/src/client/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ use header::{BlockNumber, Header, ExtendedHeader};
use io::IoChannel;
use log_entry::LocalizedLogEntry;
use miner::{Miner, MinerService};
use miner::service_transaction_checker::ServiceTransactionChecker;
use ethcore_miner::pool::VerifiedTransaction;
use parking_lot::{Mutex, RwLock};
use rand::OsRng;
Expand Down Expand Up @@ -2112,10 +2113,15 @@ impl BlockChainClient for Client {

fn transact_contract(&self, address: Address, data: Bytes) -> Result<(), transaction::Error> {
let authoring_params = self.importer.miner.authoring_params();
let service_transaction_checker = ServiceTransactionChecker::default();
let gas_price = match service_transaction_checker.check_address(self, address) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

address is contract address you try to call, not the sender (which should be checked here).

Ok(true) => U256::zero(),
_ => self.importer.miner.sensible_gas_price(),
};
let transaction = Transaction {
nonce: self.latest_nonce(&authoring_params.author),
action: Action::Call(address),
gas: self.importer.miner.sensible_gas_limit(),
gas: gas_price,
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think you meant to change gas_price not gas.

gas_price: self.importer.miner.sensible_gas_price(),
value: U256::zero(),
data: data,
Expand Down
2 changes: 1 addition & 1 deletion ethcore/src/miner/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
//! Keeps track of transactions and currently sealed pending block.

mod miner;
mod service_transaction_checker;
pub mod service_transaction_checker;
Copy link
Collaborator

Choose a reason for hiding this comment

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

please group with other pub mod


pub mod pool_client;
#[cfg(feature = "stratum")]
Expand Down
12 changes: 11 additions & 1 deletion ethcore/src/miner/service_transaction_checker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
use client::{RegistryInfo, CallContract, BlockId};
use transaction::SignedTransaction;
use ethabi::FunctionOutputDecoder;
use ethereum_types::Address;

use_contract!(service_transaction, "res/contracts/service_transaction.json");

Expand All @@ -29,7 +30,7 @@ const SERVICE_TRANSACTION_CONTRACT_REGISTRY_NAME: &'static str = "service_transa
pub struct ServiceTransactionChecker;

impl ServiceTransactionChecker {
/// Checks if given address is whitelisted to send service transactions.
/// Checks if given address in tx is whitelisted to send service transactions.
pub fn check<C: CallContract + RegistryInfo>(&self, client: &C, tx: &SignedTransaction) -> Result<bool, String> {
let sender = tx.sender();
let hash = tx.hash();
Expand All @@ -48,4 +49,13 @@ impl ServiceTransactionChecker {
let value = client.call_contract(BlockId::Latest, address, data)?;
decoder.decode(&value).map_err(|e| e.to_string())
}

/// Checks if given address is whitelisted to send service transactions.
pub fn check_address<C: CallContract + RegistryInfo>(&self, client: &C, sender: Address) -> Result<bool, String> {
VladLupashevskyi marked this conversation as resolved.
Show resolved Hide resolved
let contract_address = client.registry_address(SERVICE_TRANSACTION_CONTRACT_REGISTRY_NAME.to_owned(), BlockId::Latest)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Would the performance be a concern here? registry_address would need to query the state if registrar is set.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think that an issue, some cache would be nice (best would be if previously queried registry entries would be refreshed after a block is imported), but the code is not super hot (only in \.transact which is called by the engine, or by the miner, called only for zero-gas-price transactions attempting to enter the pool).

.ok_or_else(|| "contract is not configured")?;
let (data, decoder) = service_transaction::functions::certified::call(sender);
let value = client.call_contract(BlockId::Latest, contract_address, data)?;
decoder.decode(&value).map_err(|e| e.to_string())
}
}