From ae224f73aec9526e372222de50093ffc501de626 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C3=89lo=C3=AFs?= <c@elo.tf>
Date: Fri, 21 Mar 2025 19:10:59 +0100
Subject: [PATCH] Resolve "Perf: use host function to verify legacy revocation
document" (nodes/rust/duniter-v2s!320)
* check metadata with our own subxt fork
* rustfmt
* use host function for sig verification of legacy revoke document
---
Cargo.lock | 3 +-
Cargo.toml | 3 --
pallets/distance/src/mock.rs | 36 ++++++++++++++++++++++-
pallets/duniter-wot/src/mock.rs | 43 +++++++++++++++++++++++++++-
pallets/duniter-wot/src/tests.rs | 5 ++--
pallets/identity/Cargo.toml | 1 -
pallets/identity/src/lib.rs | 22 +++++++++-----
pallets/identity/src/mock.rs | 1 +
pallets/quota/src/mock.rs | 1 +
runtime/common/src/pallets_config.rs | 1 +
scripts/check_metadata.sh | 2 +-
11 files changed, 99 insertions(+), 19 deletions(-)
diff --git a/Cargo.lock b/Cargo.lock
index b8f42cc06..92c7b383b 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -1,6 +1,6 @@
# This file is automatically @generated by Cargo.
# It is not intended for manual editing.
-version = 3
+version = 4
[[package]]
name = "Inflector"
@@ -9058,7 +9058,6 @@ dependencies = [
"base64 0.22.1",
"bs58",
"duniter-primitives",
- "ed25519-dalek",
"frame-benchmarking",
"frame-support",
"frame-system",
diff --git a/Cargo.toml b/Cargo.toml
index 54ee0162e..44e1a7c9b 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -52,7 +52,6 @@ base64 = { version = "0.22.1", default-features = false }
countmap = { version = "0.2.0", default-features = false }
ctrlc = { version = "3.4.4", default-features = false }
cucumber = { version = "0.20.2", default-features = false }
-ed25519-dalek = { version = "2.1.1", default-features = false }
env_logger = { version = "0.11.3", default-features = false }
notify = { version = "6.1.1", default-features = false }
portpicker = { version = "0.1.1", default-features = false }
@@ -248,8 +247,6 @@ cranelift-wasm = { opt-level = 3 }
crc32fast = { opt-level = 3 }
crossbeam-deque = { opt-level = 3 }
crypto-mac = { opt-level = 3 }
-curve25519-dalek = { opt-level = 3 }
-ed25519-dalek = { opt-level = 3 }
futures-channel = { opt-level = 3 }
hashbrown = { opt-level = 3 }
hash-db = { opt-level = 3 }
diff --git a/pallets/distance/src/mock.rs b/pallets/distance/src/mock.rs
index e804df741..06b89976b 100644
--- a/pallets/distance/src/mock.rs
+++ b/pallets/distance/src/mock.rs
@@ -27,7 +27,7 @@ use sp_core::{ConstU32, H256};
use sp_runtime::{
impl_opaque_keys,
key_types::DUMMY,
- testing::{TestSignature, UintAuthorityId},
+ testing::{TestSignature as SubtrateTestSignature, UintAuthorityId},
traits::{BlakeTwo256, ConvertInto, IdentityLookup, IsMember, OpaqueKeys},
BuildStorage, KeyTypeId, Perbill,
};
@@ -36,6 +36,39 @@ type Balance = u64;
type Block = frame_system::mocking::MockBlock<Test>;
pub type AccountId = u64;
+pub struct AccountId32Mock(u64);
+impl From<AccountId32Mock> for u64 {
+ fn from(account: AccountId32Mock) -> u64 {
+ account.0
+ }
+}
+impl From<[u8; 32]> for AccountId32Mock {
+ fn from(bytes: [u8; 32]) -> Self {
+ Self(u64::from_be_bytes([
+ bytes[0], bytes[1], bytes[2], bytes[3], bytes[4], bytes[5], bytes[6], bytes[7],
+ ]))
+ }
+}
+
+/// Test signature that impl From<ed25519::Signature> (required to compile pallet identity)
+#[derive(Clone, codec::Decode, Debug, Eq, codec::Encode, PartialEq, scale_info::TypeInfo)]
+pub struct TestSignature(SubtrateTestSignature);
+impl From<sp_core::ed25519::Signature> for TestSignature {
+ fn from(_: sp_core::ed25519::Signature) -> Self {
+ // Implementation here only to satisfy traits bounds at compilation
+ // This convertion should not be used inside pallet distance tests
+ unimplemented!()
+ }
+}
+
+impl sp_runtime::traits::Verify for TestSignature {
+ type Signer = UintAuthorityId;
+
+ fn verify<L: sp_runtime::traits::Lazy<[u8]>>(&self, msg: L, signer: &u64) -> bool {
+ <SubtrateTestSignature as sp_runtime::traits::Verify>::verify::<L>(&self.0, msg, signer)
+ }
+}
+
impl_opaque_keys! {
pub struct MockSessionKeys {
pub dummy: UintAuthorityId,
@@ -217,6 +250,7 @@ impl pallet_identity::traits::IdtyNameValidator for IdtyNameValidatorTestImpl {
}
impl pallet_identity::Config for Test {
+ type AccountId32 = AccountId32Mock;
type AccountLinker = ();
type AutorevocationPeriod = AutorevocationPeriod;
type ChangeOwnerKeyPeriod = ChangeOwnerKeyPeriod;
diff --git a/pallets/duniter-wot/src/mock.rs b/pallets/duniter-wot/src/mock.rs
index a3c550632..e246133a0 100644
--- a/pallets/duniter-wot/src/mock.rs
+++ b/pallets/duniter-wot/src/mock.rs
@@ -20,7 +20,7 @@ use frame_support::{derive_impl, parameter_types, traits::Everything};
use frame_system as system;
use sp_core::H256;
use sp_runtime::{
- testing::{TestSignature, UintAuthorityId},
+ testing::{TestSignature as SubtrateTestSignature, UintAuthorityId},
traits::{BlakeTwo256, IdentityLookup},
BuildStorage,
};
@@ -30,6 +30,46 @@ use std::collections::BTreeMap;
type AccountId = u64;
type Block = frame_system::mocking::MockBlock<Test>;
+pub struct AccountId32Mock(u64);
+impl From<AccountId32Mock> for u64 {
+ fn from(account: AccountId32Mock) -> u64 {
+ account.0
+ }
+}
+impl From<[u8; 32]> for AccountId32Mock {
+ fn from(bytes: [u8; 32]) -> Self {
+ Self(u64::from_be_bytes([
+ bytes[0], bytes[1], bytes[2], bytes[3], bytes[4], bytes[5], bytes[6], bytes[7],
+ ]))
+ }
+}
+
+/// Test signature that impl From<ed25519::Signature> (required to compile pallet identity)
+#[derive(Clone, codec::Decode, Debug, Eq, codec::Encode, PartialEq, scale_info::TypeInfo)]
+pub struct TestSignature(SubtrateTestSignature);
+
+impl TestSignature {
+ pub fn new(signer: u64, message: Vec<u8>) -> Self {
+ Self(SubtrateTestSignature(signer, message))
+ }
+}
+
+impl From<sp_core::ed25519::Signature> for TestSignature {
+ fn from(_: sp_core::ed25519::Signature) -> Self {
+ // Implementation here only to satisfy traits bounds at compilation
+ // This convertion should not be used inside pallet distance tests
+ unimplemented!()
+ }
+}
+
+impl sp_runtime::traits::Verify for TestSignature {
+ type Signer = UintAuthorityId;
+
+ fn verify<L: sp_runtime::traits::Lazy<[u8]>>(&self, msg: L, signer: &u64) -> bool {
+ <SubtrateTestSignature as sp_runtime::traits::Verify>::verify::<L>(&self.0, msg, signer)
+ }
+}
+
// Configure a mock runtime to test the pallet.
frame_support::construct_runtime!(
pub enum Test {
@@ -96,6 +136,7 @@ impl pallet_identity::traits::IdtyNameValidator for IdtyNameValidatorTestImpl {
}
impl pallet_identity::Config for Test {
+ type AccountId32 = AccountId32Mock;
type AccountLinker = ();
type AutorevocationPeriod = AutorevocationPeriod;
type ChangeOwnerKeyPeriod = ChangeOwnerKeyPeriod;
diff --git a/pallets/duniter-wot/src/tests.rs b/pallets/duniter-wot/src/tests.rs
index a6baf65e2..6db6cfe80 100644
--- a/pallets/duniter-wot/src/tests.rs
+++ b/pallets/duniter-wot/src/tests.rs
@@ -21,7 +21,6 @@ use pallet_identity::{
IdtyName, IdtyStatus, RevocationPayload, RevocationReason, REVOCATION_PAYLOAD_PREFIX,
};
use pallet_membership::MembershipRemovalReason;
-use sp_runtime::testing::TestSignature;
/// test that genesis builder creates the good number of identities
/// and good identity and certification metadate
@@ -186,7 +185,7 @@ fn test_revoke_idty() {
RuntimeOrigin::signed(1),
1,
1,
- TestSignature(
+ TestSignature::new(
1,
(
REVOCATION_PAYLOAD_PREFIX,
@@ -211,7 +210,7 @@ fn test_revoke_idty() {
RuntimeOrigin::signed(42),
2,
2,
- TestSignature(
+ TestSignature::new(
2,
(
REVOCATION_PAYLOAD_PREFIX,
diff --git a/pallets/identity/Cargo.toml b/pallets/identity/Cargo.toml
index f86d0f991..13eb8edf3 100644
--- a/pallets/identity/Cargo.toml
+++ b/pallets/identity/Cargo.toml
@@ -46,7 +46,6 @@ base64 = { workspace = true }
bs58 = { workspace = true }
codec = { workspace = true, features = ["derive"] }
duniter-primitives = { workspace = true }
-ed25519-dalek = { workspace = true }
frame-benchmarking = { workspace = true, optional = true }
frame-support = { workspace = true }
frame-system = { workspace = true }
diff --git a/pallets/identity/src/lib.rs b/pallets/identity/src/lib.rs
index 64f47fe83..0e00c7e7a 100644
--- a/pallets/identity/src/lib.rs
+++ b/pallets/identity/src/lib.rs
@@ -103,6 +103,9 @@ pub mod pallet {
#[pallet::config]
pub trait Config: frame_system::Config {
+ /// The way to deserialize accound id from 32 raw bytes
+ type AccountId32: From<[u8; 32]> + Into<Self::AccountId>;
+
/// The period during which the owner can confirm the new identity.
#[pallet::constant]
type ConfirmPeriod: Get<BlockNumberFor<Self>>;
@@ -173,7 +176,9 @@ pub mod pallet {
type Signer: IdentifyAccount<AccountId = Self::AccountId>;
/// Signature type for payload verification.
- type Signature: Parameter + Verify<Signer = Self::Signer>;
+ type Signature: Parameter
+ + Verify<Signer = Self::Signer>
+ + From<sp_core::ed25519::Signature>;
/// The overarching event type.
type RuntimeEvent: From<Event<Self>> + IsType<<Self as frame_system::Config>::RuntimeEvent>;
@@ -637,10 +642,15 @@ pub mod pallet {
)
.into_array_const::<32>()
.map_err(|_| Error::<T>::InvalidLegacyRevocationFormat)?;
- ed25519_dalek::VerifyingKey::from_bytes(&issuer)
- .map_err(|_| Error::<T>::InvalidLegacyRevocationFormat)?
- .verify_strict(document, &ed25519_dalek::Signature::from_bytes(&signature))
- .map_err(|_| Error::<T>::InvalidSignature)?;
+
+ // Verify signature
+ let revocation_key = T::AccountId32::from(issuer).into();
+ let signature = T::Signature::from(sp_core::ed25519::Signature::from(signature));
+ ensure!(
+ signature.verify(document, &revocation_key),
+ Error::<T>::InvalidSignature
+ );
+
let username = line_username
.get(14..)
.ok_or(Error::<T>::InvalidLegacyRevocationFormat)?;
@@ -657,8 +667,6 @@ pub mod pallet {
IdtyStatus::Revoked => Err(Error::<T>::AlreadyRevoked),
}?;
- let revocation_key = T::AccountId::decode(&mut &issuer[..]).unwrap();
-
ensure!(
if let Some((ref old_owner_key, last_change)) = idty_value.old_owner_key {
// old owner key can also revoke the identity until the period expired
diff --git a/pallets/identity/src/mock.rs b/pallets/identity/src/mock.rs
index 70758bc81..de48f0a2e 100644
--- a/pallets/identity/src/mock.rs
+++ b/pallets/identity/src/mock.rs
@@ -89,6 +89,7 @@ impl pallet_identity::traits::IdtyNameValidator for IdtyNameValidatorTestImpl {
}
impl pallet_identity::Config for Test {
+ type AccountId32 = AccountId;
type AccountLinker = ();
type AutorevocationPeriod = AutorevocationPeriod;
type ChangeOwnerKeyPeriod = ChangeOwnerKeyPeriod;
diff --git a/pallets/quota/src/mock.rs b/pallets/quota/src/mock.rs
index f39b053df..dcfd30f77 100644
--- a/pallets/quota/src/mock.rs
+++ b/pallets/quota/src/mock.rs
@@ -133,6 +133,7 @@ impl pallet_identity::traits::IdtyNameValidator for IdtyNameValidatorTestImpl {
}
}
impl pallet_identity::Config for Test {
+ type AccountId32 = AccountId;
type AccountLinker = ();
type AutorevocationPeriod = AutorevocationPeriod;
type ChangeOwnerKeyPeriod = ChangeOwnerKeyPeriod;
diff --git a/runtime/common/src/pallets_config.rs b/runtime/common/src/pallets_config.rs
index 04fa45d06..c3b6e728f 100644
--- a/runtime/common/src/pallets_config.rs
+++ b/runtime/common/src/pallets_config.rs
@@ -432,6 +432,7 @@ macro_rules! pallets_config {
pub const ValidationPeriod: BlockNumber = 2 * MONTHS;
}
impl pallet_identity::Config for Runtime {
+ type AccountId32 = AccountId;
type AccountLinker = Account;
type AutorevocationPeriod = AutorevocationPeriod;
type ChangeOwnerKeyPeriod = ChangeOwnerKeyPeriod;
diff --git a/scripts/check_metadata.sh b/scripts/check_metadata.sh
index 69bd92b6f..053b92c18 100755
--- a/scripts/check_metadata.sh
+++ b/scripts/check_metadata.sh
@@ -1,6 +1,6 @@
#!/bin/sh
-cargo install subxt-cli
+cargo install subxt-cli --git https://github.com/duniter/subxt --branch subxt-v0.38.0-duniter-substrate-v1.17.0
cargo build
target/debug/duniter --dev&
sleep 20
--
GitLab