Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement SMO instruction #159

Merged
merged 4 commits into from
Jul 4, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 56 additions & 2 deletions src/interpreter/blockchain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,9 @@ use crate::error::RuntimeError;
use crate::storage::InterpreterStorage;

use fuel_asm::PanicReason;
use fuel_tx::Input;
use fuel_types::{bytes, Address, AssetId, Bytes32, Bytes8, ContractId, RegisterId, Word};
use fuel_tx::{Input, Output, Receipt};
use fuel_types::bytes::{self, Deserializable};
use fuel_types::{Address, AssetId, Bytes32, Bytes8, ContractId, RegisterId, Word};

use std::mem;

Expand Down Expand Up @@ -308,4 +309,57 @@ where

self.inc_pc()
}

pub(crate) fn message_output(&mut self, a: Word, b: Word, c: Word, d: Word) -> Result<(), RuntimeError> {
if b > self.params.max_message_data_length
|| b > MEM_MAX_ACCESS_SIZE
|| a > VM_MAX_RAM - b - Address::LEN as Word
{
return Err(PanicReason::MemoryOverflow.into());
}

let idx = c;
let amount = d;

// Safety: checked len
let recipient = unsafe { Address::from_slice_unchecked(&self.memory[a as usize..a as usize + Address::LEN]) };
if recipient == Address::zeroed() {
return Err(PanicReason::ZeroedMessageOutputRecipient.into());
}

let offset = self
.tx
.output_offset(c as usize)
.map(|ofs| ofs + self.tx_offset())
.ok_or(PanicReason::OutputNotFound)?;

// halt with I/O error because tx should be serialized correctly into vm memory
let output = Output::from_bytes(&self.memory[offset..])?;

// amount isn't checked because we are allowed to send zero balances with a message
if !matches!(output, Output::Message { recipient, .. } if recipient == Address::zeroed()) {
return Err(PanicReason::NonZeroMessageOutputRecipient.into());
}

// validations passed, perform the mutations

self.base_asset_balance_sub(amount)?;

let fp = self.registers[REG_FP] as usize;
let txid = self.tx_id();
let data = a as usize + Address::LEN;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe this should use Bytes32::LEN like on line 316 for consistency.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

True, they should be the same. The concrete type is in fact Address, instead of raw bytes. Better to use that so its clear what is expected of this register:

7bd5789

let data = &self.memory[data..data + b as usize];
let data = data.to_vec();

// Safety: $fp is guaranteed to contain enough bytes
let sender = unsafe { Address::from_slice_unchecked(&self.memory[fp..fp + Address::LEN]) };

let message = Output::message(recipient, amount);
let receipt = Receipt::message_out_from_tx_output(txid, idx, sender, recipient, amount, data);

self.set_output(idx as usize, message)?;
self.append_receipt(receipt);

self.inc_pc()
}
}
8 changes: 5 additions & 3 deletions src/interpreter/executors/instruction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,11 +36,13 @@ where
// Should execute `lo` only if there is no rupture in the flow - that means
// either a breakpoint or some instruction that would skip `lo` such as
// `RET`, `JI` or `CALL`
if self.registers[REG_PC] == pc && state.should_continue() {
let result = if self.registers[REG_PC] == pc && state.should_continue() {
adlerjohn marked this conversation as resolved.
Show resolved Hide resolved
self.instruction(lo)
} else {
Ok(state)
}
};

result
}

/// Execute a provided instruction
Expand Down Expand Up @@ -287,7 +289,7 @@ where

OpcodeRepr::SMO => {
self.gas_charge(GAS_SMO)?;
todo!();
self.message_output(a, b, c, d)?;
}

OpcodeRepr::ALOC => {
Expand Down
30 changes: 18 additions & 12 deletions src/interpreter/initialization.rs
Original file line number Diff line number Diff line change
Expand Up @@ -88,20 +88,25 @@ impl<S> Interpreter<S> {

// compute the initial free balances for each asset type
pub(crate) fn initial_free_balances(&self) -> Result<HashMap<AssetId, Word>, InterpreterError> {
let base_asset = AssetId::default();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Orthogonal to this PR, but I'm thinking the base asset's ID should be a parameter in the VM. It's not necessarily guaranteed to be AssetId::default() always. Both the specs and the implementation should be modular in this regard. Can you file an issue to track changing?

let mut balances = HashMap::<AssetId, Word>::new();

// Add up all the inputs for each asset ID
for (asset_id, amount) in self.tx.inputs().iter().filter_map(|input| match input {
Input::CoinPredicate { asset_id, amount, .. } | Input::CoinSigned { asset_id, amount, .. } => {
Some((asset_id, amount))
}
_ => None,
}) {
*balances.entry(*asset_id).or_default() += amount;
}
self.tx
.inputs()
.iter()
.filter_map(|i| match i {
Input::CoinPredicate { asset_id, amount, .. } | Input::CoinSigned { asset_id, amount, .. } => {
Some((*asset_id, *amount))
}
Input::MessageSigned { amount, .. } | Input::MessagePredicate { amount, .. } => {
Some((base_asset, *amount))
}
_ => None,
})
.for_each(|(asset_id, amount)| *balances.entry(asset_id).or_default() += amount);

// Reduce by unavailable balances
let base_asset = AssetId::default();
if let Some(base_asset_balance) = balances.get_mut(&base_asset) {
// calculate the fee with used metered bytes + gas limit
let factor = self.params.gas_price_factor as f64;
Expand All @@ -124,19 +129,20 @@ impl<S> Interpreter<S> {
let fee = bytes.checked_add(gas).ok_or(ValidationError::ArithmeticOverflow)?;

// subtract total fee from base asset balance
*base_asset_balance =
let deducted_balance =
base_asset_balance
.checked_sub(fee)
.ok_or(ValidationError::InsufficientFeeAmount {
expected: fee,
provided: *base_asset_balance,
})?;

*base_asset_balance = deducted_balance;
}

// reduce free balances by coin and withdrawal outputs
// reduce free balances by coin
for (asset_id, amount) in self.tx.outputs().iter().filter_map(|output| match output {
Output::Coin { asset_id, amount, .. } => Some((asset_id, amount)),
Output::Message { .. } => todo!(),
adlerjohn marked this conversation as resolved.
Show resolved Hide resolved
_ => None,
}) {
let balance = balances
Expand Down
10 changes: 10 additions & 0 deletions src/interpreter/internal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,11 @@ impl<S> Interpreter<S> {
Ok(())
}

/// Reduces the unspent balance of the base asset
pub(crate) fn base_asset_balance_sub(&mut self, value: Word) -> Result<(), RuntimeError> {
self.external_asset_id_balance_sub(&AssetId::default(), value)
}

/// Increase the variable output with a given asset ID. Modifies both the referenced tx and the
/// serialized tx in vm memory.
pub(crate) fn set_variable_output(
Expand Down Expand Up @@ -239,6 +244,11 @@ impl<S> Interpreter<S> {
pub(crate) const fn tx_offset(&self) -> usize {
self.params.tx_offset()
}

pub(crate) fn tx_id(&self) -> &Bytes32 {
// Safety: vm parameters guarantees enough space for txid
unsafe { Bytes32::as_ref_unchecked(&self.memory[..Bytes32::LEN]) }
}
}

#[cfg(all(test, feature = "random"))]
Expand Down
2 changes: 1 addition & 1 deletion src/interpreter/transaction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ impl<S> Interpreter<S> {
.tx
.witnesses()
.get(b as usize)
.ok_or(PanicReason::OutputNotFound)
.ok_or(PanicReason::WitnessNotFound)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like an orthogonal bugfix. Ideally make a separate PR for this.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thats true, I just spotted this nit while debugging

.map(|witness| witness.serialized_size() as Word)?;

self.inc_pc()
Expand Down
6 changes: 6 additions & 0 deletions src/memory_client.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
//! In-memory client implementation

use crate::backtrace::Backtrace;
use crate::state::StateTransitionRef;
use crate::storage::MemoryStorage;
use crate::transactor::Transactor;

Expand Down Expand Up @@ -49,6 +50,11 @@ impl<'a> MemoryClient<'a> {
self.transactor.receipts()
}

/// State transition representation after the execution of a transaction.
pub const fn state_transition(&self) -> Option<&StateTransitionRef<'_>> {
self.transactor.state_transition()
}

/// Execute a transaction.
///
/// Since the memory storage is `Infallible`, associatively, the memory
Expand Down
99 changes: 93 additions & 6 deletions tests/blockchain.rs
Original file line number Diff line number Diff line change
@@ -1,13 +1,14 @@
use fuel_crypto::Hasher;
use fuel_crypto::{Hasher, SecretKey};
use fuel_tx::{StorageSlot, TransactionBuilder};
use fuel_types::bytes;
use fuel_vm::consts::*;
use fuel_vm::prelude::*;
use fuel_vm::script_with_data_offset;
use fuel_vm::util::test_helpers::TestBuilder;
use rand::rngs::StdRng;
use rand::{Rng, SeedableRng};

use fuel_tx::StorageSlot;
use fuel_vm::script_with_data_offset;
use fuel_vm::util::test_helpers::TestBuilder;
use fuel_vm::consts::*;
use fuel_vm::prelude::*;

use std::mem;

const WORD_SIZE: usize = mem::size_of::<Word>();
Expand Down Expand Up @@ -465,3 +466,89 @@ fn can_read_state_from_initial_storage_slots() {
}
}
}

#[test]
fn smo_instruction_works() {
fn execute_test<R>(rng: &mut R, balance: Word, amount: Word, data: Vec<u8>)
where
R: Rng,
{
let mut client = MemoryClient::default();

let gas_price = 1;
let gas_limit = 1_000_000;
let byte_price = 1;
let maturity = 0;

let secret = SecretKey::random(rng);
let sender = rng.gen();
let recipient = rng.gen();
let nonce = rng.gen();

let message = Output::message(Address::zeroed(), 0);

#[rustfmt::skip]
let script = vec![
Opcode::MOVI(0x10, 0), // set the txid as recipient
Opcode::MOVI(0x11, data.len() as Immediate24), // send the whole data buffer
Opcode::MOVI(0x12, 0), // tx output idx
Opcode::MOVI(0x13, amount as Immediate24), // expected output amount
Opcode::SMO(0x10,0x11,0x12,0x13),
Opcode::RET(REG_ONE)
];

let script = script.into_iter().collect();
let script_data = vec![];

let tx = TransactionBuilder::script(script, script_data)
.gas_price(gas_price)
.gas_limit(gas_limit)
.byte_price(byte_price)
.maturity(maturity)
.add_unsigned_message_input(&secret, sender, recipient, nonce, balance, data)
.add_output(message)
.finalize();

let params = client.params();
let block_height = 0;

tx.validate(block_height, params)
.expect("tx expected to be properly signed");

let txid = tx.id();
let receipts = client.transact(tx);

let success = receipts.iter().any(|r| match r {
Receipt::ScriptResult {
result: ScriptExecutionResult::Success,
..
} => true,
_ => false,
});

assert!(success);

let (recipient, transferred) = client
.state_transition()
.expect("tx was executed")
.tx()
.outputs()
.iter()
.find_map(|o| match o {
Output::Message { recipient, amount } => Some((recipient, *amount)),
_ => None,
})
.expect("failed to find message output");

assert_eq!(txid.as_ref(), recipient.as_ref());
assert_eq!(amount, transferred);
}

let rng = &mut StdRng::seed_from_u64(2322u64);

// check arbitrary amount
execute_test(rng, 1_000, 10, vec![0xfa; 15]);

// check message with zero amount
execute_test(rng, 1_000, 0, vec![0xfa; 15]);
}