Detect duplicates in the same insert batch

[carllin]

Problem

There's a race condition where duplicate shreds in the same insert batch might not get detected as duplicate. The order of events is as follows:

1. A batch of shreds with both duplicate A and B arrive to insert_shreds_handle_duplicate() from window_service: https://github.com/solana-labs/solana/blob/cfb028819a63de8f726bab3c9c981eb3155e9886/core/src/window_service.rs#L283C42-L283C72
2. Duplicate A is inserted into just_inserted_shreds here:

   solana/ledger/src/blockstore.rs

   Line 867 in cfb0288

   &mut just_inserted_shreds,

   , but is not committed to blockstore yet
3. Duplicate B is detected here:

   solana/ledger/src/blockstore.rs

   Line 1340 in cfb0288

   handle_duplicate(shred);

   , and the shred is sent to the duplicate thread detection via the dup handler:

   solana/core/src/window_service.rs

   Line 423 in cfb0288

   let _ = check_duplicate_sender.send(shred);

4. The duplicate thread checker doesn't detect a duplicate:

   solana/core/src/window_service.rs

   Lines 147 to 148 in cfb0288

   	if let Some(existing_shred_payload) =
   	blockstore.is_shred_duplicate(shred.id(), shred.payload().clone())

   because duplicate A hasn't been inserted yet

Summary of Changes

Only invoke the duplicate handler after the shreds have been committed in insert. That way all shreds are findable.

Crux of change:

solana/ledger/src/blockstore.rs

Lines 1073 to 1075 in d263794

	for shred in duplicate_shreds {
	handle_duplicate(shred);
	}

Fixes #

[codecov]

Codecov Report

Merging #32528 (d263794) into master (dba39d9) will increase coverage by 0.0%.
The diff coverage is 98.8%.

@@           Coverage Diff           @@
##           master   #32528   +/-   ##
=======================================
  Coverage    82.0%    82.0%           
=======================================
  Files         780      780           
  Lines      210704   210764   +60     
=======================================
+ Hits       172874   172926   +52     
- Misses      37830    37838    +8     

[steviez]

Nice catch, definitely a sneaky bug. Really just two nit comments

[AshwinSekar]

Nice find!

[AshwinSekar]

should we backport this to 1.16?

[ruuda]

What would be the impact of a duplicate not being detected? Is it only wasteful, or is it a correctness issue?