Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test, cmd/evm, core, core/vm: flag DAO reduction blocks #2715

Closed
wants to merge 1 commit into from

Conversation

obscuren
Copy link
Contributor

@obscuren obscuren commented Jun 18, 2016

!!! IMPORTANT: WORK IN PROGRESS. DO NOT USE (YET) !!!

The DAO soft fork that will prevent blocks being accepted that include any value transfer made from the DAO that matches the correct code hash.

The strategy works in such a way that when a user enables mining -- either thru --mine or the RPC -- it will start to go in a sort of failsafe mode where it will start to ignore transactions and blocks that reduce the DAO's balance.

If you disagree with this you can disable the failsafe by setting the --no-dao-soft-fork flag.

FINAL: Ultimately this decision is not ours to make, it's yours. Please try to leave all biases out of your decision making process and act responsibly -- The go-ethereum Team


CHANGED

The original proposal has changed to be generic. There's no --no-dao-soft-fork nor a --yes-dao-soft-fork. There's simply the notion of --illegal-code-hashes that takes a list of hashes that are placed in "ignore-mode". This will also allow anyone to make a proposal to the majority of the miners to ask them for help for any future possible soft forks by allowing them to ignore blocks that take certain actions undesirable by the community.

TASKS

  • Implemented DAO code hash filtering & error reporting
  • Mining flag

@obscuren obscuren added the core label Jun 18, 2016
@obscuren obscuren self-assigned this Jun 18, 2016
@robotally
Copy link

robotally commented Jun 18, 2016

Vote Count Reviewers
👍 0
👎 0

Updated: Sat Jun 18 11:06:08 UTC 2016

@krisives
Copy link

I lost some ETH when I made a bad contract too, can I get a soft-fork flag?

@dong77
Copy link

dong77 commented Jun 18, 2016

I'm in favor of the soft/hard fork solution. Thank you @obscuren.

@@ -100,6 +102,9 @@ func exec(env vm.Environment, caller vm.ContractRef, address, codeAddr *common.A
}
}
env.Transfer(from, to, value)
if !createAccount && env.Db().GetCodeHash(*codeAddr) == daoHash && value.Cmp(common.Big0) > 0 {
Copy link
Member

Choose a reason for hiding this comment

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

Not really sure about this value comparison. Doesn't this actually block value transfers into the dao rather than stop funds leaking out of it?

Shouldn't a simpler comparison be to check for dao code match at the top of this method, if matches query the balance, and then query the balance after the end of execution. If the balance went down, set the flag?

@karalabe
Copy link
Member

@krisives This soft fork would only lock up the funds that were taken, it does not give it back to anyone.

@krisives
Copy link

krisives commented Jun 18, 2016

@karalabe Agreed, but can I get a soft-fork flag to lock the funds of someone who has my coins because of the smart contract I created didn't perform as I expected? Also I want this flag to be default anyone can turn it off with --no-krisives-softfork

@karalabe
Copy link
Member

@krisives if it collapses the ecosystem, sure ;)

@papococ
Copy link

papococ commented Jun 18, 2016

@krisives sure you can get your own soft-fork flag, all you have to do is create it yourself and convince the majority of the community that it's the morally right thing to do. good luck!

@krisives
Copy link

@karalabe How is it proven The DAO failing will destroy the ecosystem more than policing the blockchain?

@papococ It will be default anyone who doesn't want can simply run --no-krisives-softfork like with this

@kieranelby
Copy link

Wouldn't it be more neutral to have options:
--enable-dao-soft-fork
--disable-dao-soft-fork
And require one of them to be explicitly specified?

@papococ
Copy link

papococ commented Jun 18, 2016

@krisives again, yes go create it yourself and request people to download it and adopt it. stop your passive aggressive comments here and go do it!

@ghost
Copy link

ghost commented Jun 18, 2016

I've seen some people on IRC who bet on Ethereum crashing (shorting), they are willing to pay up to 0.5 BTC for each user who votes for -no-krisives-softfork

@OlegTheCat
Copy link

Ultimately this decision is not ours to make, it's yours.

And, of course, the behaviour that forces fork was made default.

¯_(ツ)_/¯

@krisives
Copy link

@papococ I'd rather not flood the project with duplicate pull requests. I just want a default flag that gives me special treatment in the protocol, what's wrong with that?

@plneteth
Copy link

i mine through Ethpool does anyone know if they are supporting this? If not what pools are?

@dong77
Copy link

dong77 commented Jun 18, 2016

There is no way to reach consensus here. Just merge it and let others fork the code and set another default value.

@papococ
Copy link

papococ commented Jun 18, 2016

@krisives there's nothing wrong with that, just don't request other people to do work for you. if you want that to happen, then make it happen... yourself

@xtrapower
Copy link

Unbelievable how quite some people value absolute idealism higher than a pragmatic solution to a bug that overthrew the whole ecosystem into a huge mess. We should be more flexible at this point of development IMO, we don't even understand the code of the first big Ethereum project ourselves and allow people to put USD 250.000.000 in it...

Thanks you for that solution!

@JaTochNietDan
Copy link

If you disagree with this you can disable the failsafe by setting the --no-dao-soft-fork flag.

Why is it the default? There is no possibility for the disagreement side to win with this default.

@karalabe
Copy link
Member

Everyone, please take the political debates back to other forums. This one is meant to discuss code and purely code. Locking conversations for now as it's getting out of hand.

@ethereum ethereum locked and limited conversation to collaborators Jun 18, 2016
@obscuren
Copy link
Contributor Author

@krisives I have been thinking about this actually. Something like --prevent-contract-execution=... list of code hashes ... This means that everyone could propose a soft fork in the future without us having to intervene.

@obscuren
Copy link
Contributor Author

Please continue the debate here 👍

@obscuren
Copy link
Contributor Author

The original proposal has changed to be generic. There's no --no-dao-soft-fork nor a --yes-dao-soft-fork. There's simply the notion of --illegal-code-hashes that takes a list of hashes that are placed in "ignore-mode". This will also allow anyone to make a proposal to the majority of the miners to ask them for help for any future possible soft forks by allowing them to ignore blocks that take certain actions undesirable by the community.

@Gustav-Simonsson
Copy link

Would suggest renaming Illegal to Invalid to avoid associations with law.

if err != nil {
return nil, nil, nil, err
}

for _, codeHash := range env.CodeHashes {
Copy link

Choose a reason for hiding this comment

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

Minor performance suggestion: consider changing env.CodeHashes to map[common.Hash]struct{} and IllegalCodeHashes to an array and loop over IllegalCodeHashes instead, to avoid linear complexity on env.CodeHashes

Probably not very significant but most likely IllegalCodeHashes.length will be 1 and env.CodeHashes.length will be longer - could even be a few thousand for complex txs.

@Gustav-Simonsson
Copy link

Marking code hash for DELEGATECALL too: obscuren#7

This implements a generic approach to enabling soft forks by allowing
anyone to put in hashes of contracts that should not be interacted from.
This will help "The DAO" in their endevour to stop any whithdrawals from
any DAO contract by convincing the mining community to accept their code
hash.
@karalabe
Copy link
Member

Reowned #2725.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet