From 78a8e87a9d15370ff0f8b8b38492c907c67f3ba2 Mon Sep 17 00:00:00 2001 From: maclane Date: Mon, 13 Jul 2026 12:40:54 -0400 Subject: [PATCH] fix(tbtc): keep FROST activation wallets signable Preserve persisted legacy tECDSA shares on the legacy bridge in the cgo FROST build, require the complete interactive ROAST path before starting distributed DKG, and start ROAST receiving only after the first subscriber exists. --- .../roast_interactive_signing_frost_native.go | 12 ++ ...t_interactive_signing_frost_native_test.go | 38 ++++++ pkg/frost/signing/roast_retry_readiness.go | 26 ++-- .../signing/roast_retry_readiness_test.go | 16 +++ .../roast_runner_bus_net_frost_native.go | 21 ++- .../roast_runner_bus_net_frost_native_test.go | 76 +++++++++++ ...ast_selection_consume_frost_roast_retry.go | 2 +- pkg/tbtc/frost_dkg_execution_frost_native.go | 19 +-- ...cution_frost_native_no_roast_retry_test.go | 31 +++++ .../frost_dkg_execution_frost_native_test.go | 63 +++++++++ ...solver_build_frost_native_scaffold_test.go | 97 ++++++++++++++ ...resolver_build_frost_native_tbtc_signer.go | 72 +--------- ...ver_build_frost_native_tbtc_signer_test.go | 123 +++++++++++++----- ...terial_resolver_build_frost_native_test.go | 89 ------------- 14 files changed, 476 insertions(+), 209 deletions(-) create mode 100644 pkg/tbtc/frost_dkg_execution_frost_native_no_roast_retry_test.go create mode 100644 pkg/tbtc/signer_material_resolver_build_frost_native_scaffold_test.go diff --git a/pkg/frost/signing/roast_interactive_signing_frost_native.go b/pkg/frost/signing/roast_interactive_signing_frost_native.go index e5a916031b..fb2b77bbdb 100644 --- a/pkg/frost/signing/roast_interactive_signing_frost_native.go +++ b/pkg/frost/signing/roast_interactive_signing_frost_native.go @@ -48,6 +48,18 @@ func registeredInteractiveSigningEngine() interactiveSigningEngine { return provider() } +// InteractiveSigningReady reports whether this process has every pre-wallet +// prerequisite needed to sign material produced by distributed DKG: both +// operator opt-ins, the ROAST transition producer, and an interactive engine. +// It intentionally does not require a wallet-scoped coordinator registration; +// that registration can happen only after DKG has persisted the wallet's key +// group and is enforced by the signing path when the wallet is used. +func InteractiveSigningReady() bool { + return InteractiveSigningOptInEnabled() && + RoastRetryInfrastructureReady() && + registeredInteractiveSigningEngine() != nil +} + // ResetInteractiveSigningEngineProviderForTest clears the registered provider. // Tests defer it so a registration does not leak into other tests. func ResetInteractiveSigningEngineProviderForTest() { diff --git a/pkg/frost/signing/roast_interactive_signing_frost_native_test.go b/pkg/frost/signing/roast_interactive_signing_frost_native_test.go index 62ca2edcd4..a64fd6554f 100644 --- a/pkg/frost/signing/roast_interactive_signing_frost_native_test.go +++ b/pkg/frost/signing/roast_interactive_signing_frost_native_test.go @@ -76,6 +76,44 @@ func TestRegisterInteractiveSigningEngineProvider(t *testing.T) { } } +func TestInteractiveSigningReady_RequiresCompletePath(t *testing.T) { + ResetInteractiveSigningEngineProviderForTest() + t.Cleanup(ResetInteractiveSigningEngineProviderForTest) + + t.Setenv(InteractiveSigningOptInEnvVar, "true") + t.Setenv(RoastRetryReadinessOptInEnvVar, "true") + RegisterInteractiveSigningEngineProvider( + func() interactiveSigningEngine { return newFakeInteractiveSigningEngine() }, + ) + + if actual, expected := InteractiveSigningReady(), roastTransitionProducerAvailable(); actual != expected { + t.Fatalf( + "unexpected complete-path readiness\nexpected: [%t]\nactual: [%t]", + expected, + actual, + ) + } + + t.Setenv(InteractiveSigningOptInEnvVar, "") + if InteractiveSigningReady() { + t.Fatal("interactive signing must not be ready without its operator opt-in") + } + + t.Setenv(InteractiveSigningOptInEnvVar, "true") + t.Setenv(RoastRetryReadinessOptInEnvVar, "") + if InteractiveSigningReady() { + t.Fatal("interactive signing must not be ready without ROAST retry opt-in") + } + + t.Setenv(RoastRetryReadinessOptInEnvVar, "true") + RegisterInteractiveSigningEngineProvider( + func() interactiveSigningEngine { return nil }, + ) + if InteractiveSigningReady() { + t.Fatal("interactive signing must not be ready without an engine") + } +} + func TestInteractiveRoastSigningThreshold(t *testing.T) { // The persisted DKG threshold is returned verbatim - NOT derived from the // per-attempt dishonest threshold. diff --git a/pkg/frost/signing/roast_retry_readiness.go b/pkg/frost/signing/roast_retry_readiness.go index 71ebfb3165..6e5d8afeb9 100644 --- a/pkg/frost/signing/roast_retry_readiness.go +++ b/pkg/frost/signing/roast_retry_readiness.go @@ -61,6 +61,20 @@ func RoastRetryReadinessOptInEnabled() bool { return strings.EqualFold(value, "true") } +// RoastRetryInfrastructureReady reports whether this process can support the +// ROAST retry path before any wallet-scoped coordinator exists: the operator +// readiness opt-in is set AND the transition producer is present in this build. +// The producer requires both frost_native and frost_roast_retry. +// +// This is deliberately weaker than RoastRetryActive: a newly selected DKG member +// must verify that the signing infrastructure exists before creating a wallet, +// but that wallet's coordinators cannot be registered until its DKG material has +// been persisted. Once the wallet exists, the active gates below additionally +// require the appropriate coordinator registration. +func RoastRetryInfrastructureReady() bool { + return RoastRetryReadinessOptInEnabled() && roastTransitionProducerAvailable() +} + // RoastRetryActive reports whether ROAST retry orchestration is runtime-active: // the readiness opt-in is set, a coordinator is registered, AND this build // contains the transition producer (frost_native). It is the deterministic, @@ -77,16 +91,8 @@ func RoastRetryReadinessOptInEnabled() bool { // can never be created instead of using the uniform legacy shuffle (Codex P2-1). // Always false in builds without the frost_roast_retry tag (the registration and // producer default stubs both report unavailable). -// readinessAndProducerReady is the build+env prefix shared by RoastRetryActive and -// RoastRetryActiveForKeyGroupMember: the readiness opt-in is set AND the transition -// producer is built in (frost_native). Both gates additionally require a registered -// coordinator (any entry / this wallet-seat's). -func readinessAndProducerReady() bool { - return RoastRetryReadinessOptInEnabled() && roastTransitionProducerAvailable() -} - func RoastRetryActive() bool { - if !readinessAndProducerReady() { + if !RoastRetryInfrastructureReady() { return false } _, ok := RegisteredRoastRetryCoordinator() @@ -106,7 +112,7 @@ func RoastRetryActive() bool { // that cross-wallet fracture on the numbering path.) Always false in builds without // the frost_roast_retry tag. func RoastRetryActiveForKeyGroupMember(keyGroupID string, member group.MemberIndex) bool { - if !readinessAndProducerReady() { + if !RoastRetryInfrastructureReady() { return false } _, ok := RegisteredRoastRetryCoordinatorForKeyGroupMember(keyGroupID, member) diff --git a/pkg/frost/signing/roast_retry_readiness_test.go b/pkg/frost/signing/roast_retry_readiness_test.go index 9eb0e82746..8fe4069685 100644 --- a/pkg/frost/signing/roast_retry_readiness_test.go +++ b/pkg/frost/signing/roast_retry_readiness_test.go @@ -70,6 +70,22 @@ func TestRoastRetryReadinessOptInEnabled_MirrorsEnsureResult(t *testing.T) { } } +func TestRoastRetryInfrastructureReady_RequiresOptInAndProducer(t *testing.T) { + t.Setenv(RoastRetryReadinessOptInEnvVar, "true") + if actual, expected := RoastRetryInfrastructureReady(), roastTransitionProducerAvailable(); actual != expected { + t.Fatalf( + "unexpected infrastructure readiness with opt-in enabled\nexpected: [%t]\nactual: [%t]", + expected, + actual, + ) + } + + t.Setenv(RoastRetryReadinessOptInEnvVar, "") + if RoastRetryInfrastructureReady() { + t.Fatal("infrastructure must not be ready without the operator opt-in") + } +} + func TestRoastRetryReadinessOptInEnvVar_MatchesRFC(t *testing.T) { const expected = "KEEP_CORE_FROST_ROAST_RETRY_ENABLED" if RoastRetryReadinessOptInEnvVar != expected { diff --git a/pkg/frost/signing/roast_runner_bus_net_frost_native.go b/pkg/frost/signing/roast_runner_bus_net_frost_native.go index 651372b5d2..0f2e97eedc 100644 --- a/pkg/frost/signing/roast_runner_bus_net_frost_native.go +++ b/pkg/frost/signing/roast_runner_bus_net_frost_native.go @@ -159,13 +159,15 @@ type broadcastChannelRunnerBus struct { mu sync.Mutex subscribers []*RunnerBusSubscriber + startOnce sync.Once } // NewBroadcastChannelRunnerBus returns a RunnerBus over the given wallet signing -// broadcast channel. It registers the runner message unmarshalers and installs -// a receive handler for the lifetime of ctx; cancel ctx (e.g. at session end) -// to stop receiving. The channel and membershipValidator are the ones the -// existing signing.Request already carries; this adapter does not create them. +// broadcast channel. It registers the runner message unmarshalers; the first +// Subscribe installs the receive handler for the lifetime of ctx, after that +// subscriber has been registered. Cancel ctx (e.g. at session end) to stop +// receiving. The channel and membershipValidator are the ones the existing +// signing.Request already carries; this adapter does not create them. func NewBroadcastChannelRunnerBus( ctx context.Context, logger log.StandardLogger, @@ -195,14 +197,17 @@ func NewBroadcastChannelRunnerBus( } registerRunnerTransportUnmarshalers(channel) - channel.Recv(ctx, b.handleMessage) return b, nil } // Subscribe registers a receiver and returns its typed streams. Subscribing // before any Broadcast is the caller's responsibility (the runner subscribes in -// its constructor). +// its constructor). The first subscription starts inbound delivery only AFTER +// the subscriber is visible to handleMessage. Starting Recv in the bus +// constructor would leave a window where a message is handled with zero +// subscribers and dropped; pkg/net would then suppress its retransmissions as +// already seen, permanently losing it for this signing attempt. func (b *broadcastChannelRunnerBus) Subscribe() *RunnerBusSubscriber { s := &RunnerBusSubscriber{ commitments: make(chan RunnerMessage, b.streamBuffer), @@ -217,6 +222,10 @@ func (b *broadcastChannelRunnerBus) Subscribe() *RunnerBusSubscriber { b.subscribers = append(b.subscribers, s) b.mu.Unlock() + b.startOnce.Do(func() { + b.channel.Recv(b.ctx, b.handleMessage) + }) + return s } diff --git a/pkg/frost/signing/roast_runner_bus_net_frost_native_test.go b/pkg/frost/signing/roast_runner_bus_net_frost_native_test.go index f8cfd45efe..906555d9a9 100644 --- a/pkg/frost/signing/roast_runner_bus_net_frost_native_test.go +++ b/pkg/frost/signing/roast_runner_bus_net_frost_native_test.go @@ -25,6 +25,37 @@ type fakeNetMessage struct { payload interface{} } +// immediateRecvBroadcastChannel invokes its receive handler synchronously from +// Recv. It models the worst-case ordering for the production adapter: an +// already-arrived peer message is ready at the exact instant receiving starts. +// If Recv starts before Subscribe registers its stream, that message is lost. +type immediateRecvBroadcastChannel struct { + message net.Message + recvCalls int +} + +func (c *immediateRecvBroadcastChannel) Name() string { return "runner-bus-test" } +func (c *immediateRecvBroadcastChannel) Send( + context.Context, + net.TaggedMarshaler, + ...net.RetransmissionStrategy, +) error { + return nil +} +func (c *immediateRecvBroadcastChannel) Recv( + _ context.Context, + handler func(net.Message), +) { + c.recvCalls++ + if c.message != nil { + handler(c.message) + } +} +func (c *immediateRecvBroadcastChannel) SetUnmarshaler(func() net.TaggedUnmarshaler) {} +func (c *immediateRecvBroadcastChannel) SetFilter(net.BroadcastChannelFilter) error { + return nil +} + func (m fakeNetMessage) TransportSenderID() net.TransportIdentifier { return nil } func (m fakeNetMessage) SenderPublicKey() []byte { return m.senderPublicKey } func (m fakeNetMessage) Payload() interface{} { return m.payload } @@ -42,6 +73,7 @@ func (m fakeNetMessage) Type() string { // authenticated public-key bytes and an outsider's key (not in the group). type runnerBusAuthFixture struct { bus *broadcastChannelRunnerBus + validator *group.MembershipValidator operatorA []byte // seats 1 and 3 operatorB []byte // seat 2 outsider []byte // not selected @@ -71,13 +103,16 @@ func newRunnerBusAuthFixture(t *testing.T, streamSize int) runnerBusAuthFixture ) bus := &broadcastChannelRunnerBus{ + ctx: context.Background(), logger: &testutils.MockLogger{}, + channel: &immediateRecvBroadcastChannel{}, membershipValidator: validator, streamBuffer: streamSize, seenBound: defaultRunnerBusSeenBound, } return runnerBusAuthFixture{ bus: bus, + validator: validator, operatorA: operatorA, operatorB: operatorB, outsider: outsider, @@ -85,6 +120,47 @@ func newRunnerBusAuthFixture(t *testing.T, streamSize int) runnerBusAuthFixture } } +func TestBroadcastChannelRunnerBus_StartsReceivingAfterSubscribe(t *testing.T) { + f := newRunnerBusAuthFixture(t, 8) + channel := &immediateRecvBroadcastChannel{ + message: shareMessage(1, f.operatorA, "already-arrived-share"), + } + + bus, err := NewBroadcastChannelRunnerBus( + context.Background(), + &testutils.MockLogger{}, + channel, + f.validator, + ) + if err != nil { + t.Fatalf("new runner bus: %v", err) + } + if channel.recvCalls != 0 { + t.Fatalf("constructor started receiving before a subscriber existed") + } + + // Recv synchronously delivers channel.message. Subscribe must register the + // stream before it starts Recv or this message would be dropped forever. + sub := bus.Subscribe() + if channel.recvCalls != 1 { + t.Fatalf("expected Subscribe to start receiving once, got [%d] calls", channel.recvCalls) + } + select { + case msg := <-sub.Shares(): + if string(msg.Payload) != "already-arrived-share" { + t.Fatalf("unexpected payload: [%q]", msg.Payload) + } + default: + t.Fatal("message delivered as Recv started was lost before subscription") + } + + // Additional subscribers must not install duplicate handlers. + bus.Subscribe() + if channel.recvCalls != 1 { + t.Fatalf("expected receiving to start once, got [%d] calls", channel.recvCalls) + } +} + func shareMessage(sender group.MemberIndex, authorPublicKey []byte, payload string) fakeNetMessage { return fakeNetMessage{ senderPublicKey: authorPublicKey, diff --git a/pkg/frost/signing/roast_selection_consume_frost_roast_retry.go b/pkg/frost/signing/roast_selection_consume_frost_roast_retry.go index 51fcbe6aa7..1b937a8822 100644 --- a/pkg/frost/signing/roast_selection_consume_frost_roast_retry.go +++ b/pkg/frost/signing/roast_selection_consume_frost_roast_retry.go @@ -68,7 +68,7 @@ func ConsumeRoastTransitionForSelection( // Build/env readiness is group-uniform (readiness opt-in AND the transition // producer is built in). If not ready, a uniform legacy fallback every honest node // makes identically. - if !readinessAndProducerReady() { + if !RoastRetryInfrastructureReady() { return nil, nil, ErrRoastSelectionFallBackToLegacy } diff --git a/pkg/tbtc/frost_dkg_execution_frost_native.go b/pkg/tbtc/frost_dkg_execution_frost_native.go index d78d4b2ef0..80bca245c5 100644 --- a/pkg/tbtc/frost_dkg_execution_frost_native.go +++ b/pkg/tbtc/frost_dkg_execution_frost_native.go @@ -42,18 +42,21 @@ func executeFrostDKGIfPossible( return false } - // Distributed DKG produces signing material usable ONLY via the interactive - // path; with interactive signing disabled, signing would route to the coarse - // path, which cannot reconstruct a distributed key, and every signature would - // fail. Refuse to run rather than create an unsignable wallet, coupling the DKG - // switch to the signing switch. - if !frostsigning.InteractiveSigningOptInEnabled() { + // Distributed DKG produces signing material usable ONLY via the complete + // interactive ROAST path. The interactive audit flag alone is insufficient: + // without the ROAST readiness opt-in or a build containing the transition + // producer, orchestration takes its static fallback and reaches the removed + // coarse primitive. Refuse to run rather than create an unsignable wallet. + if !frostsigning.InteractiveSigningReady() { logger.Errorf( "FROST DKG with seed [0x%x] selected this operator, but the distributed "+ - "DKG requires interactive signing (%s) to be enabled; refusing to run to "+ - "avoid creating a wallet that cannot sign", + "DKG requires the complete interactive ROAST signing path (%s=true, "+ + "%s=true, a frost_native+frost_roast_retry build, and a registered "+ + "interactive engine); refusing to run to avoid creating a wallet that "+ + "cannot sign", event.Seed, frostsigning.InteractiveSigningOptInEnvVar, + frostsigning.RoastRetryReadinessOptInEnvVar, ) return false } diff --git a/pkg/tbtc/frost_dkg_execution_frost_native_no_roast_retry_test.go b/pkg/tbtc/frost_dkg_execution_frost_native_no_roast_retry_test.go new file mode 100644 index 0000000000..e7a0c36578 --- /dev/null +++ b/pkg/tbtc/frost_dkg_execution_frost_native_no_roast_retry_test.go @@ -0,0 +1,31 @@ +//go:build frost_native && !frost_roast_retry + +package tbtc + +import ( + "context" + "math/big" + "testing" + + frostsigning "github.com/keep-network/keep-core/pkg/frost/signing" + "github.com/keep-network/keep-core/pkg/protocol/group" +) + +func TestExecuteFrostDKGIfPossible_RequiresRoastRetryBuild(t *testing.T) { + t.Setenv(frostsigning.InteractiveSigningOptInEnvVar, "true") + t.Setenv(frostsigning.RoastRetryReadinessOptInEnvVar, "true") + registerFrostDKGReadinessTestEngine(t) + + executed := executeFrostDKGIfPossible( + context.Background(), + nil, + nil, + &FrostDKGStartedEvent{Seed: big.NewInt(100)}, + []group.MemberIndex{1}, + nil, + ) + + if executed { + t.Fatal("DKG must not execute in a build without the ROAST retry path") + } +} diff --git a/pkg/tbtc/frost_dkg_execution_frost_native_test.go b/pkg/tbtc/frost_dkg_execution_frost_native_test.go index 57b28daa2e..c52cb9ed4d 100644 --- a/pkg/tbtc/frost_dkg_execution_frost_native_test.go +++ b/pkg/tbtc/frost_dkg_execution_frost_native_test.go @@ -4,7 +4,9 @@ package tbtc import ( "bytes" + "context" "encoding/hex" + "math/big" "testing" "github.com/keep-network/keep-core/pkg/chain" @@ -13,6 +15,67 @@ import ( "github.com/keep-network/keep-core/pkg/protocol/group" ) +func TestExecuteFrostDKGIfPossible_RequiresRoastRetryReadiness(t *testing.T) { + t.Setenv(frostsigning.InteractiveSigningOptInEnvVar, "true") + t.Setenv(frostsigning.RoastRetryReadinessOptInEnvVar, "") + registerFrostDKGReadinessTestEngine(t) + + executed := executeFrostDKGIfPossible( + context.Background(), + nil, + nil, + &FrostDKGStartedEvent{Seed: big.NewInt(100)}, + []group.MemberIndex{1}, + nil, + ) + + if executed { + t.Fatal("DKG must not execute without ROAST retry readiness") + } +} + +type frostDKGReadinessTestEngine struct{} + +func (*frostDKGReadinessTestEngine) BuildTaprootTx( + string, + []frostsigning.NativeTBTCSignerTxInput, + []frostsigning.NativeTBTCSignerTxOutput, + *string, +) (*frostsigning.NativeTBTCSignerTxResult, error) { + return nil, nil +} + +func (*frostDKGReadinessTestEngine) VerifySignatureShare( + string, + []byte, + []byte, + uint16, + *[32]byte, +) (frostsigning.NativeShareVerificationVerdict, error) { + return frostsigning.NativeShareVerdictValid, nil +} + +func registerFrostDKGReadinessTestEngine(t *testing.T) { + t.Helper() + + previous := frostsigning.CurrentNativeTBTCSignerEngine() + frostsigning.UnregisterNativeTBTCSignerEngine() + if err := frostsigning.RegisterNativeTBTCSignerEngine( + &frostDKGReadinessTestEngine{}, + ); err != nil { + t.Fatalf("failed to register native engine: [%v]", err) + } + + t.Cleanup(func() { + frostsigning.UnregisterNativeTBTCSignerEngine() + if previous != nil { + if err := frostsigning.RegisterNativeTBTCSignerEngine(previous); err != nil { + t.Errorf("failed to restore native engine: [%v]", err) + } + } + }) +} + func TestLowestLocalActiveMemberIndex(t *testing.T) { testCases := map[string]struct { local []group.MemberIndex diff --git a/pkg/tbtc/signer_material_resolver_build_frost_native_scaffold_test.go b/pkg/tbtc/signer_material_resolver_build_frost_native_scaffold_test.go new file mode 100644 index 0000000000..4c6cf0e3c5 --- /dev/null +++ b/pkg/tbtc/signer_material_resolver_build_frost_native_scaffold_test.go @@ -0,0 +1,97 @@ +//go:build frost_native && !(frost_tbtc_signer && cgo) + +package tbtc + +import ( + "bytes" + "encoding/hex" + "encoding/json" + "testing" + + frostsigning "github.com/keep-network/keep-core/pkg/frost/signing" + "github.com/keep-network/keep-core/pkg/tecdsa" +) + +func TestRegisterSignerMaterialResolverForBuild_UsesDefaultProvider( + t *testing.T, +) { + // This transitional non-cgo resolver builds legacy-wallet-pubkey signer + // material. The real cgo activation build keeps legacy shares on the legacy + // bridge and has separate coverage for that contract. + t.Setenv(frostsigning.AcceptScaffoldKeyGroupEnvVar, "true") + + UnregisterSignerMaterialResolver() + UnregisterSignerMaterialResolverProviderForBuild() + t.Cleanup(UnregisterSignerMaterialResolver) + t.Cleanup(UnregisterSignerMaterialResolverProviderForBuild) + + err := RegisterSignerMaterialResolverForBuild() + if err != nil { + t.Fatalf("unexpected build resolver registration error: [%v]", err) + } + + privateKeyShare := createMockSigner(t).privateKeyShare + + result, err := resolveSignerMaterial(privateKeyShare) + if err != nil { + t.Fatalf("unexpected resolver error: [%v]", err) + } + + nativeSignerMaterial, ok := result.(*frostsigning.NativeSignerMaterial) + if !ok { + t.Fatalf( + "unexpected resolved signer material type\nexpected: [%T]\nactual: [%T]", + &frostsigning.NativeSignerMaterial{}, + result, + ) + } + + expectedPayload, err := privateKeyShare.Marshal() + if err != nil { + t.Fatalf("failed marshaling expected private key share: [%v]", err) + } + + if nativeSignerMaterial.Format != frostsigning.NativeSignerMaterialFormatFrostTBTCSignerV1 { + t.Fatalf( + "unexpected native signer material format\nexpected: [%s]\nactual: [%s]", + frostsigning.NativeSignerMaterialFormatFrostTBTCSignerV1, + nativeSignerMaterial.Format, + ) + } + + var payload frostsigning.NativeTBTCSignerMaterialPayload + if err := json.Unmarshal(nativeSignerMaterial.Payload, &payload); err != nil { + t.Fatalf("failed unmarshalling tbtc signer material payload: [%v]", err) + } + + if payload.KeyGroup == "" { + t.Fatal("expected non-empty tbtc-signer key group") + } + + if payload.KeyGroupSource == "" { + t.Fatal("expected non-empty tbtc-signer key group source") + } + + legacyPrivateKeySharePayload, err := hex.DecodeString(payload.LegacyPrivateKeyShareHex) + if err != nil { + t.Fatalf("failed decoding legacy private key share payload: [%v]", err) + } + + decodedPrivateKeyShare := &tecdsa.PrivateKeyShare{} + if err := decodedPrivateKeyShare.Unmarshal(legacyPrivateKeySharePayload); err != nil { + t.Fatalf("failed unmarshalling decoded private key share: [%v]", err) + } + + actualPayload, err := decodedPrivateKeyShare.Marshal() + if err != nil { + t.Fatalf("failed marshaling decoded private key share: [%v]", err) + } + + if !bytes.Equal(expectedPayload, actualPayload) { + t.Fatalf( + "unexpected resolved signer payload\nexpected: [%x]\nactual: [%x]", + expectedPayload, + actualPayload, + ) + } +} diff --git a/pkg/tbtc/signer_material_resolver_build_frost_native_tbtc_signer.go b/pkg/tbtc/signer_material_resolver_build_frost_native_tbtc_signer.go index 268b53a521..e7c020e119 100644 --- a/pkg/tbtc/signer_material_resolver_build_frost_native_tbtc_signer.go +++ b/pkg/tbtc/signer_material_resolver_build_frost_native_tbtc_signer.go @@ -2,15 +2,7 @@ package tbtc -import ( - "crypto/sha256" - "encoding/hex" - "encoding/json" - "fmt" - - frostsigning "github.com/keep-network/keep-core/pkg/frost/signing" - "github.com/keep-network/keep-core/pkg/tecdsa" -) +import "fmt" func registerSignerMaterialResolverForBuild() error { provider := currentSignerMaterialResolverProviderForBuild() @@ -31,61 +23,9 @@ func registerSignerMaterialResolverForBuild() error { } func defaultSignerMaterialResolverProviderForBuild() (SignerMaterialResolver, error) { - return &buildTaggedNativeSignerMaterialResolver{}, nil -} - -// buildTaggedNativeSignerMaterialResolver derives transitional signer material -// for frost_tbtc_signer builds. It carries a deterministic key-group handle and -// embeds legacy private-key-share bytes to preserve temporary Go-side fallback. -type buildTaggedNativeSignerMaterialResolver struct{} - -func (btnsmr *buildTaggedNativeSignerMaterialResolver) ResolveSignerMaterial( - privateKeyShare *tecdsa.PrivateKeyShare, -) (any, error) { - if privateKeyShare == nil { - return nil, fmt.Errorf("private key share is nil") - } - - legacyPrivateKeySharePayload, err := privateKeyShare.Marshal() - if err != nil { - return nil, fmt.Errorf("cannot marshal private key share: [%w]", err) - } - - walletPublicKeyBytes, err := marshalPublicKey(privateKeyShare.PublicKey()) - if err != nil { - return nil, fmt.Errorf("cannot marshal wallet public key: [%w]", err) - } - - keyGroupDigest := sha256.Sum256(walletPublicKeyBytes) - - // Scaffold-era key-group derivation: the current value identifies - // placeholder material derived from the legacy wallet public-key hash, - // not the output of a real FROST DKG run. Refuse to surface that material - // at all unless the operator has explicitly opted in via - // AcceptScaffoldKeyGroupEnvVar — production deployments must never set - // this. See native_tbtc_signer_material.go for the env-var contract. - if !frostsigning.AcceptScaffoldKeyGroupEnabled() { - return nil, fmt.Errorf( - "refusing to build scaffold-era %q signer material; set %s=true to "+ - "opt in for local/CI use only, never in production", - frostsigning.NativeTBTCSignerKeyGroupSourceLegacyWalletPubKey, - frostsigning.AcceptScaffoldKeyGroupEnvVar, - ) - } - - // TODO: Replace this placeholder key-group derivation with Rust DKG output. - // The current value identifies scaffold-era material only. - payload, err := json.Marshal(frostsigning.NativeTBTCSignerMaterialPayload{ - KeyGroup: hex.EncodeToString(keyGroupDigest[:]), - KeyGroupSource: frostsigning.NativeTBTCSignerKeyGroupSourceLegacyWalletPubKey, - LegacyPrivateKeyShareHex: hex.EncodeToString(legacyPrivateKeySharePayload), - }) - if err != nil { - return nil, fmt.Errorf("cannot marshal tbtc signer material payload: [%w]", err) - } - - return &frostsigning.NativeSignerMaterial{ - Format: frostsigning.NativeSignerMaterialFormatFrostTBTCSignerV1, - Payload: payload, - }, nil + // Legacy tECDSA shares identify existing wallets that still need the legacy + // signing bridge during migration. Native FROST DKG supplies and persists its + // own signer material directly, so it does not need legacy shares converted + // into scaffold-era FROST material here. + return &legacyPrivateKeyShareSignerMaterialResolver{}, nil } diff --git a/pkg/tbtc/signer_material_resolver_build_frost_native_tbtc_signer_test.go b/pkg/tbtc/signer_material_resolver_build_frost_native_tbtc_signer_test.go index 24e5fa44c5..633d0facc2 100644 --- a/pkg/tbtc/signer_material_resolver_build_frost_native_tbtc_signer_test.go +++ b/pkg/tbtc/signer_material_resolver_build_frost_native_tbtc_signer_test.go @@ -3,30 +3,18 @@ package tbtc import ( - "strings" + "bytes" "testing" frostsigning "github.com/keep-network/keep-core/pkg/frost/signing" + "github.com/keep-network/keep-core/pkg/tbtc/gen/pb" + "github.com/keep-network/keep-core/pkg/tecdsa" + "google.golang.org/protobuf/proto" ) -// TestRegisterSignerMaterialResolverForBuild_DefaultProviderRefusesScaffoldWithoutOptIn -// asserts the cgo frost_tbtc_signer resolver REFUSES to surface scaffold-era -// signer material unless the operator opts in via AcceptScaffoldKeyGroupEnvVar. -// -// This is cgo-only behaviour: only the frost_tbtc_signer (cgo) resolver carries -// the resolver-level refusal guard. The non-cgo frost_native resolver -// intentionally PERMITS scaffold resolution -- it is the transitional build, and -// the deeper native_ffi_primitive guard refuses scaffold material at signing -// time instead. The migration tests in signer_material_encoding_frost_native_test.go -// (tagged for the non-cgo build) assert that permissive behaviour. This test -// therefore lives behind the cgo tag so it exercises the resolver whose contract -// it describes; previously it was tagged plain frost_native and failed under any -// non-cgo frost_native build. -func TestRegisterSignerMaterialResolverForBuild_DefaultProviderRefusesScaffoldWithoutOptIn( +func TestRegisterSignerMaterialResolverForBuild_DefaultProviderPreservesLegacyShare( t *testing.T, ) { - // Force the env var to "" so a stray external value cannot suppress the - // scaffold refusal during this regression test. t.Setenv(frostsigning.AcceptScaffoldKeyGroupEnvVar, "") UnregisterSignerMaterialResolver() @@ -40,25 +28,102 @@ func TestRegisterSignerMaterialResolverForBuild_DefaultProviderRefusesScaffoldWi } privateKeyShare := createMockSigner(t).privateKeyShare - _, err = resolveSignerMaterial(privateKeyShare) - if err == nil { - t.Fatal( - "expected scaffold-refusal error from default resolver without opt-in", + resolvedSignerMaterial, err := resolveSignerMaterial(privateKeyShare) + if err != nil { + t.Fatalf("unexpected resolver error: [%v]", err) + } + + resolvedPrivateKeyShare, ok := resolvedSignerMaterial.(*tecdsa.PrivateKeyShare) + if !ok { + t.Fatalf( + "unexpected resolved signer material type\nexpected: [%T]\nactual: [%T]", + &tecdsa.PrivateKeyShare{}, + resolvedSignerMaterial, ) } - if !strings.Contains(err.Error(), frostsigning.AcceptScaffoldKeyGroupEnvVar) { + if resolvedPrivateKeyShare != privateKeyShare { t.Fatalf( - "expected scaffold-refusal error to reference %s; got: [%v]", - frostsigning.AcceptScaffoldKeyGroupEnvVar, - err, + "unexpected resolved private key share\nexpected: [%v]\nactual: [%v]", + privateKeyShare, + resolvedPrivateKeyShare, ) } - if !strings.Contains(err.Error(), frostsigning.NativeTBTCSignerKeyGroupSourceLegacyWalletPubKey) { +} + +func TestSignerMarshalling_LegacyRoundtripStaysOnLegacyBridgeInTBTCSignerBuild( + t *testing.T, +) { + t.Setenv(frostsigning.AcceptScaffoldKeyGroupEnvVar, "") + + UnregisterSignerMaterialResolver() + UnregisterSignerMaterialResolverProviderForBuild() + t.Cleanup(UnregisterSignerMaterialResolver) + t.Cleanup(UnregisterSignerMaterialResolverProviderForBuild) + + if err := RegisterSignerMaterialResolverForBuild(); err != nil { + t.Fatalf("unexpected build resolver registration error: [%v]", err) + } + + legacySigner := createMockSigner(t) + legacySigner.signerMaterial = legacySigner.privateKeyShare + + legacyPrivateKeyShareBytes, err := legacySigner.privateKeyShare.Marshal() + if err != nil { + t.Fatalf("unexpected private key share marshal error: [%v]", err) + } + + encodedSigner, err := legacySigner.Marshal() + if err != nil { + t.Fatalf("unexpected signer marshal error: [%v]", err) + } + + unmarshaledSigner := &signer{} + if err := unmarshaledSigner.Unmarshal(encodedSigner); err != nil { + t.Fatalf("unexpected signer unmarshal error: [%v]", err) + } + + resolvedPrivateKeyShare, ok := unmarshaledSigner.signerMaterial.(*tecdsa.PrivateKeyShare) + if !ok { + t.Fatalf( + "unexpected signer material type after legacy unmarshal\nexpected: [%T]\nactual: [%T]", + &tecdsa.PrivateKeyShare{}, + unmarshaledSigner.signerMaterial, + ) + } + + resolvedPrivateKeyShareBytes, err := resolvedPrivateKeyShare.Marshal() + if err != nil { + t.Fatalf("unexpected resolved private key share marshal error: [%v]", err) + } + + if !bytes.Equal(legacyPrivateKeyShareBytes, resolvedPrivateKeyShareBytes) { + t.Fatalf( + "unexpected resolved private key share\nexpected: [%x]\nactual: [%x]", + legacyPrivateKeyShareBytes, + resolvedPrivateKeyShareBytes, + ) + } + + roundtripEncodedSigner, err := unmarshaledSigner.Marshal() + if err != nil { + t.Fatalf("unexpected roundtrip signer marshal error: [%v]", err) + } + + roundtripPBSigner := &pb.Signer{} + if err := proto.Unmarshal(roundtripEncodedSigner, roundtripPBSigner); err != nil { + t.Fatalf("unexpected roundtrip proto unmarshal error: [%v]", err) + } + + if bytes.HasPrefix(roundtripPBSigner.PrivateKeyShare, signerMaterialEnvelopePrefix) { + t.Fatal("expected legacy signer material to remain outside the native envelope") + } + + if !bytes.Equal(legacyPrivateKeyShareBytes, roundtripPBSigner.PrivateKeyShare) { t.Fatalf( - "expected scaffold-refusal error to reference %s; got: [%v]", - frostsigning.NativeTBTCSignerKeyGroupSourceLegacyWalletPubKey, - err, + "unexpected roundtrip private key share\nexpected: [%x]\nactual: [%x]", + legacyPrivateKeyShareBytes, + roundtripPBSigner.PrivateKeyShare, ) } } diff --git a/pkg/tbtc/signer_material_resolver_build_frost_native_test.go b/pkg/tbtc/signer_material_resolver_build_frost_native_test.go index ae472686fb..ee03a562ce 100644 --- a/pkg/tbtc/signer_material_resolver_build_frost_native_test.go +++ b/pkg/tbtc/signer_material_resolver_build_frost_native_test.go @@ -3,99 +3,10 @@ package tbtc import ( - "bytes" - "encoding/hex" - "encoding/json" "errors" "testing" - - frostsigning "github.com/keep-network/keep-core/pkg/frost/signing" - "github.com/keep-network/keep-core/pkg/tecdsa" ) -func TestRegisterSignerMaterialResolverForBuild_UsesDefaultProvider( - t *testing.T, -) { - // Default scaffold-era resolver builds legacy-wallet-pubkey signer - // material; production refuses it but local/CI tests can opt in. - t.Setenv(frostsigning.AcceptScaffoldKeyGroupEnvVar, "true") - - UnregisterSignerMaterialResolver() - UnregisterSignerMaterialResolverProviderForBuild() - t.Cleanup(UnregisterSignerMaterialResolver) - t.Cleanup(UnregisterSignerMaterialResolverProviderForBuild) - - err := RegisterSignerMaterialResolverForBuild() - if err != nil { - t.Fatalf("unexpected build resolver registration error: [%v]", err) - } - - privateKeyShare := createMockSigner(t).privateKeyShare - - result, err := resolveSignerMaterial(privateKeyShare) - if err != nil { - t.Fatalf("unexpected resolver error: [%v]", err) - } - - nativeSignerMaterial, ok := result.(*frostsigning.NativeSignerMaterial) - if !ok { - t.Fatalf( - "unexpected resolved signer material type\nexpected: [%T]\nactual: [%T]", - &frostsigning.NativeSignerMaterial{}, - result, - ) - } - - expectedPayload, err := privateKeyShare.Marshal() - if err != nil { - t.Fatalf("failed marshaling expected private key share: [%v]", err) - } - - if nativeSignerMaterial.Format != frostsigning.NativeSignerMaterialFormatFrostTBTCSignerV1 { - t.Fatalf( - "unexpected native signer material format\nexpected: [%s]\nactual: [%s]", - frostsigning.NativeSignerMaterialFormatFrostTBTCSignerV1, - nativeSignerMaterial.Format, - ) - } - - var payload frostsigning.NativeTBTCSignerMaterialPayload - if err := json.Unmarshal(nativeSignerMaterial.Payload, &payload); err != nil { - t.Fatalf("failed unmarshalling tbtc signer material payload: [%v]", err) - } - - if payload.KeyGroup == "" { - t.Fatal("expected non-empty tbtc-signer key group") - } - - if payload.KeyGroupSource == "" { - t.Fatal("expected non-empty tbtc-signer key group source") - } - - legacyPrivateKeySharePayload, err := hex.DecodeString(payload.LegacyPrivateKeyShareHex) - if err != nil { - t.Fatalf("failed decoding legacy private key share payload: [%v]", err) - } - - decodedPrivateKeyShare := &tecdsa.PrivateKeyShare{} - if err := decodedPrivateKeyShare.Unmarshal(legacyPrivateKeySharePayload); err != nil { - t.Fatalf("failed unmarshalling decoded private key share: [%v]", err) - } - - actualPayload, err := decodedPrivateKeyShare.Marshal() - if err != nil { - t.Fatalf("failed marshaling decoded private key share: [%v]", err) - } - - if !bytes.Equal(expectedPayload, actualPayload) { - t.Fatalf( - "unexpected resolved signer payload\nexpected: [%x]\nactual: [%x]", - expectedPayload, - actualPayload, - ) - } -} - func TestRegisterSignerMaterialResolverForBuild_UsesRegisteredProvider( t *testing.T, ) {