DAO Rescue soft fork #1309
12 participants
rphmeier
commented on an outdated diff
| @@ -222,6 +222,22 @@ impl State { | ||
| let options = TransactOptions { tracing: tracing, vm_tracing: false, check_nonce: true }; | ||
| let e = try!(Executive::new(self, env_info, engine, vm_factory).transact(t, options)); | ||
| + // dao attack soft fork | ||
| + if engine.schedule(&env_info).block_dao_transactions { | ||
| + // collect all the addresses which have changed. | ||
| + let addresses = self.cache.borrow().deref().iter().map(|(addr, _)| addr.clone()).collect::<Vec<_>>(); |
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
|
arkpar
and 2 others
commented on an outdated diff
| @@ -222,6 +222,22 @@ impl State { | ||
| let options = TransactOptions { tracing: tracing, vm_tracing: false, check_nonce: true }; | ||
| let e = try!(Executive::new(self, env_info, engine, vm_factory).transact(t, options)); | ||
| + // dao attack soft fork | ||
| + if engine.schedule(&env_info).block_dao_transactions { | ||
| + // collect all the addresses which have changed. | ||
| + let addresses = self.cache.borrow().deref().iter().map(|(addr, _)| addr.clone()).collect::<Vec<_>>(); | ||
| + | ||
| + for a in addresses.iter() { | ||
| + if self.code(a).map(|c| c.sha3() == H256::from("7278d050619a624f84f51987149ddb439cdaadfba5966f7cfaea7ad44340a4ba")).unwrap_or(false) { |
|
Move the hash to a constant or at least out of the loop so that it is not converted from hex string on each iteration Should the hash maybe be a list in the case where more hashes need to be added in the coming days/future?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
|
Please don't forget the change in adjusting the block gas limit and the respective trigger, or is that not part of the soft fork anymore?
| @@ -101,7 +101,10 @@ impl Engine for Ethash { | ||
| if env_info.number < self.ethash_params.frontier_compatibility_mode_limit { | ||
| Schedule::new_frontier() | ||
| } else { | ||
| - Schedule::new_homestead() | ||
| + let mut s = Schedule::new_homestead(); | ||
| + // TODO: make dependent on gaslimit > 4000000 of block 1760000. |
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
|
Is it really appropriate to "fix" the result of correct contract execution by the evm within the client? I'd argue this makes for poor separation of responsibilities.
this is merely code. people are free to run it, or not, as they choose.
Why not allow transactions to go back to the DAO? Or is that very complicated?
Or does it do that already by looking whether the balance decreases? ...
Right. This only blocks taking funds out of the DAO.
I might make a revision of the yellow paper with it in if there's enough desire. Right now I don't think there's much point since it's only a soft fork - all this code can be removed once the issue is resolved.
If someone sent a really expensive tx that did lots of computation then at the end of its execution made a call to an contract that contained the DOA codehash, would this potentially slow down miner running this patch? The DOA attacker might have an incentive to do this.
correct. this was discussed already on the security call. it would allow a griefing vector for miners. however, three things mitigate this:
- the soft fork, along with all its logic, is not expected to last longer than a week;
- it is mainly a problem for transactions that would have miners do significant computation prior to discovery that they're invalid (transactors can already grief miners to some degree by spamming transactions with too little ether in the 'from' account). ultimately if this became a problem, miners could include additional logic to automatically drop or deprioritize transactions whose gas is substantial.
- following this PR, a further one to ensure nodes do not relay such transactions should go in which would all but remove the dos effects of this vector.
Thanks for the fast response Gavin! one issue: a large amount of ETH has already been split out into child DAOs by entirely legitimate transactions before the theft today. These legitimate actors are waiting for the 27-day creation period and 14-day proposal period to withdraw their ETH. Your code to block the thief also blocks the legitimate splits? How do the legitimate splits recover their ETH? This is a critical issue.
it is indeed important. however, in general, such DAOs are also susceptible to this same attack.
given that we expect the hard fork to happen, conservatively, within the next two weeks, then it should (a) not block "good splits" from proceeding and (b) ensure that those "good splits" are not themselves compromised by a copycat attack.
Expect it? Have you reached consensus with the community? A lot oppose.. This is centralization. If this were an accounts issues with the protocol I'd be for it, but it's not.
I would like to keep using parity but without this. I disagree on both the sf and hk to recover user funds and I have invested money in TheDAO. Could you refactor this in CLI option? So miner who disagree can run it without
as far as i'm aware, no "consensus" need be reached for me to publish this code.
and no, this is not centralisation, this is called facilitation.
ultimately you are perfectly free to sit on your chain, running an old or modified version of Parity, and idly stand by, chanting verses from the yellow paper, and watch a malicious attacker steal $200m of ether.
completely your choice.
i have not yet made a decision on whether to include such a flag.
right now, i feel that ethcore distributing such a binary, given a solution exists, could be construed as facilitating an attack on a $100m+ "investment facility". however, i also want to help people express themselves as much as possible. as such i'm in a bit of a quandary. i will chat with the team, sleep on it and get back to you tomorrow.
in any case, you are perfectly free to maintain a fork of Parity without this PR merged. it's Free software, after all!
You seem emotionally attached to this. Will these be done for any multi million hacks/thefts as well with a clear address as the culprit?
as is inevitable with such matters, each case must be considered on its individual merits.
Are you going to also include something to revert all the ether sent to null addresses?
closing this conversation it's OT for a PR. if you wish to continue discussing the merits of soft-forks i suggest you take it to your local ethereum meetup group.
Introduces
block_dao_transactionsas a field of Schedule.If true, blocks may not contain transactions which reduce the balance of any contract whose codehash is
7278d050619a624f84f51987149ddb439cdaadfba5966f7cfaea7ad44340a4ba.