Add DeleteBlock() #10203
Add DeleteBlock() #10203
Conversation
beacon-chain/db/kv/blocks.go
Outdated
| return s.db.Update(func(tx *bolt.Tx) error { | ||
| bkt := tx.Bucket(finalizedBlockRootsIndexBucket) | ||
| if b := bkt.Get(root[:]); b != nil { | ||
| panic(errFinalizedInvalid) |
There was a problem hiding this comment.
| panic(errFinalizedInvalid) | |
| return errFinalizedInvalid |
There was a problem hiding this comment.
Do we need to delete the associated state as well? States are stored by block root I believe
There was a problem hiding this comment.
+1
We shouldn't panic. Panic shouldn't be a DB behavior, rather a caller side behavior. Otherwise, we are assuming DeleteBlock could only be used for optimistic syncing
There was a problem hiding this comment.
Indeed, this was moved from my implementation in Kintsugi that was only for optimistic sync, I think it's fixed now, will panic in the caller but that's going to be in the Kintsugi branch
beacon-chain/db/kv/blocks.go
Outdated
| return s.db.Update(func(tx *bolt.Tx) error { | ||
| bkt := tx.Bucket(finalizedBlockRootsIndexBucket) | ||
| if b := bkt.Get(root[:]); b != nil { | ||
| panic(errFinalizedInvalid) |
There was a problem hiding this comment.
+1
We shouldn't panic. Panic shouldn't be a DB behavior, rather a caller side behavior. Otherwise, we are assuming DeleteBlock could only be used for optimistic syncing
Exposes a method do delete blocks from DB. If the given root is for a finalized block, then the function panics.