[finalizer] Update confirmation block number in finalizer #600
Conversation
ian-shim
force-pushed
the
finalizer-update-confirmation-block
branch
from
a3a2b85 to
2814241
Compare
| return &newMetadata, s.blobMetadataStore.UpdateBlobMetadata(ctx, existingMetadata.GetBlobKey(), &newMetadata) | ||
| } | ||
|
|
||
| func (s *SharedBlobStore) MarkBlobFinalized(ctx context.Context, metadataKey disperser.BlobKey) error { | ||
| return s.blobMetadataStore.SetBlobStatus(ctx, metadataKey, disperser.Finalized) | ||
| func (s *SharedBlobStore) MarkBlobFinalized(ctx context.Context, existingMetadata *disperser.BlobMetadata, confirmationBlockNumber uint64) (*disperser.BlobMetadata, error) { |
There was a problem hiding this comment.
It looks a more robust and simpler interface if the entire read-validate-update status-write back flow is in one function, as these steps are all tightly coupled.
There was a problem hiding this comment.
what do you mean by read-validate-update status-write back flow?
There was a problem hiding this comment.
Instead of taking a "existingMetadata" from the caller, this method can just take the potentially new confirmation block number: and internally it reads the metadata, compare the confirmation number with the given one, and if they are different, update it and write back. It's cleaner as it doesn't need to assume a "existingMetadata".
There was a problem hiding this comment.
I reverted this method back so it just updates the blob status.
I added a separate method UpdateConfirmationBlockNumber and update the confirmation block number regardless of whether the block is finalized or not as we need to update the confirmation block even when it's in CONFIRMED state.
ian-shim
force-pushed
the
finalizer-update-confirmation-block
branch
2 times, most recently
from
4d710d7 to
e9cdadb
Compare
ian-shim
changed the title
| func (s *BlobMetadataStore) UpdateConfirmationBlockNumber(ctx context.Context, existingMetadata *disperser.BlobMetadata, confirmationBlockNumber uint32) error { | ||
| updated := *existingMetadata | ||
| if updated.ConfirmationInfo == nil { | ||
| return fmt.Errorf("failed to update confirmation block number because confirmation info is missing for metadata %s", existingMetadata.GetBlobKey().String()) |
There was a problem hiding this comment.
nit: "... is missing for blobkey ..."
ian-shim
force-pushed
the
finalizer-update-confirmation-block
branch
from
e9cdadb to
288d981
Compare
Why are these changes needed?
The
confirmationBlockNumberretrieved from the confirmation transaction may have been updated due to chain reorg.This PR makes the finalizer update the
confirmationBlockNumberto keep the metadata in sync.Checks