axone-protocol / axoned

⛓️ Axone blockchain πŸ’«
https://axone.xyz
Apache License 2.0
162 stars 121 forks source link

πŸ› Logic: Gas limit override tx limit #706

Closed amimart closed 1 month ago

amimart commented 1 month ago

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

bdeneux commented 1 month ago

@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.