From 468940ffcc385d5bccc4fa39ce73b5ee4af945a6 Mon Sep 17 00:00:00 2001 From: Rohit Sarpotdar Date: Tue, 24 Feb 2026 22:38:58 +0530 Subject: [PATCH 1/6] added ValidTransaction tag for check_signed --- pallets/transaction-storage/src/lib.rs | 23 ++++++++++------ pallets/transaction-storage/src/tests.rs | 35 ++++++++++++++++++++++++ 2 files changed, 50 insertions(+), 8 deletions(-) diff --git a/pallets/transaction-storage/src/lib.rs b/pallets/transaction-storage/src/lib.rs index 118f8cf70..e52ac39bb 100644 --- a/pallets/transaction-storage/src/lib.rs +++ b/pallets/transaction-storage/src/lib.rs @@ -1136,23 +1136,30 @@ pub mod pallet { // This allows anyone to store/renew pre-authorized content without consuming their // own account authorization. let consume = context.consume_authorization(); - Self::check_authorization( + let used_preimage_auth = Self::check_authorization( AuthorizationScope::Preimage(content_hash), size as u32, consume, ) - .or_else(|_| { + .is_ok(); + + if !used_preimage_auth { Self::check_authorization( AuthorizationScope::Account(who.clone()), size as u32, consume, - ) - })?; + )?; + } - Ok(context.want_valid_transaction().then(|| ValidTransaction { - priority: T::StoreRenewPriority::get(), - longevity: T::StoreRenewLongevity::get(), - ..Default::default() + Ok(context.want_valid_transaction().then(|| { + let builder = ValidTransaction::with_tag_prefix("TransactionStorageCheckedSigned") + .priority(T::StoreRenewPriority::get()) + .longevity(T::StoreRenewLongevity::get()); + if used_preimage_auth { + builder.and_provides(content_hash).into() + } else { + builder.and_provides((who, content_hash)).into() + } })) } diff --git a/pallets/transaction-storage/src/tests.rs b/pallets/transaction-storage/src/tests.rs index 789965d67..4b622f6fd 100644 --- a/pallets/transaction-storage/src/tests.rs +++ b/pallets/transaction-storage/src/tests.rs @@ -609,6 +609,41 @@ fn signed_renew_prefers_preimage_authorization() { }); } +#[test] +fn validate_signed_account_authorization_has_provides_tag() { + new_test_ext().execute_with(|| { + run_to_block(1, || None); + let who = 1u64; + assert_ok!(TransactionStorage::authorize_account(RuntimeOrigin::root(), who, 1, 2000,)); + + let call = Call::store { data: vec![0u8; 2000] }; + + // validate_signed still doesn't consume authorization (correct behaviour). + for _ in 0..10 { + assert_ok!(TransactionStorage::validate_signed(&who, &call)); + } + assert_eq!( + TransactionStorage::account_authorization_extent(who), + AuthorizationExtent { transactions: 1, bytes: 2000 }, + ); + + let vt = TransactionStorage::validate_signed(&who, &call).unwrap(); + assert!(!vt.provides.is_empty(), "validate_signed must emit a `provides` tag"); + + // Two calls with the same signer + content produce identical tags, confirming + // that the mempool will deduplicate them. + let vt2 = TransactionStorage::validate_signed(&who, &call).unwrap(); + assert_eq!(vt.provides, vt2.provides); + + // pre_dispatch still enforces the authorization: only the first succeeds. + assert_ok!(TransactionStorage::pre_dispatch_signed(&who, &call)); + assert_noop!( + TransactionStorage::pre_dispatch_signed(&who, &call), + InvalidTransaction::Payment, + ); + }); +} + // ---- Migration tests ---- /// Write old-format `OldTransactionInfo` entries as raw bytes into the `Transactions` From 772f107d6b282c02e0041f87444201f62cb93754 Mon Sep 17 00:00:00 2001 From: Branislav Kontur Date: Thu, 26 Feb 2026 11:22:18 +0100 Subject: [PATCH 2/6] Apply suggestion from @bkontur --- pallets/transaction-storage/src/tests.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pallets/transaction-storage/src/tests.rs b/pallets/transaction-storage/src/tests.rs index a331fced3..077d012c2 100644 --- a/pallets/transaction-storage/src/tests.rs +++ b/pallets/transaction-storage/src/tests.rs @@ -735,7 +735,7 @@ fn validate_signed_account_authorization_has_provides_tag() { let call = Call::store { data: vec![0u8; 2000] }; // validate_signed still doesn't consume authorization (correct behaviour). - for _ in 0..10 { + for _ in 0..2 { assert_ok!(TransactionStorage::validate_signed(&who, &call)); } assert_eq!( From 7f3446531add97069fc4696dd4b59b6a86a8c3a5 Mon Sep 17 00:00:00 2001 From: Branislav Kontur Date: Thu, 26 Feb 2026 11:31:25 +0100 Subject: [PATCH 3/6] Fix call to renamed parameter in check_store_renew_unsigned The `hash` parameter was renamed to `content_hash` but the call site was not updated, causing a compilation error. --- pallets/transaction-storage/src/lib.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/pallets/transaction-storage/src/lib.rs b/pallets/transaction-storage/src/lib.rs index 75c060f5c..8b9f489a7 100644 --- a/pallets/transaction-storage/src/lib.rs +++ b/pallets/transaction-storage/src/lib.rs @@ -1069,7 +1069,7 @@ pub mod pallet { fn check_store_renew_unsigned( size: usize, - hash: impl FnOnce() -> ContentHash, + content_hash: impl FnOnce() -> ContentHash, context: CheckContext, ) -> Result, TransactionValidityError> { if !Self::data_size_ok(size) { @@ -1080,17 +1080,17 @@ pub mod pallet { return Err(InvalidTransaction::ExhaustsResources.into()); } - let hash = hash(); + let content_hash = content_hash(); Self::check_authorization( - AuthorizationScope::Preimage(hash), + AuthorizationScope::Preimage(content_hash), size as u32, context.consume_authorization(), )?; Ok(context.want_valid_transaction().then(|| { ValidTransaction::with_tag_prefix("TransactionStorageStoreRenew") - .and_provides(hash) + .and_provides(content_hash) .priority(T::StoreRenewPriority::get()) .longevity(T::StoreRenewLongevity::get()) .into() From 165166d9b29b9e4ac0e48d7a98eab13969eaabb1 Mon Sep 17 00:00:00 2001 From: Branislav Kontur Date: Thu, 26 Feb 2026 11:42:46 +0100 Subject: [PATCH 4/6] Unify preimage ValidTransaction tag between signed and unsigned paths Extract preimage_store_renew_valid_transaction helper so both check_store_renew_unsigned and check_signed use the same tag prefix and provides, ensuring the tx pool deduplicates across signed and unsigned submissions of the same pre-authorized content. --- pallets/transaction-storage/src/lib.rs | 29 +++++++++++++++----------- 1 file changed, 17 insertions(+), 12 deletions(-) diff --git a/pallets/transaction-storage/src/lib.rs b/pallets/transaction-storage/src/lib.rs index 8b9f489a7..81c18be65 100644 --- a/pallets/transaction-storage/src/lib.rs +++ b/pallets/transaction-storage/src/lib.rs @@ -1067,6 +1067,14 @@ pub mod pallet { } } + fn preimage_store_renew_valid_transaction(content_hash: ContentHash) -> ValidTransaction { + ValidTransaction::with_tag_prefix("TransactionStorageStoreRenew") + .and_provides(content_hash) + .priority(T::StoreRenewPriority::get()) + .longevity(T::StoreRenewLongevity::get()) + .into() + } + fn check_store_renew_unsigned( size: usize, content_hash: impl FnOnce() -> ContentHash, @@ -1088,13 +1096,9 @@ pub mod pallet { context.consume_authorization(), )?; - Ok(context.want_valid_transaction().then(|| { - ValidTransaction::with_tag_prefix("TransactionStorageStoreRenew") - .and_provides(content_hash) - .priority(T::StoreRenewPriority::get()) - .longevity(T::StoreRenewLongevity::get()) - .into() - })) + Ok(context + .want_valid_transaction() + .then(|| Self::preimage_store_renew_valid_transaction(content_hash))) } fn check_unsigned( @@ -1208,13 +1212,14 @@ pub mod pallet { } Ok(context.want_valid_transaction().then(|| { - let builder = ValidTransaction::with_tag_prefix("TransactionStorageCheckedSigned") - .priority(T::StoreRenewPriority::get()) - .longevity(T::StoreRenewLongevity::get()); if used_preimage_auth { - builder.and_provides(content_hash).into() + Self::preimage_store_renew_valid_transaction(content_hash) } else { - builder.and_provides((who, content_hash)).into() + ValidTransaction::with_tag_prefix("TransactionStorageCheckedSigned") + .and_provides((who, content_hash)) + .priority(T::StoreRenewPriority::get()) + .longevity(T::StoreRenewLongevity::get()) + .into() } })) } From badb0694ac79bc5ec99a0032df5185d9a85ebe91 Mon Sep 17 00:00:00 2001 From: Branislav Kontur Date: Thu, 26 Feb 2026 11:48:02 +0100 Subject: [PATCH 5/6] Test that signed preimage tags match unsigned preimage tags Extend validate_signed_account_authorization_has_provides_tag to verify that the preimage-authorized signed path produces the same ValidTransaction provides tag as the unsigned path, ensuring the transaction pool deduplicates across both submission types. --- pallets/transaction-storage/src/tests.rs | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/pallets/transaction-storage/src/tests.rs b/pallets/transaction-storage/src/tests.rs index 077d012c2..69bffec77 100644 --- a/pallets/transaction-storage/src/tests.rs +++ b/pallets/transaction-storage/src/tests.rs @@ -757,6 +757,29 @@ fn validate_signed_account_authorization_has_provides_tag() { TransactionStorage::pre_dispatch_signed(&who, &call), InvalidTransaction::Payment, ); + + // Now test the preimage-authorized path: signed preimage tags must match unsigned + // preimage tags so the pool deduplicates across both submission types. + let data = vec![0u8; 2000]; + let content_hash = blake2_256(&data); + assert_ok!(TransactionStorage::authorize_preimage( + RuntimeOrigin::root(), + content_hash, + 2000, + )); + // Re-authorize account so validate_signed can fall through if needed. + assert_ok!(TransactionStorage::authorize_account(RuntimeOrigin::root(), who, 1, 2000)); + + let signed_vt = TransactionStorage::validate_signed(&who, &call).unwrap(); + let unsigned_vt = ::validate_unsigned( + TransactionSource::External, + &call, + ) + .unwrap(); + assert_eq!( + signed_vt.provides, unsigned_vt.provides, + "signed preimage path must produce the same tag as unsigned preimage path" + ); }); } From 4a4d9010072600eb50643df6bbded9d6d8e6417a Mon Sep 17 00:00:00 2001 From: Branislav Kontur Date: Thu, 26 Feb 2026 11:53:54 +0100 Subject: [PATCH 6/6] Test cross-signer dedup for preimage-authorized content Verify that different signers submitting the same preimage-authorized content produce identical provides tags, confirming dedup is content-based and not signer-based. --- pallets/transaction-storage/src/tests.rs | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/pallets/transaction-storage/src/tests.rs b/pallets/transaction-storage/src/tests.rs index 69bffec77..7be4e5b32 100644 --- a/pallets/transaction-storage/src/tests.rs +++ b/pallets/transaction-storage/src/tests.rs @@ -780,6 +780,15 @@ fn validate_signed_account_authorization_has_provides_tag() { signed_vt.provides, unsigned_vt.provides, "signed preimage path must produce the same tag as unsigned preimage path" ); + + // A different signer submitting the same pre-authorized content must get the same + // tag, proving dedup is content-based, not signer-based. + let other_who = 2u64; + let other_vt = TransactionStorage::validate_signed(&other_who, &call).unwrap(); + assert_eq!( + signed_vt.provides, other_vt.provides, + "different signers with same preimage-authorized content must share the same tag" + ); }); }