search

filecoin-project / venus

Filecoin Full Node Implementation in Go
https://venus.filecoin.io
Other
2.06k stars 463 forks source link

Chain data stored twice on disk #2636

Closed ZenGround0 closed 5 years ago

ZenGround0 commented 5 years ago

Description

Our design should not be storing chain data once in a bitswap datastore and then copying it all over to a chain datastore. Originally I enforced this separation to reason about security. Specifically I wanted to prevent code from ever assuming a block had been verified without verifying it first. The clunky way the code does this now is by only writing verified blocks to the chain datastore and always assuming reads from that datastore are good.

Right now this is utter overkill. The datastore separation is not needed because (1) during node operation all tipsets added to the chainstore are kept in memory and (2) during load of a previously synced chain the node reads the head cids from the protected chain datastore. Furthermore when we (3) stop keeping all chain store tipsets in memory we can simply store metadata in the protected chain store without putting all chain data in the protected store.

Acceptance criteria

(1) Chain bits are only in one place on disk (2) go-filecoin nodes still never ever read unverified blocks thinking that they are verified.

Risks + pitfalls

Right now the default store might be going to disk in some cases to check that a block is verified. All Get and Has chain store methods need to be carefully audited to make sure they don't violate AC (2) when doing away with chain copying.

Where to begin

chain/default_store.go should be organized to never check membership in the conceptual chain based on data of a certain cid existing in the datastore.

anorth commented 5 years ago

I think it is fine for a single blockstore to be used for blockchain blocks and messages, as well as the state trees. This store will be available to bitswap/graphsync. It is not necessary to validate that a block connects to the existing chain before storing it. The store must ensure that content matches its CID, though.

However, we do need a distinguished store of a node's idea of the head tipset CIDs. This metadata about the chain is the security-critical piece of information. Code should only assume a block is validated if it can be reached from this, but I think that's a reasonable thing to maintain.

We have discussed this a bit in person, and I believe our plan is to remove the duplicate block data from the chain store (which remains as a store of the head CID and tip index). We don't need to hold the entire chain in RAM during initial sync: just write the blocks (later: headers) to store directly and read them back for validation later.

ZenGround0 commented 5 years ago

I agree with everything except:

Code should only assume a block is validated if it can be reached from this, but I think that's a reasonable thing to maintain.

The syncing system is currently designed to implicitly track a frontier of validated blocks (the leaves in the block tree) not a single head. I consider redesigning this and losing out on de-duplicated validation during reorgs to be out of scope.