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

Handle messages that would result in execution error #332

Closed
aakoshh opened this issue Jun 26, 2023 · 1 comment · Fixed by consensus-shipyard/fendermint#130
Closed

Handle messages that would result in execution error #332

aakoshh opened this issue Jun 26, 2023 · 1 comment · Fixed by consensus-shipyard/fendermint#130

Comments

@aakoshh
Copy link
Contributor

aakoshh commented Jun 26, 2023

Apparently if there is no gas limit set on a message whatsoever, the execute_message method of the FVM fails, instead of returning an ApplyRet. I'm not sure how Lotus handles it, but we can do a multitude of things

  • encode the result as an ApplyRet in the FvmExecState, or
  • introduce a special error case checked in the interpreters (both query and signed are affected), or
  • use ABCI++ to filter such messages in the prepare_proposal, and reject such blocks in the process_proposal method

Not sure which is the best approach yet 🤔

@aakoshh aakoshh changed the title Reject blocks with messages that would run to error on the FVM Handle messages that would result in execution error Jun 26, 2023
@aakoshh
Copy link
Contributor Author

aakoshh commented Jun 26, 2023

I actually started with option 1) in consensus-shipyard/fendermint#128, but later thought option 3) would be better. Then I realised that the way I discovered this problem was with with an eth_call, which would not be filtered out with ABCI++

So the only option is to handle it with an error, and perhaps penalize the validator who put it into a block, or also use ABCI++ to never vote for a block which contains such messages.

We must also add this check to the check interpreter to get fast feedback, although, again, queries would not be affected by that.

@jsoares jsoares transferred this issue from consensus-shipyard/fendermint Dec 19, 2023
@jsoares jsoares closed this as not planned Won't fix, can't repro, duplicate, stale Mar 13, 2024
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