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

Execution Error When Calling bal After Low-Level Call and Calling The Other Contract First #1535

Closed
bitzoic opened this issue Dec 11, 2023 · 0 comments · Fixed by #1536
Closed
Labels
bug Something isn't working

Comments

@bitzoic
Copy link
Member

bitzoic commented Dec 11, 2023

The following describes a bug resulting in an execution error when checking the balance of a contract after performing a low-level call transferring some tokens and calling the other contract first.

In our test case, we have two contracts:

  1. A dummy contract with a simple function:
contract;

abi DummyContract {
    fn dummy_function();
}

impl DummyContract for Contract {
    fn dummy_function() {
        let val = 0;
    }
}
  1. A contract that performs a low level call and transfers some value and has a balance function:
contract;

use std::{
    bytes::Bytes,
    context::this_balance,
    low_level_call::{
        call_with_function_selector,
        CallParams,
    },
};

abi CallerContract {
    // Just performs a low level call
    #[storage(read, write)]
    fn make_low_level_call(
        target: ContractId,
        function_selector: Bytes,
        calldata: Bytes,
        single_value_type_arg: bool,
        asset_id: AssetId,
        amount: u64,
        gas: u64,
    );
    // Just returns the balance
    fn balance(asset_id: AssetId) -> u64;
}

impl CallerContract for Contract {
    #[storage(read, write)]
    fn make_low_level_call(
        target: ContractId,
        function_selector: Bytes,
        calldata: Bytes,
        single_value_type_arg: bool,
        asset_id: AssetId,
        amount: u64,
        gas: u64,
    ) {
        // Create the CallParams
        let call_params = CallParams {
            coins: amount,
            asset_id: asset_id,
            gas: gas,
        };
        // Perform the low level call
        call_with_function_selector(
            target,
            function_selector,
            calldata,
            single_value_type_arg,
            call_params,
        );
    }

    fn balance(asset_id: AssetId) -> u64 {
        // Returns the balance
        this_balance(asset_id)
    }
}

If we call dummy_function() first, then we call make_low_level_call(), and finally calling balance() will result in the following error:

failures:

---- test_low_level_transfer stdout ----
thread 'test_low_level_transfer' panicked at tests/harness.rs:100:78:
called `Result::unwrap()` on an `Err` value: IOError(Custom { kind: Other, error: "Response errors; Transaction(0x4d9212eb422fd3c70cbeea07b59de5aafe1dbad5c05652a9e04b5a6e66b6c462) execution error: Storage(Storage error: LoadError([154, 179, 167, 175, 128, 242, 217, 166, 32, 27, 132, 68, 48, 89, 6, 197, 70, 71, 223, 248, 160, 158, 138, 142, 110, 114, 32, 84, 16, 67, 254, 83]))" })
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace


failures:
    test_low_level_transfer

NOTE:

  • Calling the balance() function before making the low-level call does not result in the error.
  • The balance is verified to be transferred in the test suite.
  • Not calling the dummy_function() first will not trigger the error.

Rust SDK test:

use fuels::{prelude::*, types::ContractId, core::codec::{calldata, fn_selector}};

// Load abi from json
abigen!(
    Contract(
        name = "CallerContract",
        abi = "out/debug/test_balances-abi.json"
    ),
    Contract(
        name = "DummyContract",
        abi = "../test_sdk/out/debug/test_sdk-abi.json"
    )
);

async fn get_contract_instance() -> (CallerContract<WalletUnlocked>, DummyContract<WalletUnlocked>, ContractId, ContractId, WalletUnlocked) {
    // Launch a local network and deploy the contract
    let mut wallets = launch_custom_provider_and_get_wallets(
        WalletsConfig::new(
            Some(1),             /* Single wallet */
            Some(1),             /* Single coin (UTXO) */
            Some(1_000_000_000), /* Amount per coin */
        ),
        None,
        None,
    )
    .await
    .unwrap();
    let wallet = wallets.pop().unwrap();

    let id = Contract::load_from(
        "./out/debug/test_balances.bin",
        LoadConfiguration::default(),
    )
    .unwrap()
    .deploy(&wallet, TxPolicies::default())
    .await
    .unwrap();

    let instance = CallerContract::new(id.clone(), wallet.clone());

    let call_id = Contract::load_from(
        "../test_sdk/out/debug/test_sdk.bin",
        LoadConfiguration::default(),
    )
    .unwrap()
    .deploy(&wallet, TxPolicies::default())
    .await
    .unwrap();

    let call_instance = DummyContract::new(call_id.clone(), wallet.clone());

    (instance, call_instance, id.into(), call_id.into(), wallet)
}

#[tokio::test]
async fn test_low_level_transfer() {
    let (caller_instance, dummy_instance, _id, dummy_id, wallet) = get_contract_instance().await;

    // Force transfer 10 BASE_ASSET_ID tokens to the caller contract
    wallet
        .force_transfer_to_contract(
            caller_instance.contract_id(),
            10,
            BASE_ASSET_ID,
            TxPolicies::default(),
        )
        .await
        .unwrap();

    // Check that the balance is indeed 10
    assert_eq!(caller_instance.methods().balance(BASE_ASSET_ID).call().await.unwrap().value, 10);

    // Call a function in the other contract, if we comment this out and we have no issue
    dummy_instance.methods().dummy_function().call().await.unwrap();    

    // Perform the low level call with value transfer
    let function_selector = Bytes(fn_selector!(dummy_function()));
    let calldata = Bytes(calldata!().unwrap());
    let forwarded_gas =  10_000_000;
    let single_value_type_arg = true;
    let amount = 10;
    let asset = BASE_ASSET_ID;
    let _ = caller_instance
        .methods()
        .make_low_level_call(
            dummy_id, 
            function_selector, 
            calldata,
            single_value_type_arg, 
            asset,
            amount,
            forwarded_gas,
        )
        .append_variable_outputs(1)
        .with_contract_ids(&[Bech32ContractId::from(dummy_id)])
        .call()
        .await;

    // This will pass, the balance of the caller contract is in fact 0.
    let contract_balances = wallet
        .provider()
        .unwrap()
        .get_contract_balances(&caller_instance.contract_id())
        .await
        .unwrap();
    let asset_balance = contract_balances.get(&BASE_ASSET_ID).unwrap();
    assert_eq!(*asset_balance, 0);

    // THIS FAILS!!!!!!!! We cannot access the balance of the contract.
    assert_eq!(caller_instance.methods().balance(BASE_ASSET_ID).call().await.unwrap().value, 0);
}

Tested using forc v0.48.1 and fuel-core v0.21.0

@bitzoic bitzoic added the bug Something isn't working label Dec 11, 2023
xgreenx added a commit that referenced this issue Dec 11, 2023
It was possible to remove the SMT nodes in the foreign contract because
it was not prefixed with the `ContractId`. The caller could use the same
storage key/asset ID with exactly the same values to delete the SMT
nodes in another contract.

Fixes #1535

---------

Co-authored-by: Hannes Karppila <[email protected]>
Co-authored-by: Brandon Vrooman <[email protected]>
crypto523 pushed a commit to crypto523/fuel-core that referenced this issue Oct 7, 2024
It was possible to remove the SMT nodes in the foreign contract because
it was not prefixed with the `ContractId`. The caller could use the same
storage key/asset ID with exactly the same values to delete the SMT
nodes in another contract.

Fixes FuelLabs/fuel-core#1535

---------

Co-authored-by: Hannes Karppila <[email protected]>
Co-authored-by: Brandon Vrooman <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant