Add more test cases for transfer and transfer_output

[MitchTurner]

#510

[MitchTurner]

The existing test for for transfer_output was simply wrong. It just happened to work because so many of the test parameters were the same :(

[xgreenx]

You can actually replace all new functions with CheckedMemConstLen and read, it should be shorter and more readable and will unify the code

[MitchTurner]

Went with

fn get_address_from_memory(
    offset: Word,
    memory: &[u8; MEM_SIZE],
) -> Result<Address, RuntimeError> {
    get_from_memory::<{ Address::LEN }, Address>(offset, memory)
}

fn get_from_memory<const SIZE: usize, T>(
    offset: Word,
    memory: &[u8; MEM_SIZE],
) -> Result<T, RuntimeError>
where
    T: From<[u8; SIZE]>,
{
    let range = CheckedMemConstLen::<SIZE>::new(offset)?;
    let bytes = range.read(memory);
    let value = T::from(*bytes);
    Ok(value)
}

Which I'm happy with.

BUT

I wish that LEN was a associated const for some trait and then we could let the type checker fill in the generics for us:

fn get_from_memory<T>(
    offset: Word,
    memory: &[u8; MEM_SIZE],
) -> Result<T, RuntimeError>
where
    T: WithSize
    T: From<[u8; <T as WithSize>::SIZE]>,
{
    let range = CheckedMemConstLen::<<T as WithSize>::SIZE>::new(offset)?;
    let bytes = range.read(memory);
    let value = T::from(*bytes);
    Ok(value)
}

I would add a trait for it, but then we'd need to import that trait everywhere we want the LEN and that's not worth it.

[Dentosal]

I find this even cleaner, as it's modular yet still readable:

let contract_id = ContractId::from(read_bytes(self.memory, contract_id_addr)?);

[MitchTurner]

Agreed. I added that change. I'm keeping the helper functions though because I prefer my code to read declaratively.

[xgreenx]

Hmm, but I think it is already descriptive enough: "Read bytes from memory with offset as ContractId"=)

let contract_id = ContractId::from(read_bytes(memory, offset)?);

[MitchTurner]

Okay. 2v1. I'll change it.

[xgreenx]

You can create the type itself. The same is applicable to ContractId and Adress=) And instead of &ASSET_ID[..] you can use ASSET_ID.as_ref()

Suggested change

	const ASSET_ID: [u8; AssetId::LEN] = [2u8; AssetId::LEN];
	const ASSET_ID: AssetId = AssetId::new([2u8; AssetId::LEN]);

[xgreenx]

It would be nice if you tested something with contract_id_offset and asset_id_offset arguments. Otherwise, it is strange why do we have them=D

[MitchTurner]

I added some simple tests that swap them. I think that's good enough because it shows that it actually uses memory. There are infinite other tests we could write including the offsets and none of them seem particularly interesting off the top of my head.

[xgreenx]

It would be nice to test some bad cases like memory overflow=)