From 268da199257d6df2bc2d58db8bbc548d2e0cce7f Mon Sep 17 00:00:00 2001
From: Hugo Trentesaux <hugo@trentesaux.fr>
Date: Mon, 13 Jan 2025 19:41:24 +0100
Subject: [PATCH 1/3] fix an uncaught bug
---
pallets/smith-members/src/lib.rs | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/pallets/smith-members/src/lib.rs b/pallets/smith-members/src/lib.rs
index 1ed34c47b..153189d98 100644
--- a/pallets/smith-members/src/lib.rs
+++ b/pallets/smith-members/src/lib.rs
@@ -240,10 +240,13 @@ pub mod pallet {
received_certs: issuers_,
},
);
- ExpiresOn::<T>::append(
- CurrentSession::<T>::get() + T::SmithInactivityMaxDuration::get(),
- receiver,
- );
+ // if smith is offline, schedule expire
+ if !*is_online {
+ ExpiresOn::<T>::append(
+ CurrentSession::<T>::get() + T::SmithInactivityMaxDuration::get(),
+ receiver,
+ );
+ }
}
for (issuer, issued_certs) in cert_meta_by_issuer {
--
GitLab
From 64d970091906edd86806e070b003b9eca186f601 Mon Sep 17 00:00:00 2001
From: Hugo Trentesaux <hugo@trentesaux.fr>
Date: Mon, 13 Jan 2025 19:41:54 +0100
Subject: [PATCH 2/3] reveal bug from #276
---
pallets/smith-members/src/tests.rs | 43 ++++++++++++++++++++++++++++++
1 file changed, 43 insertions(+)
diff --git a/pallets/smith-members/src/tests.rs b/pallets/smith-members/src/tests.rs
index 0ac9ba7ed..c04bd2d17 100644
--- a/pallets/smith-members/src/tests.rs
+++ b/pallets/smith-members/src/tests.rs
@@ -628,6 +628,49 @@ fn certifying_an_online_smith() {
});
}
+/// Shows that scheduled expiration stay scheduled forever, even in the past
+/// This is harmful for storage because we store unnecessary data
+#[test]
+fn expires_on_keeps_growing() {
+ new_test_ext(GenesisConfig {
+ initial_smiths: btreemap![
+ 1 => (true, vec![2, 3]),
+ 2 => (true, vec![1,3]),
+ 3 => (true, vec![1,2]),
+ ],
+ })
+ .execute_with(|| {
+ // Alice goes offline, and is set to expire
+ Pallet::<Runtime>::on_smith_goes_offline(1);
+ // The expiration block is present in smith data
+ assert_eq!(
+ Smiths::<Runtime>::get(1),
+ Some(SmithMeta {
+ status: Smith,
+ expires_on: Some(5),
+ issued_certs: vec![2, 3],
+ received_certs: vec![2, 3]
+ })
+ );
+ // It is also present in ExpiresOn schedule
+ assert_eq!(ExpiresOn::<Runtime>::get(5), Some(vec![1]));
+ // Go to expiration session
+ Pallet::<Runtime>::on_new_session(5);
+ // Alice is expired
+ assert_eq!(
+ Smiths::<Runtime>::get(1),
+ Some(SmithMeta {
+ status: Excluded,
+ expires_on: None,
+ issued_certs: vec![2, 3],
+ received_certs: vec![]
+ })
+ );
+ // but ExpiresOn schedule is still there (and will stay forever)
+ assert_eq!(ExpiresOn::<Runtime>::get(5), Some(vec![1]));
+ });
+}
+
#[test]
fn invitation_on_non_wot_member() {
new_test_ext(GenesisConfig {
--
GitLab
From 0f55fd7f9063a8c91b717fa78d64b8c0e2fa48b5 Mon Sep 17 00:00:00 2001
From: Hugo Trentesaux <hugo@trentesaux.fr>
Date: Mon, 13 Jan 2025 19:43:23 +0100
Subject: [PATCH 3/3] fix bug from #276
---
pallets/smith-members/src/lib.rs | 2 +-
pallets/smith-members/src/tests.rs | 9 ++++-----
2 files changed, 5 insertions(+), 6 deletions(-)
diff --git a/pallets/smith-members/src/lib.rs b/pallets/smith-members/src/lib.rs
index 153189d98..e7c48962a 100644
--- a/pallets/smith-members/src/lib.rs
+++ b/pallets/smith-members/src/lib.rs
@@ -519,7 +519,7 @@ impl<T: Config> Pallet<T> {
/// Handle the removal of Smiths whose expiration time has been reached at a given session index.
fn on_exclude_expired_smiths(at: SessionIndex) {
- if let Some(smiths_to_remove) = ExpiresOn::<T>::get(at) {
+ if let Some(smiths_to_remove) = ExpiresOn::<T>::take(at) {
for smith in smiths_to_remove {
if let Some(smith_meta) = Smiths::<T>::get(smith) {
if let Some(expires_on) = smith_meta.expires_on {
diff --git a/pallets/smith-members/src/tests.rs b/pallets/smith-members/src/tests.rs
index c04bd2d17..c92bffb68 100644
--- a/pallets/smith-members/src/tests.rs
+++ b/pallets/smith-members/src/tests.rs
@@ -628,10 +628,9 @@ fn certifying_an_online_smith() {
});
}
-/// Shows that scheduled expiration stay scheduled forever, even in the past
-/// This is harmful for storage because we store unnecessary data
+/// Test that scheduled expiration is removed after session
#[test]
-fn expires_on_keeps_growing() {
+fn expires_on_cleans_up() {
new_test_ext(GenesisConfig {
initial_smiths: btreemap![
1 => (true, vec![2, 3]),
@@ -666,8 +665,8 @@ fn expires_on_keeps_growing() {
received_certs: vec![]
})
);
- // but ExpiresOn schedule is still there (and will stay forever)
- assert_eq!(ExpiresOn::<Runtime>::get(5), Some(vec![1]));
+ // ExpiresOn is clean
+ assert_eq!(ExpiresOn::<Runtime>::get(5), None);
});
}
--
GitLab