Expose pkg/emulator signing library with injectable finalizer#102
Expose pkg/emulator signing library with injectable finalizer#102Kukks wants to merge 7 commits into
Conversation
The Docker build COPYs each module's go.mod/go.sum for layer caching and runs go mod download per module; pkg/emulator was missing, failing the root go mod download (Build and Scan + integration). The Makefile unit target lists each separate module explicitly; pkg/emulator (a new module) was not included, so its tests never ran.
There was a problem hiding this comment.
🔍 Arkana Code Review — arkade-os/emulator#102
Scope: Promotes internal/application → pkg/emulator, introduces injectable Finalizer interface, exposes public gRPC handler. Protocol-critical: touches signing paths and finalization logic.
✅ What looks good
-
Finalizerinterface (pkg/emulator/service.go:18-21): Minimal, clean, 2-method interface. Structurally satisfied by go-sdk's grpc client — zero coupling. Good API design. -
Signing-only path (
pkg/emulator/tx.go:120):!isFinalizer || s.finalizer == nilis correct. Whenfinalizeris nil, the service returns the signed ark tx + checkpoints without attempting arkd round-trip. The guard is correctly placed after all signing and checkpoint signature verification. -
Close()nil-safety (pkg/emulator/service.go:104-108): Type-asserting a nil interface value returns(nil, false)in Go — this is safe. No panic risk whenfinalizeris nil. -
Config refactor (
internal/config/config.go:183-204): The arkd pubkey fetch + parse logic moved cleanly fromNew()toAppService(). Same validation, same error handling. The standalone emulator behavior is preserved. -
No wire-protocol changes:
emulatorv1proto is untouched. Existing clients are unaffected. -
Test coverage:
TestSubmitTxSigningOnlyproperly exercises the nil-finalizer signing-only path with a fully-formed VTXO spend — verifies the emulator signs both ark tx and checkpoint without attempting finalization. -
New()constructor (pkg/emulator/service.go:73): Correctly validatesarkdPubKey != nil(required for finalizer role determination even in signing-only mode, sincefinalizerAccumulatorneeds it).finalizeris allowed to be nil — intentional. -
No cross-repo breakage: No external consumers of
internal/applicationorpkg/emulatorexist yet. No downstream SDKs affected.
⚠️ Issues
1. Dead code: internal/interface/grpc/handlers/emulator_handler.go should be removed
Severity: Medium (maintenance hazard)
internal/interface/grpc/service.go:143 now uses grpchandler.New() from pkg/emulator/grpchandler, not handlers.New() from the internal package. The internal emulator_handler.go is ~300 lines of dead code that duplicates the public handler line-for-line. Only NewHealthHandler() from health_handler.go is still used from the handlers package.
Keeping both will cause silent drift — someone fixes a bug in one handler, the other stays broken. Recommend deleting internal/interface/grpc/handlers/emulator_handler.go in this PR.
Files: internal/interface/grpc/handlers/emulator_handler.go (entire file), internal/interface/grpc/service.go:14 (unused import of handlers for emulator funcs)
📝 Minor / Non-blocking
-
pkg/emulator/service.go:73— unusedctxparameter:Newtakes_ context.Contextbut never uses it (the arkdGetInfocall moved to config). Consider removing it to signal that construction is synchronous, or keep it if you anticipate future use. Non-blocking. -
pkg/emulator/grpchandler/handler.go:500—Newreturn type: Returnsemulatorv1.EmulatorServiceServer(interface) vs the internal handler returning*handler(concrete). The public version is better — just noting the inconsistency with the dead internal handler reinforces point⚠️ 1 above.
🔐 Protocol-Critical Flag
This PR modifies signing paths (SubmitTx, SubmitFinalization), key handling, and finalization logic. While the changes are mechanically sound and the behavioral modification (signing-only mode) is well-guarded, this requires human review before merge per protocol-critical policy.
Specific areas for human verification:
pkg/emulator/tx.go:120— the|| s.finalizer == nilguard: confirm that signing-only consumers (e.g.,arkd-signer) will never need to trigger finalization through this pathpkg/emulator/finalization.go— unchanged but now public: confirm the forfeit signing and commitment signing logic is safe to expose as a library
Summary: Clean refactor, good interface design, solid test. One dead code issue to fix. Flagging for human review due to protocol-critical scope.
Summary
Turns the emulator's signing service into a reusable Go library so other services (e.g.
arkd-signer) can perform signing-only ArkadeScript signing by importing it, instead of running a separate emulator process. The standalone emulator keeps working unchanged.Changes
Promote
internal/application→pkg/emulator(new module,package emulator). Pure mechanical move — exportsService,New, and the request/response types.Injectable finalization. New
Finalizerinterface (the arkd round-trip subset used bySubmitTx):SubmitTx(ctx, arkTx string, checkpoints []string) (txid, finalArkTx string, arkdCheckpoints []string, error)FinalizeTx(ctx, txid string, checkpoints []string) errorNewnow takesarkdPubKey *btcec.PublicKeyandfinalizer Finalizer(instead ofarkdURL).SubmitTxfinalizes only whenfinalizer != nil; withnilit returns the signed ark tx + checkpoints (signing-only). The go-sdk client structurally satisfiesFinalizer, so the standalone passes it directly.Public gRPC handler
pkg/emulator/grpchandler(theEmulatorServiceimpl), so consumers can register it on their own server. Health handler/interceptors stay internal.Standalone rewired:
internal/configbuilds the go-sdk client, fetchesarkdPubKeyviaGetInfoat startup, and passes the client as the finalizer. Behavior unchanged.Compatibility
emulatorv1wire-protocol changes.Testing
pkg/emulatorunit tests pass (incl. a newnil-finalizer signing-only test).