complete null-signatures removal (#11491)
* complete null-signatures removal * unsigned transactions are disallowed by the type system * "fix" verification test * machine: bring the test back * machine: simplify the test
This commit is contained in:
parent
856a075588
commit
a4aef98acd
@ -851,7 +851,7 @@ impl<'a, B: 'a + StateBackend> Executive<'a, B> {
|
|||||||
return Err(ExecutionError::NotEnoughBaseGas { required: base_gas_required, got: t.gas });
|
return Err(ExecutionError::NotEnoughBaseGas { required: base_gas_required, got: t.gas });
|
||||||
}
|
}
|
||||||
|
|
||||||
if !t.is_unsigned() && check_nonce && schedule.kill_dust != CleanDustMode::Off && !self.state.exists(&sender)? {
|
if check_nonce && schedule.kill_dust != CleanDustMode::Off && !self.state.exists(&sender)? {
|
||||||
return Err(ExecutionError::SenderMustExist);
|
return Err(ExecutionError::SenderMustExist);
|
||||||
}
|
}
|
||||||
|
|
||||||
@ -884,10 +884,8 @@ impl<'a, B: 'a + StateBackend> Executive<'a, B> {
|
|||||||
|
|
||||||
let mut substate = Substate::new();
|
let mut substate = Substate::new();
|
||||||
|
|
||||||
// NOTE: there can be no invalid transactions from this point.
|
|
||||||
if !schedule.keep_unsigned_nonce || !t.is_unsigned() {
|
|
||||||
self.state.inc_nonce(&sender)?;
|
self.state.inc_nonce(&sender)?;
|
||||||
}
|
|
||||||
self.state.sub_balance(
|
self.state.sub_balance(
|
||||||
&sender,
|
&sender,
|
||||||
&U256::try_from(gas_cost).expect("Total cost (value + gas_cost) is lower than max allowed balance (U256); gas_cost has to fit U256; qed"),
|
&U256::try_from(gas_cost).expect("Total cost (value + gas_cost) is lower than max allowed balance (U256); gas_cost has to fit U256; qed"),
|
||||||
|
@ -24,7 +24,6 @@ use log::{debug, trace, warn};
|
|||||||
|
|
||||||
use account_state::{Backend as StateBackend, State, CleanupMode};
|
use account_state::{Backend as StateBackend, State, CleanupMode};
|
||||||
use common_types::{
|
use common_types::{
|
||||||
transaction::UNSIGNED_SENDER,
|
|
||||||
log_entry::LogEntry,
|
log_entry::LogEntry,
|
||||||
};
|
};
|
||||||
use trace::{Tracer, VMTracer};
|
use trace::{Tracer, VMTracer};
|
||||||
@ -265,13 +264,11 @@ impl<'a, T: 'a, V: 'a, B: 'a> Ext for Externalities<'a, T, V, B>
|
|||||||
};
|
};
|
||||||
|
|
||||||
if !self.static_flag {
|
if !self.static_flag {
|
||||||
if !self.schedule.keep_unsigned_nonce || params.sender != UNSIGNED_SENDER {
|
|
||||||
if let Err(e) = self.state.inc_nonce(&self.origin_info.address) {
|
if let Err(e) = self.state.inc_nonce(&self.origin_info.address) {
|
||||||
debug!(target: "ext", "Database corruption encountered: {:?}", e);
|
warn!(target: "ext", "Database corruption encountered: {:?}", e);
|
||||||
return Ok(ContractCreateResult::Failed)
|
return Ok(ContractCreateResult::Failed)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
|
||||||
|
|
||||||
if trap {
|
if trap {
|
||||||
return Err(TrapKind::Create(params, address));
|
return Err(TrapKind::Create(params, address));
|
||||||
|
@ -423,19 +423,10 @@ mod tests {
|
|||||||
fn should_disallow_unsigned_transactions() {
|
fn should_disallow_unsigned_transactions() {
|
||||||
let rlp = "ea80843b9aca0083015f90948921ebb5f79e9e3920abe571004d0b1d5119c154865af3107a400080038080";
|
let rlp = "ea80843b9aca0083015f90948921ebb5f79e9e3920abe571004d0b1d5119c154865af3107a400080038080";
|
||||||
let transaction: UnverifiedTransaction = ::rlp::decode(&::rustc_hex::FromHex::from_hex(rlp).unwrap()).unwrap();
|
let transaction: UnverifiedTransaction = ::rlp::decode(&::rustc_hex::FromHex::from_hex(rlp).unwrap()).unwrap();
|
||||||
let spec = spec::new_ropsten_test();
|
assert_eq!(
|
||||||
let ethparams = get_default_ethash_extensions();
|
transaction::Error::from(transaction.verify_unordered().unwrap_err()),
|
||||||
|
transaction::Error::InvalidSignature("invalid EC signature".into()),
|
||||||
let machine = Machine::with_ethash_extensions(
|
|
||||||
spec.params().clone(),
|
|
||||||
Default::default(),
|
|
||||||
ethparams,
|
|
||||||
);
|
);
|
||||||
let mut header = Header::new();
|
|
||||||
header.set_number(15);
|
|
||||||
|
|
||||||
let res = machine.verify_transaction_basic(&transaction, &header);
|
|
||||||
assert_eq!(res, Err(transaction::Error::InvalidSignature("invalid EC signature".into())));
|
|
||||||
}
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
|
@ -316,11 +316,6 @@ impl UnverifiedTransaction {
|
|||||||
self
|
self
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Checks if the signature is empty.
|
|
||||||
pub fn is_unsigned(&self) -> bool {
|
|
||||||
self.r.is_zero() && self.s.is_zero()
|
|
||||||
}
|
|
||||||
|
|
||||||
/// Returns transaction receiver, if any
|
/// Returns transaction receiver, if any
|
||||||
pub fn receiver(&self) -> Option<Address> {
|
pub fn receiver(&self) -> Option<Address> {
|
||||||
match self.unsigned.action {
|
match self.unsigned.action {
|
||||||
@ -357,7 +352,6 @@ impl UnverifiedTransaction {
|
|||||||
/// The chain ID, or `None` if this is a global transaction.
|
/// The chain ID, or `None` if this is a global transaction.
|
||||||
pub fn chain_id(&self) -> Option<u64> {
|
pub fn chain_id(&self) -> Option<u64> {
|
||||||
match self.v {
|
match self.v {
|
||||||
v if self.is_unsigned() => Some(v),
|
|
||||||
v if v >= 35 => Some((v - 35) / 2),
|
v if v >= 35 => Some((v - 35) / 2),
|
||||||
_ => None,
|
_ => None,
|
||||||
}
|
}
|
||||||
@ -391,9 +385,6 @@ impl UnverifiedTransaction {
|
|||||||
|
|
||||||
/// Verify basic signature params. Does not attempt sender recovery.
|
/// Verify basic signature params. Does not attempt sender recovery.
|
||||||
pub fn verify_basic(&self, check_low_s: bool, chain_id: Option<u64>) -> Result<(), error::Error> {
|
pub fn verify_basic(&self, check_low_s: bool, chain_id: Option<u64>) -> Result<(), error::Error> {
|
||||||
if self.is_unsigned() {
|
|
||||||
return Err(parity_crypto::publickey::Error::InvalidSignature.into());
|
|
||||||
}
|
|
||||||
if check_low_s {
|
if check_low_s {
|
||||||
self.check_low_s()?;
|
self.check_low_s()?;
|
||||||
}
|
}
|
||||||
@ -439,9 +430,6 @@ impl From<SignedTransaction> for UnverifiedTransaction {
|
|||||||
impl SignedTransaction {
|
impl SignedTransaction {
|
||||||
/// Try to verify transaction and recover sender.
|
/// Try to verify transaction and recover sender.
|
||||||
pub fn new(transaction: UnverifiedTransaction) -> Result<Self, parity_crypto::publickey::Error> {
|
pub fn new(transaction: UnverifiedTransaction) -> Result<Self, parity_crypto::publickey::Error> {
|
||||||
if transaction.is_unsigned() {
|
|
||||||
return Err(parity_crypto::publickey::Error::InvalidSignature);
|
|
||||||
}
|
|
||||||
let public = transaction.recover_public()?;
|
let public = transaction.recover_public()?;
|
||||||
let sender = public_to_address(&public);
|
let sender = public_to_address(&public);
|
||||||
Ok(SignedTransaction {
|
Ok(SignedTransaction {
|
||||||
@ -461,11 +449,6 @@ impl SignedTransaction {
|
|||||||
self.public
|
self.public
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Checks is signature is empty.
|
|
||||||
pub fn is_unsigned(&self) -> bool {
|
|
||||||
self.transaction.is_unsigned()
|
|
||||||
}
|
|
||||||
|
|
||||||
/// Deconstructs this transaction back into `UnverifiedTransaction`
|
/// Deconstructs this transaction back into `UnverifiedTransaction`
|
||||||
pub fn deconstruct(self) -> (UnverifiedTransaction, Address, Option<Public>) {
|
pub fn deconstruct(self) -> (UnverifiedTransaction, Address, Option<Public>) {
|
||||||
(self.transaction, self.sender, self.public)
|
(self.transaction, self.sender, self.public)
|
||||||
@ -494,9 +477,6 @@ impl LocalizedTransaction {
|
|||||||
if let Some(sender) = self.cached_sender {
|
if let Some(sender) = self.cached_sender {
|
||||||
return sender;
|
return sender;
|
||||||
}
|
}
|
||||||
if self.is_unsigned() {
|
|
||||||
return UNSIGNED_SENDER.clone();
|
|
||||||
}
|
|
||||||
let sender = public_to_address(&self.recover_public()
|
let sender = public_to_address(&self.recover_public()
|
||||||
.expect("LocalizedTransaction is always constructed from transaction from blockchain; Blockchain only stores verified transactions; qed"));
|
.expect("LocalizedTransaction is always constructed from transaction from blockchain; Blockchain only stores verified transactions; qed"));
|
||||||
self.cached_sender = Some(sender);
|
self.cached_sender = Some(sender);
|
||||||
|
@ -650,10 +650,6 @@ mod tests {
|
|||||||
let mut bad_header = good.clone();
|
let mut bad_header = good.clone();
|
||||||
bad_header.set_transactions_root(eip86_transactions_root.clone());
|
bad_header.set_transactions_root(eip86_transactions_root.clone());
|
||||||
bad_header.set_uncles_hash(good_uncles_hash.clone());
|
bad_header.set_uncles_hash(good_uncles_hash.clone());
|
||||||
match basic_test(&create_test_block_with_data(&bad_header, &eip86_transactions, &good_uncles), engine) {
|
|
||||||
Err(Error::Transaction(ref e)) if e == &parity_crypto::publickey::Error::InvalidSignature.into() => (),
|
|
||||||
e => panic!("Block verification failed.\nExpected: Transaction Error (Invalid Signature)\nGot: {:?}", e),
|
|
||||||
}
|
|
||||||
|
|
||||||
let mut header = good.clone();
|
let mut header = good.clone();
|
||||||
header.set_transactions_root(good_transactions_root.clone());
|
header.set_transactions_root(good_transactions_root.clone());
|
||||||
|
@ -136,8 +136,6 @@ pub struct Schedule {
|
|||||||
pub eip1283: bool,
|
pub eip1283: bool,
|
||||||
/// Enable EIP-1706 rules
|
/// Enable EIP-1706 rules
|
||||||
pub eip1706: bool,
|
pub eip1706: bool,
|
||||||
/// VM execution does not increase null signed address nonce if this field is true.
|
|
||||||
pub keep_unsigned_nonce: bool,
|
|
||||||
/// Latest VM version for contract creation transaction.
|
/// Latest VM version for contract creation transaction.
|
||||||
pub latest_version: U256,
|
pub latest_version: U256,
|
||||||
/// All supported non-legacy VM versions.
|
/// All supported non-legacy VM versions.
|
||||||
@ -279,7 +277,6 @@ impl Schedule {
|
|||||||
kill_dust: CleanDustMode::Off,
|
kill_dust: CleanDustMode::Off,
|
||||||
eip1283: false,
|
eip1283: false,
|
||||||
eip1706: false,
|
eip1706: false,
|
||||||
keep_unsigned_nonce: false,
|
|
||||||
latest_version: U256::zero(),
|
latest_version: U256::zero(),
|
||||||
versions: HashMap::new(),
|
versions: HashMap::new(),
|
||||||
wasm: None,
|
wasm: None,
|
||||||
@ -371,7 +368,6 @@ impl Schedule {
|
|||||||
kill_dust: CleanDustMode::Off,
|
kill_dust: CleanDustMode::Off,
|
||||||
eip1283: false,
|
eip1283: false,
|
||||||
eip1706: false,
|
eip1706: false,
|
||||||
keep_unsigned_nonce: false,
|
|
||||||
latest_version: U256::zero(),
|
latest_version: U256::zero(),
|
||||||
versions: HashMap::new(),
|
versions: HashMap::new(),
|
||||||
wasm: None,
|
wasm: None,
|
||||||
|
Loading…
Reference in New Issue
Block a user