filecoin-project / ref-fvm

Reference implementation of the Filecoin Virtual Machine
https://fvm.filecoin.io/
Other
380 stars 136 forks source link

EVM Storage: Hashing as syscall #893

Closed aakoshh closed 1 year ago

aakoshh commented 2 years ago

Part of https://github.com/filecoin-project/ref-fvm/issues/859

Even though https://github.com/filecoin-project/ref-fvm/issues/890 disables key hashing in the HAMT, ~it will still calculate hashes for the CID of Nodes.~

Normal HAMT usage will still do hashing of keys, which native smart contracts would be using. This could be done as a syscall, rather than in Wasm, similarly to I guess how the EVM opcodes are implemented. See https://github.com/filecoin-project/ref-fvm/issues/374

Hashing would not have any effect on the storage footprint per se, but it's related to performance and overall gas cost. Its effect can be measured as part of the gas model epic and memory expansion epic under Syscall pricing and Execution gas refinement.

Stebalien commented 2 years ago

Even though https://github.com/filecoin-project/ref-fvm/issues/890 disables key hashing in the HAMT, it will still calculate hashes for the CID of Nodes. This could be done as a syscall, rather than in Wasm, similarly to I guess how the EVM opcodes are implemented.

For cids, the HAMT delegates to the blockstore. In the case of the actors, that turns out to be

https://github.com/filecoin-project/builtin-actors/blob/18c2291d4ccfac409e05270bd1d33d40fe196d65/runtime/src/runtime/actor_blockstore.rs#L18

Basically:

aakoshh commented 2 years ago

Ah, brilliant, thanks for pointing that out @Stebalien. Okay then this ticket makes no sense in the context of the EVM, because all we need is not to call hashing at all. I changed the description to mention other actors instead that will use the HAMT as intended.

Stebalien commented 1 year ago

Fixed by https://github.com/filecoin-project/builtin-actors/pull/712