Commit ab169e24 authored by Andreas Fackler's avatar Andreas Fackler

Address review comments.

parent 0d7d0c8e
Pipeline #71712 passed with stages
in 15 minutes and 49 seconds
......@@ -397,30 +397,69 @@ pub trait BlockChainClient:
/// Schedule state-altering transaction to be executed on the next pending block.
fn transact_contract(&self, address: Address, data: Bytes) -> Result<(), transaction::Error> {
self.transact(Action::Call(address), data, None, None, None)
self.transact(TransactionRequest::call(address, data))
}
/// Returns a signed transaction.
///
/// Gas limit, gas price, or nonce can be set explicitly, e.g. to create service
/// transactions with zero gas price, or sequences of transactions with consecutive nonces.
///
/// If these are `None`, the defaults are used.
fn create_transaction(
&self,
action: Action,
data: Bytes,
gas: Option<U256>,
gas_price: Option<U256>,
nonce: Option<U256>
) -> Result<SignedTransaction, transaction::Error>;
/// Returns a transaction signed with the key configured in the engine signer.
fn create_transaction(&self, tx_request: TransactionRequest) -> Result<SignedTransaction, transaction::Error>;
/// Schedule state-altering transaction to be executed on the next pending
/// block with the given gas and nonce parameters.
///
/// If `None` is passed for `gas`, `gas_price` or `nonce`, sensible values are selected automatically.
fn transact(&self, action: Action, data: Bytes, gas: Option<U256>, gas_price: Option<U256>, nonce: Option<U256>)
-> Result<(), transaction::Error>;
fn transact(&self, tx_request: TransactionRequest) -> Result<(), transaction::Error>;
}
/// The data required for a `Client` to create a transaction.
///
/// Gas limit, gas price, or nonce can be set explicitly, e.g. to create service
/// transactions with zero gas price, or sequences of transactions with consecutive nonces.
pub struct TransactionRequest {
pub action: Action,
pub data: Bytes,
pub gas: Option<U256>,
pub gas_price: Option<U256>,
pub nonce: Option<U256>,
}
impl TransactionRequest {
/// Creates a request to call a contract at `address` with the specified call data.
pub fn call(address: Address, data: Bytes) -> TransactionRequest {
TransactionRequest {
action: Action::Call(address),
data,
gas: None,
gas_price: None,
nonce: None,
}
}
/// Creates a request to create a new contract, with the specified bytecode.
pub fn create(data: Bytes) -> TransactionRequest {
TransactionRequest {
action: Action::Create,
data,
gas: None,
gas_price: None,
nonce: None,
}
}
/// Sets a gas limit. If this is not specified, a sensible default is used.
pub fn gas(mut self, gas: U256) -> TransactionRequest {
self.gas = Some(gas);
self
}
/// Sets a gas price. If this is not specified, a sensible default is used.
pub fn gas_price(mut self, gas_price: U256) -> TransactionRequest {
self.gas_price = Some(gas_price);
self
}
/// Sets a nonce. If this is not specified, the appropriate latest nonce for the author is used.
pub fn nonce(mut self, nonce: U256) -> TransactionRequest {
self.nonce = Some(nonce);
self
}
}
/// resets the blockchain
......
......@@ -189,7 +189,9 @@ pub trait Engine: Sync + Send {
///
/// This is called when the miner prepares a new block that this node will author and seal. It returns a list of
/// transactions that will be added to the block before any other transactions from the queue.
fn on_prepare_block(&self, _block: &ExecutedBlock) -> Result<Vec<SignedTransaction>, Error> { Ok(Vec::new()) }
fn generate_engine_transactions(&self, _block: &ExecutedBlock) -> Result<Vec<SignedTransaction>, Error> {
Ok(Vec::new())
}
/// Returns the engine's current sealing state.
fn sealing_state(&self) -> SealingState { SealingState::External }
......
......@@ -40,7 +40,7 @@ use std::sync::{Weak, Arc};
use std::time::{UNIX_EPOCH, Duration};
use std::u64;
use client_traits::{EngineClient, ForceUpdateSealing};
use client_traits::{EngineClient, ForceUpdateSealing, TransactionRequest};
use engine::{Engine, ConstructedVerifier};
use block_reward::{self, BlockRewardContract, RewardKind};
use ethjson;
......@@ -59,7 +59,6 @@ use rand::rngs::OsRng;
use rlp::{encode, Decodable, DecoderError, Encodable, RlpStream, Rlp};
use ethereum_types::{H256, H520, Address, U128, U256};
use parking_lot::{Mutex, RwLock};
use parity_bytes::Bytes;
use time_utils::CheckedSystemTime;
use common_types::{
ancestry_action::AncestryAction,
......@@ -76,7 +75,7 @@ use common_types::{
errors::{BlockError, EthcoreError as Error, EngineError},
ids::BlockId,
snapshot::Snapshotting,
transaction::{Action, SignedTransaction},
transaction::SignedTransaction,
};
use unexpected::{Mismatch, OutOfBounds};
......@@ -1081,21 +1080,21 @@ impl AuthorityRound {
EngineError::RequiresClient
})?;
let full_client = client.as_full_client()
.ok_or(EngineError::FailedSystemCall("Failed to upgrade to BlockchainClient.".to_string()))?;
.ok_or_else(|| EngineError::FailedSystemCall("Failed to upgrade to BlockchainClient.".to_string()))?;
// Random number generation
let contract = util::BoundContract::new(&*client, BlockId::Latest, contract_addr);
let phase = randomness::RandomnessPhase::load(&contract, our_addr)
.map_err(|err| EngineError::RandomnessLoadError(err.to_string()))?;
.map_err(|err| EngineError::Custom(format!("Randomness error in load(): {:?}", err)))?;
let data = match phase.advance(&contract, &mut OsRng, signer.as_ref())
.map_err(|err| EngineError::RandomnessAdvanceError(err.to_string()))? {
.map_err(|err| EngineError::Custom(format!("Randomness error in advance(): {:?}", err)))? {
Some(data) => data,
None => return Ok(Vec::new()), // Nothing to commit or reveal at the moment.
};
let nonce = block.state.nonce(&our_addr)?;
let action = Action::Call(contract_addr);
Ok(vec![full_client.create_transaction(action, data, None, Some(U256::zero()), Some(nonce))?])
let tx_request = TransactionRequest::call(contract_addr, data).gas_price(U256::zero()).nonce(nonce);
Ok(vec![full_client.create_transaction(tx_request)?])
}
}
......@@ -1481,7 +1480,7 @@ impl Engine for AuthorityRound {
block_reward::apply_block_rewards(&rewards, block, &self.machine)
}
fn on_prepare_block(&self, block: &ExecutedBlock) -> Result<Vec<SignedTransaction>, Error> {
fn generate_engine_transactions(&self, block: &ExecutedBlock) -> Result<Vec<SignedTransaction>, Error> {
self.run_randomness_phase(block)
}
......@@ -2841,7 +2840,7 @@ mod tests {
"validators": {
"list" : ["0x1000000000000000000000000000000000000001"]
},
"blockRewardContractTransition": "0",
"blockRewardContractTransition": "0",
"blockRewardContractAddress": "0x2000000000000000000000000000000000000002",
"blockRewardContractTransitions": {
"7": "0x3000000000000000000000000000000000000003",
......@@ -2872,7 +2871,7 @@ mod tests {
"validators": {
"list" : ["0x1000000000000000000000000000000000000001"]
},
"blockRewardContractTransition": "7",
"blockRewardContractTransition": "7",
"blockRewardContractAddress": "0x2000000000000000000000000000000000000002",
"blockRewardContractTransitions": {
"0": "0x3000000000000000000000000000000000000003",
......
......@@ -59,7 +59,6 @@ impl<'a> fmt::Debug for BoundContract<'a> {
impl<'a> BoundContract<'a> {
/// Create a new `BoundContract`.
#[inline]
pub fn new(client: &dyn EngineClient, block_id: BlockId, contract_addr: Address) -> BoundContract {
BoundContract {
client,
......
......@@ -79,6 +79,7 @@ use client_traits::{
StateOrBlock,
Tick,
TransactionInfo,
TransactionRequest,
ForceUpdateSealing
};
use db::{keys::BlockDetails, Readable, Writable};
......@@ -2154,14 +2155,9 @@ impl BlockChainClient for Client {
}
}
fn create_transaction(
&self,
action: Action,
data: Bytes,
gas: Option<U256>,
gas_price: Option<U256>,
nonce: Option<U256>
) -> Result<SignedTransaction, transaction::Error> {
fn create_transaction(&self, TransactionRequest { action, data, gas, gas_price, nonce }: TransactionRequest)
-> Result<SignedTransaction, transaction::Error>
{
let authoring_params = self.importer.miner.authoring_params();
let service_transaction_checker = self.importer.miner.service_transaction_checker();
let gas_price = if let Some(checker) = service_transaction_checker {
......@@ -2186,10 +2182,8 @@ impl BlockChainClient for Client {
Ok(SignedTransaction::new(transaction.with_signature(signature, chain_id))?)
}
fn transact(&self, action: Action, data: Bytes, gas: Option<U256>, gas_price: Option<U256>, nonce: Option<U256>)
-> Result<(), transaction::Error>
{
let signed = self.create_transaction(action, data, gas, gas_price, nonce)?;
fn transact(&self, tx_request: TransactionRequest) -> Result<(), transaction::Error> {
let signed = self.create_transaction(tx_request)?;
self.importer.miner.import_own_transaction(self, signed.into())
}
}
......
......@@ -430,57 +430,55 @@ impl Miner {
let chain_info = chain.chain_info();
// Open block
let mut sealing = self.sealing.lock();
let best_hash = chain_info.best_block_hash;
let original_work_hash = sealing.queue.peek_last_ref().map(|pb| pb.header.hash());
// Some engines add transactions to the block for their own purposes, e.g. AuthorityRound RANDAO.
let engine_txs: Vec<SignedTransaction>;
let mut open_block;
// check to see if last ClosedBlock in would_seals is actually same parent block.
// if so
// duplicate, re-open and push any new transactions.
// if at least one was pushed successfully, close and enqueue new ClosedBlock;
// otherwise, leave everything alone.
// otherwise, author a fresh block.
match sealing.queue.get_pending_if(|b| b.header.parent_hash() == &best_hash) {
Some(old_block) => {
trace!(target: "miner", "prepare_block: Already have previous work; updating and returning");
engine_txs = Vec::new();
// add transactions to old_block
open_block = chain.reopen_block(old_block);
}
None => {
// block not found - create it.
trace!(target: "miner", "prepare_block: No existing work - making new block");
let params = self.params.read().clone();
open_block = match chain.prepare_open_block(
params.author,
params.gas_range_target,
params.extra_data,
) {
Ok(block) => block,
Err(err) => {
warn!(target: "miner", "Open new block failed with error {:?}. This is likely an error in \
chain specification or on-chain consensus smart contracts.", err);
return None;
}
};
// Before adding from the queue to the new block, give the engine a chance to add transactions.
engine_txs = match self.engine.on_prepare_block(&open_block) {
Ok(transactions) => transactions,
Err(err) => {
error!(target: "miner", "Failed to prepare engine transactions for new block: {:?}. \
This is likely an error in chain specification or on-chain consensus smart \
contracts.", err);
return None;
let (mut open_block, original_work_hash, engine_txs) = {
let mut sealing = self.sealing.lock();
let last_work_hash = sealing.queue.peek_last_ref().map(|pb| pb.header.hash());
let best_hash = chain_info.best_block_hash;
// check to see if last ClosedBlock in would_seals is actually same parent block.
// if so
// duplicate, re-open and push any new transactions.
// if at least one was pushed successfully, close and enqueue new ClosedBlock;
// otherwise, leave everything alone.
// otherwise, author a fresh block.
match sealing.queue.get_pending_if(|b| b.header.parent_hash() == &best_hash) {
Some(old_block) => {
trace!(target: "miner", "prepare_block: Already have previous work; updating and returning");
// add transactions to old_block
(chain.reopen_block(old_block), last_work_hash, Vec::new())
}
None => {
// block not found - create it.
trace!(target: "miner", "prepare_block: No existing work - making new block");
let params = self.params.read().clone();
let block = match chain.prepare_open_block(
params.author,
params.gas_range_target,
params.extra_data,
) {
Ok(block) => block,
Err(err) => {
warn!(target: "miner", "Open new block failed with error {:?}. This is likely an error in \
chain specification or on-chain consensus smart contracts.", err);
return None;
}
};
// Before adding from the queue to the new block, give the engine a chance to add transactions.
match self.engine.generate_engine_transactions(&block) {
Ok(transactions) => (block, last_work_hash, transactions),
Err(err) => {
error!(target: "miner", "Failed to prepare engine transactions for new block: {:?}. \
This is likely an error in chain specification or on-chain consensus smart \
contracts.", err);
return None;
}
}
};
}
}
}
};
if self.options.infinite_pending_block {
open_block.remove_gas_limit();
......
......@@ -72,7 +72,7 @@ use client::{
use client_traits::{
BlockInfo, Nonce, Balance, ChainInfo, TransactionInfo, BlockChainClient, ImportBlock,
AccountData, BlockChain, IoClient, BadBlocks, ScheduleInfo, StateClient, ProvingBlockChainClient,
StateOrBlock, ForceUpdateSealing
StateOrBlock, ForceUpdateSealing, TransactionRequest
};
use engine::Engine;
use machine::executed::Executed;
......@@ -912,14 +912,9 @@ impl BlockChainClient for TestBlockChainClient {
}
}
fn create_transaction(
&self,
action: Action,
data: Bytes,
gas: Option<U256>,
gas_price: Option<U256>,
nonce: Option<U256>
) -> Result<SignedTransaction, transaction::Error> {
fn create_transaction(&self, TransactionRequest { action, data, gas, gas_price, nonce }: TransactionRequest)
-> Result<SignedTransaction, transaction::Error>
{
let transaction = Transaction {
nonce: nonce.unwrap_or_else(|| self.latest_nonce(&self.miner.authoring_params().author)),
action,
......@@ -933,16 +928,8 @@ impl BlockChainClient for TestBlockChainClient {
Ok(SignedTransaction::new(transaction.with_signature(sig, chain_id)).unwrap())
}
fn transact(
&self,
action: Action,
data: Bytes,
gas: Option<U256>,
gas_price: Option<U256>,
_nonce: Option<U256>,
) -> Result<(), transaction::Error>
{
let signed = self.create_transaction(action, data, gas, gas_price, None)?;
fn transact(&self, tx_request: TransactionRequest) -> Result<(), transaction::Error> {
let signed = self.create_transaction(tx_request)?;
self.miner.import_own_transaction(self, signed.into())
}
}
......
......@@ -35,10 +35,6 @@ pub enum EngineError {
BadSealFieldSize(OutOfBounds<usize>),
/// Validation proof insufficient.
InsufficientProof(String),
/// Randomness error in load method
RandomnessLoadError(String),
/// Randomness error in advance method
RandomnessAdvanceError(String),
/// Failed system call.
FailedSystemCall(String),
/// Failed to decode the result of a system call.
......@@ -98,8 +94,6 @@ impl fmt::Display for EngineError {
UnexpectedMessage => "This Engine should not be fed messages.".into(),
BadSealFieldSize(ref oob) => format!("Seal field has an unexpected length: {}", oob),
InsufficientProof(ref msg) => format!("Insufficient validation proof: {}", msg),
RandomnessLoadError(ref rerr) => format!("Randomness error in load(): {:?}", rerr),
RandomnessAdvanceError(ref rerr) => format!("Randomness error in advance(): {:?}", rerr),
FailedSystemCall(ref msg) => format!("Failed to make system call: {}", msg),
SystemCallResultDecoding(ref msg) => format!("Failed to decode the result of a system call: {}", msg),
SystemCallResultInvalid(ref msg) => format!("The result of a system call is invalid: {}", msg),
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment