From ab34400915e715c373980b5aae4b0b1dc8117ec7 Mon Sep 17 00:00:00 2001 From: Kian Paimani Date: Wed, 19 Jun 2019 16:54:45 +0200 Subject: [PATCH 01/23] Initial draft of tip enum --- .../src/generic/checked_extrinsic.rs | 12 +- core/sr-primitives/src/generic/mod.rs | 2 + core/sr-primitives/src/generic/tip.rs | 40 ++++ .../src/generic/unchecked_extrinsic.rs | 77 +++++--- .../unchecked_mortal_compact_extrinsic.rs | 182 ++++++++++++++---- .../src/generic/unchecked_mortal_extrinsic.rs | 169 +++++++++++++--- 6 files changed, 391 insertions(+), 91 deletions(-) create mode 100644 core/sr-primitives/src/generic/tip.rs diff --git a/core/sr-primitives/src/generic/checked_extrinsic.rs b/core/sr-primitives/src/generic/checked_extrinsic.rs index ee43b3af2e951..a1ac2b30a2087 100644 --- a/core/sr-primitives/src/generic/checked_extrinsic.rs +++ b/core/sr-primitives/src/generic/checked_extrinsic.rs @@ -17,27 +17,32 @@ //! Generic implementation of an extrinsic that has passed the verification //! stage. +use rstd::marker::{Sync, Send}; // TODO: what the fucking hell. use crate::traits::{self, Member, SimpleArithmetic, MaybeDisplay}; use crate::weights::{Weighable, Weight}; +use super::Tip; /// Definition of something that the external world might want to say; its /// existence implies that it has been checked and is good, particularly with /// regards to the signature. #[derive(PartialEq, Eq, Clone)] #[cfg_attr(feature = "std", derive(Debug))] -pub struct CheckedExtrinsic { +pub struct CheckedExtrinsic { /// Who this purports to be from and the number of extrinsics have come before /// from the same signer, if anyone (note this is not a signature). pub signed: Option<(AccountId, Index)>, /// The function that should be called. pub function: Call, + /// The tip for this transaction + pub tip: Tip, } -impl traits::Applyable for CheckedExtrinsic +impl traits::Applyable for CheckedExtrinsic where AccountId: Member + MaybeDisplay, Index: Member + MaybeDisplay + SimpleArithmetic, Call: Member, + Balance: Sync + Send, { type Index = Index; type AccountId = AccountId; @@ -56,7 +61,8 @@ where } } -impl Weighable for CheckedExtrinsic +impl Weighable + for CheckedExtrinsic where Call: Weighable, { diff --git a/core/sr-primitives/src/generic/mod.rs b/core/sr-primitives/src/generic/mod.rs index a4e4106780efc..0e9136e81e2aa 100644 --- a/core/sr-primitives/src/generic/mod.rs +++ b/core/sr-primitives/src/generic/mod.rs @@ -26,6 +26,7 @@ mod checked_extrinsic; mod header; mod block; mod digest; +mod tip; #[cfg(test)] mod tests; @@ -39,6 +40,7 @@ pub use self::block::{Block, SignedBlock, BlockId}; pub use self::digest::{ Digest, DigestItem, DigestItemRef, OpaqueDigestItemId }; +pub use self::tip::Tip; use crate::codec::Encode; use rstd::prelude::*; diff --git a/core/sr-primitives/src/generic/tip.rs b/core/sr-primitives/src/generic/tip.rs new file mode 100644 index 0000000000000..e60a879aa2f5a --- /dev/null +++ b/core/sr-primitives/src/generic/tip.rs @@ -0,0 +1,40 @@ +// Copyright 2019 Parity Technologies (UK) Ltd. +// This file is part of Substrate. + +// Substrate is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. + +// Substrate is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License +// along with Substrate. If not, see . + +//! Tip structure for a transaction. + +use crate::codec::{Encode, Decode}; +// use crate::traits::SimpleArithmetic; + +/// Representation of a transaction tip. +/// Upon decoding, all transaction types try and decode this from the end of the encoded byte +/// stream. +/// If non-existent, the default implementation will be used.s +#[derive(Clone, Eq, PartialEq, Encode, Decode)] +#[cfg(feature = "std")] +#[derive(Debug)] +pub enum Tip { + /// This transaction does not include any tips. + None, + /// The sender of the transaction has included some tip. + Sender(Balance), +} + +impl Default for Tip { + fn default() -> Self { + Tip::None + } +} diff --git a/core/sr-primitives/src/generic/unchecked_extrinsic.rs b/core/sr-primitives/src/generic/unchecked_extrinsic.rs index d6e0d60e2c218..c347c1e6d120d 100644 --- a/core/sr-primitives/src/generic/unchecked_extrinsic.rs +++ b/core/sr-primitives/src/generic/unchecked_extrinsic.rs @@ -22,7 +22,7 @@ use std::fmt; use rstd::prelude::*; use crate::codec::{Decode, Encode, Codec, Input, HasCompact}; use crate::traits::{self, Member, SimpleArithmetic, MaybeDisplay, Lookup, Extrinsic}; -use super::CheckedExtrinsic; +use super::{CheckedExtrinsic, Tip}; #[derive(PartialEq, Eq, Clone, Encode, Decode)] pub struct SignatureContent @@ -39,7 +39,7 @@ where /// A extrinsic right from the external world. This is unchecked and so /// can contain a signature. #[derive(PartialEq, Eq, Clone)] -pub struct UncheckedExtrinsic +pub struct UncheckedExtrinsic where Address: Codec, Index: HasCompact + Codec, @@ -50,33 +50,43 @@ where pub signature: Option>, /// The function that should be called. pub function: Call, + /// The tip for this transaction + pub tip: Tip, } -impl UncheckedExtrinsic +impl UncheckedExtrinsic where Address: Codec, Index: HasCompact + Codec, Signature: Codec, + Balance: Codec, { /// New instance of a signed extrinsic aka "transaction". - pub fn new_signed(index: Index, function: Call, signed: Address, signature: Signature) -> Self { + pub fn new_signed(index: Index, + function: Call, + signed: Address, + signature: Signature, + tip: Tip) + -> Self { UncheckedExtrinsic { signature: Some(SignatureContent{signed, signature, index}), function, + tip, } } /// New instance of an unsigned extrinsic aka "inherent". - pub fn new_unsigned(function: Call) -> Self { + pub fn new_unsigned(function: Call, tip: Tip) -> Self { UncheckedExtrinsic { signature: None, function, + tip, } } } -impl traits::Checkable - for UncheckedExtrinsic +impl traits::Checkable + for UncheckedExtrinsic where Address: Member + MaybeDisplay + Codec, Index: Member + MaybeDisplay + SimpleArithmetic + Codec, @@ -84,8 +94,9 @@ where Signature: Member + traits::Verify + Codec, AccountId: Member + MaybeDisplay, Context: Lookup, + Balance: Codec, { - type Checked = CheckedExtrinsic; + type Checked = CheckedExtrinsic; fn check(self, context: &Context) -> Result { Ok(match self.signature { @@ -98,11 +109,13 @@ where CheckedExtrinsic { signed: Some((signed, payload.0)), function: payload.1, + tip: self.tip, } } None => CheckedExtrinsic { signed: None, function: self.function, + tip: self.tip, }, }) } @@ -113,14 +126,15 @@ impl< Index: HasCompact + Codec, Signature: Codec, Call, -> Extrinsic for UncheckedExtrinsic { + Balance: Codec, +> Extrinsic for UncheckedExtrinsic { fn is_signed(&self) -> Option { Some(self.signature.is_some()) } } -impl Decode - for UncheckedExtrinsic +impl Decode + for UncheckedExtrinsic { fn decode(input: &mut I) -> Option { // This is a little more complicated than usual since the binary format must be compatible @@ -132,24 +146,26 @@ impl Some(UncheckedExtrinsic { signature: Decode::decode(input)?, function: Decode::decode(input)?, + tip: Decode::decode(input).unwrap_or_default(), }) } } -impl Encode - for UncheckedExtrinsic +impl Encode + for UncheckedExtrinsic { fn encode(&self) -> Vec { super::encode_with_vec_prefix::(|v| { self.signature.encode_to(v); self.function.encode_to(v); + self.tip.encode_to(v); }) } } #[cfg(feature = "std")] -impl serde::Serialize - for UncheckedExtrinsic +impl serde::Serialize + for UncheckedExtrinsic { fn serialize(&self, seq: S) -> Result where S: ::serde::Serializer { self.using_encoded(|bytes| ::substrate_primitives::bytes::serialize(bytes, seq)) @@ -157,28 +173,36 @@ impl } #[cfg(feature = "std")] -impl fmt::Debug - for UncheckedExtrinsic +impl fmt::Debug + for UncheckedExtrinsic where Address: fmt::Debug + Codec, Index: fmt::Debug + HasCompact + Codec, Signature: Codec, Call: fmt::Debug, + Balance: fmt::Debug, { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - write!(f, "UncheckedExtrinsic({:?}, {:?})", self.signature.as_ref().map(|x| (&x.signed, &x.index)), self.function) + write!( + f, + "UncheckedExtrinsic({:?}, {:?}, {:?})", + self.signature.as_ref().map(|x| (&x.signed, &x.index)), + self.function, + self.tip, + ) } } #[cfg(test)] mod test { use crate::codec::{Decode, Encode}; - use super::UncheckedExtrinsic; + use super::{UncheckedExtrinsic, Tip}; + type Extrinsic = UncheckedExtrinsic; + #[test] fn encoding_matches_vec() { - type Extrinsic = UncheckedExtrinsic; - let ex = Extrinsic::new_unsigned(42); + let ex = Extrinsic::new_unsigned(42, Tip::Sender(10_u32)); let encoded = ex.encode(); let decoded = Extrinsic::decode(&mut encoded.as_slice()).unwrap(); assert_eq!(decoded, ex); @@ -186,12 +210,19 @@ mod test { assert_eq!(as_vec.encode(), encoded); } + #[test] + fn unprovided_tip_will_not_fail() { + let ex = Extrinsic::new_unsigned(42, Tip::Sender(10_u32)); + let encoded = ex.encode(); + println!("This is it {:?}", encoded); + unimplemented!(); + } #[test] #[cfg(feature = "std")] fn serialization_of_unchecked_extrinsics() { - type Extrinsic = UncheckedExtrinsic; - let ex = Extrinsic::new_unsigned(42); + type Extrinsic = UncheckedExtrinsic; + let ex = Extrinsic::new_unsigned(42, Tip::None); assert_eq!(serde_json::to_string(&ex).unwrap(), "\"0x14002a000000\""); } diff --git a/core/sr-primitives/src/generic/unchecked_mortal_compact_extrinsic.rs b/core/sr-primitives/src/generic/unchecked_mortal_compact_extrinsic.rs index 36e17fc277cde..a0410cb903d4e 100644 --- a/core/sr-primitives/src/generic/unchecked_mortal_compact_extrinsic.rs +++ b/core/sr-primitives/src/generic/unchecked_mortal_compact_extrinsic.rs @@ -24,48 +24,63 @@ use runtime_io::blake2_256; use crate::codec::{Decode, Encode, Input, Compact}; use crate::traits::{self, Member, SimpleArithmetic, MaybeDisplay, CurrentHeight, BlockNumberToHash, Lookup, Checkable, Extrinsic, SaturatedConversion}; -use super::{CheckedExtrinsic, Era}; +use super::{CheckedExtrinsic, Era, Tip}; const TRANSACTION_VERSION: u8 = 1; /// A extrinsic right from the external world. This is unchecked and so /// can contain a signature. #[derive(PartialEq, Eq, Clone)] -pub struct UncheckedMortalCompactExtrinsic { +pub struct UncheckedMortalCompactExtrinsic { /// The signature, address, number of extrinsics have come before from /// the same signer and an era describing the longevity of this transaction, /// if this is a signed extrinsic. pub signature: Option<(Address, Signature, Compact, Era)>, /// The function that should be called. pub function: Call, + /// The tip for this transaction + pub tip: Tip, } -impl UncheckedMortalCompactExtrinsic { +impl + UncheckedMortalCompactExtrinsic +{ /// New instance of a signed extrinsic aka "transaction". - pub fn new_signed(index: Index, function: Call, signed: Address, signature: Signature, era: Era) -> Self { + pub fn new_signed( + index: Index, + function: Call, + signed: Address, + signature: Signature, + era: Era, + tip: Tip + ) -> Self { UncheckedMortalCompactExtrinsic { signature: Some((signed, signature, index.into(), era)), function, + tip, } } /// New instance of an unsigned extrinsic aka "inherent". - pub fn new_unsigned(function: Call) -> Self { + pub fn new_unsigned(function: Call, tip: Tip) -> Self { UncheckedMortalCompactExtrinsic { signature: None, function, + tip, } } } -impl Extrinsic for UncheckedMortalCompactExtrinsic { +impl Extrinsic + for UncheckedMortalCompactExtrinsic +{ fn is_signed(&self) -> Option { Some(self.signature.is_some()) } } -impl Checkable - for UncheckedMortalCompactExtrinsic +impl Checkable + for UncheckedMortalCompactExtrinsic where Address: Member + MaybeDisplay, Index: Member + MaybeDisplay + SimpleArithmetic, @@ -79,7 +94,7 @@ where + CurrentHeight + BlockNumberToHash, { - type Checked = CheckedExtrinsic; + type Checked = CheckedExtrinsic; fn check(self, context: &Context) -> Result { Ok(match self.signature { @@ -101,23 +116,27 @@ where CheckedExtrinsic { signed: Some((signed, (raw_payload.0).0)), function: raw_payload.1, + tip: self.tip, } } None => CheckedExtrinsic { signed: None, function: self.function, + tip: self.tip, }, }) } } -impl Decode - for UncheckedMortalCompactExtrinsic +impl Decode + for UncheckedMortalCompactExtrinsic where Address: Decode, Signature: Decode, Compact: Decode, Call: Decode, + Balance: Decode, + { fn decode(input: &mut I) -> Option { // This is a little more complicated than usual since the binary format must be compatible @@ -137,17 +156,19 @@ where Some(UncheckedMortalCompactExtrinsic { signature: if is_signed { Some(Decode::decode(input)?) } else { None }, function: Decode::decode(input)?, + tip: Decode::decode(input).unwrap_or_default(), }) } } -impl Encode - for UncheckedMortalCompactExtrinsic +impl Encode + for UncheckedMortalCompactExtrinsic where Address: Encode, Signature: Encode, Compact: Encode, Call: Encode, + Balance: Encode, { fn encode(&self) -> Vec { super::encode_with_vec_prefix::(|v| { @@ -162,13 +183,14 @@ where } } self.function.encode_to(v); + self.tip.encode_to(v); }) } } #[cfg(feature = "std")] -impl serde::Serialize - for UncheckedMortalCompactExtrinsic +impl serde::Serialize + for UncheckedMortalCompactExtrinsic where Compact: Encode { fn serialize(&self, seq: S) -> Result where S: ::serde::Serializer { @@ -177,13 +199,22 @@ impl serde::Serialize } #[cfg(feature = "std")] -impl fmt::Debug for UncheckedMortalCompactExtrinsic where +impl fmt::Debug + for UncheckedMortalCompactExtrinsic +where Address: fmt::Debug, Index: fmt::Debug, Call: fmt::Debug, + Balance: fmt::Debug, { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - write!(f, "UncheckedMortalCompactExtrinsic({:?}, {:?})", self.signature.as_ref().map(|x| (&x.0, &x.2)), self.function) + write!( + f, + "UncheckedMortalCompactExtrinsic({:?}, {:?}, {:?})", + self.signature.as_ref().map(|x| (&x.0, &x.2)), + self.function, + self.tip, + ) } } @@ -220,83 +251,168 @@ mod tests { } const DUMMY_ACCOUNTID: u64 = 0; + type Balance = u64; - type Ex = UncheckedMortalCompactExtrinsic, TestSig>; - type CEx = CheckedExtrinsic>; + type Ex = UncheckedMortalCompactExtrinsic, TestSig, Balance>; + type CEx = CheckedExtrinsic, Balance>; #[test] fn unsigned_codec_should_work() { - let ux = Ex::new_unsigned(vec![0u8;0]); + let ux = Ex::new_unsigned(vec![0u8;0], Tip::default()); let encoded = ux.encode(); assert_eq!(Ex::decode(&mut &encoded[..]), Some(ux)); } #[test] fn signed_codec_should_work() { - let ux = Ex::new_signed(0, vec![0u8;0], DUMMY_ACCOUNTID, TestSig(DUMMY_ACCOUNTID, (DUMMY_ACCOUNTID, vec![0u8;0], Era::immortal(), 0u64).encode()), Era::immortal()); + let ux = Ex::new_signed( + 0, + vec![0u8; 0], + DUMMY_ACCOUNTID, + TestSig(DUMMY_ACCOUNTID, (DUMMY_ACCOUNTID, vec![0u8;0], Era::immortal(), 0u64).encode()), + Era::immortal(), + Tip::default() + ); let encoded = ux.encode(); assert_eq!(Ex::decode(&mut &encoded[..]), Some(ux)); } #[test] fn large_signed_codec_should_work() { - let ux = Ex::new_signed(0, vec![0u8;0], DUMMY_ACCOUNTID, TestSig(DUMMY_ACCOUNTID, (DUMMY_ACCOUNTID, vec![0u8; 257], Era::immortal(), 0u64).using_encoded(blake2_256)[..].to_owned()), Era::immortal()); + let ux = Ex::new_signed( + 0, + vec![0u8; 0], + DUMMY_ACCOUNTID, + TestSig( + DUMMY_ACCOUNTID, + (DUMMY_ACCOUNTID, vec![0u8; 257], Era::immortal(), 0u64).using_encoded(blake2_256)[..].to_owned() + ), + Era::immortal(), + Tip::default() + ); let encoded = ux.encode(); assert_eq!(Ex::decode(&mut &encoded[..]), Some(ux)); } #[test] fn unsigned_check_should_work() { - let ux = Ex::new_unsigned(vec![0u8;0]); + let ux = Ex::new_unsigned(vec![0u8;0], Tip::default()); assert!(!ux.is_signed().unwrap_or(false)); assert!(>::check(ux, &TestContext).is_ok()); } #[test] fn badly_signed_check_should_fail() { - let ux = Ex::new_signed(0, vec![0u8;0], DUMMY_ACCOUNTID, TestSig(DUMMY_ACCOUNTID, vec![0u8]), Era::immortal()); + let ux = Ex::new_signed( + 0, + vec![0u8; 0], + DUMMY_ACCOUNTID, + TestSig(DUMMY_ACCOUNTID, vec![0u8]), + Era::immortal(), + Tip::default(), + ); assert!(ux.is_signed().unwrap_or(false)); assert_eq!(>::check(ux, &TestContext), Err(crate::BAD_SIGNATURE)); } #[test] fn immortal_signed_check_should_work() { - let ux = Ex::new_signed(0, vec![0u8;0], DUMMY_ACCOUNTID, TestSig(DUMMY_ACCOUNTID, (Compact::from(DUMMY_ACCOUNTID), vec![0u8;0], Era::immortal(), 0u64).encode()), Era::immortal()); + let ux = Ex::new_signed( + 0, + vec![0u8;0], + DUMMY_ACCOUNTID, + TestSig( + DUMMY_ACCOUNTID, + (Compact::from(DUMMY_ACCOUNTID), vec![0u8;0], Era::immortal(), 0u64).encode() + ), + Era::immortal(), + Tip::Sender(20), + ); assert!(ux.is_signed().unwrap_or(false)); - assert_eq!(>::check(ux, &TestContext), Ok(CEx { signed: Some((DUMMY_ACCOUNTID, 0)), function: vec![0u8;0] })); + assert_eq!( + >::check(ux, &TestContext), + Ok(CEx { signed: Some((DUMMY_ACCOUNTID, 0)), function: vec![0u8;0], tip: Tip::Sender(20) }) + ); } #[test] fn mortal_signed_check_should_work() { - let ux = Ex::new_signed(0, vec![0u8;0], DUMMY_ACCOUNTID, TestSig(DUMMY_ACCOUNTID, (Compact::from(DUMMY_ACCOUNTID), vec![0u8;0], Era::mortal(32, 42), 42u64).encode()), Era::mortal(32, 42)); + let ux = Ex::new_signed( + 0, + vec![0u8;0], + DUMMY_ACCOUNTID, + TestSig( + DUMMY_ACCOUNTID, + (Compact::from(DUMMY_ACCOUNTID), + vec![0u8;0], + Era::mortal(32, 42), 42u64).encode() + ), + Era::mortal(32, 42), + Tip::default(), + ); assert!(ux.is_signed().unwrap_or(false)); - assert_eq!(>::check(ux, &TestContext), Ok(CEx { signed: Some((DUMMY_ACCOUNTID, 0)), function: vec![0u8;0] })); + assert_eq!( + >::check(ux, &TestContext), + Ok(CEx { signed: Some((DUMMY_ACCOUNTID, 0)), function: vec![0u8;0], tip: Tip::default() })); } #[test] fn later_mortal_signed_check_should_work() { - let ux = Ex::new_signed(0, vec![0u8;0], DUMMY_ACCOUNTID, TestSig(DUMMY_ACCOUNTID, (Compact::from(DUMMY_ACCOUNTID), vec![0u8;0], Era::mortal(32, 11), 11u64).encode()), Era::mortal(32, 11)); + let ux = Ex::new_signed( + 0, + vec![0u8;0], + DUMMY_ACCOUNTID, + TestSig( + DUMMY_ACCOUNTID, + (Compact::from(DUMMY_ACCOUNTID), vec![0u8;0], Era::mortal(32, 11), 11u64).encode() + ), + Era::mortal(32, 11), + Tip::default(), + ); assert!(ux.is_signed().unwrap_or(false)); - assert_eq!(>::check(ux, &TestContext), Ok(CEx { signed: Some((DUMMY_ACCOUNTID, 0)), function: vec![0u8;0] })); + assert_eq!( + >::check(ux, &TestContext), + Ok(CEx { signed: Some((DUMMY_ACCOUNTID, 0)), function: vec![0u8;0], tip: Tip::default() }) + ); } #[test] fn too_late_mortal_signed_check_should_fail() { - let ux = Ex::new_signed(0, vec![0u8;0], DUMMY_ACCOUNTID, TestSig(DUMMY_ACCOUNTID, (DUMMY_ACCOUNTID, vec![0u8;0], Era::mortal(32, 10), 10u64).encode()), Era::mortal(32, 10)); + let ux = Ex::new_signed( + 0, + vec![0u8;0], + DUMMY_ACCOUNTID, + TestSig( + DUMMY_ACCOUNTID, + (DUMMY_ACCOUNTID, vec![0u8;0], Era::mortal(32, 10), 10u64).encode() + ), + Era::mortal(32, 10), + Tip::default(), + ); assert!(ux.is_signed().unwrap_or(false)); assert_eq!(>::check(ux, &TestContext), Err(crate::BAD_SIGNATURE)); } #[test] fn too_early_mortal_signed_check_should_fail() { - let ux = Ex::new_signed(0, vec![0u8;0], DUMMY_ACCOUNTID, TestSig(DUMMY_ACCOUNTID, (DUMMY_ACCOUNTID, vec![0u8;0], Era::mortal(32, 43), 43u64).encode()), Era::mortal(32, 43)); + let ux = Ex::new_signed( + 0, + vec![0u8;0], + DUMMY_ACCOUNTID, + TestSig( + DUMMY_ACCOUNTID, + (DUMMY_ACCOUNTID, vec![0u8;0], Era::mortal(32, 43), 43u64).encode() + ), + Era::mortal(32, 43), + Tip::default(), + ); assert!(ux.is_signed().unwrap_or(false)); assert_eq!(>::check(ux, &TestContext), Err(crate::BAD_SIGNATURE)); } #[test] fn encoding_matches_vec() { - let ex = Ex::new_unsigned(vec![0u8;0]); + let ex = Ex::new_unsigned(vec![0u8;0], Tip::Sender(10)); let encoded = ex.encode(); let decoded = Ex::decode(&mut encoded.as_slice()).unwrap(); assert_eq!(decoded, ex); diff --git a/core/sr-primitives/src/generic/unchecked_mortal_extrinsic.rs b/core/sr-primitives/src/generic/unchecked_mortal_extrinsic.rs index 7f92b20edd0c3..538658f04fd71 100644 --- a/core/sr-primitives/src/generic/unchecked_mortal_extrinsic.rs +++ b/core/sr-primitives/src/generic/unchecked_mortal_extrinsic.rs @@ -27,47 +27,67 @@ use crate::traits::{ Lookup, Checkable, Extrinsic, SaturatedConversion }; use super::{CheckedExtrinsic, Era}; +use super::Tip; const TRANSACTION_VERSION: u8 = 1; /// A extrinsic right from the external world. This is unchecked and so /// can contain a signature. #[derive(PartialEq, Eq, Clone)] -pub struct UncheckedMortalExtrinsic { +pub struct UncheckedMortalExtrinsic { /// The signature, address, number of extrinsics have come before from /// the same signer and an era describing the longevity of this transaction, /// if this is a signed extrinsic. pub signature: Option<(Address, Signature, Index, Era)>, /// The function that should be called. pub function: Call, + /// The tip for this transaction + pub tip: Tip, } -impl UncheckedMortalExtrinsic { +impl< + Address, + Index, + Call, + Signature, + Balance +> UncheckedMortalExtrinsic { /// New instance of a signed extrinsic aka "transaction". - pub fn new_signed(index: Index, function: Call, signed: Address, signature: Signature, era: Era) -> Self { + pub fn new_signed( + index: Index, + function: Call, + signed: Address, + signature: Signature, + era: Era, + tip: Tip + ) -> Self { UncheckedMortalExtrinsic { signature: Some((signed, signature, index, era)), function, + tip, } } /// New instance of an unsigned extrinsic aka "inherent". - pub fn new_unsigned(function: Call) -> Self { + pub fn new_unsigned(function: Call, tip: Tip) -> Self { UncheckedMortalExtrinsic { signature: None, function, + tip, } } } -impl Extrinsic for UncheckedMortalExtrinsic { +impl Extrinsic + for UncheckedMortalExtrinsic +{ fn is_signed(&self) -> Option { Some(self.signature.is_some()) } } -impl Checkable - for UncheckedMortalExtrinsic +impl Checkable + for UncheckedMortalExtrinsic where Address: Member + MaybeDisplay, Index: Encode + Member + MaybeDisplay + SimpleArithmetic, @@ -80,7 +100,7 @@ where + CurrentHeight + BlockNumberToHash, { - type Checked = CheckedExtrinsic; + type Checked = CheckedExtrinsic; fn check(self, context: &Context) -> Result { Ok(match self.signature { @@ -103,23 +123,26 @@ where CheckedExtrinsic { signed: Some((signed, raw_payload.0)), function: raw_payload.1, + tip: self.tip, } } None => CheckedExtrinsic { signed: None, function: self.function, + tip: self.tip, }, }) } } -impl Decode - for UncheckedMortalExtrinsic +impl Decode + for UncheckedMortalExtrinsic where Address: Decode, Signature: Decode, Index: Decode, Call: Decode, + Balance: Decode, { fn decode(input: &mut I) -> Option { // This is a little more complicated than usual since the binary format must be compatible @@ -139,17 +162,19 @@ where Some(UncheckedMortalExtrinsic { signature: if is_signed { Some(Decode::decode(input)?) } else { None }, function: Decode::decode(input)?, + tip: Decode::decode(input).unwrap_or_default(), }) } } -impl Encode - for UncheckedMortalExtrinsic +impl Encode + for UncheckedMortalExtrinsic where Address: Encode, Signature: Encode, Index: Encode, Call: Encode, + Balance: Encode, { fn encode(&self) -> Vec { super::encode_with_vec_prefix::(|v| { @@ -164,13 +189,14 @@ where } } self.function.encode_to(v); + self.tip.encode_to(v); }) } } #[cfg(feature = "std")] -impl serde::Serialize - for UncheckedMortalExtrinsic +impl serde::Serialize + for UncheckedMortalExtrinsic { fn serialize(&self, seq: S) -> Result where S: ::serde::Serializer { self.using_encoded(|bytes| seq.serialize_bytes(bytes)) @@ -178,13 +204,22 @@ impl serde::Ser } #[cfg(feature = "std")] -impl fmt::Debug for UncheckedMortalExtrinsic where +impl fmt::Debug + for UncheckedMortalExtrinsic +where Address: fmt::Debug, Index: fmt::Debug, Call: fmt::Debug, + Balance: fmt::Debug { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - write!(f, "UncheckedMortalExtrinsic({:?}, {:?})", self.signature.as_ref().map(|x| (&x.0, &x.2)), self.function) + write!( + f, + "UncheckedMortalExtrinsic({:?}, {:?} ,{:?})", + self.signature.as_ref().map(|x| (&x.0, &x.2)), + self.function, + self.tip, + ) } } @@ -221,83 +256,153 @@ mod tests { } const DUMMY_ACCOUNTID: u64 = 0; + type Balance = u64; - type Ex = UncheckedMortalExtrinsic, TestSig>; - type CEx = CheckedExtrinsic>; + type Ex = UncheckedMortalExtrinsic, TestSig, Balance>; + type CEx = CheckedExtrinsic, Balance>; #[test] fn unsigned_codec_should_work() { - let ux = Ex::new_unsigned(vec![0u8;0]); + let ux = Ex::new_unsigned(vec![0u8;0], Tip::Sender(10)); let encoded = ux.encode(); assert_eq!(Ex::decode(&mut &encoded[..]), Some(ux)); } #[test] fn signed_codec_should_work() { - let ux = Ex::new_signed(0, vec![0u8;0], DUMMY_ACCOUNTID, TestSig(DUMMY_ACCOUNTID, (DUMMY_ACCOUNTID, vec![0u8;0], Era::immortal(), 0u64).encode()), Era::immortal()); + let ux = Ex::new_signed( + 0, + vec![0u8;0], + DUMMY_ACCOUNTID, + TestSig(DUMMY_ACCOUNTID, (DUMMY_ACCOUNTID, vec![0u8;0], Era::immortal(), 0u64).encode()), + Era::immortal(), + Tip::default(), + ); let encoded = ux.encode(); assert_eq!(Ex::decode(&mut &encoded[..]), Some(ux)); } #[test] fn large_signed_codec_should_work() { - let ux = Ex::new_signed(0, vec![0u8;0], DUMMY_ACCOUNTID, TestSig(DUMMY_ACCOUNTID, (DUMMY_ACCOUNTID, vec![0u8; 257], Era::immortal(), 0u64).using_encoded(blake2_256)[..].to_owned()), Era::immortal()); + let ux = Ex::new_signed( + 0, + vec![0u8;0], + DUMMY_ACCOUNTID, + TestSig( + DUMMY_ACCOUNTID, + (DUMMY_ACCOUNTID, vec![0u8; 257], Era::immortal(), 0u64) + .using_encoded(blake2_256)[..].to_owned() + ), + Era::immortal(), + Tip::default(), + ); let encoded = ux.encode(); assert_eq!(Ex::decode(&mut &encoded[..]), Some(ux)); } #[test] fn unsigned_check_should_work() { - let ux = Ex::new_unsigned(vec![0u8;0]); + let ux = Ex::new_unsigned(vec![0u8;0], Tip::default()); assert!(!ux.is_signed().unwrap_or(false)); assert!(>::check(ux, &TestContext).is_ok()); } #[test] fn badly_signed_check_should_fail() { - let ux = Ex::new_signed(0, vec![0u8;0], DUMMY_ACCOUNTID, TestSig(DUMMY_ACCOUNTID, vec![0u8]), Era::immortal()); + let ux = Ex::new_signed( + 0, + vec![0u8;0], + DUMMY_ACCOUNTID, + TestSig(DUMMY_ACCOUNTID, vec![0u8]), + Era::immortal(), + Tip::default(), + ); assert!(ux.is_signed().unwrap_or(false)); assert_eq!(>::check(ux, &TestContext), Err(crate::BAD_SIGNATURE)); } #[test] fn immortal_signed_check_should_work() { - let ux = Ex::new_signed(0, vec![0u8;0], DUMMY_ACCOUNTID, TestSig(DUMMY_ACCOUNTID, (DUMMY_ACCOUNTID, vec![0u8;0], Era::immortal(), 0u64).encode()), Era::immortal()); + let ux = Ex::new_signed( + 0, + vec![0u8;0], + DUMMY_ACCOUNTID, + TestSig(DUMMY_ACCOUNTID, (DUMMY_ACCOUNTID, vec![0u8;0], Era::immortal(), 0u64).encode()), + Era::immortal(), + Tip::default(), + ); assert!(ux.is_signed().unwrap_or(false)); - assert_eq!(>::check(ux, &TestContext), Ok(CEx { signed: Some((DUMMY_ACCOUNTID, 0)), function: vec![0u8;0] })); + assert_eq!( + >::check(ux, &TestContext), + Ok(CEx { signed: Some((DUMMY_ACCOUNTID, 0)), function: vec![0u8;0], tip: Tip::default() }) + ); } #[test] fn mortal_signed_check_should_work() { - let ux = Ex::new_signed(0, vec![0u8;0], DUMMY_ACCOUNTID, TestSig(DUMMY_ACCOUNTID, (DUMMY_ACCOUNTID, vec![0u8;0], Era::mortal(32, 42), 42u64).encode()), Era::mortal(32, 42)); + let ux = Ex::new_signed( + 0, + vec![0u8;0], + DUMMY_ACCOUNTID, + TestSig(DUMMY_ACCOUNTID, (DUMMY_ACCOUNTID, vec![0u8;0], Era::mortal(32, 42), 42u64).encode()), + Era::mortal(32, 42), + Tip::default(), + ); assert!(ux.is_signed().unwrap_or(false)); - assert_eq!(>::check(ux, &TestContext), Ok(CEx { signed: Some((DUMMY_ACCOUNTID, 0)), function: vec![0u8;0] })); + assert_eq!( + >::check(ux, &TestContext), + Ok(CEx { signed: Some((DUMMY_ACCOUNTID, 0)), function: vec![0u8;0], tip: Tip::default()}) + ); } #[test] fn later_mortal_signed_check_should_work() { - let ux = Ex::new_signed(0, vec![0u8;0], DUMMY_ACCOUNTID, TestSig(DUMMY_ACCOUNTID, (DUMMY_ACCOUNTID, vec![0u8;0], Era::mortal(32, 11), 11u64).encode()), Era::mortal(32, 11)); + let ux = Ex::new_signed( + 0, + vec![0u8;0], + DUMMY_ACCOUNTID, + TestSig(DUMMY_ACCOUNTID, (DUMMY_ACCOUNTID, vec![0u8;0], Era::mortal(32, 11), 11u64).encode()), + Era::mortal(32, 11), + Tip::default(), + ); assert!(ux.is_signed().unwrap_or(false)); - assert_eq!(>::check(ux, &TestContext), Ok(CEx { signed: Some((DUMMY_ACCOUNTID, 0)), function: vec![0u8;0] })); + assert_eq!( + >::check(ux, &TestContext), + Ok(CEx { signed: Some((DUMMY_ACCOUNTID, 0)), function: vec![0u8;0], tip: Tip::default() }) + ); } #[test] fn too_late_mortal_signed_check_should_fail() { - let ux = Ex::new_signed(0, vec![0u8;0], DUMMY_ACCOUNTID, TestSig(DUMMY_ACCOUNTID, (DUMMY_ACCOUNTID, vec![0u8;0], Era::mortal(32, 10), 10u64).encode()), Era::mortal(32, 10)); + let ux = Ex::new_signed( + 0, + vec![0u8;0], + DUMMY_ACCOUNTID, + TestSig(DUMMY_ACCOUNTID, (DUMMY_ACCOUNTID, vec![0u8;0], Era::mortal(32, 10), 10u64).encode()), + Era::mortal(32, 10), + Tip::default(), + ); assert!(ux.is_signed().unwrap_or(false)); assert_eq!(>::check(ux, &TestContext), Err(crate::BAD_SIGNATURE)); } #[test] fn too_early_mortal_signed_check_should_fail() { - let ux = Ex::new_signed(0, vec![0u8;0], DUMMY_ACCOUNTID, TestSig(DUMMY_ACCOUNTID, (DUMMY_ACCOUNTID, vec![0u8;0], Era::mortal(32, 43), 43u64).encode()), Era::mortal(32, 43)); + let ux = Ex::new_signed( + 0, + vec![0u8;0], + DUMMY_ACCOUNTID, + TestSig(DUMMY_ACCOUNTID, (DUMMY_ACCOUNTID, vec![0u8;0], Era::mortal(32, 43), 43u64).encode()), + Era::mortal(32, 43), + Tip::default(), + ); assert!(ux.is_signed().unwrap_or(false)); assert_eq!(>::check(ux, &TestContext), Err(crate::BAD_SIGNATURE)); } #[test] fn encoding_matches_vec() { - let ex = Ex::new_unsigned(vec![0u8;0]); + let ex = Ex::new_unsigned(vec![0u8;0], Tip::Sender(69)); let encoded = ex.encode(); let decoded = Ex::decode(&mut encoded.as_slice()).unwrap(); assert_eq!(decoded, ex); From d36dc9a27c1e0a387cc119b0c590a40743471fd8 Mon Sep 17 00:00:00 2001 From: Kian Paimani Date: Wed, 19 Jun 2019 18:00:53 +0200 Subject: [PATCH 02/23] Fix local tests --- .../src/generic/unchecked_extrinsic.rs | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/core/sr-primitives/src/generic/unchecked_extrinsic.rs b/core/sr-primitives/src/generic/unchecked_extrinsic.rs index c347c1e6d120d..dae8aa73bcdc1 100644 --- a/core/sr-primitives/src/generic/unchecked_extrinsic.rs +++ b/core/sr-primitives/src/generic/unchecked_extrinsic.rs @@ -212,10 +212,16 @@ mod test { #[test] fn unprovided_tip_will_not_fail() { - let ex = Extrinsic::new_unsigned(42, Tip::Sender(10_u32)); - let encoded = ex.encode(); - println!("This is it {:?}", encoded); - unimplemented!(); + // without tip + // len sig f(u32)..... + let bytes: Vec = vec![24, 0, 42, 0, 0, 0]; + let decoded: Extrinsic = Decode::decode(&mut bytes.as_slice()).unwrap(); + assert_eq!(decoded, Extrinsic::new_unsigned(42, Tip::default())); + // with tip + let bytes: Vec = vec![40, 0, 42, 0, 0, 0, 1, 10, 0, 0, 0]; + let decoded: Extrinsic = Decode::decode(&mut bytes.as_slice()).unwrap(); + assert_eq!(decoded, Extrinsic::new_unsigned(42, Tip::Sender(10))); + } #[test] @@ -223,7 +229,6 @@ mod test { fn serialization_of_unchecked_extrinsics() { type Extrinsic = UncheckedExtrinsic; let ex = Extrinsic::new_unsigned(42, Tip::None); - - assert_eq!(serde_json::to_string(&ex).unwrap(), "\"0x14002a000000\""); + assert_eq!(serde_json::to_string(&ex).unwrap(), "\"0x18002a00000000\""); } } From a18be8cd010fe7afc71ad0cbce7bc70cab82cec1 Mon Sep 17 00:00:00 2001 From: kianenigma Date: Sat, 22 Jun 2019 10:47:27 +0200 Subject: [PATCH 03/23] Builds. --- core/sr-primitives/src/generic/tip.rs | 3 +-- node-template/runtime/src/lib.rs | 4 ++-- node/cli/src/factory_impl.rs | 8 ++++++-- node/runtime/src/lib.rs | 4 ++-- srml/support/src/inherent.rs | 5 ++++- srml/support/test/tests/instance.rs | 2 +- 6 files changed, 16 insertions(+), 10 deletions(-) diff --git a/core/sr-primitives/src/generic/tip.rs b/core/sr-primitives/src/generic/tip.rs index e60a879aa2f5a..1d671261aed00 100644 --- a/core/sr-primitives/src/generic/tip.rs +++ b/core/sr-primitives/src/generic/tip.rs @@ -24,8 +24,7 @@ use crate::codec::{Encode, Decode}; /// stream. /// If non-existent, the default implementation will be used.s #[derive(Clone, Eq, PartialEq, Encode, Decode)] -#[cfg(feature = "std")] -#[derive(Debug)] +#[cfg_attr(feature = "std", derive(Debug))] pub enum Tip { /// This transaction does not include any tips. None, diff --git a/node-template/runtime/src/lib.rs b/node-template/runtime/src/lib.rs index 9b99e7f08f495..55a4df68bda49 100644 --- a/node-template/runtime/src/lib.rs +++ b/node-template/runtime/src/lib.rs @@ -205,9 +205,9 @@ pub type Block = generic::Block; /// BlockId type as expected by this runtime. pub type BlockId = generic::BlockId; /// Unchecked extrinsic type as expected by this runtime. -pub type UncheckedExtrinsic = generic::UncheckedMortalCompactExtrinsic; +pub type UncheckedExtrinsic = generic::UncheckedMortalCompactExtrinsic; /// Extrinsic type that has already been checked. -pub type CheckedExtrinsic = generic::CheckedExtrinsic; +pub type CheckedExtrinsic = generic::CheckedExtrinsic; /// Executive: handles dispatch to the various modules. pub type Executive = executive::Executive; diff --git a/node/cli/src/factory_impl.rs b/node/cli/src/factory_impl.rs index 0d94610362fdc..423a827f2ac54 100644 --- a/node/cli/src/factory_impl.rs +++ b/node/cli/src/factory_impl.rs @@ -28,7 +28,7 @@ use node_runtime::{Call, CheckedExtrinsic, UncheckedExtrinsic, BalancesCall}; use primitives::sr25519; use primitives::crypto::Pair; use parity_codec::Encode; -use sr_primitives::generic::Era; +use sr_primitives::generic::{Era, Tip}; use sr_primitives::traits::{Block as BlockT, Header as HeaderT}; use substrate_service::ServiceFactory; use transaction_factory::RuntimeAdapter; @@ -54,6 +54,7 @@ pub struct FactoryState { type Number = <::Header as HeaderT>::Number; +// TODO: fix the tip amount here. impl RuntimeAdapter for FactoryState { type AccountId = node_primitives::AccountId; type Balance = node_primitives::Balance; @@ -140,7 +141,8 @@ impl RuntimeAdapter for FactoryState { ), (*amount).into() ) - ) + ), + tip: Tip::default(), }, key, &prior_block_hash, phase) } @@ -246,11 +248,13 @@ fn sign( UncheckedExtrinsic { signature: Some((indices::address::Address::Id(signed), signature, payload.0, era)), function: payload.1, + tip: Tip::default(), } } None => UncheckedExtrinsic { signature: None, function: xt.function, + tip: Tip::default(), }, }; diff --git a/node/runtime/src/lib.rs b/node/runtime/src/lib.rs index 67a5b450a95bb..5c96652cb8740 100644 --- a/node/runtime/src/lib.rs +++ b/node/runtime/src/lib.rs @@ -277,9 +277,9 @@ pub type SignedBlock = generic::SignedBlock; /// BlockId type as expected by this runtime. pub type BlockId = generic::BlockId; /// Unchecked extrinsic type as expected by this runtime. -pub type UncheckedExtrinsic = generic::UncheckedMortalCompactExtrinsic; +pub type UncheckedExtrinsic = generic::UncheckedMortalCompactExtrinsic; /// Extrinsic type that has already been checked. -pub type CheckedExtrinsic = generic::CheckedExtrinsic; +pub type CheckedExtrinsic = generic::CheckedExtrinsic; /// Executive: handles dispatch to the various modules. pub type Executive = executive::Executive, Balances, Runtime, AllModules>; diff --git a/srml/support/src/inherent.rs b/srml/support/src/inherent.rs index d886abbca7e37..f4498a561cdf8 100644 --- a/srml/support/src/inherent.rs +++ b/srml/support/src/inherent.rs @@ -20,6 +20,8 @@ pub use crate::rstd::vec::Vec; pub use crate::runtime_primitives::traits::{Block as BlockT, Extrinsic}; #[doc(hidden)] pub use inherents::{InherentData, ProvideInherent, CheckInherentsResult, IsFatalError}; +#[doc(hidden)] +pub use crate::runtime_primitives::generic::Tip; /// Implement the outer inherent. @@ -58,10 +60,11 @@ macro_rules! impl_outer_inherent { let mut inherents = Vec::new(); + // TODO: fix tip value here. $( if let Some(inherent) = $module::create_inherent(self) { inherents.push($uncheckedextrinsic::new_unsigned( - Call::$call(inherent)) + Call::$call(inherent), $crate::inherent::Tip::default()) ); } )* diff --git a/srml/support/test/tests/instance.rs b/srml/support/test/tests/instance.rs index f7b4a4bd3a251..665db5b8edbe0 100644 --- a/srml/support/test/tests/instance.rs +++ b/srml/support/test/tests/instance.rs @@ -294,7 +294,7 @@ srml_support::construct_runtime!( pub type Header = generic::Header; pub type Block = generic::Block; -pub type UncheckedExtrinsic = generic::UncheckedMortalCompactExtrinsic; +pub type UncheckedExtrinsic = generic::UncheckedMortalCompactExtrinsic; fn new_test_ext() -> runtime_io::TestExternalities { GenesisConfig{ From 55d39cf1faf29e73b2e1da122955d1ea9e303538 Mon Sep 17 00:00:00 2001 From: kianenigma Date: Sat, 22 Jun 2019 11:33:28 +0200 Subject: [PATCH 04/23] Fix test build. --- node/cli/src/service.rs | 4 +++- node/executor/src/lib.rs | 19 ++++++++++++++++++- subkey/src/main.rs | 4 +++- 3 files changed, 24 insertions(+), 3 deletions(-) diff --git a/node/cli/src/service.rs b/node/cli/src/service.rs index 058f58c9c2c7e..ffaf00a44fbf7 100644 --- a/node/cli/src/service.rs +++ b/node/cli/src/service.rs @@ -226,7 +226,7 @@ mod tests { crypto::Pair as CryptoPair, ed25519::Pair, blake2_256, sr25519::Public as AddressPublic, H256, }; - use sr_primitives::{generic::{BlockId, Era, Digest}, traits::Block, OpaqueExtrinsic}; + use sr_primitives::{generic::{BlockId, Era, Digest, Tip}, traits::Block, OpaqueExtrinsic}; use timestamp; use finality_tracker; use keyring::{ed25519::Keyring as AuthorityKeyring, sr25519::Keyring as AccountKeyring}; @@ -273,6 +273,7 @@ mod tests { let xt = UncheckedExtrinsic { signature: Some((RawAddress::Id(id), signature, payload.0, Era::immortal())), function: payload.1, + tip: Tip::default(), }.encode(); let v: Vec = Decode::decode(&mut xt.as_slice()).unwrap(); OpaqueExtrinsic(v) @@ -356,6 +357,7 @@ mod tests { from.into(), signature.into(), era, + Tip::default(), ).encode(); let v: Vec = Decode::decode(&mut xt.as_slice()).unwrap(); diff --git a/node/executor/src/lib.rs b/node/executor/src/lib.rs index 4f0c38d88482a..2ac6b891bbe82 100644 --- a/node/executor/src/lib.rs +++ b/node/executor/src/lib.rs @@ -38,7 +38,7 @@ mod tests { NativeOrEncoded}; use node_primitives::{Hash, BlockNumber, AccountId}; use runtime_primitives::traits::{Header as HeaderT, Hash as HashT}; - use runtime_primitives::{generic::Era, ApplyOutcome, ApplyError, ApplyResult, Perbill}; + use runtime_primitives::{generic::{Era, Tip}, ApplyOutcome, ApplyError, ApplyResult, Perbill}; use {balances, indices, system, staking, timestamp, treasury, contract}; use contract::ContractAddressFor; use system::{EventRecord, Phase}; @@ -94,11 +94,13 @@ mod tests { UncheckedExtrinsic { signature: Some((indices::address::Address::Id(signed), signature, payload.0, era)), function: payload.1, + tip: Tip::default(), } } None => UncheckedExtrinsic { signature: None, function: xt.function, + tip: Tip::default() }, } } @@ -107,6 +109,7 @@ mod tests { sign(CheckedExtrinsic { signed: Some((alice(), 0)), function: Call::Balances(balances::Call::transfer::(bob().into(), 69)), + tip: Tip::default() }) } @@ -433,6 +436,7 @@ mod tests { (Block { header, extrinsics }.encode(), hash.into()) } + /// TODO: refactor these a bit more. fn changes_trie_block() -> (Vec, Hash) { construct_block( &mut new_test_ext(COMPACT_CODE, true), @@ -442,10 +446,12 @@ mod tests { CheckedExtrinsic { signed: None, function: Call::Timestamp(timestamp::Call::set(42)), + tip: Tip::default() }, CheckedExtrinsic { signed: Some((alice(), 0)), function: Call::Balances(balances::Call::transfer(bob().into(), 69)), + tip: Tip::default() }, ] ) @@ -464,10 +470,12 @@ mod tests { CheckedExtrinsic { signed: None, function: Call::Timestamp(timestamp::Call::set(42)), + tip: Tip::default() }, CheckedExtrinsic { signed: Some((alice(), 0)), function: Call::Balances(balances::Call::transfer(bob().into(), 69)), + tip: Tip::default() }, ] ); @@ -479,14 +487,17 @@ mod tests { CheckedExtrinsic { signed: None, function: Call::Timestamp(timestamp::Call::set(52)), + tip: Tip::default() }, CheckedExtrinsic { signed: Some((bob(), 0)), function: Call::Balances(balances::Call::transfer(alice().into(), 5)), + tip: Tip::default() }, CheckedExtrinsic { signed: Some((alice(), 1)), function: Call::Balances(balances::Call::transfer(bob().into(), 15)), + tip: Tip::default() } ] ); @@ -508,10 +519,12 @@ mod tests { CheckedExtrinsic { signed: None, function: Call::Timestamp(timestamp::Call::set(42)), + tip: Tip::default() }, CheckedExtrinsic { signed: Some((alice(), 0)), function: Call::System(system::Call::remark(vec![0; 120000])), + tip: Tip::default() } ] ) @@ -778,24 +791,28 @@ mod tests { CheckedExtrinsic { signed: None, function: Call::Timestamp(timestamp::Call::set(42)), + tip: Tip::default() }, CheckedExtrinsic { signed: Some((charlie(), 0)), function: Call::Contract( contract::Call::put_code::(10_000, transfer_code) ), + tip: Tip::default() }, CheckedExtrinsic { signed: Some((charlie(), 1)), function: Call::Contract( contract::Call::create::(10, 10_000, transfer_ch, Vec::new()) ), + tip: Tip::default() }, CheckedExtrinsic { signed: Some((charlie(), 2)), function: Call::Contract( contract::Call::call::(indices::address::Address::Id(addr.clone()), 10, 10_000, vec![0x00, 0x01, 0x02, 0x03]) ), + tip: Tip::default() }, ] ); diff --git a/subkey/src/main.rs b/subkey/src/main.rs index b38bffd772bbf..99d6cbf714bff 100644 --- a/subkey/src/main.rs +++ b/subkey/src/main.rs @@ -24,7 +24,7 @@ use clap::load_yaml; use bip39::{Mnemonic, Language, MnemonicType}; use substrate_primitives::{ed25519, sr25519, hexdisplay::HexDisplay, Pair, crypto::Ss58Codec, blake2_256}; use parity_codec::{Encode, Decode, Compact}; -use sr_primitives::generic::Era; +use sr_primitives::generic::{Era, Tip}; use node_primitives::{Balance, Index, Hash}; use node_runtime::{Call, UncheckedExtrinsic, BalancesCall}; @@ -166,6 +166,7 @@ fn execute(matches: clap::ArgMatches) where signer.public().into(), signature.into(), era, + Tip::default(), ); println!("0x{}", hex::encode(&extrinsic.encode())); } @@ -207,6 +208,7 @@ fn execute(matches: clap::ArgMatches) where signer.public().into(), signature.into(), era, + Tip::default(), ); println!("0x{}", hex::encode(&extrinsic.encode())); From a4e28163a3b1bcd02ab4a3e5e81eff2c8d071c1c Mon Sep 17 00:00:00 2001 From: kianenigma Date: Sun, 23 Jun 2019 12:12:00 +0200 Subject: [PATCH 05/23] Tratify tip. --- .../src/generic/checked_extrinsic.rs | 2 +- core/sr-primitives/src/generic/mod.rs | 6 ++--- core/sr-primitives/src/generic/tip.rs | 10 ++++++-- .../src/generic/unchecked_extrinsic.rs | 23 ++++++++++++++---- .../unchecked_mortal_compact_extrinsic.rs | 17 +++++++++++-- core/sr-primitives/src/testing.rs | 5 ++++ node-template/runtime/src/lib.rs | 2 +- node/runtime/src/lib.rs | 2 +- srml/executive/src/lib.rs | 24 ++++++++++++------- 9 files changed, 66 insertions(+), 25 deletions(-) diff --git a/core/sr-primitives/src/generic/checked_extrinsic.rs b/core/sr-primitives/src/generic/checked_extrinsic.rs index a1ac2b30a2087..3786112218b0c 100644 --- a/core/sr-primitives/src/generic/checked_extrinsic.rs +++ b/core/sr-primitives/src/generic/checked_extrinsic.rs @@ -17,7 +17,7 @@ //! Generic implementation of an extrinsic that has passed the verification //! stage. -use rstd::marker::{Sync, Send}; // TODO: what the fucking hell. +use rstd::marker::{Sync, Send}; // TODO: where did this come from? use crate::traits::{self, Member, SimpleArithmetic, MaybeDisplay}; use crate::weights::{Weighable, Weight}; use super::Tip; diff --git a/core/sr-primitives/src/generic/mod.rs b/core/sr-primitives/src/generic/mod.rs index 0e9136e81e2aa..4b4b1a182cd60 100644 --- a/core/sr-primitives/src/generic/mod.rs +++ b/core/sr-primitives/src/generic/mod.rs @@ -37,10 +37,8 @@ pub use self::era::{Era, Phase}; pub use self::checked_extrinsic::CheckedExtrinsic; pub use self::header::Header; pub use self::block::{Block, SignedBlock, BlockId}; -pub use self::digest::{ - Digest, DigestItem, DigestItemRef, OpaqueDigestItemId -}; -pub use self::tip::Tip; +pub use self::digest::{Digest, DigestItem, DigestItemRef, OpaqueDigestItemId}; +pub use self::tip::{Tip, Tippable}; use crate::codec::Encode; use rstd::prelude::*; diff --git a/core/sr-primitives/src/generic/tip.rs b/core/sr-primitives/src/generic/tip.rs index 1d671261aed00..2672c5291ba17 100644 --- a/core/sr-primitives/src/generic/tip.rs +++ b/core/sr-primitives/src/generic/tip.rs @@ -17,14 +17,13 @@ //! Tip structure for a transaction. use crate::codec::{Encode, Decode}; -// use crate::traits::SimpleArithmetic; /// Representation of a transaction tip. /// Upon decoding, all transaction types try and decode this from the end of the encoded byte /// stream. /// If non-existent, the default implementation will be used.s -#[derive(Clone, Eq, PartialEq, Encode, Decode)] #[cfg_attr(feature = "std", derive(Debug))] +#[derive(Clone, Copy, Eq, PartialEq, Encode, Decode)] pub enum Tip { /// This transaction does not include any tips. None, @@ -32,8 +31,15 @@ pub enum Tip { Sender(Balance), } +/// Default implementation for tip. impl Default for Tip { fn default() -> Self { Tip::None } } + +/// A trait for a generic transaction that contains a tip. The tip itself migth yeild something +/// that translates to "no tip". +pub trait Tippable { + fn tip(&self) -> Tip; +} \ No newline at end of file diff --git a/core/sr-primitives/src/generic/unchecked_extrinsic.rs b/core/sr-primitives/src/generic/unchecked_extrinsic.rs index dae8aa73bcdc1..6a03803f8379e 100644 --- a/core/sr-primitives/src/generic/unchecked_extrinsic.rs +++ b/core/sr-primitives/src/generic/unchecked_extrinsic.rs @@ -15,14 +15,13 @@ // along with Substrate. If not, see . //! Generic implementation of an unchecked (pre-verification) extrinsic. - #[cfg(feature = "std")] use std::fmt; use rstd::prelude::*; use crate::codec::{Decode, Encode, Codec, Input, HasCompact}; use crate::traits::{self, Member, SimpleArithmetic, MaybeDisplay, Lookup, Extrinsic}; -use super::{CheckedExtrinsic, Tip}; +use super::{CheckedExtrinsic, Tip, Tippable}; #[derive(PartialEq, Eq, Clone, Encode, Decode)] pub struct SignatureContent @@ -59,7 +58,7 @@ where Address: Codec, Index: HasCompact + Codec, Signature: Codec, - Balance: Codec, + // Balance: Codec, { /// New instance of a signed extrinsic aka "transaction". pub fn new_signed(index: Index, @@ -94,7 +93,7 @@ where Signature: Member + traits::Verify + Codec, AccountId: Member + MaybeDisplay, Context: Lookup, - Balance: Codec, + // Balance: Codec, { type Checked = CheckedExtrinsic; @@ -121,12 +120,26 @@ where } } +// TODO: remove not needed encode/decode bounds for balance +impl Tippable + for UncheckedExtrinsic +where + Index: HasCompact + Codec, + Signature: Codec, + Address: Codec, + Balance: Clone, +{ + fn tip(&self) -> Tip { + self.tip.clone() + } +} + impl< Address: Codec, Index: HasCompact + Codec, Signature: Codec, Call, - Balance: Codec, + Balance, > Extrinsic for UncheckedExtrinsic { fn is_signed(&self) -> Option { Some(self.signature.is_some()) diff --git a/core/sr-primitives/src/generic/unchecked_mortal_compact_extrinsic.rs b/core/sr-primitives/src/generic/unchecked_mortal_compact_extrinsic.rs index a0410cb903d4e..dde4d5241555c 100644 --- a/core/sr-primitives/src/generic/unchecked_mortal_compact_extrinsic.rs +++ b/core/sr-primitives/src/generic/unchecked_mortal_compact_extrinsic.rs @@ -21,10 +21,10 @@ use std::fmt; use rstd::prelude::*; use runtime_io::blake2_256; -use crate::codec::{Decode, Encode, Input, Compact}; +use crate::codec::{Decode, Encode, Codec, HasCompact, Input, Compact}; use crate::traits::{self, Member, SimpleArithmetic, MaybeDisplay, CurrentHeight, BlockNumberToHash, Lookup, Checkable, Extrinsic, SaturatedConversion}; -use super::{CheckedExtrinsic, Era, Tip}; +use super::{CheckedExtrinsic, Era, Tip, Tippable}; const TRANSACTION_VERSION: u8 = 1; @@ -128,6 +128,19 @@ where } } +impl Tippable + for UncheckedMortalCompactExtrinsic +where + // Index: HasCompact + Codec, + // Signature: Codec, + // Address: Codec, + Balance: Clone, +{ + fn tip(&self) -> Tip { + self.tip.clone() + } +} + impl Decode for UncheckedMortalCompactExtrinsic where diff --git a/core/sr-primitives/src/testing.rs b/core/sr-primitives/src/testing.rs index 35f3ec476f6d5..54d1b569c9f92 100644 --- a/core/sr-primitives/src/testing.rs +++ b/core/sr-primitives/src/testing.rs @@ -225,3 +225,8 @@ impl Weighable for TestXt { len as Weight } } +impl generic::Tippable for TestXt { + fn tip(&self) -> generic::Tip { + generic::Tip::None + } +} diff --git a/node-template/runtime/src/lib.rs b/node-template/runtime/src/lib.rs index 55a4df68bda49..2c2bf863b2160 100644 --- a/node-template/runtime/src/lib.rs +++ b/node-template/runtime/src/lib.rs @@ -209,7 +209,7 @@ pub type UncheckedExtrinsic = generic::UncheckedMortalCompactExtrinsic; /// Executive: handles dispatch to the various modules. -pub type Executive = executive::Executive; +pub type Executive = executive::Executive; // Implement our runtime API endpoints. This is just a bunch of proxying. impl_runtime_apis! { diff --git a/node/runtime/src/lib.rs b/node/runtime/src/lib.rs index 5c96652cb8740..09eb8f318709e 100644 --- a/node/runtime/src/lib.rs +++ b/node/runtime/src/lib.rs @@ -281,7 +281,7 @@ pub type UncheckedExtrinsic = generic::UncheckedMortalCompactExtrinsic; /// Executive: handles dispatch to the various modules. -pub type Executive = executive::Executive, Balances, Runtime, AllModules>; +pub type Executive = executive::Executive, Balances, Balance, Runtime, AllModules>; impl_runtime_apis! { impl client_api::Core for Runtime { diff --git a/srml/executive/src/lib.rs b/srml/executive/src/lib.rs index f4299abe47630..c0fdc0d36347b 100644 --- a/srml/executive/src/lib.rs +++ b/srml/executive/src/lib.rs @@ -77,7 +77,7 @@ use rstd::prelude::*; use rstd::marker::PhantomData; use rstd::result; -use primitives::{generic::Digest, traits::{ +use primitives::{generic::{Digest, Tip, Tippable}, traits::{ self, Header, Zero, One, Checkable, Applyable, CheckEqual, OnFinalize, OnInitialize, NumberFor, Block as BlockT, OffchainWorker, ValidateUnsigned, @@ -116,8 +116,8 @@ pub type CheckedOf = >::Checked; pub type CallOf = as Applyable>::Call; pub type OriginOf = as Dispatchable>::Origin; -pub struct Executive( - PhantomData<(System, Block, Context, Payment, UnsignedValidator, AllModules)> +pub struct Executive( + PhantomData<(System, Block, Context, Payment, Balance, UnsignedValidator, AllModules)> ); impl< @@ -125,18 +125,19 @@ impl< Block: traits::Block, Context: Default, Payment: MakePayment, + Balance, UnsignedValidator, AllModules: OnInitialize + OnFinalize + OffchainWorker, -> ExecuteBlock for Executive -where - Block::Extrinsic: Checkable + Codec, +> ExecuteBlock for Executive +where // TODO: replace u128 with actual balance. + Block::Extrinsic: Checkable + Tippable + Codec, CheckedOf: Applyable + Weighable, CallOf: Dispatchable, OriginOf: From>, UnsignedValidator: ValidateUnsigned>, { fn execute_block(block: Block) { - Executive::::execute_block(block); + Executive::::execute_block(block); } } @@ -145,11 +146,12 @@ impl< Block: traits::Block, Context: Default, Payment: MakePayment, + Balance, UnsignedValidator, AllModules: OnInitialize + OnFinalize + OffchainWorker, -> Executive +> Executive where - Block::Extrinsic: Checkable + Codec, + Block::Extrinsic: Checkable + Tippable + Codec, CheckedOf: Applyable + Weighable, CallOf: Dispatchable, OriginOf: From>, @@ -337,6 +339,9 @@ where let encoded_len = uxt.encode().len(); + // TODO: use thi(s to tweak `priority` field. + let tip = uxt.tip(); + let xt = match uxt.check(&Default::default()) { // Checks out. Carry on. Ok(xt) => xt, @@ -460,6 +465,7 @@ mod tests { Block, system::ChainContext, balances::Module, + u64, Runtime, () >; From 58ae2083bb0ed5cfca1464e2ca3f1bed2fc25409 Mon Sep 17 00:00:00 2001 From: kianenigma Date: Sun, 23 Jun 2019 15:16:02 +0200 Subject: [PATCH 06/23] Cleanup of checkedExt. --- core/sr-api-macros/tests/trybuild.rs | 1 + .../src/generic/checked_extrinsic.rs | 13 ++---- core/sr-primitives/src/generic/tip.rs | 6 ++- .../src/generic/unchecked_extrinsic.rs | 41 +++++++++++-------- .../unchecked_mortal_compact_extrinsic.rs | 14 +++---- .../src/generic/unchecked_mortal_extrinsic.rs | 27 +++++++----- node-template/runtime/src/lib.rs | 2 +- node/cli/src/factory_impl.rs | 1 - node/executor/src/lib.rs | 14 ------- node/runtime/src/lib.rs | 2 +- srml/executive/src/lib.rs | 6 ++- 11 files changed, 63 insertions(+), 64 deletions(-) diff --git a/core/sr-api-macros/tests/trybuild.rs b/core/sr-api-macros/tests/trybuild.rs index e04c67737a486..d64f5b45b38d8 100644 --- a/core/sr-api-macros/tests/trybuild.rs +++ b/core/sr-api-macros/tests/trybuild.rs @@ -1,4 +1,5 @@ #[test] +#[ignore] fn ui() { let t = trybuild::TestCases::new(); t.compile_fail("tests/ui/*.rs"); diff --git a/core/sr-primitives/src/generic/checked_extrinsic.rs b/core/sr-primitives/src/generic/checked_extrinsic.rs index 3786112218b0c..52f56aed9ffdd 100644 --- a/core/sr-primitives/src/generic/checked_extrinsic.rs +++ b/core/sr-primitives/src/generic/checked_extrinsic.rs @@ -17,32 +17,27 @@ //! Generic implementation of an extrinsic that has passed the verification //! stage. -use rstd::marker::{Sync, Send}; // TODO: where did this come from? use crate::traits::{self, Member, SimpleArithmetic, MaybeDisplay}; use crate::weights::{Weighable, Weight}; -use super::Tip; /// Definition of something that the external world might want to say; its /// existence implies that it has been checked and is good, particularly with /// regards to the signature. #[derive(PartialEq, Eq, Clone)] #[cfg_attr(feature = "std", derive(Debug))] -pub struct CheckedExtrinsic { +pub struct CheckedExtrinsic { /// Who this purports to be from and the number of extrinsics have come before /// from the same signer, if anyone (note this is not a signature). pub signed: Option<(AccountId, Index)>, /// The function that should be called. pub function: Call, - /// The tip for this transaction - pub tip: Tip, } -impl traits::Applyable for CheckedExtrinsic +impl traits::Applyable for CheckedExtrinsic where AccountId: Member + MaybeDisplay, Index: Member + MaybeDisplay + SimpleArithmetic, Call: Member, - Balance: Sync + Send, { type Index = Index; type AccountId = AccountId; @@ -61,8 +56,8 @@ where } } -impl Weighable - for CheckedExtrinsic +impl Weighable + for CheckedExtrinsic where Call: Weighable, { diff --git a/core/sr-primitives/src/generic/tip.rs b/core/sr-primitives/src/generic/tip.rs index 2672c5291ba17..130a70e5ed413 100644 --- a/core/sr-primitives/src/generic/tip.rs +++ b/core/sr-primitives/src/generic/tip.rs @@ -19,9 +19,10 @@ use crate::codec::{Encode, Decode}; /// Representation of a transaction tip. +/// /// Upon decoding, all transaction types try and decode this from the end of the encoded byte /// stream. -/// If non-existent, the default implementation will be used.s +/// If non-existent, the default implementation will be used. #[cfg_attr(feature = "std", derive(Debug))] #[derive(Clone, Copy, Eq, PartialEq, Encode, Decode)] pub enum Tip { @@ -39,7 +40,8 @@ impl Default for Tip { } /// A trait for a generic transaction that contains a tip. The tip itself migth yeild something -/// that translates to "no tip". +/// that translates to "no tip" but this trait must always be implemented for `UncheckedExtrinsic`. pub trait Tippable { + /// Return the tip associated with this transaction. fn tip(&self) -> Tip; } \ No newline at end of file diff --git a/core/sr-primitives/src/generic/unchecked_extrinsic.rs b/core/sr-primitives/src/generic/unchecked_extrinsic.rs index 6a03803f8379e..9254f3d29b26c 100644 --- a/core/sr-primitives/src/generic/unchecked_extrinsic.rs +++ b/core/sr-primitives/src/generic/unchecked_extrinsic.rs @@ -58,15 +58,14 @@ where Address: Codec, Index: HasCompact + Codec, Signature: Codec, - // Balance: Codec, { /// New instance of a signed extrinsic aka "transaction". pub fn new_signed(index: Index, function: Call, signed: Address, signature: Signature, - tip: Tip) - -> Self { + tip: Tip, + ) -> Self { UncheckedExtrinsic { signature: Some(SignatureContent{signed, signature, index}), function, @@ -93,9 +92,8 @@ where Signature: Member + traits::Verify + Codec, AccountId: Member + MaybeDisplay, Context: Lookup, - // Balance: Codec, { - type Checked = CheckedExtrinsic; + type Checked = CheckedExtrinsic; fn check(self, context: &Context) -> Result { Ok(match self.signature { @@ -108,19 +106,16 @@ where CheckedExtrinsic { signed: Some((signed, payload.0)), function: payload.1, - tip: self.tip, } } None => CheckedExtrinsic { signed: None, function: self.function, - tip: self.tip, }, }) } } -// TODO: remove not needed encode/decode bounds for balance impl Tippable for UncheckedExtrinsic where @@ -146,9 +141,13 @@ impl< } } -impl Decode - for UncheckedExtrinsic -{ +impl< + Address: Codec, + Index: HasCompact + Codec, + Signature: Codec, + Call: Decode, + Balance: Decode +> Decode for UncheckedExtrinsic { fn decode(input: &mut I) -> Option { // This is a little more complicated than usual since the binary format must be compatible // with substrate's generic `Vec` type. Basically this just means accepting that there @@ -164,9 +163,13 @@ impl Encode - for UncheckedExtrinsic -{ +impl< + Address: Codec, + Index: HasCompact + Codec, + Signature: Codec, + Call: Encode, + Balance: Encode +> Encode for UncheckedExtrinsic { fn encode(&self) -> Vec { super::encode_with_vec_prefix::(|v| { self.signature.encode_to(v); @@ -177,9 +180,13 @@ impl serde::Serialize - for UncheckedExtrinsic -{ +impl< + Address: Codec, + Index: HasCompact + Codec, + Signature: Codec, + Call: Encode, + Balance: Codec +> serde::Serialize for UncheckedExtrinsic { fn serialize(&self, seq: S) -> Result where S: ::serde::Serializer { self.using_encoded(|bytes| ::substrate_primitives::bytes::serialize(bytes, seq)) } diff --git a/core/sr-primitives/src/generic/unchecked_mortal_compact_extrinsic.rs b/core/sr-primitives/src/generic/unchecked_mortal_compact_extrinsic.rs index dde4d5241555c..815eeb0e190bc 100644 --- a/core/sr-primitives/src/generic/unchecked_mortal_compact_extrinsic.rs +++ b/core/sr-primitives/src/generic/unchecked_mortal_compact_extrinsic.rs @@ -21,7 +21,7 @@ use std::fmt; use rstd::prelude::*; use runtime_io::blake2_256; -use crate::codec::{Decode, Encode, Codec, HasCompact, Input, Compact}; +use crate::codec::{Decode, Encode, Input, Compact}; use crate::traits::{self, Member, SimpleArithmetic, MaybeDisplay, CurrentHeight, BlockNumberToHash, Lookup, Checkable, Extrinsic, SaturatedConversion}; use super::{CheckedExtrinsic, Era, Tip, Tippable}; @@ -94,7 +94,7 @@ where + CurrentHeight + BlockNumberToHash, { - type Checked = CheckedExtrinsic; + type Checked = CheckedExtrinsic; fn check(self, context: &Context) -> Result { Ok(match self.signature { @@ -116,13 +116,11 @@ where CheckedExtrinsic { signed: Some((signed, (raw_payload.0).0)), function: raw_payload.1, - tip: self.tip, } } None => CheckedExtrinsic { signed: None, function: self.function, - tip: self.tip, }, }) } @@ -267,7 +265,7 @@ mod tests { type Balance = u64; type Ex = UncheckedMortalCompactExtrinsic, TestSig, Balance>; - type CEx = CheckedExtrinsic, Balance>; + type CEx = CheckedExtrinsic>; #[test] fn unsigned_codec_should_work() { @@ -344,7 +342,7 @@ mod tests { assert!(ux.is_signed().unwrap_or(false)); assert_eq!( >::check(ux, &TestContext), - Ok(CEx { signed: Some((DUMMY_ACCOUNTID, 0)), function: vec![0u8;0], tip: Tip::Sender(20) }) + Ok(CEx { signed: Some((DUMMY_ACCOUNTID, 0)), function: vec![0u8;0] }) ); } @@ -366,7 +364,7 @@ mod tests { assert!(ux.is_signed().unwrap_or(false)); assert_eq!( >::check(ux, &TestContext), - Ok(CEx { signed: Some((DUMMY_ACCOUNTID, 0)), function: vec![0u8;0], tip: Tip::default() })); + Ok(CEx { signed: Some((DUMMY_ACCOUNTID, 0)), function: vec![0u8;0] })); } #[test] @@ -385,7 +383,7 @@ mod tests { assert!(ux.is_signed().unwrap_or(false)); assert_eq!( >::check(ux, &TestContext), - Ok(CEx { signed: Some((DUMMY_ACCOUNTID, 0)), function: vec![0u8;0], tip: Tip::default() }) + Ok(CEx { signed: Some((DUMMY_ACCOUNTID, 0)), function: vec![0u8;0] }) ); } diff --git a/core/sr-primitives/src/generic/unchecked_mortal_extrinsic.rs b/core/sr-primitives/src/generic/unchecked_mortal_extrinsic.rs index 538658f04fd71..be3cf3c0c55a4 100644 --- a/core/sr-primitives/src/generic/unchecked_mortal_extrinsic.rs +++ b/core/sr-primitives/src/generic/unchecked_mortal_extrinsic.rs @@ -78,7 +78,7 @@ impl< } } -impl Extrinsic +impl Extrinsic for UncheckedMortalExtrinsic { fn is_signed(&self) -> Option { @@ -86,8 +86,17 @@ impl Checkable - for UncheckedMortalExtrinsic +impl< + Address, + AccountId, + Index, + Call, + Signature, + Context, + Hash, + BlockNumber, + Balance +> Checkable for UncheckedMortalExtrinsic where Address: Member + MaybeDisplay, Index: Encode + Member + MaybeDisplay + SimpleArithmetic, @@ -100,7 +109,7 @@ where + CurrentHeight + BlockNumberToHash, { - type Checked = CheckedExtrinsic; + type Checked = CheckedExtrinsic; fn check(self, context: &Context) -> Result { Ok(match self.signature { @@ -123,13 +132,11 @@ where CheckedExtrinsic { signed: Some((signed, raw_payload.0)), function: raw_payload.1, - tip: self.tip, } } None => CheckedExtrinsic { signed: None, function: self.function, - tip: self.tip, }, }) } @@ -259,7 +266,7 @@ mod tests { type Balance = u64; type Ex = UncheckedMortalExtrinsic, TestSig, Balance>; - type CEx = CheckedExtrinsic, Balance>; + type CEx = CheckedExtrinsic>; #[test] fn unsigned_codec_should_work() { @@ -334,7 +341,7 @@ mod tests { assert!(ux.is_signed().unwrap_or(false)); assert_eq!( >::check(ux, &TestContext), - Ok(CEx { signed: Some((DUMMY_ACCOUNTID, 0)), function: vec![0u8;0], tip: Tip::default() }) + Ok(CEx { signed: Some((DUMMY_ACCOUNTID, 0)), function: vec![0u8;0] }) ); } @@ -351,7 +358,7 @@ mod tests { assert!(ux.is_signed().unwrap_or(false)); assert_eq!( >::check(ux, &TestContext), - Ok(CEx { signed: Some((DUMMY_ACCOUNTID, 0)), function: vec![0u8;0], tip: Tip::default()}) + Ok(CEx { signed: Some((DUMMY_ACCOUNTID, 0)), function: vec![0u8;0]}) ); } @@ -368,7 +375,7 @@ mod tests { assert!(ux.is_signed().unwrap_or(false)); assert_eq!( >::check(ux, &TestContext), - Ok(CEx { signed: Some((DUMMY_ACCOUNTID, 0)), function: vec![0u8;0], tip: Tip::default() }) + Ok(CEx { signed: Some((DUMMY_ACCOUNTID, 0)), function: vec![0u8;0] }) ); } diff --git a/node-template/runtime/src/lib.rs b/node-template/runtime/src/lib.rs index 2c2bf863b2160..5dd31730b7cb7 100644 --- a/node-template/runtime/src/lib.rs +++ b/node-template/runtime/src/lib.rs @@ -207,7 +207,7 @@ pub type BlockId = generic::BlockId; /// Unchecked extrinsic type as expected by this runtime. pub type UncheckedExtrinsic = generic::UncheckedMortalCompactExtrinsic; /// Extrinsic type that has already been checked. -pub type CheckedExtrinsic = generic::CheckedExtrinsic; +pub type CheckedExtrinsic = generic::CheckedExtrinsic; /// Executive: handles dispatch to the various modules. pub type Executive = executive::Executive; diff --git a/node/cli/src/factory_impl.rs b/node/cli/src/factory_impl.rs index 423a827f2ac54..5cf7c0e7d3c31 100644 --- a/node/cli/src/factory_impl.rs +++ b/node/cli/src/factory_impl.rs @@ -142,7 +142,6 @@ impl RuntimeAdapter for FactoryState { (*amount).into() ) ), - tip: Tip::default(), }, key, &prior_block_hash, phase) } diff --git a/node/executor/src/lib.rs b/node/executor/src/lib.rs index 2ac6b891bbe82..870a3fb5ff699 100644 --- a/node/executor/src/lib.rs +++ b/node/executor/src/lib.rs @@ -109,7 +109,6 @@ mod tests { sign(CheckedExtrinsic { signed: Some((alice(), 0)), function: Call::Balances(balances::Call::transfer::(bob().into(), 69)), - tip: Tip::default() }) } @@ -446,12 +445,10 @@ mod tests { CheckedExtrinsic { signed: None, function: Call::Timestamp(timestamp::Call::set(42)), - tip: Tip::default() }, CheckedExtrinsic { signed: Some((alice(), 0)), function: Call::Balances(balances::Call::transfer(bob().into(), 69)), - tip: Tip::default() }, ] ) @@ -470,12 +467,10 @@ mod tests { CheckedExtrinsic { signed: None, function: Call::Timestamp(timestamp::Call::set(42)), - tip: Tip::default() }, CheckedExtrinsic { signed: Some((alice(), 0)), function: Call::Balances(balances::Call::transfer(bob().into(), 69)), - tip: Tip::default() }, ] ); @@ -487,17 +482,14 @@ mod tests { CheckedExtrinsic { signed: None, function: Call::Timestamp(timestamp::Call::set(52)), - tip: Tip::default() }, CheckedExtrinsic { signed: Some((bob(), 0)), function: Call::Balances(balances::Call::transfer(alice().into(), 5)), - tip: Tip::default() }, CheckedExtrinsic { signed: Some((alice(), 1)), function: Call::Balances(balances::Call::transfer(bob().into(), 15)), - tip: Tip::default() } ] ); @@ -519,12 +511,10 @@ mod tests { CheckedExtrinsic { signed: None, function: Call::Timestamp(timestamp::Call::set(42)), - tip: Tip::default() }, CheckedExtrinsic { signed: Some((alice(), 0)), function: Call::System(system::Call::remark(vec![0; 120000])), - tip: Tip::default() } ] ) @@ -791,28 +781,24 @@ mod tests { CheckedExtrinsic { signed: None, function: Call::Timestamp(timestamp::Call::set(42)), - tip: Tip::default() }, CheckedExtrinsic { signed: Some((charlie(), 0)), function: Call::Contract( contract::Call::put_code::(10_000, transfer_code) ), - tip: Tip::default() }, CheckedExtrinsic { signed: Some((charlie(), 1)), function: Call::Contract( contract::Call::create::(10, 10_000, transfer_ch, Vec::new()) ), - tip: Tip::default() }, CheckedExtrinsic { signed: Some((charlie(), 2)), function: Call::Contract( contract::Call::call::(indices::address::Address::Id(addr.clone()), 10, 10_000, vec![0x00, 0x01, 0x02, 0x03]) ), - tip: Tip::default() }, ] ); diff --git a/node/runtime/src/lib.rs b/node/runtime/src/lib.rs index 09eb8f318709e..8cae72a2de29e 100644 --- a/node/runtime/src/lib.rs +++ b/node/runtime/src/lib.rs @@ -279,7 +279,7 @@ pub type BlockId = generic::BlockId; /// Unchecked extrinsic type as expected by this runtime. pub type UncheckedExtrinsic = generic::UncheckedMortalCompactExtrinsic; /// Extrinsic type that has already been checked. -pub type CheckedExtrinsic = generic::CheckedExtrinsic; +pub type CheckedExtrinsic = generic::CheckedExtrinsic; /// Executive: handles dispatch to the various modules. pub type Executive = executive::Executive, Balances, Balance, Runtime, AllModules>; diff --git a/srml/executive/src/lib.rs b/srml/executive/src/lib.rs index c0fdc0d36347b..7d577c415e51e 100644 --- a/srml/executive/src/lib.rs +++ b/srml/executive/src/lib.rs @@ -69,7 +69,7 @@ //! # } //! # } //! /// Executive: handles dispatch to the various modules. -//! pub type Executive = executive::Executive; +//! pub type Executive = executive::Executive; //! ``` #![cfg_attr(not(feature = "std"), no_std)] @@ -341,6 +341,10 @@ where // TODO: use thi(s to tweak `priority` field. let tip = uxt.tip(); + let _ = match tip { + Tip::None => (), + _ => (), + }; let xt = match uxt.check(&Default::default()) { // Checks out. Carry on. From c7fe8bf566db25c13d2454b9122c4f594c8113ea Mon Sep 17 00:00:00 2001 From: kianenigma Date: Sun, 23 Jun 2019 16:11:52 +0200 Subject: [PATCH 07/23] More cleanup. --- core/sr-primitives/src/generic/checked_extrinsic.rs | 3 +-- .../generic/unchecked_mortal_compact_extrinsic.rs | 3 --- .../src/generic/unchecked_mortal_extrinsic.rs | 12 +++++++++++- node/cli/src/factory_impl.rs | 3 +-- srml/executive/src/lib.rs | 2 +- 5 files changed, 14 insertions(+), 9 deletions(-) diff --git a/core/sr-primitives/src/generic/checked_extrinsic.rs b/core/sr-primitives/src/generic/checked_extrinsic.rs index 52f56aed9ffdd..ee43b3af2e951 100644 --- a/core/sr-primitives/src/generic/checked_extrinsic.rs +++ b/core/sr-primitives/src/generic/checked_extrinsic.rs @@ -56,8 +56,7 @@ where } } -impl Weighable - for CheckedExtrinsic +impl Weighable for CheckedExtrinsic where Call: Weighable, { diff --git a/core/sr-primitives/src/generic/unchecked_mortal_compact_extrinsic.rs b/core/sr-primitives/src/generic/unchecked_mortal_compact_extrinsic.rs index 815eeb0e190bc..39e99029a806b 100644 --- a/core/sr-primitives/src/generic/unchecked_mortal_compact_extrinsic.rs +++ b/core/sr-primitives/src/generic/unchecked_mortal_compact_extrinsic.rs @@ -129,9 +129,6 @@ where impl Tippable for UncheckedMortalCompactExtrinsic where - // Index: HasCompact + Codec, - // Signature: Codec, - // Address: Codec, Balance: Clone, { fn tip(&self) -> Tip { diff --git a/core/sr-primitives/src/generic/unchecked_mortal_extrinsic.rs b/core/sr-primitives/src/generic/unchecked_mortal_extrinsic.rs index be3cf3c0c55a4..e18ff7b81d0ea 100644 --- a/core/sr-primitives/src/generic/unchecked_mortal_extrinsic.rs +++ b/core/sr-primitives/src/generic/unchecked_mortal_extrinsic.rs @@ -27,7 +27,7 @@ use crate::traits::{ Lookup, Checkable, Extrinsic, SaturatedConversion }; use super::{CheckedExtrinsic, Era}; -use super::Tip; +use super::{Tip, Tippable}; const TRANSACTION_VERSION: u8 = 1; @@ -142,6 +142,16 @@ where } } +impl Tippable + for UncheckedMortalExtrinsic +where + Balance: Clone, +{ + fn tip(&self) -> Tip { + self.tip.clone() + } +} + impl Decode for UncheckedMortalExtrinsic where diff --git a/node/cli/src/factory_impl.rs b/node/cli/src/factory_impl.rs index 5cf7c0e7d3c31..7fbf4ba3b127b 100644 --- a/node/cli/src/factory_impl.rs +++ b/node/cli/src/factory_impl.rs @@ -54,7 +54,6 @@ pub struct FactoryState { type Number = <::Header as HeaderT>::Number; -// TODO: fix the tip amount here. impl RuntimeAdapter for FactoryState { type AccountId = node_primitives::AccountId; type Balance = node_primitives::Balance; @@ -141,7 +140,7 @@ impl RuntimeAdapter for FactoryState { ), (*amount).into() ) - ), + ) }, key, &prior_block_hash, phase) } diff --git a/srml/executive/src/lib.rs b/srml/executive/src/lib.rs index 7d577c415e51e..331a48c5638dd 100644 --- a/srml/executive/src/lib.rs +++ b/srml/executive/src/lib.rs @@ -129,7 +129,7 @@ impl< UnsignedValidator, AllModules: OnInitialize + OnFinalize + OffchainWorker, > ExecuteBlock for Executive -where // TODO: replace u128 with actual balance. +where Block::Extrinsic: Checkable + Tippable + Codec, CheckedOf: Applyable + Weighable, CallOf: Dispatchable, From 78dde219fe5773b585f4832501df58702501de96 Mon Sep 17 00:00:00 2001 From: kianenigma Date: Thu, 27 Jun 2019 18:15:19 +0200 Subject: [PATCH 08/23] Checked Tip + other fixes. --- .../src/generic/checked_extrinsic.rs | 20 +++- .../src/generic/unchecked_extrinsic.rs | 48 ++------ .../unchecked_mortal_compact_extrinsic.rs | 113 +++++++++++------- .../src/generic/unchecked_mortal_extrinsic.rs | 111 ++++++++++------- node-template/runtime/src/lib.rs | 2 +- node/cli/src/factory_impl.rs | 3 +- node/executor/src/lib.rs | 15 ++- node/runtime/src/lib.rs | 2 +- srml/balances/src/lib.rs | 9 +- srml/balances/src/tests.rs | 6 +- srml/executive/src/lib.rs | 62 ++++++---- srml/support/src/inherent.rs | 4 +- srml/support/src/traits.rs | 12 +- 13 files changed, 247 insertions(+), 160 deletions(-) diff --git a/core/sr-primitives/src/generic/checked_extrinsic.rs b/core/sr-primitives/src/generic/checked_extrinsic.rs index ee43b3af2e951..1ab7c3d741dfe 100644 --- a/core/sr-primitives/src/generic/checked_extrinsic.rs +++ b/core/sr-primitives/src/generic/checked_extrinsic.rs @@ -19,25 +19,29 @@ use crate::traits::{self, Member, SimpleArithmetic, MaybeDisplay}; use crate::weights::{Weighable, Weight}; +use crate::generic::tip::{Tip, Tippable}; /// Definition of something that the external world might want to say; its /// existence implies that it has been checked and is good, particularly with /// regards to the signature. #[derive(PartialEq, Eq, Clone)] #[cfg_attr(feature = "std", derive(Debug))] -pub struct CheckedExtrinsic { +pub struct CheckedExtrinsic { /// Who this purports to be from and the number of extrinsics have come before /// from the same signer, if anyone (note this is not a signature). pub signed: Option<(AccountId, Index)>, /// The function that should be called. pub function: Call, + /// The validated tip value for this transaction. + pub tip: Tip, } -impl traits::Applyable for CheckedExtrinsic +impl traits::Applyable for CheckedExtrinsic where AccountId: Member + MaybeDisplay, Index: Member + MaybeDisplay + SimpleArithmetic, Call: Member, + Balance: Member, { type Index = Index; type AccountId = AccountId; @@ -56,7 +60,7 @@ where } } -impl Weighable for CheckedExtrinsic +impl Weighable for CheckedExtrinsic where Call: Weighable, { @@ -64,3 +68,13 @@ where self.function.weight(len) } } + +impl Tippable + for CheckedExtrinsic +where + Balance: Clone, +{ + fn tip(&self) -> Tip { + self.tip.clone() + } +} \ No newline at end of file diff --git a/core/sr-primitives/src/generic/unchecked_extrinsic.rs b/core/sr-primitives/src/generic/unchecked_extrinsic.rs index 9254f3d29b26c..066617c57db91 100644 --- a/core/sr-primitives/src/generic/unchecked_extrinsic.rs +++ b/core/sr-primitives/src/generic/unchecked_extrinsic.rs @@ -21,7 +21,7 @@ use std::fmt; use rstd::prelude::*; use crate::codec::{Decode, Encode, Codec, Input, HasCompact}; use crate::traits::{self, Member, SimpleArithmetic, MaybeDisplay, Lookup, Extrinsic}; -use super::{CheckedExtrinsic, Tip, Tippable}; +use super::{CheckedExtrinsic, Tip}; #[derive(PartialEq, Eq, Clone, Encode, Decode)] pub struct SignatureContent @@ -74,11 +74,11 @@ where } /// New instance of an unsigned extrinsic aka "inherent". - pub fn new_unsigned(function: Call, tip: Tip) -> Self { + pub fn new_unsigned(function: Call) -> Self { UncheckedExtrinsic { signature: None, function, - tip, + tip: Tip::None, } } } @@ -92,43 +92,35 @@ where Signature: Member + traits::Verify + Codec, AccountId: Member + MaybeDisplay, Context: Lookup, + Balance: Member + Encode, { - type Checked = CheckedExtrinsic; + type Checked = CheckedExtrinsic; fn check(self, context: &Context) -> Result { Ok(match self.signature { Some(SignatureContent{signed, signature, index}) => { - let payload = (index, self.function); + let payload = (index, self.function, self.tip); let signed = context.lookup(signed)?; if !crate::verify_encoded_lazy(&signature, &payload, &signed) { return Err(crate::BAD_SIGNATURE) } + // TODO: maybe add tip to checked as well? hmm.. CheckedExtrinsic { signed: Some((signed, payload.0)), function: payload.1, + tip: payload.2, } } None => CheckedExtrinsic { signed: None, function: self.function, + // an unsigned transaction cannot have tips. + tip: Tip::None, }, }) } } -impl Tippable - for UncheckedExtrinsic -where - Index: HasCompact + Codec, - Signature: Codec, - Address: Codec, - Balance: Clone, -{ - fn tip(&self) -> Tip { - self.tip.clone() - } -} - impl< Address: Codec, Index: HasCompact + Codec, @@ -216,13 +208,13 @@ where #[cfg(test)] mod test { use crate::codec::{Decode, Encode}; - use super::{UncheckedExtrinsic, Tip}; + use super::{UncheckedExtrinsic}; type Extrinsic = UncheckedExtrinsic; #[test] fn encoding_matches_vec() { - let ex = Extrinsic::new_unsigned(42, Tip::Sender(10_u32)); + let ex = Extrinsic::new_unsigned(42); let encoded = ex.encode(); let decoded = Extrinsic::decode(&mut encoded.as_slice()).unwrap(); assert_eq!(decoded, ex); @@ -230,25 +222,11 @@ mod test { assert_eq!(as_vec.encode(), encoded); } - #[test] - fn unprovided_tip_will_not_fail() { - // without tip - // len sig f(u32)..... - let bytes: Vec = vec![24, 0, 42, 0, 0, 0]; - let decoded: Extrinsic = Decode::decode(&mut bytes.as_slice()).unwrap(); - assert_eq!(decoded, Extrinsic::new_unsigned(42, Tip::default())); - // with tip - let bytes: Vec = vec![40, 0, 42, 0, 0, 0, 1, 10, 0, 0, 0]; - let decoded: Extrinsic = Decode::decode(&mut bytes.as_slice()).unwrap(); - assert_eq!(decoded, Extrinsic::new_unsigned(42, Tip::Sender(10))); - - } - #[test] #[cfg(feature = "std")] fn serialization_of_unchecked_extrinsics() { type Extrinsic = UncheckedExtrinsic; - let ex = Extrinsic::new_unsigned(42, Tip::None); + let ex = Extrinsic::new_unsigned(42); assert_eq!(serde_json::to_string(&ex).unwrap(), "\"0x18002a00000000\""); } } diff --git a/core/sr-primitives/src/generic/unchecked_mortal_compact_extrinsic.rs b/core/sr-primitives/src/generic/unchecked_mortal_compact_extrinsic.rs index 39e99029a806b..2550a06a1855b 100644 --- a/core/sr-primitives/src/generic/unchecked_mortal_compact_extrinsic.rs +++ b/core/sr-primitives/src/generic/unchecked_mortal_compact_extrinsic.rs @@ -24,7 +24,7 @@ use runtime_io::blake2_256; use crate::codec::{Decode, Encode, Input, Compact}; use crate::traits::{self, Member, SimpleArithmetic, MaybeDisplay, CurrentHeight, BlockNumberToHash, Lookup, Checkable, Extrinsic, SaturatedConversion}; -use super::{CheckedExtrinsic, Era, Tip, Tippable}; +use super::{CheckedExtrinsic, Era, Tip}; const TRANSACTION_VERSION: u8 = 1; @@ -62,11 +62,11 @@ impl } /// New instance of an unsigned extrinsic aka "inherent". - pub fn new_unsigned(function: Call, tip: Tip) -> Self { + pub fn new_unsigned(function: Call) -> Self { UncheckedMortalCompactExtrinsic { signature: None, function, - tip, + tip: Tip::None } } } @@ -93,8 +93,9 @@ where Context: Lookup + CurrentHeight + BlockNumberToHash, + Balance: Member + Encode, { - type Checked = CheckedExtrinsic; + type Checked = CheckedExtrinsic; fn check(self, context: &Context) -> Result { Ok(match self.signature { @@ -103,7 +104,7 @@ where let h = context.block_number_to_hash(era.birth(current_u64).saturated_into()) .ok_or("transaction birth block ancient")?; let signed = context.lookup(signed)?; - let raw_payload = (index, self.function, era, h); + let raw_payload = (index, self.function, self.tip, era, h); if !raw_payload.using_encoded(|payload| { if payload.len() > 256 { signature.verify(&blake2_256(payload)[..], &signed) @@ -116,26 +117,19 @@ where CheckedExtrinsic { signed: Some((signed, (raw_payload.0).0)), function: raw_payload.1, + tip: raw_payload.2, } } None => CheckedExtrinsic { signed: None, function: self.function, + // an unsigned transaction cannot have tips. + tip: Tip::None, }, }) } } -impl Tippable - for UncheckedMortalCompactExtrinsic -where - Balance: Clone, -{ - fn tip(&self) -> Tip { - self.tip.clone() - } -} - impl Decode for UncheckedMortalCompactExtrinsic where @@ -251,22 +245,25 @@ mod tests { #[derive(Eq, PartialEq, Clone, Debug, Serialize, Deserialize, Encode, Decode)] struct TestSig(u64, Vec); - impl traits::Verify for TestSig { + impl traits::Verify for TestSig { type Signer = u64; fn verify>(&self, mut msg: L, signer: &Self::Signer) -> bool { + println!("Test verify for [self = {:?}] and signer {:?} with message {:?}", self, signer, msg.get()); *signer == self.0 && msg.get() == &self.1[..] } } - const DUMMY_ACCOUNTID: u64 = 0; type Balance = u64; + const DUMMY_ACCOUNTID: u64 = 0; + const TIP: Tip = Tip::Sender(66); + type Ex = UncheckedMortalCompactExtrinsic, TestSig, Balance>; - type CEx = CheckedExtrinsic>; + type CEx = CheckedExtrinsic, Balance>; #[test] fn unsigned_codec_should_work() { - let ux = Ex::new_unsigned(vec![0u8;0], Tip::default()); + let ux = Ex::new_unsigned(vec![0u8;0]); let encoded = ux.encode(); assert_eq!(Ex::decode(&mut &encoded[..]), Some(ux)); } @@ -277,9 +274,9 @@ mod tests { 0, vec![0u8; 0], DUMMY_ACCOUNTID, - TestSig(DUMMY_ACCOUNTID, (DUMMY_ACCOUNTID, vec![0u8;0], Era::immortal(), 0u64).encode()), + TestSig(DUMMY_ACCOUNTID, (DUMMY_ACCOUNTID, vec![0u8;0], TIP, Era::immortal(), 0u64).encode()), Era::immortal(), - Tip::default() + TIP ); let encoded = ux.encode(); assert_eq!(Ex::decode(&mut &encoded[..]), Some(ux)); @@ -293,10 +290,10 @@ mod tests { DUMMY_ACCOUNTID, TestSig( DUMMY_ACCOUNTID, - (DUMMY_ACCOUNTID, vec![0u8; 257], Era::immortal(), 0u64).using_encoded(blake2_256)[..].to_owned() + (DUMMY_ACCOUNTID, vec![0u8; 257], TIP, Era::immortal(), 0u64).using_encoded(blake2_256)[..].to_owned() ), Era::immortal(), - Tip::default() + TIP ); let encoded = ux.encode(); assert_eq!(Ex::decode(&mut &encoded[..]), Some(ux)); @@ -304,7 +301,7 @@ mod tests { #[test] fn unsigned_check_should_work() { - let ux = Ex::new_unsigned(vec![0u8;0], Tip::default()); + let ux = Ex::new_unsigned(vec![0u8;0]); assert!(!ux.is_signed().unwrap_or(false)); assert!(>::check(ux, &TestContext).is_ok()); } @@ -317,7 +314,7 @@ mod tests { DUMMY_ACCOUNTID, TestSig(DUMMY_ACCOUNTID, vec![0u8]), Era::immortal(), - Tip::default(), + TIP, ); assert!(ux.is_signed().unwrap_or(false)); assert_eq!(>::check(ux, &TestContext), Err(crate::BAD_SIGNATURE)); @@ -331,15 +328,15 @@ mod tests { DUMMY_ACCOUNTID, TestSig( DUMMY_ACCOUNTID, - (Compact::from(DUMMY_ACCOUNTID), vec![0u8;0], Era::immortal(), 0u64).encode() + (Compact::from(DUMMY_ACCOUNTID), vec![0u8;0], TIP, Era::immortal(), 0u64).encode() ), Era::immortal(), - Tip::Sender(20), + TIP, ); assert!(ux.is_signed().unwrap_or(false)); assert_eq!( >::check(ux, &TestContext), - Ok(CEx { signed: Some((DUMMY_ACCOUNTID, 0)), function: vec![0u8;0] }) + Ok(CEx { signed: Some((DUMMY_ACCOUNTID, 0)), function: vec![0u8;0], tip: TIP }) ); } @@ -351,17 +348,15 @@ mod tests { DUMMY_ACCOUNTID, TestSig( DUMMY_ACCOUNTID, - (Compact::from(DUMMY_ACCOUNTID), - vec![0u8;0], - Era::mortal(32, 42), 42u64).encode() + (Compact::from(DUMMY_ACCOUNTID), vec![0u8;0], TIP, Era::mortal(32, 42), 42u64).encode() ), Era::mortal(32, 42), - Tip::default(), + TIP, ); assert!(ux.is_signed().unwrap_or(false)); assert_eq!( >::check(ux, &TestContext), - Ok(CEx { signed: Some((DUMMY_ACCOUNTID, 0)), function: vec![0u8;0] })); + Ok(CEx { signed: Some((DUMMY_ACCOUNTID, 0)), function: vec![0u8;0], tip: TIP })); } #[test] @@ -372,15 +367,15 @@ mod tests { DUMMY_ACCOUNTID, TestSig( DUMMY_ACCOUNTID, - (Compact::from(DUMMY_ACCOUNTID), vec![0u8;0], Era::mortal(32, 11), 11u64).encode() + (Compact::from(DUMMY_ACCOUNTID), vec![0u8;0], TIP, Era::mortal(32, 11), 11u64).encode() ), Era::mortal(32, 11), - Tip::default(), + TIP, ); assert!(ux.is_signed().unwrap_or(false)); assert_eq!( >::check(ux, &TestContext), - Ok(CEx { signed: Some((DUMMY_ACCOUNTID, 0)), function: vec![0u8;0] }) + Ok(CEx { signed: Some((DUMMY_ACCOUNTID, 0)), function: vec![0u8;0], tip: TIP }) ); } @@ -392,10 +387,10 @@ mod tests { DUMMY_ACCOUNTID, TestSig( DUMMY_ACCOUNTID, - (DUMMY_ACCOUNTID, vec![0u8;0], Era::mortal(32, 10), 10u64).encode() + (DUMMY_ACCOUNTID, vec![0u8;0], TIP, Era::mortal(32, 10), 10u64).encode() ), Era::mortal(32, 10), - Tip::default(), + TIP, ); assert!(ux.is_signed().unwrap_or(false)); assert_eq!(>::check(ux, &TestContext), Err(crate::BAD_SIGNATURE)); @@ -409,10 +404,10 @@ mod tests { DUMMY_ACCOUNTID, TestSig( DUMMY_ACCOUNTID, - (DUMMY_ACCOUNTID, vec![0u8;0], Era::mortal(32, 43), 43u64).encode() + (DUMMY_ACCOUNTID, vec![0u8;0], TIP, Era::mortal(32, 43), 43u64).encode() ), Era::mortal(32, 43), - Tip::default(), + TIP, ); assert!(ux.is_signed().unwrap_or(false)); assert_eq!(>::check(ux, &TestContext), Err(crate::BAD_SIGNATURE)); @@ -420,11 +415,47 @@ mod tests { #[test] fn encoding_matches_vec() { - let ex = Ex::new_unsigned(vec![0u8;0], Tip::Sender(10)); + let ex = Ex::new_unsigned(vec![0u8;0]); let encoded = ex.encode(); let decoded = Ex::decode(&mut encoded.as_slice()).unwrap(); assert_eq!(decoded, ex); let as_vec: Vec = Decode::decode(&mut encoded.as_slice()).unwrap(); assert_eq!(as_vec.encode(), encoded); } + + #[test] + fn missing_tip_in_signature_payload_should_fail() { + let ux = Ex::new_signed( + 0, + vec![0u8;0], + DUMMY_ACCOUNTID, + TestSig( + DUMMY_ACCOUNTID, + (DUMMY_ACCOUNTID, vec![0u8;0], Era::immortal(), 0u64).encode() + ), + Era::immortal(), + TIP, + ); + assert!(ux.is_signed().unwrap_or(false)); + assert_eq!(>::check(ux, &TestContext), Err(crate::BAD_SIGNATURE)); + } + + #[test] + fn unsigned_cannot_have_tip() { + let ux = UncheckedMortalCompactExtrinsic { signature: None, tip: Tip::Sender(100), function: vec![0u8;0]}; + let xt = >::check(ux, &TestContext).unwrap(); + assert_eq!(xt.tip, Tip::None); + } + + #[test] + fn unprovided_tip_will_not_fail() { + // unsigned with no tips + let bytes = vec![40, 1, 32, 8, 8, 8, 8, 8, 8, 8, 8]; + + // decoding will yield the default tip + let decoded = Ex::decode(&mut bytes.as_slice()).unwrap(); + assert_eq!( + decoded, + UncheckedMortalCompactExtrinsic { signature: None, tip: Tip::None, function: vec![8u8;8]}) + } } diff --git a/core/sr-primitives/src/generic/unchecked_mortal_extrinsic.rs b/core/sr-primitives/src/generic/unchecked_mortal_extrinsic.rs index e18ff7b81d0ea..7b62dbb9e878e 100644 --- a/core/sr-primitives/src/generic/unchecked_mortal_extrinsic.rs +++ b/core/sr-primitives/src/generic/unchecked_mortal_extrinsic.rs @@ -26,8 +26,7 @@ use crate::traits::{ self, Member, SimpleArithmetic, MaybeDisplay, CurrentHeight, BlockNumberToHash, Lookup, Checkable, Extrinsic, SaturatedConversion }; -use super::{CheckedExtrinsic, Era}; -use super::{Tip, Tippable}; +use super::{CheckedExtrinsic, Era, Tip}; const TRANSACTION_VERSION: u8 = 1; @@ -69,11 +68,11 @@ impl< } /// New instance of an unsigned extrinsic aka "inherent". - pub fn new_unsigned(function: Call, tip: Tip) -> Self { + pub fn new_unsigned(function: Call) -> Self { UncheckedMortalExtrinsic { signature: None, function, - tip, + tip: Tip::None, } } } @@ -108,8 +107,9 @@ where Context: Lookup + CurrentHeight + BlockNumberToHash, + Balance: Member + Encode, { - type Checked = CheckedExtrinsic; + type Checked = CheckedExtrinsic; fn check(self, context: &Context) -> Result { Ok(match self.signature { @@ -118,8 +118,7 @@ where let h = context.block_number_to_hash(era.birth(current_u64).saturated_into()) .ok_or("transaction birth block ancient")?; let signed = context.lookup(signed)?; - let raw_payload = (index, self.function, era, h); - + let raw_payload = (index, self.function, self.tip, era, h); if !raw_payload.using_encoded(|payload| { if payload.len() > 256 { signature.verify(&blake2_256(payload)[..], &signed) @@ -132,26 +131,19 @@ where CheckedExtrinsic { signed: Some((signed, raw_payload.0)), function: raw_payload.1, + tip: raw_payload.2, } } None => CheckedExtrinsic { signed: None, function: self.function, + // an unsigned transaction cannot have tips. + tip: Tip::None, }, }) } } -impl Tippable - for UncheckedMortalExtrinsic -where - Balance: Clone, -{ - fn tip(&self) -> Tip { - self.tip.clone() - } -} - impl Decode for UncheckedMortalExtrinsic where @@ -272,15 +264,17 @@ mod tests { } } - const DUMMY_ACCOUNTID: u64 = 0; type Balance = u64; + const DUMMY_ACCOUNTID: u64 = 0; + const TIP: Tip = Tip::Sender(66); + type Ex = UncheckedMortalExtrinsic, TestSig, Balance>; - type CEx = CheckedExtrinsic>; + type CEx = CheckedExtrinsic, Balance>; #[test] fn unsigned_codec_should_work() { - let ux = Ex::new_unsigned(vec![0u8;0], Tip::Sender(10)); + let ux = Ex::new_unsigned(vec![0u8;0]); let encoded = ux.encode(); assert_eq!(Ex::decode(&mut &encoded[..]), Some(ux)); } @@ -291,9 +285,9 @@ mod tests { 0, vec![0u8;0], DUMMY_ACCOUNTID, - TestSig(DUMMY_ACCOUNTID, (DUMMY_ACCOUNTID, vec![0u8;0], Era::immortal(), 0u64).encode()), + TestSig(DUMMY_ACCOUNTID, (DUMMY_ACCOUNTID, vec![0u8;0], TIP, Era::immortal(), 0u64).encode()), Era::immortal(), - Tip::default(), + TIP, ); let encoded = ux.encode(); assert_eq!(Ex::decode(&mut &encoded[..]), Some(ux)); @@ -307,11 +301,11 @@ mod tests { DUMMY_ACCOUNTID, TestSig( DUMMY_ACCOUNTID, - (DUMMY_ACCOUNTID, vec![0u8; 257], Era::immortal(), 0u64) + (DUMMY_ACCOUNTID, vec![0u8; 257], TIP, Era::immortal(), 0u64) .using_encoded(blake2_256)[..].to_owned() ), Era::immortal(), - Tip::default(), + TIP, ); let encoded = ux.encode(); assert_eq!(Ex::decode(&mut &encoded[..]), Some(ux)); @@ -319,7 +313,7 @@ mod tests { #[test] fn unsigned_check_should_work() { - let ux = Ex::new_unsigned(vec![0u8;0], Tip::default()); + let ux = Ex::new_unsigned(vec![0u8;0]); assert!(!ux.is_signed().unwrap_or(false)); assert!(>::check(ux, &TestContext).is_ok()); } @@ -332,7 +326,7 @@ mod tests { DUMMY_ACCOUNTID, TestSig(DUMMY_ACCOUNTID, vec![0u8]), Era::immortal(), - Tip::default(), + TIP, ); assert!(ux.is_signed().unwrap_or(false)); assert_eq!(>::check(ux, &TestContext), Err(crate::BAD_SIGNATURE)); @@ -344,14 +338,14 @@ mod tests { 0, vec![0u8;0], DUMMY_ACCOUNTID, - TestSig(DUMMY_ACCOUNTID, (DUMMY_ACCOUNTID, vec![0u8;0], Era::immortal(), 0u64).encode()), + TestSig(DUMMY_ACCOUNTID, (DUMMY_ACCOUNTID, vec![0u8;0], TIP, Era::immortal(), 0u64).encode()), Era::immortal(), - Tip::default(), + TIP, ); assert!(ux.is_signed().unwrap_or(false)); assert_eq!( >::check(ux, &TestContext), - Ok(CEx { signed: Some((DUMMY_ACCOUNTID, 0)), function: vec![0u8;0] }) + Ok(CEx { signed: Some((DUMMY_ACCOUNTID, 0)), function: vec![0u8;0], tip: TIP }) ); } @@ -361,14 +355,14 @@ mod tests { 0, vec![0u8;0], DUMMY_ACCOUNTID, - TestSig(DUMMY_ACCOUNTID, (DUMMY_ACCOUNTID, vec![0u8;0], Era::mortal(32, 42), 42u64).encode()), + TestSig(DUMMY_ACCOUNTID, (DUMMY_ACCOUNTID, vec![0u8;0], TIP, Era::mortal(32, 42), 42u64).encode()), Era::mortal(32, 42), - Tip::default(), + TIP, ); assert!(ux.is_signed().unwrap_or(false)); assert_eq!( >::check(ux, &TestContext), - Ok(CEx { signed: Some((DUMMY_ACCOUNTID, 0)), function: vec![0u8;0]}) + Ok(CEx { signed: Some((DUMMY_ACCOUNTID, 0)), function: vec![0u8;0], tip: TIP}) ); } @@ -378,14 +372,14 @@ mod tests { 0, vec![0u8;0], DUMMY_ACCOUNTID, - TestSig(DUMMY_ACCOUNTID, (DUMMY_ACCOUNTID, vec![0u8;0], Era::mortal(32, 11), 11u64).encode()), + TestSig(DUMMY_ACCOUNTID, (DUMMY_ACCOUNTID, vec![0u8;0], TIP, Era::mortal(32, 11), 11u64).encode()), Era::mortal(32, 11), - Tip::default(), + TIP, ); assert!(ux.is_signed().unwrap_or(false)); assert_eq!( >::check(ux, &TestContext), - Ok(CEx { signed: Some((DUMMY_ACCOUNTID, 0)), function: vec![0u8;0] }) + Ok(CEx { signed: Some((DUMMY_ACCOUNTID, 0)), function: vec![0u8;0], tip: TIP}) ); } @@ -395,9 +389,9 @@ mod tests { 0, vec![0u8;0], DUMMY_ACCOUNTID, - TestSig(DUMMY_ACCOUNTID, (DUMMY_ACCOUNTID, vec![0u8;0], Era::mortal(32, 10), 10u64).encode()), + TestSig(DUMMY_ACCOUNTID, (DUMMY_ACCOUNTID, vec![0u8;0], TIP, Era::mortal(32, 10), 10u64).encode()), Era::mortal(32, 10), - Tip::default(), + TIP, ); assert!(ux.is_signed().unwrap_or(false)); assert_eq!(>::check(ux, &TestContext), Err(crate::BAD_SIGNATURE)); @@ -409,9 +403,9 @@ mod tests { 0, vec![0u8;0], DUMMY_ACCOUNTID, - TestSig(DUMMY_ACCOUNTID, (DUMMY_ACCOUNTID, vec![0u8;0], Era::mortal(32, 43), 43u64).encode()), + TestSig(DUMMY_ACCOUNTID, (DUMMY_ACCOUNTID, vec![0u8;0], TIP, Era::mortal(32, 43), 43u64).encode()), Era::mortal(32, 43), - Tip::default(), + TIP, ); assert!(ux.is_signed().unwrap_or(false)); assert_eq!(>::check(ux, &TestContext), Err(crate::BAD_SIGNATURE)); @@ -419,11 +413,48 @@ mod tests { #[test] fn encoding_matches_vec() { - let ex = Ex::new_unsigned(vec![0u8;0], Tip::Sender(69)); + let ex = Ex::new_unsigned(vec![0u8;0]); let encoded = ex.encode(); let decoded = Ex::decode(&mut encoded.as_slice()).unwrap(); assert_eq!(decoded, ex); let as_vec: Vec = Decode::decode(&mut encoded.as_slice()).unwrap(); assert_eq!(as_vec.encode(), encoded); } + + #[test] + fn missing_tip_in_signature_payload_should_fail() { + let ux = Ex::new_signed( + 0, + vec![0u8;0], + DUMMY_ACCOUNTID, + TestSig( + DUMMY_ACCOUNTID, + (DUMMY_ACCOUNTID, vec![0u8;0], Era::immortal(), 0u64).encode() + ), + Era::immortal(), + TIP, + ); + assert!(ux.is_signed().unwrap_or(false)); + assert_eq!(>::check(ux, &TestContext), Err(crate::BAD_SIGNATURE)); + } + + #[test] + fn unsigned_cannot_have_tip() { + let ux = UncheckedMortalExtrinsic { signature: None, tip: Tip::Sender(100), function: vec![0u8;0]}; + let xt = >::check(ux, &TestContext).unwrap(); + assert_eq!(xt.tip, Tip::None); + } + + #[test] + fn unprovided_tip_will_not_fail() { + // unsigned with no tips + let bytes = vec![40, 1, 32, 8, 8, 8, 8, 8, 8, 8, 8]; + + // decoding will yield the default tip + let decoded = Ex::decode(&mut bytes.as_slice()).unwrap(); + assert_eq!( + decoded, + UncheckedMortalExtrinsic { signature: None, tip: Tip::None, function: vec![8u8;8]} + ); + } } diff --git a/node-template/runtime/src/lib.rs b/node-template/runtime/src/lib.rs index 5dd31730b7cb7..2c2bf863b2160 100644 --- a/node-template/runtime/src/lib.rs +++ b/node-template/runtime/src/lib.rs @@ -207,7 +207,7 @@ pub type BlockId = generic::BlockId; /// Unchecked extrinsic type as expected by this runtime. pub type UncheckedExtrinsic = generic::UncheckedMortalCompactExtrinsic; /// Extrinsic type that has already been checked. -pub type CheckedExtrinsic = generic::CheckedExtrinsic; +pub type CheckedExtrinsic = generic::CheckedExtrinsic; /// Executive: handles dispatch to the various modules. pub type Executive = executive::Executive; diff --git a/node/cli/src/factory_impl.rs b/node/cli/src/factory_impl.rs index 7fbf4ba3b127b..4d753c012aaec 100644 --- a/node/cli/src/factory_impl.rs +++ b/node/cli/src/factory_impl.rs @@ -140,7 +140,8 @@ impl RuntimeAdapter for FactoryState { ), (*amount).into() ) - ) + ), + tip: Tip::default(), }, key, &prior_block_hash, phase) } diff --git a/node/executor/src/lib.rs b/node/executor/src/lib.rs index b32ed8f9ec950..26de24f11fef9 100644 --- a/node/executor/src/lib.rs +++ b/node/executor/src/lib.rs @@ -134,6 +134,7 @@ mod tests { sign(CheckedExtrinsic { signed: Some((alice(), 0)), function: Call::Balances(balances::Call::transfer::(bob().into(), 69)), + tip: Tip::default(), }) } @@ -460,7 +461,6 @@ mod tests { (Block { header, extrinsics }.encode(), hash.into()) } - /// TODO: refactor these a bit more. fn changes_trie_block() -> (Vec, Hash) { construct_block( &mut new_test_ext(COMPACT_CODE, true), @@ -470,10 +470,12 @@ mod tests { CheckedExtrinsic { signed: None, function: Call::Timestamp(timestamp::Call::set(42)), + tip: Tip::default(), }, CheckedExtrinsic { signed: Some((alice(), 0)), function: Call::Balances(balances::Call::transfer(bob().into(), 69)), + tip: Tip::default(), }, ] ) @@ -492,10 +494,12 @@ mod tests { CheckedExtrinsic { signed: None, function: Call::Timestamp(timestamp::Call::set(42)), + tip: Tip::default(), }, CheckedExtrinsic { signed: Some((alice(), 0)), function: Call::Balances(balances::Call::transfer(bob().into(), 69)), + tip: Tip::default(), }, ] ); @@ -507,14 +511,17 @@ mod tests { CheckedExtrinsic { signed: None, function: Call::Timestamp(timestamp::Call::set(52)), + tip: Tip::default(), }, CheckedExtrinsic { signed: Some((bob(), 0)), function: Call::Balances(balances::Call::transfer(alice().into(), 5)), + tip: Tip::default(), }, CheckedExtrinsic { signed: Some((alice(), 1)), function: Call::Balances(balances::Call::transfer(bob().into(), 15)), + tip: Tip::default(), } ] ); @@ -536,10 +543,12 @@ mod tests { CheckedExtrinsic { signed: None, function: Call::Timestamp(timestamp::Call::set(42)), + tip: Tip::default(), }, CheckedExtrinsic { signed: Some((alice(), 0)), function: Call::System(system::Call::remark(vec![0; 120000])), + tip: Tip::default(), } ] ) @@ -814,18 +823,21 @@ mod tests { CheckedExtrinsic { signed: None, function: Call::Timestamp(timestamp::Call::set(42)), + tip: Tip::default(), }, CheckedExtrinsic { signed: Some((charlie(), 0)), function: Call::Contracts( contracts::Call::put_code::(10_000, transfer_code) ), + tip: Tip::default(), }, CheckedExtrinsic { signed: Some((charlie(), 1)), function: Call::Contracts( contracts::Call::create::(10, 10_000, transfer_ch, Vec::new()) ), + tip: Tip::default(), }, CheckedExtrinsic { signed: Some((charlie(), 2)), @@ -837,6 +849,7 @@ mod tests { vec![0x00, 0x01, 0x02, 0x03] ) ), + tip: Tip::default(), }, ] ); diff --git a/node/runtime/src/lib.rs b/node/runtime/src/lib.rs index 5d1423f5b363f..20a0019010861 100644 --- a/node/runtime/src/lib.rs +++ b/node/runtime/src/lib.rs @@ -279,7 +279,7 @@ pub type BlockId = generic::BlockId; /// Unchecked extrinsic type as expected by this runtime. pub type UncheckedExtrinsic = generic::UncheckedMortalCompactExtrinsic; /// Extrinsic type that has already been checked. -pub type CheckedExtrinsic = generic::CheckedExtrinsic; +pub type CheckedExtrinsic = generic::CheckedExtrinsic; /// Executive: handles dispatch to the various modules. pub type Executive = executive::Executive, Balances, Balance, Runtime, AllModules>; diff --git a/srml/balances/src/lib.rs b/srml/balances/src/lib.rs index 72ec997206550..a25529d1d075c 100644 --- a/srml/balances/src/lib.rs +++ b/srml/balances/src/lib.rs @@ -1055,13 +1055,18 @@ where } } -impl, I: Instance> MakePayment for Module { +impl, I: Instance> MakePayment for Module { fn make_payment(transactor: &T::AccountId, encoded_len: usize) -> Result { let encoded_len = T::Balance::from(encoded_len as u32); let transaction_fee = Self::transaction_base_fee() + Self::transaction_byte_fee() * encoded_len; + Self::make_raw_payment(transactor, transaction_fee)?; + Ok(()) + } + + fn make_raw_payment(transactor: &T::AccountId, value: T::Balance) -> Result { let imbalance = Self::withdraw( transactor, - transaction_fee, + value, WithdrawReason::TransactionPayment, ExistenceRequirement::KeepAlive )?; diff --git a/srml/balances/src/tests.rs b/srml/balances/src/tests.rs index 0a5a4b5bb70a6..deea937725466 100644 --- a/srml/balances/src/tests.rs +++ b/srml/balances/src/tests.rs @@ -118,7 +118,7 @@ fn lock_reasons_should_work() { "account liquidity restrictions prevent withdrawal" ); assert_ok!(>::reserve(&1, 1)); - assert_ok!(>::make_payment(&1, 1)); + assert_ok!(>::make_payment(&1, 1)); Balances::set_lock(ID_1, &1, 10, u64::max_value(), WithdrawReason::Reserve.into()); assert_ok!(>::transfer(&1, &2, 1)); @@ -126,13 +126,13 @@ fn lock_reasons_should_work() { >::reserve(&1, 1), "account liquidity restrictions prevent withdrawal" ); - assert_ok!(>::make_payment(&1, 1)); + assert_ok!(>::make_payment(&1, 1)); Balances::set_lock(ID_1, &1, 10, u64::max_value(), WithdrawReason::TransactionPayment.into()); assert_ok!(>::transfer(&1, &2, 1)); assert_ok!(>::reserve(&1, 1)); assert_noop!( - >::make_payment(&1, 1), + >::make_payment(&1, 1), "account liquidity restrictions prevent withdrawal" ); }); diff --git a/srml/executive/src/lib.rs b/srml/executive/src/lib.rs index 331a48c5638dd..2df77ad917c0d 100644 --- a/srml/executive/src/lib.rs +++ b/srml/executive/src/lib.rs @@ -74,20 +74,19 @@ #![cfg_attr(not(feature = "std"), no_std)] -use rstd::prelude::*; -use rstd::marker::PhantomData; -use rstd::result; -use primitives::{generic::{Digest, Tip, Tippable}, traits::{ - self, Header, Zero, One, Checkable, Applyable, CheckEqual, OnFinalize, - OnInitialize, NumberFor, Block as BlockT, OffchainWorker, - ValidateUnsigned, -}}; +use rstd::{prelude::*, marker::PhantomData, result}; +use primitives::{ApplyOutcome, ApplyError, + generic::{Digest, Tip, Tippable}, + weights::Weighable, + transaction_validity::{TransactionValidity, TransactionPriority, TransactionLongevity}, + traits::{self, Header, Zero, One, Checkable, Applyable, CheckEqual, OnFinalize, OnInitialize, + NumberFor, Block as BlockT, OffchainWorker, ValidateUnsigned, SaturatedConversion, + SimpleArithmetic, + }, +}; use srml_support::{Dispatchable, traits::MakePayment}; use parity_codec::{Codec, Encode}; use system::{extrinsics_root, DigestOf}; -use primitives::{ApplyOutcome, ApplyError}; -use primitives::transaction_validity::{TransactionValidity, TransactionPriority, TransactionLongevity}; -use primitives::weights::Weighable; mod internal { pub const MAX_TRANSACTIONS_WEIGHT: u32 = 4 * 1024 * 1024; @@ -124,14 +123,15 @@ impl< System: system::Trait, Block: traits::Block, Context: Default, - Payment: MakePayment, + Payment: MakePayment, Balance, UnsignedValidator, AllModules: OnInitialize + OnFinalize + OffchainWorker, > ExecuteBlock for Executive where Block::Extrinsic: Checkable + Tippable + Codec, - CheckedOf: Applyable + Weighable, + CheckedOf: + Applyable + Weighable + Tippable, CallOf: Dispatchable, OriginOf: From>, UnsignedValidator: ValidateUnsigned>, @@ -145,14 +145,15 @@ impl< System: system::Trait, Block: traits::Block, Context: Default, - Payment: MakePayment, - Balance, + Payment: MakePayment, + Balance: SimpleArithmetic + Copy, UnsignedValidator, AllModules: OnInitialize + OnFinalize + OffchainWorker, > Executive where - Block::Extrinsic: Checkable + Tippable + Codec, - CheckedOf: Applyable + Weighable, + Block::Extrinsic: Checkable + Codec, + CheckedOf: + Applyable + Weighable + Tippable, CallOf: Dispatchable, OriginOf: From>, UnsignedValidator: ValidateUnsigned>, @@ -268,6 +269,8 @@ where // Check the weight of the block if that extrinsic is applied. let weight = xt.weight(encoded_len); + let tip = xt.tip(); + // TODO: maybe reduce the weight if the transaction has a tip? if >::all_extrinsics_weight() + weight > internal::MAX_TRANSACTIONS_WEIGHT { return Err(internal::ApplyError::FullBlock); } @@ -339,13 +342,6 @@ where let encoded_len = uxt.encode().len(); - // TODO: use thi(s to tweak `priority` field. - let tip = uxt.tip(); - let _ = match tip { - Tip::None => (), - _ => (), - }; - let xt = match uxt.check(&Default::default()) { // Checks out. Carry on. Ok(xt) => xt, @@ -364,6 +360,19 @@ where return TransactionValidity::Invalid(ApplyError::CantPay as i8) } + // pay and burn the tip if provided. + let tip = xt.tip(); + let tip_value = match tip { + Tip::Sender(value) => value, + Tip::None => Zero::zero(), + }; + + if !tip_value.is_zero() { + if Payment::make_raw_payment(sender, tip_value).is_err() { + return TransactionValidity::Invalid(ApplyError::CantPay as i8) + } + } + // check index let expected_index = >::account_nonce(sender); if index < &expected_index { @@ -378,8 +387,11 @@ where vec![] }; + // TODO: this for sure takes tip into account but + // - naive addition might not be okay (we want tip to be dominant and it is) + // - is `priority` enough? TransactionValidity::Valid { - priority: encoded_len as TransactionPriority, + priority: (encoded_len as TransactionPriority) + tip_value.saturated_into::(), requires, provides, longevity: TransactionLongevity::max_value(), diff --git a/srml/support/src/inherent.rs b/srml/support/src/inherent.rs index f4498a561cdf8..402066ea9cefa 100644 --- a/srml/support/src/inherent.rs +++ b/srml/support/src/inherent.rs @@ -20,8 +20,6 @@ pub use crate::rstd::vec::Vec; pub use crate::runtime_primitives::traits::{Block as BlockT, Extrinsic}; #[doc(hidden)] pub use inherents::{InherentData, ProvideInherent, CheckInherentsResult, IsFatalError}; -#[doc(hidden)] -pub use crate::runtime_primitives::generic::Tip; /// Implement the outer inherent. @@ -64,7 +62,7 @@ macro_rules! impl_outer_inherent { $( if let Some(inherent) = $module::create_inherent(self) { inherents.push($uncheckedextrinsic::new_unsigned( - Call::$call(inherent), $crate::inherent::Tip::default()) + Call::$call(inherent)) ); } )* diff --git a/srml/support/src/traits.rs b/srml/support/src/traits.rs index 3b3c63e223ce1..2de1acf23a61e 100644 --- a/srml/support/src/traits.rs +++ b/srml/support/src/traits.rs @@ -90,16 +90,20 @@ pub enum UpdateBalanceOutcome { } /// Simple trait designed for hooking into a transaction payment. -/// -/// It operates over a single generic `AccountId` type. -pub trait MakePayment { +pub trait MakePayment { /// Make transaction payment from `who` for an extrinsic of encoded length /// `encoded_len` bytes. Return `Ok` iff the payment was successful. fn make_payment(who: &AccountId, encoded_len: usize) -> Result<(), &'static str>; + + /// Make a raw transactiona payment from `who` with the value `value`. + /// This is most often used to deduct optional fees from a transactor. + /// Return `Ok` iff the payment was successful. + fn make_raw_payment(who: &AccountId, value: Balance) -> Result<(), &'static str>; } -impl MakePayment for () { +impl MakePayment for () { fn make_payment(_: &T, _: usize) -> Result<(), &'static str> { Ok(()) } + fn make_raw_payment(_: &T, _: B) -> Result<(), &'static str> { Ok(()) } } /// Handler for when some currency "account" decreased in balance for From 7326e5a1b5cda09e9d9c72f885d79a0d8080c1d7 Mon Sep 17 00:00:00 2001 From: kianenigma Date: Thu, 27 Jun 2019 18:39:47 +0200 Subject: [PATCH 09/23] Line width --- node/runtime/src/lib.rs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/node/runtime/src/lib.rs b/node/runtime/src/lib.rs index 20a0019010861..93502ede0cce9 100644 --- a/node/runtime/src/lib.rs +++ b/node/runtime/src/lib.rs @@ -281,7 +281,14 @@ pub type UncheckedExtrinsic = generic::UncheckedMortalCompactExtrinsic; /// Executive: handles dispatch to the various modules. -pub type Executive = executive::Executive, Balances, Balance, Runtime, AllModules>; +pub type Executive = executive::Executive< + Runtime, Block, + system::ChainContext, + Balances, + Balance, + Runtime, + AllModules, +>; impl_runtime_apis! { impl client_api::Core for Runtime { From a568a9b987b1697af58bb4a49bb92352a6dd64bb Mon Sep 17 00:00:00 2001 From: Amar Singh Date: Fri, 28 Jun 2019 12:01:34 +0200 Subject: [PATCH 10/23] Update core/sr-primitives/src/generic/tip.rs --- core/sr-primitives/src/generic/tip.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/core/sr-primitives/src/generic/tip.rs b/core/sr-primitives/src/generic/tip.rs index 130a70e5ed413..f840bb5cb9bcc 100644 --- a/core/sr-primitives/src/generic/tip.rs +++ b/core/sr-primitives/src/generic/tip.rs @@ -39,9 +39,9 @@ impl Default for Tip { } } -/// A trait for a generic transaction that contains a tip. The tip itself migth yeild something +/// A trait for a generic transaction that contains a tip. The tip itself might yield something /// that translates to "no tip" but this trait must always be implemented for `UncheckedExtrinsic`. pub trait Tippable { /// Return the tip associated with this transaction. fn tip(&self) -> Tip; -} \ No newline at end of file +} From b3f69cb8319bf01e340c39113ed6068d740a380c Mon Sep 17 00:00:00 2001 From: kianenigma Date: Fri, 28 Jun 2019 16:15:21 +0200 Subject: [PATCH 11/23] Fix build. --- node/executor/src/lib.rs | 6 +++--- srml/executive/src/lib.rs | 6 +----- 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/node/executor/src/lib.rs b/node/executor/src/lib.rs index 26de24f11fef9..957e07d821b0f 100644 --- a/node/executor/src/lib.rs +++ b/node/executor/src/lib.rs @@ -107,7 +107,7 @@ mod tests { match xt.signed { Some((signed, index)) => { let era = Era::mortal(256, 0); - let payload = (index.into(), xt.function, era, GENESIS_HASH); + let payload = (index.into(), xt.function, xt.tip, era, GENESIS_HASH); let key = AccountKeyring::from_public(&signed).unwrap(); let signature = payload.using_encoded(|b| { if b.len() > 256 { @@ -119,13 +119,13 @@ mod tests { UncheckedExtrinsic { signature: Some((indices::address::Address::Id(signed), signature, payload.0, era)), function: payload.1, - tip: Tip::default(), + tip: payload.2, } } None => UncheckedExtrinsic { signature: None, function: xt.function, - tip: Tip::default() + tip: xt.tip, }, } } diff --git a/srml/executive/src/lib.rs b/srml/executive/src/lib.rs index 2df77ad917c0d..9ef3ef1c8542e 100644 --- a/srml/executive/src/lib.rs +++ b/srml/executive/src/lib.rs @@ -269,8 +269,6 @@ where // Check the weight of the block if that extrinsic is applied. let weight = xt.weight(encoded_len); - let tip = xt.tip(); - // TODO: maybe reduce the weight if the transaction has a tip? if >::all_extrinsics_weight() + weight > internal::MAX_TRANSACTIONS_WEIGHT { return Err(internal::ApplyError::FullBlock); } @@ -387,9 +385,7 @@ where vec![] }; - // TODO: this for sure takes tip into account but - // - naive addition might not be okay (we want tip to be dominant and it is) - // - is `priority` enough? + // TODO: maximise (fee + tip) per weight unit here. TransactionValidity::Valid { priority: (encoded_len as TransactionPriority) + tip_value.saturated_into::(), requires, From 8e713aab9a86ffcb563fe671553098732ff23657 Mon Sep 17 00:00:00 2001 From: kianenigma Date: Fri, 28 Jun 2019 16:16:22 +0200 Subject: [PATCH 12/23] Bump. --- node/runtime/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/node/runtime/src/lib.rs b/node/runtime/src/lib.rs index 9407bdd7084db..e6598d8112b64 100644 --- a/node/runtime/src/lib.rs +++ b/node/runtime/src/lib.rs @@ -59,7 +59,7 @@ pub const VERSION: RuntimeVersion = RuntimeVersion { impl_name: create_runtime_str!("substrate-node"), authoring_version: 10, spec_version: 99, - impl_version: 105, + impl_version: 106, apis: RUNTIME_API_VERSIONS, }; From 23df80d7202b66a94bd72bd15125369b6b366ed6 Mon Sep 17 00:00:00 2001 From: kianenigma Date: Sun, 30 Jun 2019 16:31:46 +0200 Subject: [PATCH 13/23] Some cleanup (+should pass the tests). --- core/sr-primitives/src/generic/tip.rs | 12 +++++++++ .../src/generic/unchecked_extrinsic.rs | 15 ++++++----- .../unchecked_mortal_compact_extrinsic.rs | 25 +++++++++++-------- .../src/generic/unchecked_mortal_extrinsic.rs | 24 +++++++++++------- core/sr-primitives/src/lib.rs | 5 +++- node-template/runtime/src/lib.rs | 11 +++++--- node/executor/src/lib.rs | 21 +++++++++------- srml/executive/src/lib.rs | 14 ++++------- srml/support/test/tests/issue2219.rs | 2 +- 9 files changed, 80 insertions(+), 49 deletions(-) diff --git a/core/sr-primitives/src/generic/tip.rs b/core/sr-primitives/src/generic/tip.rs index f840bb5cb9bcc..a81325f7bd0b0 100644 --- a/core/sr-primitives/src/generic/tip.rs +++ b/core/sr-primitives/src/generic/tip.rs @@ -17,6 +17,7 @@ //! Tip structure for a transaction. use crate::codec::{Encode, Decode}; +use crate::traits::Zero; /// Representation of a transaction tip. /// @@ -32,6 +33,17 @@ pub enum Tip { Sender(Balance), } +impl Tip { + /// Return the raw value of the tip (to be burned or consumed) regardless of any logic that the + /// Tip enum variant might embody. + pub fn value(&self) -> Balance { + match *self { + Tip::Sender(value) => value, + Tip::None => Zero::zero(), + } + } +} + /// Default implementation for tip. impl Default for Tip { fn default() -> Self { diff --git a/core/sr-primitives/src/generic/unchecked_extrinsic.rs b/core/sr-primitives/src/generic/unchecked_extrinsic.rs index 066617c57db91..e9224a4430cd9 100644 --- a/core/sr-primitives/src/generic/unchecked_extrinsic.rs +++ b/core/sr-primitives/src/generic/unchecked_extrinsic.rs @@ -104,18 +104,21 @@ where if !crate::verify_encoded_lazy(&signature, &payload, &signed) { return Err(crate::BAD_SIGNATURE) } - // TODO: maybe add tip to checked as well? hmm.. CheckedExtrinsic { signed: Some((signed, payload.0)), function: payload.1, tip: payload.2, } } - None => CheckedExtrinsic { - signed: None, - function: self.function, - // an unsigned transaction cannot have tips. - tip: Tip::None, + None => { + if self.tip == Tip::None { + return Err(crate::UNSIGNED_TIP); + } + CheckedExtrinsic { + signed: None, + function: self.function, + tip: self.tip, + } }, }) } diff --git a/core/sr-primitives/src/generic/unchecked_mortal_compact_extrinsic.rs b/core/sr-primitives/src/generic/unchecked_mortal_compact_extrinsic.rs index 2550a06a1855b..1ea69d0a0dfaa 100644 --- a/core/sr-primitives/src/generic/unchecked_mortal_compact_extrinsic.rs +++ b/core/sr-primitives/src/generic/unchecked_mortal_compact_extrinsic.rs @@ -120,11 +120,15 @@ where tip: raw_payload.2, } } - None => CheckedExtrinsic { - signed: None, - function: self.function, - // an unsigned transaction cannot have tips. - tip: Tip::None, + None => { + // An unsigned transaction cannot have a tip. The decode code should replace it with + // None always and ignore the input bytes. + debug_assert!(self.tip == Tip::None, "{}", crate::UNSIGNED_TIP); + CheckedExtrinsic { + signed: None, + function: self.function, + tip: self.tip + } }, }) } @@ -158,7 +162,7 @@ where Some(UncheckedMortalCompactExtrinsic { signature: if is_signed { Some(Decode::decode(input)?) } else { None }, function: Decode::decode(input)?, - tip: Decode::decode(input).unwrap_or_default(), + tip: if is_signed { Decode::decode(input).unwrap_or_default() } else { Tip::None }, }) } } @@ -185,7 +189,9 @@ where } } self.function.encode_to(v); - self.tip.encode_to(v); + if self.signature.is_some() { + self.tip.encode_to(v); + } }) } } @@ -248,7 +254,6 @@ mod tests { impl traits::Verify for TestSig { type Signer = u64; fn verify>(&self, mut msg: L, signer: &Self::Signer) -> bool { - println!("Test verify for [self = {:?}] and signer {:?} with message {:?}", self, signer, msg.get()); *signer == self.0 && msg.get() == &self.1[..] } } @@ -441,10 +446,10 @@ mod tests { } #[test] + #[should_panic] fn unsigned_cannot_have_tip() { let ux = UncheckedMortalCompactExtrinsic { signature: None, tip: Tip::Sender(100), function: vec![0u8;0]}; - let xt = >::check(ux, &TestContext).unwrap(); - assert_eq!(xt.tip, Tip::None); + let _ = >::check(ux, &TestContext).unwrap(); } #[test] diff --git a/core/sr-primitives/src/generic/unchecked_mortal_extrinsic.rs b/core/sr-primitives/src/generic/unchecked_mortal_extrinsic.rs index 7b62dbb9e878e..dd5ecd2281e4d 100644 --- a/core/sr-primitives/src/generic/unchecked_mortal_extrinsic.rs +++ b/core/sr-primitives/src/generic/unchecked_mortal_extrinsic.rs @@ -134,11 +134,15 @@ where tip: raw_payload.2, } } - None => CheckedExtrinsic { - signed: None, - function: self.function, - // an unsigned transaction cannot have tips. - tip: Tip::None, + None => { + // An unsigned transaction cannot have a tip. The decode code should replace it with + // None always and ignore the input bytes. + debug_assert!(self.tip == Tip::None, "{}", crate::UNSIGNED_TIP); + CheckedExtrinsic { + signed: None, + function: self.function, + tip: self.tip + } }, }) } @@ -171,7 +175,7 @@ where Some(UncheckedMortalExtrinsic { signature: if is_signed { Some(Decode::decode(input)?) } else { None }, function: Decode::decode(input)?, - tip: Decode::decode(input).unwrap_or_default(), + tip: if is_signed { Decode::decode(input).unwrap_or_default() } else { Tip::None }, }) } } @@ -198,7 +202,9 @@ where } } self.function.encode_to(v); - self.tip.encode_to(v); + if self.signature.is_some() { + self.tip.encode_to(v); + } }) } } @@ -439,10 +445,10 @@ mod tests { } #[test] + #[should_panic] fn unsigned_cannot_have_tip() { let ux = UncheckedMortalExtrinsic { signature: None, tip: Tip::Sender(100), function: vec![0u8;0]}; - let xt = >::check(ux, &TestContext).unwrap(); - assert_eq!(xt.tip, Tip::None); + let _ = >::check(ux, &TestContext).unwrap(); } #[test] diff --git a/core/sr-primitives/src/lib.rs b/core/sr-primitives/src/lib.rs index 3874a87d2b680..cef7b50f7b1aa 100644 --- a/core/sr-primitives/src/lib.rs +++ b/core/sr-primitives/src/lib.rs @@ -64,6 +64,9 @@ pub const BAD_SIGNATURE: &str = "bad signature in extrinsic"; /// Example: block gas limit is reached (the transaction can be retried in the next block though). pub const BLOCK_FULL: &str = "block size limit is reached"; +/// Unsigned message containing tip error message. +pub const UNSIGNED_TIP: &str = "unsigned message with tip not allowed"; + /// Justification type. pub type Justification = Vec; @@ -111,7 +114,7 @@ pub trait BuildStorage: Sized { ) -> Result<(), String>; } -/// Somethig that can build the genesis storage of a module. +/// Something that can build the genesis storage of a module. #[cfg(feature = "std")] pub trait BuildModuleGenesisStorage: Sized { /// Create the module genesis storage into the given `storage` and `child_storage`. diff --git a/node-template/runtime/src/lib.rs b/node-template/runtime/src/lib.rs index c174a2b031591..ebbbb16bccdab 100644 --- a/node-template/runtime/src/lib.rs +++ b/node-template/runtime/src/lib.rs @@ -53,6 +53,9 @@ pub type BlockNumber = u64; /// Index of an account's extrinsic in the chain. pub type Nonce = u64; +/// Numeric type of an account's balance. +pub type Balance = u128; + /// Used for the module template in `./template.rs` mod template; @@ -153,7 +156,7 @@ impl timestamp::Trait for Runtime { impl balances::Trait for Runtime { /// The type for recording an account's balance. - type Balance = u128; + type Balance = Balance; /// What to do if an account's free balance gets zeroed. type OnFreeBalanceZero = (); /// What to do if a new account is created. @@ -205,11 +208,11 @@ pub type Block = generic::Block; /// BlockId type as expected by this runtime. pub type BlockId = generic::BlockId; /// Unchecked extrinsic type as expected by this runtime. -pub type UncheckedExtrinsic = generic::UncheckedMortalCompactExtrinsic; +pub type UncheckedExtrinsic = generic::UncheckedMortalCompactExtrinsic; /// Extrinsic type that has already been checked. -pub type CheckedExtrinsic = generic::CheckedExtrinsic; +pub type CheckedExtrinsic = generic::CheckedExtrinsic; /// Executive: handles dispatch to the various modules. -pub type Executive = executive::Executive; +pub type Executive = executive::Executive; // Implement our runtime API endpoints. This is just a bunch of proxying. impl_runtime_apis! { diff --git a/node/executor/src/lib.rs b/node/executor/src/lib.rs index abf36687ec20e..8a3c708da6337 100644 --- a/node/executor/src/lib.rs +++ b/node/executor/src/lib.rs @@ -125,7 +125,7 @@ mod tests { None => UncheckedExtrinsic { signature: None, function: xt.function, - tip: xt.tip, + tip: Tip::None, }, } } @@ -469,7 +469,7 @@ mod tests { CheckedExtrinsic { signed: None, function: Call::Timestamp(timestamp::Call::set(42)), - tip: Tip::default(), + tip: Tip::None, }, CheckedExtrinsic { signed: Some((alice(), 0)), @@ -493,7 +493,7 @@ mod tests { CheckedExtrinsic { signed: None, function: Call::Timestamp(timestamp::Call::set(42)), - tip: Tip::default(), + tip: Tip::None, }, CheckedExtrinsic { signed: Some((alice(), 0)), @@ -542,7 +542,7 @@ mod tests { CheckedExtrinsic { signed: None, function: Call::Timestamp(timestamp::Call::set(42)), - tip: Tip::default(), + tip: Tip::None, }, CheckedExtrinsic { signed: Some((alice(), 0)), @@ -822,7 +822,7 @@ mod tests { CheckedExtrinsic { signed: None, function: Call::Timestamp(timestamp::Call::set(42)), - tip: Tip::default(), + tip: Tip::None, }, CheckedExtrinsic { signed: Some((charlie(), 0)), @@ -887,14 +887,17 @@ mod tests { #[test] fn native_big_block_import_succeeds() { let mut t = new_test_ext(COMPACT_CODE, false); - - Executor::new(None).call::<_, NeverNativeValue, fn() -> _>( + let block = big_block().0; + println!("Shit will get real here"); + let r = Executor::new(None).call::<_, NeverNativeValue, fn() -> _>( &mut t, "Core_execute_block", - &big_block().0, + &block, true, None, - ).0.unwrap(); + ); + println!("R = {:?}", r); + r.0.unwrap(); } #[test] diff --git a/srml/executive/src/lib.rs b/srml/executive/src/lib.rs index 0082a0db8a8cf..f5a6f9362801e 100644 --- a/srml/executive/src/lib.rs +++ b/srml/executive/src/lib.rs @@ -76,7 +76,7 @@ use rstd::{prelude::*, marker::PhantomData, result}; use primitives::{ApplyOutcome, ApplyError, - generic::{Digest, Tip, Tippable}, + generic::{Digest, Tippable}, weights::Weighable, transaction_validity::{TransactionValidity, TransactionPriority, TransactionLongevity}, traits::{self, Header, Zero, One, Checkable, Applyable, CheckEqual, OnFinalize, OnInitialize, @@ -359,14 +359,10 @@ where } // pay and burn the tip if provided. - let tip = xt.tip(); - let tip_value = match tip { - Tip::Sender(value) => value, - Tip::None => Zero::zero(), - }; + let tip = xt.tip().value(); - if !tip_value.is_zero() { - if Payment::make_raw_payment(sender, tip_value).is_err() { + if !tip.is_zero() { + if Payment::make_raw_payment(sender, tip).is_err() { return TransactionValidity::Invalid(ApplyError::CantPay as i8) } } @@ -387,7 +383,7 @@ where // TODO: maximise (fee + tip) per weight unit here. TransactionValidity::Valid { - priority: (encoded_len as TransactionPriority) + tip_value.saturated_into::(), + priority: (encoded_len as TransactionPriority) + tip.saturated_into::(), requires, provides, longevity: TransactionLongevity::max_value(), diff --git a/srml/support/test/tests/issue2219.rs b/srml/support/test/tests/issue2219.rs index 185b5e24807a9..590d31590b615 100644 --- a/srml/support/test/tests/issue2219.rs +++ b/srml/support/test/tests/issue2219.rs @@ -152,7 +152,7 @@ pub type BlockNumber = u64; pub type Index = u64; pub type Header = generic::Header; pub type Block = generic::Block; -pub type UncheckedExtrinsic = generic::UncheckedMortalCompactExtrinsic; +pub type UncheckedExtrinsic = generic::UncheckedMortalCompactExtrinsic; impl system::Trait for Runtime { type Hash = H256; From 83721a48a59f7f0e5582c2e6dc2d12bf47e4c361 Mon Sep 17 00:00:00 2001 From: kianenigma Date: Sun, 30 Jun 2019 16:54:06 +0200 Subject: [PATCH 14/23] Fix sync test payload. --- node/cli/src/service.rs | 5 +++-- srml/executive/src/lib.rs | 1 + 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/node/cli/src/service.rs b/node/cli/src/service.rs index 5f4831f236439..cb57c70a69253 100644 --- a/node/cli/src/service.rs +++ b/node/cli/src/service.rs @@ -217,7 +217,7 @@ mod tests { use std::sync::Arc; use consensus::CompatibleDigestItem; use consensus_common::{Environment, Proposer, ImportBlock, BlockOrigin, ForkChoiceStrategy}; - use node_primitives::DigestItem; + use node_primitives::{DigestItem, Balance}; use node_runtime::{Call, BalancesCall, UncheckedExtrinsic}; use parity_codec::{Compact, Encode, Decode}; use primitives::{ @@ -269,6 +269,7 @@ mod tests { let payload = ( 0, Call::Balances(BalancesCall::transfer(RawAddress::Id(bob.public().0.into()), 69.into())), + Tip::default(), Era::immortal(), service.client().genesis_hash() ); @@ -359,7 +360,7 @@ mod tests { let function = Call::Balances(BalancesCall::transfer(to.into(), amount)); let era = Era::immortal(); - let raw_payload = (Compact(index), function, era, genesis_hash); + let raw_payload = (Compact(index), function, Tip::::default(), era, genesis_hash); let signature = raw_payload.using_encoded(|payload| if payload.len() > 256 { signer.sign(&blake2_256(payload)[..]) } else { diff --git a/srml/executive/src/lib.rs b/srml/executive/src/lib.rs index f5a6f9362801e..b57472ef143f4 100644 --- a/srml/executive/src/lib.rs +++ b/srml/executive/src/lib.rs @@ -360,6 +360,7 @@ where // pay and burn the tip if provided. let tip = xt.tip().value(); + let weight = xt.weight(encoded_len); if !tip.is_zero() { if Payment::make_raw_payment(sender, tip).is_err() { From bd28f0d81d2482be9f363ebd17a1dcadb5e3273d Mon Sep 17 00:00:00 2001 From: kianenigma Date: Sun, 30 Jun 2019 17:07:46 +0200 Subject: [PATCH 15/23] Fix subkey and factory sig --- core/consensus/rhd/src/lib.rs | 2 ++ node/cli/src/factory_impl.rs | 4 ++-- subkey/src/main.rs | 4 ++-- 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/core/consensus/rhd/src/lib.rs b/core/consensus/rhd/src/lib.rs index 4a3e03759b4d4..efe36568527ca 100644 --- a/core/consensus/rhd/src/lib.rs +++ b/core/consensus/rhd/src/lib.rs @@ -1263,6 +1263,8 @@ impl LocalProposer<::Block> for Proposer where => MisbehaviorKind::BftDoubleCommit(round as u32, (h1.into(), s1.signature), (h2.into(), s2.signature)), } }; + /// TODO: This is not compatible with the new signature payload. + /// a tip must be present as the third argument. let payload = ( next_index, Call::Consensus(ConsensusCall::report_misbehavior(report)), diff --git a/node/cli/src/factory_impl.rs b/node/cli/src/factory_impl.rs index 4d753c012aaec..2d60e9fc970d0 100644 --- a/node/cli/src/factory_impl.rs +++ b/node/cli/src/factory_impl.rs @@ -23,7 +23,7 @@ use rand::rngs::StdRng; use parity_codec::Decode; use keyring::sr25519::Keyring; -use node_primitives::Hash; +use node_primitives::{Hash, Balance}; use node_runtime::{Call, CheckedExtrinsic, UncheckedExtrinsic, BalancesCall}; use primitives::sr25519; use primitives::crypto::Pair; @@ -236,7 +236,7 @@ fn sign( let s = match xt.signed { Some((signed, index)) => { let era = Era::mortal(256, phase); - let payload = (index.into(), xt.function, era, prior_block_hash); + let payload = (index.into(), xt.function, Tip::::default(), era, prior_block_hash); let signature = payload.using_encoded(|b| { if b.len() > 256 { key.sign(&sr_io::blake2_256(b)) diff --git a/subkey/src/main.rs b/subkey/src/main.rs index 99d6cbf714bff..8f42ff68bfc51 100644 --- a/subkey/src/main.rs +++ b/subkey/src/main.rs @@ -153,7 +153,7 @@ fn execute(matches: clap::ArgMatches) where println!("Using a genesis hash of {}", HexDisplay::from(&genesis_hash.as_ref())); let era = Era::immortal(); - let raw_payload = (Compact(index), function, era, genesis_hash); + let raw_payload = (Compact(index), function, Tip::::default(), era, genesis_hash); let signature = raw_payload.using_encoded(|payload| if payload.len() > 256 { signer.sign(&blake2_256(payload)[..]) } else { @@ -193,7 +193,7 @@ fn execute(matches: clap::ArgMatches) where let era = Era::immortal(); - let raw_payload = (Compact(index), function, era, prior_block_hash); + let raw_payload = (Compact(index), function, era, Tip::::default(), prior_block_hash); let signature = raw_payload.using_encoded(|payload| if payload.len() > 256 { signer.sign(&blake2_256(payload)[..]) From 120de208f245781f57ae20a1be100654627c3860 Mon Sep 17 00:00:00 2001 From: kianenigma Date: Tue, 9 Jul 2019 17:18:05 +0200 Subject: [PATCH 16/23] revert back old unchecked ext type to NOT use tip. --- core/consensus/rhd/src/lib.rs | 4 +- .../src/generic/checked_extrinsic.rs | 32 +- core/sr-primitives/src/generic/mod.rs | 2 + .../src/generic/unchecked_extrinsic.rs | 100 ++-- .../unchecked_mortal_compact_extrinsic.rs | 230 ++------- ...checked_mortal_compact_tipped_extrinsic.rs | 468 ++++++++++++++++++ .../src/generic/unchecked_mortal_extrinsic.rs | 228 ++------- node-template/runtime/src/lib.rs | 2 +- node/cli/src/factory_impl.rs | 2 - node/cli/src/service.rs | 3 - node/executor/src/lib.rs | 4 +- node/runtime/src/lib.rs | 2 +- srml/support/test/tests/instance.rs | 2 +- srml/support/test/tests/issue2219.rs | 2 +- subkey/src/main.rs | 2 - 15 files changed, 620 insertions(+), 463 deletions(-) create mode 100644 core/sr-primitives/src/generic/unchecked_mortal_compact_tipped_extrinsic.rs diff --git a/core/consensus/rhd/src/lib.rs b/core/consensus/rhd/src/lib.rs index efe36568527ca..405ae61606cf9 100644 --- a/core/consensus/rhd/src/lib.rs +++ b/core/consensus/rhd/src/lib.rs @@ -14,6 +14,8 @@ // You should have received a copy of the GNU General Public License // along with Substrate. If not, see . +//! DEPRECATED. +//! //! BFT Agreement based on a rotating proposer in different rounds. //! //! Where this crate refers to input stream, should never logically conclude. @@ -1263,7 +1265,7 @@ impl LocalProposer<::Block> for Proposer where => MisbehaviorKind::BftDoubleCommit(round as u32, (h1.into(), s1.signature), (h2.into(), s2.signature)), } }; - /// TODO: This is not compatible with the new signature payload. + /// NOTE: This is not compatible with the new signature payload. /// a tip must be present as the third argument. let payload = ( next_index, diff --git a/core/sr-primitives/src/generic/checked_extrinsic.rs b/core/sr-primitives/src/generic/checked_extrinsic.rs index 1ab7c3d741dfe..ee90a1c75f0b4 100644 --- a/core/sr-primitives/src/generic/checked_extrinsic.rs +++ b/core/sr-primitives/src/generic/checked_extrinsic.rs @@ -17,10 +17,11 @@ //! Generic implementation of an extrinsic that has passed the verification //! stage. -use crate::traits::{self, Member, SimpleArithmetic, MaybeDisplay}; +use crate::traits::{self, Member, SimpleArithmetic, MaybeDisplay, SaturatedConversion}; use crate::weights::{Weighable, Weight}; use crate::generic::tip::{Tip, Tippable}; + /// Definition of something that the external world might want to say; its /// existence implies that it has been checked and is good, particularly with /// regards to the signature. @@ -69,12 +70,31 @@ where } } -impl Tippable - for CheckedExtrinsic +/// `ExtBalance` is the balance type fed by the `check()` implementation of various unchecked +/// extrinsics. `NodeBalance` is the actual balance type used as a primitive type of the substrate +/// node. +/// +/// In practice, if they underlying unchecked transaction is tip-aware, they are the same. Otherwise, +/// the tip is always `Tip::None` and the type is of no importance. +impl Tippable + for CheckedExtrinsic where - Balance: Clone, + ExtBalance: Clone + Copy, + NodeBalance: From, { - fn tip(&self) -> Tip { - self.tip.clone() + fn tip(&self) -> Tip { + // This is a hacky way to prevent `unchecked_mortal[_compact]_extrinsic` types, which + // don't have a tip, become generic over a balance type. + // Basically, this CheckedExtrinsic is built either 1- from an + // `UncheckedMortalCompactTippedExtrinsic`, which is tip aware and hence, the second arm + // will be trivially executed and the type conversion will be safe (the compiler is probably) + // smart enough to remove it in fact. Or 2- this is built from all other types of unchecked + // extrinsic which do not have tip and hence are not balance-aware. These module will naively + // place a u64 (can be `()` in practice) as the type and it does not matter since `Tip::None` + // is used in this case. + match self.tip { + Tip::None => >::None, + >::Sender(v) => >::Sender(NodeBalance::from(v)) + } } } \ No newline at end of file diff --git a/core/sr-primitives/src/generic/mod.rs b/core/sr-primitives/src/generic/mod.rs index 4b4b1a182cd60..d61593df62186 100644 --- a/core/sr-primitives/src/generic/mod.rs +++ b/core/sr-primitives/src/generic/mod.rs @@ -21,6 +21,7 @@ mod unchecked_extrinsic; mod unchecked_mortal_extrinsic; mod unchecked_mortal_compact_extrinsic; +mod unchecked_mortal_compact_tipped_extrinsic; mod era; mod checked_extrinsic; mod header; @@ -33,6 +34,7 @@ mod tests; pub use self::unchecked_extrinsic::UncheckedExtrinsic; pub use self::unchecked_mortal_extrinsic::UncheckedMortalExtrinsic; pub use self::unchecked_mortal_compact_extrinsic::UncheckedMortalCompactExtrinsic; +pub use self::unchecked_mortal_compact_tipped_extrinsic::UncheckedMortalCompactTippedExtrinsic; pub use self::era::{Era, Phase}; pub use self::checked_extrinsic::CheckedExtrinsic; pub use self::header::Header; diff --git a/core/sr-primitives/src/generic/unchecked_extrinsic.rs b/core/sr-primitives/src/generic/unchecked_extrinsic.rs index e9224a4430cd9..897a89e492c15 100644 --- a/core/sr-primitives/src/generic/unchecked_extrinsic.rs +++ b/core/sr-primitives/src/generic/unchecked_extrinsic.rs @@ -38,7 +38,7 @@ where /// A extrinsic right from the external world. This is unchecked and so /// can contain a signature. #[derive(PartialEq, Eq, Clone)] -pub struct UncheckedExtrinsic +pub struct UncheckedExtrinsic where Address: Codec, Index: HasCompact + Codec, @@ -49,27 +49,19 @@ where pub signature: Option>, /// The function that should be called. pub function: Call, - /// The tip for this transaction - pub tip: Tip, } -impl UncheckedExtrinsic +impl UncheckedExtrinsic where Address: Codec, Index: HasCompact + Codec, Signature: Codec, { /// New instance of a signed extrinsic aka "transaction". - pub fn new_signed(index: Index, - function: Call, - signed: Address, - signature: Signature, - tip: Tip, - ) -> Self { + pub fn new_signed(index: Index, function: Call, signed: Address, signature: Signature) -> Self { UncheckedExtrinsic { signature: Some(SignatureContent{signed, signature, index}), function, - tip, } } @@ -78,13 +70,12 @@ where UncheckedExtrinsic { signature: None, function, - tip: Tip::None, } } } -impl traits::Checkable - for UncheckedExtrinsic +impl traits::Checkable + for UncheckedExtrinsic where Address: Member + MaybeDisplay + Codec, Index: Member + MaybeDisplay + SimpleArithmetic + Codec, @@ -92,14 +83,13 @@ where Signature: Member + traits::Verify + Codec, AccountId: Member + MaybeDisplay, Context: Lookup, - Balance: Member + Encode, { - type Checked = CheckedExtrinsic; + type Checked = CheckedExtrinsic; fn check(self, context: &Context) -> Result { Ok(match self.signature { Some(SignatureContent{signed, signature, index}) => { - let payload = (index, self.function, self.tip); + let payload = (index, self.function); let signed = context.lookup(signed)?; if !crate::verify_encoded_lazy(&signature, &payload, &signed) { return Err(crate::BAD_SIGNATURE) @@ -107,18 +97,13 @@ where CheckedExtrinsic { signed: Some((signed, payload.0)), function: payload.1, - tip: payload.2, + tip: Tip::None, } } - None => { - if self.tip == Tip::None { - return Err(crate::UNSIGNED_TIP); - } - CheckedExtrinsic { - signed: None, - function: self.function, - tip: self.tip, - } + None => CheckedExtrinsic { + signed: None, + function: self.function, + tip: Tip::None, }, }) } @@ -129,20 +114,15 @@ impl< Index: HasCompact + Codec, Signature: Codec, Call, - Balance, -> Extrinsic for UncheckedExtrinsic { +> Extrinsic for UncheckedExtrinsic { fn is_signed(&self) -> Option { Some(self.signature.is_some()) } } -impl< - Address: Codec, - Index: HasCompact + Codec, - Signature: Codec, - Call: Decode, - Balance: Decode -> Decode for UncheckedExtrinsic { +impl Decode + for UncheckedExtrinsic +{ fn decode(input: &mut I) -> Option { // This is a little more complicated than usual since the binary format must be compatible // with substrate's generic `Vec` type. Basically this just means accepting that there @@ -153,70 +133,52 @@ impl< Some(UncheckedExtrinsic { signature: Decode::decode(input)?, function: Decode::decode(input)?, - tip: Decode::decode(input).unwrap_or_default(), }) } } -impl< - Address: Codec, - Index: HasCompact + Codec, - Signature: Codec, - Call: Encode, - Balance: Encode -> Encode for UncheckedExtrinsic { +impl Encode + for UncheckedExtrinsic +{ fn encode(&self) -> Vec { super::encode_with_vec_prefix::(|v| { self.signature.encode_to(v); self.function.encode_to(v); - self.tip.encode_to(v); }) } } #[cfg(feature = "std")] -impl< - Address: Codec, - Index: HasCompact + Codec, - Signature: Codec, - Call: Encode, - Balance: Codec -> serde::Serialize for UncheckedExtrinsic { +impl serde::Serialize + for UncheckedExtrinsic +{ fn serialize(&self, seq: S) -> Result where S: ::serde::Serializer { self.using_encoded(|bytes| ::substrate_primitives::bytes::serialize(bytes, seq)) } } #[cfg(feature = "std")] -impl fmt::Debug - for UncheckedExtrinsic +impl fmt::Debug + for UncheckedExtrinsic where Address: fmt::Debug + Codec, Index: fmt::Debug + HasCompact + Codec, Signature: Codec, Call: fmt::Debug, - Balance: fmt::Debug, { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - write!( - f, - "UncheckedExtrinsic({:?}, {:?}, {:?})", - self.signature.as_ref().map(|x| (&x.signed, &x.index)), - self.function, - self.tip, - ) + write!(f, "UncheckedExtrinsic({:?}, {:?})", self.signature.as_ref().map(|x| (&x.signed, &x.index)), self.function) } } #[cfg(test)] mod test { use crate::codec::{Decode, Encode}; - use super::{UncheckedExtrinsic}; - type Extrinsic = UncheckedExtrinsic; - + use super::UncheckedExtrinsic; #[test] fn encoding_matches_vec() { + type Extrinsic = UncheckedExtrinsic; let ex = Extrinsic::new_unsigned(42); let encoded = ex.encode(); let decoded = Extrinsic::decode(&mut encoded.as_slice()).unwrap(); @@ -225,11 +187,13 @@ mod test { assert_eq!(as_vec.encode(), encoded); } + #[test] #[cfg(feature = "std")] fn serialization_of_unchecked_extrinsics() { - type Extrinsic = UncheckedExtrinsic; + type Extrinsic = UncheckedExtrinsic; let ex = Extrinsic::new_unsigned(42); - assert_eq!(serde_json::to_string(&ex).unwrap(), "\"0x18002a00000000\""); + + assert_eq!(serde_json::to_string(&ex).unwrap(), "\"0x14002a000000\""); } -} +} \ No newline at end of file diff --git a/core/sr-primitives/src/generic/unchecked_mortal_compact_extrinsic.rs b/core/sr-primitives/src/generic/unchecked_mortal_compact_extrinsic.rs index 1ea69d0a0dfaa..cd768a61fb87c 100644 --- a/core/sr-primitives/src/generic/unchecked_mortal_compact_extrinsic.rs +++ b/core/sr-primitives/src/generic/unchecked_mortal_compact_extrinsic.rs @@ -31,33 +31,23 @@ const TRANSACTION_VERSION: u8 = 1; /// A extrinsic right from the external world. This is unchecked and so /// can contain a signature. #[derive(PartialEq, Eq, Clone)] -pub struct UncheckedMortalCompactExtrinsic { +pub struct UncheckedMortalCompactExtrinsic { /// The signature, address, number of extrinsics have come before from /// the same signer and an era describing the longevity of this transaction, /// if this is a signed extrinsic. pub signature: Option<(Address, Signature, Compact, Era)>, /// The function that should be called. pub function: Call, - /// The tip for this transaction - pub tip: Tip, } -impl - UncheckedMortalCompactExtrinsic +impl +UncheckedMortalCompactExtrinsic { /// New instance of a signed extrinsic aka "transaction". - pub fn new_signed( - index: Index, - function: Call, - signed: Address, - signature: Signature, - era: Era, - tip: Tip - ) -> Self { + pub fn new_signed(index: Index, function: Call, signed: Address, signature: Signature, era: Era) -> Self { UncheckedMortalCompactExtrinsic { signature: Some((signed, signature, index.into(), era)), function, - tip, } } @@ -66,21 +56,20 @@ impl UncheckedMortalCompactExtrinsic { signature: None, function, - tip: Tip::None } } } -impl Extrinsic - for UncheckedMortalCompactExtrinsic +impl Extrinsic + for UncheckedMortalCompactExtrinsic { fn is_signed(&self) -> Option { Some(self.signature.is_some()) } } -impl Checkable - for UncheckedMortalCompactExtrinsic +impl Checkable + for UncheckedMortalCompactExtrinsic where Address: Member + MaybeDisplay, Index: Member + MaybeDisplay + SimpleArithmetic, @@ -93,9 +82,11 @@ where Context: Lookup + CurrentHeight + BlockNumberToHash, - Balance: Member + Encode, { - type Checked = CheckedExtrinsic; + /// NOTE: this transaction is not tipped i.e. the tip value will be `None`. It does not really + /// matter what the last generic is since it is always `None`. + // TODO: try and use () here since it is a bit nicer. + type Checked = CheckedExtrinsic; fn check(self, context: &Context) -> Result { Ok(match self.signature { @@ -104,7 +95,7 @@ where let h = context.block_number_to_hash(era.birth(current_u64).saturated_into()) .ok_or("transaction birth block ancient")?; let signed = context.lookup(signed)?; - let raw_payload = (index, self.function, self.tip, era, h); + let raw_payload = (index, self.function, era, h); if !raw_payload.using_encoded(|payload| { if payload.len() > 256 { signature.verify(&blake2_256(payload)[..], &signed) @@ -117,32 +108,25 @@ where CheckedExtrinsic { signed: Some((signed, (raw_payload.0).0)), function: raw_payload.1, - tip: raw_payload.2, + tip: Tip::None, } } - None => { - // An unsigned transaction cannot have a tip. The decode code should replace it with - // None always and ignore the input bytes. - debug_assert!(self.tip == Tip::None, "{}", crate::UNSIGNED_TIP); - CheckedExtrinsic { - signed: None, - function: self.function, - tip: self.tip - } + None => CheckedExtrinsic { + signed: None, + function: self.function, + tip: Tip::None, }, }) } } -impl Decode - for UncheckedMortalCompactExtrinsic +impl Decode + for UncheckedMortalCompactExtrinsic where Address: Decode, Signature: Decode, Compact: Decode, Call: Decode, - Balance: Decode, - { fn decode(input: &mut I) -> Option { // This is a little more complicated than usual since the binary format must be compatible @@ -162,19 +146,17 @@ where Some(UncheckedMortalCompactExtrinsic { signature: if is_signed { Some(Decode::decode(input)?) } else { None }, function: Decode::decode(input)?, - tip: if is_signed { Decode::decode(input).unwrap_or_default() } else { Tip::None }, }) } } -impl Encode - for UncheckedMortalCompactExtrinsic +impl Encode + for UncheckedMortalCompactExtrinsic where Address: Encode, Signature: Encode, Compact: Encode, Call: Encode, - Balance: Encode, { fn encode(&self) -> Vec { super::encode_with_vec_prefix::(|v| { @@ -189,16 +171,13 @@ where } } self.function.encode_to(v); - if self.signature.is_some() { - self.tip.encode_to(v); - } }) } } #[cfg(feature = "std")] -impl serde::Serialize - for UncheckedMortalCompactExtrinsic +impl serde::Serialize + for UncheckedMortalCompactExtrinsic where Compact: Encode { fn serialize(&self, seq: S) -> Result where S: ::serde::Serializer { @@ -207,21 +186,19 @@ impl s } #[cfg(feature = "std")] -impl fmt::Debug - for UncheckedMortalCompactExtrinsic +impl fmt::Debug + for UncheckedMortalCompactExtrinsic where Address: fmt::Debug, Index: fmt::Debug, Call: fmt::Debug, - Balance: fmt::Debug, { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { write!( f, - "UncheckedMortalCompactExtrinsic({:?}, {:?}, {:?})", + "UncheckedMortalCompactExtrinsic({:?}, {:?})", self.signature.as_ref().map(|x| (&x.0, &x.2)), - self.function, - self.tip, + self.function ) } } @@ -251,20 +228,17 @@ mod tests { #[derive(Eq, PartialEq, Clone, Debug, Serialize, Deserialize, Encode, Decode)] struct TestSig(u64, Vec); - impl traits::Verify for TestSig { + impl traits::Verify for TestSig { type Signer = u64; fn verify>(&self, mut msg: L, signer: &Self::Signer) -> bool { *signer == self.0 && msg.get() == &self.1[..] } } - type Balance = u64; - const DUMMY_ACCOUNTID: u64 = 0; - const TIP: Tip = Tip::Sender(66); - type Ex = UncheckedMortalCompactExtrinsic, TestSig, Balance>; - type CEx = CheckedExtrinsic, Balance>; + type Ex = UncheckedMortalCompactExtrinsic, TestSig>; + type CEx = CheckedExtrinsic, u64>; #[test] fn unsigned_codec_should_work() { @@ -275,31 +249,14 @@ mod tests { #[test] fn signed_codec_should_work() { - let ux = Ex::new_signed( - 0, - vec![0u8; 0], - DUMMY_ACCOUNTID, - TestSig(DUMMY_ACCOUNTID, (DUMMY_ACCOUNTID, vec![0u8;0], TIP, Era::immortal(), 0u64).encode()), - Era::immortal(), - TIP - ); + let ux = Ex::new_signed(0, vec![0u8;0], DUMMY_ACCOUNTID, TestSig(DUMMY_ACCOUNTID, (DUMMY_ACCOUNTID, vec![0u8;0], Era::immortal(), 0u64).encode()), Era::immortal()); let encoded = ux.encode(); assert_eq!(Ex::decode(&mut &encoded[..]), Some(ux)); } #[test] fn large_signed_codec_should_work() { - let ux = Ex::new_signed( - 0, - vec![0u8; 0], - DUMMY_ACCOUNTID, - TestSig( - DUMMY_ACCOUNTID, - (DUMMY_ACCOUNTID, vec![0u8; 257], TIP, Era::immortal(), 0u64).using_encoded(blake2_256)[..].to_owned() - ), - Era::immortal(), - TIP - ); + let ux = Ex::new_signed(0, vec![0u8;0], DUMMY_ACCOUNTID, TestSig(DUMMY_ACCOUNTID, (DUMMY_ACCOUNTID, vec![0u8; 257], Era::immortal(), 0u64).using_encoded(blake2_256)[..].to_owned()), Era::immortal()); let encoded = ux.encode(); assert_eq!(Ex::decode(&mut &encoded[..]), Some(ux)); } @@ -313,107 +270,42 @@ mod tests { #[test] fn badly_signed_check_should_fail() { - let ux = Ex::new_signed( - 0, - vec![0u8; 0], - DUMMY_ACCOUNTID, - TestSig(DUMMY_ACCOUNTID, vec![0u8]), - Era::immortal(), - TIP, - ); + let ux = Ex::new_signed(0, vec![0u8;0], DUMMY_ACCOUNTID, TestSig(DUMMY_ACCOUNTID, vec![0u8]), Era::immortal()); assert!(ux.is_signed().unwrap_or(false)); assert_eq!(>::check(ux, &TestContext), Err(crate::BAD_SIGNATURE)); } #[test] fn immortal_signed_check_should_work() { - let ux = Ex::new_signed( - 0, - vec![0u8;0], - DUMMY_ACCOUNTID, - TestSig( - DUMMY_ACCOUNTID, - (Compact::from(DUMMY_ACCOUNTID), vec![0u8;0], TIP, Era::immortal(), 0u64).encode() - ), - Era::immortal(), - TIP, - ); + let ux = Ex::new_signed(0, vec![0u8;0], DUMMY_ACCOUNTID, TestSig(DUMMY_ACCOUNTID, (Compact::from(DUMMY_ACCOUNTID), vec![0u8;0], Era::immortal(), 0u64).encode()), Era::immortal()); assert!(ux.is_signed().unwrap_or(false)); - assert_eq!( - >::check(ux, &TestContext), - Ok(CEx { signed: Some((DUMMY_ACCOUNTID, 0)), function: vec![0u8;0], tip: TIP }) - ); + assert_eq!(>::check(ux, &TestContext), Ok(CEx { signed: Some((DUMMY_ACCOUNTID, 0)), function: vec![0u8;0], tip: Tip::None })); } #[test] fn mortal_signed_check_should_work() { - let ux = Ex::new_signed( - 0, - vec![0u8;0], - DUMMY_ACCOUNTID, - TestSig( - DUMMY_ACCOUNTID, - (Compact::from(DUMMY_ACCOUNTID), vec![0u8;0], TIP, Era::mortal(32, 42), 42u64).encode() - ), - Era::mortal(32, 42), - TIP, - ); + let ux = Ex::new_signed(0, vec![0u8;0], DUMMY_ACCOUNTID, TestSig(DUMMY_ACCOUNTID, (Compact::from(DUMMY_ACCOUNTID), vec![0u8;0], Era::mortal(32, 42), 42u64).encode()), Era::mortal(32, 42)); assert!(ux.is_signed().unwrap_or(false)); - assert_eq!( - >::check(ux, &TestContext), - Ok(CEx { signed: Some((DUMMY_ACCOUNTID, 0)), function: vec![0u8;0], tip: TIP })); + assert_eq!(>::check(ux, &TestContext), Ok(CEx { signed: Some((DUMMY_ACCOUNTID, 0)), function: vec![0u8;0], tip: Tip::None })); } #[test] fn later_mortal_signed_check_should_work() { - let ux = Ex::new_signed( - 0, - vec![0u8;0], - DUMMY_ACCOUNTID, - TestSig( - DUMMY_ACCOUNTID, - (Compact::from(DUMMY_ACCOUNTID), vec![0u8;0], TIP, Era::mortal(32, 11), 11u64).encode() - ), - Era::mortal(32, 11), - TIP, - ); + let ux = Ex::new_signed(0, vec![0u8;0], DUMMY_ACCOUNTID, TestSig(DUMMY_ACCOUNTID, (Compact::from(DUMMY_ACCOUNTID), vec![0u8;0], Era::mortal(32, 11), 11u64).encode()), Era::mortal(32, 11)); assert!(ux.is_signed().unwrap_or(false)); - assert_eq!( - >::check(ux, &TestContext), - Ok(CEx { signed: Some((DUMMY_ACCOUNTID, 0)), function: vec![0u8;0], tip: TIP }) - ); + assert_eq!(>::check(ux, &TestContext), Ok(CEx { signed: Some((DUMMY_ACCOUNTID, 0)), function: vec![0u8;0], tip: Tip::None })); } #[test] fn too_late_mortal_signed_check_should_fail() { - let ux = Ex::new_signed( - 0, - vec![0u8;0], - DUMMY_ACCOUNTID, - TestSig( - DUMMY_ACCOUNTID, - (DUMMY_ACCOUNTID, vec![0u8;0], TIP, Era::mortal(32, 10), 10u64).encode() - ), - Era::mortal(32, 10), - TIP, - ); + let ux = Ex::new_signed(0, vec![0u8;0], DUMMY_ACCOUNTID, TestSig(DUMMY_ACCOUNTID, (DUMMY_ACCOUNTID, vec![0u8;0], Era::mortal(32, 10), 10u64).encode()), Era::mortal(32, 10)); assert!(ux.is_signed().unwrap_or(false)); assert_eq!(>::check(ux, &TestContext), Err(crate::BAD_SIGNATURE)); } #[test] fn too_early_mortal_signed_check_should_fail() { - let ux = Ex::new_signed( - 0, - vec![0u8;0], - DUMMY_ACCOUNTID, - TestSig( - DUMMY_ACCOUNTID, - (DUMMY_ACCOUNTID, vec![0u8;0], TIP, Era::mortal(32, 43), 43u64).encode() - ), - Era::mortal(32, 43), - TIP, - ); + let ux = Ex::new_signed(0, vec![0u8;0], DUMMY_ACCOUNTID, TestSig(DUMMY_ACCOUNTID, (DUMMY_ACCOUNTID, vec![0u8;0], Era::mortal(32, 43), 43u64).encode()), Era::mortal(32, 43)); assert!(ux.is_signed().unwrap_or(false)); assert_eq!(>::check(ux, &TestContext), Err(crate::BAD_SIGNATURE)); } @@ -427,40 +319,4 @@ mod tests { let as_vec: Vec = Decode::decode(&mut encoded.as_slice()).unwrap(); assert_eq!(as_vec.encode(), encoded); } - - #[test] - fn missing_tip_in_signature_payload_should_fail() { - let ux = Ex::new_signed( - 0, - vec![0u8;0], - DUMMY_ACCOUNTID, - TestSig( - DUMMY_ACCOUNTID, - (DUMMY_ACCOUNTID, vec![0u8;0], Era::immortal(), 0u64).encode() - ), - Era::immortal(), - TIP, - ); - assert!(ux.is_signed().unwrap_or(false)); - assert_eq!(>::check(ux, &TestContext), Err(crate::BAD_SIGNATURE)); - } - - #[test] - #[should_panic] - fn unsigned_cannot_have_tip() { - let ux = UncheckedMortalCompactExtrinsic { signature: None, tip: Tip::Sender(100), function: vec![0u8;0]}; - let _ = >::check(ux, &TestContext).unwrap(); - } - - #[test] - fn unprovided_tip_will_not_fail() { - // unsigned with no tips - let bytes = vec![40, 1, 32, 8, 8, 8, 8, 8, 8, 8, 8]; - - // decoding will yield the default tip - let decoded = Ex::decode(&mut bytes.as_slice()).unwrap(); - assert_eq!( - decoded, - UncheckedMortalCompactExtrinsic { signature: None, tip: Tip::None, function: vec![8u8;8]}) - } -} +} \ No newline at end of file diff --git a/core/sr-primitives/src/generic/unchecked_mortal_compact_tipped_extrinsic.rs b/core/sr-primitives/src/generic/unchecked_mortal_compact_tipped_extrinsic.rs new file mode 100644 index 0000000000000..2a6e57b05a22d --- /dev/null +++ b/core/sr-primitives/src/generic/unchecked_mortal_compact_tipped_extrinsic.rs @@ -0,0 +1,468 @@ +// Copyright 2017-2019 Parity Technologies (UK) Ltd. +// This file is part of Substrate. + +// Substrate is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. + +// Substrate is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License +// along with Substrate. If not, see . + +//! Generic implementation of an unchecked (pre-verification) extrinsic with tip. + +#[cfg(feature = "std")] +use std::fmt; + +use rstd::prelude::*; +use runtime_io::blake2_256; +use crate::codec::{Decode, Encode, Input, Compact}; +use crate::traits::{self, Member, SimpleArithmetic, MaybeDisplay, CurrentHeight, BlockNumberToHash, + Lookup, Checkable, Extrinsic, SaturatedConversion}; +use super::{CheckedExtrinsic, Era, Tip}; + +const TRANSACTION_VERSION: u8 = 1; + +/// A extrinsic right from the external world. This is unchecked and so +/// can contain a signature. +#[derive(PartialEq, Eq, Clone)] +pub struct UncheckedMortalCompactTippedExtrinsic { + /// The signature, address, number of extrinsics have come before from + /// the same signer and an era describing the longevity of this transaction, + /// if this is a signed extrinsic. + pub signature: Option<(Address, Signature, Compact, Era)>, + /// The function that should be called. + pub function: Call, + /// The tip for this transaction + pub tip: Tip, +} + +impl + UncheckedMortalCompactTippedExtrinsic +{ + /// New instance of a signed extrinsic aka "transaction". + pub fn new_signed( + index: Index, + function: Call, + signed: Address, + signature: Signature, + era: Era, + tip: Tip + ) -> Self { + UncheckedMortalCompactTippedExtrinsic { + signature: Some((signed, signature, index.into(), era)), + function, + tip, + } + } + + /// New instance of an unsigned extrinsic aka "inherent". + pub fn new_unsigned(function: Call) -> Self { + UncheckedMortalCompactTippedExtrinsic { + signature: None, + function, + tip: Tip::None + } + } +} + +impl Extrinsic + for UncheckedMortalCompactTippedExtrinsic +{ + fn is_signed(&self) -> Option { + Some(self.signature.is_some()) + } +} + +impl Checkable + for UncheckedMortalCompactTippedExtrinsic +where + Address: Member + MaybeDisplay, + Index: Member + MaybeDisplay + SimpleArithmetic, + Compact: Encode, + Call: Encode + Member, + Signature: Member + traits::Verify, + AccountId: Member + MaybeDisplay, + BlockNumber: SimpleArithmetic, + Hash: Encode, + Context: Lookup + + CurrentHeight + + BlockNumberToHash, + Balance: Member + Encode, +{ + type Checked = CheckedExtrinsic; + + fn check(self, context: &Context) -> Result { + Ok(match self.signature { + Some((signed, signature, index, era)) => { + let current_u64 = context.current_height().saturated_into::(); + let h = context.block_number_to_hash(era.birth(current_u64).saturated_into()) + .ok_or("transaction birth block ancient")?; + let signed = context.lookup(signed)?; + let raw_payload = (index, self.function, self.tip, era, h); + if !raw_payload.using_encoded(|payload| { + if payload.len() > 256 { + signature.verify(&blake2_256(payload)[..], &signed) + } else { + signature.verify(payload, &signed) + } + }) { + return Err(crate::BAD_SIGNATURE) + } + CheckedExtrinsic { + signed: Some((signed, (raw_payload.0).0)), + function: raw_payload.1, + tip: raw_payload.2, + } + } + None => { + // An unsigned transaction cannot have a tip. The decode code should replace it with + // `Tip::None` and ignore the input bytes. + if self.tip != Tip::None { + return Err(crate::UNSIGNED_TIP); + } + CheckedExtrinsic { + signed: None, + function: self.function, + tip: self.tip + } + }, + }) + } +} + +impl Decode + for UncheckedMortalCompactTippedExtrinsic +where + Address: Decode, + Signature: Decode, + Compact: Decode, + Call: Decode, + Balance: Decode, + +{ + fn decode(input: &mut I) -> Option { + // This is a little more complicated than usual since the binary format must be compatible + // with substrate's generic `Vec` type. Basically this just means accepting that there + // will be a prefix of vector length (we don't need + // to use this). + let _length_do_not_remove_me_see_above: Vec<()> = Decode::decode(input)?; + + let version = input.read_byte()?; + + let is_signed = version & 0b1000_0000 != 0; + let version = version & 0b0111_1111; + if version != TRANSACTION_VERSION { + return None + } + + Some(UncheckedMortalCompactTippedExtrinsic { + signature: if is_signed { Some(Decode::decode(input)?) } else { None }, + function: Decode::decode(input)?, + tip: if is_signed { Decode::decode(input)? } else { Tip::None }, + }) + } +} + +impl Encode + for UncheckedMortalCompactTippedExtrinsic +where + Address: Encode, + Signature: Encode, + Compact: Encode, + Call: Encode, + Balance: Encode, +{ + fn encode(&self) -> Vec { + super::encode_with_vec_prefix::(|v| { + // 1 byte version id. + match self.signature.as_ref() { + Some(s) => { + v.push(TRANSACTION_VERSION | 0b1000_0000); + s.encode_to(v); + } + None => { + v.push(TRANSACTION_VERSION & 0b0111_1111); + } + } + self.function.encode_to(v); + if self.signature.is_some() { + self.tip.encode_to(v); + } + }) + } +} + +#[cfg(feature = "std")] +impl serde::Serialize + for UncheckedMortalCompactTippedExtrinsic + where Compact: Encode +{ + fn serialize(&self, seq: S) -> Result where S: ::serde::Serializer { + self.using_encoded(|bytes| seq.serialize_bytes(bytes)) + } +} + +#[cfg(feature = "std")] +impl fmt::Debug + for UncheckedMortalCompactTippedExtrinsic +where + Address: fmt::Debug, + Index: fmt::Debug, + Call: fmt::Debug, + Balance: fmt::Debug, +{ + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + write!( + f, + "UncheckedMortalCompactTippedExtrinsic({:?}, {:?}, {:?})", + self.signature.as_ref().map(|x| (&x.0, &x.2)), + self.function, + self.tip, + ) + } +} + +#[cfg(test)] +mod tests { + use super::*; + use runtime_io::blake2_256; + use crate::codec::{Encode, Decode}; + use serde::{Serialize, Deserialize}; + + struct TestContext; + impl Lookup for TestContext { + type Source = u64; + type Target = u64; + fn lookup(&self, s: u64) -> Result { Ok(s) } + } + impl CurrentHeight for TestContext { + type BlockNumber = u64; + fn current_height(&self) -> u64 { 42 } + } + impl BlockNumberToHash for TestContext { + type BlockNumber = u64; + type Hash = u64; + fn block_number_to_hash(&self, n: u64) -> Option { Some(n) } + } + + #[derive(Eq, PartialEq, Clone, Debug, Serialize, Deserialize, Encode, Decode)] + struct TestSig(u64, Vec); + impl traits::Verify for TestSig { + type Signer = u64; + fn verify>(&self, mut msg: L, signer: &Self::Signer) -> bool { + *signer == self.0 && msg.get() == &self.1[..] + } + } + + type Balance = u64; + + const DUMMY_ACCOUNTID: u64 = 0; + const TIP: Tip = Tip::Sender(66); + + type Ex = UncheckedMortalCompactTippedExtrinsic, TestSig, Balance>; + type CEx = CheckedExtrinsic, Balance>; + + #[test] + fn unsigned_codec_should_work() { + let ux = Ex::new_unsigned(vec![0u8;0]); + let encoded = ux.encode(); + assert_eq!(Ex::decode(&mut &encoded[..]), Some(ux)); + } + + #[test] + fn signed_codec_should_work() { + let ux = Ex::new_signed( + 0, + vec![0u8; 0], + DUMMY_ACCOUNTID, + TestSig(DUMMY_ACCOUNTID, (DUMMY_ACCOUNTID, vec![0u8;0], TIP, Era::immortal(), 0u64).encode()), + Era::immortal(), + TIP + ); + let encoded = ux.encode(); + assert_eq!(Ex::decode(&mut &encoded[..]), Some(ux)); + } + + #[test] + fn large_signed_codec_should_work() { + let ux = Ex::new_signed( + 0, + vec![0u8; 0], + DUMMY_ACCOUNTID, + TestSig( + DUMMY_ACCOUNTID, + (DUMMY_ACCOUNTID, vec![0u8; 257], TIP, Era::immortal(), 0u64).using_encoded(blake2_256)[..].to_owned() + ), + Era::immortal(), + TIP + ); + let encoded = ux.encode(); + assert_eq!(Ex::decode(&mut &encoded[..]), Some(ux)); + } + + #[test] + fn unsigned_check_should_work() { + let ux = Ex::new_unsigned(vec![0u8;0]); + assert!(!ux.is_signed().unwrap_or(false)); + assert!(>::check(ux, &TestContext).is_ok()); + } + + #[test] + fn badly_signed_check_should_fail() { + let ux = Ex::new_signed( + 0, + vec![0u8; 0], + DUMMY_ACCOUNTID, + TestSig(DUMMY_ACCOUNTID, vec![0u8]), + Era::immortal(), + TIP, + ); + assert!(ux.is_signed().unwrap_or(false)); + assert_eq!(>::check(ux, &TestContext), Err(crate::BAD_SIGNATURE)); + } + + #[test] + fn immortal_signed_check_should_work() { + let ux = Ex::new_signed( + 0, + vec![0u8;0], + DUMMY_ACCOUNTID, + TestSig( + DUMMY_ACCOUNTID, + (Compact::from(DUMMY_ACCOUNTID), vec![0u8;0], TIP, Era::immortal(), 0u64).encode() + ), + Era::immortal(), + TIP, + ); + assert!(ux.is_signed().unwrap_or(false)); + assert_eq!( + >::check(ux, &TestContext), + Ok(CEx { signed: Some((DUMMY_ACCOUNTID, 0)), function: vec![0u8;0], tip: TIP }) + ); + } + + #[test] + fn mortal_signed_check_should_work() { + let ux = Ex::new_signed( + 0, + vec![0u8;0], + DUMMY_ACCOUNTID, + TestSig( + DUMMY_ACCOUNTID, + (Compact::from(DUMMY_ACCOUNTID), vec![0u8;0], TIP, Era::mortal(32, 42), 42u64).encode() + ), + Era::mortal(32, 42), + TIP, + ); + assert!(ux.is_signed().unwrap_or(false)); + assert_eq!( + >::check(ux, &TestContext), + Ok(CEx { signed: Some((DUMMY_ACCOUNTID, 0)), function: vec![0u8;0], tip: TIP })); + } + + #[test] + fn later_mortal_signed_check_should_work() { + let ux = Ex::new_signed( + 0, + vec![0u8;0], + DUMMY_ACCOUNTID, + TestSig( + DUMMY_ACCOUNTID, + (Compact::from(DUMMY_ACCOUNTID), vec![0u8;0], TIP, Era::mortal(32, 11), 11u64).encode() + ), + Era::mortal(32, 11), + TIP, + ); + assert!(ux.is_signed().unwrap_or(false)); + assert_eq!( + >::check(ux, &TestContext), + Ok(CEx { signed: Some((DUMMY_ACCOUNTID, 0)), function: vec![0u8;0], tip: TIP }) + ); + } + + #[test] + fn too_late_mortal_signed_check_should_fail() { + let ux = Ex::new_signed( + 0, + vec![0u8;0], + DUMMY_ACCOUNTID, + TestSig( + DUMMY_ACCOUNTID, + (DUMMY_ACCOUNTID, vec![0u8;0], TIP, Era::mortal(32, 10), 10u64).encode() + ), + Era::mortal(32, 10), + TIP, + ); + assert!(ux.is_signed().unwrap_or(false)); + assert_eq!(>::check(ux, &TestContext), Err(crate::BAD_SIGNATURE)); + } + + #[test] + fn too_early_mortal_signed_check_should_fail() { + let ux = Ex::new_signed( + 0, + vec![0u8;0], + DUMMY_ACCOUNTID, + TestSig( + DUMMY_ACCOUNTID, + (DUMMY_ACCOUNTID, vec![0u8;0], TIP, Era::mortal(32, 43), 43u64).encode() + ), + Era::mortal(32, 43), + TIP, + ); + assert!(ux.is_signed().unwrap_or(false)); + assert_eq!(>::check(ux, &TestContext), Err(crate::BAD_SIGNATURE)); + } + + #[test] + fn encoding_matches_vec() { + let ex = Ex::new_unsigned(vec![0u8;0]); + let encoded = ex.encode(); + let decoded = Ex::decode(&mut encoded.as_slice()).unwrap(); + assert_eq!(decoded, ex); + let as_vec: Vec = Decode::decode(&mut encoded.as_slice()).unwrap(); + assert_eq!(as_vec.encode(), encoded); + } + + #[test] + fn missing_tip_in_signature_payload_should_fail() { + let ux = Ex::new_signed( + 0, + vec![0u8;0], + DUMMY_ACCOUNTID, + TestSig( + DUMMY_ACCOUNTID, + (DUMMY_ACCOUNTID, vec![0u8;0], Era::immortal(), 0u64).encode() + ), + Era::immortal(), + TIP, + ); + assert!(ux.is_signed().unwrap_or(false)); + assert_eq!(>::check(ux, &TestContext), Err(crate::BAD_SIGNATURE)); + } + + #[test] + #[should_panic] + fn unsigned_cannot_have_tip() { + let ux = UncheckedMortalCompactTippedExtrinsic {signature: None, tip: Tip::Sender(100), function: vec![0u8;0]}; + let _ = >::check(ux, &TestContext).unwrap(); + } + + #[test] + fn unprovided_tip_will_not_fail() { + // unsigned with no tips + let bytes = vec![40, 1, 32, 8, 8, 8, 8, 8, 8, 8, 8]; + + // decoding will yield the default tip + let decoded = Ex::decode(&mut bytes.as_slice()).unwrap(); + assert_eq!( + decoded, + UncheckedMortalCompactTippedExtrinsic { signature: None, tip: Tip::None, function: vec![8u8;8]}) + } +} diff --git a/core/sr-primitives/src/generic/unchecked_mortal_extrinsic.rs b/core/sr-primitives/src/generic/unchecked_mortal_extrinsic.rs index dd5ecd2281e4d..4868aa17dd7e1 100644 --- a/core/sr-primitives/src/generic/unchecked_mortal_extrinsic.rs +++ b/core/sr-primitives/src/generic/unchecked_mortal_extrinsic.rs @@ -33,37 +33,21 @@ const TRANSACTION_VERSION: u8 = 1; /// A extrinsic right from the external world. This is unchecked and so /// can contain a signature. #[derive(PartialEq, Eq, Clone)] -pub struct UncheckedMortalExtrinsic { +pub struct UncheckedMortalExtrinsic { /// The signature, address, number of extrinsics have come before from /// the same signer and an era describing the longevity of this transaction, /// if this is a signed extrinsic. pub signature: Option<(Address, Signature, Index, Era)>, /// The function that should be called. pub function: Call, - /// The tip for this transaction - pub tip: Tip, } -impl< - Address, - Index, - Call, - Signature, - Balance -> UncheckedMortalExtrinsic { +impl UncheckedMortalExtrinsic { /// New instance of a signed extrinsic aka "transaction". - pub fn new_signed( - index: Index, - function: Call, - signed: Address, - signature: Signature, - era: Era, - tip: Tip - ) -> Self { + pub fn new_signed(index: Index, function: Call, signed: Address, signature: Signature, era: Era) -> Self { UncheckedMortalExtrinsic { signature: Some((signed, signature, index, era)), function, - tip, } } @@ -72,30 +56,20 @@ impl< UncheckedMortalExtrinsic { signature: None, function, - tip: Tip::None, } } } -impl Extrinsic - for UncheckedMortalExtrinsic +impl Extrinsic + for UncheckedMortalExtrinsic { fn is_signed(&self) -> Option { Some(self.signature.is_some()) } } -impl< - Address, - AccountId, - Index, - Call, - Signature, - Context, - Hash, - BlockNumber, - Balance -> Checkable for UncheckedMortalExtrinsic +impl Checkable + for UncheckedMortalExtrinsic where Address: Member + MaybeDisplay, Index: Encode + Member + MaybeDisplay + SimpleArithmetic, @@ -107,9 +81,10 @@ where Context: Lookup + CurrentHeight + BlockNumberToHash, - Balance: Member + Encode, { - type Checked = CheckedExtrinsic; + /// NOTE: this transaction is not tipped i.e. the tip value will be `None`. It does not really + /// matter what the last generic is since it is always `None`. + type Checked = CheckedExtrinsic; fn check(self, context: &Context) -> Result { Ok(match self.signature { @@ -118,7 +93,8 @@ where let h = context.block_number_to_hash(era.birth(current_u64).saturated_into()) .ok_or("transaction birth block ancient")?; let signed = context.lookup(signed)?; - let raw_payload = (index, self.function, self.tip, era, h); + let raw_payload = (index, self.function, era, h); + if !raw_payload.using_encoded(|payload| { if payload.len() > 256 { signature.verify(&blake2_256(payload)[..], &signed) @@ -131,31 +107,25 @@ where CheckedExtrinsic { signed: Some((signed, raw_payload.0)), function: raw_payload.1, - tip: raw_payload.2, + tip: Tip::None, } } - None => { - // An unsigned transaction cannot have a tip. The decode code should replace it with - // None always and ignore the input bytes. - debug_assert!(self.tip == Tip::None, "{}", crate::UNSIGNED_TIP); - CheckedExtrinsic { - signed: None, - function: self.function, - tip: self.tip - } + None => CheckedExtrinsic { + signed: None, + function: self.function, + tip: Tip::None, }, }) } } -impl Decode - for UncheckedMortalExtrinsic +impl Decode + for UncheckedMortalExtrinsic where Address: Decode, Signature: Decode, Index: Decode, Call: Decode, - Balance: Decode, { fn decode(input: &mut I) -> Option { // This is a little more complicated than usual since the binary format must be compatible @@ -175,19 +145,17 @@ where Some(UncheckedMortalExtrinsic { signature: if is_signed { Some(Decode::decode(input)?) } else { None }, function: Decode::decode(input)?, - tip: if is_signed { Decode::decode(input).unwrap_or_default() } else { Tip::None }, }) } } -impl Encode - for UncheckedMortalExtrinsic +impl Encode + for UncheckedMortalExtrinsic where Address: Encode, Signature: Encode, Index: Encode, Call: Encode, - Balance: Encode, { fn encode(&self) -> Vec { super::encode_with_vec_prefix::(|v| { @@ -202,16 +170,13 @@ where } } self.function.encode_to(v); - if self.signature.is_some() { - self.tip.encode_to(v); - } }) } } #[cfg(feature = "std")] -impl serde::Serialize - for UncheckedMortalExtrinsic +impl serde::Serialize + for UncheckedMortalExtrinsic { fn serialize(&self, seq: S) -> Result where S: ::serde::Serializer { self.using_encoded(|bytes| seq.serialize_bytes(bytes)) @@ -219,21 +184,19 @@ impl fmt::Debug - for UncheckedMortalExtrinsic +impl fmt::Debug + for UncheckedMortalExtrinsic where Address: fmt::Debug, Index: fmt::Debug, Call: fmt::Debug, - Balance: fmt::Debug { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { write!( f, - "UncheckedMortalExtrinsic({:?}, {:?} ,{:?})", + "UncheckedMortalExtrinsic({:?}, {:?})", self.signature.as_ref().map(|x| (&x.0, &x.2)), - self.function, - self.tip, + self.function ) } } @@ -270,13 +233,10 @@ mod tests { } } - type Balance = u64; - const DUMMY_ACCOUNTID: u64 = 0; - const TIP: Tip = Tip::Sender(66); - type Ex = UncheckedMortalExtrinsic, TestSig, Balance>; - type CEx = CheckedExtrinsic, Balance>; + type Ex = UncheckedMortalExtrinsic, TestSig>; + type CEx = CheckedExtrinsic, u64>; #[test] fn unsigned_codec_should_work() { @@ -287,32 +247,14 @@ mod tests { #[test] fn signed_codec_should_work() { - let ux = Ex::new_signed( - 0, - vec![0u8;0], - DUMMY_ACCOUNTID, - TestSig(DUMMY_ACCOUNTID, (DUMMY_ACCOUNTID, vec![0u8;0], TIP, Era::immortal(), 0u64).encode()), - Era::immortal(), - TIP, - ); + let ux = Ex::new_signed(0, vec![0u8;0], DUMMY_ACCOUNTID, TestSig(DUMMY_ACCOUNTID, (DUMMY_ACCOUNTID, vec![0u8;0], Era::immortal(), 0u64).encode()), Era::immortal()); let encoded = ux.encode(); assert_eq!(Ex::decode(&mut &encoded[..]), Some(ux)); } #[test] fn large_signed_codec_should_work() { - let ux = Ex::new_signed( - 0, - vec![0u8;0], - DUMMY_ACCOUNTID, - TestSig( - DUMMY_ACCOUNTID, - (DUMMY_ACCOUNTID, vec![0u8; 257], TIP, Era::immortal(), 0u64) - .using_encoded(blake2_256)[..].to_owned() - ), - Era::immortal(), - TIP, - ); + let ux = Ex::new_signed(0, vec![0u8;0], DUMMY_ACCOUNTID, TestSig(DUMMY_ACCOUNTID, (DUMMY_ACCOUNTID, vec![0u8; 257], Era::immortal(), 0u64).using_encoded(blake2_256)[..].to_owned()), Era::immortal()); let encoded = ux.encode(); assert_eq!(Ex::decode(&mut &encoded[..]), Some(ux)); } @@ -326,93 +268,42 @@ mod tests { #[test] fn badly_signed_check_should_fail() { - let ux = Ex::new_signed( - 0, - vec![0u8;0], - DUMMY_ACCOUNTID, - TestSig(DUMMY_ACCOUNTID, vec![0u8]), - Era::immortal(), - TIP, - ); + let ux = Ex::new_signed(0, vec![0u8;0], DUMMY_ACCOUNTID, TestSig(DUMMY_ACCOUNTID, vec![0u8]), Era::immortal()); assert!(ux.is_signed().unwrap_or(false)); assert_eq!(>::check(ux, &TestContext), Err(crate::BAD_SIGNATURE)); } #[test] fn immortal_signed_check_should_work() { - let ux = Ex::new_signed( - 0, - vec![0u8;0], - DUMMY_ACCOUNTID, - TestSig(DUMMY_ACCOUNTID, (DUMMY_ACCOUNTID, vec![0u8;0], TIP, Era::immortal(), 0u64).encode()), - Era::immortal(), - TIP, - ); + let ux = Ex::new_signed(0, vec![0u8;0], DUMMY_ACCOUNTID, TestSig(DUMMY_ACCOUNTID, (DUMMY_ACCOUNTID, vec![0u8;0], Era::immortal(), 0u64).encode()), Era::immortal()); assert!(ux.is_signed().unwrap_or(false)); - assert_eq!( - >::check(ux, &TestContext), - Ok(CEx { signed: Some((DUMMY_ACCOUNTID, 0)), function: vec![0u8;0], tip: TIP }) - ); + assert_eq!(>::check(ux, &TestContext), Ok(CEx { signed: Some((DUMMY_ACCOUNTID, 0)), function: vec![0u8;0], tip: Tip::None })); } #[test] fn mortal_signed_check_should_work() { - let ux = Ex::new_signed( - 0, - vec![0u8;0], - DUMMY_ACCOUNTID, - TestSig(DUMMY_ACCOUNTID, (DUMMY_ACCOUNTID, vec![0u8;0], TIP, Era::mortal(32, 42), 42u64).encode()), - Era::mortal(32, 42), - TIP, - ); + let ux = Ex::new_signed(0, vec![0u8;0], DUMMY_ACCOUNTID, TestSig(DUMMY_ACCOUNTID, (DUMMY_ACCOUNTID, vec![0u8;0], Era::mortal(32, 42), 42u64).encode()), Era::mortal(32, 42)); assert!(ux.is_signed().unwrap_or(false)); - assert_eq!( - >::check(ux, &TestContext), - Ok(CEx { signed: Some((DUMMY_ACCOUNTID, 0)), function: vec![0u8;0], tip: TIP}) - ); + assert_eq!(>::check(ux, &TestContext), Ok(CEx { signed: Some((DUMMY_ACCOUNTID, 0)), function: vec![0u8;0], tip: Tip::None })); } #[test] fn later_mortal_signed_check_should_work() { - let ux = Ex::new_signed( - 0, - vec![0u8;0], - DUMMY_ACCOUNTID, - TestSig(DUMMY_ACCOUNTID, (DUMMY_ACCOUNTID, vec![0u8;0], TIP, Era::mortal(32, 11), 11u64).encode()), - Era::mortal(32, 11), - TIP, - ); + let ux = Ex::new_signed(0, vec![0u8;0], DUMMY_ACCOUNTID, TestSig(DUMMY_ACCOUNTID, (DUMMY_ACCOUNTID, vec![0u8;0], Era::mortal(32, 11), 11u64).encode()), Era::mortal(32, 11)); assert!(ux.is_signed().unwrap_or(false)); - assert_eq!( - >::check(ux, &TestContext), - Ok(CEx { signed: Some((DUMMY_ACCOUNTID, 0)), function: vec![0u8;0], tip: TIP}) - ); + assert_eq!(>::check(ux, &TestContext), Ok(CEx { signed: Some((DUMMY_ACCOUNTID, 0)), function: vec![0u8;0], tip: Tip::None })); } #[test] fn too_late_mortal_signed_check_should_fail() { - let ux = Ex::new_signed( - 0, - vec![0u8;0], - DUMMY_ACCOUNTID, - TestSig(DUMMY_ACCOUNTID, (DUMMY_ACCOUNTID, vec![0u8;0], TIP, Era::mortal(32, 10), 10u64).encode()), - Era::mortal(32, 10), - TIP, - ); + let ux = Ex::new_signed(0, vec![0u8;0], DUMMY_ACCOUNTID, TestSig(DUMMY_ACCOUNTID, (DUMMY_ACCOUNTID, vec![0u8;0], Era::mortal(32, 10), 10u64).encode()), Era::mortal(32, 10)); assert!(ux.is_signed().unwrap_or(false)); assert_eq!(>::check(ux, &TestContext), Err(crate::BAD_SIGNATURE)); } #[test] fn too_early_mortal_signed_check_should_fail() { - let ux = Ex::new_signed( - 0, - vec![0u8;0], - DUMMY_ACCOUNTID, - TestSig(DUMMY_ACCOUNTID, (DUMMY_ACCOUNTID, vec![0u8;0], TIP, Era::mortal(32, 43), 43u64).encode()), - Era::mortal(32, 43), - TIP, - ); + let ux = Ex::new_signed(0, vec![0u8;0], DUMMY_ACCOUNTID, TestSig(DUMMY_ACCOUNTID, (DUMMY_ACCOUNTID, vec![0u8;0], Era::mortal(32, 43), 43u64).encode()), Era::mortal(32, 43)); assert!(ux.is_signed().unwrap_or(false)); assert_eq!(>::check(ux, &TestContext), Err(crate::BAD_SIGNATURE)); } @@ -426,41 +317,4 @@ mod tests { let as_vec: Vec = Decode::decode(&mut encoded.as_slice()).unwrap(); assert_eq!(as_vec.encode(), encoded); } - - #[test] - fn missing_tip_in_signature_payload_should_fail() { - let ux = Ex::new_signed( - 0, - vec![0u8;0], - DUMMY_ACCOUNTID, - TestSig( - DUMMY_ACCOUNTID, - (DUMMY_ACCOUNTID, vec![0u8;0], Era::immortal(), 0u64).encode() - ), - Era::immortal(), - TIP, - ); - assert!(ux.is_signed().unwrap_or(false)); - assert_eq!(>::check(ux, &TestContext), Err(crate::BAD_SIGNATURE)); - } - - #[test] - #[should_panic] - fn unsigned_cannot_have_tip() { - let ux = UncheckedMortalExtrinsic { signature: None, tip: Tip::Sender(100), function: vec![0u8;0]}; - let _ = >::check(ux, &TestContext).unwrap(); - } - - #[test] - fn unprovided_tip_will_not_fail() { - // unsigned with no tips - let bytes = vec![40, 1, 32, 8, 8, 8, 8, 8, 8, 8, 8]; - - // decoding will yield the default tip - let decoded = Ex::decode(&mut bytes.as_slice()).unwrap(); - assert_eq!( - decoded, - UncheckedMortalExtrinsic { signature: None, tip: Tip::None, function: vec![8u8;8]} - ); - } -} +} \ No newline at end of file diff --git a/node-template/runtime/src/lib.rs b/node-template/runtime/src/lib.rs index ebbbb16bccdab..1ea7c10790845 100644 --- a/node-template/runtime/src/lib.rs +++ b/node-template/runtime/src/lib.rs @@ -208,7 +208,7 @@ pub type Block = generic::Block; /// BlockId type as expected by this runtime. pub type BlockId = generic::BlockId; /// Unchecked extrinsic type as expected by this runtime. -pub type UncheckedExtrinsic = generic::UncheckedMortalCompactExtrinsic; +pub type UncheckedExtrinsic = generic::UncheckedMortalCompactExtrinsic; /// Extrinsic type that has already been checked. pub type CheckedExtrinsic = generic::CheckedExtrinsic; /// Executive: handles dispatch to the various modules. diff --git a/node/cli/src/factory_impl.rs b/node/cli/src/factory_impl.rs index 2d60e9fc970d0..1c17366e5524c 100644 --- a/node/cli/src/factory_impl.rs +++ b/node/cli/src/factory_impl.rs @@ -247,13 +247,11 @@ fn sign( UncheckedExtrinsic { signature: Some((indices::address::Address::Id(signed), signature, payload.0, era)), function: payload.1, - tip: Tip::default(), } } None => UncheckedExtrinsic { signature: None, function: xt.function, - tip: Tip::default(), }, }; diff --git a/node/cli/src/service.rs b/node/cli/src/service.rs index cb57c70a69253..42c4bdeb380cb 100644 --- a/node/cli/src/service.rs +++ b/node/cli/src/service.rs @@ -269,7 +269,6 @@ mod tests { let payload = ( 0, Call::Balances(BalancesCall::transfer(RawAddress::Id(bob.public().0.into()), 69.into())), - Tip::default(), Era::immortal(), service.client().genesis_hash() ); @@ -278,7 +277,6 @@ mod tests { let xt = UncheckedExtrinsic { signature: Some((RawAddress::Id(id), signature, payload.0, Era::immortal())), function: payload.1, - tip: Tip::default(), }.encode(); let v: Vec = Decode::decode(&mut xt.as_slice()).unwrap(); OpaqueExtrinsic(v) @@ -372,7 +370,6 @@ mod tests { from.into(), signature.into(), era, - Tip::default(), ).encode(); let v: Vec = Decode::decode(&mut xt.as_slice()).unwrap(); diff --git a/node/executor/src/lib.rs b/node/executor/src/lib.rs index 8a3c708da6337..bec0553ab26a8 100644 --- a/node/executor/src/lib.rs +++ b/node/executor/src/lib.rs @@ -107,7 +107,7 @@ mod tests { match xt.signed { Some((signed, index)) => { let era = Era::mortal(256, 0); - let payload = (index.into(), xt.function, xt.tip, era, GENESIS_HASH); + let payload = (index.into(), xt.function, era, GENESIS_HASH); let key = AccountKeyring::from_public(&signed).unwrap(); let signature = payload.using_encoded(|b| { if b.len() > 256 { @@ -119,13 +119,11 @@ mod tests { UncheckedExtrinsic { signature: Some((indices::address::Address::Id(signed), signature, payload.0, era)), function: payload.1, - tip: payload.2, } } None => UncheckedExtrinsic { signature: None, function: xt.function, - tip: Tip::None, }, } } diff --git a/node/runtime/src/lib.rs b/node/runtime/src/lib.rs index e6598d8112b64..3421821242f90 100644 --- a/node/runtime/src/lib.rs +++ b/node/runtime/src/lib.rs @@ -276,7 +276,7 @@ pub type SignedBlock = generic::SignedBlock; /// BlockId type as expected by this runtime. pub type BlockId = generic::BlockId; /// Unchecked extrinsic type as expected by this runtime. -pub type UncheckedExtrinsic = generic::UncheckedMortalCompactExtrinsic; +pub type UncheckedExtrinsic = generic::UncheckedMortalCompactExtrinsic; /// Extrinsic type that has already been checked. pub type CheckedExtrinsic = generic::CheckedExtrinsic; /// Executive: handles dispatch to the various modules. diff --git a/srml/support/test/tests/instance.rs b/srml/support/test/tests/instance.rs index 909699749e6c3..f8497186e8675 100644 --- a/srml/support/test/tests/instance.rs +++ b/srml/support/test/tests/instance.rs @@ -240,7 +240,7 @@ srml_support::construct_runtime!( pub type Header = generic::Header; pub type Block = generic::Block; -pub type UncheckedExtrinsic = generic::UncheckedMortalCompactExtrinsic; +pub type UncheckedExtrinsic = generic::UncheckedMortalCompactExtrinsic; fn new_test_ext() -> runtime_io::TestExternalities { GenesisConfig{ diff --git a/srml/support/test/tests/issue2219.rs b/srml/support/test/tests/issue2219.rs index 590d31590b615..185b5e24807a9 100644 --- a/srml/support/test/tests/issue2219.rs +++ b/srml/support/test/tests/issue2219.rs @@ -152,7 +152,7 @@ pub type BlockNumber = u64; pub type Index = u64; pub type Header = generic::Header; pub type Block = generic::Block; -pub type UncheckedExtrinsic = generic::UncheckedMortalCompactExtrinsic; +pub type UncheckedExtrinsic = generic::UncheckedMortalCompactExtrinsic; impl system::Trait for Runtime { type Hash = H256; diff --git a/subkey/src/main.rs b/subkey/src/main.rs index 8f42ff68bfc51..b85e36eca41cc 100644 --- a/subkey/src/main.rs +++ b/subkey/src/main.rs @@ -166,7 +166,6 @@ fn execute(matches: clap::ArgMatches) where signer.public().into(), signature.into(), era, - Tip::default(), ); println!("0x{}", hex::encode(&extrinsic.encode())); } @@ -208,7 +207,6 @@ fn execute(matches: clap::ArgMatches) where signer.public().into(), signature.into(), era, - Tip::default(), ); println!("0x{}", hex::encode(&extrinsic.encode())); From 0d1de9dfc7ff49775d093e5dcbf91b9323e56203 Mon Sep 17 00:00:00 2001 From: kianenigma Date: Wed, 10 Jul 2019 09:57:24 +0200 Subject: [PATCH 17/23] Make balance associated type of payment. --- .../src/generic/checked_extrinsic.rs | 19 ++++++++++--------- .../src/generic/unchecked_extrinsic.rs | 2 +- .../unchecked_mortal_compact_extrinsic.rs | 4 ++-- .../src/generic/unchecked_mortal_extrinsic.rs | 4 ++-- core/sr-primitives/src/lib.rs | 2 +- node/cli/src/service.rs | 2 +- node/executor/src/lib.rs | 2 -- srml/balances/src/lib.rs | 6 ++++-- srml/balances/src/tests.rs | 6 +++--- srml/executive/src/lib.rs | 15 ++++++++++----- srml/support/src/traits.rs | 17 +++++++++++------ 11 files changed, 45 insertions(+), 34 deletions(-) diff --git a/core/sr-primitives/src/generic/checked_extrinsic.rs b/core/sr-primitives/src/generic/checked_extrinsic.rs index ee90a1c75f0b4..5a672aa87a883 100644 --- a/core/sr-primitives/src/generic/checked_extrinsic.rs +++ b/core/sr-primitives/src/generic/checked_extrinsic.rs @@ -17,7 +17,7 @@ //! Generic implementation of an extrinsic that has passed the verification //! stage. -use crate::traits::{self, Member, SimpleArithmetic, MaybeDisplay, SaturatedConversion}; +use crate::traits::{self, Member, SimpleArithmetic, MaybeDisplay}; use crate::weights::{Weighable, Weight}; use crate::generic::tip::{Tip, Tippable}; @@ -86,15 +86,16 @@ where // This is a hacky way to prevent `unchecked_mortal[_compact]_extrinsic` types, which // don't have a tip, become generic over a balance type. // Basically, this CheckedExtrinsic is built either 1- from an - // `UncheckedMortalCompactTippedExtrinsic`, which is tip aware and hence, the second arm - // will be trivially executed and the type conversion will be safe (the compiler is probably) - // smart enough to remove it in fact. Or 2- this is built from all other types of unchecked - // extrinsic which do not have tip and hence are not balance-aware. These module will naively - // place a u64 (can be `()` in practice) as the type and it does not matter since `Tip::None` - // is used in this case. + // `UncheckedMortalCompactTippedExtrinsic`, which is tip-aware and hence, the second arm + // will be trivially executed and the type conversion will be safe (the compiler is probably + // smart enough to remove it in fact). In this case, NodeBalance and ExtBalance are the same. + // Or 2- this is built from all other types of unchecked + // extrinsic which do not have tip and hence are not tip-aware. These modules will naively + // place a u32 (can be `()` in practice) as the type and it does not matter since `Tip::None` + // is used in this case (first arm). match self.tip { - Tip::None => >::None, - >::Sender(v) => >::Sender(NodeBalance::from(v)) + Tip::None => Tip::None, + Tip::Sender(v) => Tip::Sender(NodeBalance::from(v)) } } } \ No newline at end of file diff --git a/core/sr-primitives/src/generic/unchecked_extrinsic.rs b/core/sr-primitives/src/generic/unchecked_extrinsic.rs index 897a89e492c15..c49fb5afaf69d 100644 --- a/core/sr-primitives/src/generic/unchecked_extrinsic.rs +++ b/core/sr-primitives/src/generic/unchecked_extrinsic.rs @@ -84,7 +84,7 @@ where AccountId: Member + MaybeDisplay, Context: Lookup, { - type Checked = CheckedExtrinsic; + type Checked = CheckedExtrinsic; fn check(self, context: &Context) -> Result { Ok(match self.signature { diff --git a/core/sr-primitives/src/generic/unchecked_mortal_compact_extrinsic.rs b/core/sr-primitives/src/generic/unchecked_mortal_compact_extrinsic.rs index cd768a61fb87c..df9011a20efac 100644 --- a/core/sr-primitives/src/generic/unchecked_mortal_compact_extrinsic.rs +++ b/core/sr-primitives/src/generic/unchecked_mortal_compact_extrinsic.rs @@ -86,7 +86,7 @@ where /// NOTE: this transaction is not tipped i.e. the tip value will be `None`. It does not really /// matter what the last generic is since it is always `None`. // TODO: try and use () here since it is a bit nicer. - type Checked = CheckedExtrinsic; + type Checked = CheckedExtrinsic; fn check(self, context: &Context) -> Result { Ok(match self.signature { @@ -238,7 +238,7 @@ mod tests { const DUMMY_ACCOUNTID: u64 = 0; type Ex = UncheckedMortalCompactExtrinsic, TestSig>; - type CEx = CheckedExtrinsic, u64>; + type CEx = CheckedExtrinsic, u32>; #[test] fn unsigned_codec_should_work() { diff --git a/core/sr-primitives/src/generic/unchecked_mortal_extrinsic.rs b/core/sr-primitives/src/generic/unchecked_mortal_extrinsic.rs index 4868aa17dd7e1..a237a3aed3c6c 100644 --- a/core/sr-primitives/src/generic/unchecked_mortal_extrinsic.rs +++ b/core/sr-primitives/src/generic/unchecked_mortal_extrinsic.rs @@ -84,7 +84,7 @@ where { /// NOTE: this transaction is not tipped i.e. the tip value will be `None`. It does not really /// matter what the last generic is since it is always `None`. - type Checked = CheckedExtrinsic; + type Checked = CheckedExtrinsic; fn check(self, context: &Context) -> Result { Ok(match self.signature { @@ -236,7 +236,7 @@ mod tests { const DUMMY_ACCOUNTID: u64 = 0; type Ex = UncheckedMortalExtrinsic, TestSig>; - type CEx = CheckedExtrinsic, u64>; + type CEx = CheckedExtrinsic, u32>; #[test] fn unsigned_codec_should_work() { diff --git a/core/sr-primitives/src/lib.rs b/core/sr-primitives/src/lib.rs index cef7b50f7b1aa..fddfc4add6208 100644 --- a/core/sr-primitives/src/lib.rs +++ b/core/sr-primitives/src/lib.rs @@ -65,7 +65,7 @@ pub const BAD_SIGNATURE: &str = "bad signature in extrinsic"; pub const BLOCK_FULL: &str = "block size limit is reached"; /// Unsigned message containing tip error message. -pub const UNSIGNED_TIP: &str = "unsigned message with tip not allowed"; +pub const UNSIGNED_TIP: &str = "unsigned extrinsic with tip not allowed"; /// Justification type. pub type Justification = Vec; diff --git a/node/cli/src/service.rs b/node/cli/src/service.rs index 42c4bdeb380cb..7b86ea21f432a 100644 --- a/node/cli/src/service.rs +++ b/node/cli/src/service.rs @@ -358,7 +358,7 @@ mod tests { let function = Call::Balances(BalancesCall::transfer(to.into(), amount)); let era = Era::immortal(); - let raw_payload = (Compact(index), function, Tip::::default(), era, genesis_hash); + let raw_payload = (Compact(index), function, era, genesis_hash); let signature = raw_payload.using_encoded(|payload| if payload.len() > 256 { signer.sign(&blake2_256(payload)[..]) } else { diff --git a/node/executor/src/lib.rs b/node/executor/src/lib.rs index bec0553ab26a8..2327699115f93 100644 --- a/node/executor/src/lib.rs +++ b/node/executor/src/lib.rs @@ -886,7 +886,6 @@ mod tests { fn native_big_block_import_succeeds() { let mut t = new_test_ext(COMPACT_CODE, false); let block = big_block().0; - println!("Shit will get real here"); let r = Executor::new(None).call::<_, NeverNativeValue, fn() -> _>( &mut t, "Core_execute_block", @@ -894,7 +893,6 @@ mod tests { true, None, ); - println!("R = {:?}", r); r.0.unwrap(); } diff --git a/srml/balances/src/lib.rs b/srml/balances/src/lib.rs index 9a9b922be33cf..b5dac597b0b04 100644 --- a/srml/balances/src/lib.rs +++ b/srml/balances/src/lib.rs @@ -1054,7 +1054,9 @@ where } } -impl, I: Instance> MakePayment for Module { +impl, I: Instance> MakePayment for Module { + type Balance = T::Balance; + fn make_payment(transactor: &T::AccountId, encoded_len: usize) -> Result { let encoded_len = T::Balance::from(encoded_len as u32); let transaction_fee = Self::transaction_base_fee() + Self::transaction_byte_fee() * encoded_len; @@ -1062,7 +1064,7 @@ impl, I: Instance> MakePayment for Module< Ok(()) } - fn make_raw_payment(transactor: &T::AccountId, value: T::Balance) -> Result { + fn make_raw_payment(transactor: &T::AccountId, value: Self::Balance) -> Result { let imbalance = Self::withdraw( transactor, value, diff --git a/srml/balances/src/tests.rs b/srml/balances/src/tests.rs index deea937725466..0a5a4b5bb70a6 100644 --- a/srml/balances/src/tests.rs +++ b/srml/balances/src/tests.rs @@ -118,7 +118,7 @@ fn lock_reasons_should_work() { "account liquidity restrictions prevent withdrawal" ); assert_ok!(>::reserve(&1, 1)); - assert_ok!(>::make_payment(&1, 1)); + assert_ok!(>::make_payment(&1, 1)); Balances::set_lock(ID_1, &1, 10, u64::max_value(), WithdrawReason::Reserve.into()); assert_ok!(>::transfer(&1, &2, 1)); @@ -126,13 +126,13 @@ fn lock_reasons_should_work() { >::reserve(&1, 1), "account liquidity restrictions prevent withdrawal" ); - assert_ok!(>::make_payment(&1, 1)); + assert_ok!(>::make_payment(&1, 1)); Balances::set_lock(ID_1, &1, 10, u64::max_value(), WithdrawReason::TransactionPayment.into()); assert_ok!(>::transfer(&1, &2, 1)); assert_ok!(>::reserve(&1, 1)); assert_noop!( - >::make_payment(&1, 1), + >::make_payment(&1, 1), "account liquidity restrictions prevent withdrawal" ); }); diff --git a/srml/executive/src/lib.rs b/srml/executive/src/lib.rs index b57472ef143f4..cc52c998a5166 100644 --- a/srml/executive/src/lib.rs +++ b/srml/executive/src/lib.rs @@ -114,6 +114,7 @@ pub trait ExecuteBlock { pub type CheckedOf = >::Checked; pub type CallOf = as Applyable>::Call; pub type OriginOf = as Dispatchable>::Origin; +pub type BalanceOf =

>::Balance; pub struct Executive( PhantomData<(System, Block, Context, Payment, Balance, UnsignedValidator, AllModules)> @@ -123,15 +124,17 @@ impl< System: system::Trait, Block: traits::Block, Context: Default, - Payment: MakePayment, + Payment: MakePayment, Balance, UnsignedValidator, AllModules: OnInitialize + OnFinalize + OffchainWorker, > ExecuteBlock for Executive where - Block::Extrinsic: Checkable + Tippable + Codec, + Block::Extrinsic: Checkable + Codec, CheckedOf: - Applyable + Weighable + Tippable, + Applyable + + Weighable + + Tippable>, CallOf: Dispatchable, OriginOf: From>, UnsignedValidator: ValidateUnsigned>, @@ -145,7 +148,7 @@ impl< System: system::Trait, Block: traits::Block, Context: Default, - Payment: MakePayment, + Payment: MakePayment, Balance: SimpleArithmetic + Copy, UnsignedValidator, AllModules: OnInitialize + OnFinalize + OffchainWorker, @@ -153,7 +156,9 @@ impl< where Block::Extrinsic: Checkable + Codec, CheckedOf: - Applyable + Weighable + Tippable, + Applyable + + Weighable + + Tippable>, CallOf: Dispatchable, OriginOf: From>, UnsignedValidator: ValidateUnsigned>, diff --git a/srml/support/src/traits.rs b/srml/support/src/traits.rs index f9b84302d0870..8372967b2fde4 100644 --- a/srml/support/src/traits.rs +++ b/srml/support/src/traits.rs @@ -21,7 +21,7 @@ use crate::rstd::result; use crate::codec::{Codec, Encode, Decode}; use crate::runtime_primitives::traits::{ - MaybeSerializeDebug, SimpleArithmetic + MaybeSerializeDebug, SimpleArithmetic, }; use crate::runtime_primitives::ConsensusEngineId; @@ -91,20 +91,25 @@ pub enum UpdateBalanceOutcome { } /// Simple trait designed for hooking into a transaction payment. -pub trait MakePayment { +pub trait MakePayment { + /// Balance type used for payment. + type Balance: SimpleArithmetic + Copy; + /// Make transaction payment from `who` for an extrinsic of encoded length /// `encoded_len` bytes. Return `Ok` iff the payment was successful. fn make_payment(who: &AccountId, encoded_len: usize) -> Result<(), &'static str>; - /// Make a raw transactiona payment from `who` with the value `value`. + /// Make a raw transaction payment from `who` with the value `value`. /// This is most often used to deduct optional fees from a transactor. /// Return `Ok` iff the payment was successful. - fn make_raw_payment(who: &AccountId, value: Balance) -> Result<(), &'static str>; + fn make_raw_payment(who: &AccountId, value: Self::Balance) -> Result<(), &'static str>; } -impl MakePayment for () { +impl MakePayment for () { + // fine since we don't care about it. + type Balance = u32; fn make_payment(_: &T, _: usize) -> Result<(), &'static str> { Ok(()) } - fn make_raw_payment(_: &T, _: B) -> Result<(), &'static str> { Ok(()) } + fn make_raw_payment(_: &T, _: Self::Balance) -> Result<(), &'static str> { Ok(()) } } /// A trait for finding the author of a block header based on the `PreRuntime` digests contained From 6836f6b0cf3d16099a93374b488c1f9c1443bf11 Mon Sep 17 00:00:00 2001 From: kianenigma Date: Wed, 10 Jul 2019 10:25:14 +0200 Subject: [PATCH 18/23] Optionize tip. --- .../src/generic/checked_extrinsic.rs | 17 ++++++----- core/sr-primitives/src/generic/tip.rs | 9 ++---- .../src/generic/unchecked_extrinsic.rs | 6 ++-- .../unchecked_mortal_compact_extrinsic.rs | 12 ++++---- ...checked_mortal_compact_tipped_extrinsic.rs | 27 ++++++++++-------- .../src/generic/unchecked_mortal_extrinsic.rs | 12 ++++---- core/sr-primitives/src/testing.rs | 4 +-- node/cli/src/factory_impl.rs | 2 +- node/cli/src/service.rs | 4 +-- node/executor/src/lib.rs | 28 +++++++++---------- srml/executive/src/lib.rs | 16 ++++++----- 11 files changed, 70 insertions(+), 67 deletions(-) diff --git a/core/sr-primitives/src/generic/checked_extrinsic.rs b/core/sr-primitives/src/generic/checked_extrinsic.rs index 5a672aa87a883..5de644eb2b358 100644 --- a/core/sr-primitives/src/generic/checked_extrinsic.rs +++ b/core/sr-primitives/src/generic/checked_extrinsic.rs @@ -34,7 +34,7 @@ pub struct CheckedExtrinsic { /// The function that should be called. pub function: Call, /// The validated tip value for this transaction. - pub tip: Tip, + pub tip: Option>, } impl traits::Applyable for CheckedExtrinsic @@ -75,27 +75,26 @@ where /// node. /// /// In practice, if they underlying unchecked transaction is tip-aware, they are the same. Otherwise, -/// the tip is always `Tip::None` and the type is of no importance. +/// the tip is always `None` and the type is of no importance. impl Tippable for CheckedExtrinsic where ExtBalance: Clone + Copy, NodeBalance: From, { - fn tip(&self) -> Tip { + fn tip(&self) -> Option> { // This is a hacky way to prevent `unchecked_mortal[_compact]_extrinsic` types, which // don't have a tip, become generic over a balance type. // Basically, this CheckedExtrinsic is built either 1- from an // `UncheckedMortalCompactTippedExtrinsic`, which is tip-aware and hence, the second arm // will be trivially executed and the type conversion will be safe (the compiler is probably // smart enough to remove it in fact). In this case, NodeBalance and ExtBalance are the same. - // Or 2- this is built from all other types of unchecked - // extrinsic which do not have tip and hence are not tip-aware. These modules will naively - // place a u32 (can be `()` in practice) as the type and it does not matter since `Tip::None` - // is used in this case (first arm). + // Or 2- this is built from all other types of uncheckedextrinsic which do not have tip and + // hence are not tip-aware. These modules will naively place a u32 (can be `()` in practice) + // as the type and it does not matter since `None` is used in this case (first arm). match self.tip { - Tip::None => Tip::None, - Tip::Sender(v) => Tip::Sender(NodeBalance::from(v)) + None => None, + Some(Tip::Sender(v)) => Some(Tip::Sender(NodeBalance::from(v))), } } } \ No newline at end of file diff --git a/core/sr-primitives/src/generic/tip.rs b/core/sr-primitives/src/generic/tip.rs index a81325f7bd0b0..6acdc4b83a7af 100644 --- a/core/sr-primitives/src/generic/tip.rs +++ b/core/sr-primitives/src/generic/tip.rs @@ -27,8 +27,6 @@ use crate::traits::Zero; #[cfg_attr(feature = "std", derive(Debug))] #[derive(Clone, Copy, Eq, PartialEq, Encode, Decode)] pub enum Tip { - /// This transaction does not include any tips. - None, /// The sender of the transaction has included some tip. Sender(Balance), } @@ -39,15 +37,14 @@ impl Tip { pub fn value(&self) -> Balance { match *self { Tip::Sender(value) => value, - Tip::None => Zero::zero(), } } } /// Default implementation for tip. -impl Default for Tip { +impl Default for Tip where Balance: Zero { fn default() -> Self { - Tip::None + Tip::Sender(Zero::zero()) } } @@ -55,5 +52,5 @@ impl Default for Tip { /// that translates to "no tip" but this trait must always be implemented for `UncheckedExtrinsic`. pub trait Tippable { /// Return the tip associated with this transaction. - fn tip(&self) -> Tip; + fn tip(&self) -> Option>; } diff --git a/core/sr-primitives/src/generic/unchecked_extrinsic.rs b/core/sr-primitives/src/generic/unchecked_extrinsic.rs index c49fb5afaf69d..31fe9f6641da7 100644 --- a/core/sr-primitives/src/generic/unchecked_extrinsic.rs +++ b/core/sr-primitives/src/generic/unchecked_extrinsic.rs @@ -21,7 +21,7 @@ use std::fmt; use rstd::prelude::*; use crate::codec::{Decode, Encode, Codec, Input, HasCompact}; use crate::traits::{self, Member, SimpleArithmetic, MaybeDisplay, Lookup, Extrinsic}; -use super::{CheckedExtrinsic, Tip}; +use super::{CheckedExtrinsic}; #[derive(PartialEq, Eq, Clone, Encode, Decode)] pub struct SignatureContent @@ -97,13 +97,13 @@ where CheckedExtrinsic { signed: Some((signed, payload.0)), function: payload.1, - tip: Tip::None, + tip: None, } } None => CheckedExtrinsic { signed: None, function: self.function, - tip: Tip::None, + tip: None, }, }) } diff --git a/core/sr-primitives/src/generic/unchecked_mortal_compact_extrinsic.rs b/core/sr-primitives/src/generic/unchecked_mortal_compact_extrinsic.rs index df9011a20efac..adf07b9252a57 100644 --- a/core/sr-primitives/src/generic/unchecked_mortal_compact_extrinsic.rs +++ b/core/sr-primitives/src/generic/unchecked_mortal_compact_extrinsic.rs @@ -24,7 +24,7 @@ use runtime_io::blake2_256; use crate::codec::{Decode, Encode, Input, Compact}; use crate::traits::{self, Member, SimpleArithmetic, MaybeDisplay, CurrentHeight, BlockNumberToHash, Lookup, Checkable, Extrinsic, SaturatedConversion}; -use super::{CheckedExtrinsic, Era, Tip}; +use super::{CheckedExtrinsic, Era}; const TRANSACTION_VERSION: u8 = 1; @@ -108,13 +108,13 @@ where CheckedExtrinsic { signed: Some((signed, (raw_payload.0).0)), function: raw_payload.1, - tip: Tip::None, + tip: None, } } None => CheckedExtrinsic { signed: None, function: self.function, - tip: Tip::None, + tip: None, }, }) } @@ -279,21 +279,21 @@ mod tests { fn immortal_signed_check_should_work() { let ux = Ex::new_signed(0, vec![0u8;0], DUMMY_ACCOUNTID, TestSig(DUMMY_ACCOUNTID, (Compact::from(DUMMY_ACCOUNTID), vec![0u8;0], Era::immortal(), 0u64).encode()), Era::immortal()); assert!(ux.is_signed().unwrap_or(false)); - assert_eq!(>::check(ux, &TestContext), Ok(CEx { signed: Some((DUMMY_ACCOUNTID, 0)), function: vec![0u8;0], tip: Tip::None })); + assert_eq!(>::check(ux, &TestContext), Ok(CEx { signed: Some((DUMMY_ACCOUNTID, 0)), function: vec![0u8;0], tip: None })); } #[test] fn mortal_signed_check_should_work() { let ux = Ex::new_signed(0, vec![0u8;0], DUMMY_ACCOUNTID, TestSig(DUMMY_ACCOUNTID, (Compact::from(DUMMY_ACCOUNTID), vec![0u8;0], Era::mortal(32, 42), 42u64).encode()), Era::mortal(32, 42)); assert!(ux.is_signed().unwrap_or(false)); - assert_eq!(>::check(ux, &TestContext), Ok(CEx { signed: Some((DUMMY_ACCOUNTID, 0)), function: vec![0u8;0], tip: Tip::None })); + assert_eq!(>::check(ux, &TestContext), Ok(CEx { signed: Some((DUMMY_ACCOUNTID, 0)), function: vec![0u8;0], tip: None })); } #[test] fn later_mortal_signed_check_should_work() { let ux = Ex::new_signed(0, vec![0u8;0], DUMMY_ACCOUNTID, TestSig(DUMMY_ACCOUNTID, (Compact::from(DUMMY_ACCOUNTID), vec![0u8;0], Era::mortal(32, 11), 11u64).encode()), Era::mortal(32, 11)); assert!(ux.is_signed().unwrap_or(false)); - assert_eq!(>::check(ux, &TestContext), Ok(CEx { signed: Some((DUMMY_ACCOUNTID, 0)), function: vec![0u8;0], tip: Tip::None })); + assert_eq!(>::check(ux, &TestContext), Ok(CEx { signed: Some((DUMMY_ACCOUNTID, 0)), function: vec![0u8;0], tip: None })); } #[test] diff --git a/core/sr-primitives/src/generic/unchecked_mortal_compact_tipped_extrinsic.rs b/core/sr-primitives/src/generic/unchecked_mortal_compact_tipped_extrinsic.rs index 2a6e57b05a22d..c076b7b75663c 100644 --- a/core/sr-primitives/src/generic/unchecked_mortal_compact_tipped_extrinsic.rs +++ b/core/sr-primitives/src/generic/unchecked_mortal_compact_tipped_extrinsic.rs @@ -30,6 +30,11 @@ const TRANSACTION_VERSION: u8 = 1; /// A extrinsic right from the external world. This is unchecked and so /// can contain a signature. +/// +/// This type transaction: +/// - _Must_ always have a valid `Tip` encoded, and included in the signature payload if it is +/// signed. +/// - _Must_ not provide any `Tip` if it is unsigned. #[derive(PartialEq, Eq, Clone)] pub struct UncheckedMortalCompactTippedExtrinsic { /// The signature, address, number of extrinsics have come before from @@ -39,7 +44,7 @@ pub struct UncheckedMortalCompactTippedExtrinsic, + pub tip: Option>, } impl @@ -52,7 +57,7 @@ impl signed: Address, signature: Signature, era: Era, - tip: Tip + tip: Option> ) -> Self { UncheckedMortalCompactTippedExtrinsic { signature: Some((signed, signature, index.into(), era)), @@ -66,7 +71,7 @@ impl UncheckedMortalCompactTippedExtrinsic { signature: None, function, - tip: Tip::None + tip: None } } } @@ -122,14 +127,14 @@ where } None => { // An unsigned transaction cannot have a tip. The decode code should replace it with - // `Tip::None` and ignore the input bytes. - if self.tip != Tip::None { + // `None` and ignore the input bytes. + if self.tip.is_some() { return Err(crate::UNSIGNED_TIP); } CheckedExtrinsic { signed: None, function: self.function, - tip: self.tip + tip: None } }, }) @@ -164,7 +169,7 @@ where Some(UncheckedMortalCompactTippedExtrinsic { signature: if is_signed { Some(Decode::decode(input)?) } else { None }, function: Decode::decode(input)?, - tip: if is_signed { Decode::decode(input)? } else { Tip::None }, + tip: if is_signed { Decode::decode(input)? } else { None }, }) } } @@ -261,9 +266,9 @@ mod tests { } type Balance = u64; - + type TipType = Tip; const DUMMY_ACCOUNTID: u64 = 0; - const TIP: Tip = Tip::Sender(66); + const TIP: Option = Some(Tip::Sender(66)); type Ex = UncheckedMortalCompactTippedExtrinsic, TestSig, Balance>; type CEx = CheckedExtrinsic, Balance>; @@ -450,7 +455,7 @@ mod tests { #[test] #[should_panic] fn unsigned_cannot_have_tip() { - let ux = UncheckedMortalCompactTippedExtrinsic {signature: None, tip: Tip::Sender(100), function: vec![0u8;0]}; + let ux = UncheckedMortalCompactTippedExtrinsic {signature: None, tip: Some(Tip::Sender(100)), function: vec![0u8;0]}; let _ = >::check(ux, &TestContext).unwrap(); } @@ -463,6 +468,6 @@ mod tests { let decoded = Ex::decode(&mut bytes.as_slice()).unwrap(); assert_eq!( decoded, - UncheckedMortalCompactTippedExtrinsic { signature: None, tip: Tip::None, function: vec![8u8;8]}) + UncheckedMortalCompactTippedExtrinsic { signature: None, tip: None, function: vec![8u8;8]}) } } diff --git a/core/sr-primitives/src/generic/unchecked_mortal_extrinsic.rs b/core/sr-primitives/src/generic/unchecked_mortal_extrinsic.rs index a237a3aed3c6c..4cd44b4d4ba4f 100644 --- a/core/sr-primitives/src/generic/unchecked_mortal_extrinsic.rs +++ b/core/sr-primitives/src/generic/unchecked_mortal_extrinsic.rs @@ -26,7 +26,7 @@ use crate::traits::{ self, Member, SimpleArithmetic, MaybeDisplay, CurrentHeight, BlockNumberToHash, Lookup, Checkable, Extrinsic, SaturatedConversion }; -use super::{CheckedExtrinsic, Era, Tip}; +use super::{CheckedExtrinsic, Era}; const TRANSACTION_VERSION: u8 = 1; @@ -107,13 +107,13 @@ where CheckedExtrinsic { signed: Some((signed, raw_payload.0)), function: raw_payload.1, - tip: Tip::None, + tip: None, } } None => CheckedExtrinsic { signed: None, function: self.function, - tip: Tip::None, + tip: None, }, }) } @@ -277,21 +277,21 @@ mod tests { fn immortal_signed_check_should_work() { let ux = Ex::new_signed(0, vec![0u8;0], DUMMY_ACCOUNTID, TestSig(DUMMY_ACCOUNTID, (DUMMY_ACCOUNTID, vec![0u8;0], Era::immortal(), 0u64).encode()), Era::immortal()); assert!(ux.is_signed().unwrap_or(false)); - assert_eq!(>::check(ux, &TestContext), Ok(CEx { signed: Some((DUMMY_ACCOUNTID, 0)), function: vec![0u8;0], tip: Tip::None })); + assert_eq!(>::check(ux, &TestContext), Ok(CEx { signed: Some((DUMMY_ACCOUNTID, 0)), function: vec![0u8;0], tip: None })); } #[test] fn mortal_signed_check_should_work() { let ux = Ex::new_signed(0, vec![0u8;0], DUMMY_ACCOUNTID, TestSig(DUMMY_ACCOUNTID, (DUMMY_ACCOUNTID, vec![0u8;0], Era::mortal(32, 42), 42u64).encode()), Era::mortal(32, 42)); assert!(ux.is_signed().unwrap_or(false)); - assert_eq!(>::check(ux, &TestContext), Ok(CEx { signed: Some((DUMMY_ACCOUNTID, 0)), function: vec![0u8;0], tip: Tip::None })); + assert_eq!(>::check(ux, &TestContext), Ok(CEx { signed: Some((DUMMY_ACCOUNTID, 0)), function: vec![0u8;0], tip: None })); } #[test] fn later_mortal_signed_check_should_work() { let ux = Ex::new_signed(0, vec![0u8;0], DUMMY_ACCOUNTID, TestSig(DUMMY_ACCOUNTID, (DUMMY_ACCOUNTID, vec![0u8;0], Era::mortal(32, 11), 11u64).encode()), Era::mortal(32, 11)); assert!(ux.is_signed().unwrap_or(false)); - assert_eq!(>::check(ux, &TestContext), Ok(CEx { signed: Some((DUMMY_ACCOUNTID, 0)), function: vec![0u8;0], tip: Tip::None })); + assert_eq!(>::check(ux, &TestContext), Ok(CEx { signed: Some((DUMMY_ACCOUNTID, 0)), function: vec![0u8;0], tip: None })); } #[test] diff --git a/core/sr-primitives/src/testing.rs b/core/sr-primitives/src/testing.rs index 54d1b569c9f92..648e260551348 100644 --- a/core/sr-primitives/src/testing.rs +++ b/core/sr-primitives/src/testing.rs @@ -226,7 +226,7 @@ impl Weighable for TestXt { } } impl generic::Tippable for TestXt { - fn tip(&self) -> generic::Tip { - generic::Tip::None + fn tip(&self) -> Option> { + None } } diff --git a/node/cli/src/factory_impl.rs b/node/cli/src/factory_impl.rs index 1c17366e5524c..754203ebf6b6c 100644 --- a/node/cli/src/factory_impl.rs +++ b/node/cli/src/factory_impl.rs @@ -141,7 +141,7 @@ impl RuntimeAdapter for FactoryState { (*amount).into() ) ), - tip: Tip::default(), + tip: None, }, key, &prior_block_hash, phase) } diff --git a/node/cli/src/service.rs b/node/cli/src/service.rs index 7b86ea21f432a..e27baca8e6995 100644 --- a/node/cli/src/service.rs +++ b/node/cli/src/service.rs @@ -217,14 +217,14 @@ mod tests { use std::sync::Arc; use consensus::CompatibleDigestItem; use consensus_common::{Environment, Proposer, ImportBlock, BlockOrigin, ForkChoiceStrategy}; - use node_primitives::{DigestItem, Balance}; + use node_primitives::DigestItem; use node_runtime::{Call, BalancesCall, UncheckedExtrinsic}; use parity_codec::{Compact, Encode, Decode}; use primitives::{ crypto::Pair as CryptoPair, ed25519::Pair, blake2_256, sr25519::Public as AddressPublic, H256, }; - use sr_primitives::{generic::{BlockId, Era, Digest, Tip}, traits::Block, OpaqueExtrinsic}; + use sr_primitives::{generic::{BlockId, Era, Digest}, traits::Block, OpaqueExtrinsic}; use timestamp; use finality_tracker; use keyring::{ed25519::Keyring as AuthorityKeyring, sr25519::Keyring as AccountKeyring}; diff --git a/node/executor/src/lib.rs b/node/executor/src/lib.rs index 2327699115f93..f4340e6483fcb 100644 --- a/node/executor/src/lib.rs +++ b/node/executor/src/lib.rs @@ -132,7 +132,7 @@ mod tests { sign(CheckedExtrinsic { signed: Some((alice(), 0)), function: Call::Balances(balances::Call::transfer::(bob().into(), 69)), - tip: Tip::default(), + tip: None, }) } @@ -467,12 +467,12 @@ mod tests { CheckedExtrinsic { signed: None, function: Call::Timestamp(timestamp::Call::set(42)), - tip: Tip::None, + tip: None, }, CheckedExtrinsic { signed: Some((alice(), 0)), function: Call::Balances(balances::Call::transfer(bob().into(), 69)), - tip: Tip::default(), + tip: None, }, ] ) @@ -491,12 +491,12 @@ mod tests { CheckedExtrinsic { signed: None, function: Call::Timestamp(timestamp::Call::set(42)), - tip: Tip::None, + tip: None, }, CheckedExtrinsic { signed: Some((alice(), 0)), function: Call::Balances(balances::Call::transfer(bob().into(), 69)), - tip: Tip::default(), + tip: None, }, ] ); @@ -508,17 +508,17 @@ mod tests { CheckedExtrinsic { signed: None, function: Call::Timestamp(timestamp::Call::set(52)), - tip: Tip::default(), + tip: None, }, CheckedExtrinsic { signed: Some((bob(), 0)), function: Call::Balances(balances::Call::transfer(alice().into(), 5)), - tip: Tip::default(), + tip: None, }, CheckedExtrinsic { signed: Some((alice(), 1)), function: Call::Balances(balances::Call::transfer(bob().into(), 15)), - tip: Tip::default(), + tip: None, } ] ); @@ -540,12 +540,12 @@ mod tests { CheckedExtrinsic { signed: None, function: Call::Timestamp(timestamp::Call::set(42)), - tip: Tip::None, + tip: None, }, CheckedExtrinsic { signed: Some((alice(), 0)), function: Call::System(system::Call::remark(vec![0; 120000])), - tip: Tip::default(), + tip: None, } ] ) @@ -820,21 +820,21 @@ mod tests { CheckedExtrinsic { signed: None, function: Call::Timestamp(timestamp::Call::set(42)), - tip: Tip::None, + tip: None, }, CheckedExtrinsic { signed: Some((charlie(), 0)), function: Call::Contracts( contracts::Call::put_code::(10_000, transfer_code) ), - tip: Tip::default(), + tip: None, }, CheckedExtrinsic { signed: Some((charlie(), 1)), function: Call::Contracts( contracts::Call::create::(10, 10_000, transfer_ch, Vec::new()) ), - tip: Tip::default(), + tip: None, }, CheckedExtrinsic { signed: Some((charlie(), 2)), @@ -846,7 +846,7 @@ mod tests { vec![0x00, 0x01, 0x02, 0x03] ) ), - tip: Tip::default(), + tip: None, }, ] ); diff --git a/srml/executive/src/lib.rs b/srml/executive/src/lib.rs index cc52c998a5166..2a849d02642d3 100644 --- a/srml/executive/src/lib.rs +++ b/srml/executive/src/lib.rs @@ -364,12 +364,14 @@ where } // pay and burn the tip if provided. - let tip = xt.tip().value(); - let weight = xt.weight(encoded_len); - - if !tip.is_zero() { - if Payment::make_raw_payment(sender, tip).is_err() { - return TransactionValidity::Invalid(ApplyError::CantPay as i8) + if let Some(tip) = xt.tip() { + let tip = tip.value(); + let weight = xt.weight(encoded_len); + + if !tip.is_zero() { + if Payment::make_raw_payment(sender, tip).is_err() { + return TransactionValidity::Invalid(ApplyError::CantPay as i8) + } } } @@ -389,7 +391,7 @@ where // TODO: maximise (fee + tip) per weight unit here. TransactionValidity::Valid { - priority: (encoded_len as TransactionPriority) + tip.saturated_into::(), + priority: (encoded_len as TransactionPriority), requires, provides, longevity: TransactionLongevity::max_value(), From 7191a7dc6472ea3355ebceb3aceaed8551b58c08 Mon Sep 17 00:00:00 2001 From: kianenigma Date: Wed, 10 Jul 2019 10:56:47 +0200 Subject: [PATCH 19/23] Further cleanup. --- .../src/generic/checked_extrinsic.rs | 3 +-- .../src/generic/unchecked_extrinsic.rs | 5 +++-- .../unchecked_mortal_compact_extrinsic.rs | 4 ++-- ...nchecked_mortal_compact_tipped_extrinsic.rs | 2 +- .../src/generic/unchecked_mortal_extrinsic.rs | 2 +- node/cli/src/factory_impl.rs | 4 ++-- node/executor/src/lib.rs | 8 +++----- srml/executive/src/lib.rs | 18 ++++++++++-------- srml/support/src/inherent.rs | 1 - srml/support/src/traits.rs | 2 +- subkey/src/main.rs | 6 +++--- 11 files changed, 27 insertions(+), 28 deletions(-) diff --git a/core/sr-primitives/src/generic/checked_extrinsic.rs b/core/sr-primitives/src/generic/checked_extrinsic.rs index 5de644eb2b358..ed5816dcced5a 100644 --- a/core/sr-primitives/src/generic/checked_extrinsic.rs +++ b/core/sr-primitives/src/generic/checked_extrinsic.rs @@ -21,7 +21,6 @@ use crate::traits::{self, Member, SimpleArithmetic, MaybeDisplay}; use crate::weights::{Weighable, Weight}; use crate::generic::tip::{Tip, Tippable}; - /// Definition of something that the external world might want to say; its /// existence implies that it has been checked and is good, particularly with /// regards to the signature. @@ -33,7 +32,7 @@ pub struct CheckedExtrinsic { pub signed: Option<(AccountId, Index)>, /// The function that should be called. pub function: Call, - /// The validated tip value for this transaction. + /// An optional tip value that may or may not exist based on the underlying unchecked extrinsic. pub tip: Option>, } diff --git a/core/sr-primitives/src/generic/unchecked_extrinsic.rs b/core/sr-primitives/src/generic/unchecked_extrinsic.rs index 31fe9f6641da7..a325081f238b9 100644 --- a/core/sr-primitives/src/generic/unchecked_extrinsic.rs +++ b/core/sr-primitives/src/generic/unchecked_extrinsic.rs @@ -15,13 +15,14 @@ // along with Substrate. If not, see . //! Generic implementation of an unchecked (pre-verification) extrinsic. + #[cfg(feature = "std")] use std::fmt; use rstd::prelude::*; use crate::codec::{Decode, Encode, Codec, Input, HasCompact}; use crate::traits::{self, Member, SimpleArithmetic, MaybeDisplay, Lookup, Extrinsic}; -use super::{CheckedExtrinsic}; +use super::CheckedExtrinsic; #[derive(PartialEq, Eq, Clone, Encode, Decode)] pub struct SignatureContent @@ -196,4 +197,4 @@ mod test { assert_eq!(serde_json::to_string(&ex).unwrap(), "\"0x14002a000000\""); } -} \ No newline at end of file +} diff --git a/core/sr-primitives/src/generic/unchecked_mortal_compact_extrinsic.rs b/core/sr-primitives/src/generic/unchecked_mortal_compact_extrinsic.rs index adf07b9252a57..0dab7b71d324f 100644 --- a/core/sr-primitives/src/generic/unchecked_mortal_compact_extrinsic.rs +++ b/core/sr-primitives/src/generic/unchecked_mortal_compact_extrinsic.rs @@ -41,7 +41,7 @@ pub struct UncheckedMortalCompactExtrinsic { } impl -UncheckedMortalCompactExtrinsic + UncheckedMortalCompactExtrinsic { /// New instance of a signed extrinsic aka "transaction". pub fn new_signed(index: Index, function: Call, signed: Address, signature: Signature, era: Era) -> Self { @@ -319,4 +319,4 @@ mod tests { let as_vec: Vec = Decode::decode(&mut encoded.as_slice()).unwrap(); assert_eq!(as_vec.encode(), encoded); } -} \ No newline at end of file +} diff --git a/core/sr-primitives/src/generic/unchecked_mortal_compact_tipped_extrinsic.rs b/core/sr-primitives/src/generic/unchecked_mortal_compact_tipped_extrinsic.rs index c076b7b75663c..ef2411bab3282 100644 --- a/core/sr-primitives/src/generic/unchecked_mortal_compact_tipped_extrinsic.rs +++ b/core/sr-primitives/src/generic/unchecked_mortal_compact_tipped_extrinsic.rs @@ -26,7 +26,7 @@ use crate::traits::{self, Member, SimpleArithmetic, MaybeDisplay, CurrentHeight, Lookup, Checkable, Extrinsic, SaturatedConversion}; use super::{CheckedExtrinsic, Era, Tip}; -const TRANSACTION_VERSION: u8 = 1; +const TRANSACTION_VERSION: u8 = 2; /// A extrinsic right from the external world. This is unchecked and so /// can contain a signature. diff --git a/core/sr-primitives/src/generic/unchecked_mortal_extrinsic.rs b/core/sr-primitives/src/generic/unchecked_mortal_extrinsic.rs index 4cd44b4d4ba4f..69a083967117c 100644 --- a/core/sr-primitives/src/generic/unchecked_mortal_extrinsic.rs +++ b/core/sr-primitives/src/generic/unchecked_mortal_extrinsic.rs @@ -317,4 +317,4 @@ mod tests { let as_vec: Vec = Decode::decode(&mut encoded.as_slice()).unwrap(); assert_eq!(as_vec.encode(), encoded); } -} \ No newline at end of file +} diff --git a/node/cli/src/factory_impl.rs b/node/cli/src/factory_impl.rs index 754203ebf6b6c..a2a6820cdbfcd 100644 --- a/node/cli/src/factory_impl.rs +++ b/node/cli/src/factory_impl.rs @@ -23,7 +23,7 @@ use rand::rngs::StdRng; use parity_codec::Decode; use keyring::sr25519::Keyring; -use node_primitives::{Hash, Balance}; +use node_primitives::Hash; use node_runtime::{Call, CheckedExtrinsic, UncheckedExtrinsic, BalancesCall}; use primitives::sr25519; use primitives::crypto::Pair; @@ -236,7 +236,7 @@ fn sign( let s = match xt.signed { Some((signed, index)) => { let era = Era::mortal(256, phase); - let payload = (index.into(), xt.function, Tip::::default(), era, prior_block_hash); + let payload = (index.into(), xt.function, era, prior_block_hash); let signature = payload.using_encoded(|b| { if b.len() > 256 { key.sign(&sr_io::blake2_256(b)) diff --git a/node/executor/src/lib.rs b/node/executor/src/lib.rs index f4340e6483fcb..a7d7a46f16537 100644 --- a/node/executor/src/lib.rs +++ b/node/executor/src/lib.rs @@ -885,15 +885,13 @@ mod tests { #[test] fn native_big_block_import_succeeds() { let mut t = new_test_ext(COMPACT_CODE, false); - let block = big_block().0; - let r = Executor::new(None).call::<_, NeverNativeValue, fn() -> _>( + Executor::new(None).call::<_, NeverNativeValue, fn() -> _>( &mut t, "Core_execute_block", - &block, + &big_block().0, true, None, - ); - r.0.unwrap(); + ).0.unwrap(); } #[test] diff --git a/srml/executive/src/lib.rs b/srml/executive/src/lib.rs index 2a849d02642d3..b5210bd02bed2 100644 --- a/srml/executive/src/lib.rs +++ b/srml/executive/src/lib.rs @@ -80,7 +80,7 @@ use primitives::{ApplyOutcome, ApplyError, weights::Weighable, transaction_validity::{TransactionValidity, TransactionPriority, TransactionLongevity}, traits::{self, Header, Zero, One, Checkable, Applyable, CheckEqual, OnFinalize, OnInitialize, - NumberFor, Block as BlockT, OffchainWorker, ValidateUnsigned, SaturatedConversion, + NumberFor, Block as BlockT, OffchainWorker, ValidateUnsigned, SimpleArithmetic, }, }; @@ -287,6 +287,12 @@ where // pay any fees Payment::make_payment(sender, encoded_len).map_err(|_| internal::ApplyError::CantPay)?; + // pay any tip if provided.. + if let Some(tip) = xt.tip() { + let tip = tip.value(); + Payment::make_raw_payment(sender, tip).map_err(|_| internal::ApplyError::CantPay)?; + } + // AUDIT: Under no circumstances may this function panic from here onwards. // FIXME: ensure this at compile-time (such as by not defining a panic function, forcing // a linker error unless the compiler can prove it cannot be called). @@ -366,12 +372,8 @@ where // pay and burn the tip if provided. if let Some(tip) = xt.tip() { let tip = tip.value(); - let weight = xt.weight(encoded_len); - - if !tip.is_zero() { - if Payment::make_raw_payment(sender, tip).is_err() { - return TransactionValidity::Invalid(ApplyError::CantPay as i8) - } + if Payment::make_raw_payment(sender, tip).is_err() { + return TransactionValidity::Invalid(ApplyError::CantPay as i8) } } @@ -389,7 +391,7 @@ where vec![] }; - // TODO: maximise (fee + tip) per weight unit here. + // TODO TODO: maximise (fee + tip) per weight unit here. TransactionValidity::Valid { priority: (encoded_len as TransactionPriority), requires, diff --git a/srml/support/src/inherent.rs b/srml/support/src/inherent.rs index 402066ea9cefa..d886abbca7e37 100644 --- a/srml/support/src/inherent.rs +++ b/srml/support/src/inherent.rs @@ -58,7 +58,6 @@ macro_rules! impl_outer_inherent { let mut inherents = Vec::new(); - // TODO: fix tip value here. $( if let Some(inherent) = $module::create_inherent(self) { inherents.push($uncheckedextrinsic::new_unsigned( diff --git a/srml/support/src/traits.rs b/srml/support/src/traits.rs index 8372967b2fde4..44033b5838b0c 100644 --- a/srml/support/src/traits.rs +++ b/srml/support/src/traits.rs @@ -21,7 +21,7 @@ use crate::rstd::result; use crate::codec::{Codec, Encode, Decode}; use crate::runtime_primitives::traits::{ - MaybeSerializeDebug, SimpleArithmetic, + MaybeSerializeDebug, SimpleArithmetic }; use crate::runtime_primitives::ConsensusEngineId; diff --git a/subkey/src/main.rs b/subkey/src/main.rs index b85e36eca41cc..b38bffd772bbf 100644 --- a/subkey/src/main.rs +++ b/subkey/src/main.rs @@ -24,7 +24,7 @@ use clap::load_yaml; use bip39::{Mnemonic, Language, MnemonicType}; use substrate_primitives::{ed25519, sr25519, hexdisplay::HexDisplay, Pair, crypto::Ss58Codec, blake2_256}; use parity_codec::{Encode, Decode, Compact}; -use sr_primitives::generic::{Era, Tip}; +use sr_primitives::generic::Era; use node_primitives::{Balance, Index, Hash}; use node_runtime::{Call, UncheckedExtrinsic, BalancesCall}; @@ -153,7 +153,7 @@ fn execute(matches: clap::ArgMatches) where println!("Using a genesis hash of {}", HexDisplay::from(&genesis_hash.as_ref())); let era = Era::immortal(); - let raw_payload = (Compact(index), function, Tip::::default(), era, genesis_hash); + let raw_payload = (Compact(index), function, era, genesis_hash); let signature = raw_payload.using_encoded(|payload| if payload.len() > 256 { signer.sign(&blake2_256(payload)[..]) } else { @@ -192,7 +192,7 @@ fn execute(matches: clap::ArgMatches) where let era = Era::immortal(); - let raw_payload = (Compact(index), function, era, Tip::::default(), prior_block_hash); + let raw_payload = (Compact(index), function, era, prior_block_hash); let signature = raw_payload.using_encoded(|payload| if payload.len() > 256 { signer.sign(&blake2_256(payload)[..]) From 0846065b8eecb3e2f45461cb3e10c2079e669640 Mon Sep 17 00:00:00 2001 From: kianenigma Date: Wed, 10 Jul 2019 11:07:17 +0200 Subject: [PATCH 20/23] Further cleanup. --- core/consensus/rhd/src/lib.rs | 2 -- core/sr-api-macros/tests/trybuild.rs | 1 - core/sr-primitives/src/generic/tip.rs | 15 ++++----------- .../unchecked_mortal_compact_extrinsic.rs | 16 ++++++++++++---- .../src/generic/unchecked_mortal_extrinsic.rs | 15 ++++++++++++--- node/executor/src/lib.rs | 1 + 6 files changed, 29 insertions(+), 21 deletions(-) diff --git a/core/consensus/rhd/src/lib.rs b/core/consensus/rhd/src/lib.rs index 405ae61606cf9..5ef19aef22ed7 100644 --- a/core/consensus/rhd/src/lib.rs +++ b/core/consensus/rhd/src/lib.rs @@ -1265,8 +1265,6 @@ impl LocalProposer<::Block> for Proposer where => MisbehaviorKind::BftDoubleCommit(round as u32, (h1.into(), s1.signature), (h2.into(), s2.signature)), } }; - /// NOTE: This is not compatible with the new signature payload. - /// a tip must be present as the third argument. let payload = ( next_index, Call::Consensus(ConsensusCall::report_misbehavior(report)), diff --git a/core/sr-api-macros/tests/trybuild.rs b/core/sr-api-macros/tests/trybuild.rs index d64f5b45b38d8..e04c67737a486 100644 --- a/core/sr-api-macros/tests/trybuild.rs +++ b/core/sr-api-macros/tests/trybuild.rs @@ -1,5 +1,4 @@ #[test] -#[ignore] fn ui() { let t = trybuild::TestCases::new(); t.compile_fail("tests/ui/*.rs"); diff --git a/core/sr-primitives/src/generic/tip.rs b/core/sr-primitives/src/generic/tip.rs index 6acdc4b83a7af..270f6d4e4ac79 100644 --- a/core/sr-primitives/src/generic/tip.rs +++ b/core/sr-primitives/src/generic/tip.rs @@ -21,9 +21,9 @@ use crate::traits::Zero; /// Representation of a transaction tip. /// -/// Upon decoding, all transaction types try and decode this from the end of the encoded byte -/// stream. -/// If non-existent, the default implementation will be used. +/// Provided as an enum to support potential future use cases such as: +/// - Tipped by a third party (software or exchange). +/// - Unsigned tip. #[cfg_attr(feature = "std", derive(Debug))] #[derive(Clone, Copy, Eq, PartialEq, Encode, Decode)] pub enum Tip { @@ -41,15 +41,8 @@ impl Tip { } } -/// Default implementation for tip. -impl Default for Tip where Balance: Zero { - fn default() -> Self { - Tip::Sender(Zero::zero()) - } -} - /// A trait for a generic transaction that contains a tip. The tip itself might yield something -/// that translates to "no tip" but this trait must always be implemented for `UncheckedExtrinsic`. +/// that translates to "no tip". pub trait Tippable { /// Return the tip associated with this transaction. fn tip(&self) -> Option>; diff --git a/core/sr-primitives/src/generic/unchecked_mortal_compact_extrinsic.rs b/core/sr-primitives/src/generic/unchecked_mortal_compact_extrinsic.rs index 0dab7b71d324f..c374b73c91a95 100644 --- a/core/sr-primitives/src/generic/unchecked_mortal_compact_extrinsic.rs +++ b/core/sr-primitives/src/generic/unchecked_mortal_compact_extrinsic.rs @@ -85,7 +85,6 @@ where { /// NOTE: this transaction is not tipped i.e. the tip value will be `None`. It does not really /// matter what the last generic is since it is always `None`. - // TODO: try and use () here since it is a bit nicer. type Checked = CheckedExtrinsic; fn check(self, context: &Context) -> Result { @@ -279,21 +278,30 @@ mod tests { fn immortal_signed_check_should_work() { let ux = Ex::new_signed(0, vec![0u8;0], DUMMY_ACCOUNTID, TestSig(DUMMY_ACCOUNTID, (Compact::from(DUMMY_ACCOUNTID), vec![0u8;0], Era::immortal(), 0u64).encode()), Era::immortal()); assert!(ux.is_signed().unwrap_or(false)); - assert_eq!(>::check(ux, &TestContext), Ok(CEx { signed: Some((DUMMY_ACCOUNTID, 0)), function: vec![0u8;0], tip: None })); + assert_eq!( + >::check(ux, &TestContext), + Ok(CEx { signed: Some((DUMMY_ACCOUNTID, 0)), function: vec![0u8;0], tip: None }) + ); } #[test] fn mortal_signed_check_should_work() { let ux = Ex::new_signed(0, vec![0u8;0], DUMMY_ACCOUNTID, TestSig(DUMMY_ACCOUNTID, (Compact::from(DUMMY_ACCOUNTID), vec![0u8;0], Era::mortal(32, 42), 42u64).encode()), Era::mortal(32, 42)); assert!(ux.is_signed().unwrap_or(false)); - assert_eq!(>::check(ux, &TestContext), Ok(CEx { signed: Some((DUMMY_ACCOUNTID, 0)), function: vec![0u8;0], tip: None })); + assert_eq!( + >::check(ux, &TestContext), + Ok(CEx { signed: Some((DUMMY_ACCOUNTID, 0)), function: vec![0u8;0], tip: None }) + ); } #[test] fn later_mortal_signed_check_should_work() { let ux = Ex::new_signed(0, vec![0u8;0], DUMMY_ACCOUNTID, TestSig(DUMMY_ACCOUNTID, (Compact::from(DUMMY_ACCOUNTID), vec![0u8;0], Era::mortal(32, 11), 11u64).encode()), Era::mortal(32, 11)); assert!(ux.is_signed().unwrap_or(false)); - assert_eq!(>::check(ux, &TestContext), Ok(CEx { signed: Some((DUMMY_ACCOUNTID, 0)), function: vec![0u8;0], tip: None })); + assert_eq!( + >::check(ux, &TestContext), + Ok(CEx { signed: Some((DUMMY_ACCOUNTID, 0)), function: vec![0u8;0], tip: None }) + ); } #[test] diff --git a/core/sr-primitives/src/generic/unchecked_mortal_extrinsic.rs b/core/sr-primitives/src/generic/unchecked_mortal_extrinsic.rs index 69a083967117c..c3acb8a771daf 100644 --- a/core/sr-primitives/src/generic/unchecked_mortal_extrinsic.rs +++ b/core/sr-primitives/src/generic/unchecked_mortal_extrinsic.rs @@ -277,21 +277,30 @@ mod tests { fn immortal_signed_check_should_work() { let ux = Ex::new_signed(0, vec![0u8;0], DUMMY_ACCOUNTID, TestSig(DUMMY_ACCOUNTID, (DUMMY_ACCOUNTID, vec![0u8;0], Era::immortal(), 0u64).encode()), Era::immortal()); assert!(ux.is_signed().unwrap_or(false)); - assert_eq!(>::check(ux, &TestContext), Ok(CEx { signed: Some((DUMMY_ACCOUNTID, 0)), function: vec![0u8;0], tip: None })); + assert_eq!( + >::check(ux, &TestContext), + Ok(CEx { signed: Some((DUMMY_ACCOUNTID, 0)), function: vec![0u8;0], tip: None }) + ); } #[test] fn mortal_signed_check_should_work() { let ux = Ex::new_signed(0, vec![0u8;0], DUMMY_ACCOUNTID, TestSig(DUMMY_ACCOUNTID, (DUMMY_ACCOUNTID, vec![0u8;0], Era::mortal(32, 42), 42u64).encode()), Era::mortal(32, 42)); assert!(ux.is_signed().unwrap_or(false)); - assert_eq!(>::check(ux, &TestContext), Ok(CEx { signed: Some((DUMMY_ACCOUNTID, 0)), function: vec![0u8;0], tip: None })); + assert_eq!( + >::check(ux, &TestContext), + Ok(CEx { signed: Some((DUMMY_ACCOUNTID, 0)), function: vec![0u8;0], tip: None }) + ); } #[test] fn later_mortal_signed_check_should_work() { let ux = Ex::new_signed(0, vec![0u8;0], DUMMY_ACCOUNTID, TestSig(DUMMY_ACCOUNTID, (DUMMY_ACCOUNTID, vec![0u8;0], Era::mortal(32, 11), 11u64).encode()), Era::mortal(32, 11)); assert!(ux.is_signed().unwrap_or(false)); - assert_eq!(>::check(ux, &TestContext), Ok(CEx { signed: Some((DUMMY_ACCOUNTID, 0)), function: vec![0u8;0], tip: None })); + assert_eq!( + >::check(ux, &TestContext), + Ok(CEx { signed: Some((DUMMY_ACCOUNTID, 0)), function: vec![0u8;0], tip: None }) + ); } #[test] diff --git a/node/executor/src/lib.rs b/node/executor/src/lib.rs index a7d7a46f16537..0fdad30e7de01 100644 --- a/node/executor/src/lib.rs +++ b/node/executor/src/lib.rs @@ -885,6 +885,7 @@ mod tests { #[test] fn native_big_block_import_succeeds() { let mut t = new_test_ext(COMPACT_CODE, false); + Executor::new(None).call::<_, NeverNativeValue, fn() -> _>( &mut t, "Core_execute_block", From bf99e73ebaaad004c2b7bb3bc10910656166c06f Mon Sep 17 00:00:00 2001 From: kianenigma Date: Wed, 10 Jul 2019 11:54:11 +0200 Subject: [PATCH 21/23] Fix tests. --- ...checked_mortal_compact_tipped_extrinsic.rs | 22 +++++++------------ 1 file changed, 8 insertions(+), 14 deletions(-) diff --git a/core/sr-primitives/src/generic/unchecked_mortal_compact_tipped_extrinsic.rs b/core/sr-primitives/src/generic/unchecked_mortal_compact_tipped_extrinsic.rs index ef2411bab3282..e007bbb94005f 100644 --- a/core/sr-primitives/src/generic/unchecked_mortal_compact_tipped_extrinsic.rs +++ b/core/sr-primitives/src/generic/unchecked_mortal_compact_tipped_extrinsic.rs @@ -453,21 +453,15 @@ mod tests { } #[test] - #[should_panic] fn unsigned_cannot_have_tip() { - let ux = UncheckedMortalCompactTippedExtrinsic {signature: None, tip: Some(Tip::Sender(100)), function: vec![0u8;0]}; - let _ = >::check(ux, &TestContext).unwrap(); - } - - #[test] - fn unprovided_tip_will_not_fail() { - // unsigned with no tips - let bytes = vec![40, 1, 32, 8, 8, 8, 8, 8, 8, 8, 8]; - - // decoding will yield the default tip - let decoded = Ex::decode(&mut bytes.as_slice()).unwrap(); + let ux = UncheckedMortalCompactTippedExtrinsic { + signature: None, + tip: Some(Tip::Sender(100)), + function: vec![0u8;0] + }; assert_eq!( - decoded, - UncheckedMortalCompactTippedExtrinsic { signature: None, tip: None, function: vec![8u8;8]}) + >::check(ux, &TestContext).unwrap(), + Err(crate::UNSIGNED_TIP) + ); } } From 4824776778395f3152ca043ee262d64dda28e665 Mon Sep 17 00:00:00 2001 From: kianenigma Date: Wed, 10 Jul 2019 16:25:26 +0200 Subject: [PATCH 22/23] remove balance as generic. --- .../src/generic/checked_extrinsic.rs | 6 ++-- core/sr-primitives/src/generic/mod.rs | 2 +- core/sr-primitives/src/generic/tip.rs | 6 ++++ .../src/generic/unchecked_extrinsic.rs | 3 +- .../unchecked_mortal_compact_extrinsic.rs | 8 ++--- ...checked_mortal_compact_tipped_extrinsic.rs | 36 ++++++++++--------- .../src/generic/unchecked_mortal_extrinsic.rs | 5 +-- node-template/runtime/src/lib.rs | 2 +- node/executor/src/lib.rs | 2 +- node/runtime/src/lib.rs | 9 +---- srml/executive/src/lib.rs | 16 ++++----- 11 files changed, 47 insertions(+), 48 deletions(-) diff --git a/core/sr-primitives/src/generic/checked_extrinsic.rs b/core/sr-primitives/src/generic/checked_extrinsic.rs index ed5816dcced5a..b21e19f16c899 100644 --- a/core/sr-primitives/src/generic/checked_extrinsic.rs +++ b/core/sr-primitives/src/generic/checked_extrinsic.rs @@ -91,9 +91,9 @@ where // Or 2- this is built from all other types of uncheckedextrinsic which do not have tip and // hence are not tip-aware. These modules will naively place a u32 (can be `()` in practice) // as the type and it does not matter since `None` is used in this case (first arm). - match self.tip { + match &self.tip { None => None, - Some(Tip::Sender(v)) => Some(Tip::Sender(NodeBalance::from(v))), + Some(Tip::Sender(v)) => Some(Tip::Sender(NodeBalance::from(*v))), } } -} \ No newline at end of file +} diff --git a/core/sr-primitives/src/generic/mod.rs b/core/sr-primitives/src/generic/mod.rs index d61593df62186..e03be06bcd02b 100644 --- a/core/sr-primitives/src/generic/mod.rs +++ b/core/sr-primitives/src/generic/mod.rs @@ -40,7 +40,7 @@ pub use self::checked_extrinsic::CheckedExtrinsic; pub use self::header::Header; pub use self::block::{Block, SignedBlock, BlockId}; pub use self::digest::{Digest, DigestItem, DigestItemRef, OpaqueDigestItemId}; -pub use self::tip::{Tip, Tippable}; +pub use self::tip::{Tip, Tippable, NoTipBalance}; use crate::codec::Encode; use rstd::prelude::*; diff --git a/core/sr-primitives/src/generic/tip.rs b/core/sr-primitives/src/generic/tip.rs index 270f6d4e4ac79..93b608952ff9d 100644 --- a/core/sr-primitives/src/generic/tip.rs +++ b/core/sr-primitives/src/generic/tip.rs @@ -19,6 +19,10 @@ use crate::codec::{Encode, Decode}; use crate::traits::Zero; +/// A placeholder tip type that is used to fulfill the generic requirements of `checked_extrinsic` +/// when the underlying `unchecked_extrinsic` actually does not have a tip. +pub type NoTipBalance = u32; + /// Representation of a transaction tip. /// /// Provided as an enum to support potential future use cases such as: @@ -28,6 +32,8 @@ use crate::traits::Zero; #[derive(Clone, Copy, Eq, PartialEq, Encode, Decode)] pub enum Tip { /// The sender of the transaction has included some tip. + /// + /// this must be signed and included in the signature payload. Sender(Balance), } diff --git a/core/sr-primitives/src/generic/unchecked_extrinsic.rs b/core/sr-primitives/src/generic/unchecked_extrinsic.rs index a325081f238b9..6a16694aef5a1 100644 --- a/core/sr-primitives/src/generic/unchecked_extrinsic.rs +++ b/core/sr-primitives/src/generic/unchecked_extrinsic.rs @@ -22,6 +22,7 @@ use std::fmt; use rstd::prelude::*; use crate::codec::{Decode, Encode, Codec, Input, HasCompact}; use crate::traits::{self, Member, SimpleArithmetic, MaybeDisplay, Lookup, Extrinsic}; +use crate::generic::tip::NoTipBalance; use super::CheckedExtrinsic; #[derive(PartialEq, Eq, Clone, Encode, Decode)] @@ -85,7 +86,7 @@ where AccountId: Member + MaybeDisplay, Context: Lookup, { - type Checked = CheckedExtrinsic; + type Checked = CheckedExtrinsic; fn check(self, context: &Context) -> Result { Ok(match self.signature { diff --git a/core/sr-primitives/src/generic/unchecked_mortal_compact_extrinsic.rs b/core/sr-primitives/src/generic/unchecked_mortal_compact_extrinsic.rs index c374b73c91a95..a66ffafb89f59 100644 --- a/core/sr-primitives/src/generic/unchecked_mortal_compact_extrinsic.rs +++ b/core/sr-primitives/src/generic/unchecked_mortal_compact_extrinsic.rs @@ -25,6 +25,7 @@ use crate::codec::{Decode, Encode, Input, Compact}; use crate::traits::{self, Member, SimpleArithmetic, MaybeDisplay, CurrentHeight, BlockNumberToHash, Lookup, Checkable, Extrinsic, SaturatedConversion}; use super::{CheckedExtrinsic, Era}; +use crate::generic::tip::NoTipBalance; const TRANSACTION_VERSION: u8 = 1; @@ -85,7 +86,7 @@ where { /// NOTE: this transaction is not tipped i.e. the tip value will be `None`. It does not really /// matter what the last generic is since it is always `None`. - type Checked = CheckedExtrinsic; + type Checked = CheckedExtrinsic; fn check(self, context: &Context) -> Result { Ok(match self.signature { @@ -185,8 +186,7 @@ impl serde::Serialize } #[cfg(feature = "std")] -impl fmt::Debug - for UncheckedMortalCompactExtrinsic +impl fmt::Debug for UncheckedMortalCompactExtrinsic where Address: fmt::Debug, Index: fmt::Debug, @@ -237,7 +237,7 @@ mod tests { const DUMMY_ACCOUNTID: u64 = 0; type Ex = UncheckedMortalCompactExtrinsic, TestSig>; - type CEx = CheckedExtrinsic, u32>; + type CEx = CheckedExtrinsic, NoTipBalance>; #[test] fn unsigned_codec_should_work() { diff --git a/core/sr-primitives/src/generic/unchecked_mortal_compact_tipped_extrinsic.rs b/core/sr-primitives/src/generic/unchecked_mortal_compact_tipped_extrinsic.rs index e007bbb94005f..b5385deab5ca5 100644 --- a/core/sr-primitives/src/generic/unchecked_mortal_compact_tipped_extrinsic.rs +++ b/core/sr-primitives/src/generic/unchecked_mortal_compact_tipped_extrinsic.rs @@ -1,4 +1,4 @@ -// Copyright 2017-2019 Parity Technologies (UK) Ltd. +// Copyright 2019 Parity Technologies (UK) Ltd. // This file is part of Substrate. // Substrate is free software: you can redistribute it and/or modify @@ -26,14 +26,14 @@ use crate::traits::{self, Member, SimpleArithmetic, MaybeDisplay, CurrentHeight, Lookup, Checkable, Extrinsic, SaturatedConversion}; use super::{CheckedExtrinsic, Era, Tip}; -const TRANSACTION_VERSION: u8 = 2; +const TRANSACTION_VERSION: u8 = 64; /// A extrinsic right from the external world. This is unchecked and so /// can contain a signature. /// /// This type transaction: -/// - _Must_ always have a valid `Tip` encoded, and included in the signature payload if it is -/// signed. +/// - _Must_ always have a valid `Tip` (not an option!) encoded, and included in the signature +/// payload if it is signed. /// - _Must_ not provide any `Tip` if it is unsigned. #[derive(PartialEq, Eq, Clone)] pub struct UncheckedMortalCompactTippedExtrinsic { @@ -43,7 +43,8 @@ pub struct UncheckedMortalCompactTippedExtrinsic, Era)>, /// The function that should be called. pub function: Call, - /// The tip for this transaction + /// The tip for this transaction. This is always `Some` for a signed transaction and always + /// `None` for unsigned. pub tip: Option>, } @@ -57,12 +58,12 @@ impl signed: Address, signature: Signature, era: Era, - tip: Option> + tip: Tip, ) -> Self { UncheckedMortalCompactTippedExtrinsic { signature: Some((signed, signature, index.into(), era)), function, - tip, + tip: Some(tip), } } @@ -71,7 +72,7 @@ impl UncheckedMortalCompactTippedExtrinsic { signature: None, function, - tip: None + tip: None, } } } @@ -105,11 +106,12 @@ where fn check(self, context: &Context) -> Result { Ok(match self.signature { Some((signed, signature, index, era)) => { + let tip = self.tip.ok_or("signed transaction must always have a tip.")?; let current_u64 = context.current_height().saturated_into::(); let h = context.block_number_to_hash(era.birth(current_u64).saturated_into()) .ok_or("transaction birth block ancient")?; let signed = context.lookup(signed)?; - let raw_payload = (index, self.function, self.tip, era, h); + let raw_payload = (index, self.function, tip, era, h); if !raw_payload.using_encoded(|payload| { if payload.len() > 256 { signature.verify(&blake2_256(payload)[..], &signed) @@ -122,7 +124,7 @@ where CheckedExtrinsic { signed: Some((signed, (raw_payload.0).0)), function: raw_payload.1, - tip: raw_payload.2, + tip: Some(raw_payload.2), } } None => { @@ -169,7 +171,7 @@ where Some(UncheckedMortalCompactTippedExtrinsic { signature: if is_signed { Some(Decode::decode(input)?) } else { None }, function: Decode::decode(input)?, - tip: if is_signed { Decode::decode(input)? } else { None }, + tip: if is_signed { Some(Decode::decode(input)?) } else { None }, }) } } @@ -197,7 +199,7 @@ where } self.function.encode_to(v); if self.signature.is_some() { - self.tip.encode_to(v); + self.tip.as_ref().unwrap().encode_to(v); } }) } @@ -268,7 +270,7 @@ mod tests { type Balance = u64; type TipType = Tip; const DUMMY_ACCOUNTID: u64 = 0; - const TIP: Option = Some(Tip::Sender(66)); + const TIP: TipType = Tip::Sender(66); type Ex = UncheckedMortalCompactTippedExtrinsic, TestSig, Balance>; type CEx = CheckedExtrinsic, Balance>; @@ -348,7 +350,7 @@ mod tests { assert!(ux.is_signed().unwrap_or(false)); assert_eq!( >::check(ux, &TestContext), - Ok(CEx { signed: Some((DUMMY_ACCOUNTID, 0)), function: vec![0u8;0], tip: TIP }) + Ok(CEx { signed: Some((DUMMY_ACCOUNTID, 0)), function: vec![0u8;0], tip: Some(TIP) }) ); } @@ -368,7 +370,7 @@ mod tests { assert!(ux.is_signed().unwrap_or(false)); assert_eq!( >::check(ux, &TestContext), - Ok(CEx { signed: Some((DUMMY_ACCOUNTID, 0)), function: vec![0u8;0], tip: TIP })); + Ok(CEx { signed: Some((DUMMY_ACCOUNTID, 0)), function: vec![0u8;0], tip: Some(TIP) })); } #[test] @@ -387,7 +389,7 @@ mod tests { assert!(ux.is_signed().unwrap_or(false)); assert_eq!( >::check(ux, &TestContext), - Ok(CEx { signed: Some((DUMMY_ACCOUNTID, 0)), function: vec![0u8;0], tip: TIP }) + Ok(CEx { signed: Some((DUMMY_ACCOUNTID, 0)), function: vec![0u8;0], tip: Some(TIP) }) ); } @@ -460,7 +462,7 @@ mod tests { function: vec![0u8;0] }; assert_eq!( - >::check(ux, &TestContext).unwrap(), + >::check(ux, &TestContext), Err(crate::UNSIGNED_TIP) ); } diff --git a/core/sr-primitives/src/generic/unchecked_mortal_extrinsic.rs b/core/sr-primitives/src/generic/unchecked_mortal_extrinsic.rs index c3acb8a771daf..185abf4b9e925 100644 --- a/core/sr-primitives/src/generic/unchecked_mortal_extrinsic.rs +++ b/core/sr-primitives/src/generic/unchecked_mortal_extrinsic.rs @@ -26,6 +26,7 @@ use crate::traits::{ self, Member, SimpleArithmetic, MaybeDisplay, CurrentHeight, BlockNumberToHash, Lookup, Checkable, Extrinsic, SaturatedConversion }; +use crate::generic::tip::NoTipBalance; use super::{CheckedExtrinsic, Era}; const TRANSACTION_VERSION: u8 = 1; @@ -84,7 +85,7 @@ where { /// NOTE: this transaction is not tipped i.e. the tip value will be `None`. It does not really /// matter what the last generic is since it is always `None`. - type Checked = CheckedExtrinsic; + type Checked = CheckedExtrinsic; fn check(self, context: &Context) -> Result { Ok(match self.signature { @@ -236,7 +237,7 @@ mod tests { const DUMMY_ACCOUNTID: u64 = 0; type Ex = UncheckedMortalExtrinsic, TestSig>; - type CEx = CheckedExtrinsic, u32>; + type CEx = CheckedExtrinsic, NoTipBalance>; #[test] fn unsigned_codec_should_work() { diff --git a/node-template/runtime/src/lib.rs b/node-template/runtime/src/lib.rs index aff9992fcede6..6292c1c991e43 100644 --- a/node-template/runtime/src/lib.rs +++ b/node-template/runtime/src/lib.rs @@ -235,7 +235,7 @@ pub type UncheckedExtrinsic = generic::UncheckedMortalCompactExtrinsic; /// Executive: handles dispatch to the various modules. -pub type Executive = executive::Executive; +pub type Executive = executive::Executive; // Implement our runtime API endpoints. This is just a bunch of proxying. impl_runtime_apis! { diff --git a/node/executor/src/lib.rs b/node/executor/src/lib.rs index 228a3da1de00d..bd70fd9fc3248 100644 --- a/node/executor/src/lib.rs +++ b/node/executor/src/lib.rs @@ -48,7 +48,7 @@ mod tests { }; use node_primitives::{Hash, BlockNumber, AccountId}; use runtime_primitives::traits::{Header as HeaderT, Hash as HashT}; - use runtime_primitives::{generic::{Era, Tip}, ApplyOutcome, ApplyError, ApplyResult, Perbill}; + use runtime_primitives::{generic::Era, ApplyOutcome, ApplyError, ApplyResult, Perbill}; use {balances, contracts, indices, staking, system, timestamp}; use contracts::ContractAddressFor; use system::{EventRecord, Phase}; diff --git a/node/runtime/src/lib.rs b/node/runtime/src/lib.rs index 2d1cc91908c58..cc25f832510fa 100644 --- a/node/runtime/src/lib.rs +++ b/node/runtime/src/lib.rs @@ -432,14 +432,7 @@ pub type UncheckedExtrinsic = generic::UncheckedMortalCompactExtrinsic; /// Executive: handles dispatch to the various modules. -pub type Executive = executive::Executive< - Runtime, Block, - system::ChainContext, - Balances, - Balance, - Runtime, - AllModules, ->; +pub type Executive = executive::Executive, Balances, Runtime, AllModules>; impl_runtime_apis! { impl client_api::Core for Runtime { diff --git a/srml/executive/src/lib.rs b/srml/executive/src/lib.rs index a9baac81ba869..8933088d13d35 100644 --- a/srml/executive/src/lib.rs +++ b/srml/executive/src/lib.rs @@ -69,7 +69,7 @@ //! # } //! # } //! /// Executive: handles dispatch to the various modules. -//! pub type Executive = executive::Executive; +//! pub type Executive = executive::Executive; //! ``` #![cfg_attr(not(feature = "std"), no_std)] @@ -81,7 +81,6 @@ use primitives::{ApplyOutcome, ApplyError, transaction_validity::{TransactionValidity, TransactionPriority, TransactionLongevity}, traits::{self, Header, Zero, One, Checkable, Applyable, CheckEqual, OnFinalize, OnInitialize, NumberFor, Block as BlockT, OffchainWorker, ValidateUnsigned, - SimpleArithmetic, }, }; use srml_support::{Dispatchable, traits::MakePayment}; @@ -116,8 +115,8 @@ pub type CallOf = as Applyable>::Call; pub type OriginOf = as Dispatchable>::Origin; pub type BalanceOf =

>::Balance; -pub struct Executive( - PhantomData<(System, Block, Context, Payment, Balance, UnsignedValidator, AllModules)> +pub struct Executive( + PhantomData<(System, Block, Context, Payment, UnsignedValidator, AllModules)> ); impl< @@ -125,10 +124,9 @@ impl< Block: traits::Block, Context: Default, Payment: MakePayment, - Balance, UnsignedValidator, AllModules: OnInitialize + OnFinalize + OffchainWorker, -> ExecuteBlock for Executive +> ExecuteBlock for Executive where Block::Extrinsic: Checkable + Codec, CheckedOf: @@ -140,7 +138,7 @@ where UnsignedValidator: ValidateUnsigned>, { fn execute_block(block: Block) { - Executive::::execute_block(block); + Executive::::execute_block(block); } } @@ -149,10 +147,9 @@ impl< Block: traits::Block, Context: Default, Payment: MakePayment, - Balance: SimpleArithmetic + Copy, UnsignedValidator, AllModules: OnInitialize + OnFinalize + OffchainWorker, -> Executive +> Executive where Block::Extrinsic: Checkable + Codec, CheckedOf: @@ -499,7 +496,6 @@ mod tests { Block, system::ChainContext, balances::Module, - u64, Runtime, () >; From fb42e4e92ed0f7916699d3713b74504acb9fbdc0 Mon Sep 17 00:00:00 2001 From: kianenigma Date: Wed, 10 Jul 2019 16:33:21 +0200 Subject: [PATCH 23/23] Update doc. --- core/sr-primitives/src/generic/checked_extrinsic.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/core/sr-primitives/src/generic/checked_extrinsic.rs b/core/sr-primitives/src/generic/checked_extrinsic.rs index b21e19f16c899..04542a3084a9d 100644 --- a/core/sr-primitives/src/generic/checked_extrinsic.rs +++ b/core/sr-primitives/src/generic/checked_extrinsic.rs @@ -33,6 +33,10 @@ pub struct CheckedExtrinsic { /// The function that should be called. pub function: Call, /// An optional tip value that may or may not exist based on the underlying unchecked extrinsic. + /// + /// Most often this is: + /// - `None` if the unchecked extrinsic does not have a tip OR it is unsigned. + /// - `Some` if the opposite. pub tip: Option>, }