Contract method with struct arguments don't work with IR

[mohammadfawaz]

Here's an example:

contract;

pub struct Inner {
    foo: u64
}

pub struct StructOne {
    inn: Inner,
}

pub struct StructTwo {
    foo: u64,
}

abi MyTest {
    fn something(input1: StructOne, input2: StructTwo) -> u64;
}

impl MyTest for Contract {
    fn something(input1: StructOne, input2: StructTwo) -> u64 {
        let v = input1.inn.foo + input2.foo; 
        v + 1
    }    
}

with the following test harness:

use fuel_tx::Salt;
use fuels_abigen_macro::abigen;
use fuels_contract::{contract::Contract, parameters::TxParameters};
use fuels_signers::util::test_helpers;

abigen!(
    IncTest,
    "out/debug/proj-abi.json"
);

#[tokio::test]
async fn mint() {
    let salt = Salt::from([0u8; 32]);
    let compiled =
        Contract::load_sway_contract("out/debug/proj.bin", salt)
            .unwrap();

    let (provider, wallet) = test_helpers::setup_test_provider_and_wallet().await;
    let id = Contract::deploy(&compiled, &provider, &wallet, TxParameters::default())
        .await
        .unwrap();

    let instance = IncTest::new(id.to_string(), provider, wallet);

    let param_one = StructOne { inn: Inner { foo : 42 } };
    let param_two = StructTwo { foo: 42 };

    let res = instance.something(param_one, param_two).call().await.unwrap();

    assert_eq!(res.value, 42 + 42 + 1);
}

The assert fails.

The problem is that nested structs are represented as pointers to pointers to pointers etc.. instead of just a pile of data with offsets (which is what the old codegen used to do). The SDK does not currently expect that. We should decide what's best to do here.

• Either update the compiler to match the old codegen.
• Or figure out how to teach the SDK about the new data layout.

Speaking with @otrho , the best way forward seems to be a revamp of how extract_value works and maybe replace it with a combination of get_ptr and store.

Edit: fix a bug in the SDK code.

[otrho]

I tried out the test above and yep, it fails. But it seems to me it's still just because the SDK is special casing structs to be by value when their size is 1 word. Each of the structs above are only 1 word in size and are passed around by value in the original ASM.

As soon as you add second fields to these structs (and ignore them) the test starts passing.

So I don't think the solution is anything more than to fix for always-by-reference structs in FuelLabs/fuels-rs#201

Am I still missing something?

[mohammadfawaz]

Edit... misread Toby's comment.. Checking again.

[mohammadfawaz]

You're right.. I tried various things and the moment you pass anything larger than a word, the test passes, regardless of how many arguments you have. However, something still doesn't add up because the way this check works: https://github.com/FuelLabs/fuels-rs/blob/7fda121547ee28de91145f8e8034db7e02b79b48/packages/fuels-contract/src/contract.rs#L341 doesn't seem to match the behaviour above, so maybe something else is going on.

[mohammadfawaz]

Ahh this is coming down to the checks we have in the new codegen regarding the size of the data, instead of checking the data type.

For this simple test:

contract;

pub struct StructOne {
    foo: u64,
    bar: u64,
}

abi MyTest {
    fn something(input1: StructOne) -> u64;
}

impl MyTest for Contract {
    fn something(input1: StructOne) -> u64 {
        input1.foo
    }    
}

If we remove bar, we get the intermediate assembly on the left. If we keep it, we get the assembly on the right. The assembly on the left doesn't work.

That's why I was seeing the chain of lw for nested structs. It just happened that the structs I was working with all had size 1 word...

Let me try to fix this.