ethereum / ethereumj

DEPRECATED! Java implementation of the Ethereum yellowpaper. For JSON-RPC and other client features check Ethereum Harmony
GNU Lesser General Public License v3.0
2.18k stars 1.1k forks source link

fix same composed key of contract storage nodes #1158

Closed ithinker1991 closed 6 years ago

ithinker1991 commented 6 years ago

32-bytes composed key should be [first 16-bytes of address hash:second 16-bytes of node key]

There seems a BIG problem. I noticed in solidity, for dynamic type such as stringbytes first 16-bytes of node-key are same and second 16-bytes of node are different. So if one field has been encoded to many storage-key. the composed key will be same.

For example s1 = "1111111111111111111111111111111111111112222222222222222222222222222222222222222333333333333333333333333333333333333";

encoded result:

key of storage node                             value of storage node                                                        
0000000000000000000000000000000000000000000000000000000000000000 00000000000000000000000000000000000000000000000000000000000000e7
290decd9548b62a8d60345a988386fc84ba6bc95484008f6362f93160ef3e563 3131313131313131313131313131313131313131313131313131313131313131
290decd9548b62a8d60345a988386fc84ba6bc95484008f6362f93160ef3e564 3131313131313132323232323232323232323232323232323232323232323232
290decd9548b62a8d60345a988386fc84ba6bc95484008f6362f93160ef3e565 3232323232323232323232323232323333333333333333333333333333333333
290decd9548b62a8d60345a988386fc84ba6bc95484008f6362f93160ef3e566 3333333333333333333333333333333333333300000000000000000000000000
ithinker1991 commented 6 years ago

https://github.com/ethereum/ethereumj/issues/1157

mkalinin commented 6 years ago

There is a path that solidity uses to address storage variable. It is a 32-byte value that is defined by compiler for each storage variable. Storage key is not the same as storage variable path, it's a sha3 from that path, key = sha3(path). Thus lower 16 bytes of the key reflect whole path value whatever it is.

ithinker1991 commented 6 years ago

@mkalinin thank you for your reply. But I have see sha3(path) in class SecureTrie. The class SecureTrie is used in many scenarios. Why uses SecureTrie instead of TrieImp

Nashatyrev commented 6 years ago

@ithinker1991 because of Ethereum specification. Originally this was done to avoid malicious Trie manipulation. I.e. you may intentionally add such data to a contract that Trie depth becomes very large which causes significant performance degradation for Trie operations