DAO Rescue soft fork #1309

gavofyork · 2016-06-17T12:55:41Z

Introduces block_dao_transactions as a field of Schedule.

If true, blocks may not contain transactions which reduce the balance of any contract whose codehash is 7278d050619a624f84f51987149ddb439cdaadfba5966f7cfaea7ad44340a4ba.

rphmeier · 2016-06-17T13:05:11Z

ethcore/src/state.rs

+		// 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<_>>();


unnecessary deref() call

obviously a non-blocker :)

chriseth · 2016-06-17T13:21:49Z

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?

gavofyork · 2016-06-17T13:29:00Z

ethcore/src/ethereum/ethash.rs

@@ -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.	


@chriseth trigger still todo ^^^

julian1 · 2016-06-17T13:48:46Z

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.

gavofyork · 2016-06-17T13:54:28Z

this is merely code. people are free to run it, or not, as they choose.

julian1 · 2016-06-17T13:56:36Z

Will the yellow paper be revised to specify the new behavior?

seweso · 2016-06-17T14:29:57Z

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? ...

gavofyork · 2016-06-17T14:54:36Z

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.

wanderer · 2016-06-17T17:11:41Z

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.

gavofyork · 2016-06-17T17:33:40Z

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.

john8841 · 2016-06-17T18:04:10Z

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.

gavofyork · 2016-06-17T18:10:44Z

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.

juscamarena · 2016-06-17T19:43:06Z

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.

marcogiglio · 2016-06-17T19:51:34Z

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

gavofyork · 2016-06-17T19:53:43Z

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.

gavofyork · 2016-06-17T19:58:47Z

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!

juscamarena · 2016-06-17T20:16:04Z

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?

gavofyork · 2016-06-17T20:20:54Z

as is inevitable with such matters, each case must be considered on its individual merits.

juscamarena · 2016-06-17T20:22:03Z

Are you going to also include something to revert all the ether sent to null addresses?

gavofyork · 2016-06-17T20:24:18Z

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.

DAO Rescue soft fork

70430c1

gavofyork added the A0-pleasereview 🤓 Pull request needs code review. label Jun 17, 2016

rphmeier reviewed Jun 17, 2016
View reviewed changes

gavofyork reviewed Jun 17, 2016
View reviewed changes

Address minor issues.

847b952

gavofyork added A0-pleasereview 🤓 Pull request needs code review. and removed A6-mustntgrumble 💦 Pull request has areas for improvement. The author need not address them before merging. labels Jun 17, 2016

arkpar added A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. labels Jun 17, 2016

Merge branch 'master' into daosoftfork

1ea9e59

Fix tests.

8b1c996

gavofyork merged commit 16412eb into master Jun 17, 2016

gavofyork deleted the daosoftfork branch June 17, 2016 20:15

openethereum locked and limited conversation to collaborators Jun 17, 2016

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

DAO Rescue soft fork #1309

DAO Rescue soft fork #1309

gavofyork commented Jun 17, 2016

rphmeier Jun 17, 2016

chriseth commented Jun 17, 2016

gavofyork Jun 17, 2016

julian1 commented Jun 17, 2016

gavofyork commented Jun 17, 2016

julian1 commented Jun 17, 2016

seweso commented Jun 17, 2016 •

edited

Loading

gavofyork commented Jun 17, 2016

wanderer commented Jun 17, 2016 •

edited

Loading

gavofyork commented Jun 17, 2016 •

edited

Loading

john8841 commented Jun 17, 2016

gavofyork commented Jun 17, 2016

juscamarena commented Jun 17, 2016

marcogiglio commented Jun 17, 2016 •

edited

Loading

gavofyork commented Jun 17, 2016

gavofyork commented Jun 17, 2016

juscamarena commented Jun 17, 2016

gavofyork commented Jun 17, 2016 •

edited

Loading

juscamarena commented Jun 17, 2016

gavofyork commented Jun 17, 2016

DAO Rescue soft fork #1309

DAO Rescue soft fork #1309

Conversation

gavofyork commented Jun 17, 2016

rphmeier Jun 17, 2016

Choose a reason for hiding this comment

chriseth commented Jun 17, 2016

gavofyork Jun 17, 2016

Choose a reason for hiding this comment

julian1 commented Jun 17, 2016

gavofyork commented Jun 17, 2016

julian1 commented Jun 17, 2016

seweso commented Jun 17, 2016 • edited Loading

gavofyork commented Jun 17, 2016

wanderer commented Jun 17, 2016 • edited Loading

gavofyork commented Jun 17, 2016 • edited Loading

john8841 commented Jun 17, 2016

gavofyork commented Jun 17, 2016

juscamarena commented Jun 17, 2016

marcogiglio commented Jun 17, 2016 • edited Loading

gavofyork commented Jun 17, 2016

gavofyork commented Jun 17, 2016

juscamarena commented Jun 17, 2016

gavofyork commented Jun 17, 2016 • edited Loading

juscamarena commented Jun 17, 2016

gavofyork commented Jun 17, 2016

seweso commented Jun 17, 2016 •

edited

Loading

wanderer commented Jun 17, 2016 •

edited

Loading

gavofyork commented Jun 17, 2016 •

edited

Loading

marcogiglio commented Jun 17, 2016 •

edited

Loading

gavofyork commented Jun 17, 2016 •

edited

Loading