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

Pass all gas parameter in ExternalCallContext #1184

Open
iFrostizz opened this issue Jul 23, 2024 · 3 comments · May be fixed by #1417
Open

Pass all gas parameter in ExternalCallContext #1184

iFrostizz opened this issue Jul 23, 2024 · 3 comments · May be fixed by #1417
Assignees

Comments

@iFrostizz
Copy link
Contributor

It is currently a bit unpractical to supply a constant max_units because some programs called externally may use more than others, and if we offload this to the parameters of the function then it adds more complexity. It would be nice to be able to pass an option to the current max_units field of the ExternalCallContext and in the case of this value being None, any unit that has not been spent would be refunded to the current context.

@richardpringle
Copy link
Contributor

We can probably use an Option<NonZeroU64> and just treat it as a u64 on the go side. If the value is 0, that means to take what's needed.

@iFrostizz iFrostizz self-assigned this Aug 19, 2024
@iFrostizz iFrostizz linked a pull request Aug 20, 2024 that will close this issue
@richardpringle
Copy link
Contributor

Instead of passing in an Option<NonZeroU64> directly, I think it would be better to use a builder pattern for both Gas and Value.

ctx
    .call_contract_builder()
    .max_fuel(42u64)
    .transfer_value(1_000_000u64)
    .call_function("<function_name>", args)?;

And we could keep

ctx.call_function("<function_name>", args)?;

as the default usage when you don't need to provide a limited amount of gas or value

@iFrostizz
Copy link
Contributor Author

When using constants, it's clear that passing 0 to forward all gas makes sense because the current behaviour would be not being able to execute anything but there could be footguns if the gas is dynamic and it would be harder to reason about when all the gas is forwarded vs what is passed. Currently done with an enum #1417 which avoids this pitfall but the builder pattern looks better.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants