Skip to content
Snippets Groups Projects
Commit 89ea4cf5 authored by Éloïs's avatar Éloïs
Browse files

Smith members can't revoke idty nor change its address (!102)

* add test test_smith_member_cant_revoke_its_idty

* add test test_smith_member_cant_change_idty_address

* migration v400

* fix(idty): smish members can't revoke nor change address

* pallet ud: remove temporary hotfix call

* deps: remove librocksdb-sys

* cargo config: add sub-command to run benchmarks on a pallet
parent fe298c11
Branches
Tags
1 merge request!102Smith members can't revoke idty nor change its address
Showing
with 205 additions and 149 deletions
......@@ -3,5 +3,6 @@ cucumber = "test -p duniter-end2end-tests --test cucumber_tests --"
sanity-gdev = "test -p duniter-live-tests --test sanity_gdev -- --nocapture"
tu = "test --workspace --exclude duniter-end2end-tests --exclude duniter-live-tests"
tb = "test --features runtime-benchmarks -p"
rbp = "run --release --features runtime-benchmarks -- benchmark pallet --chain=dev --steps=50 --repeat=20 --extrinsic=* --execution=wasm --wasm-execution=compiled --heap-pages=4096 --header=./file_header.txt --output=. --pallet"
xtask = "run --package xtask --"
......@@ -177,7 +177,6 @@ httparse = { opt-level = 3 }
integer-sqrt = { opt-level = 3 }
keccak = { opt-level = 3 }
libm = { opt-level = 3 }
librocksdb-sys = { opt-level = 3 }
libsecp256k1 = { opt-level = 3 }
libz-sys = { opt-level = 3 }
mio = { opt-level = 3 }
......
......@@ -17,9 +17,6 @@
#![cfg_attr(not(feature = "std"), no_std)]
#![allow(clippy::type_complexity)]
//pub mod traits;
mod types;
#[cfg(test)]
mod mock;
......@@ -30,14 +27,12 @@ mod tests;
mod benchmarking;*/
pub use pallet::*;
pub use types::*;
use frame_support::dispatch::UnfilteredDispatchable;
use frame_support::pallet_prelude::*;
use frame_system::RawOrigin;
use pallet_certification::traits::SetNextIssuableOn;
use pallet_identity::{IdtyEvent, IdtyStatus};
use sp_membership::traits::IsInPendingMemberships;
use sp_runtime::traits::IsMember;
type IdtyIndex = u32;
......@@ -102,25 +97,33 @@ pub mod pallet {
impl<AccountId, T: Config<I>, I: 'static> pallet_identity::traits::EnsureIdtyCallAllowed<T>
for Pallet<T, I>
where
T: frame_system::Config<AccountId = AccountId>
+ pallet_membership::Config<I, MetaData = MembershipMetaData<AccountId>>,
T: frame_system::Config<AccountId = AccountId> + pallet_membership::Config<I>,
{
fn can_create_identity(creator: IdtyIndex) -> bool {
if !T::IsSubWot::get() {
let cert_meta = pallet_certification::Pallet::<T, I>::idty_cert_meta(creator);
cert_meta.received_count >= T::MinCertForCreateIdtyRight::get()
&& cert_meta.next_issuable_on <= frame_system::pallet::Pallet::<T>::block_number()
&& cert_meta.issued_count < T::MaxByIssuer::get()
} else {
true
}
fn can_confirm_identity(idty_index: IdtyIndex, owner_key: AccountId) -> bool {
}
fn can_confirm_identity(idty_index: IdtyIndex) -> bool {
if !T::IsSubWot::get() {
pallet_membership::Pallet::<T, I>::force_request_membership(
RawOrigin::Root.into(),
idty_index,
MembershipMetaData(owner_key),
Default::default(),
)
.is_ok()
} else {
true
}
}
fn can_validate_identity(idty_index: IdtyIndex) -> bool {
// TODO replace this code by te commented one for distance feature
if !T::IsSubWot::get() {
// TODO replace this code by the commented one for distance feature
/*let idty_cert_meta = pallet_certification::Pallet::<T, I>::idty_cert_meta(idty_index);
idty_cert_meta.received_count >= T::MinCertForMembership::get() as u32*/
pallet_membership::Pallet::<T, I>::claim_membership(
......@@ -128,6 +131,23 @@ where
Some(idty_index),
)
.is_ok()
} else {
true
}
}
fn can_change_identity_address(idty_index: IdtyIndex) -> bool {
if T::IsSubWot::get() {
!pallet_membership::Pallet::<T, I>::is_member(&idty_index)
} else {
true
}
}
fn can_remove_identity(idty_index: IdtyIndex) -> bool {
if T::IsSubWot::get() {
!pallet_membership::Pallet::<T, I>::is_member(&idty_index)
} else {
true
}
}
}
......@@ -141,14 +161,8 @@ impl<T: Config<I>, I: 'static> pallet_certification::traits::IsCertAllowed<IdtyI
}
if let Some(receiver_data) = pallet_identity::Pallet::<T>::identity(receiver) {
match receiver_data.status {
IdtyStatus::ConfirmedByOwner => true,
IdtyStatus::ConfirmedByOwner | IdtyStatus::Validated => true,
IdtyStatus::Created => false,
IdtyStatus::Validated => {
pallet_membership::Pallet::<T, I>::is_member(&receiver)
|| pallet_membership::Pallet::<T, I>::is_in_pending_memberships(
receiver,
)
}
}
} else {
// Receiver not found
......
......@@ -122,7 +122,7 @@ impl pallet_identity::Config for Test {
type ChangeOwnerKeyPeriod = ChangeOwnerKeyPeriod;
type ConfirmPeriod = ConfirmPeriod;
type Event = Event;
type EnsureIdtyCallAllowed = DuniterWot;
type EnsureIdtyCallAllowed = (DuniterWot, SmithsSubWot);
type IdtyCreationPeriod = IdtyCreationPeriod;
type IdtyData = ();
type IdtyNameValidator = IdtyNameValidatorTestImpl;
......@@ -150,7 +150,7 @@ impl pallet_membership::Config<Instance1> for Test {
type IdtyId = IdtyIndex;
type IdtyIdOf = IdentityIndexOf<Self>;
type MembershipPeriod = MembershipPeriod;
type MetaData = crate::MembershipMetaData<u64>;
type MetaData = ();
type OnEvent = DuniterWot;
type PendingMembershipPeriod = PendingMembershipPeriod;
type RevocationPeriod = RevocationPeriod;
......@@ -206,7 +206,7 @@ impl pallet_membership::Config<Instance2> for Test {
type IdtyId = IdtyIndex;
type IdtyIdOf = IdentityIndexOf<Self>;
type MembershipPeriod = SmithsMembershipPeriod;
type MetaData = crate::MembershipMetaData<u64>;
type MetaData = ();
type OnEvent = SmithsSubWot;
type PendingMembershipPeriod = SmithsPendingMembershipPeriod;
type RevocationPeriod = SmithsRevocationPeriod;
......
......@@ -16,11 +16,15 @@
use crate::mock::*;
use crate::mock::{Identity, System};
use frame_support::assert_noop;
use frame_support::assert_ok;
use codec::Encode;
use frame_support::instances::Instance1;
use frame_support::{assert_noop, assert_ok};
use frame_system::{EventRecord, Phase};
use pallet_identity::{IdtyName, IdtyStatus};
use pallet_identity::{
IdtyName, IdtyStatus, NewOwnerKeyPayload, RevocationPayload, NEW_OWNER_KEY_PAYLOAD_PREFIX,
REVOCATION_PAYLOAD_PREFIX,
};
use sp_runtime::testing::TestSignature;
#[test]
fn test_genesis_build() {
......@@ -56,10 +60,7 @@ fn test_join_smiths() {
run_to_block(2);
// Dave shoud be able to request smith membership
assert_ok!(SmithsMembership::request_membership(
Origin::signed(4),
crate::MembershipMetaData(4)
));
assert_ok!(SmithsMembership::request_membership(Origin::signed(4), ()));
System::assert_has_event(Event::SmithsMembership(
pallet_membership::Event::MembershipRequested(4),
));
......@@ -95,6 +96,53 @@ fn test_smith_certs_expirations_should_revoke_smith_membership() {
});
}
#[test]
fn test_smith_member_cant_change_its_idty_address() {
new_test_ext(5, 3).execute_with(|| {
run_to_block(2);
let genesis_hash = System::block_hash(0);
let new_key_payload = NewOwnerKeyPayload {
genesis_hash: &genesis_hash,
idty_index: 3u32,
old_owner_key: &3u64,
};
// Identity 3 can't change it's address
assert_noop!(
Identity::change_owner_key(
Origin::signed(3),
13,
TestSignature(13, (NEW_OWNER_KEY_PAYLOAD_PREFIX, new_key_payload).encode())
),
pallet_identity::Error::<Test>::NotAllowedToChangeIdtyAddress
);
});
}
#[test]
fn test_smith_member_cant_revoke_its_idty() {
new_test_ext(5, 3).execute_with(|| {
run_to_block(2);
let revocation_payload = RevocationPayload {
idty_index: 3u32,
genesis_hash: System::block_hash(0),
};
// Identity 3 can't change it's address
assert_noop!(
Identity::revoke_identity(
Origin::signed(3),
3,
3,
TestSignature(3, (REVOCATION_PAYLOAD_PREFIX, revocation_payload).encode())
),
pallet_identity::Error::<Test>::NotAllowedToRemoveIdty
);
});
}
#[test]
fn test_revoke_smiths_them_rejoin() {
new_test_ext(5, 4).execute_with(|| {
......@@ -109,16 +157,13 @@ fn test_revoke_smiths_them_rejoin() {
// Dave should not be able to re-request membership before the RevocationPeriod end
run_to_block(3);
assert_noop!(
SmithsMembership::request_membership(Origin::signed(4), crate::MembershipMetaData(4)),
SmithsMembership::request_membership(Origin::signed(4), ()),
pallet_membership::Error::<Test, crate::Instance2>::MembershipRevokedRecently
);
// At block #6, Dave shoud be able to request smith membership
run_to_block(6);
assert_ok!(SmithsMembership::request_membership(
Origin::signed(4),
crate::MembershipMetaData(4)
));
assert_ok!(SmithsMembership::request_membership(Origin::signed(4), ()));
// Then, Alice should be able to send a smith cert to Dave
assert_ok!(SmithsCert::add_cert(Origin::signed(1), 1, 4));
......
// Copyright 2021 Axiom-Team
//
// This file is part of Substrate-Libre-Currency.
//
// Substrate-Libre-Currency is free software: you can redistribute it and/or modify
// it under the terms of the GNU Affero General Public License as published by
// the Free Software Foundation, version 3 of the License.
//
// Substrate-Libre-Currency is distributed in the hope that it will be useful,
// but WITHOUT ANY WARRANTY; without even the implied warranty of
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
// GNU Affero General Public License for more details.
//
// You should have received a copy of the GNU Affero General Public License
// along with Substrate-Libre-Currency. If not, see <https://www.gnu.org/licenses/>.
use crate::IdtyIndex;
use frame_support::pallet_prelude::*;
#[derive(Clone, Encode, Decode, Eq, PartialEq, RuntimeDebug, TypeInfo)]
pub struct MembershipMetaData<AccountId>(pub AccountId);
impl<AccountId: Eq> sp_membership::traits::Validate<AccountId> for MembershipMetaData<AccountId> {
fn validate(&self, account_id: &AccountId) -> bool {
&self.0 == account_id
}
}
/*impl From<AccountId> for MembershipMetaData {
fn from(account_id: AccountId) -> Self {
Self(account_id)
}
}*/
/*impl Into<AccountId> for MembershipMetaData {
fn into(self) -> AccountId {
self.0
}
}*/
#[cfg_attr(feature = "std", derive(Debug))]
#[derive(codec::Decode, codec::Encode, Eq, PartialEq, TypeInfo)]
pub enum WotDiff {
AddNode(IdtyIndex),
AddPendingLink(IdtyIndex, IdtyIndex),
AddLink(IdtyIndex, IdtyIndex),
DelLink(IdtyIndex, IdtyIndex),
DisableNode(IdtyIndex),
}
impl Default for WotDiff {
fn default() -> Self {
unreachable!()
}
}
......@@ -70,7 +70,8 @@ pub mod pallet {
type ChangeOwnerKeyPeriod: Get<Self::BlockNumber>;
/// Because this pallet emits events, it depends on the runtime's definition of an event.
type Event: From<Event<Self>> + IsType<<Self as frame_system::Config>::Event>;
/// Management of the authorizations of the different calls. (The default implementation only allows root)
/// Management of the authorizations of the different calls.
/// The default implementation allows everything.
type EnsureIdtyCallAllowed: EnsureIdtyCallAllowed<Self>;
#[pallet::constant]
/// Minimum duration between the creation of 2 identities by the same creator
......@@ -340,7 +341,7 @@ pub mod pallet {
if <IdentitiesNames<T>>::contains_key(&idty_name) {
return Err(Error::<T>::IdtyNameAlreadyExist.into());
}
if !T::EnsureIdtyCallAllowed::can_confirm_identity(idty_index, who.clone()) {
if !T::EnsureIdtyCallAllowed::can_confirm_identity(idty_index) {
return Err(Error::<T>::NotAllowedToConfirmIdty.into());
}
......@@ -415,6 +416,11 @@ pub mod pallet {
Error::<T>::OwnerKeyAlreadyUsed
);
ensure!(
T::EnsureIdtyCallAllowed::can_change_identity_address(idty_index),
Error::<T>::NotAllowedToChangeIdtyAddress
);
let block_number = frame_system::Pallet::<T>::block_number();
let maybe_old_old_owner_key =
if let Some((old_owner_key, last_change)) = idty_value.old_owner_key {
......@@ -497,6 +503,11 @@ pub mod pallet {
Error::<T>::InvalidRevocationKey
);
ensure!(
T::EnsureIdtyCallAllowed::can_remove_identity(idty_index),
Error::<T>::NotAllowedToRemoveIdty
);
let genesis_hash = frame_system::Pallet::<T>::block_hash(T::BlockNumber::zero());
let revocation_payload = RevocationPayload {
genesis_hash,
......@@ -597,8 +608,12 @@ pub mod pallet {
InvalidRevocationKey,
/// Revocation payload signature is invalid
InvalidRevocationSig,
/// Identity not allowed to change address
NotAllowedToChangeIdtyAddress,
/// Not allowed to confirm identity
NotAllowedToConfirmIdty,
/// Not allowed to remove identity
NotAllowedToRemoveIdty,
/// Not allowed to validate identity
NotAllowedToValidateIdty,
/// Identity creation period is not respected
......
......@@ -21,18 +21,33 @@ use sp_runtime::traits::Saturating;
pub trait EnsureIdtyCallAllowed<T: Config> {
fn can_create_identity(creator: T::IdtyIndex) -> bool;
fn can_confirm_identity(idty_index: T::IdtyIndex, owner_key: T::AccountId) -> bool;
fn can_confirm_identity(idty_index: T::IdtyIndex) -> bool;
fn can_validate_identity(idty_index: T::IdtyIndex) -> bool;
fn can_change_identity_address(idty_index: T::IdtyIndex) -> bool;
fn can_remove_identity(idty_index: T::IdtyIndex) -> bool;
}
impl<T: Config> EnsureIdtyCallAllowed<T> for () {
fn can_create_identity(_: T::IdtyIndex) -> bool {
#[impl_for_tuples(5)]
#[allow(clippy::let_and_return)]
impl<T: Config> EnsureIdtyCallAllowed<T> for Tuple {
fn can_create_identity(creator: T::IdtyIndex) -> bool {
for_tuples!( #( if !Tuple::can_create_identity(creator) { return false; } )* );
true
}
fn can_confirm_identity(idty_index: T::IdtyIndex) -> bool {
for_tuples!( #( if !Tuple::can_confirm_identity(idty_index) { return false; } )* );
true
}
fn can_validate_identity(idty_index: T::IdtyIndex) -> bool {
for_tuples!( #( if !Tuple::can_validate_identity(idty_index) { return false; } )* );
true
}
fn can_confirm_identity(_: T::IdtyIndex, _: T::AccountId) -> bool {
fn can_change_identity_address(idty_index: T::IdtyIndex) -> bool {
for_tuples!( #( if !Tuple::can_change_identity_address(idty_index) { return false; } )* );
true
}
fn can_validate_identity(_: T::IdtyIndex) -> bool {
fn can_remove_identity(idty_index: T::IdtyIndex) -> bool {
for_tuples!( #( if !Tuple::can_remove_identity(idty_index) { return false; } )* );
true
}
}
......
......@@ -72,7 +72,7 @@ pub mod pallet {
/// Something that give the IdtyId on an account id
type IdtyIdOf: Convert<Self::AccountId, Option<Self::IdtyId>>;
/// Optional metadata
type MetaData: Parameter + Validate<Self::AccountId>;
type MetaData: Default + Parameter + Validate<Self::AccountId>;
#[pallet::constant]
/// Maximum life span of a non-renewable membership (in number of blocks)
type MembershipPeriod: Get<Self::BlockNumber>;
......
......@@ -398,22 +398,6 @@ pub mod pallet {
) -> DispatchResultWithPostInfo {
Self::do_transfer_ud(origin, dest, value, ExistenceRequirement::KeepAlive)
}
#[pallet::weight(T::WeightInfo::force_set_first_eligible_ud())]
pub fn force_set_first_eligible_ud(
origin: OriginFor<T>,
who: T::AccountId,
first_eligible_ud: FirstEligibleUd,
) -> DispatchResultWithPostInfo {
ensure_root(origin)?;
T::MembersStorage::try_mutate_exists(&who, |maybe_first_eligible_ud| {
if let Some(ref mut first_eligible_ud_) = maybe_first_eligible_ud {
*first_eligible_ud_ = first_eligible_ud;
}
Ok(().into())
})
}
}
// PUBLIC FUNCTIONS
......
......@@ -26,7 +26,6 @@ pub trait WeightInfo {
fn claim_uds(n: u32) -> Weight;
fn transfer_ud() -> Weight;
fn transfer_ud_keep_alive() -> Weight;
fn force_set_first_eligible_ud() -> Weight;
}
// Insecure weights implementation, use it for tests only!
......@@ -83,11 +82,4 @@ impl WeightInfo for () {
.saturating_add(RocksDbWeight::get().reads(2 as Weight))
.saturating_add(RocksDbWeight::get().writes(2 as Weight))
}
// Storage: Identity IdentityIndexOf (r:1 w:0)
// Storage: Identity Identities (r:1 w:1)
fn force_set_first_eligible_ud() -> Weight {
(0 as Weight)
.saturating_add(RocksDbWeight::get().reads(2 as Weight))
.saturating_add(RocksDbWeight::get().writes(1 as Weight))
}
}
......@@ -106,6 +106,11 @@ pub struct SmithsMembershipMetaData<SessionKeysWrapper> {
pub p2p_endpoint: sp_runtime::RuntimeString,
pub session_keys: SessionKeysWrapper,
}
impl<SessionKeysWrapper> Default for SmithsMembershipMetaData<SessionKeysWrapper> {
fn default() -> Self {
unreachable!()
}
}
impl<SessionKeysWrapper> sp_membership::traits::Validate<AccountId>
for SmithsMembershipMetaData<SessionKeysWrapper>
{
......
......@@ -75,18 +75,15 @@ where
pub struct OnMembershipEventHandler<Inner, Runtime>(core::marker::PhantomData<(Inner, Runtime)>);
type MembershipMetaData = pallet_duniter_wot::MembershipMetaData<AccountId>;
impl<
Inner: sp_membership::traits::OnEvent<IdtyIndex, MembershipMetaData>,
Inner: sp_membership::traits::OnEvent<IdtyIndex, ()>,
Runtime: frame_system::Config<AccountId = AccountId>
+ pallet_identity::Config<IdtyData = IdtyData, IdtyIndex = IdtyIndex>
+ pallet_membership::Config<Instance1, MetaData = MembershipMetaData>
+ pallet_membership::Config<Instance1, MetaData = ()>
+ pallet_universal_dividend::Config,
> sp_membership::traits::OnEvent<IdtyIndex, MembershipMetaData>
for OnMembershipEventHandler<Inner, Runtime>
> sp_membership::traits::OnEvent<IdtyIndex, ()> for OnMembershipEventHandler<Inner, Runtime>
{
fn on_event(membership_event: &sp_membership::Event<IdtyIndex, MembershipMetaData>) -> Weight {
fn on_event(membership_event: &sp_membership::Event<IdtyIndex, ()>) -> Weight {
(match membership_event {
sp_membership::Event::MembershipRevoked(idty_index) => {
if let Some(idty_value) = pallet_identity::Identities::<Runtime>::get(idty_index) {
......
......@@ -430,7 +430,7 @@ macro_rules! pallets_config {
type ChangeOwnerKeyPeriod = ChangeOwnerKeyPeriod;
type ConfirmPeriod = ConfirmPeriod;
type Event = Event;
type EnsureIdtyCallAllowed = Wot;
type EnsureIdtyCallAllowed = (Wot, SmithsSubWot);
type IdtyCreationPeriod = IdtyCreationPeriod;
type IdtyData = IdtyData;
type IdtyIndex = IdtyIndex;
......@@ -451,7 +451,7 @@ macro_rules! pallets_config {
type IdtyId = IdtyIndex;
type IdtyIdOf = common_runtime::providers::IdentityIndexOf<Self>;
type MembershipPeriod = MembershipPeriod;
type MetaData = pallet_duniter_wot::MembershipMetaData<AccountId>;
type MetaData = ();
type OnEvent = OnMembershipEventHandler<Wot, Runtime>;
type PendingMembershipPeriod = PendingMembershipPeriod;
type RevocationPeriod = frame_support::traits::ConstU32<0>;
......
......@@ -96,11 +96,4 @@ impl<T: frame_system::Config> pallet_universal_dividend::WeightInfo for WeightIn
.saturating_add(T::DbWeight::get().reads(2 as Weight))
.saturating_add(T::DbWeight::get().writes(2 as Weight))
}
// Storage: Identity IdentityIndexOf (r:1 w:0)
// Storage: Identity Identities (r:1 w:1)
fn force_set_first_eligible_ud() -> Weight {
(57_000_000 as Weight)
.saturating_add(T::DbWeight::get().reads(2 as Weight))
.saturating_add(T::DbWeight::get().writes(1 as Weight))
}
}
......@@ -28,6 +28,8 @@ extern crate frame_benchmarking;
pub mod parameters;
mod migrations_v400;
pub use self::parameters::*;
pub use common_runtime::{
constants::*, entities::*, handlers::*, AccountId, Address, Balance, BlockNumber,
......@@ -127,6 +129,7 @@ pub type Executive = frame_executive::Executive<
frame_system::ChainContext<Runtime>,
Runtime,
AllPalletsWithSystem,
migrations_v400::MigrationsV400,
>;
pub type TechnicalCommitteeInstance = Instance2;
......
// Copyright 2021-2022 Axiom-Team
//
// This file is part of Duniter-v2S.
//
// Duniter-v2S is free software: you can redistribute it and/or modify
// it under the terms of the GNU Affero General Public License as published by
// the Free Software Foundation, version 3 of the License.
//
// Duniter-v2S is distributed in the hope that it will be useful,
// but WITHOUT ANY WARRANTY; without even the implied warranty of
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
// GNU Affero General Public License for more details.
//
// You should have received a copy of the GNU Affero General Public License
// along with Duniter-v2S. If not, see <https://www.gnu.org/licenses/>.
use crate::*;
pub struct MigrationsV400;
impl frame_support::traits::OnRuntimeUpgrade for MigrationsV400 {
fn on_runtime_upgrade() -> Weight {
let mut weight = 1_000_000_000; // Safety margin
type OldvalueType = AccountId;
pallet_membership::PendingMembership::<Runtime, Instance1>::translate_values(
|_: OldvalueType| {
weight += <Runtime as frame_system::Config>::DbWeight::get().write;
Some(())
},
);
weight
}
#[cfg(feature = "try-runtime")]
fn pre_upgrade() -> Result<(), &'static str> {
Ok(())
}
#[cfg(feature = "try-runtime")]
fn post_upgrade() -> Result<(), &'static str> {
Ok(())
}
}
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Please to comment