fix: properly handle error paths when reverting (explicit or otherwise)

README.md

@@ -30,14 +30,16 @@ bytecode unchanged.
 - [x] Compiler-state assertions (e.g. expected stack depth)
 - [ ] Automatic stack permutation
 - [ ] Standalone compiler
-- [x] In-process EVM execution
+- [x] In-process EVM execution (geth)
 - [x] Debugger
   * [x] Stepping
   * [ ] Breakpoints
   * [x] Programmatic inspection (e.g. native Go tests at opcode resolution)
     * [x] Memory
     * [x] Stack
   * [ ] User interface
+- [ ] Source mapping
+- [ ] Coverage analysis
 - [ ] Fork testing with RPC URL
 
 ### Documentation
@@ -87,7 +89,7 @@ state := dbg.State() // is updated on calls to Step() / FastForward()
 
 for !dbg.Done() {
   dbg.Step()
-  fmt.Println("Peek-a-boo", state.CopeContext.Stack().Back(0))
+  fmt.Println("Peek-a-boo", state.ScopeContext.Stack().Back(0))
 }
 
 result, err := results()

compile.go

@@ -152,6 +152,10 @@ CodeLoop:
 
 		case PUSHJUMPDEST:
 
+		case Raw:
+			code, _ := use.Bytecode() // always returns nil error
+			buf.Write(code)
+
 		default:
 			code, err := use.Bytecode()
 			if err != nil {

run.go

@@ -29,26 +29,36 @@ func (c Code) Run(callData []byte, opts ...runopts.Option) ([]byte, error) {
 //
 // If execution never completes, such that dbg.Done() always returns false, then
 // the goroutine will be leaked.
-func (c Code) StartDebugging(callData []byte, opts ...runopts.Option) (*runopts.Debugger, func() ([]byte, error)) {
+//
+// Any compilation error will be returned by StartDebugging() while execution
+// errors are returned by a call to the returned function. Said execution errors
+// can be errors.Unwrap()d to access the same error available in
+// `dbg.State().Err`.
+func (c Code) StartDebugging(callData []byte, opts ...runopts.Option) (*runopts.Debugger, func() ([]byte, error), error) {
+	compiled, err := c.Compile()
+	if err != nil {
+		return nil, nil, fmt.Errorf("%T.Compile(): %v", c, err)
+	}
+
 	dbg := runopts.NewDebugger()
 	opts = append(opts, dbg)
 
 	var (
 		result []byte
-		err    error
+		resErr error
 	)
 	done := make(chan struct{})
 	go func() {
-		result, err = c.Run(callData, opts...)
+		result, resErr = runBytecode(compiled, callData, opts...)
 		close(done)
 	}()
 
 	dbg.Wait()
 
 	return dbg, func() ([]byte, error) {
 		<-done
-		return result, err
-	}
+		return result, resErr
+	}, nil
 }
 
 func runBytecode(compiled, callData []byte, opts ...runopts.Option) ([]byte, error) {
@@ -71,7 +81,7 @@ func runBytecode(compiled, callData []byte, opts ...runopts.Option) ([]byte, err
 
 	out, err := interp.Run(cc, callData, cfg.ReadOnly)
 	if err != nil {
-		return nil, fmt.Errorf("%T.Run([%T.Compile()], [callData], readOnly=%t): %v", interp, Code{}, cfg.ReadOnly, err)
+		return nil, fmt.Errorf("%T.Run([%T.Compile()], [callData], readOnly=%t): %w", interp, Code{}, cfg.ReadOnly, err)
 	}
 	return out, nil
 }

runopts/debugger.go

@@ -11,6 +11,8 @@ import (
 // Execution SHOULD be advanced until Debugger.Done() returns true otherwise
 // resources will be leaked. Best practice is to always call FastForward(),
 // usually in a deferred function.
+//
+// Debugger.State().Err SHOULD be checked once Debugger.Done() returns true.
 func NewDebugger() *Debugger {
 	started := make(chan started)
 	step := make(chan step)
@@ -198,7 +200,7 @@ func (d *debugger) CaptureState(pc uint64, op vm.OpCode, gasLeft, gasCost uint64
 
 	defer func() {
 		switch op {
-		case vm.STOP, vm.RETURN, vm.REVERT:
+		case vm.STOP, vm.RETURN: // REVERT will end up in CaptureFault().
 			// Unlike d.started, we don't use a sync.Once for this because
 			// if it's called twice then we have a bug and want to know
 			// about it.
@@ -222,8 +224,6 @@ func (d *debugger) CaptureFault(pc uint64, op vm.OpCode, gasLeft, gasCost uint64
 	d.setStarted()
 	defer func() { close(d.done) }()
 
-	// TODO: communicate the fault to the user
-
 	d.last.PC = pc
 	d.last.Op = op
 	d.last.GasLeft = gasLeft

runopts/debugger_test.go

@@ -2,9 +2,13 @@ package runopts_test
 
 import (
 	"bytes"
+	"errors"
 	"fmt"
+	"reflect"
+	"strings"
 	"testing"
 
+	"github.com/ethereum/go-ethereum/core/vm"
 	. "github.com/solidifylabs/specialops"
 )
 
@@ -34,7 +38,10 @@ func TestDebugger(t *testing.T) {
 
 	for ffAt, steps := 0, len(wantPCs); ffAt < steps; ffAt++ { // using range wantPCs, while the same, is misleading
 		t.Run(fmt.Sprintf("fast-forward after step %d", ffAt), func(t *testing.T) {
-			dbg, results := code.StartDebugging(nil)
+			dbg, results, err := code.StartDebugging(nil)
+			if err != nil { // compilation error
+				t.Fatalf("%T.StartDebugging(nil) error %v", code, err)
+			}
 			defer dbg.FastForward() // best practice to avoid resource leakage
 
 			state := dbg.State() // can be called any time
@@ -67,3 +74,72 @@ func TestDebugger(t *testing.T) {
 		})
 	}
 }
+
+func TestDebuggerCompilationError(t *testing.T) {
+	code := Code{
+		ExpectStackDepth(5),
+	}
+	if _, _, err := code.StartDebugging(nil); err == nil || !strings.Contains(err.Error(), "Compile()") {
+		t.Errorf("%T.StartDebugging(nil) with known compilation failure got err %v; want containing %q", code, err, "Compile()")
+	}
+}
+
+func TestDebuggerErrors(t *testing.T) {
+	const invalid = vm.OpCode(0xf8)
+	if vm.StringToOp(invalid.String()) != 0 {
+		// This may happen if the above opcode is added. Any invalid value suffices.
+		t.Fatalf("Bad test setup; %[1]T(%[1]d) = %[1]v is valid; want invalid", invalid)
+	}
+
+	tests := []struct {
+		name        string
+		code        Code
+		wantErrType reflect.Type // errors.As doesn't play nicely with any/error
+	}{
+		{
+			name: "immediate underflow",
+			code: Code{
+				SetStackDepth(2), RETURN, // compiles to {RETURN}
+			},
+			wantErrType: reflect.TypeOf(new(vm.ErrStackUnderflow)),
+		},
+		{
+			name: "delayed underflow",
+			code: Code{
+				PUSH0, SetStackDepth(2), RETURN, // compiles to {PUSH0, RETURN}
+			},
+			wantErrType: reflect.TypeOf(new(vm.ErrStackUnderflow)),
+		},
+		{
+			name: "invalid opcode",
+			code: Code{
+				Raw{byte(invalid)},
+			},
+			wantErrType: reflect.TypeOf(new(vm.ErrInvalidOpCode)),
+		},
+		{
+			name: "explicit revert",
+			code: Code{
+				Fn(REVERT, PUSH0, PUSH0),
+			},
+			wantErrType: reflect.TypeOf(errors.New("")),
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			dbg, results, err := tt.code.StartDebugging(nil)
+			if err != nil {
+				t.Fatalf("%T.StartDebugging(nil) error %v", tt.code, err)
+			}
+			dbg.FastForward()
+
+			if err := dbg.State().Err; reflect.TypeOf(err) != tt.wantErrType {
+				t.Errorf("%T.State().Err = %T(%v); want type %v", dbg, err, err, tt.wantErrType)
+			}
+			if _, err := results(); reflect.TypeOf(errors.Unwrap(err)) != tt.wantErrType {
+				t.Errorf("%T.StartDebugging() results function returned error %T(%v); want type %v wrapped", dbg, err, err, tt.wantErrType)
+			}
+		})
+	}
+}

specialops.go

@@ -66,6 +66,15 @@ func (o opCode) String() string {
 	return vm.OpCode(o).String()
 }
 
+// Raw is a Bytecoder that bypasses all compiler checks and simply appends its
+// contents to bytecode. It can be used for raw data, not meant to be executed.
+type Raw []byte
+
+// Bytecode returns `r` unchanged, and a nil error.
+func (r Raw) Bytecode() ([]byte, error) {
+	return []byte(r), nil
+}
+
 // A JUMPDEST is a Bytecoder that is converted into a vm.JUMPDEST while also
 // storing its location in the bytecode for use via a PUSHJUMPDEST or
 // PUSH[string|JUMPDEST](<lbl>).