From d17dcd6508a404d07b890d661e859eed4580e38d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pascal=20Eng=C3=A9libert?= <tuxmain@zettascript.org>
Date: Mon, 6 Jan 2025 12:41:09 +0100
Subject: [PATCH] fix: disable UD when membership removed
 (nodes/rust/duniter-v2s!299)

* fix: disable UD when membership removed
---
 pallets/universal-dividend/src/types.rs |  4 +-
 runtime/common/src/handlers.rs          | 27 +++++++------
 runtime/common/src/providers.rs         |  2 +-
 runtime/gdev/tests/integration_tests.rs | 51 +++++++++++++++++++++++++
 4 files changed, 66 insertions(+), 18 deletions(-)

diff --git a/pallets/universal-dividend/src/types.rs b/pallets/universal-dividend/src/types.rs
index bc5bcd721..12fb017cf 100644
--- a/pallets/universal-dividend/src/types.rs
+++ b/pallets/universal-dividend/src/types.rs
@@ -22,9 +22,7 @@ use sp_runtime::RuntimeDebug;
 pub type UdIndex = u16;
 
 /// Represents the first eligible Universal Dividend.
-#[derive(
-    Clone, Copy, Default, Eq, PartialEq, RuntimeDebug, serde::Deserialize, serde::Serialize,
-)]
+#[derive(Clone, Default, Eq, PartialEq, RuntimeDebug, serde::Deserialize, serde::Serialize)]
 pub struct FirstEligibleUd(pub Option<NonZeroU16>);
 
 impl FirstEligibleUd {
diff --git a/runtime/common/src/handlers.rs b/runtime/common/src/handlers.rs
index ff9aba851..46c61afa6 100644
--- a/runtime/common/src/handlers.rs
+++ b/runtime/common/src/handlers.rs
@@ -111,21 +111,20 @@ impl<
         // duniter-wot related actions
         let mut weight = pallet_duniter_wot::Pallet::<Runtime>::on_removed(idty_index);
 
-        let mut add_db_reads_writes = |reads, writes| {
-            weight += Runtime::DbWeight::get().reads_writes(reads, writes);
-        };
-
-        // When membership is removed, call on_removed_member handler which auto claims UD.
-        if let Some(idty_value) = pallet_identity::Identities::<Runtime>::get(idty_index) {
-            add_db_reads_writes(1, 0);
-            if let Some(first_ud_index) = idty_value.data.first_eligible_ud.into() {
-                add_db_reads_writes(1, 0);
-                weight += pallet_universal_dividend::Pallet::<Runtime>::on_removed_member(
-                    first_ud_index,
-                    &idty_value.owner_key,
-                );
+        // When membership is removed:
+        // - call on_removed_member handler which auto claims UD;
+        // - set the first_eligible_ud to None so the identity cannot claim UD anymore.
+        pallet_identity::Identities::<Runtime>::mutate(idty_index, |maybe_idty_value| {
+            if let Some(idty_value) = maybe_idty_value {
+                if let Some(first_ud_index) = idty_value.data.first_eligible_ud.0.take() {
+                    weight += pallet_universal_dividend::Pallet::<Runtime>::on_removed_member(
+                        first_ud_index.into(),
+                        &idty_value.owner_key,
+                    );
+                }
             }
-        }
+        });
+        weight += Runtime::DbWeight::get().reads_writes(1, 1);
 
         // When membership is removed, also remove from smith member.
         weight.saturating_add(
diff --git a/runtime/common/src/providers.rs b/runtime/common/src/providers.rs
index 015043f01..1b20fbd0a 100644
--- a/runtime/common/src/providers.rs
+++ b/runtime/common/src/providers.rs
@@ -59,7 +59,7 @@ where
     ) -> Result<R, E> {
         pallet_identity::Pallet::<T>::try_mutate_exists(key, |maybe_idty_data| {
             if let Some(ref mut idty_data) = maybe_idty_data {
-                let mut maybe_first_eligible_ud = Some(idty_data.first_eligible_ud);
+                let mut maybe_first_eligible_ud = Some(idty_data.first_eligible_ud.clone());
                 let result = f(&mut maybe_first_eligible_ud)?;
                 if let Some(first_eligible_ud) = maybe_first_eligible_ud {
                     idty_data.first_eligible_ud = first_eligible_ud;
diff --git a/runtime/gdev/tests/integration_tests.rs b/runtime/gdev/tests/integration_tests.rs
index 2d6b5d157..5ca299b43 100644
--- a/runtime/gdev/tests/integration_tests.rs
+++ b/runtime/gdev/tests/integration_tests.rs
@@ -731,6 +731,57 @@ fn test_revoke_identity_after_one_ud() {
     });
 }
 
+// test that UD cannot be claimed after revocation
+#[test]
+fn test_claim_ud_after_revoke() {
+    ExtBuilder::new(1, 3, 4).build().execute_with(|| {
+        run_to_block(
+            (<Runtime as pallet_universal_dividend::Config>::UdCreationPeriod::get()
+                / <Runtime as pallet_babe::Config>::ExpectedBlockTime::get()
+                + 1) as u32,
+        );
+
+        // before UD, bob has 0 (initial amount)
+        run_to_block(1);
+        assert_eq!(
+            Balances::free_balance(AccountKeyring::Bob.to_account_id()),
+            0
+        );
+
+        // revoke identity
+        Identity::do_revoke_identity(2, pallet_identity::RevocationReason::Root);
+
+        assert_eq!(
+            Balances::free_balance(AccountKeyring::Bob.to_account_id()),
+            1_000
+        );
+
+        // go after UD creation block
+        run_to_block(
+            (<Runtime as pallet_universal_dividend::Config>::UdCreationPeriod::get()
+                / <Runtime as pallet_babe::Config>::ExpectedBlockTime::get()
+                + 1) as u32,
+        );
+
+        assert_eq!(
+            Balances::free_balance(AccountKeyring::Bob.to_account_id()),
+            1_000
+        );
+
+        assert_err!(
+            UniversalDividend::claim_uds(
+                frame_system::RawOrigin::Signed(AccountKeyring::Bob.to_account_id()).into()
+            ),
+            pallet_universal_dividend::Error::<Runtime>::AccountNotAllowedToClaimUds,
+        );
+
+        assert_eq!(
+            Balances::free_balance(AccountKeyring::Bob.to_account_id()),
+            1_000
+        );
+    });
+}
+
 /// test that UD are auto claimed when membership expires
 /// and that claimed UD matches expectations
 #[test]
-- 
GitLab