update comments

Author: zhangchiqing

storage/epoch_protocol_state.go

@@ -34,16 +34,11 @@ type EpochProtocolStateEntries interface {
 	//
 	// CAUTION:
 	//   - The caller must acquire the lock [storage.LockInsertBlock] and hold it until the database write has been committed.
-	//   - OVERWRITES existing data (potential for data corruption):
-	//     This method silently overrides existing data without any sanity checks whether data for the same key already exits.
-	//     Note that the Flow protocol mandates that for a previously persisted key, the data is never changed to a different
-	//     value. Changing data could cause the node to publish inconsistent data and to be slashed, or the protocol to be
-	//     compromised as a whole. This method does not contain any safeguards to prevent such data corruption. The lock proof
-	//     serves as a reminder that the CALLER is responsible to ensure that the DEDUPLICATION CHECK is done elsewhere
-	//     ATOMICALLY with this write operation.
+	//   - The lock proof serves as a reminder that the CALLER is responsible to ensure that the operation.InsertHeader is
+	//		 done ATOMICALLY with this write operation.
 	//
 	// Expected errors during normal operations:
-	// - [storage.ErrDataMismatch] if a _different_ KV store for the given stateID has already been persisted
+	//   - [storage.ErrAlreadyExists] if a KV store for the given blockID has already been indexed
 	BatchIndex(lctx lockctx.Proof, rw ReaderBatchWriter, blockID flow.Identifier, epochProtocolStateID flow.Identifier) error
 
 	// ByID returns the flow.RichEpochStateEntry by its ID.

storage/operation/epoch_protocol_state.go

@@ -28,12 +28,9 @@ func RetrieveEpochProtocolState(r storage.Reader, entryID flow.Identifier, entry
 // CAUTION:
 //   - The caller must acquire the lock [storage.LockInsertBlock] and hold it until the database write has been committed.
 //   - OVERWRITES existing data (potential for data corruption):
-//     This method silently overrides existing data without any sanity checks whether data for the same key already exits.
-//     Note that the Flow protocol mandates that for a previously persisted key, the data is never changed to a different
-//     value. Changing data could cause the node to publish inconsistent data and to be slashed, or the protocol to be
-//     compromised as a whole. This method does not contain any safeguards to prevent such data corruption. The lock proof
-//     serves as a reminder that the CALLER is responsible to ensure that the DEDUPLICATION CHECK is done elsewhere
-//     ATOMICALLY with this write operation.
+//     The lock proof serves as a reminder that the CALLER is responsible to ensure that the DEDUPLICATION CHECK is done elsewhere
+//     ATOMICALLY within this write operation. Currently it's done by operation.InsertHeader where it performs a check
+//     to ensure the blockID is new, therefore any data indexed by this blockID is new as well.
 //
 // No error returns are expected during normal operation.
 func IndexEpochProtocolState(lctx lockctx.Proof, w storage.Writer, blockID flow.Identifier, epochProtocolStateEntryID flow.Identifier) error {

storage/operation/protocol_kv_store.go

@@ -11,7 +11,8 @@ import (
 )
 
 // InsertProtocolKVStore inserts a protocol KV store by protocol kv store ID.
-// The caller must ensure the protocolKVStoreID is the hash of the given kvStore,
+// This function can be called, and does not require the caller to hold any lock proof,
+// but the caller must ensure the protocolKVStoreID is the hash of the given kvStore,
 // This is currently true, see makeVersionedModelID in state/protocol/protocol_state/kvstore/models.go
 // No expected error returns during normal operations.
 func InsertProtocolKVStore(rw storage.ReaderBatchWriter, protocolKVStoreID flow.Identifier, kvStore *flow.PSKeyValueStoreData) error {
@@ -27,9 +28,12 @@ func RetrieveProtocolKVStore(r storage.Reader, protocolKVStoreID flow.Identifier
 
 // IndexProtocolKVStore indexes a protocol KV store by block ID.
 //
-// CAUTION: To prevent data corruption, we need to guarantee atomicity of existence-check and the subsequent
-// database write. Hence, we require the caller to acquire [storage.LockInsertBlock] and hold it until the
-// database write has been committed.
+// CAUTION:
+//   - The caller must acquire the lock [storage.LockInsertBlock] and hold it until the database write has been committed.
+//   - OVERWRITES existing data (potential for data corruption):
+//     The lock proof serves as a reminder that the CALLER is responsible to ensure that the DEDUPLICATION CHECK is done elsewhere
+//     ATOMICALLY within this write operation. Currently it's done by operation.InsertHeader where it performs a check
+//     to ensure the blockID is new, therefore any data indexed by this blockID is new as well.
 //
 // Expected error returns during normal operations:
 //   - [storage.ErrAlreadyExists] if a KV store for the given blockID has already been indexed

storage/store/proposal_signatures.go

@@ -47,8 +47,7 @@ func newProposalSignatures(collector module.CacheMetrics, db storage.DB) *propos
 // CAUTION:
 //   - The caller must acquire either the lock [storage.LockInsertBlock] or [storage.LockInsertOrFinalizeClusterBlock]
 //     but not both and hold the lock until the database write has been committed.
-//   - OVERWRITES existing data (potential for data corruption):
-//     The lock proof serves as a reminder that the CALLER is responsible to ensure that the DEDUPLICATION CHECK
+//   - The lock proof serves as a reminder that the CALLER is responsible to ensure that the DEDUPLICATION CHECK
 //     is done elsewhere ATOMICALLY with this write operation. It is intended that this function is called only for new
 //     blocks, i.e. no signature was previously persisted for it.
 //

storage/store/protocol_kv_store.go

@@ -104,13 +104,6 @@ func (s *ProtocolKVStore) BatchStore(rw storage.ReaderBatchWriter, stateID flow.
 //
 // CAUTION:
 //   - The caller must acquire the lock [storage.LockInsertBlock] and hold it until the database write has been committed.
-//   - OVERWRITES existing data (potential for data corruption):
-//     This method silently overrides existing data without any sanity checks whether data for the same key already exits.
-//     Note that the Flow protocol mandates that for a previously persisted key, the data is never changed to a different
-//     value. Changing data could cause the node to publish inconsistent data and to be slashed, or the protocol to be
-//     compromised as a whole. This method does not contain any safeguards to prevent such data corruption. The lock proof
-//     serves as a reminder that the CALLER is responsible to ensure that the DEDUPLICATION CHECK is done elsewhere
-//     ATOMICALLY with this write operation.
 //
 // Expected errors during normal operations:
 // - [storage.ErrAlreadyExist] if a KV store for the given blockID has already been indexed.