From e360cff92e9e4e2e9c48d260ef391949fccb1bd9 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bastian=20K=C3=B6cher?= <bkchr@users.noreply.github.com>
Date: Sat, 12 Jun 2021 01:38:17 +0100
Subject: [PATCH] pallet-offences: Switch to partition_point (#9049)

This changes the code to use `partition_point` instead of
`binary_search_by_key`, because this was very likely the problematic
pallet 2 weeks ago on polkadot.
---
 substrate/frame/offences/src/lib.rs   | 11 ++---
 substrate/frame/offences/src/mock.rs  |  5 +++
 substrate/frame/offences/src/tests.rs | 61 ++++++++++++++++++++++++++-
 3 files changed, 68 insertions(+), 9 deletions(-)

diff --git a/substrate/frame/offences/src/lib.rs b/substrate/frame/offences/src/lib.rs
index 82665099d6..1076dd6154 100644
--- a/substrate/frame/offences/src/lib.rs
+++ b/substrate/frame/offences/src/lib.rs
@@ -281,15 +281,10 @@ impl<T: Config, O: Offence<T::IdentificationTuple>> ReportIndexStorage<T, O> {
 	fn insert(&mut self, time_slot: &O::TimeSlot, report_id: ReportIdOf<T>) {
 		// Insert the report id into the list while maintaining the ordering by the time
 		// slot.
-		let pos = match self
+		let pos = self
 			.same_kind_reports
-			.binary_search_by_key(&time_slot, |&(ref when, _)| when)
-		{
-			Ok(pos) => pos,
-			Err(pos) => pos,
-		};
-		self.same_kind_reports
-			.insert(pos, (time_slot.clone(), report_id));
+			.partition_point(|&(ref when, _)| when <= time_slot);
+		self.same_kind_reports.insert(pos, (time_slot.clone(), report_id));
 
 		// Update the list of concurrent reports.
 		self.concurrent_reports.push(report_id);
diff --git a/substrate/frame/offences/src/mock.rs b/substrate/frame/offences/src/mock.rs
index e7655d7ee2..a494ab02eb 100644
--- a/substrate/frame/offences/src/mock.rs
+++ b/substrate/frame/offences/src/mock.rs
@@ -170,3 +170,8 @@ impl<T: Clone> offence::Offence<T> for Offence<T> {
 		Perbill::from_percent(5 + offenders_count * 100 / validator_set_count)
 	}
 }
+
+/// Create the report id for the given `offender` and `time_slot` combination.
+pub fn report_id(time_slot: u128, offender: u64) -> H256 {
+	Offences::report_id::<Offence<u64>>(&time_slot, &offender)
+}
diff --git a/substrate/frame/offences/src/tests.rs b/substrate/frame/offences/src/tests.rs
index edc22cb239..d2e0f2d63d 100644
--- a/substrate/frame/offences/src/tests.rs
+++ b/substrate/frame/offences/src/tests.rs
@@ -22,7 +22,7 @@
 use super::*;
 use crate::mock::{
 	Offences, System, Offence, Event, KIND, new_test_ext, with_on_offence_fractions,
-	offence_reports,
+	offence_reports, report_id,
 };
 use sp_runtime::Perbill;
 use frame_system::{EventRecord, Phase};
@@ -284,3 +284,62 @@ fn should_properly_count_offences() {
 		);
 	});
 }
+
+/// We insert offences in sorted order using the time slot in the `same_kind_reports`.
+/// This test ensures that it works as expected.
+#[test]
+fn should_properly_sort_offences() {
+	new_test_ext().execute_with(|| {
+		// given
+		let time_slot = 42;
+		assert_eq!(offence_reports(KIND, time_slot), vec![]);
+
+		let offence1 = Offence {
+			validator_set_count: 5,
+			time_slot,
+			offenders: vec![5],
+		};
+		let offence2 = Offence {
+			validator_set_count: 5,
+			time_slot,
+			offenders: vec![4],
+		};
+		let offence3 = Offence {
+			validator_set_count: 5,
+			time_slot: time_slot + 1,
+			offenders: vec![6, 7],
+		};
+		let offence4 = Offence {
+			validator_set_count: 5,
+			time_slot: time_slot - 1,
+			offenders: vec![3],
+		};
+		Offences::report_offence(vec![], offence1).unwrap();
+		with_on_offence_fractions(|f| {
+			assert_eq!(f.clone(), vec![Perbill::from_percent(25)]);
+			f.clear();
+		});
+
+		// when
+		// report for the second time
+		Offences::report_offence(vec![], offence2).unwrap();
+		Offences::report_offence(vec![], offence3).unwrap();
+		Offences::report_offence(vec![], offence4).unwrap();
+
+		// then
+		let same_kind_reports =
+			Vec::<(u128, sp_core::H256)>::decode(
+				&mut &crate::ReportsByKindIndex::<crate::mock::Runtime>::get(KIND)[..],
+			).unwrap();
+		assert_eq!(
+			same_kind_reports,
+			vec![
+				(time_slot - 1, report_id(time_slot - 1, 3)),
+				(time_slot, report_id(time_slot, 5)),
+				(time_slot, report_id(time_slot, 4)),
+				(time_slot + 1, report_id(time_slot + 1, 6)),
+				(time_slot + 1, report_id(time_slot + 1, 7)),
+			]
+		);
+	});
+}