From b5bdbeb5182fa054d92258531c9d59554660c3a7 Mon Sep 17 00:00:00 2001 From: Svyatoslav Nikolsky Date: Thu, 2 Jul 2020 23:41:53 +0300 Subject: [PATCH] Sign PoA transactions from wasm env (#155) * sign PoA transactions from wasm env * cargo fmt --all --- bridges/bin/node/runtime/Cargo.toml | 9 +- bridges/bin/node/runtime/src/exchange.rs | 43 +++--- bridges/primitives/ethereum-poa/src/lib.rs | 143 ++++++++++++------ .../primitives/ethereum-poa/src/signatures.rs | 81 +++++++++- 4 files changed, 208 insertions(+), 68 deletions(-) diff --git a/bridges/bin/node/runtime/Cargo.toml b/bridges/bin/node/runtime/Cargo.toml index ddee4842a929..3e473687334e 100644 --- a/bridges/bin/node/runtime/Cargo.toml +++ b/bridges/bin/node/runtime/Cargo.toml @@ -9,9 +9,6 @@ repository = "https://github.com/paritytech/parity-bridges-common/" [dependencies] hex-literal = "0.2" -[dev-dependencies] -ethereum-tx-sign = "3.0" - [dependencies.codec] package = "parity-scale-codec" version = "1.0.0" @@ -202,6 +199,12 @@ default-features = false rev = "606c56d2e2f69f68f3947551224be6a3515dff60" git = "https://github.com/paritytech/substrate/" +[dev-dependencies.sp-bridge-eth-poa] +version = "0.1.0" +default-features = false +features = ["test-helpers"] +path = "../../../primitives/ethereum-poa" + [build-dependencies.wasm-builder-runner] version = "1.0.5" package = "substrate-wasm-builder-runner" diff --git a/bridges/bin/node/runtime/src/exchange.rs b/bridges/bin/node/runtime/src/exchange.rs index e269882a50d4..8a627e8a2c86 100644 --- a/bridges/bin/node/runtime/src/exchange.rs +++ b/bridges/bin/node/runtime/src/exchange.rs @@ -31,7 +31,7 @@ use codec::{Decode, Encode}; use frame_support::RuntimeDebug; use hex_literal::hex; use pallet_bridge_currency_exchange::Blockchain; -use sp_bridge_eth_poa::transaction_decode; +use sp_bridge_eth_poa::{transaction_decode, RawTransaction}; use sp_currency_exchange::{ Error as ExchangeError, LockFundsTransaction, MaybeLockFundsTransaction, Result as ExchangeResult, }; @@ -48,7 +48,7 @@ pub struct EthereumTransactionInclusionProof { /// Index of the transaction within the block. pub index: u64, /// The proof itself (right now it is all RLP-encoded transactions of the block). - pub proof: Vec>, + pub proof: Vec, } /// We uniquely identify transfer by the pair (sender, nonce). @@ -69,7 +69,7 @@ pub struct EthereumTransactionTag { pub struct EthBlockchain; impl Blockchain for EthBlockchain { - type Transaction = Vec; + type Transaction = RawTransaction; type TransactionInclusionProof = EthereumTransactionInclusionProof; fn verify_transaction_inclusion_proof(proof: &Self::TransactionInclusionProof) -> Option { @@ -88,7 +88,7 @@ impl Blockchain for EthBlockchain { pub struct EthTransaction; impl MaybeLockFundsTransaction for EthTransaction { - type Transaction = Vec; + type Transaction = RawTransaction; type Id = EthereumTransactionTag; type Recipient = crate::AccountId; type Amount = crate::Balance; @@ -99,19 +99,19 @@ impl MaybeLockFundsTransaction for EthTransaction { let tx = transaction_decode(raw_tx).map_err(|_| ExchangeError::InvalidTransaction)?; // we only accept transactions sending funds directly to the pre-configured address - if tx.to != Some(LOCK_FUNDS_ADDRESS.into()) { + if tx.unsigned.to != Some(LOCK_FUNDS_ADDRESS.into()) { frame_support::debug::error!( target: "runtime", "Failed to parse fund locks transaction. Invalid peer recipient: {:?}", - tx.to, + tx.unsigned.to, ); return Err(ExchangeError::InvalidTransaction); } let mut recipient_raw = sp_core::H256::default(); - match tx.payload.len() { - 32 => recipient_raw.as_fixed_bytes_mut().copy_from_slice(&tx.payload), + match tx.unsigned.payload.len() { + 32 => recipient_raw.as_fixed_bytes_mut().copy_from_slice(&tx.unsigned.payload), len => { frame_support::debug::error!( target: "runtime", @@ -122,13 +122,13 @@ impl MaybeLockFundsTransaction for EthTransaction { return Err(ExchangeError::InvalidRecipient); } } - let amount = tx.value.low_u128(); + let amount = tx.unsigned.value.low_u128(); - if tx.value != amount.into() { + if tx.unsigned.value != amount.into() { frame_support::debug::error!( target: "runtime", "Failed to parse fund locks transaction. Invalid amount: {}", - tx.value, + tx.unsigned.value, ); return Err(ExchangeError::InvalidAmount); @@ -137,7 +137,7 @@ impl MaybeLockFundsTransaction for EthTransaction { Ok(LockFundsTransaction { id: EthereumTransactionTag { account: *tx.sender.as_fixed_bytes(), - nonce: tx.nonce, + nonce: tx.unsigned.nonce, }, recipient: crate::AccountId::from(*recipient_raw.as_fixed_bytes()), amount, @@ -149,29 +149,36 @@ impl MaybeLockFundsTransaction for EthTransaction { mod tests { use super::*; use hex_literal::hex; + use sp_bridge_eth_poa::{ + signatures::{SecretKey, SignTransaction}, + UnsignedTransaction, + }; fn ferdie() -> crate::AccountId { hex!("1cbd2d43530a44705ad088af313e18f80b53ef16b36177cd4b77b846f2a5f07c").into() } - fn prepare_ethereum_transaction(editor: impl Fn(&mut ethereum_tx_sign::RawTransaction)) -> Vec { + fn prepare_ethereum_transaction(editor: impl Fn(&mut UnsignedTransaction)) -> Vec { // prepare tx for OpenEthereum private dev chain: // chain id is 0x11 // sender secret is 0x4d5db4107d237df6a3d58ee5f70ae63d73d7658d4026f2eefd2f204c81682cb7 let chain_id = 0x11_u64; - let signer = hex!("4d5db4107d237df6a3d58ee5f70ae63d73d7658d4026f2eefd2f204c81682cb7"); + let signer = SecretKey::parse(&hex!( + "4d5db4107d237df6a3d58ee5f70ae63d73d7658d4026f2eefd2f204c81682cb7" + )) + .unwrap(); let ferdie_id = ferdie(); let ferdie_raw: &[u8; 32] = ferdie_id.as_ref(); - let mut eth_tx = ethereum_tx_sign::RawTransaction { + let mut eth_tx = UnsignedTransaction { nonce: 0.into(), to: Some(LOCK_FUNDS_ADDRESS.into()), value: 100.into(), gas: 100_000.into(), gas_price: 100_000.into(), - data: ferdie_raw.to_vec(), + payload: ferdie_raw.to_vec(), }; editor(&mut eth_tx); - eth_tx.sign(&signer.into(), &chain_id) + eth_tx.sign_by(&signer, Some(chain_id)) } #[test] @@ -211,7 +218,7 @@ mod tests { fn transaction_with_invalid_recipient_rejected() { assert_eq!( EthTransaction::parse(&prepare_ethereum_transaction(|tx| { - tx.data.clear(); + tx.payload.clear(); })), Err(ExchangeError::InvalidRecipient), ); diff --git a/bridges/primitives/ethereum-poa/src/lib.rs b/bridges/primitives/ethereum-poa/src/lib.rs index ca804b32bda9..a4fac244bf79 100644 --- a/bridges/primitives/ethereum-poa/src/lib.rs +++ b/bridges/primitives/ethereum-poa/src/lib.rs @@ -97,12 +97,24 @@ pub struct Header { } /// Parsed ethereum transaction. -#[derive(Debug, PartialEq)] +#[derive(PartialEq, RuntimeDebug)] pub struct Transaction { /// Sender address. pub sender: Address, + /// Unsigned portion of ethereum transaction. + pub unsigned: UnsignedTransaction, +} + +/// Unsigned portion of ethereum transaction. +#[derive(PartialEq, RuntimeDebug)] +#[cfg_attr(test, derive(Clone))] +pub struct UnsignedTransaction { /// Sender nonce. pub nonce: U256, + /// Gas price. + pub gas_price: U256, + /// Gas limit. + pub gas: U256, /// Transaction destination address. None if it is contract creation transaction. pub to: Option
, /// Transaction value. @@ -259,6 +271,55 @@ impl Header { } } +impl UnsignedTransaction { + /// Decode unsigned portion of raw transaction RLP. + pub fn decode(raw_tx: &[u8]) -> Result { + let tx_rlp = Rlp::new(raw_tx); + let to = tx_rlp.at(3)?; + Ok(UnsignedTransaction { + nonce: tx_rlp.val_at(0)?, + gas_price: tx_rlp.val_at(1)?, + gas: tx_rlp.val_at(2)?, + to: match to.is_empty() { + false => Some(to.as_val()?), + true => None, + }, + value: tx_rlp.val_at(4)?, + payload: tx_rlp.val_at(5)?, + }) + } + + /// Returns message that has to be signed to sign this transaction. + pub fn message(&self, chain_id: Option) -> H256 { + keccak_256(&self.rlp(chain_id)).into() + } + + /// Returns unsigned transaction RLP. + pub fn rlp(&self, chain_id: Option) -> Bytes { + let mut stream = RlpStream::new_list(if chain_id.is_some() { 9 } else { 6 }); + self.rlp_to(chain_id, &mut stream); + stream.out() + } + + /// Encode to given rlp stream. + pub fn rlp_to(&self, chain_id: Option, stream: &mut RlpStream) { + stream.append(&self.nonce); + stream.append(&self.gas_price); + stream.append(&self.gas); + match self.to { + Some(to) => stream.append(&to), + None => stream.append(&""), + }; + stream.append(&self.value); + stream.append(&self.payload); + if let Some(chain_id) = chain_id { + stream.append(&chain_id); + stream.append(&0u8); + stream.append(&0u8); + } + } +} + impl Receipt { /// Returns receipt RLP. fn rlp(&self) -> Bytes { @@ -373,17 +434,8 @@ impl std::fmt::Debug for Bloom { /// Decode Ethereum transaction. pub fn transaction_decode(raw_tx: &[u8]) -> Result { // parse transaction fields + let unsigned = UnsignedTransaction::decode(raw_tx)?; let tx_rlp = Rlp::new(raw_tx); - let nonce: U256 = tx_rlp.val_at(0)?; - let gas_price = tx_rlp.at(1)?; - let gas = tx_rlp.at(2)?; - let action = tx_rlp.at(3)?; - let to = match action.is_empty() { - false => Some(action.as_val()?), - true => None, - }; - let value: U256 = tx_rlp.val_at(4)?; - let payload: Bytes = tx_rlp.val_at(5)?; let v: u64 = tx_rlp.val_at(6)?; let r: U256 = tx_rlp.val_at(7)?; let s: U256 = tx_rlp.val_at(8)?; @@ -401,31 +453,16 @@ pub fn transaction_decode(raw_tx: &[u8]) -> Result Header; } +/// Utilities for signing transactions. +pub trait SignTransaction { + /// Sign transaction by given author. + fn sign_by(self, author: &SecretKey, chain_id: Option) -> RawTransaction; +} + impl SignHeader for Header { fn sign_by(mut self, author: &SecretKey) -> Self { self.author = secret_to_address(author); @@ -48,6 +60,24 @@ impl SignHeader for Header { } } +impl SignTransaction for UnsignedTransaction { + fn sign_by(self, author: &SecretKey, chain_id: Option) -> RawTransaction { + let message = self.message(chain_id); + let signature = sign(author, message); + let signature_r = U256::from_big_endian(&signature.as_fixed_bytes()[..32][..]); + let signature_s = U256::from_big_endian(&signature.as_fixed_bytes()[32..64][..]); + let signature_v = signature.as_fixed_bytes()[64] as u64; + let signature_v = signature_v + if let Some(n) = chain_id { 35 + n * 2 } else { 27 }; + + let mut stream = rlp::RlpStream::new_list(9); + self.rlp_to(None, &mut stream); + stream.append(&signature_v); + stream.append(&signature_r); + stream.append(&signature_s); + stream.out() + } +} + /// Return author's signature over given message. pub fn sign(author: &SecretKey, message: H256) -> H520 { let (signature, recovery_id) = secp256k1::sign(&Message::parse(message.as_fixed_bytes()), author); @@ -64,3 +94,50 @@ pub fn secret_to_address(secret: &SecretKey) -> Address { raw_public.copy_from_slice(&public.serialize()[1..]); public_to_address(&raw_public) } + +#[cfg(test)] +mod tests { + use super::*; + use crate::{transaction_decode, Transaction}; + + #[test] + fn transaction_signed_properly() { + // case1: with chain_id replay protection + to + let signer = SecretKey::parse(&[1u8; 32]).unwrap(); + let signer_address = secret_to_address(&signer); + let unsigned = UnsignedTransaction { + nonce: 100.into(), + gas_price: 200.into(), + gas: 300.into(), + to: Some([42u8; 20].into()), + value: 400.into(), + payload: vec![1, 2, 3], + }; + let raw_tx = unsigned.clone().sign_by(&signer, Some(42)); + assert_eq!( + transaction_decode(&raw_tx), + Ok(Transaction { + sender: signer_address, + unsigned, + }), + ); + + // case2: without chain_id replay protection + contract creation + let unsigned = UnsignedTransaction { + nonce: 100.into(), + gas_price: 200.into(), + gas: 300.into(), + to: None, + value: 400.into(), + payload: vec![1, 2, 3], + }; + let raw_tx = unsigned.clone().sign_by(&signer, None); + assert_eq!( + transaction_decode(&raw_tx), + Ok(Transaction { + sender: signer_address, + unsigned, + }), + ); + } +}