Skip to content

Commit

Permalink
Make SMO instruction take data ptr as an argument (#404)
Browse files Browse the repository at this point in the history
* Make SMO instruction take data ptr as an argument

* Remove todo comment

* Add tests, fix issues, cleanup

* Add a new error type MessageDataTooLong
  • Loading branch information
Dentosal authored Mar 22, 2023
1 parent 7c98ecb commit d313654
Show file tree
Hide file tree
Showing 5 changed files with 65 additions and 54 deletions.
5 changes: 4 additions & 1 deletion fuel-asm/src/panic_reason.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}
Expand Down Expand Up @@ -137,6 +139,7 @@ impl From<u8> for PanicReason {
0x1f => IllegalJump,
0x20 => ContractIdAlreadyDeployed,
0x21 => ContractMismatch,
0x22 => MessageDataTooLong,
_ => UnknownPanicReason,
}
}
Expand All @@ -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;
Expand Down
37 changes: 16 additions & 21 deletions fuel-vm/src/interpreter/blockchain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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],
Expand All @@ -685,10 +679,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,
}
Expand All @@ -699,14 +692,16 @@ impl<Tx> MessageOutputCtx<'_, Tx> {
Tx: ExecutableTransaction,
{
let recipient_address = CheckedMemValue::<Address>::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 call_abi = CheckedMemRange::new_with_constraint(
recipient_address.end() as Word,
self.call_abi_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::MessageDataTooLong));
}

let msg_data_range = CheckedMemRange::new(self.msg_data_ptr, self.msg_data_len as usize)?;

let recipient = recipient_address.try_from(self.memory)?;

Expand All @@ -716,7 +711,7 @@ impl<Tx> 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(
Expand All @@ -725,7 +720,7 @@ impl<Tx> MessageOutputCtx<'_, Tx> {
*sender,
recipient,
self.amount_coins_to_send,
call_abi,
msg_data,
);

append_receipt(
Expand Down
50 changes: 25 additions & 25 deletions fuel-vm/src/interpreter/blockchain/smo_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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,
Expand All @@ -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: 0,
msg_data_len: 1,
amount_coins_to_send: 0,
..Default::default()
} => matches Ok(Output { .. })
Expand All @@ -51,18 +51,18 @@ impl Default for Input {
#[test_case(
Input {
recipient_mem_address: 0,
call_abi_len: 0,
message_output_idx: 0,
msg_data_ptr: 0,
msg_data_len: 0,
amount_coins_to_send: 0,
..Default::default()
} => Err(RuntimeError::Recoverable(PanicReason::MemoryOverflow))
; "Call abi can't be zero"
)]
#[test_case(
Input {
recipient_mem_address: Word::MAX,
call_abi_len: 1,
message_output_idx: 0,
recipient_mem_address: 0,
msg_data_ptr: Word::MAX,
msg_data_len: 1,
amount_coins_to_send: 0,
max_message_data_length: Word::MAX,
..Default::default()
Expand All @@ -71,31 +71,31 @@ impl Default for Input {
)]
#[test_case(
Input {
recipient_mem_address: VM_MAX_RAM - 64,
call_abi_len: 100,
message_output_idx: 0,
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,
call_abi_len: 101,
message_output_idx: 0,
recipient_mem_address: 0,
msg_data_ptr: 0,
msg_data_len: 101,
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(
Input {
recipient_mem_address: 400,
call_abi_len: 10,
message_output_idx: 0,
msg_data_ptr: 0,
msg_data_len: 10,
amount_coins_to_send: 30,
balance: [(AssetId::zeroed(), 29)].into_iter().collect(),
..Default::default()
Expand All @@ -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,
Expand All @@ -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 })
Expand Down
2 changes: 1 addition & 1 deletion fuel-vm/src/interpreter/executors/instruction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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))?;
}

Expand Down
25 changes: 19 additions & 6 deletions fuel-vm/src/tests/blockchain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::<u8>(), rng.gen::<u8>()];

#[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, 1), // send the whole data buffer
op::movi(0x12, 0), // tx output idx
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)
];

Expand Down Expand Up @@ -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() {
Expand Down

0 comments on commit d313654

Please sign in to comment.