remove null signatures

ethcore/machine/src/machine.rs

@@ -343,7 +343,7 @@ impl Machine {
  } else {
  None
  };
- t.verify_basic(check_low_s, chain_id, false)?;
+ t.verify_basic(check_low_s, chain_id)?;
 
  Ok(())
  }

ethcore/src/test_helpers/evm_test_client.rs

@@ -260,7 +260,7 @@ impl<'a> EvmTestClient<'a> {
  ) -> std::result::Result<TransactSuccess<T::Output, V::Output>, TransactErr> {
  let initial_gas = transaction.gas;
  // Verify transaction
- let is_ok = transaction.verify_basic(true, None, false);
+ let is_ok = transaction.verify_basic(true, None);
  if let Err(error) = is_ok {
  return Err(
  TransactErr{

ethcore/types/src/transaction/transaction.rs

@@ -31,7 +31,8 @@ use transaction::error;
 type Bytes = Vec<u8>;
 type BlockNumber = u64;
 
-/// Fake address for unsigned transactions as defined by EIP-86.
+/// Fake address for unsigned transactions as defined by Legacy EIP-86.
+#[cfg(any(test, feature = "test-helpers"))]
 pub const UNSIGNED_SENDER: Address = H160([0xff; 20]);
 
 /// System sender address for internal state updates.
@@ -138,6 +139,7 @@ impl Transaction {
  }
 }
 
+#[cfg(any(test, feature = "test-helpers"))]
 impl From<ethjson::transaction::Transaction> for SignedTransaction {
  fn from(t: ethjson::transaction::Transaction) -> Self {
  let to: Option<ethjson::hash::Address> = t.to.into();
@@ -237,7 +239,8 @@ impl Transaction {
  }
  }
 
- /// Add EIP-86 compatible empty signature.
+ /// Legacy EIP-86 compatible empty signature.
+ #[cfg(any(test, feature = "test-helpers"))]
  pub fn null_sign(self, chain_id: u64) -> SignedTransaction {
  SignedTransaction {
  transaction: UnverifiedTransaction {
@@ -295,7 +298,7 @@ impl rlp::Decodable for UnverifiedTransaction {
  v: d.val_at(6)?,
  r: d.val_at(7)?,
  s: d.val_at(8)?,
- hash: hash,
+ hash,
  })
  }
 }
@@ -369,7 +372,7 @@ impl UnverifiedTransaction {
  /// Checks whether the signature has a low 's' value.
  pub fn check_low_s(&self) -> Result<(), parity_crypto::publickey::Error> {
  if !self.signature().is_low_s() {
- Err(parity_crypto::publickey::Error::InvalidSignature.into())
+ Err(parity_crypto::publickey::Error::InvalidSignature)
  } else {
  Ok(())
  }
@@ -386,17 +389,12 @@ impl UnverifiedTransaction {
  }
 
  /// Verify basic signature params. Does not attempt sender recovery.
- pub fn verify_basic(&self, check_low_s: bool, chain_id: Option<u64>, allow_empty_signature: bool) -> Result<(), error::Error> {
- if check_low_s && !(allow_empty_signature && self.is_unsigned()) {
- self.check_low_s()?;
- }
- // Disallow unsigned transactions in case EIP-86 is disabled.
- if !allow_empty_signature && self.is_unsigned() {
+ 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());
  }
- // EIP-86: Transactions of this form MUST have gasprice = 0, nonce = 0, value = 0, and do NOT increment the nonce of account 0.
- if allow_empty_signature && self.is_unsigned() && !(self.gas_price.is_zero() && self.value.is_zero() && self.nonce.is_zero()) {
- return Err(parity_crypto::publickey::Error::InvalidSignature.into())
+ if check_low_s {
+ self.check_low_s()?;
  }
  match (self.chain_id(), chain_id) {
  (None, _) => {},
@@ -441,20 +439,15 @@ impl SignedTransaction {
  /// Try to verify transaction and recover sender.
  pub fn new(transaction: UnverifiedTransaction) -> Result<Self, parity_crypto::publickey::Error> {
  if transaction.is_unsigned() {
- Ok(SignedTransaction {
- transaction: transaction,
- sender: UNSIGNED_SENDER,
- public: None,
- })
- } else {
- let public = transaction.recover_public()?;
- let sender = public_to_address(&public);
- Ok(SignedTransaction {
- transaction: transaction,
- sender: sender,
- public: Some(public),
- })
+ return Err(parity_crypto::publickey::Error::InvalidSignature);
  }
+ let public = transaction.recover_public()?;
+ let sender = public_to_address(&public);
+ Ok(SignedTransaction {
+ transaction,
+ sender,
+ public: Some(public),
+ })
  }
 
  /// Returns transaction sender.
@@ -645,6 +638,24 @@ mod tests {
  assert_eq!(t.chain_id(), None);
  }
 
+ #[test]
+ fn should_reject_null_signature() {
+ let t = Transaction {
+ nonce: U256::zero(),
+ gas_price: U256::from(10000000000u64),
+ gas: U256::from(21000),
+ action: Action::Call(Address::from_str("d46e8dd67c5d32be8058bb8eb970870f07244567").unwrap()),
+ value: U256::from(1),
+ data: vec![]
+ }.null_sign(1);
+
+ let res = SignedTransaction::new(t.transaction);
+ match res {
+ Err(parity_crypto::publickey::Error::InvalidSignature) => {}
+ _ => panic!("null signature should be rejected"),
+ }
+ }
+
  #[test]
  fn should_recover_from_chain_specific_signing() {
  use parity_crypto::publickey::{Random, Generator};

evmbin/Cargo.toml

@@ -11,7 +11,7 @@ path = "./src/main.rs"
 
 [dependencies]
 account-state = { path = "../ethcore/account-state" }
-common-types = { path = "../ethcore/types" }
+common-types = { path = "../ethcore/types", features = ["test-helpers"] }
 docopt = "1.0"
 env_logger = "0.5"
 ethcore = { path = "../ethcore", features = ["test-helpers", "json-tests"] }

json/src/spec/builtin.rs

@@ -203,6 +203,28 @@ mod tests {
  ]);
  }
 
+ #[test]
+ fn deserialization_alt_bn128_const_operations() {
+ let s = r#"{
+ "name": "alt_bn128_mul",
+ "pricing": {
+ "100500": {
+ "price": { "alt_bn128_const_operations": { "price": 123 }}
+ }
+ }
+ }"#;
+ let builtin: Builtin = serde_json::from_str::<BuiltinCompat>(s).unwrap().into();
+ assert_eq!(builtin.name, "alt_bn128_mul");
+ assert_eq!(builtin.pricing, map![
+ 100500 => PricingAt {
+ info: None,
+ price: Pricing::AltBn128ConstOperations(AltBn128ConstOperations { 
+ price: 123,
+ }),
+ }
+ ]);
+ }
+
  #[test]
  fn activate_at() {
  let s = r#"{

json/src/uint.rs

@@ -125,7 +125,6 @@ pub fn validate_optional_non_zero<'de, D>(d: D) -> Result<Option<Uint>, D::Error
 mod test {
  use super::Uint;
  use ethereum_types::U256;
- use serde_json::error::Category;
 
  #[test]
  fn uint_deserialization() {