Skip to content
This repository was archived by the owner on Nov 15, 2023. It is now read-only.
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion core/sr-primitives/src/generic/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ mod digest;
#[cfg(test)]
mod tests;

pub use self::unchecked_extrinsic::UncheckedExtrinsic;
pub use self::unchecked_extrinsic::{UncheckedExtrinsic, SignedPayload};
pub use self::era::{Era, Phase};
pub use self::checked_extrinsic::CheckedExtrinsic;
pub use self::header::Header;
Expand Down
90 changes: 79 additions & 11 deletions core/sr-primitives/src/generic/unchecked_extrinsic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,12 +72,22 @@ impl<Address, Call, Signature, Extra: SignedExtension> Extrinsic
{
type Call = Call;

type SignaturePayload = (
Address,
Signature,
Extra,
);

fn is_signed(&self) -> Option<bool> {
Some(self.signature.is_some())
}

fn new_unsigned(function: Call) -> Option<Self> {
Some(UncheckedExtrinsic::new_unsigned(function))
fn new(function: Call, signed_data: Option<Self::SignaturePayload>) -> Option<Self> {
Some(if let Some((address, signature, extra)) = signed_data {
UncheckedExtrinsic::new_signed(function, address, signature, extra)
} else {
UncheckedExtrinsic::new_unsigned(function)
})
}
}

Expand All @@ -98,21 +108,18 @@ where
fn check(self, lookup: &Lookup) -> Result<Self::Checked, &'static str> {
Ok(match self.signature {
Some((signed, signature, extra)) => {
let additional_signed = extra.additional_signed()?;
let raw_payload = (self.function, extra, additional_signed);
let signed = lookup.lookup(signed)?;
let raw_payload = SignedPayload::new(self.function, extra)?;
if !raw_payload.using_encoded(|payload| {
if payload.len() > 256 {
signature.verify(&blake2_256(payload)[..], &signed)
} else {
signature.verify(payload, &signed)
}
signature.verify(payload, &signed)
}) {
return Err(crate::BAD_SIGNATURE)
}

let (function, extra, _) = raw_payload.deconstruct();
CheckedExtrinsic {
signed: Some((signed, raw_payload.1)),
function: raw_payload.0,
signed: Some((signed, extra)),
function,
}
}
None => CheckedExtrinsic {
Expand All @@ -123,6 +130,67 @@ where
}
}

/// A payload that has been signed for a unchecked extrinsics.
///
/// Note that the payload that we sign to produce unchecked extrinsic signature
/// is going to be different than the `SignaturePayload` - so the thing the extrinsic
/// actually contains.
pub struct SignedPayload<Call, Extra: SignedExtension> {
raw_payload: (
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I could not find a reason but I just wonder if you also argued/tought of making this a flat struct rather than one with a single element of a tuple? or is there a strong reason?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, makes sense. I guess, arguably self.raw_payload.using_encoded is more readable than self.0, but indeed it feels simpler to avoid naming it.

Call,
Extra,
Extra::AdditionalSigned,
),
}

impl<Call, Extra> SignedPayload<Call, Extra> where
Call: Encode,
Extra: SignedExtension,
{
/// Create new `SignedPayload`.
///
/// This function may fail if `additional_signed` of `Extra` is not available.
pub fn new(call: Call, extra: Extra) -> Result<Self, &'static str> {
let additional_signed = extra.additional_signed()?;
let raw_payload = (call, extra, additional_signed);
Ok(Self { raw_payload })
}

/// Create new `SignedPayload` from raw components.
pub fn from_raw(call: Call, extra: Extra, additional_signed: Extra::AdditionalSigned) -> Self {
Self {
raw_payload: (call, extra, additional_signed),
}
}

/// Get an encoded version of this payload.
///
/// Payloads longer than 256 bytes are going to be `blake2_256`-hashed.
pub fn using_encoded<O>(&self, f: impl FnOnce(&[u8]) -> O) -> O {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we should give this a different name?

self.raw_payload.using_encoded(|payload| {
if payload.len() > 256 {
f(&blake2_256(payload)[..])
} else {
f(payload)
}
})
}

/// Deconstruct the payload into it's components.
pub fn deconstruct(self) -> (Call, Extra, Extra::AdditionalSigned) {
self.raw_payload
}
}

impl<Call, Extra> Encode for SignedPayload<Call, Extra> where
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this implementation required?

Copy link
Contributor Author

@tomusdrw tomusdrw Aug 30, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The generic Signer works on anything that is Encode. I'll rather remove the using_encoded function, I've only added it to be able to document the behavior.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can also put documentation on top of a trait implementation. It will be shown in rustdocs as welll.

Call: Encode,
Extra: SignedExtension,
{
fn using_encoded<R, F: FnOnce(&[u8]) -> R>(&self, f: F) -> R {
self.using_encoded(f)
}
}

impl<Address, Call, Signature, Extra> Decode
for UncheckedExtrinsic<Address, Call, Signature, Extra>
where
Expand Down
7 changes: 1 addition & 6 deletions core/sr-primitives/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -847,12 +847,7 @@ impl<'a> ::serde::Deserialize<'a> for OpaqueExtrinsic {

impl traits::Extrinsic for OpaqueExtrinsic {
type Call = ();

fn is_signed(&self) -> Option<bool> {
None
}

fn new_unsigned(_call: Self::Call) -> Option<Self> { None }
type SignaturePayload = ();
}

#[cfg(test)]
Expand Down
6 changes: 4 additions & 2 deletions core/sr-primitives/src/testing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,7 @@ pub struct ExtrinsicWrapper<Xt>(Xt);

impl<Xt> traits::Extrinsic for ExtrinsicWrapper<Xt> {
type Call = ();
type SignaturePayload = ();

fn is_signed(&self) -> Option<bool> {
None
Expand Down Expand Up @@ -274,13 +275,14 @@ impl<Call: Codec + Sync + Send, Context, Extra> Checkable<Context> for TestXt<Ca
}
impl<Call: Codec + Sync + Send, Extra> traits::Extrinsic for TestXt<Call, Extra> {
type Call = Call;
type SignaturePayload = (u64, Extra);

fn is_signed(&self) -> Option<bool> {
Some(self.0.is_some())
}

fn new_unsigned(_c: Call) -> Option<Self> {
None
fn new(c: Call, sig: Option<Self::SignaturePayload>) -> Option<Self> {
Some(TestXt(sig, c))
}
}

Expand Down
17 changes: 14 additions & 3 deletions core/sr-primitives/src/traits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -743,13 +743,24 @@ pub trait Extrinsic: Sized {
/// The function call.
type Call;

/// The payload we carry for signed extrinsics.
///
/// Usually it will contain a `Signature` and
/// may include some additional data that are specific to signed
/// extrinsics.
type SignaturePayload;

/// Is this `Extrinsic` signed?
/// If no information are available about signed/unsigned, `None` should be returned.
fn is_signed(&self) -> Option<bool> { None }

/// New instance of an unsigned extrinsic aka "inherent". `None` if this is an opaque
/// extrinsic type.
fn new_unsigned(_call: Self::Call) -> Option<Self> { None }
/// Create new instance of the extrinsic.
///
/// Extrinsics can be split into:
/// 1. Inherents (no signature; created by validators during block production)
/// 2. Unsigned Transactions (no signature; represent "system calls" or other special kinds of calls)
/// 3. Signed Transactions (with signature; a regular transactions with known origin)
fn new(_call: Self::Call, _signed_data: Option<Self::SignaturePayload>) -> Option<Self> { None }
}

/// Extract the hashing type for a block.
Expand Down
3 changes: 2 additions & 1 deletion core/test-runtime/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,7 @@ impl BlindCheckable for Extrinsic {

impl ExtrinsicT for Extrinsic {
type Call = Extrinsic;
type SignaturePayload = ();

fn is_signed(&self) -> Option<bool> {
if let Extrinsic::IncludeData(_) = *self {
Expand All @@ -150,7 +151,7 @@ impl ExtrinsicT for Extrinsic {
}
}

fn new_unsigned(call: Self::Call) -> Option<Self> {
fn new(call: Self::Call, _signature_payload: Option<Self::SignaturePayload>) -> Option<Self> {
Some(call)
}
}
Expand Down
14 changes: 8 additions & 6 deletions node/cli/src/service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -462,15 +462,17 @@ mod tests {
let check_weight = system::CheckWeight::new();
let take_fees = balances::TakeFees::from(0);
let extra = (check_version, check_genesis, check_era, check_nonce, check_weight, take_fees);

let raw_payload = (function, extra.clone(), version, genesis_hash, genesis_hash);
let signature = raw_payload.using_encoded(|payload| if payload.len() > 256 {
signer.sign(&blake2_256(payload)[..])
} else {
let raw_payload = SignedPayload::from_raw(
function,
extra,
(version, genesis_hash, genesis_hash, (), (), ())
);
let signature = raw_payload.using_encoded(|payload| {
signer.sign(payload)
});
let (function, extra, _) = raw_payload.deconstruct();
let xt = UncheckedExtrinsic::new_signed(
raw_payload.0,
function,
from.into(),
signature.into(),
extra,
Expand Down
50 changes: 47 additions & 3 deletions node/runtime/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ use sr_primitives::{ApplyResult, impl_opaque_keys, generic, create_runtime_str,
use sr_primitives::transaction_validity::TransactionValidity;
use sr_primitives::weights::Weight;
use sr_primitives::traits::{
BlakeTwo256, Block as BlockT, DigestFor, NumberFor, StaticLookup,
self, BlakeTwo256, Block as BlockT, DigestFor, NumberFor, StaticLookup, SaturatedConversion,
};
use version::RuntimeVersion;
use elections::VoteIndex;
Expand All @@ -48,6 +48,7 @@ use version::NativeVersion;
use primitives::OpaqueMetadata;
use grandpa::{AuthorityId as GrandpaId, AuthorityWeight as GrandpaWeight};
use im_online::sr25519::{AuthorityId as ImOnlineId};
use system::offchain::TransactionSubmitter;

#[cfg(any(feature = "std", test))]
pub use sr_primitives::BuildStorage;
Expand Down Expand Up @@ -80,7 +81,7 @@ pub const VERSION: RuntimeVersion = RuntimeVersion {
// implementation changes and behavior does not, then leave spec_version as
// is and increment impl_version.
spec_version: 154,
impl_version: 157,
impl_version: 158,
apis: RUNTIME_API_VERSIONS,
};

Expand Down Expand Up @@ -396,7 +397,7 @@ impl im_online::Trait for Runtime {
type AuthorityId = ImOnlineId;
type Call = Call;
type Event = Event;
type UncheckedExtrinsic = UncheckedExtrinsic;
type SubmitTransaction = TransactionSubmitter<ImOnlineId, Runtime, UncheckedExtrinsic>;
type ReportUnresponsiveness = Offences;
type CurrentElectedSet = staking::CurrentElectedStashAccounts<Runtime>;
}
Expand Down Expand Up @@ -424,6 +425,33 @@ impl finality_tracker::Trait for Runtime {
type ReportLatency = ReportLatency;
}

impl system::offchain::CreateTransaction<Runtime, UncheckedExtrinsic> for Runtime {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

where is this being used?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We pass this as SubmitTransaction to im-online:

type SubmitTransaction = TransactionSubmitter<ImOnlineId, Runtime, UncheckedExtrinsic>;

(see Runtime as the second parameter)

type Signature = Signature;

fn create_transaction<F: system::offchain::Signer<AccountId, Self::Signature>>(
call: Call,
account: AccountId,
index: Index,
) -> Option<(Call, <UncheckedExtrinsic as traits::Extrinsic>::SignaturePayload)> {
let period = 1 << 8;
let current_block = System::block_number().saturated_into::<u64>();
let tip = 0;
let extra: SignedExtra = (
system::CheckVersion::<Runtime>::new(),
system::CheckGenesis::<Runtime>::new(),
system::CheckEra::<Runtime>::from(generic::Era::mortal(period, current_block)),
system::CheckNonce::<Runtime>::from(index),
system::CheckWeight::<Runtime>::new(),
balances::TakeFees::<Runtime>::from(tip),
);
let raw_payload = SignedPayload::new(call, extra).ok()?;
let signature = F::sign(account.clone(), &raw_payload)?;
let address = Indices::unlookup(account);
let (call, extra, _) = raw_payload.deconstruct();
Some((call, (address, signature, extra)))
}
}

construct_runtime!(
pub enum Runtime where
Block = Block,
Expand Down Expand Up @@ -475,6 +503,8 @@ pub type SignedExtra = (
);
/// Unchecked extrinsic type as expected by this runtime.
pub type UncheckedExtrinsic = generic::UncheckedExtrinsic<Address, Call, Signature, SignedExtra>;
/// The payload being signed in transactions.
pub type SignedPayload = generic::SignedPayload<Call, SignedExtra>;
/// Extrinsic type that has already been checked.
pub type CheckedExtrinsic = generic::CheckedExtrinsic<AccountId, Call, SignedExtra>;
/// Executive: handles dispatch to the various modules.
Expand Down Expand Up @@ -609,3 +639,17 @@ impl_runtime_apis! {
}
}
}

#[cfg(test)]
mod tests {
use super::*;
use system::offchain::SubmitSignedTransaction;

fn is_submit_signed_transaction(_arg: impl SubmitSignedTransaction<Runtime, Call>) {}

#[test]
fn validate_bounds() {
let x = SubmitTransaction::default();
is_submit_signed_transaction(x);
}
}
1 change: 0 additions & 1 deletion srml/balances/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1187,7 +1187,6 @@ pub struct TakeFees<T: Trait<I>, I: Instance = DefaultInstance>(#[codec(compact)

impl<T: Trait<I>, I: Instance> TakeFees<T, I> {
/// utility constructor. Used only in client/factory code.
#[cfg(feature = "std")]
pub fn from(fee: T::Balance) -> Self {
Self(fee)
}
Expand Down
15 changes: 6 additions & 9 deletions srml/im-online/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ use session::historical::IdentificationTuple;
use sr_io::Printable;
use sr_primitives::{
Perbill, ApplyError,
traits::{Convert, Extrinsic as ExtrinsicT, Member},
traits::{Convert, Member},
transaction_validity::{TransactionValidity, TransactionLongevity, ValidTransaction},
};
use sr_staking_primitives::{
Expand All @@ -87,6 +87,7 @@ use srml_support::{
Parameter, StorageValue, StorageDoubleMap,
};
use system::ensure_none;
use system::offchain::SubmitUnsignedTransaction;

pub mod sr25519 {
mod app_sr25519 {
Expand Down Expand Up @@ -141,7 +142,6 @@ struct WorkerStatus<BlockNumber> {
// Error which may occur while executing the off-chain code.
enum OffchainErr {
DecodeWorkerStatus,
ExtrinsicCreation,
FailedSigning,
NetworkState,
SubmitTransaction,
Expand All @@ -151,7 +151,6 @@ impl Printable for OffchainErr {
fn print(self) {
match self {
OffchainErr::DecodeWorkerStatus => print("Offchain error: decoding WorkerStatus failed!"),
OffchainErr::ExtrinsicCreation => print("Offchain error: extrinsic creation failed!"),
OffchainErr::FailedSigning => print("Offchain error: signing failed!"),
OffchainErr::NetworkState => print("Offchain error: fetching network state failed!"),
OffchainErr::SubmitTransaction => print("Offchain error: submitting transaction failed!"),
Expand Down Expand Up @@ -183,9 +182,8 @@ pub trait Trait: system::Trait + session::historical::Trait {
/// A dispatchable call type.
type Call: From<Call<Self>>;

/// A extrinsic right from the external world. This is unchecked and so
/// can contain a signature.
type UncheckedExtrinsic: ExtrinsicT<Call=<Self as Trait>::Call> + Encode + Decode;
/// A transaction submitter.
type SubmitTransaction: SubmitUnsignedTransaction<Self, <Self as Trait>::Call>;

/// A type that gives us the ability to submit unresponsiveness offence reports.
type ReportUnresponsiveness:
Expand Down Expand Up @@ -340,9 +338,8 @@ impl<T: Trait> Module<T> {

let signature = key.sign(&heartbeat_data.encode()).ok_or(OffchainErr::FailedSigning)?;
let call = Call::heartbeat(heartbeat_data, signature);
let ex = T::UncheckedExtrinsic::new_unsigned(call.into())
.ok_or(OffchainErr::ExtrinsicCreation)?;
sr_io::submit_transaction(&ex).map_err(|_| OffchainErr::SubmitTransaction)?;
T::SubmitTransaction::submit_unsigned(call)
.map_err(|_| OffchainErr::SubmitTransaction)?;

// once finished we set the worker status without comparing
// if the existing value changed in the meantime. this is
Expand Down
Loading