Prevent account storage leakage #270
Conversation
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
pepyakin
added
the
A3-in_progress
pepyakin
added
A0-please_review
|
@rphmeier done! |
pepyakin
changed the title
rphmeier
added
A8-looksgood
and removed
A0-please_review
|
@arkpar mentioned that keys were first hashed before being placed in the merkle tree - if this is true then enumeration won't work. are there tests to ensure it's working as expected? |
@gavofyork Yes, they are hashed in runtime before calling to the externalities. Because of this, Apart of this, I haven't found any additional hashing on top of that (Except
Yeah, there are tests but it seems I forgot to add test for the trie backend specifically, sorry about that. Rebased and added test for prefix walking for the trie backend. |
|
cool - at some point (poc-2 or -3; whenever we restart the chain) we'll want to remove that blanket hashing that |
|
Added comment to #227 about it |
* FIx extracter panic * Change rpc 'chainx_getAddressByAccount' * Add chainx_getDepositList * Add withdraw_list * Update wasm * Update btc header info * Flow substrate-rpc * Fix build error * Add 'state_getKeys' rpc * Delete old bind * Add withdrawal id in result Fix rebind bug * Refine a bit * Fix sign error * Update wasm Modify test
paritytech/polkadot#195 introduced logic of deletion of accounts if their balance is too low.
An account can have not only balances and nonces, but also a key-value storage, that can be used by substrate smart-contracts and this storage should be deleted when account itself gets deleted. However, #195 delayed the removal of the account's storage.
This PR aims to implement deletion of the contract storage along with account itself.
In particular, it adds a
clear_prefixfunction in externalities which takes a prefix, walks runtime storage trie and removes all keys that starts with the given prefix.Then, the storage of contracts are put in a custom implementation of a storage container:
StorageDoubleMap. It basically is a map which uses two keys instead of one to address a value. Also, it provides a special functionremove_prefixwhich removes all entries from the storage that shares a common key (i.e.AccountId).This custom container is required because we need to use blake2 for hasing contract's storage keys (since they ought to be controlled by smart-contract code which is untrusted) and we have no means to cleanly specify the way of hashing of keys for the storage data containers.