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

Non-canonical state-change parameter #8406

Closed
wants to merge 7 commits into from
Closed
Show file tree
Hide file tree
Changes from 6 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
86 changes: 86 additions & 0 deletions ethcore/res/ethereum/multisig_test.json

Large diffs are not rendered by default.

22 changes: 21 additions & 1 deletion ethcore/src/machine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ use client::{BlockInfo, CallContract};
use error::Error;
use executive::Executive;
use header::{BlockNumber, Header};
use spec::CommonParams;
use spec::{CommonParams, IrregularStateChangeAccount};
use state::{CleanupMode, Substate};
use trace::{NoopTracer, NoopVMTracer, Tracer, ExecutiveTracer, RewardType, Tracing};
use transaction::{self, SYSTEM_ADDRESS, UnverifiedTransaction, SignedTransaction};
Expand Down Expand Up @@ -193,6 +193,26 @@ impl EthereumMachine {
.and_then(|b| state.transfer_balance(child, beneficiary, &b, CleanupMode::NoEmpty))?;
}
}

if let Some(irregular_changes) = self.params.irregular_state_changes.get(&block.header().number()) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

This block does not depend on ethash_params, does it have to be inside that particular if? If yes could you please add a comment explaining that.

Copy link
Contributor

Choose a reason for hiding this comment

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

definitely should be moved outside. other engines might have irregular state changes too.

let state = block.state_mut();
for &(address, ref irregular_account) in irregular_changes {
let &IrregularStateChangeAccount::Set { nonce, balance, ref code, ref storage } = irregular_account;

if let Some(nonce) = nonce {
state.set_nonce(&address, &nonce)?;
}
if let Some(balance) = balance {
state.set_balance(&address, &balance)?;
}
if let &Some(ref code) = code {
state.reset_code(&address, code.clone())?;
}
for &(key, value) in storage {
state.set_storage(&address, key, value)?;
}
}
}
}

Ok(())
Expand Down
2 changes: 1 addition & 1 deletion ethcore/src/spec/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,4 +21,4 @@ mod seal;
mod spec;

pub use self::genesis::Genesis;
pub use self::spec::{Spec, SpecHardcodedSync, SpecParams, CommonParams, OptimizeFor};
pub use self::spec::{Spec, SpecHardcodedSync, SpecParams, CommonParams, OptimizeFor, IrregularStateChangeAccount};
38 changes: 37 additions & 1 deletion ethcore/src/spec/spec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@

//! Parameters for a block chain.

use std::collections::BTreeMap;
use std::collections::{BTreeMap, HashMap};
use std::io::Read;
use std::path::Path;
use std::sync::Arc;
Expand Down Expand Up @@ -53,6 +53,39 @@ fn fmt_err<F: ::std::fmt::Display>(f: F) -> String {
format!("Spec json is invalid: {}", f)
}

/// Ireegular state change accounts applied at specific blocks.
#[derive(Debug, Clone, PartialEq)]
pub enum IrregularStateChangeAccount {
/// Force setting values on an account.
Set {
/// New nonce forced setting.
nonce: Option<U256>,
/// New code forced setting.
code: Option<Bytes>,
/// New balance forced setting.
balance: Option<U256>,
/// Storage values forced setting.
storage: Vec<(H256, H256)>,
}
}

impl From<::ethjson::spec::IrregularStateChangeAccount> for IrregularStateChangeAccount {
fn from(p: ::ethjson::spec::IrregularStateChangeAccount) -> Self {
match p {
::ethjson::spec::IrregularStateChangeAccount::Set {
nonce, code, balance, storage
} => {
IrregularStateChangeAccount::Set {
nonce: nonce.map(Into::into),
code: code.map(Into::into),
balance: balance.map(Into::into),
storage: storage.unwrap_or_default().into_iter().map(|(k, v)| (k.into(), v.into())).collect(),
}
},
}
}
}

/// Parameters common to ethereum-like blockchains.
/// NOTE: when adding bugfix hard-fork parameters,
/// add to `contains_bugfix_hard_fork`
Expand Down Expand Up @@ -123,6 +156,8 @@ pub struct CommonParams {
pub max_code_size_transition: BlockNumber,
/// Transaction permission managing contract address.
pub transaction_permission_contract: Option<Address>,
/// Irregular state change list.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Would be good to mention that the irregular state changes are applied at the very beginning of the block to avoid confusion.

pub irregular_state_changes: HashMap<u64, Vec<(Address, IrregularStateChangeAccount)>>,
Copy link
Collaborator

Choose a reason for hiding this comment

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

What does u64 represent here? Maybe use BlockNumber instead?

}

impl CommonParams {
Expand Down Expand Up @@ -244,6 +279,7 @@ impl From<ethjson::spec::Params> for CommonParams {
BlockNumber::max_value(),
Into::into
),
irregular_state_changes: p.irregular_state_changes.unwrap_or_else(HashMap::new).into_iter().map(|(k, v)| (k.into(), v.into_iter().map(|(k, v)| (k.into(), v.into())).collect())).collect(),
Copy link
Collaborator

Choose a reason for hiding this comment

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

Perhaps split it into multiple lines for readability?

p.irregular_state_changes
  .unwrap_or_else(HashMap::new)
  .into_iter()
  .map(|(k, v)| (
    k.into(),
    v.into_iter().map(|(k, v)| (k.into(), v.into())).collect()
  ))
  .collect()

}
}
}
Expand Down
10 changes: 10 additions & 0 deletions ethcore/src/state/account.rs
Original file line number Diff line number Diff line change
Expand Up @@ -349,6 +349,11 @@ impl Account {
self.nonce = self.nonce + U256::from(1u8);
}

/// Set the nonce of the account to a particular value.
pub fn set_nonce(&mut self, nonce: &U256) {
self.nonce = *nonce;
}

/// Increase account balance.
pub fn add_balance(&mut self, x: &U256) {
self.balance = self.balance + *x;
Expand All @@ -361,6 +366,11 @@ impl Account {
self.balance = self.balance - *x;
}

/// Directly set the account balance.
pub fn set_balance(&mut self, x: &U256) {
self.balance = *x;
}

/// Commit the `storage_changes` to the backing DB and update `storage_root`.
pub fn commit_storage(&mut self, trie_factory: &TrieFactory, db: &mut HashDB) -> trie::Result<()> {
let mut t = trie_factory.from_existing(db, &mut self.storage_root)?;
Expand Down
11 changes: 11 additions & 0 deletions ethcore/src/state/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -652,6 +652,12 @@ impl<B: Backend> State<B> {
Ok(())
}

/// Directly set the balance of account `a`.
pub fn set_balance(&mut self, a: &Address, bala: &U256) -> trie::Result<()> {
Copy link
Collaborator

Choose a reason for hiding this comment

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

s/bala/balance

self.require(a, false)?.set_balance(bala);
Ok(())
}

/// Subtracts `by` from the balance of `from` and adds it to that of `to`.
pub fn transfer_balance(&mut self, from: &Address, to: &Address, by: &U256, mut cleanup_mode: CleanupMode) -> trie::Result<()> {
self.sub_balance(from, by, &mut cleanup_mode)?;
Expand All @@ -664,6 +670,11 @@ impl<B: Backend> State<B> {
self.require(a, false).map(|mut x| x.inc_nonce())
}

/// Set the nonce of an account.
pub fn set_nonce(&mut self, a: &Address, nonce: &U256) -> trie::Result<()> {
self.require(a, false).map(|mut x| x.set_nonce(nonce))
}

/// Mutate storage of account `a` so that it is `value` for `key`.
pub fn set_storage(&mut self, a: &Address, key: H256, value: H256) -> trie::Result<()> {
trace!(target: "state", "set_storage({}:{:x} to {:x})", a, key, value);
Expand Down
2 changes: 1 addition & 1 deletion json/src/spec/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ pub mod hardcoded_sync;
pub use self::account::Account;
pub use self::builtin::{Builtin, Pricing, Linear};
pub use self::genesis::Genesis;
pub use self::params::Params;
pub use self::params::{Params, IrregularStateChangeAccount};
pub use self::spec::Spec;
pub use self::seal::{Seal, Ethereum, AuthorityRoundSeal, TendermintSeal};
pub use self::engine::Engine;
Expand Down
28 changes: 28 additions & 0 deletions json/src/spec/params.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,35 @@

//! Spec params deserialization.

use std::collections::HashMap;
use uint::{self, Uint};
use hash::{H256, Address};
use bytes::Bytes;

/// See main CommonParams docs.
#[derive(Clone, Debug, PartialEq, Deserialize)]
pub enum IrregularStateChangeAccount {
/// See main CommonParams docs.
#[serde(rename="set")]
Set {
/// See main CommonParams docs.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not super clear from CommonParams docs what it does. It seems safe explanatory, but I would add a comment here (especially for storage - it should clearly state that the storage entries are added and it doesn't replace existing storage entries)

#[serde(rename="nonce")]
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 the renames are actually required.

nonce: Option<Uint>,

/// See main CommonParams docs.
#[serde(rename="code")]
code: Option<Bytes>,

/// See main CommonParams docs.
#[serde(rename="balance")]
balance: Option<Uint>,

/// See main CommonParams docs.
#[serde(rename="storage")]
storage: Option<HashMap<H256, H256>>,
}
}

/// Spec params.
#[derive(Debug, PartialEq, Deserialize)]
pub struct Params {
Expand Down Expand Up @@ -122,6 +147,9 @@ pub struct Params {
/// Wasm activation block height, if not activated from start
#[serde(rename="wasmActivationTransition")]
pub wasm_activation_transition: Option<Uint>,
/// See main CommonParams docs.
#[serde(rename="irregularStateChanges")]
pub irregular_state_changes: Option<HashMap<Uint, HashMap<Address, IrregularStateChangeAccount>>>,
}

#[cfg(test)]
Expand Down