diff --git a/Cargo.lock b/Cargo.lock index fe0de516f105f..82aac5fb2a8de 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -10469,7 +10469,7 @@ dependencies = [ [[package]] name = "pallet-identity" -version = "28.0.0" +version = "29.0.0" dependencies = [ "enumflags2", "frame-benchmarking", diff --git a/prdoc/pr_4646.prdoc b/prdoc/pr_4646.prdoc new file mode 100644 index 0000000000000..0252deb57bdda --- /dev/null +++ b/prdoc/pr_4646.prdoc @@ -0,0 +1,20 @@ +# Schema: Polkadot SDK PRDoc Schema (prdoc) v1.0.0 +# See doc at https://raw.githubusercontent.com/paritytech/polkadot-sdk/master/prdoc/schema_user.json + +title: "[Identity] Remove double encoding username signature payload" + +doc: + - audience: Runtime Dev + description: | + The signature payload for setting a username for an account in `pallet-identity` is now just + the raw bytes of said username (still including the suffix), removing the need to first + encode these bytes before signing. + - audience: Runtime User + description: | + The signature payload for setting a username for an account in `pallet-identity` is now just + the raw bytes of said username (still including the suffix), removing the need to first + encode these bytes before signing. + +crates: + - name: pallet-identity + bump: major diff --git a/substrate/frame/identity/Cargo.toml b/substrate/frame/identity/Cargo.toml index e0bce8a77bdc6..987e418048d36 100644 --- a/substrate/frame/identity/Cargo.toml +++ b/substrate/frame/identity/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "pallet-identity" -version = "28.0.0" +version = "29.0.0" authors.workspace = true edition.workspace = true license = "Apache-2.0" diff --git a/substrate/frame/identity/src/benchmarking.rs b/substrate/frame/identity/src/benchmarking.rs index cdcdb9522615d..957549b19f859 100644 --- a/substrate/frame/identity/src/benchmarking.rs +++ b/substrate/frame/identity/src/benchmarking.rs @@ -22,7 +22,6 @@ use super::*; use crate::Pallet as Identity; -use codec::Encode; use frame_benchmarking::{account, v2::*, whitelisted_caller, BenchmarkError}; use frame_support::{ assert_ok, ensure, @@ -623,17 +622,17 @@ mod benchmarks { let username = bench_username(); let bounded_username = bounded_username::(username.clone(), suffix.clone()); - let encoded_username = Encode::encode(&bounded_username.to_vec()); let public = sr25519_generate(0.into(), None); let who_account: T::AccountId = MultiSigner::Sr25519(public).into_account().into(); let who_lookup = T::Lookup::unlookup(who_account.clone()); - let signature = - MultiSignature::Sr25519(sr25519_sign(0.into(), &public, &encoded_username).unwrap()); + let signature = MultiSignature::Sr25519( + sr25519_sign(0.into(), &public, &bounded_username[..]).unwrap(), + ); // Verify signature here to avoid surprise errors at runtime - assert!(signature.verify(&encoded_username[..], &public.into())); + assert!(signature.verify(&bounded_username[..], &public.into())); #[extrinsic_call] _(RawOrigin::Signed(authority.clone()), who_lookup, username, Some(signature.into())); diff --git a/substrate/frame/identity/src/lib.rs b/substrate/frame/identity/src/lib.rs index 5a36101cc2f79..50d6de32ac61e 100644 --- a/substrate/frame/identity/src/lib.rs +++ b/substrate/frame/identity/src/lib.rs @@ -1116,8 +1116,7 @@ pub mod pallet { if let Some(s) = signature { // Account has pre-signed an authorization. Verify the signature provided and grant // the username directly. - let encoded = Encode::encode(&bounded_username.to_vec()); - Self::validate_signature(&encoded, &s, &who)?; + Self::validate_signature(&bounded_username[..], &s, &who)?; Self::insert_username(&who, bounded_username); } else { // The user must accept the username, therefore, queue it. @@ -1267,12 +1266,12 @@ impl Pallet { /// Validate a signature. Supports signatures on raw `data` or `data` wrapped in HTML ``. pub fn validate_signature( - data: &Vec, + data: &[u8], signature: &T::OffchainSignature, signer: &T::AccountId, ) -> DispatchResult { // Happy path, user has signed the raw data. - if signature.verify(&data[..], &signer) { + if signature.verify(data, &signer) { return Ok(()) } // NOTE: for security reasons modern UIs implicitly wrap the data requested to sign into diff --git a/substrate/frame/identity/src/tests.rs b/substrate/frame/identity/src/tests.rs index 60579a23b91b9..b1a953d487ce2 100644 --- a/substrate/frame/identity/src/tests.rs +++ b/substrate/frame/identity/src/tests.rs @@ -1042,13 +1042,13 @@ fn set_username_with_signature_without_existing_identity_should_work() { // set up username let (username, username_to_sign) = test_username_of(b"42".to_vec(), suffix); - let encoded_username = Encode::encode(&username_to_sign.to_vec()); // set up user and sign message let public = sr25519_generate(0.into(), None); let who_account: AccountIdOf = MultiSigner::Sr25519(public).into_account().into(); - let signature = - MultiSignature::Sr25519(sr25519_sign(0.into(), &public, &encoded_username).unwrap()); + let signature = MultiSignature::Sr25519( + sr25519_sign(0.into(), &public, &username_to_sign[..]).unwrap(), + ); assert_ok!(Identity::set_username_for( RuntimeOrigin::signed(authority), @@ -1093,13 +1093,13 @@ fn set_username_with_signature_with_existing_identity_should_work() { // set up username let (username, username_to_sign) = test_username_of(b"42".to_vec(), suffix); - let encoded_username = Encode::encode(&username_to_sign.to_vec()); // set up user and sign message let public = sr25519_generate(0.into(), None); let who_account: AccountIdOf = MultiSigner::Sr25519(public).into_account().into(); - let signature = - MultiSignature::Sr25519(sr25519_sign(0.into(), &public, &encoded_username).unwrap()); + let signature = MultiSignature::Sr25519( + sr25519_sign(0.into(), &public, &username_to_sign[..]).unwrap(), + ); // Set an identity for who. They need some balance though. Balances::make_free_balance_be(&who_account, 1000); @@ -1156,13 +1156,13 @@ fn set_username_with_bytes_signature_should_work() { let unwrapped_username = username_to_sign.to_vec(); // Sign an unwrapped version, as in `username.suffix`. - let unwrapped_encoded = Encode::encode(&unwrapped_username); - let signature_on_unwrapped = - MultiSignature::Sr25519(sr25519_sign(0.into(), &public, &unwrapped_encoded).unwrap()); + let signature_on_unwrapped = MultiSignature::Sr25519( + sr25519_sign(0.into(), &public, &unwrapped_username[..]).unwrap(), + ); // Trivial assert_ok!(Identity::validate_signature( - &unwrapped_encoded, + &unwrapped_username, &signature_on_unwrapped, &who_account )); @@ -1174,7 +1174,7 @@ fn set_username_with_bytes_signature_should_work() { let mut wrapped_username: Vec = Vec::with_capacity(unwrapped_username.len() + prehtml.len() + posthtml.len()); wrapped_username.extend(prehtml); - wrapped_username.extend(unwrapped_encoded.clone()); + wrapped_username.extend(&unwrapped_username); wrapped_username.extend(posthtml); let signature_on_wrapped = MultiSignature::Sr25519(sr25519_sign(0.into(), &public, &wrapped_username).unwrap()); @@ -1182,7 +1182,7 @@ fn set_username_with_bytes_signature_should_work() { // We want to call `validate_signature` on the *unwrapped* username, but the signature on // the *wrapped* data. assert_ok!(Identity::validate_signature( - &unwrapped_encoded, + &unwrapped_username, &signature_on_wrapped, &who_account )); @@ -1401,9 +1401,8 @@ fn setting_primary_should_work() { // set up username let (first_username, first_to_sign) = test_username_of(b"42".to_vec(), suffix.clone()); - let encoded_username = Encode::encode(&first_to_sign.to_vec()); let first_signature = - MultiSignature::Sr25519(sr25519_sign(0.into(), &public, &encoded_username).unwrap()); + MultiSignature::Sr25519(sr25519_sign(0.into(), &public, &first_to_sign[..]).unwrap()); assert_ok!(Identity::set_username_for( RuntimeOrigin::signed(authority.clone()), @@ -1427,9 +1426,8 @@ fn setting_primary_should_work() { // set up username let (second_username, second_to_sign) = test_username_of(b"101".to_vec(), suffix); - let encoded_username = Encode::encode(&second_to_sign.to_vec()); let second_signature = - MultiSignature::Sr25519(sr25519_sign(0.into(), &public, &encoded_username).unwrap()); + MultiSignature::Sr25519(sr25519_sign(0.into(), &public, &second_to_sign[..]).unwrap()); assert_ok!(Identity::set_username_for( RuntimeOrigin::signed(authority), @@ -1510,10 +1508,8 @@ fn must_own_primary() { let pi_account: AccountIdOf = MultiSigner::Sr25519(pi_public).into_account().into(); let (pi_username, pi_to_sign) = test_username_of(b"username314159".to_vec(), suffix.clone()); - let encoded_pi_username = Encode::encode(&pi_to_sign.to_vec()); - let pi_signature = MultiSignature::Sr25519( - sr25519_sign(0.into(), &pi_public, &encoded_pi_username).unwrap(), - ); + let pi_signature = + MultiSignature::Sr25519(sr25519_sign(0.into(), &pi_public, &pi_to_sign[..]).unwrap()); assert_ok!(Identity::set_username_for( RuntimeOrigin::signed(authority.clone()), pi_account.clone(), @@ -1525,10 +1521,8 @@ fn must_own_primary() { let e_public = sr25519_generate(1.into(), None); let e_account: AccountIdOf = MultiSigner::Sr25519(e_public).into_account().into(); let (e_username, e_to_sign) = test_username_of(b"username271828".to_vec(), suffix.clone()); - let encoded_e_username = Encode::encode(&e_to_sign.to_vec()); - let e_signature = MultiSignature::Sr25519( - sr25519_sign(1.into(), &e_public, &encoded_e_username).unwrap(), - ); + let e_signature = + MultiSignature::Sr25519(sr25519_sign(1.into(), &e_public, &e_to_sign[..]).unwrap()); assert_ok!(Identity::set_username_for( RuntimeOrigin::signed(authority.clone()), e_account.clone(), @@ -1633,13 +1627,13 @@ fn removing_dangling_usernames_should_work() { // set up username let (username, username_to_sign) = test_username_of(b"42".to_vec(), suffix.clone()); - let encoded_username = Encode::encode(&username_to_sign.to_vec()); // set up user and sign message let public = sr25519_generate(0.into(), None); let who_account: AccountIdOf = MultiSigner::Sr25519(public).into_account().into(); - let signature = - MultiSignature::Sr25519(sr25519_sign(0.into(), &public, &encoded_username).unwrap()); + let signature = MultiSignature::Sr25519( + sr25519_sign(0.into(), &public, &username_to_sign[..]).unwrap(), + ); // Set an identity for who. They need some balance though. Balances::make_free_balance_be(&who_account, 1000); @@ -1657,11 +1651,10 @@ fn removing_dangling_usernames_should_work() { // Now they set up a second username. let (username_two, username_two_to_sign) = test_username_of(b"43".to_vec(), suffix); - let encoded_username_two = Encode::encode(&username_two_to_sign.to_vec()); // set up user and sign message let signature_two = MultiSignature::Sr25519( - sr25519_sign(0.into(), &public, &encoded_username_two).unwrap(), + sr25519_sign(0.into(), &public, &username_two_to_sign[..]).unwrap(), ); assert_ok!(Identity::set_username_for(