-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Conversation
ethcore/src/machine.rs
Outdated
let state = block.state_mut(); | ||
for &(address, ref irregular_account) in irregular_changes { | ||
match irregular_account { | ||
&IrregularStateChangeAccount::Set { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
single arm match can be replaced by an irrefutable destructuring.
let &IrregularStateChangeAccount::Set { nonce, balance, ref code, ref storage } = irregular_account;
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the idea here was to also support other types of "irregular changes" with this format. For example, we might also have IrregularStateChangeAccount::Collect { accounts: Vec<Address>, target: Address }
, collecting all accounts to target (thus support DAO hard fork change). Once we have more than one type we can't use let
any more.
Another issue is that with let
, if the IrregularStateChangeAccount
enum changed to have more variants, it cannot detect it and throw type errors.
So I suggest we keep it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if the IrregularStateChangeAccount enum changed to have more variants, it cannot detect it and throw type errors.
that's not true. only irrefutable pattern matches can be used in a let-binding. adding another variant would make that pattern match no longer irrefutable.
I'm not going to push it any further because it's a small issue, but I don't think we should introduce more verbosity just because we might add different variants in the future.
Also: the DAO hard fork could also be implemented using only the variant we have now; the post-fork balances are historically available now that the fork itself is over.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good although lacks comprehensive test suite. Can we prepare a common test corpus that could be re-used between client implementations?
"0x863DF6BFa4469f3ead0bE8f9F2AAE51c91A907b4": { | ||
"set": { | ||
"nonce": "0x1", | ||
"code": "0x606060405236156100ef576000357c0100000000000000000000000000000000000000000000000000000000900463ffffffff168063173825d91461016d5780632f54bf6e146101a35780634123cb6b146101f157806352375093146102175780635c52c2f51461023d578063659010e71461024f5780637065cb4814610275578063746c9171146102ab578063797af627146102d1578063b20d30a91461030d578063b61d27f61461032d578063b75c7dc61461039c578063ba51a6df146103c0578063c2cf7326146103e0578063c41a360a1461043b578063f00d4b5d1461049b578063f1736d86146104f0575b61016b5b6000341115610168577fe1fffcc4923d04b559f4d29a8bfc6cda04eb5b0d3c460751c2402c5c5cc9109c3334604051808373ffffffffffffffffffffffffffffffffffffffff1673ffffffffffffffffffffffffffffffffffffffff1681526020018281526020019250505060405180910390a15b5b565b005b341561017557fe5b6101a1600480803573ffffffffffffffffffffffffffffffffffffffff16906020019091905050610516565b005b34156101ab57fe5b6101d7600480803573ffffffffffffffffffffffffffffffffffffffff16906020019091905050610659565b604051808215151515815260200191505060405180910390f35b34156101f957fe5b610201610691565b6040518082815260200191505060405180910390f35b341561021f57fe5b610227610697565b6040518082815260200191505060405180910390f35b341561024557fe5b61024d61069d565b005b341561025757fe5b61025f6106d7565b6040518082815260200191505060405180910390f35b341561027d57fe5b6102a9600480803573ffffffffffffffffffffffffffffffffffffffff169060200190919050506106dd565b005b34156102b357fe5b6102bb610829565b6040518082815260200191505060405180910390f35b34156102d957fe5b6102f360048080356000191690602001909190505061082f565b604051808215151515815260200191505060405180910390f35b341561031557fe5b61032b6004808035906020019091905050610dcc565b005b341561033557fe5b61037e600480803573ffffffffffffffffffffffffffffffffffffffff169060200190919080359060200190919080359060200190820180359060200191909192905050610e06565b60405180826000191660001916815260200191505060405180910390f35b34156103a457fe5b6103be60048080356000191690602001909190505061127d565b005b34156103c857fe5b6103de6004808035906020019091905050611392565b005b34156103e857fe5b61042160048080356000191690602001909190803573ffffffffffffffffffffffffffffffffffffffff1690602001909190505061141a565b604051808215151515815260200191505060405180910390f35b341561044357fe5b610459600480803590602001909190505061149c565b604051808273ffffffffffffffffffffffffffffffffffffffff1673ffffffffffffffffffffffffffffffffffffffff16815260200191505060405180910390f35b34156104a357fe5b6104ee600480803573ffffffffffffffffffffffffffffffffffffffff1690602001909190803573ffffffffffffffffffffffffffffffffffffffff169060200190919050506114bf565b005b34156104f857fe5b610500611672565b6040518082815260200191505060405180910390f35b600060003660405180838380828437820191505092505050604051809103902061053f81611678565b156106535761010560008473ffffffffffffffffffffffffffffffffffffffff168152602001908152602001600020549150600082141561057f57610652565b600160015403600054111561059357610652565b6000600583610100811015156105a557fe5b0160005b5081905550600061010560008573ffffffffffffffffffffffffffffffffffffffff168152602001908152602001600020819055506105e6611890565b6105ee6119d0565b7f58619076adf5bb0943d100ef88d52d7c3fd691b19d3a9071b555b651fbf418da83604051808273ffffffffffffffffffffffffffffffffffffffff1673ffffffffffffffffffffffffffffffffffffffff16815260200191505060405180910390a15b5b5b505050565b6000600061010560008473ffffffffffffffffffffffffffffffffffffffff168152602001908152602001600020541190505b919050565b60015481565b60045481565b6000366040518083838082843782019150509250505060405180910390206106c481611678565b156106d35760006003819055505b5b5b50565b60035481565b60003660405180838380828437820191505092505050604051809103902061070481611678565b156108245761071282610659565b1561071c57610823565b610724611890565b60fa600154101515610739576107386119d0565b5b60fa60015410151561074a57610823565b6001600081548092919060010191905055508173ffffffffffffffffffffffffffffffffffffffff1660056001546101008110151561078557fe5b0160005b508190555060015461010560008473ffffffffffffffffffffffffffffffffffffffff168152602001908152602001600020819055507f994a936646fe87ffe4f1e469d3d6aa417d6b855598397f323de5b449f765f0c382604051808273ffffffffffffffffffffffffffffffffffffffff1673ffffffffffffffffffffffffffffffffffffffff16815260200191505060405180910390a15b5b5b5050565b60005481565b600060008261083d81611678565b15610dc45760006101086000866000191660001916815260200190815260200160002060000160009054906101000a900473ffffffffffffffffffffffffffffffffffffffff1673ffffffffffffffffffffffffffffffffffffffff161415806108c757506000610108600086600019166000191681526020019081526020016000206001015414155b80610906575060006101086000866000191660001916815260200190815260200160002060020180546001816001161561010002031660029004905014155b15610dc25760006101086000866000191660001916815260200190815260200160002060000160009054906101000a900473ffffffffffffffffffffffffffffffffffffffff1673ffffffffffffffffffffffffffffffffffffffff161415610a5057610a496101086000866000191660001916815260200190815260200160002060010154610108600087600019166000191681526020019081526020016000206002018054600181600116156101000203166002900480601f016020809104026020016040519081016040528092919081815260200182805460018160011615610100020316600290048015610a3f5780601f10610a1457610100808354040283529160200191610a3f565b820191906000526020600020905b815481529060010190602001808311610a2257829003601f168201915b5050505050611b37565b9150610b71565b6101086000856000191660001916815260200190815260200160002060000160009054906101000a900473ffffffffffffffffffffffffffffffffffffffff1673ffffffffffffffffffffffffffffffffffffffff166101086000866000191660001916815260200190815260200160002060010154610108600087600019166000191681526020019081526020016000206002016040518082805460018160011615610100020316600290048015610b4a5780601f10610b1f57610100808354040283529160200191610b4a565b820191906000526020600020905b815481529060010190602001808311610b2d57829003601f168201915b505091505060006040518083038185876185025a03f1925050501515610b705760006000fd5b5b7fe3a3a4111a84df27d76b68dc721e65c7711605ea5eee4afd3a9c58195217365c338561010860008860001916600019168152602001908152602001600020600101546101086000896000191660001916815260200190815260200160002060000160009054906101000a900473ffffffffffffffffffffffffffffffffffffffff1661010860008a6000191660001916815260200190815260200160002060020187604051808773ffffffffffffffffffffffffffffffffffffffff1673ffffffffffffffffffffffffffffffffffffffff16815260200186600019166000191681526020018581526020018473ffffffffffffffffffffffffffffffffffffffff1673ffffffffffffffffffffffffffffffffffffffff168152602001806020018373ffffffffffffffffffffffffffffffffffffffff1673ffffffffffffffffffffffffffffffffffffffff168152602001828103825284818154600181600116156101000203166002900481526020019150805460018160011615610100020316600290048015610d475780601f10610d1c57610100808354040283529160200191610d47565b820191906000526020600020905b815481529060010190602001808311610d2a57829003601f168201915b505097505050505050505060405180910390a16101086000856000191660001916815260200190815260200160002060006000820160006101000a81549073ffffffffffffffffffffffffffffffffffffffff02191690556001820160009055600282016000610db79190611be6565b505060019250610dc3565b5b5b5b5050919050565b600036604051808383808284378201915050925050506040518091039020610df381611678565b15610e0157816002819055505b5b5b5050565b60006000610e1333610659565b1561127357600084849050148015610e305750610e2f85611b51565b5b80610e3d57506001600054145b15610fed5760008673ffffffffffffffffffffffffffffffffffffffff161415610ea457610e9d8585858080601f016020809104026020016040519081016040528093929190818152602001838380828437820191505050505050611b37565b9050610ef3565b8573ffffffffffffffffffffffffffffffffffffffff168585856040518083838082843782019150509250505060006040518083038185876185025a03f1925050501515610ef25760006000fd5b5b7f9738cd1a8777c86b011f7b01d87d484217dc6ab5154a9d41eda5d14af8caf292338688878786604051808773ffffffffffffffffffffffffffffffffffffffff1673ffffffffffffffffffffffffffffffffffffffff1681526020018681526020018573ffffffffffffffffffffffffffffffffffffffff1673ffffffffffffffffffffffffffffffffffffffff168152602001806020018373ffffffffffffffffffffffffffffffffffffffff1673ffffffffffffffffffffffffffffffffffffffff1681526020018281038252858582818152602001925080828437820191505097505050505050505060405180910390a1611271565b6000364360405180848480828437820191505082815260200193505050506040518091039020915060006101086000846000191660001916815260200190815260200160002060000160009054906101000a900473ffffffffffffffffffffffffffffffffffffffff1673ffffffffffffffffffffffffffffffffffffffff16148015611099575060006101086000846000191660001916815260200190815260200160002060010154145b80156110d85750600061010860008460001916600019168152602001908152602001600020600201805460018160011615610100020316600290049050145b1561118f57856101086000846000191660001916815260200190815260200160002060000160006101000a81548173ffffffffffffffffffffffffffffffffffffffff021916908373ffffffffffffffffffffffffffffffffffffffff160217905550846101086000846000191660001916815260200190815260200160002060010181905550838361010860008560001916600019168152602001908152602001600020600201919061118d929190611c2e565b505b6111988261082f565b1515611270577f1733cbb53659d713b79580f79f3f9ff215f78a7c7aa45890f3b89fc5cddfbf328233878988886040518087600019166000191681526020018673ffffffffffffffffffffffffffffffffffffffff1673ffffffffffffffffffffffffffffffffffffffff1681526020018581526020018473ffffffffffffffffffffffffffffffffffffffff1673ffffffffffffffffffffffffffffffffffffffff168152602001806020018281038252848482818152602001925080828437820191505097505050505050505060405180910390a15b5b5b5b5b50949350505050565b60006000600061010560003373ffffffffffffffffffffffffffffffffffffffff16815260200190815260200160002054925060008314156112be5761138c565b8260020a9150610106600085600019166000191681526020019081526020016000209050600082826001015416111561138b5780600001600081548092919060010191905055508181600101600082825403925050819055507fc7fb647e59b18047309aa15aad418e5d7ca96d173ad704f1031a2c3d7591734b3385604051808373ffffffffffffffffffffffffffffffffffffffff1673ffffffffffffffffffffffffffffffffffffffff16815260200182600019166000191681526020019250505060405180910390a15b5b50505050565b6000366040518083838082843782019150509250505060405180910390206113b981611678565b15611415576001548211156113cd57611414565b816000819055506113dc611890565b7facbdb084c721332ac59f9b8e392196c9eb0e4932862da8eb9beaf0dad4f550da826040518082815260200191505060405180910390a15b5b5b5050565b600060006000600061010660008760001916600019168152602001908152602001600020925061010560008673ffffffffffffffffffffffffffffffffffffffff168152602001908152602001600020549150600082141561147f5760009350611493565b8160020a9050600081846001015416141593505b50505092915050565b6000600560018301610100811015156114b157fe5b0160005b505490505b919050565b60006000366040518083838082843782019150509250505060405180910390206114e881611678565b1561166b576114f683610659565b156115005761166a565b61010560008573ffffffffffffffffffffffffffffffffffffffff168152602001908152602001600020549150600082141561153b5761166a565b611543611890565b8273ffffffffffffffffffffffffffffffffffffffff166005836101008110151561156a57fe5b0160005b5081905550600061010560008673ffffffffffffffffffffffffffffffffffffffff168152602001908152602001600020819055508161010560008573ffffffffffffffffffffffffffffffffffffffff168152602001908152602001600020819055507fb532073b38c83145e3e5135377a08bf9aab55bc0fd7c1179cd4fb995d2a5159c8484604051808373ffffffffffffffffffffffffffffffffffffffff1673ffffffffffffffffffffffffffffffffffffffff1681526020018273ffffffffffffffffffffffffffffffffffffffff1673ffffffffffffffffffffffffffffffffffffffff1681526020019250505060405180910390a15b5b5b50505050565b60025481565b600060006000600061010560003373ffffffffffffffffffffffffffffffffffffffff16815260200190815260200160002054925060008314156116bb57611888565b6101066000866000191660001916815260200190815260200160002091506000826000015414156117455760005482600001819055506000826001018190555061010780548091906001016117109190611cae565b826002018190555084610107836002015481548110151561172d57fe5b906000526020600020900160005b5081600019169055505b8260020a90506000818360010154161415611887577fe1c52dc63b719ade82e8bea94cc41a0d5d28e4aaf536adb5e9cccc9ff8c1aeda3386604051808373ffffffffffffffffffffffffffffffffffffffff1673ffffffffffffffffffffffffffffffffffffffff16815260200182600019166000191681526020019250505060405180910390a16001826000015411151561185e57610107610106600087600019166000191681526020019081526020016000206002015481548110151561180a57fe5b906000526020600020900160005b5060009055610106600086600019166000191681526020019081526020016000206000600082016000905560018201600090556002820160009055505060019350611888565b8160000160008154809291906001900391905055508082600101600082825417925050819055505b5b5b505050919050565b60006000610107805490509150600090505b818110156119bc576101086000610107838154811015156118bf57fe5b906000526020600020900160005b50546000191660001916815260200190815260200160002060006000820160006101000a81549073ffffffffffffffffffffffffffffffffffffffff021916905560018201600090556002820160006119269190611be6565b505060006001026101078281548110151561193d57fe5b906000526020600020900160005b5054600019161415156119b05761010660006101078381548110151561196d57fe5b906000526020600020900160005b505460001916600019168152602001908152602001600020600060008201600090556001820160009055600282016000905550505b5b8060010190506118a2565b61010760006119cb9190611cda565b5b5050565b6000600190505b600154811015611b33575b60015481108015611a095750600060058261010081101515611a0057fe5b0160005b505414155b15611a1b5780806001019150506119e2565b5b6001600154118015611a4557506000600560015461010081101515611a3d57fe5b0160005b5054145b15611a625760016000815480929190600190039190505550611a1c565b60015481108015611a8b57506000600560015461010081101515611a8257fe5b0160005b505414155b8015611aac5750600060058261010081101515611aa457fe5b0160005b5054145b15611b2e57600560015461010081101515611ac357fe5b0160005b505460058261010081101515611ad957fe5b0160005b508190555080610105600060058461010081101515611af857fe5b0160005b50548152602001908152602001600020819055506000600560015461010081101515611b2457fe5b0160005b50819055505b6119d7565b5b50565b600081516020830184f09050803b15610000575b92915050565b6000611b5c33610659565b15611bc957600454611b6c611bcf565b1115611b89576000600381905550611b82611bcf565b6004819055505b600354826003540110158015611ba55750600254826003540111155b15611bc3578160036000828254019250508190555060019050611bc8565b600090505b5b5b919050565b60006201518042811515611bdf57fe5b0490505b90565b50805460018160011615610100020316600290046000825580601f10611c0c5750611c2b565b601f016020900490600052602060002090810190611c2a9190611cfc565b5b50565b828054600181600116156101000203166002900490600052602060002090601f016020900481019282601f10611c6f57803560ff1916838001178555611c9d565b82800160010185558215611c9d579182015b82811115611c9c578235825591602001919060010190611c81565b5b509050611caa9190611cfc565b5090565b815481835581811511611cd557818360005260206000209182019101611cd49190611d21565b5b505050565b5080546000825590600052602060002090810190611cf89190611d21565b5b50565b611d1e91905b80821115611d1a576000816000905550600101611d02565b5090565b90565b611d4391905b80821115611d3f576000816000905550600101611d27565b5090565b90565b60006001541115611d575760006000fd5b611d6081611d71565b611d6a8383611d9c565b5b5b505050565b60006001541115611d825760006000fd5b80600281905550611d91611bcf565b6004819055505b5b50565b600060006001541115611daf5760006000fd5b600082111515611dbf5760006000fd5b81835110151515611dd05760006000fd5b8251600181905550600090505b8251811015611e85578281815181101515611df457fe5b9060200190602002015173ffffffffffffffffffffffffffffffffffffffff1660058260010161010081101515611e2757fe5b0160005b50819055508060010161010560008584815181101515611e4757fe5b9060200190602002015173ffffffffffffffffffffffffffffffffffffffff168152602001908152602001600020819055505b806001019050611ddd565b816000819055505b5b5050505600a165627a7a7230582084feb3505964efa62a6ffa78d913a175fe7bc168dd50067d25b1d5ddb6d10a1e0029", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The code seems to be compiled without optimizations.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The code might be not final yet. I had issues verifying the optimized version on Etherscan, so I am working with this for now and will spend some more energy on this once the contract is in it's final reviewed and tested state.
ethcore/src/spec/spec.rs
Outdated
@@ -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(), |
There was a problem hiding this comment.
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()
ethcore/src/state/mod.rs
Outdated
@@ -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<()> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/bala/balance
json/src/spec/params.rs
Outdated
#[serde(rename="set")] | ||
Set { | ||
/// See main CommonParams docs. | ||
#[serde(rename="nonce")] |
There was a problem hiding this comment.
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.
/// See main CommonParams docs. | ||
#[serde(rename="set")] | ||
Set { | ||
/// See main CommonParams docs. |
There was a problem hiding this comment.
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)
ethcore/src/spec/spec.rs
Outdated
@@ -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. | |||
pub irregular_state_changes: HashMap<u64, Vec<(Address, IrregularStateChangeAccount)>>, |
There was a problem hiding this comment.
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?
@@ -0,0 +1,86 @@ | |||
{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The file doesn't seem to be used anywhere. Would be really awesome to get a unit test for this.
ethcore/src/machine.rs
Outdated
@@ -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()) { |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
ethcore/src/spec/spec.rs
Outdated
@@ -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. |
There was a problem hiding this comment.
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.
@sandakersmann not particularly constructive on the code changes or in general. this is a broadly useful PR for private blockchains, reimplementing the DAO hard-fork in a more general way, etc. |
The PR discussion should be used for technical comments only and not as a soapbox. Please take discussion on use-cases of this PR to the correct places. |
This PR is not changing the foundation chain spec. This has to be discussed somewhere else. |
Shall we proceed to implement this ignoring EIP-999 for now and also refactor the DAO fork as irregular state change? Otherwise, I do not see how this PR could progress in near future. |
I think we should implement this anyway, it's a good refactoring of the codebase. |
@sorpaas had some great ideas how to implement this in the
This would allow rewriting the DAO fork in a very smooth way, and both Casper FFG (EIP-1011) and Blockhash refactoring (EIP-210) could benefit from such changes. |
That approach sounds fine too, as long as it's easy to make multiple changes to one account at different points. The current method makes it really easy to see which changes are grouped together to be applied at a specific block. |
Add support for an irregular state change in chain spec (CommonParams). Add example implementation for EIP-999, including test chain spec. Could be used as groundwork to refactor the DAO fork changes (EIP-779) as well, ref. #1483.