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

Support tx parameter overrides #158

Merged
merged 5 commits into from
Mar 18, 2022
Merged

Support tx parameter overrides #158

merged 5 commits into from
Mar 18, 2022

Conversation

digorithm
Copy link
Member

@digorithm digorithm commented Mar 17, 2022

This PR adds support for setting parameters to contract calls and deployments. These parameters are byte price, gas limit, and gas price for now.

It's done through a new method tx_params() that's chained to the generated ABI method:

let result = contract_instance
        .initialize_counter(42) // Build the ABI call
        .tx_params(Parameters::new(None, Some(1_000_000), None)) // <--- this here.
        .call() // Perform the network call
        .await
        .unwrap();

This doesn't break the way contract calls were made previously. i.e. initialize_counter(42).call(); (without .tx_params()) works just like before and, also just like before, it will use default values for these parameters. If the user wants to override the default values, then they can chain tx_params() to the method call.

However, this does break previous contract deployments, as the deployment method now requires these params to be set:

 let contract_id =
        Contract::deploy(&compiled, &provider, &wallet, Parameters::default())
            .await
            .unwrap();

Closes #138.

@digorithm digorithm added the enhancement New feature or request label Mar 17, 2022
@digorithm digorithm self-assigned this Mar 17, 2022
Copy link
Contributor

@adlerjohn adlerjohn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not the biggest fan of the name with_params and Parameters. My initial reaction to .call().with_params() is that those are parameters for the call, not the transaction. Maybe .tx_params() and TxParameters?

@digorithm
Copy link
Member Author

I'm not the biggest fan of the name with_params and Parameters. My initial reaction to .call().with_params() is that those are parameters for the call, not the transaction. Maybe .tx_params() and TxParameters?

Yeah, I wasn't attached to the with_params/params naming. Will update it soon.

fuels-abigen-macro/tests/harness.rs Outdated Show resolved Hide resolved
fuels-abigen-macro/tests/harness.rs Outdated Show resolved Hide resolved
fuels-abigen-macro/tests/test_projects/token-ops/Forc.toml Outdated Show resolved Hide resolved
Copy link
Contributor

@adlerjohn adlerjohn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, but will defer to @vnepveu

Copy link
Contributor

@iqdecay iqdecay left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some naming issues, 1/2 nits and 1/2 questions to make sure I understand. Otherwise great!!!

fuels-contract/src/constants.rs Outdated Show resolved Hide resolved
fuels-contract/src/constants.rs Outdated Show resolved Hide resolved
fuels-contract/src/contract.rs Outdated Show resolved Hide resolved
fuels-contract/src/contract.rs Outdated Show resolved Hide resolved
fuels-abigen-macro/tests/harness.rs Show resolved Hide resolved
fuels-abigen-macro/tests/harness.rs Show resolved Hide resolved
@digorithm digorithm merged commit 09af27c into master Mar 18, 2022
@digorithm digorithm deleted the rodrigo/call-params branch March 18, 2022 15:13
@digorithm digorithm mentioned this pull request Mar 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Add params to a Contract's .call() method
3 participants