From d2f3e69701b22161c7d3976da604433bd1618152 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Fri, 6 Mar 2020 10:27:21 +0100 Subject: [PATCH] Revert "Build block without checking signatures (#4916)" This reverts commit dc92587bea4032e0a0fc96785bfd9aa17c95459e. --- bin/node-template/runtime/src/lib.rs | 4 - bin/node/runtime/src/lib.rs | 6 +- .../basic-authorship/src/basic_authorship.rs | 4 +- client/block-builder/src/lib.rs | 38 +++---- client/rpc/src/state/tests.rs | 2 +- client/service/src/lib.rs | 3 +- frame/executive/src/lib.rs | 86 +++------------ frame/im-online/src/tests.rs | 4 +- primitives/block-builder/src/lib.rs | 6 +- primitives/runtime/src/generic/mod.rs | 9 -- .../src/generic/unchecked_extrinsic.rs | 34 ++---- primitives/runtime/src/testing.rs | 103 +++--------------- primitives/runtime/src/traits.rs | 10 +- test-utils/runtime/src/lib.rs | 11 +- test-utils/runtime/src/system.rs | 3 +- 15 files changed, 76 insertions(+), 247 deletions(-) diff --git a/bin/node-template/runtime/src/lib.rs b/bin/node-template/runtime/src/lib.rs index 2bc4c2745007b..e5feffc32d62c 100644 --- a/bin/node-template/runtime/src/lib.rs +++ b/bin/node-template/runtime/src/lib.rs @@ -297,10 +297,6 @@ impl_runtime_apis! { Executive::apply_extrinsic(extrinsic) } - fn apply_trusted_extrinsic(extrinsic: ::Extrinsic) -> ApplyExtrinsicResult { - Executive::apply_trusted_extrinsic(extrinsic) - } - fn finalize_block() -> ::Header { Executive::finalize_block() } diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index ae0de952e49fb..0f0c7958c3f67 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -83,7 +83,7 @@ pub const VERSION: RuntimeVersion = RuntimeVersion { // and set impl_version to 0. If only runtime // implementation changes and behavior does not, then leave spec_version as // is and increment impl_version. - spec_version: 230, + spec_version: 231, impl_version: 0, apis: RUNTIME_API_VERSIONS, }; @@ -705,10 +705,6 @@ impl_runtime_apis! { Executive::apply_extrinsic(extrinsic) } - fn apply_trusted_extrinsic(extrinsic: ::Extrinsic) -> ApplyExtrinsicResult { - Executive::apply_trusted_extrinsic(extrinsic) - } - fn finalize_block() -> ::Header { Executive::finalize_block() } diff --git a/client/basic-authorship/src/basic_authorship.rs b/client/basic-authorship/src/basic_authorship.rs index 41216c2b54421..9dfe1c33be0fc 100644 --- a/client/basic-authorship/src/basic_authorship.rs +++ b/client/basic-authorship/src/basic_authorship.rs @@ -203,7 +203,7 @@ impl ProposerInner inherent_data )? { - block_builder.push_trusted(extrinsic)?; + block_builder.push(extrinsic)?; } // proceed with transactions @@ -226,7 +226,7 @@ impl ProposerInner let pending_tx_data = pending_tx.data().clone(); let pending_tx_hash = pending_tx.hash().clone(); trace!("[{:?}] Pushing to the block.", pending_tx_hash); - match sc_block_builder::BlockBuilder::push_trusted(&mut block_builder, pending_tx_data) { + match sc_block_builder::BlockBuilder::push(&mut block_builder, pending_tx_data) { Ok(()) => { debug!("[{:?}] Pushed to the block.", pending_tx_hash); } diff --git a/client/block-builder/src/lib.rs b/client/block-builder/src/lib.rs index 2666fd9cd7139..24d6462ac0ebc 100644 --- a/client/block-builder/src/lib.rs +++ b/client/block-builder/src/lib.rs @@ -145,17 +145,6 @@ where /// /// This will ensure the extrinsic can be validly executed (by executing it). pub fn push(&mut self, xt: ::Extrinsic) -> Result<(), ApiErrorFor> { - self.push_internal(xt, false) - } - - /// Push onto the block's list of extrinsics. - /// - /// This will treat incoming extrinsic `xt` as trusted and skip signature check (for signed transactions). - pub fn push_trusted(&mut self, xt: ::Extrinsic) -> Result<(), ApiErrorFor> { - self.push_internal(xt, true) - } - - fn push_internal(&mut self, xt: ::Extrinsic, skip_signature: bool) -> Result<(), ApiErrorFor> { let block_id = &self.block_id; let extrinsics = &mut self.extrinsics; @@ -172,19 +161,26 @@ where block_id, ExecutionContext::BlockConstruction, xt.clone(), - )? - } else { - api.apply_extrinsic_with_context( + )? { + Ok(_) => { + extrinsics.push(xt); + Ok(()) + } + Err(e) => Err(ApplyExtrinsicFailed::from(e).into()), + } + }) + } else { + self.api.map_api_result(|api| { + match api.apply_extrinsic_with_context( block_id, ExecutionContext::BlockConstruction, xt.clone(), - )? - }; - - match apply_result { - Ok(_) => { - extrinsics.push(xt); - Ok(()) + )? { + Ok(_) => { + extrinsics.push(xt); + Ok(()) + } + Err(tx_validity) => Err(ApplyExtrinsicFailed::Validity(tx_validity).into()), } Err(tx_validity) => Err(ApplyExtrinsicFailed::Validity(tx_validity).into()), } diff --git a/client/rpc/src/state/tests.rs b/client/rpc/src/state/tests.rs index 6508d46ddde65..a0ab11e977204 100644 --- a/client/rpc/src/state/tests.rs +++ b/client/rpc/src/state/tests.rs @@ -402,7 +402,7 @@ fn should_return_runtime_version() { let result = "{\"specName\":\"test\",\"implName\":\"parity-test\",\"authoringVersion\":1,\ \"specVersion\":1,\"implVersion\":2,\"apis\":[[\"0xdf6acb689907609b\",2],\ - [\"0x37e397fc7c91f5e4\",1],[\"0xd2bc9897eed08f15\",1],[\"0x40fe3ad401f8959a\",5],\ + [\"0x37e397fc7c91f5e4\",1],[\"0xd2bc9897eed08f15\",1],[\"0x40fe3ad401f8959a\",4],\ [\"0xc6e9a76309f39b09\",1],[\"0xdd718d5cc53262d4\",1],[\"0xcbca25e39f142387\",1],\ [\"0xf78b278be53f454c\",2],[\"0xab3c0572291feb8b\",1],[\"0xbc9d89904f5b923f\",1]]}"; diff --git a/client/service/src/lib.rs b/client/service/src/lib.rs index db56c141db0b5..bc5273e700097 100644 --- a/client/service/src/lib.rs +++ b/client/service/src/lib.rs @@ -656,7 +656,6 @@ mod tests { use futures::executor::block_on; use sp_consensus::SelectChain; use sp_runtime::traits::BlindCheckable; - use sp_runtime::generic::CheckSignature; use substrate_test_runtime_client::{prelude::*, runtime::{Extrinsic, Transfer}}; use sc_transaction_pool::{BasicPool, FullChainApi}; @@ -685,7 +684,7 @@ mod tests { // then assert_eq!(transactions.len(), 1); - assert!(transactions[0].1.clone().check(CheckSignature::Yes).is_ok()); + assert!(transactions[0].1.clone().check().is_ok()); // this should not panic let _ = transactions[0].1.transfer(); } diff --git a/frame/executive/src/lib.rs b/frame/executive/src/lib.rs index 7bf39989ecc72..d225b87e94e4e 100644 --- a/frame/executive/src/lib.rs +++ b/frame/executive/src/lib.rs @@ -80,15 +80,13 @@ use frame_support::{ weights::{GetDispatchInfo, WeighBlock, DispatchInfo} }; use sp_runtime::{ - generic::Digest, - ApplyExtrinsicResult, + generic::Digest, ApplyExtrinsicResult, traits::{ self, Header, Zero, One, Checkable, Applyable, CheckEqual, OnFinalize, OnInitialize, NumberFor, Block as BlockT, OffchainWorker, Dispatchable, Saturating, OnRuntimeUpgrade, }, transaction_validity::TransactionValidity, }; -use sp_runtime::generic::CheckSignature; use sp_runtime::traits::ValidateUnsigned; use codec::{Codec, Encode}; use frame_system::{extrinsics_root, DigestOf}; @@ -263,22 +261,13 @@ where pub fn apply_extrinsic(uxt: Block::Extrinsic) -> ApplyExtrinsicResult { let encoded = uxt.encode(); let encoded_len = encoded.len(); - Self::apply_extrinsic_with_len(uxt, encoded_len, Some(encoded), CheckSignature::Yes) - } - - /// Apply extrinsic outside of the block execution function. - /// - /// Same as `apply_extrinsic`, but skips signature checks. - pub fn apply_trusted_extrinsic(uxt: Block::Extrinsic) -> ApplyExtrinsicResult { - let encoded = uxt.encode(); - let encoded_len = encoded.len(); - Self::apply_extrinsic_with_len(uxt, encoded_len, Some(encoded), CheckSignature::No) + Self::apply_extrinsic_with_len(uxt, encoded_len, Some(encoded)) } /// Apply an extrinsic inside the block execution function. fn apply_extrinsic_no_note(uxt: Block::Extrinsic) { let l = uxt.encode().len(); - match Self::apply_extrinsic_with_len(uxt, l, None, CheckSignature::Yes) { + match Self::apply_extrinsic_with_len(uxt, l, None) { Ok(_) => (), Err(e) => { let err: &'static str = e.into(); panic!(err) }, } @@ -289,13 +278,9 @@ where uxt: Block::Extrinsic, encoded_len: usize, to_note: Option>, - check_signature: CheckSignature, ) -> ApplyExtrinsicResult { // Verify that the signature is good. - let xt = uxt.check( - check_signature, - &Default::default(), - )?; + let xt = uxt.check(&Default::default())?; // We don't need to make sure to `note_extrinsic` only after we know it's going to be // executed to prevent it from leaking in storage since at this point, it will either @@ -343,7 +328,7 @@ where /// Changes made to storage should be discarded. pub fn validate_transaction(uxt: Block::Extrinsic) -> TransactionValidity { let encoded_len = uxt.using_encoded(|d| d.len()); - let xt = uxt.check(CheckSignature::Yes, &Default::default())?; + let xt = uxt.check(&Default::default())?; let dispatch_info = xt.get_dispatch_info(); xt.validate::(dispatch_info, encoded_len) @@ -541,8 +526,8 @@ mod tests { ) } - fn sign_extra(who: u64, nonce: u64, fee: u64) -> (u64, SignedExtra) { - (who, extra(nonce, fee)) + fn sign_extra(who: u64, nonce: u64, fee: u64) -> Option<(u64, SignedExtra)> { + Some((who, extra(nonce, fee))) } #[test] @@ -551,7 +536,7 @@ mod tests { pallet_balances::GenesisConfig:: { balances: vec![(1, 211)], }.assimilate_storage(&mut t).unwrap(); - let xt = TestXt::new_signed(sign_extra(1, 0, 0), Call::Balances(BalancesCall::transfer(2, 69))); + let xt = sp_runtime::testing::TestXt(sign_extra(1, 0, 0), Call::Balances(BalancesCall::transfer(2, 69))); let weight = xt.get_dispatch_info().weight as u64; let mut t = sp_io::TestExternalities::new(t); t.execute_with(|| { @@ -631,7 +616,7 @@ mod tests { fn bad_extrinsic_not_inserted() { let mut t = new_test_ext(1); // bad nonce check! - let xt = TestXt::new_signed(sign_extra(1, 30, 0), Call::Balances(BalancesCall::transfer(33, 69))); + let xt = sp_runtime::testing::TestXt(sign_extra(1, 30, 0), Call::Balances(BalancesCall::transfer(33, 69))); t.execute_with(|| { Executive::initialize_block(&Header::new( 1, @@ -649,7 +634,7 @@ mod tests { fn block_weight_limit_enforced() { let mut t = new_test_ext(10000); // given: TestXt uses the encoded len as fixed Len: - let xt = TestXt::new_signed(sign_extra(1, 0, 0), Call::Balances(BalancesCall::transfer(33, 0))); + let xt = sp_runtime::testing::TestXt(sign_extra(1, 0, 0), Call::Balances(BalancesCall::transfer(33, 0))); let encoded = xt.encode(); let encoded_len = encoded.len() as Weight; let limit = AvailableBlockRatio::get() * MaximumBlockWeight::get() - 175; @@ -666,7 +651,7 @@ mod tests { assert_eq!(>::all_extrinsics_weight(), 175); for nonce in 0..=num_to_exhaust_block { - let xt = TestXt::new_signed( + let xt = sp_runtime::testing::TestXt( sign_extra(1, nonce.into(), 0), Call::Balances(BalancesCall::transfer(33, 0)), ); let res = Executive::apply_extrinsic(xt); @@ -686,9 +671,9 @@ mod tests { #[test] fn block_weight_and_size_is_stored_per_tx() { - let xt = TestXt::new_signed(sign_extra(1, 0, 0), Call::Balances(BalancesCall::transfer(33, 0))); - let x1 = TestXt::new_signed(sign_extra(1, 1, 0), Call::Balances(BalancesCall::transfer(33, 0))); - let x2 = TestXt::new_signed(sign_extra(1, 2, 0), Call::Balances(BalancesCall::transfer(33, 0))); + let xt = sp_runtime::testing::TestXt(sign_extra(1, 0, 0), Call::Balances(BalancesCall::transfer(33, 0))); + let x1 = sp_runtime::testing::TestXt(sign_extra(1, 1, 0), Call::Balances(BalancesCall::transfer(33, 0))); + let x2 = sp_runtime::testing::TestXt(sign_extra(1, 2, 0), Call::Balances(BalancesCall::transfer(33, 0))); let len = xt.clone().encode().len() as u32; let mut t = new_test_ext(1); t.execute_with(|| { @@ -712,7 +697,7 @@ mod tests { #[test] fn validate_unsigned() { - let xt = TestXt::new_unsigned(Call::Balances(BalancesCall::set_balance(33, 69, 69))); + let xt = sp_runtime::testing::TestXt(None, Call::Balances(BalancesCall::set_balance(33, 69, 69))); let mut t = new_test_ext(1); t.execute_with(|| { @@ -721,45 +706,6 @@ mod tests { }); } - #[test] - fn unsigned_weight_is_noted_when_applied() { - let xt = TestXt::new_unsigned(Call::Balances(BalancesCall::set_balance(33, 69, 69))); - let len = xt.clone().encode().len() as u32; - let mut t = new_test_ext(1); - t.execute_with(|| { - assert_eq!(>::all_extrinsics_weight(), 0); - assert_eq!(>::all_extrinsics_len(), 0); - - // This is okay -- balances transfer will panic since it requires ensure_signed. - assert_eq!(Executive::apply_extrinsic(xt), Ok(Err(DispatchError::BadOrigin))); - - assert_eq!(>::all_extrinsics_weight(), len); - assert_eq!(>::all_extrinsics_len(), len); - }); - } - - #[test] - fn apply_trusted_skips_signature_check_but_not_others() { - let xt1 = TestXt::new_signed(sign_extra(1, 0, 0), Call::Balances(BalancesCall::transfer(33, 0))) - .badly_signed(); - - let mut t = new_test_ext(1); - - t.execute_with(|| { - assert_eq!(Executive::apply_trusted_extrinsic(xt1), Ok(Ok(()))); - }); - - let xt2 = TestXt::new_signed(sign_extra(1, 0, 0), Call::Balances(BalancesCall::transfer(33, 0))) - .invalid(TransactionValidityError::Invalid(InvalidTransaction::Call)); - - t.execute_with(|| { - assert_eq!( - Executive::apply_trusted_extrinsic(xt2), - Err(TransactionValidityError::Invalid(InvalidTransaction::Call)) - ); - }); - } - #[test] fn can_pay_for_tx_fee_on_full_lock() { let id: LockIdentifier = *b"0 "; @@ -772,7 +718,7 @@ mod tests { 110, lock, ); - let xt = TestXt::new_signed( + let xt = sp_runtime::testing::TestXt( sign_extra(1, 0, 0), Call::System(SystemCall::remark(vec![1u8])), ); diff --git a/frame/im-online/src/tests.rs b/frame/im-online/src/tests.rs index b43adca0fd485..de7215a38c891 100644 --- a/frame/im-online/src/tests.rs +++ b/frame/im-online/src/tests.rs @@ -219,7 +219,7 @@ fn should_generate_heartbeats() { // check stuff about the transaction. let ex: Extrinsic = Decode::decode(&mut &*transaction).unwrap(); - let heartbeat = match ex.call { + let heartbeat = match ex.1 { crate::mock::Call::ImOnline(crate::Call::heartbeat(h, _)) => h, e => panic!("Unexpected call: {:?}", e), }; @@ -329,7 +329,7 @@ fn should_not_send_a_report_if_already_online() { assert_eq!(pool_state.read().transactions.len(), 0); // check stuff about the transaction. let ex: Extrinsic = Decode::decode(&mut &*transaction).unwrap(); - let heartbeat = match ex.call { + let heartbeat = match ex.1 { crate::mock::Call::ImOnline(crate::Call::heartbeat(h, _)) => h, e => panic!("Unexpected call: {:?}", e), }; diff --git a/primitives/block-builder/src/lib.rs b/primitives/block-builder/src/lib.rs index 71c2a3ccb55f4..732c937c1a085 100644 --- a/primitives/block-builder/src/lib.rs +++ b/primitives/block-builder/src/lib.rs @@ -24,17 +24,13 @@ use sp_inherents::{InherentData, CheckInherentsResult}; sp_api::decl_runtime_apis! { /// The `BlockBuilder` api trait that provides the required functionality for building a block. - #[api_version(5)] + #[api_version(4)] pub trait BlockBuilder { /// Apply the given extrinsic. /// /// Returns an inclusion outcome which specifies if this extrinsic is included in /// this block or not. fn apply_extrinsic(extrinsic: ::Extrinsic) -> ApplyExtrinsicResult; - /// Apply the given extrinsic. - /// - /// Same as `apply_extrinsic`, but skips signature verification. - fn apply_trusted_extrinsic(extrinsic: ::Extrinsic) -> ApplyExtrinsicResult; /// Finish the current block. #[renamed("finalise_block", 3)] fn finalize_block() -> ::Header; diff --git a/primitives/runtime/src/generic/mod.rs b/primitives/runtime/src/generic/mod.rs index f6399fff1382a..5e9928ba1909a 100644 --- a/primitives/runtime/src/generic/mod.rs +++ b/primitives/runtime/src/generic/mod.rs @@ -39,15 +39,6 @@ pub use self::digest::{ use crate::codec::Encode; use sp_std::prelude::*; -/// Perform singature check. -#[derive(PartialEq, Eq, Clone, Copy)] -pub enum CheckSignature { - /// Perform. - Yes, - /// Don't perform. - No, -} - fn encode_with_vec_prefix)>(encoder: F) -> Vec { let size = ::sp_std::mem::size_of::(); let reserve = match size { diff --git a/primitives/runtime/src/generic/unchecked_extrinsic.rs b/primitives/runtime/src/generic/unchecked_extrinsic.rs index 0db60e32a6e5e..a516bc1f7fa99 100644 --- a/primitives/runtime/src/generic/unchecked_extrinsic.rs +++ b/primitives/runtime/src/generic/unchecked_extrinsic.rs @@ -24,8 +24,7 @@ use crate::{ self, Member, MaybeDisplay, SignedExtension, Checkable, Extrinsic, ExtrinsicMetadata, IdentifyAccount, }, - generic::{CheckSignature, CheckedExtrinsic}, - transaction_validity::{TransactionValidityError, InvalidTransaction}, + generic::CheckedExtrinsic, transaction_validity::{TransactionValidityError, InvalidTransaction}, }; const TRANSACTION_VERSION: u8 = 4; @@ -121,26 +120,18 @@ where { type Checked = CheckedExtrinsic; - fn check(self, check_signature: CheckSignature, lookup: &Lookup) -> Result { + fn check(self, lookup: &Lookup) -> Result { Ok(match self.signature { Some((signed, signature, extra)) => { let signed = lookup.lookup(signed)?; + let raw_payload = SignedPayload::new(self.function, extra)?; + if !raw_payload.using_encoded(|payload| { + signature.verify(payload, &signed) + }) { + return Err(InvalidTransaction::BadProof.into()) + } - let (function, extra) = if let CheckSignature::No = check_signature { - (self.function, extra) - } else { - let raw_payload = SignedPayload::new(self.function, extra)?; - - if !raw_payload.using_encoded(|payload| { - signature.verify(payload, &signed) - }) { - return Err(InvalidTransaction::BadProof.into()) - } - let (function, extra, _) = raw_payload.deconstruct(); - - (function, extra) - }; - + let (function, extra, _) = raw_payload.deconstruct(); CheckedExtrinsic { signed: Some((signed, extra)), function, @@ -331,7 +322,6 @@ mod tests { use sp_io::hashing::blake2_256; use crate::codec::{Encode, Decode}; use crate::traits::{SignedExtension, IdentifyAccount, IdentityLookup}; - use crate::generic::CheckSignature; use serde::{Serialize, Deserialize}; type TestContext = IdentityLookup; @@ -412,7 +402,7 @@ mod tests { fn unsigned_check_should_work() { let ux = Ex::new_unsigned(vec![0u8; 0]); assert!(!ux.is_signed().unwrap_or(false)); - assert!(>::check(ux, CheckSignature::Yes, &Default::default()).is_ok()); + assert!(>::check(ux, &Default::default()).is_ok()); } #[test] @@ -425,7 +415,7 @@ mod tests { ); assert!(ux.is_signed().unwrap_or(false)); assert_eq!( - >::check(ux, CheckSignature::Yes, &Default::default()), + >::check(ux, &Default::default()), Err(InvalidTransaction::BadProof.into()), ); } @@ -440,7 +430,7 @@ mod tests { ); assert!(ux.is_signed().unwrap_or(false)); assert_eq!( - >::check(ux, CheckSignature::Yes, &Default::default()), + >::check(ux, &Default::default()), Ok(CEx { signed: Some((TEST_ACCOUNT, TestExtra)), function: vec![0u8; 0] }), ); } diff --git a/primitives/runtime/src/testing.rs b/primitives/runtime/src/testing.rs index e840cdd100c2e..49908bbc0d55f 100644 --- a/primitives/runtime/src/testing.rs +++ b/primitives/runtime/src/testing.rs @@ -24,10 +24,11 @@ use crate::traits::{ SignedExtension, Dispatchable, }; use crate::traits::ValidateUnsigned; -use crate::{generic::{self, CheckSignature}, KeyTypeId, ApplyExtrinsicResult}; +use crate::{generic, KeyTypeId, ApplyExtrinsicResult}; pub use sp_core::{H256, sr25519}; use sp_core::{crypto::{CryptoType, Dummy, key_types, Public}, U256}; -use crate::transaction_validity::{TransactionValidity, TransactionValidityError, InvalidTransaction}; +use crate::transaction_validity::{TransactionValidity, TransactionValidityError}; + /// Authority Id #[derive(Default, PartialEq, Eq, Clone, Encode, Decode, Debug, Hash, Serialize, Deserialize, PartialOrd, Ord)] pub struct UintAuthorityId(pub u64); @@ -293,69 +294,12 @@ impl<'a, Xt> Deserialize<'a> for Block where Block: Decode { } } -/// Test validity. -#[derive(PartialEq, Eq, Clone, Encode, Decode)] -pub enum TestValidity { - /// Valid variant that will pass all checks. - Valid, - /// Variant with invalid signature. - /// - /// Will fail signature check. - SignatureInvalid(TransactionValidityError), - /// Variant with invalid logic. - /// - /// Will fail all checks. - OtherInvalid(TransactionValidityError), -} - -/// Test transaction. +/// Test transaction, tuple of (sender, call, signed_extra) +/// with index only used if sender is some. /// -/// Used to mock actual transaction. +/// If sender is some then the transaction is signed otherwise it is unsigned. #[derive(PartialEq, Eq, Clone, Encode, Decode)] -pub struct TestXt { - /// Signature with extra. - /// - /// if some, then the transaction is signed. Transaction is unsigned otherwise. - pub signature: Option<(u64, Extra)>, - /// Validity. - /// - /// Instantiate invalid variant and transaction will fail correpsonding checks. - pub validity: TestValidity, - /// Call. - pub call: Call, -} - -impl TestXt { - /// New signed test `TextXt`. - pub fn new_signed(signature: (u64, Extra), call: Call) -> Self { - TestXt { - signature: Some(signature), - validity: TestValidity::Valid, - call, - } - } - - /// New unsigned test `TextXt`. - pub fn new_unsigned(call: Call) -> Self { - TestXt { - signature: None, - validity: TestValidity::Valid, - call, - } - } - - /// Build invalid variant of `TestXt`. - pub fn invalid(mut self, err: TransactionValidityError) -> Self { - self.validity = TestValidity::OtherInvalid(err); - self - } - - /// Build badly signed variant of `TestXt`. - pub fn badly_signed(mut self) -> Self { - self.validity = TestValidity::SignatureInvalid(TransactionValidityError::Invalid(InvalidTransaction::BadProof)); - self - } - } +pub struct TestXt(pub Option<(u64, Extra)>, pub Call); // Non-opaque extrinsics always 0. parity_util_mem::malloc_size_of_is_0!(any: TestXt); @@ -368,39 +312,24 @@ impl Serialize for TestXt where TestXt: E impl Debug for TestXt { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - write!(f, "TestXt({:?}, {}, ...)", - self.signature.as_ref().map(|x| &x.0), - if let TestValidity::Valid = self.validity { "valid" } else { "invalid" } - ) + write!(f, "TestXt({:?}, ...)", self.0.as_ref().map(|x| &x.0)) } } impl Checkable for TestXt { type Checked = Self; - fn check(self, signature: CheckSignature, _: &Context) -> Result { - match self.validity { - TestValidity::Valid => Ok(self), - TestValidity::SignatureInvalid(e) => - if let CheckSignature::No = signature { - Ok(self) - } else { - Err(e) - }, - TestValidity::OtherInvalid(e) => Err(e), - } - } + fn check(self, _: &Context) -> Result { Ok(self) } } - impl traits::Extrinsic for TestXt { type Call = Call; type SignaturePayload = (u64, Extra); fn is_signed(&self) -> Option { - Some(self.signature.is_some()) + Some(self.0.is_some()) } - fn new(call: Call, signature: Option) -> Option { - Some(TestXt { signature, call, validity: TestValidity::Valid }) + fn new(c: Call, sig: Option) -> Option { + Some(TestXt(sig, c)) } } @@ -429,14 +358,14 @@ impl Applyable for TestXt where info: Self::DispatchInfo, len: usize, ) -> ApplyExtrinsicResult { - let maybe_who = if let Some((who, extra)) = self.signature { - Extra::pre_dispatch(extra, &who, &self.call, info, len)?; + let maybe_who = if let Some((who, extra)) = self.0 { + Extra::pre_dispatch(extra, &who, &self.1, info, len)?; Some(who) } else { - Extra::pre_dispatch_unsigned(&self.call, info, len)?; + Extra::pre_dispatch_unsigned(&self.1, info, len)?; None }; - Ok(self.call.dispatch(maybe_who.into()).map_err(Into::into)) + Ok(self.1.dispatch(maybe_who.into()).map_err(Into::into)) } } diff --git a/primitives/runtime/src/traits.rs b/primitives/runtime/src/traits.rs index 4e30e545e4ea5..39e015505b20b 100644 --- a/primitives/runtime/src/traits.rs +++ b/primitives/runtime/src/traits.rs @@ -30,7 +30,7 @@ use crate::codec::{Codec, Encode, Decode}; use crate::transaction_validity::{ ValidTransaction, TransactionValidity, TransactionValidityError, UnknownTransaction, }; -use crate::generic::{Digest, DigestItem, CheckSignature}; +use crate::generic::{Digest, DigestItem}; pub use sp_arithmetic::traits::{ AtLeast32Bit, UniqueSaturatedInto, UniqueSaturatedFrom, Saturating, SaturatedConversion, Zero, One, Bounded, CheckedAdd, CheckedSub, CheckedMul, CheckedDiv, @@ -647,7 +647,7 @@ pub trait Checkable: Sized { type Checked; /// Check self, given an instance of Context. - fn check(self, signature: CheckSignature, c: &Context) -> Result; + fn check(self, c: &Context) -> Result; } /// A "checkable" piece of information, used by the standard Substrate Executive in order to @@ -659,15 +659,15 @@ pub trait BlindCheckable: Sized { type Checked; /// Check self. - fn check(self, signature: CheckSignature) -> Result; + fn check(self) -> Result; } // Every `BlindCheckable` is also a `StaticCheckable` for arbitrary `Context`. impl Checkable for T { type Checked = ::Checked; - fn check(self, signature: CheckSignature, _c: &Context) -> Result { - BlindCheckable::check(self, signature) + fn check(self, _c: &Context) -> Result { + BlindCheckable::check(self) } } diff --git a/test-utils/runtime/src/lib.rs b/test-utils/runtime/src/lib.rs index 5ddeff390107a..f4797f7ce9fdc 100644 --- a/test-utils/runtime/src/lib.rs +++ b/test-utils/runtime/src/lib.rs @@ -41,7 +41,6 @@ use sp_runtime::{ BlindCheckable, BlakeTwo256, Block as BlockT, Extrinsic as ExtrinsicT, GetNodeBlockType, GetRuntimeBlockType, Verify, IdentityLookup, }, - generic::CheckSignature, }; use sp_version::RuntimeVersion; pub use sp_core::{hash::H256}; @@ -149,7 +148,7 @@ impl serde::Serialize for Extrinsic { impl BlindCheckable for Extrinsic { type Checked = Self; - fn check(self, _signature: CheckSignature) -> Result { + fn check(self) -> Result { match self { Extrinsic::AuthoritiesChange(new_auth) => Ok(Extrinsic::AuthoritiesChange(new_auth)), Extrinsic::Transfer { transfer, signature, exhaust_resources_when_not_first } => { @@ -516,10 +515,6 @@ cfg_if! { system::execute_transaction(extrinsic) } - fn apply_trusted_extrinsic(extrinsic: ::Extrinsic) -> ApplyExtrinsicResult { - system::execute_transaction(extrinsic) - } - fn finalize_block() -> ::Header { system::finalize_block() } @@ -707,10 +702,6 @@ cfg_if! { system::execute_transaction(extrinsic) } - fn apply_trusted_extrinsic(extrinsic: ::Extrinsic) -> ApplyExtrinsicResult { - system::execute_transaction(extrinsic) - } - fn finalize_block() -> ::Header { system::finalize_block() } diff --git a/test-utils/runtime/src/system.rs b/test-utils/runtime/src/system.rs index 4ce774598f3b0..1e29f789dcb5f 100644 --- a/test-utils/runtime/src/system.rs +++ b/test-utils/runtime/src/system.rs @@ -29,7 +29,6 @@ use sp_runtime::{ transaction_validity::{ TransactionValidity, ValidTransaction, InvalidTransaction, TransactionValidityError, }, - generic::CheckSignature, }; use codec::{KeyedVec, Encode, Decode}; use frame_system::Trait; @@ -244,7 +243,7 @@ pub fn finalize_block() -> Header { #[inline(always)] fn check_signature(utx: &Extrinsic) -> Result<(), TransactionValidityError> { use sp_runtime::traits::BlindCheckable; - utx.clone().check(CheckSignature::Yes).map_err(|_| InvalidTransaction::BadProof.into()).map(|_| ()) + utx.clone().check().map_err(|_| InvalidTransaction::BadProof.into()).map(|_| ()) } fn execute_transaction_backend(utx: &Extrinsic, extrinsic_index: u32) -> ApplyExtrinsicResult {