Skip to content
This repository was archived by the owner on Nov 6, 2020. It is now read-only.
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
120 changes: 94 additions & 26 deletions ethcore/src/engines/authority_round/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -83,10 +83,8 @@ pub struct AuthorityRoundParams {
pub immediate_transitions: bool,
/// Block reward in base units.
pub block_reward: U256,
/// Block reward contract transition block.
pub block_reward_contract_transition: u64,
/// Block reward contract.
pub block_reward_contract: Option<BlockRewardContract>,
/// Block reward contract addresses with their associated starting block numbers.
pub block_reward_contract_transitions: BTreeMap<u64, BlockRewardContract>,
/// Number of accepted uncles transition block.
pub maximum_uncle_count_transition: u64,
/// Number of accepted uncles.
Expand All @@ -108,6 +106,30 @@ impl From<ethjson::spec::AuthorityRoundParams> for AuthorityRoundParams {
step_duration_usize = U16_MAX;
warn!(target: "engine", "step_duration is too high ({}), setting it to {}", step_duration_usize, U16_MAX);
}
let transition_block_num = p.block_reward_contract_transition.map_or(0, Into::into);
let mut br_transitions: BTreeMap<_, _> = p.block_reward_contract_transitions
.unwrap_or_default()
.into_iter()
.map(|(block_num, address)|
(block_num.into(), BlockRewardContract::new_from_address(address.into())))
.collect();
if (p.block_reward_contract_code.is_some() || p.block_reward_contract_address.is_some()) &&
br_transitions.keys().next().map_or(false, |&block_num| block_num <= transition_block_num)
{
let s = "blockRewardContractTransition";
panic!("{} should be less than any of the keys in {}s", s, s);
}
if let Some(code) = p.block_reward_contract_code {
br_transitions.insert(
transition_block_num,
BlockRewardContract::new_from_code(Arc::new(code.into()))
);
} else if let Some(address) = p.block_reward_contract_address {
br_transitions.insert(
transition_block_num,
BlockRewardContract::new_from_address(address.into())
);
}
AuthorityRoundParams {
step_duration: step_duration_usize as u16,
validators: new_validator_set(p.validators),
Expand All @@ -116,12 +138,7 @@ impl From<ethjson::spec::AuthorityRoundParams> for AuthorityRoundParams {
validate_step_transition: p.validate_step_transition.map_or(0, Into::into),
immediate_transitions: p.immediate_transitions.unwrap_or(false),
block_reward: p.block_reward.map_or_else(Default::default, Into::into),
block_reward_contract_transition: p.block_reward_contract_transition.map_or(0, Into::into),
block_reward_contract: match (p.block_reward_contract_code, p.block_reward_contract_address) {
(Some(code), _) => Some(BlockRewardContract::new_from_code(Arc::new(code.into()))),
(_, Some(address)) => Some(BlockRewardContract::new_from_address(address.into())),
(None, None) => None,
},
block_reward_contract_transitions: br_transitions,
maximum_uncle_count_transition: p.maximum_uncle_count_transition.map_or(0, Into::into),
maximum_uncle_count: p.maximum_uncle_count.map_or(0, Into::into),
empty_steps_transition: p.empty_steps_transition.map_or(u64::max_value(), |n| ::std::cmp::max(n.into(), 1)),
Expand Down Expand Up @@ -446,8 +463,7 @@ pub struct AuthorityRound {
epoch_manager: Mutex<EpochManager>,
immediate_transitions: bool,
block_reward: U256,
block_reward_contract_transition: u64,
block_reward_contract: Option<BlockRewardContract>,
block_reward_contract_transitions: BTreeMap<u64, BlockRewardContract>,
maximum_uncle_count_transition: u64,
maximum_uncle_count: usize,
empty_steps_transition: u64,
Expand Down Expand Up @@ -711,8 +727,7 @@ impl AuthorityRound {
epoch_manager: Mutex::new(EpochManager::blank()),
immediate_transitions: our_params.immediate_transitions,
block_reward: our_params.block_reward,
block_reward_contract_transition: our_params.block_reward_contract_transition,
block_reward_contract: our_params.block_reward_contract,
block_reward_contract_transitions: our_params.block_reward_contract_transitions,
maximum_uncle_count_transition: our_params.maximum_uncle_count_transition,
maximum_uncle_count: our_params.maximum_uncle_count,
empty_steps_transition: our_params.empty_steps_transition,
Expand Down Expand Up @@ -1275,16 +1290,16 @@ impl Engine for AuthorityRound {
let author = *block.header.author();
beneficiaries.push((author, RewardKind::Author));

let rewards: Vec<_> = match self.block_reward_contract {
Some(ref c) if block.header.number() >= self.block_reward_contract_transition => {
let mut call = engine::default_system_or_code_call(&self.machine, block);

let rewards = c.reward(beneficiaries, &mut call)?;
rewards.into_iter().map(|(author, amount)| (author, RewardKind::External, amount)).collect()
},
_ => {
beneficiaries.into_iter().map(|(author, reward_kind)| (author, reward_kind, self.block_reward)).collect()
},
let block_reward_contract_transition = self
.block_reward_contract_transitions
.range(..=block.header.number())
.last();
let rewards: Vec<_> = if let Some((_, contract)) = block_reward_contract_transition {
let mut call = engine::default_system_or_code_call(&self.machine, block);
let rewards = contract.reward(beneficiaries, &mut call)?;
rewards.into_iter().map(|(author, amount)| (author, RewardKind::External, amount)).collect()
} else {
beneficiaries.into_iter().map(|(author, reward_kind)| (author, reward_kind, self.block_reward)).collect()
};

block_reward::apply_block_rewards(&rewards, block, &self.machine)
Expand Down Expand Up @@ -1624,6 +1639,7 @@ impl Engine for AuthorityRound {
#[cfg(test)]
mod tests {
use std::collections::BTreeMap;
use std::str::FromStr;
use std::sync::Arc;
use std::sync::atomic::{AtomicUsize, Ordering as AtomicOrdering};
use hash::keccak;
Expand All @@ -1644,9 +1660,11 @@ mod tests {
};
use crate::spec::{Spec, self};
use engine::Engine;
use engines::block_reward::BlockRewardContract;
use engines::validator_set::{TestSet, SimpleList};
use super::{AuthorityRoundParams, AuthorityRound, EmptyStep, SealedEmptyStep, calculate_score};
use machine::Machine;
use ethjson;

fn build_aura<F>(f: F) -> Arc<AuthorityRound> where
F: FnOnce(&mut AuthorityRoundParams),
Expand All @@ -1663,8 +1681,7 @@ mod tests {
empty_steps_transition: u64::max_value(),
maximum_empty_steps: 0,
block_reward: Default::default(),
block_reward_contract_transition: 0,
block_reward_contract: Default::default(),
block_reward_contract_transitions: Default::default(),
strict_empty_steps_transition: 0,
};

Expand Down Expand Up @@ -2430,4 +2447,55 @@ mod tests {
set_empty_steps_seal(&mut header, step, &signature, &empty_steps);
assert_eq!(engine.verify_block_family(&header, &parent).unwrap(), ());
}

#[test]
fn should_collect_block_reward_transitions() {
let config = r#"{
"params": {
"stepDuration": "5",
"validators": {
"list" : ["0x1000000000000000000000000000000000000001"]
},
"blockRewardContractTransition": "0",
"blockRewardContractAddress": "0x2000000000000000000000000000000000000002",
"blockRewardContractTransitions": {
"7": "0x3000000000000000000000000000000000000003",
"42": "0x4000000000000000000000000000000000000004"
}
}
}"#;
let deserialized: ethjson::spec::AuthorityRound = serde_json::from_str(config).unwrap();
let params = AuthorityRoundParams::from(deserialized.params);
for ((block_num1, address1), (block_num2, address2)) in
params.block_reward_contract_transitions.iter().zip(
[(0u64, BlockRewardContract::new_from_address(Address::from_str("2000000000000000000000000000000000000002").unwrap())),
(7u64, BlockRewardContract::new_from_address(Address::from_str("3000000000000000000000000000000000000003").unwrap())),
(42u64, BlockRewardContract::new_from_address(Address::from_str("4000000000000000000000000000000000000004").unwrap())),
].iter())
{
assert_eq!(block_num1, block_num2);
assert_eq!(address1, address2);
}
}

#[test]
#[should_panic(expected="blockRewardContractTransition should be less than any of the keys in blockRewardContractTransitions")]
fn should_reject_out_of_order_block_reward_transition() {
let config = r#"{
"params": {
"stepDuration": "5",
"validators": {
"list" : ["0x1000000000000000000000000000000000000001"]
},
"blockRewardContractTransition": "7",
"blockRewardContractAddress": "0x2000000000000000000000000000000000000002",
"blockRewardContractTransitions": {
"0": "0x3000000000000000000000000000000000000003",
"42": "0x4000000000000000000000000000000000000004"
}
}
}"#;
let deserialized: ethjson::spec::AuthorityRound = serde_json::from_str(config).unwrap();
AuthorityRoundParams::from(deserialized.params);
}
}
44 changes: 40 additions & 4 deletions json/src/spec/authority_round.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,31 @@
// You should have received a copy of the GNU General Public License
// along with Parity Ethereum. If not, see <http://www.gnu.org/licenses/>.

//! Authority params deserialization.
//! Authority Round parameter deserialization.
//!
//! Here is an example of input parameters where the step duration is constant at 5 seconds, the set
//! of validators is decided by the contract at address `0x10..01` starting from block 0, and where
//! the address of the contract that computes block rewards is set to `0x20..02` for blocks 0
//! through 41 and to `0x30.03` for all blocks starting from block 42.
//!
//! ```ignore
//! "params": {
//! "stepDuration": "5",
//! "validators": {
//! "multi": {
//! "0": {
//! "contract": "0x1000000000000000000000000000000000000001"
//! }
//! }
//! },
//! "blockRewardContractTransitions": {
//! "0": "0x2000000000000000000000000000000000000002",
//! "42": "0x3000000000000000000000000000000000000003"
//! }
//! }
//! ```

use std::collections::BTreeMap;
use hash::Address;
use uint::Uint;
use bytes::Bytes;
Expand All @@ -41,11 +64,24 @@ pub struct AuthorityRoundParams {
pub immediate_transitions: Option<bool>,
/// Reward per block in wei.
pub block_reward: Option<Uint>,
/// Block at which the block reward contract should start being used.
/// Block at which the block reward contract should start being used. This option allows one to
/// add a single block reward contract transition and is compatible with the multiple address
/// option `block_reward_contract_transitions` below.
pub block_reward_contract_transition: Option<Uint>,
/// Block reward contract address (setting the block reward contract
/// overrides the static block reward definition).
/// Block reward contract address which overrides the `block_reward` setting. This option allows
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This option allows to add
This provides the option to add

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I'm a bit mixed up. Can you clarify? The sentence loses its meaning after "and" if we rewrite it this way. Originally: this option allows and this option is compatible. With the change: this provides an option and this is compatible. In the latter case I'm not sure what "this" is referring to.

Copy link
Copy Markdown

@andogro andogro Aug 20, 2019

Choose a reason for hiding this comment

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

Just attempting to make a complete sentence here rather than change the meaning- maybe would be better to say "this option allows for a single block reward contract..." or "this option allows adding" or another variation that completes the sentence and preserves the meaning?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think @andogro is saying that the wording "…allows to add…" is grammatically a bit off and that it'd improve the text if you reworded it with "…provides the option to add…" (or "set") here and on line 67 above. :)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Would the issue be fixed by changing "...allows to add..." into "...allows one to add..."?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Sure that works too.

/// one to add a single block reward contract address and is compatible with the multiple
/// address option `block_reward_contract_transitions` below.
pub block_reward_contract_address: Option<Address>,
/// Block reward contract addresses with their associated starting block numbers.
///
/// Setting the block reward contract overrides `block_reward`. If the single block reward
/// contract address is also present then it is added into the map at the block number stored in
/// `block_reward_contract_transition` or 0 if that block number is not provided. Therefore both
/// a single block reward contract transition and a map of reward contract transitions can be
/// used simulataneously in the same configuration. In such a case the code requires that the
/// block number of the single transition is strictly less than any of the block numbers in the
/// map.
pub block_reward_contract_transitions: Option<BTreeMap<Uint, Address>>,
/// Block reward code. This overrides the block reward contract address.
pub block_reward_contract_code: Option<Bytes>,
/// Block at which maximum uncle count should be considered.
Expand Down