diff --git a/Cargo.lock b/Cargo.lock index 8c7737becbe0..412616cfc881 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -9924,7 +9924,7 @@ dependencies = [ [[package]] name = "pallet-identity" -version = "29.0.1" +version = "29.0.2" dependencies = [ "enumflags2", "frame-benchmarking", diff --git a/cumulus/parachains/integration-tests/emulated/tests/people/people-rococo/Cargo.toml b/cumulus/parachains/integration-tests/emulated/tests/people/people-rococo/Cargo.toml index 118c7a3fc7e2..2121cd0f7b25 100644 --- a/cumulus/parachains/integration-tests/emulated/tests/people/people-rococo/Cargo.toml +++ b/cumulus/parachains/integration-tests/emulated/tests/people/people-rococo/Cargo.toml @@ -15,7 +15,7 @@ sp-runtime = { path = "../../../../../../../substrate/primitives/runtime", defau frame-support = { path = "../../../../../../../substrate/frame/support", default-features = false, version = "29.0.2" } pallet-balances = { path = "../../../../../../../substrate/frame/balances", default-features = false, version = "29.0.2" } pallet-message-queue = { path = "../../../../../../../substrate/frame/message-queue", default-features = false, version = "32.0.0" } -pallet-identity = { path = "../../../../../../../substrate/frame/identity", default-features = false, version = "29.0.1" } +pallet-identity = { path = "../../../../../../../substrate/frame/identity", default-features = false, version = "29.0.2" } # Polkadot xcm = { package = "staging-xcm", path = "../../../../../../../polkadot/xcm", default-features = false, version = "8.0.1" } diff --git a/cumulus/parachains/integration-tests/emulated/tests/people/people-westend/Cargo.toml b/cumulus/parachains/integration-tests/emulated/tests/people/people-westend/Cargo.toml index 5b3ee40d26aa..04b06e9b76a9 100644 --- a/cumulus/parachains/integration-tests/emulated/tests/people/people-westend/Cargo.toml +++ b/cumulus/parachains/integration-tests/emulated/tests/people/people-westend/Cargo.toml @@ -15,7 +15,7 @@ sp-runtime = { path = "../../../../../../../substrate/primitives/runtime", defau frame-support = { path = "../../../../../../../substrate/frame/support", default-features = false, version = "29.0.2" } pallet-balances = { path = "../../../../../../../substrate/frame/balances", default-features = false, version = "29.0.2" } pallet-message-queue = { path = "../../../../../../../substrate/frame/message-queue", default-features = false, version = "32.0.0" } -pallet-identity = { path = "../../../../../../../substrate/frame/identity", default-features = false, version = "29.0.1" } +pallet-identity = { path = "../../../../../../../substrate/frame/identity", default-features = false, version = "29.0.2" } # Polkadot xcm = { package = "staging-xcm", path = "../../../../../../../polkadot/xcm", default-features = false, version = "8.0.1" } diff --git a/cumulus/parachains/runtimes/people/people-rococo/Cargo.toml b/cumulus/parachains/runtimes/people/people-rococo/Cargo.toml index e27672f280c3..e0170981a857 100644 --- a/cumulus/parachains/runtimes/people/people-rococo/Cargo.toml +++ b/cumulus/parachains/runtimes/people/people-rococo/Cargo.toml @@ -28,7 +28,7 @@ frame-try-runtime = { path = "../../../../../substrate/frame/try-runtime", defau pallet-aura = { path = "../../../../../substrate/frame/aura", default-features = false, version = "28.0.0" } pallet-authorship = { path = "../../../../../substrate/frame/authorship", default-features = false, version = "29.0.0" } pallet-balances = { path = "../../../../../substrate/frame/balances", default-features = false, version = "29.0.2" } -pallet-identity = { path = "../../../../../substrate/frame/identity", default-features = false, version = "29.0.1" } +pallet-identity = { path = "../../../../../substrate/frame/identity", default-features = false, version = "29.0.2" } pallet-message-queue = { path = "../../../../../substrate/frame/message-queue", default-features = false, version = "32.0.0" } pallet-multisig = { path = "../../../../../substrate/frame/multisig", default-features = false, version = "29.0.0" } pallet-session = { path = "../../../../../substrate/frame/session", default-features = false, version = "29.0.0" } diff --git a/cumulus/parachains/runtimes/people/people-westend/Cargo.toml b/cumulus/parachains/runtimes/people/people-westend/Cargo.toml index ae172427c2e5..ad7df0b09441 100644 --- a/cumulus/parachains/runtimes/people/people-westend/Cargo.toml +++ b/cumulus/parachains/runtimes/people/people-westend/Cargo.toml @@ -28,7 +28,7 @@ frame-try-runtime = { path = "../../../../../substrate/frame/try-runtime", defau pallet-aura = { path = "../../../../../substrate/frame/aura", default-features = false, version = "28.0.0" } pallet-authorship = { path = "../../../../../substrate/frame/authorship", default-features = false, version = "29.0.0" } pallet-balances = { path = "../../../../../substrate/frame/balances", default-features = false, version = "29.0.2" } -pallet-identity = { path = "../../../../../substrate/frame/identity", default-features = false, version = "29.0.1" } +pallet-identity = { path = "../../../../../substrate/frame/identity", default-features = false, version = "29.0.2" } pallet-message-queue = { path = "../../../../../substrate/frame/message-queue", default-features = false, version = "32.0.0" } pallet-multisig = { path = "../../../../../substrate/frame/multisig", default-features = false, version = "29.0.0" } pallet-session = { path = "../../../../../substrate/frame/session", default-features = false, version = "29.0.0" } diff --git a/polkadot/runtime/common/Cargo.toml b/polkadot/runtime/common/Cargo.toml index ead9c9e189f7..23c732d3c24c 100644 --- a/polkadot/runtime/common/Cargo.toml +++ b/polkadot/runtime/common/Cargo.toml @@ -34,7 +34,7 @@ pallet-authorship = { path = "../../../substrate/frame/authorship", default-feat pallet-balances = { path = "../../../substrate/frame/balances", default-features = false, version = "29.0.2" } pallet-broker = { path = "../../../substrate/frame/broker", default-features = false, version = "0.7.2" } pallet-fast-unstake = { path = "../../../substrate/frame/fast-unstake", default-features = false, version = "28.0.0" } -pallet-identity = { path = "../../../substrate/frame/identity", default-features = false, version = "29.0.1" } +pallet-identity = { path = "../../../substrate/frame/identity", default-features = false, version = "29.0.2" } pallet-session = { path = "../../../substrate/frame/session", default-features = false, version = "29.0.0" } frame-support = { path = "../../../substrate/frame/support", default-features = false, version = "29.0.2" } pallet-staking = { path = "../../../substrate/frame/staking", default-features = false, version = "29.0.3" } diff --git a/polkadot/runtime/rococo/Cargo.toml b/polkadot/runtime/rococo/Cargo.toml index 5016539f45c2..58a699317d0e 100644 --- a/polkadot/runtime/rococo/Cargo.toml +++ b/polkadot/runtime/rococo/Cargo.toml @@ -59,7 +59,7 @@ pallet-elections-phragmen = { path = "../../../substrate/frame/elections-phragme pallet-asset-rate = { path = "../../../substrate/frame/asset-rate", default-features = false, version = "8.0.0" } frame-executive = { path = "../../../substrate/frame/executive", default-features = false, version = "29.0.0" } pallet-grandpa = { path = "../../../substrate/frame/grandpa", default-features = false, version = "29.0.0" } -pallet-identity = { path = "../../../substrate/frame/identity", default-features = false, version = "29.0.1" } +pallet-identity = { path = "../../../substrate/frame/identity", default-features = false, version = "29.0.2" } pallet-im-online = { path = "../../../substrate/frame/im-online", default-features = false, version = "28.0.0" } pallet-indices = { path = "../../../substrate/frame/indices", default-features = false, version = "29.0.0" } pallet-membership = { path = "../../../substrate/frame/membership", default-features = false, version = "29.0.0" } diff --git a/polkadot/runtime/westend/Cargo.toml b/polkadot/runtime/westend/Cargo.toml index 1a8346aac5d3..f84453863270 100644 --- a/polkadot/runtime/westend/Cargo.toml +++ b/polkadot/runtime/westend/Cargo.toml @@ -64,7 +64,7 @@ pallet-elections-phragmen = { package = "pallet-elections-phragmen", path = "../ pallet-election-provider-multi-phase = { path = "../../../substrate/frame/election-provider-multi-phase", default-features = false, version = "28.0.0" } pallet-fast-unstake = { path = "../../../substrate/frame/fast-unstake", default-features = false, version = "28.0.0" } pallet-grandpa = { path = "../../../substrate/frame/grandpa", default-features = false, version = "29.0.0" } -pallet-identity = { path = "../../../substrate/frame/identity", default-features = false, version = "29.0.1" } +pallet-identity = { path = "../../../substrate/frame/identity", default-features = false, version = "29.0.2" } pallet-im-online = { path = "../../../substrate/frame/im-online", default-features = false, version = "28.0.0" } pallet-indices = { path = "../../../substrate/frame/indices", default-features = false, version = "29.0.0" } pallet-membership = { path = "../../../substrate/frame/membership", default-features = false, version = "29.0.0" } diff --git a/prdoc/pr_4646.prdoc b/prdoc/pr_4646.prdoc new file mode 100644 index 000000000000..0252deb57bdd --- /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/bin/node/runtime/Cargo.toml b/substrate/bin/node/runtime/Cargo.toml index 24edee0cfbf3..beffc8d06677 100644 --- a/substrate/bin/node/runtime/Cargo.toml +++ b/substrate/bin/node/runtime/Cargo.toml @@ -93,7 +93,7 @@ pallet-nis = { path = "../../../frame/nis", default-features = false, version = pallet-grandpa = { path = "../../../frame/grandpa", default-features = false, version = "29.0.0" } pallet-im-online = { path = "../../../frame/im-online", default-features = false, version = "28.0.0" } pallet-indices = { path = "../../../frame/indices", default-features = false, version = "29.0.0" } -pallet-identity = { path = "../../../frame/identity", default-features = false, version = "29.0.1" } +pallet-identity = { path = "../../../frame/identity", default-features = false, version = "29.0.2" } pallet-lottery = { path = "../../../frame/lottery", default-features = false, version = "29.0.0" } pallet-membership = { path = "../../../frame/membership", default-features = false, version = "29.0.0" } pallet-message-queue = { path = "../../../frame/message-queue", default-features = false, version = "32.0.0" } diff --git a/substrate/frame/alliance/Cargo.toml b/substrate/frame/alliance/Cargo.toml index 8539e728fb29..884048af621c 100644 --- a/substrate/frame/alliance/Cargo.toml +++ b/substrate/frame/alliance/Cargo.toml @@ -32,7 +32,7 @@ frame-benchmarking = { path = "../benchmarking", default-features = false, optio frame-support = { path = "../support", default-features = false, version = "29.0.2" } frame-system = { path = "../system", default-features = false, version = "29.0.0" } -pallet-identity = { path = "../identity", default-features = false, version = "29.0.1" } +pallet-identity = { path = "../identity", default-features = false, version = "29.0.2" } pallet-collective = { path = "../collective", default-features = false, optional = true, version = "29.0.0" } [dev-dependencies] diff --git a/substrate/frame/identity/Cargo.toml b/substrate/frame/identity/Cargo.toml index 7f372f95f167..9ab287d8ca97 100644 --- a/substrate/frame/identity/Cargo.toml +++ b/substrate/frame/identity/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "pallet-identity" -version = "29.0.1" +version = "29.0.2" 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 fe2fb0b04893..44bc4baaebac 100644 --- a/substrate/frame/identity/src/benchmarking.rs +++ b/substrate/frame/identity/src/benchmarking.rs @@ -22,10 +22,7 @@ use super::*; use crate::Pallet as Identity; -use codec::Encode; -use frame_benchmarking::{ - account, impl_benchmark_test_suite, v2::*, whitelisted_caller, BenchmarkError, -}; +use frame_benchmarking::{account, v2::*, whitelisted_caller, BenchmarkError}; use frame_support::{ assert_ok, ensure, traits::{EnsureOrigin, Get, OnFinalize, OnInitialize}, @@ -625,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 746205aba395..cb2175f2421b 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 d5f50e6667d3..b6a7e64a8341 100644 --- a/substrate/frame/identity/src/tests.rs +++ b/substrate/frame/identity/src/tests.rs @@ -1061,13 +1061,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), @@ -1112,13 +1112,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); @@ -1175,13 +1175,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 )); @@ -1193,7 +1193,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()); @@ -1201,7 +1201,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 )); @@ -1420,9 +1420,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()), @@ -1446,9 +1445,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), @@ -1529,10 +1527,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(), @@ -1544,10 +1540,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(), @@ -1652,13 +1646,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); @@ -1676,11 +1670,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(