From a4c7843a073cceecf3d2adb8df26e96aa2e6bb6e Mon Sep 17 00:00:00 2001 From: Wei Tang Date: Sat, 5 May 2018 16:23:50 +0800 Subject: [PATCH] EIP 145: Bitwise shifting instructions in EVM (#8451) * Add SHL, SHR, SAR opcodes * Add have_bitwise_shifting schedule flag * Add all EIP tests for SHL * Add SHR implementation and tests * Implement SAR and add tests * Add eip145transition config param * Change map_or to map_or_else when possible --- ethcore/evm/src/instructions.rs | 10 +- ethcore/evm/src/interpreter/mod.rs | 55 +++++- ethcore/evm/src/tests.rs | 268 ++++++++++++++++++++++++++++- ethcore/src/spec/spec.rs | 39 +++-- ethcore/vm/src/schedule.rs | 12 +- ethcore/vm/src/tests.rs | 7 + json/src/spec/params.rs | 3 + 7 files changed, 374 insertions(+), 20 deletions(-) diff --git a/ethcore/evm/src/instructions.rs b/ethcore/evm/src/instructions.rs index 3fb9fb933..6ecfc7f67 100644 --- a/ethcore/evm/src/instructions.rs +++ b/ethcore/evm/src/instructions.rs @@ -187,6 +187,9 @@ lazy_static! { arr[OR as usize] = InstructionInfo::new("OR", 2, 1, GasPriceTier::VeryLow); arr[XOR as usize] = InstructionInfo::new("XOR", 2, 1, GasPriceTier::VeryLow); arr[BYTE as usize] = InstructionInfo::new("BYTE", 2, 1, GasPriceTier::VeryLow); + arr[SHL as usize] = InstructionInfo::new("SHL", 2, 1, GasPriceTier::VeryLow); + arr[SHR as usize] = InstructionInfo::new("SHR", 2, 1, GasPriceTier::VeryLow); + arr[SAR as usize] = InstructionInfo::new("SAR", 2, 1, GasPriceTier::VeryLow); arr[ADDMOD as usize] = InstructionInfo::new("ADDMOD", 3, 1, GasPriceTier::Mid); arr[MULMOD as usize] = InstructionInfo::new("MULMOD", 3, 1, GasPriceTier::Mid); arr[SIGNEXTEND as usize] = InstructionInfo::new("SIGNEXTEND", 2, 1, GasPriceTier::Low); @@ -354,6 +357,12 @@ pub const XOR: Instruction = 0x18; pub const NOT: Instruction = 0x19; /// retrieve single byte from word pub const BYTE: Instruction = 0x1a; +/// shift left operation +pub const SHL: Instruction = 0x1b; +/// logical shift right operation +pub const SHR: Instruction = 0x1c; +/// arithmetic shift right operation +pub const SAR: Instruction = 0x1d; /// compute SHA3-256 hash pub const SHA3: Instruction = 0x20; @@ -589,4 +598,3 @@ pub const REVERT: Instruction = 0xfd; pub const STATICCALL: Instruction = 0xfa; /// halt execution and register account for later deletion pub const SUICIDE: Instruction = 0xff; - diff --git a/ethcore/evm/src/interpreter/mod.rs b/ethcore/evm/src/interpreter/mod.rs index 05d1fb788..160a2e5b1 100644 --- a/ethcore/evm/src/interpreter/mod.rs +++ b/ethcore/evm/src/interpreter/mod.rs @@ -224,7 +224,8 @@ impl Interpreter { (instruction == instructions::CREATE2 && !schedule.have_create2) || (instruction == instructions::STATICCALL && !schedule.have_static_call) || ((instruction == instructions::RETURNDATACOPY || instruction == instructions::RETURNDATASIZE) && !schedule.have_return_data) || - (instruction == instructions::REVERT && !schedule.have_revert) { + (instruction == instructions::REVERT && !schedule.have_revert) || + ((instruction == instructions::SHL || instruction == instructions::SHR || instruction == instructions::SAR) && !schedule.have_bitwise_shifting) { return Err(vm::Error::BadInstruction { instruction: instruction @@ -871,6 +872,58 @@ impl Interpreter { }); } }, + instructions::SHL => { + const CONST_256: U256 = U256([256, 0, 0, 0]); + + let shift = stack.pop_back(); + let value = stack.pop_back(); + + let result = if shift >= CONST_256 { + U256::zero() + } else { + value << (shift.as_u32() as usize) + }; + stack.push(result); + }, + instructions::SHR => { + const CONST_256: U256 = U256([256, 0, 0, 0]); + + let shift = stack.pop_back(); + let value = stack.pop_back(); + + let result = if shift >= CONST_256 { + U256::zero() + } else { + value >> (shift.as_u32() as usize) + }; + stack.push(result); + }, + instructions::SAR => { + // We cannot use get_and_reset_sign/set_sign here, because the rounding looks different. + + const CONST_256: U256 = U256([256, 0, 0, 0]); + const CONST_HIBIT: U256 = U256([0, 0, 0, 0x8000000000000000]); + + let shift = stack.pop_back(); + let value = stack.pop_back(); + let sign = value & CONST_HIBIT != U256::zero(); + + let result = if shift >= CONST_256 { + if sign { + U256::max_value() + } else { + U256::zero() + } + } else { + let shift = shift.as_u32() as usize; + let mut shifted = value >> shift; + if sign { + shifted = shifted | (U256::max_value() << (256 - shift)); + } + shifted + }; + stack.push(result); + }, _ => { return Err(vm::Error::BadInstruction { instruction: instruction diff --git a/ethcore/evm/src/tests.rs b/ethcore/evm/src/tests.rs index 3758156e2..9058d073e 100644 --- a/ethcore/evm/src/tests.rs +++ b/ethcore/evm/src/tests.rs @@ -787,6 +787,273 @@ fn test_create_in_staticcall(factory: super::Factory) { assert_eq!(ext.calls.len(), 0); } +evm_test!{test_shl: test_shl_int} +fn test_shl(factory: super::Factory) { + push_two_pop_one_constantinople_test( + &factory, + 0x1b, + "0000000000000000000000000000000000000000000000000000000000000001", + "00", + "0000000000000000000000000000000000000000000000000000000000000001"); + push_two_pop_one_constantinople_test( + &factory, + 0x1b, + "0000000000000000000000000000000000000000000000000000000000000001", + "01", + "0000000000000000000000000000000000000000000000000000000000000002"); + push_two_pop_one_constantinople_test( + &factory, + 0x1b, + "0000000000000000000000000000000000000000000000000000000000000001", + "ff", + "8000000000000000000000000000000000000000000000000000000000000000"); + push_two_pop_one_constantinople_test( + &factory, + 0x1b, + "0000000000000000000000000000000000000000000000000000000000000001", + "0100", + "0000000000000000000000000000000000000000000000000000000000000000"); + push_two_pop_one_constantinople_test( + &factory, + 0x1b, + "0000000000000000000000000000000000000000000000000000000000000001", + "0101", + "0000000000000000000000000000000000000000000000000000000000000000"); + push_two_pop_one_constantinople_test( + &factory, + 0x1b, + "ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff", + "00", + "ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff"); + push_two_pop_one_constantinople_test( + &factory, + 0x1b, + "ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff", + "01", + "fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffe"); + push_two_pop_one_constantinople_test( + &factory, + 0x1b, + "ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff", + "ff", + "8000000000000000000000000000000000000000000000000000000000000000"); + push_two_pop_one_constantinople_test( + &factory, + 0x1b, + "ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff", + "0100", + "0000000000000000000000000000000000000000000000000000000000000000"); + push_two_pop_one_constantinople_test( + &factory, + 0x1b, + "0000000000000000000000000000000000000000000000000000000000000000", + "01", + "0000000000000000000000000000000000000000000000000000000000000000"); + push_two_pop_one_constantinople_test( + &factory, + 0x1b, + "7fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff", + "01", + "fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffe"); +} + +evm_test!{test_shr: test_shr_int} +fn test_shr(factory: super::Factory) { + push_two_pop_one_constantinople_test( + &factory, + 0x1c, + "0000000000000000000000000000000000000000000000000000000000000001", + "00", + "0000000000000000000000000000000000000000000000000000000000000001"); + push_two_pop_one_constantinople_test( + &factory, + 0x1c, + "0000000000000000000000000000000000000000000000000000000000000001", + "01", + "0000000000000000000000000000000000000000000000000000000000000000"); + push_two_pop_one_constantinople_test( + &factory, + 0x1c, + "8000000000000000000000000000000000000000000000000000000000000000", + "01", + "4000000000000000000000000000000000000000000000000000000000000000"); + push_two_pop_one_constantinople_test( + &factory, + 0x1c, + "8000000000000000000000000000000000000000000000000000000000000000", + "ff", + "0000000000000000000000000000000000000000000000000000000000000001"); + push_two_pop_one_constantinople_test( + &factory, + 0x1c, + "8000000000000000000000000000000000000000000000000000000000000000", + "0100", + "0000000000000000000000000000000000000000000000000000000000000000"); + push_two_pop_one_constantinople_test( + &factory, + 0x1c, + "8000000000000000000000000000000000000000000000000000000000000000", + "0101", + "0000000000000000000000000000000000000000000000000000000000000000"); + push_two_pop_one_constantinople_test( + &factory, + 0x1c, + "ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff", + "00", + "ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff"); + push_two_pop_one_constantinople_test( + &factory, + 0x1c, + "ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff", + "01", + "7fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff"); + push_two_pop_one_constantinople_test( + &factory, + 0x1c, + "ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff", + "ff", + "0000000000000000000000000000000000000000000000000000000000000001"); + push_two_pop_one_constantinople_test( + &factory, + 0x1c, + "ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff", + "0100", + "0000000000000000000000000000000000000000000000000000000000000000"); + push_two_pop_one_constantinople_test( + &factory, + 0x1c, + "0000000000000000000000000000000000000000000000000000000000000000", + "01", + "0000000000000000000000000000000000000000000000000000000000000000"); +} + +evm_test!{test_sar: test_sar_int} +fn test_sar(factory: super::Factory) { + push_two_pop_one_constantinople_test( + &factory, + 0x1d, + "0000000000000000000000000000000000000000000000000000000000000001", + "00", + "0000000000000000000000000000000000000000000000000000000000000001"); + push_two_pop_one_constantinople_test( + &factory, + 0x1d, + "0000000000000000000000000000000000000000000000000000000000000001", + "01", + "0000000000000000000000000000000000000000000000000000000000000000"); + push_two_pop_one_constantinople_test( + &factory, + 0x1d, + "8000000000000000000000000000000000000000000000000000000000000000", + "01", + "c000000000000000000000000000000000000000000000000000000000000000"); + push_two_pop_one_constantinople_test( + &factory, + 0x1d, + "8000000000000000000000000000000000000000000000000000000000000000", + "ff", + "ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff"); + push_two_pop_one_constantinople_test( + &factory, + 0x1d, + "8000000000000000000000000000000000000000000000000000000000000000", + "0100", + "ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff"); + push_two_pop_one_constantinople_test( + &factory, + 0x1d, + "8000000000000000000000000000000000000000000000000000000000000000", + "0101", + "ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff"); + push_two_pop_one_constantinople_test( + &factory, + 0x1d, + "ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff", + "00", + "ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff"); + push_two_pop_one_constantinople_test( + &factory, + 0x1d, + "ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff", + "01", + "ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff"); + push_two_pop_one_constantinople_test( + &factory, + 0x1d, + "ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff", + "ff", + "ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff"); + push_two_pop_one_constantinople_test( + &factory, + 0x1d, + "ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff", + "0100", + "ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff"); + push_two_pop_one_constantinople_test( + &factory, + 0x1d, + "0000000000000000000000000000000000000000000000000000000000000000", + "01", + "0000000000000000000000000000000000000000000000000000000000000000"); + push_two_pop_one_constantinople_test( + &factory, + 0x1d, + "4000000000000000000000000000000000000000000000000000000000000000", + "fe", + "0000000000000000000000000000000000000000000000000000000000000001"); + push_two_pop_one_constantinople_test( + &factory, + 0x1d, + "7fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff", + "f8", + "000000000000000000000000000000000000000000000000000000000000007f"); + push_two_pop_one_constantinople_test( + &factory, + 0x1d, + "7fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff", + "fe", + "0000000000000000000000000000000000000000000000000000000000000001"); + push_two_pop_one_constantinople_test( + &factory, + 0x1d, + "7fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff", + "ff", + "0000000000000000000000000000000000000000000000000000000000000000"); + push_two_pop_one_constantinople_test( + &factory, + 0x1d, + "7fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff", + "0100", + "0000000000000000000000000000000000000000000000000000000000000000"); +} + +fn push_two_pop_one_constantinople_test(factory: &super::Factory, opcode: u8, push1: &str, push2: &str, result: &str) { + let mut push1 = push1.from_hex().unwrap(); + let mut push2 = push2.from_hex().unwrap(); + assert!(push1.len() <= 32 && push1.len() != 0); + assert!(push2.len() <= 32 && push2.len() != 0); + + let mut code = Vec::new(); + code.push(0x60 + ((push1.len() - 1) as u8)); + code.append(&mut push1); + code.push(0x60 + ((push2.len() - 1) as u8)); + code.append(&mut push2); + code.push(opcode); + code.append(&mut vec![0x60, 0x00, 0x55]); + + let mut params = ActionParams::default(); + params.gas = U256::from(100_000); + params.code = Some(Arc::new(code)); + let mut ext = FakeExt::new_constantinople(); + + let _ = { + let mut vm = factory.create(¶ms.gas); + test_finalize(vm.exec(params, &mut ext)).unwrap() + }; + + assert_store(&ext, 0, result); +} + fn assert_set_contains(set: &HashSet, val: &T) { let contains = set.contains(val); if !contains { @@ -799,4 +1066,3 @@ fn assert_set_contains(set: &HashSet, val: fn assert_store(ext: &FakeExt, pos: u64, val: &str) { assert_eq!(ext.store.get(&H256::from(pos)).unwrap(), &H256::from_str(val).unwrap()); } - diff --git a/ethcore/src/spec/spec.rs b/ethcore/src/spec/spec.rs index 156ab8f15..ef69f4847 100644 --- a/ethcore/src/spec/spec.rs +++ b/ethcore/src/spec/spec.rs @@ -105,6 +105,8 @@ pub struct CommonParams { pub eip211_transition: BlockNumber, /// Number of first block where EIP-214 rules begin. pub eip214_transition: BlockNumber, + /// Number of first block where EIP-145 rules begin. + pub eip145_transition: BlockNumber, /// Number of first block where dust cleanup rules (EIP-168 and EIP169) begin. pub dust_protection_transition: BlockNumber, /// Nonce cap increase per block. Nonce cap is only checked if dust protection is enabled. @@ -152,6 +154,7 @@ impl CommonParams { schedule.have_revert = block_number >= self.eip140_transition; schedule.have_static_call = block_number >= self.eip214_transition; schedule.have_return_data = block_number >= self.eip211_transition; + schedule.have_bitwise_shifting = block_number >= self.eip145_transition; if block_number >= self.eip210_transition { schedule.blockhash_gas = 800; } @@ -198,16 +201,16 @@ impl From for CommonParams { eip155_transition: p.eip155_transition.map_or(0, Into::into), validate_receipts_transition: p.validate_receipts_transition.map_or(0, Into::into), validate_chain_id_transition: p.validate_chain_id_transition.map_or(0, Into::into), - eip86_transition: p.eip86_transition.map_or( - BlockNumber::max_value(), + eip86_transition: p.eip86_transition.map_or_else( + BlockNumber::max_value, Into::into, ), - eip140_transition: p.eip140_transition.map_or( - BlockNumber::max_value(), + eip140_transition: p.eip140_transition.map_or_else( + BlockNumber::max_value, Into::into, ), - eip210_transition: p.eip210_transition.map_or( - BlockNumber::max_value(), + eip210_transition: p.eip210_transition.map_or_else( + BlockNumber::max_value, Into::into, ), eip210_contract_address: p.eip210_contract_address.map_or(0xf0.into(), Into::into), @@ -220,20 +223,24 @@ impl From for CommonParams { Into::into, ), eip210_contract_gas: p.eip210_contract_gas.map_or(1000000.into(), Into::into), - eip211_transition: p.eip211_transition.map_or( - BlockNumber::max_value(), + eip211_transition: p.eip211_transition.map_or_else( + BlockNumber::max_value, Into::into, ), - eip214_transition: p.eip214_transition.map_or( - BlockNumber::max_value(), + eip145_transition: p.eip145_transition.map_or_else( + BlockNumber::max_value, Into::into, ), - eip658_transition: p.eip658_transition.map_or( - BlockNumber::max_value(), + eip214_transition: p.eip214_transition.map_or_else( + BlockNumber::max_value, Into::into, ), - dust_protection_transition: p.dust_protection_transition.map_or( - BlockNumber::max_value(), + eip658_transition: p.eip658_transition.map_or_else( + BlockNumber::max_value, + Into::into, + ), + dust_protection_transition: p.dust_protection_transition.map_or_else( + BlockNumber::max_value, Into::into, ), nonce_cap_increment: p.nonce_cap_increment.map_or(64, Into::into), @@ -245,8 +252,8 @@ impl From for CommonParams { max_transaction_size: p.max_transaction_size.map_or(MAX_TRANSACTION_SIZE, Into::into), max_code_size_transition: p.max_code_size_transition.map_or(0, Into::into), transaction_permission_contract: p.transaction_permission_contract.map(Into::into), - wasm_activation_transition: p.wasm_activation_transition.map_or( - BlockNumber::max_value(), + wasm_activation_transition: p.wasm_activation_transition.map_or_else( + BlockNumber::max_value, Into::into ), } diff --git a/ethcore/vm/src/schedule.rs b/ethcore/vm/src/schedule.rs index 6cfabed0d..a0085ef1e 100644 --- a/ethcore/vm/src/schedule.rs +++ b/ethcore/vm/src/schedule.rs @@ -109,6 +109,8 @@ pub struct Schedule { pub have_static_call: bool, /// RETURNDATA and RETURNDATASIZE opcodes enabled. pub have_return_data: bool, + /// SHL, SHR, SAR opcodes enabled. + pub have_bitwise_shifting: bool, /// Kill basic accounts below this balance if touched. pub kill_dust: CleanDustMode, /// Enable EIP-86 rules @@ -194,6 +196,7 @@ impl Schedule { have_create2: false, have_revert: false, have_return_data: false, + have_bitwise_shifting: false, stack_limit: 1024, max_depth: 1024, tier_step_gas: [0, 2, 3, 5, 8, 10, 20, 0], @@ -250,6 +253,13 @@ impl Schedule { schedule } + /// Schedule for the Constantinople fork of the Ethereum main net. + pub fn new_constantinople() -> Schedule { + let mut schedule = Self::new_byzantium(); + schedule.have_bitwise_shifting = true; + schedule + } + fn new(efcd: bool, hdc: bool, tcg: usize) -> Schedule { Schedule { exceptional_failed_code_deposit: efcd, @@ -257,6 +267,7 @@ impl Schedule { have_create2: false, have_revert: false, have_return_data: false, + have_bitwise_shifting: false, stack_limit: 1024, max_depth: 1024, tier_step_gas: [0, 2, 3, 5, 8, 10, 20, 0], @@ -328,4 +339,3 @@ fn schedule_evm_assumptions() { assert_eq!(s1.quad_coeff_div, 512); assert_eq!(s2.quad_coeff_div, 512); } - diff --git a/ethcore/vm/src/tests.rs b/ethcore/vm/src/tests.rs index ce4712145..daf46be0f 100644 --- a/ethcore/vm/src/tests.rs +++ b/ethcore/vm/src/tests.rs @@ -88,6 +88,13 @@ impl FakeExt { ext } + /// New fake externalities with constantinople schedule rules + pub fn new_constantinople() -> Self { + let mut ext = FakeExt::default(); + ext.schedule = Schedule::new_constantinople(); + ext + } + /// Alter fake externalities to allow wasm pub fn with_wasm(mut self) -> Self { self.schedule.wasm = Some(Default::default()); diff --git a/json/src/spec/params.rs b/json/src/spec/params.rs index ce47086df..0addf52e4 100644 --- a/json/src/spec/params.rs +++ b/json/src/spec/params.rs @@ -85,6 +85,9 @@ pub struct Params { #[serde(rename="eip211Transition")] pub eip211_transition: Option, /// See `CommonParams` docs. + #[serde(rename="eip145Transition")] + pub eip145_transition: Option, + /// See `CommonParams` docs. #[serde(rename="eip214Transition")] pub eip214_transition: Option, /// See `CommonParams` docs.