Venus: Chain API improvements

[ZenGround0]

Description

Discussing with @phritz BlockHistory seems a little off. A better solution (that could be used to implement block history) is to have a Head and GetNext method allowing tipsets to traverse themselves.

Acceptance criteria

BlockHistory is removed from the chain interface. It is reimplemented elsewhere with more irreduceable and ergonomic api methods.

Risks + pitfalls

Depending on how we go about it this work has the potential to blow up in scope. If we want tipsets to be able to traverse themselves maybe that means the GetNext method is defined on tipset which opens up a lot of complexity. This work could also start bleeding into questions of whether state or state cids should be stored on tipsets. This could start a chain reaction where we start untangling lots of technical debt in the chain store interface.

Where to begin

[travisperson]

Something I've noticed that will impact #1634, is that while the chain is syncing, chain head doesn't show the latest block, and chain ls doesn't increase. If this can be address in this issue as well, we could pretty quickly provide a way for the user to see the current height their node sees.

It appears that we download everything into memory and then save it to the store. Let me know if this is not related at all to this issue and I have file a new one.

[ZenGround0]

@travisperson unfortunately that is a much more significant change to the syncing protocol and the assumptions of the chain store.

[ZenGround0]

@phritz check out GetTipSetAndState, chainReaders already have something close to the desired method if we are shooting for chainReader.GetParent(tipset) rather than `tipset.GetParent().

The ever-present awkwardness of aggregate state storage means we'd want a wrapper around GetTipSetAndState to strip away the tsas so callers don't need to see the ugly truth. It seems bad and redundant to have both the wrapper and GetTipSetAndState on the chain reader and I'm not sure where else to put it though.

Probably the right thing to do to avoid this awkwardness is include a state cid inside the tipset directly under the hood, then tsass are just tipsets and the return type is ergonomic. One annoying thing that this opens up is that in the syncer package tipsets need to exist for a good stretch of code without having an aggregate state. Maybe the way around this is to do something goish and reduce the interface of the tipset within the syncer so that it can't access it's state id. Or maybe just leaving it nil for a while and then filling it in is fine.

[phritz]

@ZenGround0 regardless of where we put the iteration code, on tipset or in a function, i'm wondering:

• gettipsetandstate, should it take a sortedcidset instead of a key? it can key ify internally and the return type of gethead should really be a sortedcidset

i think we can replace these:

    GetTipSetAndState(ctx context.Context, tsKey string) (*TipSetAndState, error)
    GetBlock(ctx context.Context, id cid.Cid) (*types.Block, error)
        Head() consensus.TipSet
    LatestState(ctx context.Context) (state.Tree, error)
    BlockHistory(ctx context.Context) <-chan interface{}

with

GetTipSetAndState(ctx context.Context, types.SortedCidSet) (*TipSetAndState, error)
GetBlock(ctx context.Context, id cid.Cid) (*types.Block, error)
GetHead() types.SortedCidSet

plus an iterator thingy that uses the above but is not in the interface like blockhistory. if we need to get a tipset at a particular height we can add GetTipSetAndStateByHeight(ctx context.Context, types.blockheight) (*TipSetAndState, error) which makes id 4 calls instead of 5. the real improvement is moving blockhistory out. i'm not sure how to factor it down any more... do you?

[phritz]

@travisperson this issue is unrelated but you are absolutely right that that behavior is a problem. please file a tech debt and scaling labeled issue for it

[phritz]

@anorth this is a more meaty one that spans core and plumbing and gets you working with channels a little, lemme know if you want to take it. i'll let you know if/when i start.

[phritz]

note this will enable things like https://github.com/filecoin-project/go-filecoin/pull/1755 to be implemented as porcelain. there are a million ways to filter the chain, we need to provide the building blocks that enable consumers to filter how they want

[mishmosh]

Really nice to have, but not strictly required to delete node API, so may fall out of this epic.

[rosalinekarr]

@phritz, regarding your earlier suggestion, changing

Head() consensus.TipSet

to

GetHead() types.SortedCidSet

is a problem, because we use Head in a number of places where we need the full block information of the head of the chain. For example, line 229 of chain/default_syncer.go uses it to fetch the parents of the block.

Should I skip this suggestion for now, or do you have any ideas about how to make it work?

[phritz]

Use GetHead() to get the sortedcidset then GetTipSetAndState() to get the blocks?