Remove ContextOptions public destructor and disallow setting callbacks more than once

User should not be able to free resources (callback handles) that could have already been shared with Context

Author: stringintech

kernel/chainstate_manager_test.go

@@ -228,7 +228,6 @@ func (s *ChainstateManagerTestSuite) Setup(t *testing.T) {
 	if err != nil {
 		t.Fatalf("NewContextOptions() error = %v", err)
 	}
-	t.Cleanup(func() { contextOpts.Destroy() })
 
 	chainParams, err := NewChainParameters(ChainTypeRegtest)
 	if err != nil {

kernel/context.go

@@ -68,7 +68,7 @@ func (ctx *Context) destroy() {
 		ctx.ptr = nil
 	}
 	if ctx.options != nil {
-		ctx.options.Destroy()
+		ctx.options.destroy()
 		ctx.options = nil
 	}
 }

kernel/context_options.go

@@ -18,19 +18,20 @@ extern void go_validation_interface_block_checked_bridge(void* user_data, const
 */
 import "C"
 import (
+	"fmt"
 	"runtime"
 	"runtime/cgo"
 	"unsafe"
 )
 
-var _ cManagedResource = &ContextOptions{}
+var _ cResource = &ContextOptions{}
 
 // ContextOptions wraps the C btck_ContextOptions.
-// Once the options is set on a context, the context is responsible for its lifetime and should not be destroyed manually.
+// Once the options is set on a context, the context is responsible for its lifetime; otherwise it is garbage collected
 type ContextOptions struct {
 	ptr                *C.btck_ContextOptions
-	notificationHandle cgo.Handle // Prevents notification callbacks GC until Destroy() called
-	validationHandle   cgo.Handle // Prevents validation callbacks GC until Destroy() called
+	notificationHandle cgo.Handle
+	validationHandle   cgo.Handle
 }
 
 func NewContextOptions() (*ContextOptions, error) {
@@ -40,7 +41,7 @@ func NewContextOptions() (*ContextOptions, error) {
 	}
 
 	opts := &ContextOptions{ptr: ptr}
-	runtime.SetFinalizer(opts, (*ContextOptions).destroy)
+	runtime.SetFinalizer(opts, (*ContextOptions).finalize)
 	return opts, nil
 }
 
@@ -60,13 +61,10 @@ func (opts *ContextOptions) SetChainParams(chainParams *ChainParameters) {
 func (opts *ContextOptions) SetNotifications(callbacks *NotificationCallbacks) error {
 	checkReady(opts)
 	if callbacks == nil {
-		return ErrNilNotificationCallbacks
+		return fmt.Errorf("nil notification callbacks")
 	}
-
-	// Clean up existing handle if present
 	if opts.notificationHandle != 0 {
-		opts.notificationHandle.Delete()
-		opts.notificationHandle = 0
+		return fmt.Errorf("notification callbacks already set")
 	}
 
 	// Create a handle for the callbacks - this prevents garbage collection
@@ -94,13 +92,10 @@ func (opts *ContextOptions) SetNotifications(callbacks *NotificationCallbacks) e
 func (opts *ContextOptions) SetValidationInterface(callbacks *ValidationInterfaceCallbacks) error {
 	checkReady(opts)
 	if callbacks == nil {
-		return ErrNilValidationInterfaceCallbacks
+		return fmt.Errorf("nil validation interface callbacks")
 	}
-
-	// Clean up existing handle if present
 	if opts.validationHandle != 0 {
-		opts.validationHandle.Delete()
-		opts.validationHandle = 0
+		return fmt.Errorf("validation interface callbacks already set")
 	}
 
 	// Create a handle for the callbacks - this prevents garbage collection
@@ -118,6 +113,11 @@ func (opts *ContextOptions) SetValidationInterface(callbacks *ValidationInterfac
 }
 
 func (opts *ContextOptions) destroy() {
+	opts.finalize()
+	runtime.SetFinalizer(opts, nil)
+}
+
+func (opts *ContextOptions) finalize() {
 	if opts.ptr != nil {
 		C.btck_context_options_destroy(opts.ptr)
 		opts.ptr = nil
@@ -134,11 +134,6 @@ func (opts *ContextOptions) destroy() {
 	}
 }
 
-func (opts *ContextOptions) Destroy() {
-	runtime.SetFinalizer(opts, nil)
-	opts.destroy()
-}
-
 func (opts *ContextOptions) isReady() bool {
 	return opts != nil && opts.ptr != nil
 }

kernel/context_test.go

@@ -41,9 +41,6 @@ func TestNewContext(t *testing.T) {
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			opts := tt.setupOption()
-			if opts != nil {
-				defer opts.Destroy()
-			}
 
 			ctx, err := NewContext(opts)
 

kernel/errors.go

@@ -42,7 +42,6 @@ var (
 	ErrKernelScriptPubkeyCreate                  = &KernelError{Operation: "btck_script_pubkey_create"}
 	ErrKernelScriptPubkeyCopy                    = &KernelError{Operation: "btck_script_pubkey_copy"}
 	ErrKernelScriptVerify                        = &KernelError{Operation: "btck_verify_script"}
-	ErrKernelScriptVerifyInvalidFlags            = &KernelError{Operation: "btck_verify_script", Detail: "the provided bitfield for the flags was invalid"}
 	ErrKernelScriptVerifyInvalidFlagsCombination = &KernelError{Operation: "btck_verify_script", Detail: "the flags were combined in an invalid way"}
 	ErrKernelScriptVerifySpentOutputsRequired    = &KernelError{Operation: "btck_verify_script", Detail: "the taproot flag was set, so valid spent_outputs have to be provided"}
 	ErrEmptyScriptPubkeyData                     = errors.New("empty script pubkey data") // constructor error
@@ -75,12 +74,10 @@ var (
 	ErrLoggingConnectionUninitialized = &UninitializedError{ObjectName: "loggingConnection"}
 	ErrKernelLoggingConnectionCreate  = &KernelError{Operation: "btck_logging_connection_create"}
 
-	ErrInvalidChainType                = errors.New("invalid chain type")
-	ErrInvalidLogLevel                 = errors.New("invalid log level")
-	ErrInvalidLogCategory              = errors.New("invalid log category")
-	ErrInvalidScriptVerifyStatus       = errors.New("invalid script verify status")
-	ErrNilNotificationCallbacks        = errors.New("nil notification callbacks")
-	ErrNilValidationInterfaceCallbacks = errors.New("nil validation interface callbacks")
+	ErrInvalidChainType          = errors.New("invalid chain type")
+	ErrInvalidLogLevel           = errors.New("invalid log level")
+	ErrInvalidLogCategory        = errors.New("invalid log category")
+	ErrInvalidScriptVerifyStatus = errors.New("invalid script verify status")
 )
 
 // UninitializedError is returned when an operation is attempted on a