ref-fvm icon indicating copy to clipboard operation
ref-fvm copied to clipboard

Refactor the blockstore

Open Stebalien opened this issue 2 years ago • 3 comments

First, remove the multihash "code" from the arguments and use a plain u64. This will let us compile the actors without importing hashing libraries.

fixes https://github.com/filecoin-project/ref-fvm/issues/1238 fixes https://github.com/filecoin-project/ref-fvm/issues/504

Second, replace the concrete Block type with a trait. We can then implement this trait for, e.g., fvm_ipld_encoding::IpldBlock.

Finally, make the blockstore object safe. This will let us work with the blockstore as &dyn Blockstore in most cases.

Stebalien avatar Mar 03 '23 23:03 Stebalien

@anorth can I get feedback on the blockstore trait (and the block trait)? You can ignore everything else.

Stebalien avatar Mar 03 '23 23:03 Stebalien

So forgive me if I've overstepped the scope here, but I don't really get why the core Blockstore trait and implementations should do hashing and CID construction. Instead they'd be simpler as a pure (CID, data) store abstraction, and then wrappers can curry in either or both the hashing function and serialisation method, no neither need to be passed around. How bad would it be to remove (deprecate) put? I think there are few call sites, since most code uses a wrapper like CborStore.

The issue is that I'm to support two distinct use-cases:

  • FVM -> Client: The FVM will already know the CID so I need a way to "put" with a pre-computed CID (put_keyed).
  • Builtin actors -> FVM: For security, the FVM needs to compute the CID internally. I.e., it can't trust a CID passed by the user.

I can probably drop the FVM -> Client use-case here, honestly. I.e., get rid of put_keyed, leaving only put.

Stebalien avatar Mar 06 '23 17:03 Stebalien

The built-in-actors test VM is another example with the two use cases: untrusted use from within the actors, but trusted use from the surrounding test infrastructure code. However, we can probably support these with two different traits.

anorth avatar Mar 27 '23 21:03 anorth