chore: rework some comments

Author: ezdac

rolling-shutter/medley/chainsync/chainsegment/chain.go

@@ -225,8 +225,8 @@ func (bc *ChainSegment) DiffLeftAligned(other *ChainSegment) (remove, update *Ch
 		}
 		if oldHeader == nil {
 			updated = append(updated, newHeader)
-			// XXX: is just checking the hash sufficient?
-			// sanity check also the blocknum + parent hash chain?
+			// TODO: sanity check also the blocknum + parent hash chain
+			// so that we are sure that we have consecutive segments.
 		} else if oldHeader.Hash().Cmp(newHeader.Hash()) != 0 {
 			removed = append(removed, oldHeader)
 			updated = append(updated, newHeader)

rolling-shutter/medley/chainsync/syncer/fetch.go

@@ -186,7 +186,6 @@ func (f *Fetcher) FetchAndHandle(ctx context.Context, update ChainUpdateContext)
 	query := ethereum.FilterQuery{
 		Addresses: f.addresses,
 		Topics:    f.topics,
-		//FIXME: oes this work when from and to are the same?
 		FromBlock: update.Append.Earliest().Number,
 		ToBlock:   update.Append.Latest().Number,
 	}
@@ -210,7 +209,6 @@ func (f *Fetcher) FetchAndHandle(ctx context.Context, update ChainUpdateContext)
 	var result error
 	f.syncMux.RLock()
 	for _, h := range f.contractEventHandlers {
-		// TODO: copy all the logs?
 		handler := h
 		wg.Add(1)
 		go func() {

rolling-shutter/medley/chainsync/syncer/loop.go

@@ -35,17 +35,12 @@ func (f *Fetcher) handlerSync(ctx context.Context) (success bool, err error) {
 		return success, err
 	}
 	if errors.Is(err, ErrEmpy) {
-		// no chain-cache present yet,
-		// just set the chain-update without
+		// no chain-cache present yet, just set the chain-update without
 		// checking for reorgs
-		// FIXME: here we could incorporate a starting block
-		// option, fetch the starting block, and set this in
-		// the cache and sync from there.
 		removedSegment = nil
 		updatedSegment = f.chainUpdate
 		log.Trace().Msg("internal chain cache empty, setting updated chain segment")
 	} else if err != nil {
-		//TODO: what to do on db error?
 		success = false
 		return success, err
 	} else {
@@ -63,6 +58,11 @@ func (f *Fetcher) handlerSync(ctx context.Context) (success bool, err error) {
 				return success, fmt.Errorf("chain-update difference too big: %w", errUint64Overflow)
 			}
 			diff := diffBig.Uint64()
+			queryBlocks := MaxRequestBlockRange
+			// cap the extend range at the diff to the update to not overshoot
+			if diff < uint64(queryBlocks) {
+				queryBlocks = int(diff)
+			}
 
 			// we are not synced to the chain-update
 			// so first construct an update to the right of the synced chain
@@ -86,20 +86,33 @@ func (f *Fetcher) handlerSync(ctx context.Context) (success bool, err error) {
 			}
 			removedSegment = nil
 			success = false
-			result, errr := syncedChain.UpdateLatest(ctx, f.ethClient, f.chainUpdate)
-			result, errr := syncedChain.UpdateLatest(ctx, f.client, f.chainUpdate)
-			if errr != nil {
-				// TODO: for ErrUpdateBlockTooFarInPast this should shut down the
-				// client? Since we can't really play back a reorg that reaches too far in the past.
-				// Now as long as the chain-cache eviction policy is not agressive (easily doable)
-				log.Error().Err(err).Msg("error updating chain")
+		} else {
+			result, updateErr := syncedChain.UpdateLatest(ctx, f.ethClient, f.chainUpdate)
+			success = true
+			if updateErr != nil {
+				log.Error().Err(err).Msg("error updating chain with latest segment")
+				if errors.Is(err, chainsegment.ErrUpdateBlockTooFarInPast) {
+					//TODO: what should we do on 'ErrUpdateBlockTooFarInPast'?
+					//  We can't provide handler calls with the same accuracy of
+					// information on the potentially "removed" chain-segment,
+					// since our chain-cache does not have the full old chain segment
+					// in it's storage anymore, and especially the block-hash
+					// of the reorged away chain is not present anymore.
+					// The client should probably panic with a critical log error.
+					// In general this is very unlikely when the chain-cache capacity is
+					// larger than the most unlikely, still realistic reorg-size.
+					// However the described condition might currently occur during
+					// initial syncing, when the block-cache is not filled to capacity
+					// yet.
+					log.Warn().Err(err).Msg("received a reorg that pre-dates the internal chain-cache." +
+						" ignoring chain-update for now, but this condition might be irrecoverable")
+				}
 				err = updateErr
 				// Now as long as the chain-cache eviction policy is not aggressive (easily doable)
 			removedSegment = result.RemovedSegment
 			updatedSegment = result.UpdatedSegment
 			// we will process the whole segment of the chain update
 			f.chainUpdate = nil
-			success = true
 		}
 	}