Skip to content

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

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

Make transaction dependency estimation an opt-in feature #1536

Closed
nedsalk opened this issue Dec 18, 2023 · 1 comment
Closed

Make transaction dependency estimation an opt-in feature #1536

nedsalk opened this issue Dec 18, 2023 · 1 comment
Labels
feat Issue is a feature

Comments

@nedsalk
Copy link
Contributor

nedsalk commented Dec 18, 2023

Transaction dependency estimation via provider.estimateTxDependencies is currently done always before a transaction is sent to a node. This causes unnecessary round-trips to the node and strain to the network in cases where this estimation is unnecessary.

Instead of calling it on every call, we should make it an opt-in feature in cases where the user actually needs it. For more info on when this is needed, one can refer to the excellent documentation of fuels-rs on this topic.

Regarding the API, fuels-rs uses method chaining because rust allows for this pattern:

let response = contract_methods
    .mint_then_increment_from_contract(called_contract_id, amount, address)
    .estimate_tx_dependencies(Some(2))
    .await? // <----- this thing right here
    .call()
    .await?;

This API would be great but we'd have to write some questionable Promise-related code to allow it:

let response = await contract_methods
    .mint_then_increment_from_contract(called_contract_id, amount, address)
    .estimate_tx_dependencies(Some(2))
    .call();

Due to that, I'm more in favor of making it part of the call params:

const response = await contract_methods
.mint_then_increment_from_contract(called_contract_id, amount, address)
.call({
  estimateTxDependencies: 10 // boolean | number indicating how many round trips are allowed
})

fuels-rs supports this for single contract calls, multi-calls, as well as script calls. They did it via a trait called TxDependencyExtension.

@nedsalk nedsalk added the feat Issue is a feature label Dec 18, 2023
@arboleya
Copy link
Member

@arboleya arboleya added p1 and removed p1 labels Jul 22, 2024
@arboleya arboleya added the temp:notion label Sep 8, 2024 — with Linear
@arboleya arboleya added the temp-linear label Sep 8, 2024 — with Linear
@FuelLabs FuelLabs locked and limited conversation to collaborators Nov 6, 2024
@danielbate danielbate converted this issue into discussion #3376 Nov 6, 2024

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

Labels
feat Issue is a feature
Projects
None yet
Development

No branches or pull requests

2 participants