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

Proposal: Coverage/Hooks API #3415

Open
Furetur opened this issue Apr 18, 2024 · 3 comments
Open

Proposal: Coverage/Hooks API #3415

Furetur opened this issue Apr 18, 2024 · 3 comments
Labels
feature Completely new functionality I2 Regular impact S2 Regular significance U4 Nothing urgent

Comments

@Furetur
Copy link

Furetur commented Apr 18, 2024

Hello!

This is an RFC. Its goal is to discuss and approve the design of a new feature. My colleagues and I aim to implement this feature, once an agreement is reached.

Feature request

Background: Currently, there is no way to collect coverage of tests that are run on the VM. There already has been an issue on this subject and a prototype implementation (#2657). That implementation suffered from a bug in debug info which just got fixed (#3412). Now we can move further

Feature: In my view, the VM would really benefit from an Execution Hooks API that would allow to programmatically register callbacks that would be invoked for each executed instruction. Coverage collection would just be a special use case of this API.

Proposed design

I propose that we:

  1. Add a field that would store a hook callback to the VM struct:
type VM struct {
    ...
    executionHook func(scriptHash util.Uint160, offset int, opcode opcode.Opcode)
}
  1. Define a new constructor for the VM type, e.g. NewWithExecutionHook(callback), that would create an instance of VM with the specified callback
  2. Modify the step() method so that it calls the v.executionHook for each instruction
func (v *VM) step(ctx *Context) error {
+	instruction_offset := v.Context().nextip
	op, param, err := ctx.Next()
+	if v.executionHook != nil {
+		v.executionHook(v.GetCurrentScriptHash(), instruction_offset, op)
+	}
	if err != nil {
		v.state = vmstate.Fault
		return newError(ctx.ip, op, err)
	}
	return v.execute(ctx, op, param)
}
@Furetur Furetur added feature Completely new functionality I2 Regular impact labels Apr 18, 2024
@Furetur Furetur changed the title Proposal: coverage/instrumentation API Proposal: Coverage/Instrumentation API Apr 18, 2024
@roman-khimov roman-khimov added U4 Nothing urgent S2 Regular significance labels Apr 18, 2024
@Furetur Furetur changed the title Proposal: Coverage/Instrumentation API Proposal: Coverage/Hooks API Apr 18, 2024
@Furetur
Copy link
Author

Furetur commented Apr 18, 2024

I consider Execution Hooks API / Hooks API a better name for this feature, so I edited the issue

@ixje
Copy link
Contributor

ixje commented Apr 19, 2024

Just voicing my interest in this. We (COZ) would love if the end result of this implementation allows us to get the list instructions/offsets executed so we can map them back to our debug info.

@AnnaShaleva
Copy link
Member

AnnaShaleva commented Apr 22, 2024

The design LGTM, it's more generic than the one presented in 96b5d90, so vote up from my side to implement this proposal. Also, this execution hook may be used not only for the coverage calculations, but also for custom VM logger implementation that we've discussed with @roman-khimov.

the end result of this implementation allows us to get the list instructions/offsets executed

Can easily be reached with a custom execution hook that accumulates the set of instructions inside it (which also remains me some kind of Hooks-based logger that was recently implemented in EVM, ref. https://github.com/ethereum/go-ethereum/blob/1ec7af261223d6dad9370ee8263f86347b190bab/eth/tracers/logger/logger.go#L135). So this request can easily be implemented after the described proposal implementation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature Completely new functionality I2 Regular impact S2 Regular significance U4 Nothing urgent
Projects
None yet
Development

No branches or pull requests

4 participants