Fix certification renewal (!159)

* fix same bug in cucumber test

* fix(#109): handle certification renewal

* test(#109): reveal certification renewal issue

end2end-tests/tests/cucumber_tests.rs

@@ -416,7 +416,7 @@ async fn should_be_certified_by(
         .await?;
 
     // look for certification by issuer/receiver pair
-    match issuers.binary_search_by(|(issuer_, _)| issuer_index.cmp(issuer_)) {
+    match issuers.binary_search_by(|(issuer_, _)| issuer_.cmp(&issuer_index)) {
         Ok(_) => Ok(()),
         Err(_) => Err(anyhow::anyhow!(
             "no certification found from {} to {}: {:?}",

pallets/certification/src/lib.rs

@@ -406,9 +406,16 @@ pub mod pallet {
             let mut created = false;
             CertsByReceiver::<T, I>::mutate_exists(receiver, |maybe_issuers| {
                 let issuers = maybe_issuers.get_or_insert(Vec::with_capacity(0));
-                if let Err(index) = issuers.binary_search_by(|(issuer_, _)| issuer.cmp(issuer_)) {
-                    issuers.insert(index, (issuer, removable_on));
-                    created = true;
+                match issuers.binary_search_by(|(issuer_, _)| issuer_.cmp(&issuer)) {
+                    // cert exists, must be renewed
+                    Ok(index) => {
+                        issuers[index] = (issuer, removable_on);
+                    }
+                    // cert does not exist, must be created
+                    Err(index) => {
+                        issuers.insert(index, (issuer, removable_on));
+                        created = true;
+                    }
                 }
             });
 
@@ -462,6 +469,7 @@ pub mod pallet {
             total_weight
         }
         /// perform the certification removal
+        /// if block number is given only remove cert if still set to expire at this block number
         fn remove_cert_inner(
             issuer: T::IdtyIndex,
             receiver: T::IdtyIndex,
@@ -474,6 +482,7 @@ pub mod pallet {
                 if let Ok(index) = issuers.binary_search_by(|(issuer_, _)| issuer_.cmp(&issuer)) {
                     if let Some(block_number) = block_number_opt {
                         if let Some((_, removable_on)) = issuers.get(index) {
+                            // only remove cert if block number is matching
                             if *removable_on == block_number {
                                 issuers.remove(index);
                                 removed = true;

pallets/certification/src/tests.rs

@@ -176,6 +176,7 @@ fn test_cert_period() {
     });
 }
 
+// after given validity period, a certification should expire
 #[test]
 fn test_cert_expiry() {
     new_test_ext(DefaultCertificationConfig {
@@ -215,3 +216,48 @@ fn test_cert_expiry() {
         }));
     });
 }
+
+// when renewing a certification, it should not expire now, but later
+#[test]
+fn test_cert_renewal() {
+    new_test_ext(DefaultCertificationConfig {
+        apply_cert_period_at_genesis: false,
+        certs_by_receiver: btreemap![
+            0 => btreemap![
+                1 => Some(5),
+                2 => Some(20),
+            ],
+            1 => btreemap![
+                0 => Some(20),
+                2 => Some(20),
+            ],
+            2 => btreemap![
+                0 => Some(20),
+                1 => Some(20),
+            ],
+        ],
+    })
+    .execute_with(|| {
+        run_to_block(2);
+        // renew certification from bob to alice
+        // this certification should expire 10 blocks later (at block 12)
+        assert_eq!(
+            DefaultCertification::add_cert(RuntimeOrigin::signed(1), 1, 0),
+            Ok(().into())
+        );
+        System::assert_last_event(RuntimeEvent::DefaultCertification(Event::RenewedCert {
+            issuer: 1,
+            receiver: 0,
+        }));
+
+        run_to_block(12);
+        // expiry of previously renewed cert
+        System::assert_last_event(RuntimeEvent::DefaultCertification(Event::RemovedCert {
+            issuer: 1,
+            issuer_issued_count: 1,
+            receiver: 0,
+            receiver_received_count: 1,
+            expiration: true,
+        }));
+    });
+}