Add more comments, require randomness contract map.

ethcore/engines/authority-round/src/lib.rs

@@ -176,17 +176,11 @@ impl From<ethjson::spec::AuthorityRoundParams> for AuthorityRoundParams {
 				BlockRewardContract::new_from_address(address.into())
 			);
 		}
-		let randomness_contract_address = match p.randomness_contract_address {
-			None => BTreeMap::new(),
-			Some(ethjson::spec::authority_round::TransitionMap::Single(addr)) => {
-				iter::once((0, addr.into())).collect()
-			}
-			Some(ethjson::spec::authority_round::TransitionMap::Transitions(transitions)) => {
-				transitions.into_iter().map(|(ethjson::uint::Uint(block), addr)| {
-					(block.as_u64(), addr.into())
-				}).collect()
-			}
-		};
+		let randomness_contract_address = p.randomness_contract_address.map_or_else(BTreeMap::new, |transitions| {
+			transitions.into_iter().map(|(ethjson::uint::Uint(block), addr)| {
+				(block.as_u64(), addr.into())
+			}).collect()
+		});
 		AuthorityRoundParams {
 			step_durations,
 			validators: new_validator_set(p.validators),

ethcore/engines/authority-round/src/randomness.rs

@@ -38,7 +38,7 @@
 //!        |                                              |
 //!        |  call                                        |
 //!        |  `commitHash()`                              |  call
-//!        |                                              |  `revealNumber`
+//!        |                                              |  `revealNumber()`
 //!        |                                              |
 //! +------v-------+                              +-------+-------+
 //! |              |                              |               |
@@ -49,6 +49,18 @@
 //!
 //! Phase transitions are performed by the smart contract and simply queried by the engine.
 //!
+//! Randomness generation works as follows:
+//! * During the commit phase, all validators locally generate a random number, and commit that number's hash to the
+//!   contract.
+//! * During the reveal phase, all validators reveal their local random number to the contract. The contract should
+//!   verify that it matches the committed hash.
+//! * Finally, the XOR of all revealed numbers is used as an on-chain random number.
+//!
+//! An adversary can only influence that number by either controlling _all_ validators who committed, or, to a lesser
+//! extent, by not revealing committed numbers.
+//! The length of the commit and reveal phases, as well as any penalties for failure to reveal, are defined by the
+//! contract.
+//!
 //! A typical case of using `RandomnessPhase` is:
 //!
 //! 1. `RandomnessPhase::load()` the phase from the blockchain data.
@@ -84,7 +96,7 @@ pub enum RandomnessPhase {
 	/// Waiting for the next phase.
 	///
 	/// This state indicates either the successful revelation in this round or having missed the
-	/// window to make a commitment.
+	/// window to make a commitment, i.e. having failed to commit during the commit phase.
 	Waiting,
 	/// Indicates a commitment is possible, but still missing.
 	BeforeCommit,
@@ -102,9 +114,6 @@ pub enum RandomnessPhase {
 /// `BadRandNumber` will usually result in punishment by the contract or the other validators.
 #[derive(Debug, Display)]
 pub enum PhaseError {
-	/// The smart contract reported a phase as both commitment and reveal phase.
-	#[display(fmt = "Inconsistent randomness phase information")]
-	PhaseConflict,
 	/// The smart contract reported that we already revealed something while still being in the
 	/// commit phase.
 	#[display(fmt = "Revealed during commit phase")]
@@ -190,8 +199,8 @@ impl RandomnessPhase {
 	///
 	/// Returns the encoded contract call necessary to advance the randomness contract's state.
 	///
-	/// **Warning**: The `advance()` function should be called only once per block state; otherwise
-	///              spurious transactions resulting in punishments might be executed.
+	/// **Warning**: After calling the `advance()` function, wait until the returned transaction has been included in
+	/// a block before calling it again; otherwise spurious transactions resulting in punishments might be executed.
 	pub fn advance<R: Rng>(
 		self,
 		contract: &BoundContract,

ethcore/engines/authority-round/src/util.rs

@@ -68,11 +68,12 @@ impl<'a> BoundContract<'a> {
 		}
 	}
 
-	/// Perform a function call to an ethereum machine that doesn't create a transaction or change the state.
+	/// Perform a function call to an Ethereum machine that doesn't create a transaction or change the state.
 	///
 	/// Runs a constant function call on `client`. The `call` value can be serialized by calling any
 	/// api function generated by the `use_contract!` macro. This does not create any transactions, it only produces a
-	/// result based on the state at the current block.
+	/// result based on the state at the current block: It is constant in the sense that it does not alter the EVM
+	/// state.
 	pub fn call_const<D>(&self, call: (ethabi::Bytes, D)) -> Result<D::Output, CallError>
 	where
 		D: ethabi::FunctionOutputDecoder,

ethcore/res/authority_round_randomness_contract.json

@@ -12,7 +12,9 @@
 				},
 				"immediateTransitions": true,
 				"maximumEmptySteps": "2",
-				"randomnessContractAddress": "0x0000000000000000000000000000000000000042"
+				"randomnessContractAddress": {
+					"0": "0x0000000000000000000000000000000000000042"
+				}
 			}
 		}
 	},

json/src/spec/authority_round.rs

@@ -96,7 +96,7 @@ pub struct AuthorityRoundParams {
 	/// First block for which a 2/3 quorum (instead of 1/2) is required.
 	pub two_thirds_majority_transition: Option<Uint>,
 	/// The random number contract's address, or a map of contract transitions.
-	pub randomness_contract_address: Option<TransitionMap<Address>>,
+	pub randomness_contract_address: Option<BTreeMap<Uint, Address>>,
 }
 
 /// Authority engine deserialization.
@@ -107,17 +107,6 @@ pub struct AuthorityRound {
 	pub params: AuthorityRoundParams,
 }
 
-/// Either a single `T` value, or a map assigning block numbers to transitions to different values of type `T`.
-#[derive(Debug, PartialEq, Deserialize)]
-#[serde(deny_unknown_fields)]
-#[serde(untagged)]
-pub enum TransitionMap<T> {
-	/// A single value for all block numbers.
-	Single(Address),
-	/// A map of transition block numbers to new values.
-	Transitions(BTreeMap<Uint, T>),
-}
-
 #[cfg(test)]
 mod tests {
 	use super::{Address, Uint, StepDuration};
@@ -156,9 +145,9 @@ mod tests {
 		assert_eq!(deserialized.params.maximum_uncle_count_transition, Some(Uint(10_000_000.into())));
 		assert_eq!(deserialized.params.maximum_uncle_count, Some(Uint(5.into())));
 		assert_eq!(deserialized.params.randomness_contract_address.unwrap(),
-			super::TransitionMap::Transitions(vec![
+			vec![
 				(Uint(10.into()), Address(H160::from_str("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa").unwrap())),
 				(Uint(20.into()), Address(H160::from_str("bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb").unwrap())),
-			].into_iter().collect()));
+			].into_iter().collect());
 	}
 }