-
Notifications
You must be signed in to change notification settings - Fork 1.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: chainstore: store tipsetkeys in the blockstore #9904
Conversation
@@ -971,7 +969,24 @@ func (cs *ChainStore) AddToTipSetTracker(ctx context.Context, b *types.BlockHead | |||
return nil | |||
} | |||
|
|||
func (cs *ChainStore) PersistBlockHeaders(ctx context.Context, b ...*types.BlockHeader) error { | |||
func (cs *ChainStore) PersistTipset(ctx context.Context, ts *types.TipSet) error { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not something we want to refactor now, but PutTipSet
and PersistTipSet
are confusing.
err = xerrors.Errorf("failed to persist synced blocks to the chainstore: %w", err) | ||
ss.Error(err) | ||
return err | ||
if err := syncer.store.PersistTipset(ctx, ts); err != nil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This may have performance implications (although it may also have been a premature optimization?).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Noting that we were already storing the tipset key here:
Lines 653 to 663 in f438b1c
tskBlk, err := ts.Key().ToStorageBlock() | |
if err != nil { | |
log.Errorf("failed to create a block from tsk: %s", ts.Key()) | |
return err | |
} | |
err = cs.chainLocalBlockstore.Put(ctx, tskBlk) | |
if err != nil { | |
log.Errorf("failed to put block for tsk: %s", ts.Key()) | |
return err | |
} |
Feel free to merge this as it looks strictly cleaner, but should probably remove the original write.
69c9eaf
to
f60f02c
Compare
Related Issues
filecoin-project/ref-fvm#1053
Proposed Changes
PersistTipset
method that replacsPersistBlockHeaders
at all callsites (it's what we always want to do)PersistTipset
methodAddBlock
methodAdditional Info
Checklist
Before you mark the PR ready for review, please make sure that:
<PR type>: <area>: <change being made>
fix: mempool: Introduce a cache for valid signatures
PR type
: fix, feat, build, chore, ci, docs, perf, refactor, revert, style, testarea
, e.g. api, chain, state, market, mempool, multisig, networking, paych, proving, sealing, wallet, deps