dvf / blockchain

A simple Blockchain in Python
MIT License
7.82k stars 2.76k forks source link

Proofs Don't Depend on the Hash? #10

Closed gravelcycles closed 6 years ago

gravelcycles commented 7 years ago

Looking at the code, it looks to me that the proofs don't depend on the hash of a block. Couldn't I change the block and rehash everything?

So I could change transaction information, change the 'previous_hash' accordingly, and then leave the proofs the same?

The crux of the matter seems to be that the validity of the proof only depends on the current proof and the last_proof. No where does it depend on the previous_hash nor on the current hash of the block

In the valid_proof method

guess = f'{last_proof}{proof}'.encode()
guess_hash = hashlib.sha256(guess).hexdigest()
return guess_hash[:4] == "0000"
winterjung commented 7 years ago

In general blockchain, not just the proof(called nonce), but also other parameters (like timestamp, merkle root, previous block hash etc.) is hashed and check whether starting with "0000". But this is just the simple proof of work algorithm, So it checks only last_proof and proof. Additionally, the validity of blockchain is verified in valid_chain method. self.hash(last_block) method check hash is valid. So if someone change transaction history or whatever, It rejected.

gravelcycles commented 7 years ago

But theoretically, I could change a block and modify the 'previous_hash' to correspond to my change and modify the chain, correct?

So after a quick test, it looks like the proofs (or nonces) will always follow the order 100, 35293, 35089, 119678, 146502, 43538, 85724, etc. No matter what transactions you input. It seems like it would be pretty easy to fake transactions.

For educational purposes, it seems just as easy to add the hash of the previous block to the valid_proof method, which would lead to a much stronger proof of work (even for educational purposes).

ashleyniemerg commented 7 years ago

Yes, now the hashes depend on the proofs, but not vice versa. Someone pointed this out in a comment on the Medium article too: https://hackernoon.com/learn-blockchains-by-building-one-117428612f46

So this is enough to fix it?

def proof_of_work(self, last_proof):
    """                                                                                                                                             
    Simple PoW algo:                                                                                                                                
    Find a number p' (new proof) s. t. hash(pp'h) contains 4 leading zeroes, where p is the 
    previous proof and h is the hash of the previous block.                                              

    :param last_proof: <int>                                                                                                                        
    :return: <int>                                                                                                                                  
    """
    proof = 0
    previous_hash = self.hash(self.chain[-1])
    while self.valid_proof(last_proof, proof, previous_hash) is False:
        proof += 1

    return proof

@staticmethod
def valid_proof(last_proof, proof, previous_hash):
    """                                                                                                                                             
    Validates the proof: Does hash(last_proof, proof, previous_hash) contain 4 leading zeroes?                                                      

    :param last_proof: <int> Previous proof                                                                                                         
    :param proof: <int> Current proof                                                                                                               
    :param previous_hash: <str> Hash of previous block                                                                                              
    :return: <bool> True if correct, False if not                                                                                                   
    """
    guess = f'{last_proof}{proof}{previous_hash}'.encode()
    guess_hash = hashlib.sha256(guess).hexdigest()
    return guess_hash[:4] == "0000"
dvf commented 7 years ago

I think that in my quest for making things as simple as possible, I let this one slip through the cracks. The PoW was pretty simple, and adding the hash to the algorithm makes the chain of proofs inseparable from the blocks.

Thanks all 👍

Endogen commented 7 years ago

Can we expect this to be merged anytime soon?

dvf commented 7 years ago

@Endogen, reviewing it now.

stevenpack commented 7 years ago

This is awesome. @dvf you did a great thing publishing that article. In the same way you say you don't really understand something until you code it, the same is true here. The difference between grokking the security hole @2gotgrossman points out when you've actually coded along is so different to just understanding it an an academic level.

Great stuff.

dvf commented 7 years ago

@2gotgrossman can you rebase and add your changes? Looks good. 👍

fridokus commented 7 years ago

Not sure, but shouldn't also the hash of the pending transactions be included in the proof? Imagine this scenario: Node A finds proof and writes block 100. Using this proof, Node B can just take that proof and rewrite block 100, forking his own chain with block 99 as the common parent. On this chain, he will receive the mining reward. Then he can start mining on that chain, and if he finds block 101 before Node A that will be the longest valid chain and Node A will receive no reward for finding block 100.

Having a hash of the pending transactions included in the PoW will avoid this issue, since you wouldn't be able to change who receives the mining reward arbitrarily and fork your own chain without an equal amount of effort.

dvf commented 7 years ago

@fridokus Thanks for this. I'm very much aware of this issue. And it's being address in the follow-up article I'm writing.

Unfortunately, I didn't want to dive too deep. I just wanted people to get the gist of what blockchains are about. You're free to submit a PR to fix it.

dvf commented 6 years ago

This is fixed by https://github.com/dvf/blockchain/pull/11