dev: make BuiltinRunner::included public

[enitrat]

Make BuiltinRunner::included public

Description

I made this function public. I need this because when a specific function is ran in proof_mode, that function only takes N out of M builtins, where M is the total amount of builtins for the layout. Then, I can check if a builtin is included (among N) or not, and if it's not, instantly call final_stack and write it to the execution_public_memory so that I can generate a valid proof.

Description of the pull request changes and motivation.

Checklist

• Linked to Github Issue
• Unit tests added
• Integration tests added.
• This change requires new documentation.
  • Documentation has been added/updated.
  • CHANGELOG has been updated.

[JulianGCalderon]

Hi @enitrat! Could you update the changelog?

[JulianGCalderon]

There are some conflicts blocking the CI run. Could you resolve them?

[JulianGCalderon]

Hi @enitrat ! Could you give permission to edit the branch? That way we can fix the conflicts ourselves when changelog is updated, instead of making you do it.
pd: if you cannot, could you merge it with latest main?

[enitrat]

Hey @gabrielbosio @JulianGCalderon anything required from me here?

[JulianGCalderon]

Hi @enitrat! There are some conflicts in this branch. Could you allow us to push changes to it, so that we can resolve them?

[enitrat]

Hi @JulianGCalderon sorry but I don't see the "Allow edits from maintain" option on github :( Not sure why it's not showing here. I fixed the conflicts myself

[enitrat]

I don't see the "Allow edits from maintain" option on github

I think it's because my branch is made on an organization repo, not a personnal one 🤔

[gabrielbosio]

Hi, @enitrat, I'm not sure I understand why you need to call included before calling final_stack because final_stack already calls included internally.

[enitrat]

I want to call final_stack on any builtin that's not included in the program straight in the beginning, so that I don't have to include it in my entrypoint.

[gabrielbosio]

Ok, but if you call BuiltinRunner::final_stack, that function will call BuiltinRunner::included and run the branch that corresponds to non-included builtins, so I don't know why calling BuiltinRunner::included before would change the behavior.

[enitrat]

Ok, but if you call BuiltinRunner::final_stack, that function will call BuiltinRunner::included and run the branch that corresponds to non-included builtins, so I don't know why calling BuiltinRunner::included before would change the behavior.

I only want to run final_stack on builtins that are not included - because then I know that this builtin will not be used at all. To know if a builtin is included or not, I need to call included. If I call final_stack on a builtin that I'm going to use in my program, it will crash the run.

[gabrielbosio]

I want to call final_stack on any builtin that's not included in the program straight in the beginning, so that I don't have to include it in my entrypoint.

@enitrat if the idea is to call a function of a Cairo program, why not including all the builtins of the layout like in this test?

[enitrat]

because I have hundreds of different functions, called thousands of times during the execution of my program, all unit-tested by invoking them directly, and each builtin that's passed as argument is two more steps per call which is not something I want to accept

[JulianGCalderon]

Hi @enitrat!

Whether a builtin is included or not depends on: (see initialize_builtins)

• The current layout
• The current program
• Whether proof mode is enabled or not

If you have this information, you could calculate it manually as a workaround. Is doing this a possibility?

[enitrat]

If you have this information, you could calculate it manually as a workaround. Is doing this a possibility?

I don't understand why I would have to go through a workaround, when there already exists a function that, given the current layout, the program, and whether proof mode is enabled of not, returns whether the builtin is included or not. This is essentially the same as me re-coding the exact same function locally, which is not something I want to do.

At this point it would be easier for me to just fork the cairo vm and add a pub keyword than doing this :/ I don't understand why so much pushback

[gabrielbosio]

This PR makes the API of the VM bigger which is something we need to take care of because we don't know what implications this change may have long term in terms of increase of complexity and decrease of maintainability.
Also, exposing a function in the API of the VM means that all changes we may have to do to the related functionality (meaning interface or implementation) must be backwards compatible in minor versions and patches.
That's why a change like this, although is small in lines of code, might be bigger than it seems at the moment.

This is essentially the same as me re-coding the exact same function locally, which is not something I want to do.

The function we're talking about in this discussion might change its logic in the future. Even that function may be removed in the future (who knows, it's not public so a refactor like that can happen), so implementing logic in an module that uses the VM might seem like re-coding but we're talking about code in the VM that's internal and not accessible by external modules for the sake of simplicity and maintainability.

[enitrat]

Ok, then I will just keep using my fork :/