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

🐛 Logic: Gas limit override tx limit #706

Closed
amimart opened this issue Jul 23, 2024 · 1 comment · Fixed by #708
Closed

🐛 Logic: Gas limit override tx limit #706

amimart opened this issue Jul 23, 2024 · 1 comment · Fixed by #708
Assignees
Labels
bug 🐞 bugs and defects

Comments

@amimart
Copy link
Member

amimart commented Jul 23, 2024

Issue description

In the logic module the max_gas on-chain parameter is used to limit the gas consumption allowed when executing the Ask query.

In the context of a transaction execution the gas consumption limit is directly expressed in the transaction, so when the logic ask query is used in a transaction (e.g. called by a smart contract) having this parameter overrides the limit set at the transaction level and allows this way unexpected behaviours such as transaction gas over consumption, or transaction failing prematurely for false out of gas errors.

This issue is consensus safe in the sense that it is a deterministic behaviour, but can lead to transaction fees not reflecting the real underlying execution cost.

Proposed solution

I propose to remove this parameter, keep the gas consumption tracking while letting the ask logic query be unlimited in terms of gas consumption by default. If a gas limit should be positioned it should be done when forging the sdk context before calling the query.

Regarding the node operator perspective the gas limit being restricted by the transaction is quite normal, but as an RPC provider you may want to be able to restrict such queries as well, this already existing flag allows to set a global gas limit to all REST/gRPC queries and can serve as a replacement solution.

@bdeneux @ccamel

@amimart amimart added the bug 🐞 bugs and defects label Jul 23, 2024
@amimart amimart self-assigned this Jul 23, 2024
@bdeneux
Copy link
Contributor

bdeneux commented Jul 24, 2024

@amimart Yes, I think you're right! We should leverage the existing and common gas management in the SDK to avoid any misunderstandings in usage.

For nodes that expose REST and gRPC, we should consider adding a recommendation in the documentation to use the global gas limit for all REST/gRPC queries. Without this, a simple query could potentially block the node. However, it is ultimately the responsibility of the node host.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐞 bugs and defects
Projects
Status: ✅ Done
Development

Successfully merging a pull request may close this issue.

2 participants