FuelLabs / fuels-ts

Fuel Network Typescript SDK
https://docs.fuel.network/docs/fuels-ts/
Apache License 2.0
44.26k stars 1.34k forks source link

Optimize `estimateTxDependencies` method #1763

Open Torres-ssf opened 6 months ago

Torres-ssf commented 6 months ago

The estimateTxDependencies automates the addition of missing OutputVariables, essential for Sway's transfer or mint functions.

With this, users don't need to specify the variableOutputs parameter:

const { transactionResult } = await contract
  .functions
  .mint_to_addresses(address, subId, mintAmount)
  .txParams({
    variableOutputs: 3, // <—————————— this
  })
  .call();

[!WARNING] While providing a nice DX, this approach may lead to multiple dry-run calls, potentially slowing down the transaction execution and representing more use of resources, which translates into more costs.

How it works:

  1. The method performs dry runs, checking output receipts for a Revert with val equal to TransferToAddressFailed. This error indicates a missing OutputVariable.
  2. If such a Revert receipt is detected, an OutputVariable is added, and another dry run is executed. This loop continues until no related revert receipt is found.

Consider a contract call minting assets to 3 addresses:

    fn mint_to_addresses(addresses: [Address; 3], sub_id: b256, mint_amount: u64) {
        assert(sub_id == TOKEN_1 || sub_id == TOKEN_2 || sub_id == TOKEN_3);
        let mut counter = 0;
        while counter < 3 {
            mint_to_address(addresses[counter], sub_id, mint_amount);
            counter = counter + 1;
        }
    }

Outcome:

  1. Since this function mints a coin to 3x addresses, it will need:
    • 3x OutputVariable
  2. This means the dry-run call will fail:
    • 3x times
  3. And succeed only in the:
    • 4th execution
  4. This will result in:
    • 4x total dry-runs

Proposed change:


Related:

nedsalk commented 6 months ago

Related (same?):

Dhaiwat10 commented 6 months ago

Does this still need to be fixed after #1767 ?

Torres-ssf commented 6 months ago

@Dhaiwat10 If I am not mistaken, for this one we are waiting for a possible update on fuel-core that will include on the dryRun response the amount of OutputVariables required by a TX when calling dryRun.

arboleya commented 6 months ago

@Torres-ssf Did we communicate this need to someone?

Ideally, we should mark this issue as blocked and add a link here to another issue on the fuel-core repo.

Torres-ssf commented 6 months ago

@arboleya @Dhaiwat10 I reckon this issue is blocked by https://github.com/FuelLabs/fuel-vm/issues/691

It seems the plan to implement this is only after mainnet.

arboleya commented 6 months ago

That was the missing issue, then.

Thanks for pointing that out.

nedsalk commented 6 months ago

Based on the issue's proposed changes (specifically "Discontinue automatic addition of OutputVariable"), this doesn't seem like blocked. This issue is concerned with not running transaction dependency estimation by default anymore, whereas the linked fuel-vm issue - if I understood it correctly - is concerned with getting all the dependencies in one call instead of requiring multiple calls:

The FuelVM may support dry run mode, in which all validity rules will not abort the execution immediately. Instead, we will keep a record of what rules are failing and provide feedback at the end of the execution with all problems.

We can already make dependency estimation and opt-in thing, it'd just break the apps of our consumers that didn't specify the appropriate outputs.

Torres-ssf commented 1 month ago

@nedsalk This is blocked.

The desired behavior, as outlined in this issue, is for the dry-run to indicate the number of variable outputs required for the transaction to be processed.

Implementing this feature will eliminate the need for multiple dry-runs to determine the necessary outputs.

We cannot expect users to always provide the correct number of variable outputs, especially for complex transactions involving multiple parties ( wallet connectors ).