From d0ecfa82d8e598ffcb74d7ca44e7570a02235087 Mon Sep 17 00:00:00 2001 From: Hannes Karppila Date: Tue, 21 Mar 2023 15:02:26 +0200 Subject: [PATCH 1/4] Make SMO instruction take data ptr as an argument --- fuel-vm/src/interpreter/blockchain.rs | 19 +++++---- .../src/interpreter/blockchain/smo_tests.rs | 40 +++++++++---------- fuel-vm/src/tests/blockchain.rs | 4 +- 3 files changed, 31 insertions(+), 32 deletions(-) diff --git a/fuel-vm/src/interpreter/blockchain.rs b/fuel-vm/src/interpreter/blockchain.rs index 6562971ac..f95e59c37 100644 --- a/fuel-vm/src/interpreter/blockchain.rs +++ b/fuel-vm/src/interpreter/blockchain.rs @@ -245,8 +245,8 @@ where fp: fp.as_ref(), pc, recipient_mem_address: a, - call_abi_len: b, - message_output_idx: c, + msg_data_ptr: b, + msg_data_len: c, amount_coins_to_send: d, }; input.message_output() @@ -685,10 +685,9 @@ struct MessageOutputCtx<'vm, Tx> { /// A recipient_mem_address: Word, /// B - call_abi_len: Word, + msg_data_ptr: Word, /// C - #[allow(dead_code)] - message_output_idx: Word, + msg_data_len: Word, /// D amount_coins_to_send: Word, } @@ -702,9 +701,9 @@ impl MessageOutputCtx<'_, Tx> { let memory_constraint = recipient_address.end() as Word ..(arith::checked_add_word(recipient_address.end() as Word, self.max_message_data_length)? .min(MEM_MAX_ACCESS_SIZE)); - let call_abi = CheckedMemRange::new_with_constraint( - recipient_address.end() as Word, - self.call_abi_len as usize, + let msg_data_range = CheckedMemRange::new_with_constraint( + self.msg_data_ptr as Word, + self.msg_data_len as usize, memory_constraint, )?; @@ -716,7 +715,7 @@ impl MessageOutputCtx<'_, Tx> { let sender = CheckedMemConstLen::<{ Address::LEN }>::new(*self.fp)?; let txid = tx_id(self.memory); - let call_abi = call_abi.read(self.memory).to_vec(); + let msg_data = msg_data_range.read(self.memory).to_vec(); let sender = Address::from_bytes_ref(sender.read(self.memory)); let receipt = Receipt::message_out_from_tx_output( @@ -725,7 +724,7 @@ impl MessageOutputCtx<'_, Tx> { *sender, recipient, self.amount_coins_to_send, - call_abi, + msg_data, ); append_receipt( diff --git a/fuel-vm/src/interpreter/blockchain/smo_tests.rs b/fuel-vm/src/interpreter/blockchain/smo_tests.rs index 8a6176472..33429f406 100644 --- a/fuel-vm/src/interpreter/blockchain/smo_tests.rs +++ b/fuel-vm/src/interpreter/blockchain/smo_tests.rs @@ -9,9 +9,9 @@ struct Input { /// A recipient_mem_address: Word, /// B - call_abi_len: Word, + msg_data_ptr: Word, /// C - message_output_idx: Word, + msg_data_len: Word, /// D amount_coins_to_send: Word, max_message_data_length: Word, @@ -28,8 +28,8 @@ impl Default for Input { fn default() -> Self { Self { recipient_mem_address: Default::default(), - call_abi_len: Default::default(), - message_output_idx: Default::default(), + msg_data_ptr: Default::default(), + msg_data_len: Default::default(), amount_coins_to_send: Default::default(), memory: vec![(400, Address::from([1u8; 32]).to_vec())], max_message_data_length: 100, @@ -41,8 +41,8 @@ impl Default for Input { #[test_case( Input { recipient_mem_address: 400, - call_abi_len: 1, - message_output_idx: 0, + msg_data_ptr: 432, + msg_data_len: 1, amount_coins_to_send: 0, ..Default::default() } => matches Ok(Output { .. }) @@ -51,8 +51,8 @@ impl Default for Input { #[test_case( Input { recipient_mem_address: 0, - call_abi_len: 0, - message_output_idx: 0, + msg_data_ptr: 32, + msg_data_len: 0, amount_coins_to_send: 0, ..Default::default() } => Err(RuntimeError::Recoverable(PanicReason::MemoryOverflow)) @@ -61,8 +61,8 @@ impl Default for Input { #[test_case( Input { recipient_mem_address: Word::MAX, - call_abi_len: 1, - message_output_idx: 0, + msg_data_ptr: 32, + msg_data_len: 1, amount_coins_to_send: 0, max_message_data_length: Word::MAX, ..Default::default() @@ -72,8 +72,8 @@ impl Default for Input { #[test_case( Input { recipient_mem_address: VM_MAX_RAM - 64, - call_abi_len: 100, - message_output_idx: 0, + msg_data_ptr: 32, + msg_data_len: 100, amount_coins_to_send: 0, max_message_data_length: Word::MAX, ..Default::default() @@ -83,8 +83,8 @@ impl Default for Input { #[test_case( Input { recipient_mem_address: 400, - call_abi_len: 101, - message_output_idx: 0, + msg_data_ptr: 32, + msg_data_len: 101, amount_coins_to_send: 0, max_message_data_length: 100, ..Default::default() @@ -94,8 +94,8 @@ impl Default for Input { #[test_case( Input { recipient_mem_address: 400, - call_abi_len: 10, - message_output_idx: 0, + msg_data_ptr: 432, + msg_data_len: 10, amount_coins_to_send: 30, balance: [(AssetId::zeroed(), 29)].into_iter().collect(), ..Default::default() @@ -106,8 +106,8 @@ impl Default for Input { fn test_smo( Input { recipient_mem_address, - call_abi_len, - message_output_idx, + msg_data_len, + msg_data_ptr, amount_coins_to_send, memory: mem, max_message_data_length, @@ -133,8 +133,8 @@ fn test_smo( fp: Reg::new(&fp), pc: RegMut::new(&mut pc), recipient_mem_address, - call_abi_len, - message_output_idx, + msg_data_len, + msg_data_ptr, amount_coins_to_send, }; input.message_output().map(|_| Output { receipts }) diff --git a/fuel-vm/src/tests/blockchain.rs b/fuel-vm/src/tests/blockchain.rs index a1a27e92a..6c93fcee8 100644 --- a/fuel-vm/src/tests/blockchain.rs +++ b/fuel-vm/src/tests/blockchain.rs @@ -1322,8 +1322,8 @@ fn smo_instruction_works() { #[rustfmt::skip] let script = vec![ op::movi(0x10, 0), // set the txid as recipient - op::movi(0x11, 1), // send the whole data buffer - op::movi(0x12, 0), // tx output idx + op::movi(0x11, 32), // data ptr just after the recipient + op::movi(0x12, 1), // one byte of data op::movi(0x13, message_output_amount as Immediate24), // expected output amount op::smo(0x10,0x11,0x12,0x13), op::ret(RegId::ONE) From 7112d8d3d06a6b5b1f8598c595e3e15d5926af18 Mon Sep 17 00:00:00 2001 From: Hannes Karppila Date: Tue, 21 Mar 2023 16:35:08 +0200 Subject: [PATCH 2/4] Remove todo comment --- fuel-vm/src/interpreter/blockchain.rs | 6 ------ 1 file changed, 6 deletions(-) diff --git a/fuel-vm/src/interpreter/blockchain.rs b/fuel-vm/src/interpreter/blockchain.rs index f95e59c37..ea7db2b9b 100644 --- a/fuel-vm/src/interpreter/blockchain.rs +++ b/fuel-vm/src/interpreter/blockchain.rs @@ -667,12 +667,6 @@ pub(crate) fn timestamp( inc_pc(pc) } - -// TODO: Register b is the address of the begin of the data and register c -// is the end of the data(Based on the specification). -// The user doesn't specify the output index for message. -// We need to use the index of the receipt to calculate the `Nonce`. -// Add unit tests for a new usage. struct MessageOutputCtx<'vm, Tx> { max_message_data_length: u64, memory: &'vm mut [u8; MEM_SIZE], From 18bb8bb56f8b513685f2402fe9fd2436acc65f71 Mon Sep 17 00:00:00 2001 From: Hannes Karppila Date: Tue, 21 Mar 2023 18:14:00 +0200 Subject: [PATCH 3/4] Add tests, fix issues, cleanup --- fuel-vm/src/interpreter/blockchain.rs | 18 +++++++------ .../src/interpreter/blockchain/smo_tests.rs | 20 +++++++-------- .../src/interpreter/executors/instruction.rs | 2 +- fuel-vm/src/tests/blockchain.rs | 25 ++++++++++++++----- 4 files changed, 40 insertions(+), 25 deletions(-) diff --git a/fuel-vm/src/interpreter/blockchain.rs b/fuel-vm/src/interpreter/blockchain.rs index ea7db2b9b..a0638f3cc 100644 --- a/fuel-vm/src/interpreter/blockchain.rs +++ b/fuel-vm/src/interpreter/blockchain.rs @@ -692,14 +692,16 @@ impl MessageOutputCtx<'_, Tx> { Tx: ExecutableTransaction, { let recipient_address = CheckedMemValue::
::new::<{ Address::LEN }>(self.recipient_mem_address)?; - let memory_constraint = recipient_address.end() as Word - ..(arith::checked_add_word(recipient_address.end() as Word, self.max_message_data_length)? - .min(MEM_MAX_ACCESS_SIZE)); - let msg_data_range = CheckedMemRange::new_with_constraint( - self.msg_data_ptr as Word, - self.msg_data_len as usize, - memory_constraint, - )?; + + if self.msg_data_len > MEM_MAX_ACCESS_SIZE { + return Err(RuntimeError::Recoverable(PanicReason::MemoryOverflow)); + } + + if self.msg_data_len > self.max_message_data_length { + return Err(RuntimeError::Recoverable(PanicReason::MemoryOverflow)); + } + + let msg_data_range = CheckedMemRange::new(self.msg_data_ptr, self.msg_data_len as usize)?; let recipient = recipient_address.try_from(self.memory)?; diff --git a/fuel-vm/src/interpreter/blockchain/smo_tests.rs b/fuel-vm/src/interpreter/blockchain/smo_tests.rs index 33429f406..801ed2b76 100644 --- a/fuel-vm/src/interpreter/blockchain/smo_tests.rs +++ b/fuel-vm/src/interpreter/blockchain/smo_tests.rs @@ -41,7 +41,7 @@ impl Default for Input { #[test_case( Input { recipient_mem_address: 400, - msg_data_ptr: 432, + msg_data_ptr: 0, msg_data_len: 1, amount_coins_to_send: 0, ..Default::default() @@ -51,7 +51,7 @@ impl Default for Input { #[test_case( Input { recipient_mem_address: 0, - msg_data_ptr: 32, + msg_data_ptr: 0, msg_data_len: 0, amount_coins_to_send: 0, ..Default::default() @@ -60,8 +60,8 @@ impl Default for Input { )] #[test_case( Input { - recipient_mem_address: Word::MAX, - msg_data_ptr: 32, + recipient_mem_address: 0, + msg_data_ptr: Word::MAX, msg_data_len: 1, amount_coins_to_send: 0, max_message_data_length: Word::MAX, @@ -71,19 +71,19 @@ impl Default for Input { )] #[test_case( Input { - recipient_mem_address: VM_MAX_RAM - 64, - msg_data_ptr: 32, + recipient_mem_address: 0, + msg_data_ptr: VM_MAX_RAM - 64, msg_data_len: 100, amount_coins_to_send: 0, max_message_data_length: Word::MAX, ..Default::default() - } => Err(RuntimeError::Recoverable(PanicReason::ArithmeticOverflow)) + } => Err(RuntimeError::Recoverable(PanicReason::MemoryOverflow)) ; "address + call abi length overflows memory" )] #[test_case( Input { - recipient_mem_address: 400, - msg_data_ptr: 32, + recipient_mem_address: 0, + msg_data_ptr: 0, msg_data_len: 101, amount_coins_to_send: 0, max_message_data_length: 100, @@ -94,7 +94,7 @@ impl Default for Input { #[test_case( Input { recipient_mem_address: 400, - msg_data_ptr: 432, + msg_data_ptr: 0, msg_data_len: 10, amount_coins_to_send: 30, balance: [(AssetId::zeroed(), 29)].into_iter().collect(), diff --git a/fuel-vm/src/interpreter/executors/instruction.rs b/fuel-vm/src/interpreter/executors/instruction.rs index d125e1194..416aae61f 100644 --- a/fuel-vm/src/interpreter/executors/instruction.rs +++ b/fuel-vm/src/interpreter/executors/instruction.rs @@ -432,7 +432,7 @@ where Instruction::SMO(smo) => { let (a, b, c, d) = smo.unpack(); - self.dependent_gas_charge(self.gas_costs.smo, r!(b))?; + self.dependent_gas_charge(self.gas_costs.smo, r!(c))?; self.message_output(r!(a), r!(b), r!(c), r!(d))?; } diff --git a/fuel-vm/src/tests/blockchain.rs b/fuel-vm/src/tests/blockchain.rs index 6c93fcee8..0111d17f2 100644 --- a/fuel-vm/src/tests/blockchain.rs +++ b/fuel-vm/src/tests/blockchain.rs @@ -1319,13 +1319,21 @@ fn smo_instruction_works() { let secret = SecretKey::random(rng); let sender = rng.gen(); + // Two bytes of random data to send as the message + let msg_data = [rng.gen::(), rng.gen::()]; + #[rustfmt::skip] let script = vec![ + op::movi(0x10, 2), // data buffer allocation size + op::aloc(0x10), // allocate + op::movi(0x10, msg_data[0].into()), // first message byte + op::sb(RegId::HP, 0x10, 0), // store above to the message buffer + op::movi(0x10, msg_data[1].into()), // second message byte + op::sb(RegId::HP, 0x10, 1), // store above to the message buffer op::movi(0x10, 0), // set the txid as recipient - op::movi(0x11, 32), // data ptr just after the recipient - op::movi(0x12, 1), // one byte of data + op::movi(0x12, 2), // one byte of data op::movi(0x13, message_output_amount as Immediate24), // expected output amount - op::smo(0x10,0x11,0x12,0x13), + op::smo(0x10,RegId::HP,0x12,0x13), op::ret(RegId::ONE) ]; @@ -1363,15 +1371,20 @@ fn smo_instruction_works() { }); let state = client.state_transition().expect("tx was executed"); - // TODO: Add check for the `data` field too, but it requires fixing of the `smo` behaviour. let message_receipt = state.receipts().iter().find_map(|o| match o { - Receipt::MessageOut { recipient, amount, .. } => Some((*recipient, *amount)), + Receipt::MessageOut { + recipient, + amount, + data, + .. + } => Some((*recipient, *amount, data.clone())), _ => None, }); assert_eq!(message_receipt.is_some(), success); - if let Some((recipient, transferred)) = message_receipt { + if let Some((recipient, transferred, data)) = message_receipt { assert_eq!(txid.as_ref(), recipient.as_ref()); assert_eq!(message_output_amount, transferred); + assert_eq!(data, msg_data); } // get gas used from script result let gas_used = if let Receipt::ScriptResult { gas_used, .. } = state.receipts().last().unwrap() { From 6b295c9739b0237034664d3089f6fe43d7631d62 Mon Sep 17 00:00:00 2001 From: Hannes Karppila Date: Tue, 21 Mar 2023 18:59:22 +0200 Subject: [PATCH 4/4] Add a new error type MessageDataTooLong --- fuel-asm/src/panic_reason.rs | 5 ++++- fuel-vm/src/interpreter/blockchain.rs | 2 +- fuel-vm/src/interpreter/blockchain/smo_tests.rs | 2 +- 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/fuel-asm/src/panic_reason.rs b/fuel-asm/src/panic_reason.rs index 72f7e4700..b5e3e84d0 100644 --- a/fuel-asm/src/panic_reason.rs +++ b/fuel-asm/src/panic_reason.rs @@ -75,6 +75,8 @@ pub enum PanicReason { ContractIdAlreadyDeployed = 0x20, /// The loaded contract mismatch expectations. ContractMismatch = 0x21, + /// Attempting to send message data longer than `MAX_MESSAGE_DATA_LENGTH` + MessageDataTooLong = 0x22, /// The byte can't be mapped to any known `PanicReason`. UnknownPanicReason = 0x23, } @@ -137,6 +139,7 @@ impl From for PanicReason { 0x1f => IllegalJump, 0x20 => ContractIdAlreadyDeployed, 0x21 => ContractMismatch, + 0x22 => MessageDataTooLong, _ => UnknownPanicReason, } } @@ -163,7 +166,7 @@ mod tests { #[test] fn test_u8_panic_reason_round_trip() { - const LAST_PANIC_REASON: u8 = 0x22; + const LAST_PANIC_REASON: u8 = 0x23; for i in 0..LAST_PANIC_REASON { let reason = PanicReason::from(i); let i2 = reason as u8; diff --git a/fuel-vm/src/interpreter/blockchain.rs b/fuel-vm/src/interpreter/blockchain.rs index a0638f3cc..9030c8b69 100644 --- a/fuel-vm/src/interpreter/blockchain.rs +++ b/fuel-vm/src/interpreter/blockchain.rs @@ -698,7 +698,7 @@ impl MessageOutputCtx<'_, Tx> { } if self.msg_data_len > self.max_message_data_length { - return Err(RuntimeError::Recoverable(PanicReason::MemoryOverflow)); + return Err(RuntimeError::Recoverable(PanicReason::MessageDataTooLong)); } let msg_data_range = CheckedMemRange::new(self.msg_data_ptr, self.msg_data_len as usize)?; diff --git a/fuel-vm/src/interpreter/blockchain/smo_tests.rs b/fuel-vm/src/interpreter/blockchain/smo_tests.rs index 801ed2b76..4b0f9064f 100644 --- a/fuel-vm/src/interpreter/blockchain/smo_tests.rs +++ b/fuel-vm/src/interpreter/blockchain/smo_tests.rs @@ -88,7 +88,7 @@ impl Default for Input { amount_coins_to_send: 0, max_message_data_length: 100, ..Default::default() - } => Err(RuntimeError::Recoverable(PanicReason::MemoryOverflow)) + } => Err(RuntimeError::Recoverable(PanicReason::MessageDataTooLong)) ; "call abi length > max message data length" )] #[test_case(