script_debug: add C++ EvalScript hook with fExec

[febyeji]

• Adds script execution debug callback infrastructure to the C API, building on script_debug branch.
• Two call sites in the EvalScript loop invoke a registered callback with stack state, script bytes, opcode position, and an fExec

[sedited]

Thank you for your contribution! Can I ask you to do the following:

• Split the commit into two, one for the required changes in the C and C++ code, and another for adapting the Rust bindings
• Add an example binary in another commit in the style of 2e70d80 to allow us to test it out a bit.
• Add a test to check that the execution frames are propagated correctly
• I would like to feature gate this, maybe behind a "script_debug" feature. Ideally we'd pass the required ENABLE_SCRIPT_DEBUG flag as a compiler flag only if the feature is enabled explicitly by the user.
• Can you target the feature_script_debug branch instead of master for this PR? If we make progress here, that would allow us to feed the work into that feature branch for now.

[sedited]

Thanks, the feature gate seems to be working nicely.

[febyeji]

@sedited Thank you for the review! I applied your comments to new commits.

[sedited]

@febyeji did you forget to push those? I am not seeing the changes here.

[febyeji]

@sedited Sorry, I didn't push correctly. 2959888 is the last commit that I added.

Now I see that concurrent testing for script_debug triggers an error and --test-threads=1 flag fixes it. However, I wonder if we need to make each test thread only sees its own callback by changing global static g_script_debug_callback in this PR.

[sedited]

However, I wonder if we need to make each test thread only sees its own callback by changing global static g_script_debug_callback in this PR.

Mmh, I'm not sure we can fix this on a fundamental level. My impression is that your approach with filtering is as good as we can do here. There is just no way of injecting contextual information that would differentiate between a debug call and a normal call. Honestly, Or do you have another suggestion? I don't think this is a blocker though.

I would however suggest protecting the registration of the callback with a mutex.

[febyeji]

There is just no way of injecting contextual information that would differentiate between a debug call and a normal call. Honestly, Or do you have another suggestion? I don't think this is a blocker though.

I am happy to leave as it is.

I would however suggest protecting the registration of the callback with a mutex.

Thank you, I replaced AtomicBool with Mutex to protect the callback registration. I also added a few concurrency tests. 0c7f40

[sedited]

Thanks for taking my suggestions, this looks about ready. Can you squash the parallel verification fixes? I think we'll just deal with running them with --test-threads=1, that seems acceptable to me for a debug-only tool.

Are you keen to carry on with this? A richer script debugger in the style of btcdeb is what I originally had in mind for this project. If you look at it, it has not received any maintenance in a while. I think that is because it is tightly coupled to Bitcoin Core's code and tooling. The approach here is very lightweight: We just sprinkle a couple of macros in and get a full execution trace back. I think it is also superior to other similar "script debugging" projects that re-implement the entire script interpreter, which often introduces bugs.

[febyeji]

Thank you for the review! I've squashed the commits.

Are you keen to carry on with this? A richer script debugger in the style of btcdeb is what I originally had in mind for this project.

I'm keen to carry on with this. Do you have thoughts on what features to prioritize next?

[sedited]

Do you have thoughts on what features to prioritize next?

I'll merge this into the feature branch (once the CI passes), then I think there are a few things that could be worked on:

• Developing a full debugger beyond just printing frames. If you want some inspiration, this is btcdeb's current interface: https://github.com/bitcoin-core/btcdeb/blob/master/doc/btcdeb.md . I think we can do better than that now. Maybe a tui where you also have the possibility of also editing the transaction and not just the singular scripts would be nice.
• I saw that in the issue you were discussing surfacing error codes from the execution. This is something that could be added directly to the kernel API and useful outside of the debug hooks.
• Identify any places where the macro should also be placed in to catch every step of execution.