axone-protocol / axoned

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

🧠 Logic: Lack of information when `ErrorOutOfGas` in interpreter #692

Closed bdeneux closed 1 month ago

bdeneux commented 2 months ago

πŸ“ Description

When executing a predicate that results in an ErrorOutOfGas, the error message provided is misleading and lacks clarity. Specifically, the error message returned is a generic panic message (panic: {ValuePerByte}) that does not accurately reflect the underlying issue of running out of gas during the predicate's execution.

πŸ”Ž Example to Illustrate the Issue

Executing the following query results in an error message that does not clearly indicate the ErrorOutOfGas condition:

axoned query logic ask "findall(Pred, bank_balances(T, U), Pred)."

The response includes a vague error message:

answer:
  has_more: false
  results:
  - error: 'panic: {ValuePerByte}'
    substitutions: []
  variables:
  - Pred
  - T
  - U
gas_used: "100038"
height: "10194"
user_output: ""

The error message - error: 'panic: {ValuePerByte}' is not informative regarding the actual issue, which is an ErrorOutOfGas encountered during the retrieval of all balances within the interpreter.

βš™οΈ Technical Explanation and Current Handling

The mechanism for catching an ErrorOutOfGas is implemented but only triggers if the gas consumption limit is reached based on the predicate cost prior to the execution of the requested predicate.

https://github.com/axone-protocol/axoned/blob/279b2ee73f89bd648e3489821b8dcce209665d35/x/logic/keeper/interpreter.go#L108-L117

However, this handling does not account for gas exhaustion that occurs during the execution of a predicate. As a result, when the gas limit is exceeded within the predicate execution, the error reported is the panic description catch by prolog interpreter.

FYI @ccamel @amimart

amimart commented 2 months ago

Good catch! Indeed since this PR on the VM side panic are managed as prolog errors, what do you propose?

We can parse the prolog result to check if there is a panic and then check gas consumption to see if that was the failure source.

An alternative would be to consider panics in the prolog VM as exceptional behaviour managed outside the VM and make the call to Solutions.Next() panic as proposed in this PR: https://github.com/ichiban/prolog/pull/288/files. But that force us to make some changes in the VM.

The more we dig the more I think we should adapt radically the VM to our needs πŸ™„ ..

bdeneux commented 2 months ago

@amimart Exactly ! I've thinking about this solution for now, like it was done before https://github.com/axone-protocol/axoned/pull/555/files#diff-61dd11af86fdedcee9051d402c66b29ba0eb03373e9f20c4c9feb5dec6bc6e59L101-L104. It could be a good starting point without changes in VM; But, as you said, VM is beginning to show its limits :p.

PR in progress : #694