From 14e540eae2e25d6848b49df062974df12cbc4962 Mon Sep 17 00:00:00 2001
From: Hugo Trentesaux <hugo.trentesaux@lilo.org>
Date: Wed, 3 May 2023 11:25:23 +0200
Subject: [PATCH] Fix certification renewal (nodes/rust/duniter-v2s!159)
* fix same bug in cucumber test
* fix(#109): handle certification renewal
* test(#109): reveal certification renewal issue
---
end2end-tests/tests/cucumber_tests.rs | 2 +-
pallets/certification/src/lib.rs | 15 +++++++--
pallets/certification/src/tests.rs | 46 +++++++++++++++++++++++++++
3 files changed, 59 insertions(+), 4 deletions(-)
diff --git a/end2end-tests/tests/cucumber_tests.rs b/end2end-tests/tests/cucumber_tests.rs
index 50ba89a58..cd7876032 100644
--- a/end2end-tests/tests/cucumber_tests.rs
+++ b/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 {}: {:?}",
diff --git a/pallets/certification/src/lib.rs b/pallets/certification/src/lib.rs
index 27fab1982..a734fcdeb 100644
--- a/pallets/certification/src/lib.rs
+++ b/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;
diff --git a/pallets/certification/src/tests.rs b/pallets/certification/src/tests.rs
index c22880e8f..01d1b064f 100644
--- a/pallets/certification/src/tests.rs
+++ b/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,
+ }));
+ });
+}
--
GitLab