feat(tbtc): fail fast when FROST is enabled on the legacy signing backend#4133
Merged
mswilkison merged 8 commits intoJul 6, 2026
Merged
Conversation
…kend A node that participates in FROST DKG will also sign for the wallets it helps create. Native FROST wallets carry native signer material that the transitional legacy signing backend cannot process, so a node left on the default/legacy backend produces valid wallets via DKG but then fails every signing attempt (heartbeat, deposit sweep, redemption). DKG uses the native path directly, so wallet creation succeeds and masks the misconfiguration until the first signature. Fail fast at startup: when FROST DKG is enabled but the signing backend is legacy, return a clear error pointing at tbtc.frostSigningBackend (set it to "native" or "ffi"). The native backend handles both native FROST and legacy-ECDSA material, so it is always correct once FROST is enabled. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NuK4nVVvZQSgvBWsjQpuLN
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
… enabled The normal Ethereum TbtcChain satisfies FrostDKGChain even when no FROST wallet registry is configured, so the previous guard rejected the default legacy backend on ordinary non-FROST nodes and broke their startup. Gate the check on frostChain.FrostWalletRegistryAvailable() - the same signal initializeFrostDKGCoordinator uses to no-op when FROST is disabled. The helper now takes a frostEnabled bool and returns nil when FROST is off. Adds a test case covering the FROST-disabled path. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NuK4nVVvZQSgvBWsjQpuLN
…on-legacy backend name The fallback-allowed "native" mode selects a native backend name without verifying that native execution is actually available (only strict "ffi" mode checks). A FROST node on "native" with an unavailable native engine therefore passed the guard, then fell back to the legacy bridge and failed on native FROST signer material at signing time instead of startup. Add signing.NativeExecutionAvailable() (mirrors the strict-mode availability gate but is consultable in any mode) and require it in the guard in addition to the non-legacy backend check. Adds a test with an adapter that reports native execution unavailable. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NuK4nVVvZQSgvBWsjQpuLN
…llback availability
NativeExecutionAvailable delegated to the build-tagged adapter's
NativeExecutionAvailable, which in fallback-allowed ("native") mode reports
available whenever the legacy delegate is present - even on a frost_native build
where the native tbtc-signer FFI engine failed to register or is not linked. The
guard could therefore accept native/ffi and let the node start, only to fall
back to legacy (or return ErrNativeCryptographyUnavailable) when signing
DKG-persisted FROST wallets.
Check currentNativeExecutionFFIExecutor() != nil instead - the same strict path
the native bridge uses (currentFFIExecutor() != nil) - so a FROST-enabled node
without a real native engine fails fast at startup. Test now registers a native
FFI executor for the accepted case and omits it for the rejected case.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NuK4nVVvZQSgvBWsjQpuLN
…transitional FFI wrapper On a frost_native build not linked with the native signer (frost_tbtc_signer/cgo), the build init still registers the transitional NativeExecutionFFIExecutor wrapper, so currentNativeExecutionFFIExecutor() != nil returned true even with no real tbtc-signer engine present. A FROST-enabled node on native/ffi then passed the guard and started, only to fall back to legacy (or hit ErrNativeCryptographyUnavailable) on DKG-persisted FROST signer material. NativeExecutionAvailable now checks the actual linked signer engine via a build-tag-split helper: currentNativeTBTCSignerEngine() != nil on frost_native builds (the engine is registered only by frost_native && frost_tbtc_signer && cgo - the same engine the transitional primitive requires before executing natively), and false on non-frost_native builds. Adds a signing-package test for NativeExecutionAvailable and a guard test for native-without-a-linked-engine. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NuK4nVVvZQSgvBWsjQpuLN
…gning available The build-tagged buildTaggedTBTCSignerEngine wrapper is registered even when libfrost_tbtc is not actually loaded or lacks the required ABI symbol - it only discovers that later via dlsym/ABI preflight when an operation runs. So currentNativeTBTCSignerEngine() != nil alone still let the FROST startup guard pass on a frost_native && frost_tbtc_signer && cgo build with an absent/incompatible library, and the node would start only to fail DKG/signing at runtime. nativeSignerEngineAvailable now also requires ensureTBTCSignerABICompatible() == nil - the same cached dlsym probe of frost_tbtc_abi_version the wrapper uses at operation time, which returns nil only for a present, ABI-compatible library. Split into build-tag variants: signer (engine registered AND ABI probe passes), no-signer frost_native (false), and non-frost_native (false). Adds a test for the engine-registered-but-ABI-probe-fails path via the tbtcSignerABI test hook. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NuK4nVVvZQSgvBWsjQpuLN
…assertions in optional-link profile Two review follow-ups: - The guard ran after node.runCoordinationLayer(ctx), so an invalid FROST backend started the coordination-window watcher and result processor before Initialize returned the error - a caller that did not immediately cancel the context could keep doing protocol work with a bad backend. Move the check before the coordination layer starts so the fail-closed path has no protocol side effects. - The native-availability tests asserted NativeExecutionAvailable()/guard-accepts in the optional-link dev profile (frost_native frost_tbtc_signer cgo tags compiled without linking libfrost_tbtc via dlsym), where the ABI probe correctly reports unavailable - failing `go test` unless CGO_LDFLAGS provided the signer library. Skip the positive subtests when the ABI probe reports unavailable, in both pkg/frost/signing and the pkg/tbtc guard test. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NuK4nVVvZQSgvBWsjQpuLN
…ams pool starts The guard previously ran in Initialize after newNode returned. newNode, with the default PreParamsPoolSize > 0 and legacy ECDSA not disabled, constructs the legacy DKG executor (newDkgExecutor) which starts a pre-params pool scheduling CPU-heavy generation/persistence on a background context. So an invalid FROST backend failed Initialize while that background work kept running. Move the check into newNode, right after configureFrostSigningBackend configures the backend and before newDkgExecutor starts the pre-params pool, so the fail-closed path has no pre-params (or later coordination) side effects. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NuK4nVVvZQSgvBWsjQpuLN
mswilkison
merged commit ffc337e
into
feat/frost-schnorr-migration-scaffold
26 of 27 checks passed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
A node that participates in FROST DKG will also sign for the wallets it helps create. Native FROST wallets carry native signer material (
*signing.NativeSignerMaterial) that the transitional legacy signing backend cannot process.The backend is selected by
tbtc.frostSigningBackend(default:legacy). Because DKG uses the native path directly, a node left on the default/legacy backend creates wallets successfully but then fails every signing attempt:This silent, delayed failure was hit during a live end-to-end FROST rehearsal — DKG succeeded and the wallet registered, but every heartbeat signature failed until
tbtc.frostSigningBackend=nativewas set.Fix
Fail fast at startup: when FROST DKG is enabled but the signing backend is
legacy, return a clear error pointing attbtc.frostSigningBackend(set it tonativeorffi). Thenativebackend handles both native FROST and legacy-ECDSA material, so it is always correct once FROST is enabled.Test
TestVerifyFrostSigningBackendForFrost— legacy rejected, native accepted.🤖 Generated with Claude Code
https://claude.ai/code/session_01NuK4nVVvZQSgvBWsjQpuLN