From bd194844c4a3e0639cbc91a218af9ad99197dc92 Mon Sep 17 00:00:00 2001 From: Leonardo Saturnino Date: Sat, 12 Sep 2026 03:05:27 -0400 Subject: [PATCH 1/4] fix(firewall): retire the beacon branch from peer admission Peer admission was a disjunction over two applications, evaluated beacon first: a peer was admitted if its staking provider carried a legacy TokenStaking delegation, or if its provider held eligible stake in the wallet registry. TIP-092 removed every function that writes the legacy delegation record, so the first branch admits a frozen set of identities that nobody can add to or remove from, and a Council decision to zero a provider's Allowlist weight never reaches the network layer while that branch answers first. Admission now consults the tBTC application alone. A peer is admitted when it is a registered tBTC operator whose provider has positive WalletRegistry eligible stake, which the Council controls through the Allowlist, and a weight decrease requested there takes effect at the next validation. The beacon handle no longer satisfies firewall.Application at all: the recognition method, its reader interface and the production adapter are deleted rather than stubbed, so putting the beacon back into the application list fails to compile. Beacon startup registration, protocol initialization and the shared RolesOf accessor are untouched; none of them was an admission authority. Measured over every operator ever registered on either registry, 262 identities lose admission with this change. None of them holds tBTC sortition pool membership or weight. Rolling back re-admits that legacy set, so a rollback is a policy decision rather than an operator convenience, and this change is only correct on top of the eligible-stake predicate it builds on. The admission fixture is tBTC-only and its traces prove that no beacon read happens on the admission path, even when beacon reads are made to fail after construction. Startup composition is asserted on the policy the client actually hands the network layer, in ordinary and bootstrap mode alike; revocation is exercised end to end through the real watchtower guard; and four mutations - disabling the policy, restoring the legacy-delegation decision, caching an RPC failure as a denial, and caching positive decisions - each fail a named assertion. --- cmd/start.go | 13 +- cmd/start_test.go | 308 +++++++++++++++--- internal/ethtest/admission.go | 101 ++---- .../ethereum/admission_production_test.go | 103 ++---- pkg/chain/ethereum/admission_recovery_test.go | 57 +++- pkg/chain/ethereum/beacon.go | 108 ------ pkg/chain/ethereum/beacon_recognition_test.go | 184 ----------- .../ethereum/ethereum_integration_test.go | 126 +++---- pkg/chain/ethereum/tbtc.go | 33 +- 9 files changed, 442 insertions(+), 591 deletions(-) delete mode 100644 pkg/chain/ethereum/beacon_recognition_test.go diff --git a/cmd/start.go b/cmd/start.go index 8c5372d185..acd75a8f8e 100644 --- a/cmd/start.go +++ b/cmd/start.go @@ -73,7 +73,7 @@ func start(cmd *cobra.Command) error { netProvider, err := initializeNetwork( ctx, - admissionApplications(beaconChain, tbtcChain), + admissionApplications(tbtcChain), operatorPrivateKey, blockCounter, ) @@ -199,14 +199,15 @@ func isBootstrap() bool { return clientConfig.LibP2P.Bootstrap } -// admissionApplications lists the chain handles a peer can be recognized by, -// in the order the firewall policy evaluates them. The beacon comes first -// because its predicate is the one that still sees legacy stake delegations. +// admissionApplications lists the chain handles that can authorize a peer. +// Network admission follows the wallet registry's current eligible stake so +// legacy beacon registrations do not bypass authorization changes. +// Random Beacon initialization still requires its operator registration, but +// that startup prerequisite is not a network admission authority. func admissionApplications( - beaconChain *ethereum.BeaconChain, tbtcChain *ethereum.TbtcChain, ) []firewall.Application { - return []firewall.Application{beaconChain, tbtcChain} + return []firewall.Application{tbtcChain} } // admissionPolicy builds the firewall policy guarding peer connections. The diff --git a/cmd/start_test.go b/cmd/start_test.go index ebd1a07ac8..71a4b69db3 100644 --- a/cmd/start_test.go +++ b/cmd/start_test.go @@ -6,7 +6,9 @@ import ( "reflect" "strings" "testing" + "time" + "github.com/ethereum/go-ethereum/common" "github.com/keep-network/keep-core/config" "github.com/keep-network/keep-core/internal/ethtest" "github.com/keep-network/keep-core/internal/testutils" @@ -14,7 +16,9 @@ import ( "github.com/keep-network/keep-core/pkg/firewall" "github.com/keep-network/keep-core/pkg/net" "github.com/keep-network/keep-core/pkg/net/libp2p" + localnet "github.com/keep-network/keep-core/pkg/net/local" "github.com/keep-network/keep-core/pkg/net/retransmission" + "github.com/keep-network/keep-core/pkg/net/watchtower" "github.com/keep-network/keep-core/pkg/operator" "github.com/spf13/cobra" ) @@ -82,28 +86,61 @@ func TestIsBootstrap(t *testing.T) { func TestStart_AdmissionHandoff(t *testing.T) { backend := ethtest.New(t, ethtest.AdmissionState(t)) - configured := clientConfig.Ethereum - t.Cleanup(func() { clientConfig.Ethereum = configured }) + configuredEthereum := clientConfig.Ethereum + configuredNetwork := clientConfig.LibP2P + t.Cleanup(func() { + clientConfig.Ethereum = configuredEthereum + clientConfig.LibP2P = configuredNetwork + }) + clientConfig.Ethereum = backend.ChainConfig(t) + clientConfig.LibP2P.Bootstrap = false + clientConfig.LibP2P.Peers = []string{"ordinary-configured-discovery-peer"} - policy := captureAdmissionPolicy(t, func() error { + handoff := captureAdmissionPolicy(t, func() error { return start(&cobra.Command{}) }) // What the policy admits comes first: the assertions below give up on a // policy whose shape they cannot read, and stopping there would leave the // verdicts unexamined. - assertAdmissionTable(t, backend, policy) - assertNoStaticBypass(t, policy) + assertAdmissionTable(t, backend, handoff.policy) + assertNoStaticBypass(t, handoff.policy) + assertNetworkConfig(t, clientConfig.LibP2P, handoff.networkConfig) // start builds its own chain handles, so there is no instance here to - // compare them against; what it handed over is pinned by type and order. + // compare it against; what it handed over is pinned by type. assertAdmissionApplicationTypes( t, - policy, - reflect.TypeOf((*ethereum.BeaconChain)(nil)), + handoff.policy, reflect.TypeOf((*ethereum.TbtcChain)(nil)), ) + + t.Run("bootstrap_with_configured_discovery_peers", func(t *testing.T) { + bootstrapBackend := ethtest.New(t, ethtest.AdmissionState(t)) + clientConfig.Ethereum = bootstrapBackend.ChainConfig(t) + clientConfig.LibP2P.Bootstrap = true + clientConfig.LibP2P.Peers = []string{ + "bootstrap-configured-discovery-peer", + } + + bootstrapHandoff := captureAdmissionPolicy(t, func() error { + return start(&cobra.Command{}) + }) + + assertAdmissionTable(t, bootstrapBackend, bootstrapHandoff.policy) + assertNoStaticBypass(t, bootstrapHandoff.policy) + assertNetworkConfig( + t, + clientConfig.LibP2P, + bootstrapHandoff.networkConfig, + ) + assertAdmissionApplicationTypes( + t, + bootstrapHandoff.policy, + reflect.TypeOf((*ethereum.TbtcChain)(nil)), + ) + }) } // TestInitializeNetwork_AdmissionComposition covers the assembly the client @@ -120,38 +157,56 @@ func TestInitializeNetwork_AdmissionComposition(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) defer cancel() - beaconChain, tbtcChain, blockCounter, _, operatorPrivateKey, err := + _, tbtcChain, blockCounter, _, operatorPrivateKey, err := ethereum.Connect(ctx, backend.ChainConfig(t)) if err != nil { t.Fatalf("failed to connect to the fixture: %v", err) } - applications := admissionApplications(beaconChain, tbtcChain) + applications := admissionApplications(tbtcChain) - testutils.AssertIntsEqual(t, "applications", 2, len(applications)) - if applications[0] != firewall.Application(beaconChain) { - t.Error("the beacon chain is not the first application evaluated") - } - if applications[1] != firewall.Application(tbtcChain) { - t.Error("the tBTC chain is not the second application evaluated") + testutils.AssertIntsEqual(t, "applications", 1, len(applications)) + if applications[0] != firewall.Application(tbtcChain) { + t.Error("the tBTC chain is not the application evaluated") } - policy := captureAdmissionPolicy(t, func() error { - _, err := initializeNetwork( - ctx, - applications, - operatorPrivateKey, - blockCounter, - ) - return err - }) + configuredNetwork := clientConfig.LibP2P + t.Cleanup(func() { clientConfig.LibP2P = configuredNetwork }) - assertAdmissionTable(t, backend, policy) - assertNoStaticBypass(t, policy) + configurations := []struct { + name string + bootstrap bool + peer string + }{ + {"ordinary_with_configured_discovery_peers", false, "ordinary-configured-discovery-peer"}, + {"bootstrap_with_configured_discovery_peers", true, "bootstrap-configured-discovery-peer"}, + } - // The policy guards with the very handles it was given, in that order, - // rather than with a set assembled somewhere between here and the network. - assertAdmissionApplications(t, policy, beaconChain, tbtcChain) + for _, configuration := range configurations { + t.Run(configuration.name, func(t *testing.T) { + clientConfig.LibP2P = configuredNetwork + clientConfig.LibP2P.Bootstrap = configuration.bootstrap + clientConfig.LibP2P.Peers = []string{configuration.peer} + + handoff := captureAdmissionPolicy(t, func() error { + _, err := initializeNetwork( + ctx, + applications, + operatorPrivateKey, + blockCounter, + ) + return err + }) + + assertAdmissionTable(t, backend, handoff.policy) + assertNoStaticBypass(t, handoff.policy) + assertNetworkConfig(t, clientConfig.LibP2P, handoff.networkConfig) + + // The policy guards with the very handle it was given rather than + // with one assembled somewhere between here and the network. + assertAdmissionApplications(t, handoff.policy, tbtcChain) + }) + } t.Run("what a static bypass looks like", func(t *testing.T) { // The counterexample the assertions above are read against, built as a @@ -179,6 +234,132 @@ func TestInitializeNetwork_AdmissionComposition(t *testing.T) { }) } +// TestStart_AdmissionRevocation verifies that a successful decision is read +// from the wallet registry again. Legacy ownership cannot preserve admission +// after eligible stake is revoked. +func TestStart_AdmissionRevocation(t *testing.T) { + backend := ethtest.New(t, ethtest.AdmissionState(t)) + revoked := ethtest.AdmissionCaseNamed(t, "legacy_revoked") + backend.SetEligibleStake(revoked.StakingProvider(t), ethtest.TTokens(40_000)) + + configuredEthereum := clientConfig.Ethereum + t.Cleanup(func() { clientConfig.Ethereum = configuredEthereum }) + clientConfig.Ethereum = backend.ChainConfig(t) + + policy := captureAdmissionPolicy(t, func() error { + return start(&cobra.Command{}) + }).policy + + revokedKey := caseOperatorKey(t, revoked) + backend.ResetCalls() + if err := policy.Validate(revokedKey); err != nil { + t.Fatalf("expected the peer to be initially admitted: %v", err) + } + backend.AssertTrace(t, "initial admission reads", revoked.AdmissionReads(t)...) + + backend.SetEligibleStake(revoked.StakingProvider(t), ethtest.TTokens(0)) + backend.ResetCalls() + testutils.AssertErrorsSame(t, firewall.ErrNotRecognized, policy.Validate(revokedKey)) + backend.AssertTrace(t, "revocation reads", revoked.AdmissionReads(t)...) + + eligible := ethtest.AdmissionCaseNamed(t, "post_legacy_authorized") + backend.ResetCalls() + if err := policy.Validate(caseOperatorKey(t, eligible)); err != nil { + t.Fatalf("expected the eligible control to be admitted: %v", err) + } + backend.AssertTrace(t, "eligible control reads", eligible.AdmissionReads(t)...) + + unregistered := ethtest.AdmissionCaseNamed(t, "unregistered") + backend.ResetCalls() + testutils.AssertErrorsSame( + t, + firewall.ErrNotRecognized, + policy.Validate(caseOperatorKey(t, unregistered)), + ) + backend.AssertTrace( + t, + "unregistered control reads", + unregistered.AdmissionReads(t)..., + ) + backend.AssertNoUnexpectedCalls(t) +} + +// TestStart_AdmissionRevocationDisconnects verifies that the production policy +// is also re-evaluated by the watchtower for established connections. +func TestStart_AdmissionRevocationDisconnects(t *testing.T) { + backend := ethtest.New(t, ethtest.AdmissionState(t)) + revoked := ethtest.AdmissionCaseNamed(t, "legacy_revoked") + eligible := ethtest.AdmissionCaseNamed(t, "post_legacy_authorized") + backend.SetEligibleStake(revoked.StakingProvider(t), ethtest.TTokens(40_000)) + + configuredEthereum := clientConfig.Ethereum + t.Cleanup(func() { clientConfig.Ethereum = configuredEthereum }) + clientConfig.Ethereum = backend.ChainConfig(t) + + policy := captureAdmissionPolicy(t, func() error { + return start(&cobra.Command{}) + }).policy + + revokedKey := caseOperatorKey(t, revoked) + eligibleKey := caseOperatorKey(t, eligible) + if err := policy.Validate(revokedKey); err != nil { + t.Fatalf("expected the peer to be initially admitted: %v", err) + } + if err := policy.Validate(eligibleKey); err != nil { + t.Fatalf("expected the control peer to be admitted: %v", err) + } + + backend.SetEligibleStake(revoked.StakingProvider(t), ethtest.TTokens(0)) + backend.ResetCalls() + + provider := localnet.Connect() + const revokedPeer = "revoked-peer" + const eligiblePeer = "eligible-peer" + provider.AddPeer(revokedPeer, revokedKey) + provider.AddPeer(eligiblePeer, eligibleKey) + + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + watchtower.NewGuard( + ctx, + &testutils.MockLogger{}, + 10*time.Millisecond, + policy, + provider.ConnectionManager(), + ) + + deadline := time.NewTimer(5 * time.Second) + defer deadline.Stop() + poll := time.NewTicker(5 * time.Millisecond) + defer poll.Stop() + + for { + connected := connectedPeerSet(provider.ConnectionManager()) + eligibleChecked := operatorLookupObserved(backend, eligible.Operator(t)) + if !connected[revokedPeer] && connected[eligiblePeer] && eligibleChecked { + break + } + + select { + case <-deadline.C: + t.Fatalf( + "watchtower did not converge; connected peers: %v; chain calls: %v", + connected, + backend.Calls(), + ) + case <-poll.C: + } + } + + if backend.CallCount(ethtest.RandomBeaconContract, "operatorToStakingProvider") != 0 { + t.Error("watchtower admission read the beacon operator mapping") + } + if backend.CallCount(ethtest.TokenStakingContract, "rolesOf") != 0 { + t.Error("watchtower admission read legacy token staking roles") + } + backend.AssertNoUnexpectedCalls(t) +} + // TestStaticBypassKeys_ReportsAnAllowedKey is the negative control for // assertNoStaticBypass: a policy carrying one unrelated key has to be reported // as bypassing the chain. Without it, an assertion that only ever sees empty @@ -219,28 +400,34 @@ func TestStaticBypassKeys_ReportsAnAllowedKey(t *testing.T) { var errNetworkNotOpened = errors.New("the network provider is not opened here") // captureAdmissionPolicy runs a piece of the client's start path with the -// network provider constructor stubbed out, and returns the firewall that path -// handed it. The stub fails, so the path unwinds at the handoff and nothing -// behind it is started. -func captureAdmissionPolicy(t *testing.T, run func() error) net.Firewall { +// network provider constructor stubbed out, and returns the configuration and +// firewall handed to it. The stub fails, so the path unwinds at the handoff and +// nothing behind it is started. +type admissionHandoff struct { + networkConfig libp2p.Config + policy net.Firewall +} + +func captureAdmissionPolicy(t *testing.T, run func() error) admissionHandoff { t.Helper() - var captured net.Firewall + var captured admissionHandoff handoffs := 0 opened := connectNetwork - t.Cleanup(func() { connectNetwork = opened }) + defer func() { connectNetwork = opened }() connectNetwork = func( _ context.Context, - _ libp2p.Config, + config libp2p.Config, _ *operator.PrivateKey, policy net.Firewall, _ *retransmission.Ticker, _ ...libp2p.ConnectOption, ) (net.Provider, error) { handoffs++ - captured = policy + captured.networkConfig = config + captured.policy = policy return nil, errNetworkNotOpened } @@ -254,19 +441,51 @@ func captureAdmissionPolicy(t *testing.T, run func() error) net.Firewall { testutils.AssertIntsEqual(t, "network handoffs", 1, handoffs) - if captured == nil { + if captured.policy == nil { t.Fatal("the network layer was opened with no firewall") } return captured } +func assertNetworkConfig(t *testing.T, expected, actual libp2p.Config) { + t.Helper() + + if !reflect.DeepEqual(expected, actual) { + t.Errorf("unexpected network configuration\nexpected: %v\nactual: %v", expected, actual) + } +} + +func connectedPeerSet(connectionManager net.ConnectionManager) map[string]bool { + connected := make(map[string]bool) + for _, peer := range connectionManager.ConnectedPeers() { + connected[peer] = true + } + + return connected +} + +func operatorLookupObserved( + backend *ethtest.Backend, + operatorAddress common.Address, +) bool { + for _, call := range backend.CallsTo( + ethtest.WalletRegistryContract, + "operatorToStakingProvider", + ) { + if len(call.Args) == 1 && call.Args[0] == operatorAddress { + return true + } + } + + return false +} + // assertAdmissionTable drives every identity of the fixed admission table // through the given policy and fails unless both the verdict and the reads // that verdict cost are the ones the table pins. The reads are asserted // alongside the verdict because a verdict alone cannot tell an on-chain -// decision apart from a bypass, nor beacon-first evaluation apart from the -// reverse. +// decision apart from a bypass or a legacy fallback. func assertAdmissionTable( t *testing.T, backend *ethtest.Backend, @@ -290,10 +509,9 @@ func assertAdmissionTable( testutils.AssertErrorsSame(t, firewall.ErrNotRecognized, err) } - // The beacon branch opens every one of these reads, so this - // identity was judged by the chain rather than short-circuited - // ahead of it, and the tBTC branch is read only where the beacon - // declined. + // Every expected read belongs to the wallet registry, so this + // identity was judged by current eligibility without a static or + // legacy fallback. backend.AssertTrace( t, "admission reads", diff --git a/internal/ethtest/admission.go b/internal/ethtest/admission.go index 8848a4dba2..b9e02a9375 100644 --- a/internal/ethtest/admission.go +++ b/internal/ethtest/admission.go @@ -8,10 +8,9 @@ import ( ) // AdmissionCase is one synthetic operator identity together with the results -// the admission predicates are pinned to produce for it. The three result -// fields are the table this fixture exists to hold fixed; they are stated here -// rather than computed from the state below, so that a test asserting them -// cannot derive its expectation from the behaviour under test. +// the admission predicate and policy are pinned to produce for it. The result +// fields are stated here rather than computed from the state below, so a test +// asserting them cannot derive its expectation from the behaviour under test. type AdmissionCase struct { Name string @@ -28,9 +27,8 @@ type AdmissionCase struct { PendingDecrease *big.Int // The pinned outcome. - BeaconRecognizes bool - TbtcRecognizes bool - Admitted bool + TbtcRecognizes bool + Admitted bool } // StakingProvider is the single staking-provider address this identity uses on @@ -48,35 +46,10 @@ func (c AdmissionCase) Operator(t *testing.T) common.Address { return Address(t, c.OperatorKey) } -// BeaconReads is the trace the beacon predicate must produce for this -// identity: the registry mapping, and the roles behind it only when the -// registry holds one. Every entry comes from the table's own description of -// the identity, never from what a predicate did. -func (c AdmissionCase) BeaconReads(t *testing.T) []ExpectedCall { - t.Helper() - - reads := []ExpectedCall{ - Read( - RandomBeaconContract, - "operatorToStakingProvider", - c.Operator(t), - ), - } - - if c.RegisteredWithBeacon { - reads = append( - reads, - Read(TokenStakingContract, "rolesOf", c.StakingProvider(t)), - ) - } - - return reads -} - -// TbtcReads is the same for the tBTC predicate: the registry mapping, and -// eligible stake only when the registry holds a mapping. A pending -// authorization decrease is seeded for some identities and is not an admission -// credential, so it never appears in a trace. +// TbtcReads is the trace for the tBTC predicate: the registry mapping, and +// eligible stake only when the registry holds a mapping. A pending authorization +// decrease is seeded for some identities and is not an admission credential, so +// it never appears in a trace. func (c AdmissionCase) TbtcReads(t *testing.T) []ExpectedCall { t.Helper() @@ -98,35 +71,28 @@ func (c AdmissionCase) TbtcReads(t *testing.T) []ExpectedCall { return reads } -// AdmissionReads is what validating this identity through the combined policy -// must cost: the beacon branch, and the tBTC branch only when the beacon -// declines. +// AdmissionReads is what validating this identity through the production +// policy must cost. func (c AdmissionCase) AdmissionReads(t *testing.T) []ExpectedCall { t.Helper() - reads := c.BeaconReads(t) - if !c.BeaconRecognizes { - reads = append(reads, c.TbtcReads(t)...) - } - - return reads + return c.TbtcReads(t) } -// AdmissionCases returns the fixed admission table. Beacon recognition follows -// a legacy token staking delegation; tBTC recognition follows eligible stake; -// admission is the disjunction of the two, evaluated beacon first. +// AdmissionCases returns the fixed admission table. Admission follows tBTC +// registration and positive eligible stake; beacon registration and legacy +// token staking roles are present only to catch accidental fallback to them. func AdmissionCases() []AdmissionCase { return []AdmissionCase{ { // Authorized for the wallet registry after legacy token staking - // stopped recording delegations, so it has no beacon backstop. + // stopped recording delegations. Name: "post_legacy_authorized", OperatorKey: 11, StakingProviderKey: 21, RegisteredWithBeacon: true, RegisteredWithTbtc: true, EligibleStake: TTokens(40_000_000), - BeaconRecognizes: false, TbtcRecognizes: true, Admitted: true, }, @@ -141,13 +107,12 @@ func AdmissionCases() []AdmissionCase { HasLegacyDelegation: true, EligibleStake: big.NewInt(0), PendingDecrease: TTokens(30_000), - BeaconRecognizes: true, TbtcRecognizes: false, - Admitted: true, + Admitted: false, }, { - // Authorization is gone and nothing is pending, but the legacy - // delegation the beacon reads is permanent. + // Authorization is gone and nothing is pending, but a legacy + // delegation remains recorded. // See https://github.com/threshold-network/keep-core/issues/4335 Name: "legacy_revoked", OperatorKey: 13, @@ -156,35 +121,43 @@ func AdmissionCases() []AdmissionCase { RegisteredWithTbtc: true, HasLegacyDelegation: true, EligibleStake: big.NewInt(0), - BeaconRecognizes: true, TbtcRecognizes: false, - Admitted: true, + Admitted: false, }, { // Known to the wallet registry alone, by a provider that holds a - // legacy delegation and no authorization. Reading the delegation - // through the wallet registry admitted it; reading eligible stake - // does not, and it has no beacon registration to fall back on. + // legacy delegation and no authorization. A roles-based fallback + // would admit it, while eligible stake does not. Name: "registry_only_legacy", OperatorKey: 14, StakingProviderKey: 24, RegisteredWithTbtc: true, HasLegacyDelegation: true, EligibleStake: big.NewInt(0), - BeaconRecognizes: false, TbtcRecognizes: false, Admitted: false, }, { - // Carried by the beacon branch alone. + // Positive wallet-registry eligibility is sufficient even when the + // operator was never registered with the beacon. + Name: "tbtc_only_authorized", + OperatorKey: 18, + StakingProviderKey: 28, + RegisteredWithTbtc: true, + EligibleStake: TTokens(40_000), + TbtcRecognizes: true, + Admitted: true, + }, + { + // Registered only with the beacon and backed by a legacy + // delegation. Name: "beacon_only", OperatorKey: 15, StakingProviderKey: 25, RegisteredWithBeacon: true, HasLegacyDelegation: true, - BeaconRecognizes: true, TbtcRecognizes: false, - Admitted: true, + Admitted: false, }, { // Registered with both registries and authorized by neither. @@ -194,7 +167,6 @@ func AdmissionCases() []AdmissionCase { RegisteredWithBeacon: true, RegisteredWithTbtc: true, EligibleStake: big.NewInt(0), - BeaconRecognizes: false, TbtcRecognizes: false, Admitted: false, }, @@ -203,7 +175,6 @@ func AdmissionCases() []AdmissionCase { Name: "unregistered", OperatorKey: 17, StakingProviderKey: 27, - BeaconRecognizes: false, TbtcRecognizes: false, Admitted: false, }, diff --git a/pkg/chain/ethereum/admission_production_test.go b/pkg/chain/ethereum/admission_production_test.go index a0d4d07f4d..d12c743a49 100644 --- a/pkg/chain/ethereum/admission_production_test.go +++ b/pkg/chain/ethereum/admission_production_test.go @@ -16,11 +16,13 @@ import ( "github.com/keep-network/keep-core/pkg/operator" ) +var _ firewall.Application = (*TbtcChain)(nil) + // connectAdmissionFixture builds both production chain handles through the // public Connect path against a deterministic endpoint serving the fixed -// admission table. Nothing between the JSON-RPC boundary and the predicates is +// admission table. Nothing between the JSON-RPC boundary and the predicate is // substituted: the generated bindings, the chain handle constructors and the -// production admission adapters are the code under test. +// production admission adapter are the code under test. func connectAdmissionFixture(t *testing.T) ( *ethtest.Backend, *BeaconChain, @@ -78,63 +80,9 @@ func caseOperatorKey( return operatorPublicKey } -// TestBeaconChain_IsRecognized_ProductionAdapter drives the beacon branch of -// the fixed admission table through beaconAdmissionReaderChain, the adapter -// newBeaconChain installs, and through the token staking binding newBaseChain -// configures. Every read is answered by the deterministic endpoint, so the -// result is produced by the production code path rather than by a stand-in -// predicate. -func TestBeaconChain_IsRecognized_ProductionAdapter(t *testing.T) { - backend, beaconChain, _ := connectAdmissionFixture(t) - - adapter, ok := beaconChain.admission.(*beaconAdmissionReaderChain) - if !ok { - t.Fatalf( - "beacon admission is served by [%T], not the production adapter", - beaconChain.admission, - ) - } - if adapter.randomBeacon != beaconChain.randomBeacon { - t.Error("the beacon adapter reads a different RandomBeacon binding") - } - if adapter.baseChain != beaconChain.baseChain { - t.Error("the beacon adapter reads a different base chain") - } - - for _, admissionCase := range ethtest.AdmissionCases() { - t.Run(admissionCase.Name, func(t *testing.T) { - backend.ResetCalls() - - isRecognized, err := beaconChain.IsRecognized( - caseOperatorKey(t, admissionCase), - ) - if err != nil { - t.Fatal(err) - } - - testutils.AssertBoolsEqual( - t, - "beacon recognition", - admissionCase.BeaconRecognizes, - isRecognized, - ) - - // An operator the registry does not know costs exactly one read: - // the predicate must not go on to ask token staking about the - // zero address. - backend.AssertTrace( - t, - "beacon recognition reads", - admissionCase.BeaconReads(t)..., - ) - backend.AssertNoUnexpectedCalls(t) - }) - } -} - -// TestTbtcChain_IsRecognized_ProductionAdapter drives the tBTC branch of the -// same table through the WalletRegistry binding newTbtcChain installs as the -// admission reader. +// TestTbtcChain_IsRecognized_ProductionAdapter drives the admission table +// through the WalletRegistry binding newTbtcChain installs as the admission +// reader. func TestTbtcChain_IsRecognized_ProductionAdapter(t *testing.T) { backend, _, tbtcChain := connectAdmissionFixture(t) @@ -177,6 +125,15 @@ func TestTbtcChain_IsRecognized_ProductionAdapter(t *testing.T) { } } +// TestBeaconChain_DoesNotImplementFirewallApplication keeps beacon operations +// available without allowing the chain handle to become an admission source. +func TestBeaconChain_DoesNotImplementFirewallApplication(t *testing.T) { + var candidate interface{} = (*BeaconChain)(nil) + if _, ok := candidate.(firewall.Application); ok { + t.Fatal("the beacon chain implements firewall.Application") + } +} + // TestTbtcChain_IsRecognized_AtMinimumAuthorization verifies the plumbing // (seed-write-read roundtrip) through the production adapter when seeding // eligible stake at the minimum authorization level read from the registry. @@ -245,29 +202,21 @@ func TestTbtcChain_IsRecognized_AtMinimumAuthorization(t *testing.T) { backend.AssertNoUnexpectedCalls(t) } -// TestAdmission_ProductionConstruction asserts what the public construction -// path assembles and how the assembled handles behave together. Recognition is -// evaluated beacon first over an empty static allow list, which is the -// composition the client starts with. +// TestAdmission_ProductionConstruction asserts that the public construction +// path supplies the production WalletRegistry binding used for admission. func TestAdmission_ProductionConstruction(t *testing.T) { backend, beaconChain, tbtcChain := connectAdmissionFixture(t) if beaconChain.baseChain != tbtcChain.baseChain { t.Error("the two chain handles were given different base chains") } - if _, ok := beaconChain.admission.(*beaconAdmissionReaderChain); !ok { - t.Errorf( - "beacon admission is served by [%T], not the production adapter", - beaconChain.admission, - ) - } if tbtcChain.admission != tbtcAdmissionReader(tbtcChain.walletRegistry) { t.Error("tBTC admission is not the constructed WalletRegistry binding") } allowList := firewall.EmptyAllowList() policy := firewall.AnyApplicationPolicy( - []firewall.Application{beaconChain, tbtcChain}, + []firewall.Application{tbtcChain}, allowList, ) @@ -293,10 +242,8 @@ func TestAdmission_ProductionConstruction(t *testing.T) { testutils.AssertErrorsSame(t, firewall.ErrNotRecognized, err) } - // Applications are evaluated beacon first: once the beacon - // recognizes a peer, the tBTC branch is never consulted. The - // beacon branch is always read, so nothing settled this identity - // ahead of the chain. + // Only WalletRegistry reads are expected, so neither the static + // allow list nor legacy beacon state settled this identity. backend.AssertTrace( t, "admission reads", @@ -307,10 +254,10 @@ func TestAdmission_ProductionConstruction(t *testing.T) { } } -// TestBaseChain_RolesOf_ProductionBinding pins the contract the beacon branch -// reads stake delegations from. The allowlist is a separate deployment holding -// its own role mapping, and reading roles from it instead of from token -// staking would silently answer a different question. +// TestBaseChain_RolesOf_ProductionBinding pins the TokenStaking read retained +// for callers outside admission. The allowlist is a separate deployment +// holding its own role mapping, and reading roles from it would silently answer +// a different question. func TestBaseChain_RolesOf_ProductionBinding(t *testing.T) { backend, beaconChain, _ := connectAdmissionFixture(t) diff --git a/pkg/chain/ethereum/admission_recovery_test.go b/pkg/chain/ethereum/admission_recovery_test.go index 7fdd0b0cb3..7327c597c1 100644 --- a/pkg/chain/ethereum/admission_recovery_test.go +++ b/pkg/chain/ethereum/admission_recovery_test.go @@ -12,33 +12,24 @@ import ( ) // TestAdmission_TransientChainFaultIsNotADenial walks every chain read the -// admission predicates still perform and, for each of them in turn, makes the +// admission predicate performs and, for each of them in turn, makes the // endpoint fail. A read failing says nothing about the peer, so validation has // to surface the failure rather than report a non-recognition: the firewall // caches a non-recognition for an hour and would lock a legitimate peer out // for that hour over a momentary fault. Once the fault clears, the very next // validation must read the chain again and admit. // -// The identity used throughout is admitted by the tBTC branch alone, so no -// earlier branch can recognize it first and hide the read under test. +// The identity used throughout is admitted by positive eligible stake. func TestAdmission_TransientChainFaultIsNotADenial(t *testing.T) { var tests = map[string]struct { contract string method string }{ - "beacon operator lookup": { - contract: ethtest.RandomBeaconContract, - method: "operatorToStakingProvider", - }, - "beacon roles lookup": { - contract: ethtest.TokenStakingContract, - method: "rolesOf", - }, - "tbtc operator lookup": { + "tbtc_operator_lookup": { contract: ethtest.WalletRegistryContract, method: "operatorToStakingProvider", }, - "tbtc eligible stake lookup": { + "tbtc_eligible_stake_lookup": { contract: ethtest.WalletRegistryContract, method: "eligibleStake", }, @@ -46,13 +37,13 @@ func TestAdmission_TransientChainFaultIsNotADenial(t *testing.T) { for testName, test := range tests { t.Run(testName, func(t *testing.T) { - backend, beaconChain, tbtcChain := connectAdmissionFixture(t) + backend, _, tbtcChain := connectAdmissionFixture(t) admitted := ethtest.AdmissionCaseNamed(t, "post_legacy_authorized") operatorPublicKey := caseOperatorKey(t, admitted) policy := firewall.AnyApplicationPolicy( - []firewall.Application{beaconChain, tbtcChain}, + []firewall.Application{tbtcChain}, firewall.EmptyAllowList(), ) @@ -107,6 +98,38 @@ func TestAdmission_TransientChainFaultIsNotADenial(t *testing.T) { backend.AssertNoUnexpectedCalls(t) }) } + + t.Run("beacon_faults_are_not_admission_dependencies", func(t *testing.T) { + backend, _, tbtcChain := connectAdmissionFixture(t) + admitted := ethtest.AdmissionCaseNamed(t, "post_legacy_authorized") + policy := firewall.AnyApplicationPolicy( + []firewall.Application{tbtcChain}, + firewall.EmptyAllowList(), + ) + + backend.Fail( + ethtest.RandomBeaconContract, + "operatorToStakingProvider", + "beacon mapping unavailable", + ) + backend.Fail( + ethtest.TokenStakingContract, + "rolesOf", + "legacy roles unavailable", + ) + backend.ResetCalls() + + if err := policy.Validate(caseOperatorKey(t, admitted)); err != nil { + t.Fatalf("beacon faults changed the admission verdict: %v", err) + } + + backend.AssertTrace( + t, + "admission reads with beacon faults", + admitted.AdmissionReads(t)..., + ) + backend.AssertNoUnexpectedCalls(t) + }) } // TestAdmission_GenuineDenialIsCached characterizes what a real denial costs, @@ -116,7 +139,7 @@ func TestAdmission_TransientChainFaultIsNotADenial(t *testing.T) { // does not readmit the peer, and no chain read is even attempted. Only a // policy that has not already denied it sees the new state. func TestAdmission_GenuineDenialIsCached(t *testing.T) { - backend, beaconChain, tbtcChain := connectAdmissionFixture(t) + backend, _, tbtcChain := connectAdmissionFixture(t) if firewall.NegativeIsRecognizedCachePeriod != time.Hour { t.Fatalf( @@ -128,7 +151,7 @@ func TestAdmission_GenuineDenialIsCached(t *testing.T) { denied := ethtest.AdmissionCaseNamed(t, "registered_unauthorized") operatorPublicKey := caseOperatorKey(t, denied) - applications := []firewall.Application{beaconChain, tbtcChain} + applications := []firewall.Application{tbtcChain} policy := firewall.AnyApplicationPolicy( applications, firewall.EmptyAllowList(), diff --git a/pkg/chain/ethereum/beacon.go b/pkg/chain/ethereum/beacon.go index 933f27b54d..926c8e66a8 100644 --- a/pkg/chain/ethereum/beacon.go +++ b/pkg/chain/ethereum/beacon.go @@ -13,7 +13,6 @@ import ( "github.com/keep-network/keep-common/pkg/chain/ethereum" "github.com/keep-network/keep-core/pkg/chain" "github.com/keep-network/keep-core/pkg/chain/ethereum/beacon/gen/contract" - "github.com/keep-network/keep-core/pkg/operator" ) // Definitions of contract names. @@ -21,39 +20,6 @@ const ( RandomBeaconContractName = "RandomBeacon" ) -// beaconAdmissionReader narrows the chain reads that decide whether a peer is -// admitted to the network via the beacon branch: mapping an operator to a -// staking provider, and checking whether that provider currently has, or ever -// had, a stake delegation. The indirection exists so the admission predicate -// can be exercised without a chain behind it - see tbtcAdmissionReader in -// tbtc.go for the same pattern on the tBTC branch. -type beaconAdmissionReader interface { - OperatorToStakingProvider(operatorAddress common.Address) (common.Address, error) - HasStakeDelegation(stakingProvider common.Address) (bool, error) -} - -// beaconAdmissionReaderChain is the production beaconAdmissionReader, backed -// by the live RandomBeacon and TokenStaking contracts. -type beaconAdmissionReaderChain struct { - randomBeacon *contract.RandomBeacon - baseChain *baseChain -} - -func (r *beaconAdmissionReaderChain) OperatorToStakingProvider( - operatorAddress common.Address, -) (common.Address, error) { - return r.randomBeacon.OperatorToStakingProvider(operatorAddress) -} - -func (r *beaconAdmissionReaderChain) HasStakeDelegation( - stakingProvider common.Address, -) (bool, error) { - _, _, _, hasStakeDelegation, err := r.baseChain.RolesOf( - chain.Address(stakingProvider.Hex()), - ) - return hasStakeDelegation, err -} - var errNotImplemented = fmt.Errorf("not implemented") // BeaconChain represents a beacon-specific chain handle. @@ -62,7 +28,6 @@ type BeaconChain struct { randomBeacon *contract.RandomBeacon sortitionPool *contract.BeaconSortitionPool - admission beaconAdmissionReader } // newBeaconChain construct a new instance of the beacon-specific Ethereum @@ -128,10 +93,6 @@ func newBeaconChain( baseChain: baseChain, randomBeacon: randomBeacon, sortitionPool: sortitionPool, - admission: &beaconAdmissionReaderChain{ - randomBeacon: randomBeacon, - baseChain: baseChain, - }, }, nil } @@ -375,75 +336,6 @@ func (bc *BeaconChain) CalculateDKGResultHash( return beaconchain.DKGResultHashFromBytes(hash) } -// IsRecognized checks whether the given operator is recognized by the BeaconChain -// as eligible to join the network. If the operator has a stake delegation or -// had a stake delegation in the past, it will be recognized. -func (bc *BeaconChain) IsRecognized(operatorPublicKey *operator.PublicKey) (bool, error) { - operatorAddress, err := operatorPublicKeyToChainAddress(operatorPublicKey) - if err != nil { - return false, fmt.Errorf( - "cannot convert from operator key to chain address: [%w]", - err, - ) - } - - stakingProvider, err := bc.admission.OperatorToStakingProvider( - operatorAddress, - ) - if err != nil { - return false, fmt.Errorf( - "failed to map operator [%v] to a staking provider: [%w]", - operatorAddress, - err, - ) - } - - if (stakingProvider == common.Address{}) { - return false, nil - } - - // Check if the staking provider has an owner. This check ensures that there - // is/was a stake delegation for the given staking provider. - // - // This deliberately differs from TbtcChain.IsRecognized, which reads - // eligible stake instead, and the asymmetry must not be harmonised away. - // TokenStaking.authorizedStake short-circuits to zero for every application - // but one hard-coded constant - the TACo application - and the random beacon - // is not it, so RandomBeacon.eligibleStake reads zero for every staking - // provider that has ever registered a beacon operator. An eligible-stake - // predicate here would therefore recognize nobody through this branch. - // - // Peers whose provider holds tBTC eligible stake would keep their admission - // through the tBTC branch; the ones that would lose it are those this - // branch alone carries. The watchtower sweeps every connected peer once per - // libp2p.FirewallCheckTick and drops the ones that no longer validate. That - // constant is the nominal interval between sweeps, not a bound on how long - // a peer that stopped being recognized stays connected: a sweep re-reads - // this predicate through the node's own RPC endpoint, the per-peer checks - // run asynchronously, and closing the connection is a further step behind - // them. - // - // The legacy-delegation admission gap (the security half of the incident, - // where revoked providers retain beacon admission) is tracked in - // https://github.com/threshold-network/keep-core/issues/4335. - hasStakeDelegation, err := bc.admission.HasStakeDelegation( - stakingProvider, - ) - if err != nil { - return false, fmt.Errorf( - "failed to check stake delegation for staking provider [%v]: [%w]", - stakingProvider, - err, - ) - } - - if !hasStakeDelegation { - return false, nil - } - - return true, nil -} - // TODO: Implement a real SubmitRelayEntry function. func (bc *BeaconChain) SubmitRelayEntry( entry []byte, diff --git a/pkg/chain/ethereum/beacon_recognition_test.go b/pkg/chain/ethereum/beacon_recognition_test.go deleted file mode 100644 index 6c2007cf8e..0000000000 --- a/pkg/chain/ethereum/beacon_recognition_test.go +++ /dev/null @@ -1,184 +0,0 @@ -package ethereum - -import ( - "errors" - "testing" - - "github.com/ethereum/go-ethereum/common" - - "github.com/keep-network/keep-core/internal/testutils" - "github.com/keep-network/keep-core/pkg/firewall" -) - -// mockBeaconAdmissionReader stands in for the chain reads the beacon -// admission predicate performs. -type mockBeaconAdmissionReader struct { - stakingProviders map[common.Address]common.Address - hasStakeDelegations map[common.Address]bool - - stakingProviderErr error - hasStakeDelegationErr error - - stakingProviderCalls int - hasStakeDelegationCalls int -} - -func (mbar *mockBeaconAdmissionReader) OperatorToStakingProvider( - operatorAddress common.Address, -) (common.Address, error) { - mbar.stakingProviderCalls++ - - if mbar.stakingProviderErr != nil { - return common.Address{}, mbar.stakingProviderErr - } - - return mbar.stakingProviders[operatorAddress], nil -} - -func (mbar *mockBeaconAdmissionReader) HasStakeDelegation( - stakingProvider common.Address, -) (bool, error) { - mbar.hasStakeDelegationCalls++ - - if mbar.hasStakeDelegationErr != nil { - return false, mbar.hasStakeDelegationErr - } - - return mbar.hasStakeDelegations[stakingProvider], nil -} - -// TestBeaconChain_IsRecognized_HasStakeDelegation pins the beacon/tBTC -// predicate asymmetry: the beacon branch recognizes an operator purely on -// whether its staking provider currently has, or ever had, a stake -// delegation, regardless of eligible stake. -func TestBeaconChain_IsRecognized_HasStakeDelegation(t *testing.T) { - stakingProvider := common.HexToAddress("0x1") - - var tests = map[string]struct { - hasStakeDelegation bool - expectedRecognized bool - }{ - "staking provider with a stake delegation": { - hasStakeDelegation: true, - expectedRecognized: true, - }, - "staking provider with no stake delegation": { - hasStakeDelegation: false, - expectedRecognized: false, - }, - } - - for testName, test := range tests { - t.Run(testName, func(t *testing.T) { - operatorPublicKey, operatorAddress := newTestOperator(t) - - chain := &BeaconChain{ - admission: &mockBeaconAdmissionReader{ - stakingProviders: map[common.Address]common.Address{ - operatorAddress: stakingProvider, - }, - hasStakeDelegations: map[common.Address]bool{ - stakingProvider: test.hasStakeDelegation, - }, - }, - } - - isRecognized, err := chain.IsRecognized(operatorPublicKey) - if err != nil { - t.Fatal(err) - } - - testutils.AssertBoolsEqual( - t, - "recognition", - test.expectedRecognized, - isRecognized, - ) - }) - } -} - -// TestBeaconChain_IsRecognized_UnregisteredOperator covers an operator that -// has never been mapped to a staking provider. The predicate must -// short-circuit before reading the stake delegation, so that recognition -// costs exactly one chain call for an unknown peer. -func TestBeaconChain_IsRecognized_UnregisteredOperator(t *testing.T) { - operatorPublicKey, _ := newTestOperator(t) - - admission := &mockBeaconAdmissionReader{ - stakingProviders: map[common.Address]common.Address{}, - hasStakeDelegations: map[common.Address]bool{}, - } - - chain := &BeaconChain{admission: admission} - - isRecognized, err := chain.IsRecognized(operatorPublicKey) - if err != nil { - t.Fatal(err) - } - - testutils.AssertBoolsEqual(t, "recognition", false, isRecognized) - testutils.AssertIntsEqual( - t, - "stake delegation reads", - 0, - admission.hasStakeDelegationCalls, - ) -} - -// TestBeaconChain_IsRecognized_StakingProviderLookupFails asserts the -// predicate fails closed: a chain error propagates instead of being reported -// as a negative recognition. -func TestBeaconChain_IsRecognized_StakingProviderLookupFails(t *testing.T) { - operatorPublicKey, _ := newTestOperator(t) - - lookupErr := errors.New("connection refused") - - chain := &BeaconChain{ - admission: &mockBeaconAdmissionReader{ - stakingProviderErr: lookupErr, - }, - } - - isRecognized, err := chain.IsRecognized(operatorPublicKey) - - testutils.AssertBoolsEqual(t, "recognition", false, isRecognized) - if err == nil { - t.Fatal("expected the chain error to be returned to the caller") - } - if errors.Is(err, firewall.ErrNotRecognized) { - t.Fatal("chain error was reported as a non-recognition") - } - - testutils.AssertAnyErrorInChainMatchesTarget(t, lookupErr, err) -} - -// TestBeaconChain_IsRecognized_HasStakeDelegationLookupFails asserts the same -// fail-closed behaviour for the read the predicate actually decides on. -func TestBeaconChain_IsRecognized_HasStakeDelegationLookupFails(t *testing.T) { - operatorPublicKey, operatorAddress := newTestOperator(t) - - stakingProvider := common.HexToAddress("0x1") - lookupErr := errors.New("connection refused") - - chain := &BeaconChain{ - admission: &mockBeaconAdmissionReader{ - stakingProviders: map[common.Address]common.Address{ - operatorAddress: stakingProvider, - }, - hasStakeDelegationErr: lookupErr, - }, - } - - isRecognized, err := chain.IsRecognized(operatorPublicKey) - - testutils.AssertBoolsEqual(t, "recognition", false, isRecognized) - if err == nil { - t.Fatal("expected the chain error to be returned to the caller") - } - if errors.Is(err, firewall.ErrNotRecognized) { - t.Fatal("chain error was reported as a non-recognition") - } - - testutils.AssertAnyErrorInChainMatchesTarget(t, lookupErr, err) -} diff --git a/pkg/chain/ethereum/ethereum_integration_test.go b/pkg/chain/ethereum/ethereum_integration_test.go index a700f8a678..c0f3121e77 100644 --- a/pkg/chain/ethereum/ethereum_integration_test.go +++ b/pkg/chain/ethereum/ethereum_integration_test.go @@ -559,22 +559,22 @@ type admissionFacts struct { beaconOwner common.Address } -// beaconRecognized is the beacon branch, unchanged by this work: a registered +// legacyBeaconRecognized reports the historical beacon predicate: a registered // operator whose staking provider has, or ever had, a stake delegation. -func (f admissionFacts) beaconRecognized() bool { +func (f admissionFacts) legacyBeaconRecognized() bool { return f.beaconStakingProvider != zeroAddress && f.beaconOwner != zeroAddress } -// baselineTbtcRecognized is the tBTC branch as it stands on the merge base: -// the same stake-delegation predicate the beacon uses, read through the wallet +// legacyOwnershipTbtcRecognized is the historical tBTC predicate: the same +// stake-delegation predicate the beacon used, read through the wallet // registry's own operator mapping. -func (f admissionFacts) baselineTbtcRecognized() bool { +func (f admissionFacts) legacyOwnershipTbtcRecognized() bool { return f.tbtcStakingProvider != zeroAddress && f.tbtcOwner != zeroAddress } -// proposedTbtcRecognized is the tBTC branch this change settles on: eligible -// stake alone. -func (f admissionFacts) proposedTbtcRecognized() bool { +// currentTbtcRecognized is the active admission predicate: eligible stake +// alone. +func (f admissionFacts) currentTbtcRecognized() bool { return f.tbtcStakingProvider != zeroAddress && f.eligibleStake.Sign() > 0 } @@ -634,7 +634,8 @@ func readAdmissionFacts( return facts } -// admissionSplit counts a population by which branch admits it. +// admissionSplit counts a historical OR population by which branch recognizes +// it. type admissionSplit struct { beaconOnly int both int @@ -649,7 +650,7 @@ func splitPopulation( split := admissionSplit{admitted: make([]common.Address, 0)} for _, entry := range facts { - beacon := entry.beaconRecognized() + beacon := entry.legacyBeaconRecognized() tbtc := tbtcRecognized(entry) switch { @@ -673,6 +674,20 @@ func (s admissionSplit) combined() int { return s.beaconOnly + s.both + s.tbtcOnly } +func recognizedPopulation( + facts []admissionFacts, + recognized func(admissionFacts) bool, +) []common.Address { + population := make([]common.Address, 0) + for _, entry := range facts { + if recognized(entry) { + population = append(population, entry.operator) + } + } + + return population +} + func assertSplit( t *testing.T, policy string, @@ -740,10 +755,10 @@ func assertAddressSet( } } -// TestMainnetChainState_AdmissionCensus evaluates the three admission policies -// this change moves between - the merge base, the originally proposed head and -// the policy settled on - over every operator address either registry has ever -// recorded a registration for, using the chain state at the anchor. +// TestMainnetChainState_AdmissionCensus evaluates historical tBTC predicates +// behind the legacy beacon OR and the active tBTC-only policy over every +// operator address either registry has ever recorded, using the chain state at +// the anchor. // // These are predicate results over registered addresses, not counts of peers // that were connected at the anchor. An address that registered once and has @@ -778,42 +793,54 @@ func TestMainnetChainState_AdmissionCensus(t *testing.T) { facts := readAdmissionFacts(t, callers, population) - baseline := splitPopulation(facts, admissionFacts.baselineTbtcRecognized) - proposed := splitPopulation(facts, admissionFacts.proposedTbtcRecognized) + legacyOwnership := splitPopulation( + facts, + admissionFacts.legacyOwnershipTbtcRecognized, + ) + eligibleStakeWithBeacon := splitPopulation( + facts, + admissionFacts.currentTbtcRecognized, + ) + current := recognizedPopulation(facts, admissionFacts.currentTbtcRecognized) assertSplit( t, - "proposed", + "eligible stake with legacy beacon", admissionSplit{beaconOnly: 262, both: 19, tbtcOnly: 1}, - proposed, + eligibleStakeWithBeacon, + ) + testutils.AssertIntsEqual(t, "current tbtc-only admission", 20, len(current)) + testutils.AssertIntsEqual( + t, + "operators retired with legacy beacon admission", + 262, + len(difference(eligibleStakeWithBeacon.admitted, current)), ) - // The proposed policy does not admit the exact same set of addresses as the - // merge base: it admits one address the merge base does not, and stops - // admitting one address the merge base does. The combined totals match + // Eligible stake with legacy beacon does not admit the exact same set of + // addresses as legacy ownership: it admits one address legacy ownership does + // not, and stops admitting one address it does. The combined totals match // because those two happen to cancel out. gained := []string{"0xc1e20a88c2130472b25b3c382773ba85944230d2"} lost := []string{"0xc19f2434236254fcbd2d329bbe048184bba21975"} assertAddressSet( t, - "addresses the proposed policy admits over the merge base", + "addresses eligible stake adds over legacy ownership", gained, - difference(proposed.admitted, baseline.admitted), + difference(eligibleStakeWithBeacon.admitted, legacyOwnership.admitted), ) assertAddressSet( t, - "addresses the proposed policy stops admitting", + "addresses eligible stake removes from legacy ownership", lost, - difference(baseline.admitted, proposed.admitted), + difference(legacyOwnership.admitted, eligibleStakeWithBeacon.admitted), ) } -// TestMainnetChainState_DeprecatedOperatorsKeepBeaconAdmission reads the -// operators the ECDSA allowlist deliberately left out. The tBTC predicate -// rejects every one of them, and every one of them stays admitted through the -// beacon branch. That is why the beacon must keep its own predicate; who the -// change admits and stops admitting overall is settled by the census above. -func TestMainnetChainState_DeprecatedOperatorsKeepBeaconAdmission(t *testing.T) { +// TestMainnetChainState_DeprecatedOperatorsHaveNoEligibleStake reads the +// operators the ECDSA allowlist deliberately left out. The active admission +// predicate rejects each provider because its eligible stake is zero. +func TestMainnetChainState_DeprecatedOperatorsHaveNoEligibleStake(t *testing.T) { callers := newAdmissionCallers(t) weights := readAllowlistWeights(t) @@ -824,7 +851,6 @@ func TestMainnetChainState_DeprecatedOperatorsKeepBeaconAdmission(t *testing.T) for _, deprecated := range weights.DeprecatedOperatorsNotAdded { t.Run(deprecated.StakingProvider, func(t *testing.T) { stakingProvider := common.HexToAddress(deprecated.StakingProvider) - operatorAddress := common.HexToAddress(deprecated.Operator) eligibleStake := callOrFail(t, func() (*big.Int, error) { return callers.walletRegistry.EligibleStake( @@ -841,30 +867,14 @@ func TestMainnetChainState_DeprecatedOperatorsKeepBeaconAdmission(t *testing.T) ) } - beaconStakingProvider := callOrFail(t, func() (common.Address, error) { - return callers.randomBeacon.OperatorToStakingProvider( - callers.callOpts, - operatorAddress, - ) - }) - - if beaconStakingProvider == zeroAddress { - t.Error("expected the operator to be known to the beacon") - } - - if callers.rolesOwner(t, beaconStakingProvider) == zeroAddress { - t.Error("expected the beacon branch to keep admitting") - } }) } } -// TestBeaconChain_EligibleStakeIsZeroForEveryRegisteredProvider is the reason -// the beacon keeps the legacy delegation predicate. Token staking authorizes -// no stake for the beacon, so beacon eligible stake is zero for every staking -// provider that ever registered a beacon operator - the whole registered -// population at the anchor, not a subset of it. Were the beacon given the -// tBTC predicate, this branch would recognize nobody. +// TestBeaconChain_EligibleStakeIsZeroForEveryRegisteredProvider records that +// token staking authorizes no stake for the beacon. Beacon eligible stake is +// zero for the whole registered provider population at the anchor and is not +// an admission credential. func TestBeaconChain_EligibleStakeIsZeroForEveryRegisteredProvider(t *testing.T) { callers := newAdmissionCallers(t) @@ -948,12 +958,10 @@ func TestMainnetChainState_EligibleStakeAtMinimumAuthorization(t *testing.T) { } } -// TestMainnetChainState_ProviderAuthorizedAfterLegacyStakingFroze is the -// case the change exists for. Legacy token staking can no longer record a -// delegation for anyone, so a provider authorized after it froze reads a zero -// owner forever while holding full eligible stake: the delegation predicate -// rejects it permanently and the eligible stake predicate admits it. It has no -// beacon backstop either, which is the cost the change carries. +// TestMainnetChainState_ProviderAuthorizedAfterLegacyStakingFroze records a +// provider authorized after legacy token staking froze. It holds full eligible +// stake and has a zero legacy owner at the anchor, so a roles-based predicate +// rejects it while the eligible-stake predicate admits it. func TestMainnetChainState_ProviderAuthorizedAfterLegacyStakingFroze(t *testing.T) { callers := newAdmissionCallers(t) @@ -1014,6 +1022,6 @@ func TestMainnetChainState_ProviderAuthorizedAfterLegacyStakingFroze(t *testing. } if callers.rolesOwner(t, beaconStakingProvider) != zeroAddress { - t.Error("expected the provider to have no beacon backstop") + t.Error("expected the beacon-side provider to have no legacy owner") } } diff --git a/pkg/chain/ethereum/tbtc.go b/pkg/chain/ethereum/tbtc.go index 84b7728ba7..bdebfec6e3 100644 --- a/pkg/chain/ethereum/tbtc.go +++ b/pkg/chain/ethereum/tbtc.go @@ -331,36 +331,11 @@ func (tc *TbtcChain) Staking() (chain.Address, error) { // provider it is registered under currently holds eligible stake for the wallet // registry. // -// Admission for the client as a whole is a disjunction over every registered -// application, evaluated by firewall.AnyApplicationPolicy. Written out in full, -// with Ob and Ot the beacon and tBTC operator-to-staking-provider lookups: -// -// Admit(p) <=> FALSE -// OR ( Ob(p) != 0 AND rolesOf(Ob(p)).owner != 0 ) -// OR ( Ot(p) != 0 AND eligibleStake(Ot(p)) > 0 ) -// -// The leading FALSE is the static allow list, which production builds empty. -// This method contributes the third disjunct only; BeaconChain.IsRecognized -// contributes the second and deliberately keeps the rolesOf predicate. -// -// Two things the disjunction does are not visible in that formula. It is -// evaluated left to right, and an application that fails ends it outright -// instead of deferring to the next disjunct, so an identity only this branch -// would admit is refused - with an error rather than a non-recognition - -// whenever the beacon branch's own reads are failing. Beacon-side RPC health is -// therefore a hard dependency of tBTC admission and not an independent branch. -// And a genuine non-recognition is remembered for -// firewall.NegativeIsRecognizedCachePeriod, so a provider authorized after -// being turned away stays refused, with nothing re-read, until that entry -// expires. -// // Mapping an operator to a staking provider is not by itself a boundary, since -// registering an operator is permissionless on both registries. The boundary is -// the eligible stake: the wallet registry reports zero for a provider whose -// authorization has fallen below the minimum authorization, and a requested -// decrease is subtracted the moment it is requested rather than when it is -// approved. Raising it again takes the authorizer of the authorization source -// the registry reads. +// registering an operator is permissionless. The boundary is the eligible +// stake: the wallet registry reports zero for a provider whose authorization +// has fallen below the minimum authorization, and a requested decrease is +// subtracted the moment it is requested rather than when it is approved. // // Eligible stake therefore answers what the provider is currently authorized // for, not what it currently owes. A provider that is dropping its From fe85e5adb93a77956fa184390e0fc4fd70ca4750 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Ros=C5=82aniec?= Date: Wed, 16 Sep 2026 09:15:23 +0000 Subject: [PATCH 2/4] test(ethereum): tighten admission census identity checks and fix stale comments Address confirmed multi-agent-review findings for PR #4333: - TestMainnetChainState_AdmissionCensus asserted only cardinality (20 admitted, 262 retired), not identity. Add two identity-level checks: every config/_peers/mainnet seed's derived chain address must be in the current admitted set, and every retired operator must read false from WalletRegistry.IsOperatorInPool at the pinned block. - pendingDecreaseTbtcRecognized and its call sites uniformly labeled a never-deployed, superseded proposal as "historical" alongside predicates that really were live on mainnet. Reword to distinguish the two. - TestBaseChain_RolesOf_ProductionBinding's comment claimed the binding is "retained for callers outside admission" -- false since the beacon admission predicate was removed; no production caller remains. Reworded to state its actual purpose: a guard against a real misrouting bug. --- .../ethereum/admission_production_test.go | 11 +- .../ethereum/ethereum_integration_test.go | 161 +++++++++++++++++- 2 files changed, 164 insertions(+), 8 deletions(-) diff --git a/pkg/chain/ethereum/admission_production_test.go b/pkg/chain/ethereum/admission_production_test.go index d12c743a49..629f986338 100644 --- a/pkg/chain/ethereum/admission_production_test.go +++ b/pkg/chain/ethereum/admission_production_test.go @@ -254,10 +254,13 @@ func TestAdmission_ProductionConstruction(t *testing.T) { } } -// TestBaseChain_RolesOf_ProductionBinding pins the TokenStaking read retained -// for callers outside admission. The allowlist is a separate deployment -// holding its own role mapping, and reading roles from it would silently answer -// a different question. +// TestBaseChain_RolesOf_ProductionBinding pins the TokenStaking read behind +// the exported RolesOf accessor. No production code path calls it after the +// beacon admission predicate was removed; it is retained as a guard against a +// real misrouting bug rather than for any current caller -- pointing this +// read at the Allowlist instead, a separate deployment holding its own role +// mapping, would silently answer a different question. See the misrouting +// check later in this test. func TestBaseChain_RolesOf_ProductionBinding(t *testing.T) { backend, beaconChain, _ := connectAdmissionFixture(t) diff --git a/pkg/chain/ethereum/ethereum_integration_test.go b/pkg/chain/ethereum/ethereum_integration_test.go index c0f3121e77..03e2cd6a29 100644 --- a/pkg/chain/ethereum/ethereum_integration_test.go +++ b/pkg/chain/ethereum/ethereum_integration_test.go @@ -16,6 +16,7 @@ import ( "testing" "time" + "github.com/btcsuite/btcd/btcec/v2" "github.com/ethereum/go-ethereum/accounts/abi/bind" "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/ethclient" @@ -24,6 +25,10 @@ import ( beaconabi "github.com/keep-network/keep-core/pkg/chain/ethereum/beacon/gen/abi" ecdsaabi "github.com/keep-network/keep-core/pkg/chain/ethereum/ecdsa/gen/abi" thresholdabi "github.com/keep-network/keep-core/pkg/chain/ethereum/threshold/gen/abi" + "github.com/keep-network/keep-core/pkg/operator" + libp2pcrypto "github.com/libp2p/go-libp2p/core/crypto" + "github.com/libp2p/go-libp2p/core/peer" + ma "github.com/multiformats/go-multiaddr" ) // To run the tests execute: @@ -755,10 +760,118 @@ func assertAddressSet( } } -// TestMainnetChainState_AdmissionCensus evaluates historical tBTC predicates -// behind the legacy beacon OR and the active tBTC-only policy over every -// operator address either registry has ever recorded, using the chain state at -// the anchor. +// networkPublicKeyToOperatorPublicKey mirrors the conversion pkg/net/libp2p +// applies to a peer's libp2p public key. It is duplicated here because the +// production helper is unexported and this test resolves peer IDs through +// the same path the client uses to derive an operator chain address from a +// peer ID. The libp2p Secp256k1PublicKey is an alias for btcec.PublicKey, +// so the conversion is the same memory view the production code uses. +func networkPublicKeyToOperatorPublicKey( + networkPublicKey libp2pcrypto.PubKey, +) (*operator.PublicKey, error) { + secp256k1PublicKey, ok := networkPublicKey.(*libp2pcrypto.Secp256k1PublicKey) + if !ok { + return nil, fmt.Errorf( + "unrecognized libp2p public key type for peer ID", + ) + } + btcecPublicKey := (*btcec.PublicKey)(secp256k1PublicKey) + return &operator.PublicKey{ + Curve: operator.Secp256k1, + X: btcecPublicKey.X(), + Y: btcecPublicKey.Y(), + }, nil +} + +// peerAddressFromMultiaddr takes a single /ipfs/... multiaddr line from the +// config/_peers file and converts it to the chain address the production +// client would derive from it. The conversion walks peer.Decode through +// peerID.ExtractPublicKey, networkPublicKeyToOperatorPublicKey, and +// operatorPublicKeyToChainAddress: the same path the client applies to a +// peer ID it has dialed. +func peerAddressFromMultiaddr(t *testing.T, multiaddrLine string) common.Address { + t.Helper() + + parsed, err := ma.NewMultiaddr(multiaddrLine) + if err != nil { + t.Fatalf("could not parse peer multiaddr [%s]: %v", multiaddrLine, err) + } + + peerIDString, err := parsed.ValueForProtocol(ma.P_IPFS) + if err != nil { + t.Fatalf( + "could not extract peer ID from multiaddr [%s]: %v", + multiaddrLine, + err, + ) + } + + peerID, err := peer.Decode(peerIDString) + if err != nil { + t.Fatalf("could not decode peer ID [%s]: %v", peerIDString, err) + } + + networkPublicKey, err := peerID.ExtractPublicKey() + if err != nil { + t.Fatalf( + "could not extract public key from peer ID [%s]: %v", + peerIDString, + err, + ) + } + + operatorPublicKey, err := networkPublicKeyToOperatorPublicKey(networkPublicKey) + if err != nil { + t.Fatalf( + "could not convert peer [%s] public key to operator key: %v", + peerIDString, + err, + ) + } + + chainAddress, err := operatorPublicKeyToChainAddress(operatorPublicKey) + if err != nil { + t.Fatalf( + "could not convert peer [%s] operator key to chain address: %v", + peerIDString, + err, + ) + } + + return chainAddress +} + +// mainnetPeerChainAddresses reads the embedded bootstrap peer addresses from +// config/_peers/mainnet and converts each to a chain address using the same +// peer-ID-to-address path the production client applies. The list is the +// set of addresses a node bootstrapping against this config would dial. +func mainnetPeerChainAddresses(t *testing.T) []common.Address { + t.Helper() + + content, err := os.ReadFile(filepath.Join( + "..", "..", "..", + "config", "_peers", "mainnet", + )) + if err != nil { + t.Fatal(err) + } + + var addresses []common.Address + for _, line := range strings.Split(strings.TrimSpace(string(content)), "\n") { + line = strings.TrimSpace(line) + if line == "" { + continue + } + addresses = append(addresses, peerAddressFromMultiaddr(t, line)) + } + + return addresses +} + +// TestMainnetChainState_AdmissionCensus evaluates the historical beacon OR +// combined with the legacy-ownership and active eligible-stake tBTC predicates +// over every operator address either registry has ever recorded, using the +// chain state at the anchor. // // These are predicate results over registered addresses, not counts of peers // that were connected at the anchor. An address that registered once and has @@ -835,6 +948,46 @@ func TestMainnetChainState_AdmissionCensus(t *testing.T) { lost, difference(legacyOwnership.admitted, eligibleStakeWithBeacon.admitted), ) + // Tie the admitted set to the embedded bootstrap peers: every chain + // address the production client would derive from a config/_peers/mainnet + // seed must be in the current admitted set. This pins identity, not just + // cardinality. + currentHeld := make(map[common.Address]bool, len(current)) + for _, address := range current { + currentHeld[address] = true + } + peerChainAddresses := mainnetPeerChainAddresses(t) + for _, address := range peerChainAddresses { + if !currentHeld[address] { + t.Errorf( + "peer chain address [%s] from config/_peers/mainnet is not in the current tbtc-only admitted set", + address.Hex(), + ) + } + } + + // The retired operators were admitted by the legacy beacon OR but not + // by the current tBTC-only policy. Their sortition-pool membership has + // lapsed, so each one reads false at the pinned block; the wallet + // registry's IsOperatorInPool binding is the same callers/callOpts shape + // every other pinned-block read in this file uses. + for _, operatorAddress := range eligibleStakeWithBeacon.admitted { + if currentHeld[operatorAddress] { + continue + } + inPool := callOrFail(t, func() (bool, error) { + return callers.walletRegistry.IsOperatorInPool( + callers.callOpts, + operatorAddress, + ) + }) + if inPool { + t.Errorf( + "retired operator [%s] still reads as a sortition-pool member at the pinned block", + operatorAddress.Hex(), + ) + } + } } // TestMainnetChainState_DeprecatedOperatorsHaveNoEligibleStake reads the From 46c7e03199f9732598b20e832615b3446a6399e6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Ros=C5=82aniec?= Date: Wed, 16 Sep 2026 09:15:23 +0000 Subject: [PATCH 3/4] test(cmd): remove redundant admission test scaffolding Address confirmed multi-agent-review findings for PR #4333: - admissionHandoff.networkConfig / assertNetworkConfig and the bootstrap_with_configured_discovery_peers sub-tests in TestStart_AdmissionHandoff and TestInitializeNetwork_AdmissionComposition tested no distinct path: start()/initializeNetwork never branch on Bootstrap/Peers, and the stubbed connectNetwork returns before isBootstrap() is evaluated, so the bootstrap variant ran byte-identical code to its ordinary sibling. Removed the capture, the helper, and both bootstrap sub-tests, collapsing each to its single ordinary run. - TestStart_AdmissionRevocationDisconnects's doc comment implied it proves the production libp2p-to-watchtower wiring; nothing in this suite reaches that wiring (captureAdmissionPolicy stubs connectNetwork out first). Reworded to state what it actually proves: the watchtower disconnects a peer once Validate returns an error, composed here over a hand-built connection manager and guard. --- cmd/start_test.go | 108 +++++++++++++--------------------------------- 1 file changed, 30 insertions(+), 78 deletions(-) diff --git a/cmd/start_test.go b/cmd/start_test.go index 71a4b69db3..3e1f57f3ed 100644 --- a/cmd/start_test.go +++ b/cmd/start_test.go @@ -106,7 +106,6 @@ func TestStart_AdmissionHandoff(t *testing.T) { // verdicts unexamined. assertAdmissionTable(t, backend, handoff.policy) assertNoStaticBypass(t, handoff.policy) - assertNetworkConfig(t, clientConfig.LibP2P, handoff.networkConfig) // start builds its own chain handles, so there is no instance here to // compare it against; what it handed over is pinned by type. @@ -115,32 +114,6 @@ func TestStart_AdmissionHandoff(t *testing.T) { handoff.policy, reflect.TypeOf((*ethereum.TbtcChain)(nil)), ) - - t.Run("bootstrap_with_configured_discovery_peers", func(t *testing.T) { - bootstrapBackend := ethtest.New(t, ethtest.AdmissionState(t)) - clientConfig.Ethereum = bootstrapBackend.ChainConfig(t) - clientConfig.LibP2P.Bootstrap = true - clientConfig.LibP2P.Peers = []string{ - "bootstrap-configured-discovery-peer", - } - - bootstrapHandoff := captureAdmissionPolicy(t, func() error { - return start(&cobra.Command{}) - }) - - assertAdmissionTable(t, bootstrapBackend, bootstrapHandoff.policy) - assertNoStaticBypass(t, bootstrapHandoff.policy) - assertNetworkConfig( - t, - clientConfig.LibP2P, - bootstrapHandoff.networkConfig, - ) - assertAdmissionApplicationTypes( - t, - bootstrapHandoff.policy, - reflect.TypeOf((*ethereum.TbtcChain)(nil)), - ) - }) } // TestInitializeNetwork_AdmissionComposition covers the assembly the client @@ -173,40 +146,25 @@ func TestInitializeNetwork_AdmissionComposition(t *testing.T) { configuredNetwork := clientConfig.LibP2P t.Cleanup(func() { clientConfig.LibP2P = configuredNetwork }) - configurations := []struct { - name string - bootstrap bool - peer string - }{ - {"ordinary_with_configured_discovery_peers", false, "ordinary-configured-discovery-peer"}, - {"bootstrap_with_configured_discovery_peers", true, "bootstrap-configured-discovery-peer"}, - } - - for _, configuration := range configurations { - t.Run(configuration.name, func(t *testing.T) { - clientConfig.LibP2P = configuredNetwork - clientConfig.LibP2P.Bootstrap = configuration.bootstrap - clientConfig.LibP2P.Peers = []string{configuration.peer} - - handoff := captureAdmissionPolicy(t, func() error { - _, err := initializeNetwork( - ctx, - applications, - operatorPrivateKey, - blockCounter, - ) - return err - }) - - assertAdmissionTable(t, backend, handoff.policy) - assertNoStaticBypass(t, handoff.policy) - assertNetworkConfig(t, clientConfig.LibP2P, handoff.networkConfig) - - // The policy guards with the very handle it was given rather than - // with one assembled somewhere between here and the network. - assertAdmissionApplications(t, handoff.policy, tbtcChain) - }) - } + clientConfig.LibP2P.Bootstrap = false + clientConfig.LibP2P.Peers = []string{"ordinary-configured-discovery-peer"} + + handoff := captureAdmissionPolicy(t, func() error { + _, err := initializeNetwork( + ctx, + applications, + operatorPrivateKey, + blockCounter, + ) + return err + }) + + assertAdmissionTable(t, backend, handoff.policy) + assertNoStaticBypass(t, handoff.policy) + + // The policy guards with the very handle it was given rather than with one + // assembled somewhere between here and the network. + assertAdmissionApplications(t, handoff.policy, tbtcChain) t.Run("what a static bypass looks like", func(t *testing.T) { // The counterexample the assertions above are read against, built as a @@ -284,8 +242,12 @@ func TestStart_AdmissionRevocation(t *testing.T) { backend.AssertNoUnexpectedCalls(t) } -// TestStart_AdmissionRevocationDisconnects verifies that the production policy -// is also re-evaluated by the watchtower for established connections. +// TestStart_AdmissionRevocationDisconnects verifies that the watchtower +// disconnects a peer once the policy stops admitting it. The connection +// manager and the guard are built here rather than taken from the production +// libp2p wiring, which nothing in this suite exercises directly, so what is +// proven is the watchtower reacting to a Validate error over the production +// policy, not the wiring that installs it. func TestStart_AdmissionRevocationDisconnects(t *testing.T) { backend := ethtest.New(t, ethtest.AdmissionState(t)) revoked := ethtest.AdmissionCaseNamed(t, "legacy_revoked") @@ -400,12 +362,11 @@ func TestStaticBypassKeys_ReportsAnAllowedKey(t *testing.T) { var errNetworkNotOpened = errors.New("the network provider is not opened here") // captureAdmissionPolicy runs a piece of the client's start path with the -// network provider constructor stubbed out, and returns the configuration and -// firewall handed to it. The stub fails, so the path unwinds at the handoff and -// nothing behind it is started. +// network provider constructor stubbed out, and returns the firewall handed +// to it. The stub fails, so the path unwinds at the handoff and nothing +// behind it is started. type admissionHandoff struct { - networkConfig libp2p.Config - policy net.Firewall + policy net.Firewall } func captureAdmissionPolicy(t *testing.T, run func() error) admissionHandoff { @@ -419,14 +380,13 @@ func captureAdmissionPolicy(t *testing.T, run func() error) admissionHandoff { connectNetwork = func( _ context.Context, - config libp2p.Config, + _ libp2p.Config, _ *operator.PrivateKey, policy net.Firewall, _ *retransmission.Ticker, _ ...libp2p.ConnectOption, ) (net.Provider, error) { handoffs++ - captured.networkConfig = config captured.policy = policy return nil, errNetworkNotOpened } @@ -448,14 +408,6 @@ func captureAdmissionPolicy(t *testing.T, run func() error) admissionHandoff { return captured } -func assertNetworkConfig(t *testing.T, expected, actual libp2p.Config) { - t.Helper() - - if !reflect.DeepEqual(expected, actual) { - t.Errorf("unexpected network configuration\nexpected: %v\nactual: %v", expected, actual) - } -} - func connectedPeerSet(connectionManager net.ConnectionManager) map[string]bool { connected := make(map[string]bool) for _, peer := range connectionManager.ConnectedPeers() { From 69e7d760d6a309c4ce27b61b75fbc5e4c87f45b9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Ros=C5=82aniec?= Date: Wed, 16 Sep 2026 09:15:31 +0000 Subject: [PATCH 4/4] docs(retired-components): record beacon admission retirement and rollout gate Address a confirmed multi-agent-review finding for PR #4333: the release gate for the one retired identity that still holds shares in live wallets existed only in the PR description, not as a durable repo artifact. Records: the measured census split (20 admitted under the new tBTC-only predicate, 262 retired, 19 of 20 are the embedded bootstrap seeds), the wallet-continuity precondition a release must satisfy before being tagged, and an explicit placeholder for the specific excluded operator's address -- not available from the reviewed PR/issue materials, left for the PR author to fill in rather than fabricated. --- docs/retired-components.md | 52 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 52 insertions(+) diff --git a/docs/retired-components.md b/docs/retired-components.md index 3f524cd8cf..f777129365 100644 --- a/docs/retired-components.md +++ b/docs/retired-components.md @@ -26,3 +26,55 @@ Historical documents under the `docs/` tree of `keep-core-v1` (formerly `docs-v1/` here) may still mention these components for release history and archival context. They should not be used as operational runbooks for current Threshold Network deployments. + +## Beacon branch as peer-admission authority (PR #4333) + +The random-beacon branch of the peer-admission firewall has been retired. +Admission now consults the tBTC application alone: a peer is admitted when it +is a registered tBTC operator whose provider has positive +`WalletRegistry.eligibleStake`, which the Council controls through the +Allowlist. The beacon recognition method, its reader interface, the production +adapter, and the admission field are removed rather than stubbed, so +`*BeaconChain` no longer satisfies `firewall.Application` and re-adding the +beacon to the application list fails to compile. Beacon startup registration, +`beacon.Initialize`, the Chaosnet seed source, and the shared `RolesOf` +accessor are untouched; none of them was an admission authority, and the +startup-registration requirement is documented where the application list is +built rather than removed. + +### Measured census split + +Measured at a pinned mainnet block over every operator ever registered on +either registry: + +- 20 identities are admitted under the new tBTC-only predicate. +- 262 identities lose admission. +- 19 of the 20 admitted identities are the embedded bootstrap seeds. + +None of the 262 retired identities holds tBTC sortition-pool membership or +weight. Rolling this change back re-admits the whole legacy set, including any +identity the Council has since revoked; that is a policy decision, not an +operator convenience. + +### Release gate (wallet-continuity precondition) + +Exactly one of the excluded identities still holds shares in live wallets. A +release carrying this change MUST NOT be tagged until a wallet-continuity +measurement has been recorded for the affected wallets, demonstrating +coordination and signing without the affected operator. The minimum bar is a +heartbeat with at least seventy active retained members, and the change MUST +be rolled out through the embedded seed operators first. + +The specific excluded operator's address is not available from the reviewed +PR/issue materials and MUST be filled in by the PR author before this section +is considered complete: + +> Excluded operator holding live wallet shares: `` + +Three known residuals are deliberately out of scope of PR #4333 and tracked +separately: the client installs go-libp2p's default transports for outbound +dials with no explicit transport restriction, the pubsub validator filters on +the original author rather than the connection, and the mainnet-anchored +integration tests still skip in CI because no RPC endpoint is forwarded to +that job. +