From ddea30604445a39c41dc8c1bef4e28d00783d14f Mon Sep 17 00:00:00 2001
From: Demi Obenour <demi@parity.io>
Date: Thu, 14 May 2020 22:01:56 +0000
Subject: [PATCH] =?UTF-8?q?Add=20=E2=80=98transaction=5Fversion=E2=80=99?=
=?UTF-8?q?=20to=20the=20signed=20transaction=20(#5979)?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
* Add ‘transaction_version’ to the signed transaction
This allows hardware wallets to know which transactions they can safely
sign. To reduce transaction size, I reduced it to a ‘u8’ from a ‘u32’.
Fixes #5951.
* Restore transaction_version to a u32
* Fix comments
`transaction_version` is not part of a tx, but is still signed.
Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com>
* Fix the test suite
I had forgotten to change the production of transactions in the test
code.
* Fix benchmarks
* Improve docs for `CheckTxVersion` in `frame_system`
Co-authored-by: André Silva <123550+andresilva@users.noreply.github.com>
* Remove spurious cast
Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com>
Co-authored-by: André Silva <123550+andresilva@users.noreply.github.com>
---
.../bin/node-template/runtime/src/lib.rs | 3 +-
substrate/bin/node/cli/src/service.rs | 13 +++--
substrate/bin/node/executor/benches/bench.rs | 6 ++-
substrate/bin/node/executor/tests/common.rs | 6 ++-
.../node/executor/tests/submit_transaction.rs | 4 +-
substrate/bin/node/runtime/src/lib.rs | 10 +++-
substrate/bin/node/testing/src/keyring.rs | 7 +--
substrate/bin/utils/subkey/src/main.rs | 6 ++-
substrate/frame/system/src/lib.rs | 51 ++++++++++++++++---
9 files changed, 81 insertions(+), 25 deletions(-)
diff --git a/substrate/bin/node-template/runtime/src/lib.rs b/substrate/bin/node-template/runtime/src/lib.rs
index 7c3302d8edc..b1b73f3b49f 100644
--- a/substrate/bin/node-template/runtime/src/lib.rs
+++ b/substrate/bin/node-template/runtime/src/lib.rs
@@ -281,7 +281,8 @@ pub type SignedBlock = generic::SignedBlock<Block>;
pub type BlockId = generic::BlockId<Block>;
/// The SignedExtension to the basic transaction logic.
pub type SignedExtra = (
- system::CheckVersion<Runtime>,
+ system::CheckSpecVersion<Runtime>,
+ system::CheckTxVersion<Runtime>,
system::CheckGenesis<Runtime>,
system::CheckEra<Runtime>,
system::CheckNonce<Runtime>,
diff --git a/substrate/bin/node/cli/src/service.rs b/substrate/bin/node/cli/src/service.rs
index 7e27d57063e..7d6a210bee6 100644
--- a/substrate/bin/node/cli/src/service.rs
+++ b/substrate/bin/node/cli/src/service.rs
@@ -602,12 +602,16 @@ mod tests {
let from: Address = AccountPublic::from(charlie.public()).into_account().into();
let genesis_hash = service.client().block_hash(0).unwrap().unwrap();
let best_block_id = BlockId::number(service.client().chain_info().best_number);
- let version = service.client().runtime_version_at(&best_block_id).unwrap().spec_version;
+ let (spec_version, transaction_version) = {
+ let version = service.client().runtime_version_at(&best_block_id).unwrap();
+ (version.spec_version, version.transaction_version)
+ };
let signer = charlie.clone();
let function = Call::Balances(BalancesCall::transfer(to.into(), amount));
- let check_version = frame_system::CheckVersion::new();
+ let check_spec_version = frame_system::CheckSpecVersion::new();
+ let check_tx_version = frame_system::CheckTxVersion::new();
let check_genesis = frame_system::CheckGenesis::new();
let check_era = frame_system::CheckEra::from(Era::Immortal);
let check_nonce = frame_system::CheckNonce::from(index);
@@ -615,7 +619,8 @@ mod tests {
let payment = pallet_transaction_payment::ChargeTransactionPayment::from(0);
let validate_grandpa_equivocation = pallet_grandpa::ValidateEquivocationReport::new();
let extra = (
- check_version,
+ check_spec_version,
+ check_tx_version,
check_genesis,
check_era,
check_nonce,
@@ -626,7 +631,7 @@ mod tests {
let raw_payload = SignedPayload::from_raw(
function,
extra,
- (version, genesis_hash, genesis_hash, (), (), (), ())
+ (spec_version, transaction_version, genesis_hash, genesis_hash, (), (), (), ())
);
let signature = raw_payload.using_encoded(|payload| {
signer.sign(payload)
diff --git a/substrate/bin/node/executor/benches/bench.rs b/substrate/bin/node/executor/benches/bench.rs
index 4f335df90d1..0f269c301be 100644
--- a/substrate/bin/node/executor/benches/bench.rs
+++ b/substrate/bin/node/executor/benches/bench.rs
@@ -39,7 +39,9 @@ const COMPACT_CODE: &[u8] = node_runtime::WASM_BINARY;
const GENESIS_HASH: [u8; 32] = [69u8; 32];
-const VERSION: u32 = node_runtime::VERSION.spec_version;
+const TRANSACTION_VERSION: u32 = node_runtime::VERSION.transaction_version;
+
+const SPEC_VERSION: u32 = node_runtime::VERSION.spec_version;
const HEAP_PAGES: u64 = 20;
@@ -52,7 +54,7 @@ enum ExecutionMethod {
}
fn sign(xt: CheckedExtrinsic) -> UncheckedExtrinsic {
- node_testing::keyring::sign(xt, VERSION, GENESIS_HASH)
+ node_testing::keyring::sign(xt, SPEC_VERSION, TRANSACTION_VERSION, GENESIS_HASH)
}
fn new_test_ext(genesis_config: &GenesisConfig) -> TestExternalities<BlakeTwo256> {
diff --git a/substrate/bin/node/executor/tests/common.rs b/substrate/bin/node/executor/tests/common.rs
index 5a51e4312c5..e888c269cc9 100644
--- a/substrate/bin/node/executor/tests/common.rs
+++ b/substrate/bin/node/executor/tests/common.rs
@@ -71,12 +71,14 @@ pub const COMPACT_CODE: &[u8] = node_runtime::WASM_BINARY;
pub const GENESIS_HASH: [u8; 32] = [69u8; 32];
-pub const VERSION: u32 = node_runtime::VERSION.spec_version;
+pub const SPEC_VERSION: u32 = node_runtime::VERSION.spec_version;
+
+pub const TRANSACTION_VERSION: u32 = node_runtime::VERSION.transaction_version;
pub type TestExternalities<H> = CoreTestExternalities<H, u64>;
pub fn sign(xt: CheckedExtrinsic) -> UncheckedExtrinsic {
- node_testing::keyring::sign(xt, VERSION, GENESIS_HASH)
+ node_testing::keyring::sign(xt, SPEC_VERSION, TRANSACTION_VERSION, GENESIS_HASH)
}
pub fn default_transfer_call() -> pallet_balances::Call<Runtime> {
diff --git a/substrate/bin/node/executor/tests/submit_transaction.rs b/substrate/bin/node/executor/tests/submit_transaction.rs
index b968159d327..7ebee2658b7 100644
--- a/substrate/bin/node/executor/tests/submit_transaction.rs
+++ b/substrate/bin/node/executor/tests/submit_transaction.rs
@@ -122,7 +122,7 @@ fn should_submit_signed_twice_from_the_same_account() {
let s = state.read();
fn nonce(tx: UncheckedExtrinsic) -> frame_system::CheckNonce<Runtime> {
let extra = tx.signature.unwrap().2;
- extra.3
+ extra.4
}
let nonce1 = nonce(UncheckedExtrinsic::decode(&mut &*s.transactions[0]).unwrap());
let nonce2 = nonce(UncheckedExtrinsic::decode(&mut &*s.transactions[1]).unwrap());
@@ -170,7 +170,7 @@ fn should_submit_signed_twice_from_all_accounts() {
let s = state.read();
fn nonce(tx: UncheckedExtrinsic) -> frame_system::CheckNonce<Runtime> {
let extra = tx.signature.unwrap().2;
- extra.3
+ extra.4
}
let nonce1 = nonce(UncheckedExtrinsic::decode(&mut &*s.transactions[0]).unwrap());
let nonce2 = nonce(UncheckedExtrinsic::decode(&mut &*s.transactions[1]).unwrap());
diff --git a/substrate/bin/node/runtime/src/lib.rs b/substrate/bin/node/runtime/src/lib.rs
index 1eec930dfbb..85fce74ed90 100644
--- a/substrate/bin/node/runtime/src/lib.rs
+++ b/substrate/bin/node/runtime/src/lib.rs
@@ -535,7 +535,8 @@ impl<LocalCall> frame_system::offchain::CreateSignedTransaction<LocalCall> for R
.saturating_sub(1);
let tip = 0;
let extra: SignedExtra = (
- frame_system::CheckVersion::<Runtime>::new(),
+ frame_system::CheckSpecVersion::<Runtime>::new(),
+ frame_system::CheckTxVersion::<Runtime>::new(),
frame_system::CheckGenesis::<Runtime>::new(),
frame_system::CheckEra::<Runtime>::from(generic::Era::mortal(period, current_block)),
frame_system::CheckNonce::<Runtime>::from(nonce),
@@ -745,8 +746,13 @@ pub type SignedBlock = generic::SignedBlock<Block>;
/// BlockId type as expected by this runtime.
pub type BlockId = generic::BlockId<Block>;
/// The SignedExtension to the basic transaction logic.
+///
+/// When you change this, you **MUST** modify [`sign`] in `bin/node/testing/src/keyring.rs`!
+///
+/// [`sign`]: <../../testing/src/keyring.rs.html>
pub type SignedExtra = (
- frame_system::CheckVersion<Runtime>,
+ frame_system::CheckSpecVersion<Runtime>,
+ frame_system::CheckTxVersion<Runtime>,
frame_system::CheckGenesis<Runtime>,
frame_system::CheckEra<Runtime>,
frame_system::CheckNonce<Runtime>,
diff --git a/substrate/bin/node/testing/src/keyring.rs b/substrate/bin/node/testing/src/keyring.rs
index 7ed2b36502e..2b1d6f985a6 100644
--- a/substrate/bin/node/testing/src/keyring.rs
+++ b/substrate/bin/node/testing/src/keyring.rs
@@ -68,7 +68,8 @@ pub fn to_session_keys(
/// Returns transaction extra.
pub fn signed_extra(nonce: Index, extra_fee: Balance) -> SignedExtra {
(
- frame_system::CheckVersion::new(),
+ frame_system::CheckSpecVersion::new(),
+ frame_system::CheckTxVersion::new(),
frame_system::CheckGenesis::new(),
frame_system::CheckEra::from(Era::mortal(256, 0)),
frame_system::CheckNonce::from(nonce),
@@ -79,10 +80,10 @@ pub fn signed_extra(nonce: Index, extra_fee: Balance) -> SignedExtra {
}
/// Sign given `CheckedExtrinsic`.
-pub fn sign(xt: CheckedExtrinsic, version: u32, genesis_hash: [u8; 32]) -> UncheckedExtrinsic {
+pub fn sign(xt: CheckedExtrinsic, spec_version: u32, tx_version: u32, genesis_hash: [u8; 32]) -> UncheckedExtrinsic {
match xt.signed {
Some((signed, extra)) => {
- let payload = (xt.function, extra.clone(), version, genesis_hash, genesis_hash);
+ let payload = (xt.function, extra.clone(), spec_version, tx_version, genesis_hash, genesis_hash);
let key = AccountKeyring::from_account_id(&signed).unwrap();
let signature = payload.using_encoded(|b| {
if b.len() > 256 {
diff --git a/substrate/bin/utils/subkey/src/main.rs b/substrate/bin/utils/subkey/src/main.rs
index 754a2611bcd..dd618393c32 100644
--- a/substrate/bin/utils/subkey/src/main.rs
+++ b/substrate/bin/utils/subkey/src/main.rs
@@ -702,7 +702,8 @@ fn create_extrinsic<C: Crypto>(
{
let extra = |i: Index, f: Balance| {
(
- frame_system::CheckVersion::<Runtime>::new(),
+ frame_system::CheckSpecVersion::<Runtime>::new(),
+ frame_system::CheckTxVersion::<Runtime>::new(),
frame_system::CheckGenesis::<Runtime>::new(),
frame_system::CheckEra::<Runtime>::from(Era::Immortal),
frame_system::CheckNonce::<Runtime>::from(i),
@@ -715,7 +716,8 @@ fn create_extrinsic<C: Crypto>(
function,
extra(index, 0),
(
- VERSION.spec_version as u32,
+ VERSION.spec_version,
+ VERSION.transaction_version,
genesis_hash,
genesis_hash,
(),
diff --git a/substrate/frame/system/src/lib.rs b/substrate/frame/system/src/lib.rs
index ff6893d6290..f1e04c5424b 100644
--- a/substrate/frame/system/src/lib.rs
+++ b/substrate/frame/system/src/lib.rs
@@ -53,7 +53,9 @@
//! - [`CheckEra`]: Checks the era of the transaction. Contains a single payload of type `Era`.
//! - [`CheckGenesis`]: Checks the provided genesis hash of the transaction. Must be a part of the
//! signed payload of the transaction.
-//! - [`CheckVersion`]: Checks that the runtime version is the same as the one encoded in the
+//! - [`CheckSpecVersion`]: Checks that the runtime version is the same as the one used to sign the
+//! transaction.
+//! - [`CheckTxVersion`]: Checks that the transaction version is the same as the one used to sign the
//! transaction.
//!
//! Lookup the runtime aggregator file (e.g. `node/runtime`) to see the full list of signed
@@ -1735,14 +1737,49 @@ impl<T: Trait + Send + Sync> SignedExtension for CheckGenesis<T> {
}
}
+/// Ensure the transaction version registered in the transaction is the same as at present.
+#[derive(Encode, Decode, Clone, Eq, PartialEq)]
+pub struct CheckTxVersion<T: Trait + Send + Sync>(sp_std::marker::PhantomData<T>);
+
+impl<T: Trait + Send + Sync> Debug for CheckTxVersion<T> {
+ #[cfg(feature = "std")]
+ fn fmt(&self, f: &mut sp_std::fmt::Formatter) -> sp_std::fmt::Result {
+ write!(f, "CheckTxVersion")
+ }
+
+ #[cfg(not(feature = "std"))]
+ fn fmt(&self, _: &mut sp_std::fmt::Formatter) -> sp_std::fmt::Result {
+ Ok(())
+ }
+}
+
+impl<T: Trait + Send + Sync> CheckTxVersion<T> {
+ /// Create new `SignedExtension` to check transaction version.
+ pub fn new() -> Self {
+ Self(sp_std::marker::PhantomData)
+ }
+}
+
+impl<T: Trait + Send + Sync> SignedExtension for CheckTxVersion<T> {
+ type AccountId = T::AccountId;
+ type Call = <T as Trait>::Call;
+ type AdditionalSigned = u32;
+ type Pre = ();
+ const IDENTIFIER: &'static str = "CheckTxVersion";
+
+ fn additional_signed(&self) -> Result<Self::AdditionalSigned, TransactionValidityError> {
+ Ok(<Module<T>>::runtime_version().transaction_version)
+ }
+}
+
/// Ensure the runtime version registered in the transaction is the same as at present.
#[derive(Encode, Decode, Clone, Eq, PartialEq)]
-pub struct CheckVersion<T: Trait + Send + Sync>(sp_std::marker::PhantomData<T>);
+pub struct CheckSpecVersion<T: Trait + Send + Sync>(sp_std::marker::PhantomData<T>);
-impl<T: Trait + Send + Sync> Debug for CheckVersion<T> {
+impl<T: Trait + Send + Sync> Debug for CheckSpecVersion<T> {
#[cfg(feature = "std")]
fn fmt(&self, f: &mut sp_std::fmt::Formatter) -> sp_std::fmt::Result {
- write!(f, "CheckVersion")
+ write!(f, "CheckSpecVersion")
}
#[cfg(not(feature = "std"))]
@@ -1751,19 +1788,19 @@ impl<T: Trait + Send + Sync> Debug for CheckVersion<T> {
}
}
-impl<T: Trait + Send + Sync> CheckVersion<T> {
+impl<T: Trait + Send + Sync> CheckSpecVersion<T> {
/// Create new `SignedExtension` to check runtime version.
pub fn new() -> Self {
Self(sp_std::marker::PhantomData)
}
}
-impl<T: Trait + Send + Sync> SignedExtension for CheckVersion<T> {
+impl<T: Trait + Send + Sync> SignedExtension for CheckSpecVersion<T> {
type AccountId = T::AccountId;
type Call = <T as Trait>::Call;
type AdditionalSigned = u32;
type Pre = ();
- const IDENTIFIER: &'static str = "CheckVersion";
+ const IDENTIFIER: &'static str = "CheckSpecVersion";
fn additional_signed(&self) -> Result<Self::AdditionalSigned, TransactionValidityError> {
Ok(<Module<T>>::runtime_version().spec_version)
--
GitLab