From de69274f33f11e1858337f39c77398420086936f Mon Sep 17 00:00:00 2001 From: Dylan Date: Tue, 18 Aug 2026 06:42:38 -0400 Subject: [PATCH] fix(policy): a peer's 0 sat/kB is an absent fee, not a free miner MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Production report: arcade-v2-us-1 advertised miningFee 1 sat/kB on GET /policy while all six healthy endpoints in GET /health advertised 100. The size limits were the discovered 100000000/500000 rather than the built-in defaults, so discovery was working — only the fee was wrong. This is the same defect #319 reported, at a different number. That report saw a 0 floor; #319 added minDiscoveredFeePerKB and clamped the observed minimum up to 1. Its commit message named the real cause — "teranode advertises min_mining_tx_fee=0 when its policy settings are nil" — which says 0 means *not advertised*. Clamping the result instead of discarding the non-observation moved the symptom from 0 to 1 and left the cause in place, so the cheapest peer was still a node that had never quoted a fee. lowestObservedFeePerKB now skips MiningFeeSatoshis == 0, which is the sentinel highestObservedLimit has always applied to the size limits ("0 means the peer did not advertise a limit, not that it accepts nothing"). With zeros discarded the clamp is unreachable — ceilFeePerKB returns 0 only for a 0 satoshi input — so minDiscoveredFeePerKB is deleted rather than left as dead code whose doc comment asserts the discredited reading. When no fresh peer advertised a fee at all the built-in 100 applies, and accept_zero_fee remains the only route to a 0 floor. Two things that made this undiagnosable from outside are fixed with it. An undated row was fresh forever. Both aggregators inlined `!LastSeen.IsZero() && LastSeen.Before(cutoff)`, so a row with no LastSeen never expired — and every backend can produce one (a missing aerospike bin, a zero pebble timestamp, a postgres zero-value date). isFreshObservation replaces both copies and treats an undated row as stale: an observation that cannot be dated cannot be shown to be current. The per-peer gauges could not see the peer that set the floor. They were labelled by msg.BaseURL, but the row they accompany — and so the minimum /policy derives from it — is keyed off pickDatahubURL, which prefers PropagationURL. A peer announcing only a propagation URL set the floor for the whole instance while appearing in no series at all, which is exactly why an operator could see only peers advertising 100. recordPeerPolicy now takes the registered URL and labels all three gauges with it; the label is renamed base_url -> datahub_url to match, and the values now join to /health's datahub_urls[].url, normalized the same way. refreshPolicyOnce also logs fee_unadvertised_peers beside cheapest_peer_id. Tests fail against the previous behaviour, including a regression built from the exact production peer_policies rows: six datahub peers at 100 alongside one nil-policy node, which returned 1 before and returns 100 now. Claude-Session: https://claude.ai/code/session_01K8pDVtifV8xhKfhEya8wcJ --- config.example.yaml | 13 +- config/config.go | 9 +- metrics/metrics.go | 24 ++-- services/api_server/policy_refresher.go | 79 +++++++---- services/api_server/policy_refresher_test.go | 136 ++++++++++++++++--- services/p2p_client/client.go | 24 +++- services/p2p_client/client_test.go | 42 +++++- store/store.go | 12 +- 8 files changed, 259 insertions(+), 80 deletions(-) diff --git a/config.example.yaml b/config.example.yaml index b082f2b..efa1948 100644 --- a/config.example.yaml +++ b/config.example.yaml @@ -161,12 +161,13 @@ health: # # announcements — and BOTH enforces it at intake and advertises it via # # GET /policy (issue #212). Until a peer is heard it uses the built-in # # 100 sat/kB floor. Set a non-zero value to pin and enforce an exact floor. -# # An observed fee is floored at 1 sat/kB — peer announcements are -# # unauthenticated, so one peer advertising 0 must not be able to disable -# # fee enforcement for the whole instance. Only accept_zero_fee below can -# # produce a 0 floor. Only peers whose datahub URL is registered count -# # toward the observation, so /policy's inputs are the endpoints /health -# # shows. +# # A peer advertising 0 sat/kB has advertised nothing, not offered to mine +# # for free — teranode reports min_mining_tx_fee=0 whenever its policy +# # settings are nil — so such an announcement is discarded rather than +# # counted. If no fresh peer advertises a fee at all, the built-in +# # 100 sat/kB applies. Only accept_zero_fee below can produce a 0 floor. +# # Only peers whose datahub URL is registered count toward the observation, +# # so /policy's inputs are the endpoints /health shows. # min_fee_per_kb: 0 # # accept_zero_fee, when true, pins the floor to 0 sat/kB so any tx # # (including fee=0) is accepted at intake, and /policy advertises 0. This diff --git a/config/config.go b/config/config.go index 7a72ada..08896f6 100644 --- a/config/config.go +++ b/config/config.go @@ -933,10 +933,11 @@ type ValidatorConfig struct { // DefaultValidatorMinFeePerKB when no peer has been heard. A non-zero value // (or AcceptZeroFee) is reported verbatim. // - // An observed fee is floored at 1 sat/kB: peer announcements are - // unauthenticated and the rule is a bare minimum, so a single peer - // advertising 0 would otherwise disable fee enforcement entirely. Setting - // AcceptZeroFee is the only way to reach a 0 floor. + // A peer advertising 0 sat/kB is treated as having advertised nothing and + // is discarded, not counted as a node that mines for free: teranode + // reports min_mining_tx_fee=0 whenever its policy settings are nil. When + // no fresh peer advertises a fee at all, DefaultValidatorMinFeePerKB + // applies. Setting AcceptZeroFee is the only way to reach a 0 floor. MinFeePerKB uint64 `mapstructure:"min_fee_per_kb"` // AcceptZeroFee, when true, pins the validator's fee floor to exactly diff --git a/metrics/metrics.go b/metrics/metrics.go index 8ce0c62..b3b78c4 100644 --- a/metrics/metrics.go +++ b/metrics/metrics.go @@ -1001,32 +1001,36 @@ var P2PPeerBestHeight = promauto.NewGaugeVec(prometheus.GaugeOpts{ // P2PPeerMinMiningFee reports the minimum mining fee (satoshis per 1000 bytes) // each peer advertises in its node_status FeePolicy. The GET /policy endpoint // advertises the network-wide minimum of these (issue #212); this gauge makes -// the per-peer inputs visible. Labelled by base_url for the same -// bounded-cardinality reason as P2PPeerBestHeight; peers with no base_url are -// still persisted for the minimum but not surfaced here. +// the per-peer inputs visible. +// +// Labelled by the peer's registered datahub URL — the same key the stored +// policy row uses and the same value GET /health lists — for the +// bounded-cardinality reason described on P2PPeerBestHeight. It was previously +// labelled by base_url, which silently omitted any peer announcing only a +// propagation URL even though such a peer still set the network minimum. var P2PPeerMinMiningFee = promauto.NewGaugeVec(prometheus.GaugeOpts{ Name: "arcade_p2p_peer_min_mining_fee", - Help: "Minimum mining fee (satoshis per 1000 bytes) advertised by a peer via p2p node_status, by base_url.", -}, []string{"base_url"}) + Help: "Minimum mining fee (satoshis per 1000 bytes) advertised by a peer via p2p node_status, by datahub_url.", +}, []string{"datahub_url"}) // P2PPeerMaxTxSizePolicy and P2PPeerMaxScriptSizePolicy report the size limits // each peer advertises in its node_status FeePolicy. GET /policy advertises the // network-wide maximum of these — the most permissive peer, mirroring the // cheapest-peer rule used for the fee — and intake enforces it, so these gauges // are how an operator sees which peer is setting the ceiling and spots one -// advertising an outlier. Labelled by base_url for the same +// advertising an outlier. Labelled by datahub_url for the same // bounded-cardinality reason as P2PPeerMinMiningFee. Only set for peers that // actually advertise a limit: a legacy peer's absent limit would otherwise show // as 0, reading as "accepts nothing". var P2PPeerMaxTxSizePolicy = promauto.NewGaugeVec(prometheus.GaugeOpts{ Name: "arcade_p2p_peer_max_tx_size_policy", - Help: "Maximum transaction size (bytes) advertised by a peer via p2p node_status, by base_url.", -}, []string{"base_url"}) + Help: "Maximum transaction size (bytes) advertised by a peer via p2p node_status, by datahub_url.", +}, []string{"datahub_url"}) var P2PPeerMaxScriptSizePolicy = promauto.NewGaugeVec(prometheus.GaugeOpts{ Name: "arcade_p2p_peer_max_script_size_policy", - Help: "Maximum script size (bytes) advertised by a peer via p2p node_status, by base_url.", -}, []string{"base_url"}) + Help: "Maximum script size (bytes) advertised by a peer via p2p node_status, by datahub_url.", +}, []string{"datahub_url"}) // ChainTipHeight reports arcade's own view of the active chain tip — the // highest block_processing row marked active. Refreshed by the api-server's diff --git a/services/api_server/policy_refresher.go b/services/api_server/policy_refresher.go index e44a325..26f0c6b 100644 --- a/services/api_server/policy_refresher.go +++ b/services/api_server/policy_refresher.go @@ -132,9 +132,11 @@ func (s *Server) refreshPolicyOnce(ctx context.Context) { s.logger.Info("intake policy updated from network observations", zap.Uint64("prev_fee_sat_per_kb", prevFee), zap.Uint64("new_fee_sat_per_kb", newFee), - // Which peer set the floor. Without it, "why is the floor 1?" can - // only be answered by querying the store by hand. + // Which peer set the floor, and how many fresh peers advertised no + // fee at all. Without these, "why is the floor 1?" can only be + // answered by querying the store by hand. zap.String("cheapest_peer_id", cheapest), + zap.Int("fee_unadvertised_peers", countUnadvertisedFees(peers, ttl, now)), zap.Int("prev_max_tx_size", prevTxSize), zap.Int("new_max_tx_size", newTxSize), zap.Int("prev_max_script_size", prevScriptSize), @@ -163,35 +165,47 @@ func discoveredLimit(peers []store.PeerPolicy, ttl time.Duration, now time.Time, return observed } -// minDiscoveredFeePerKB is the floor under the network-observed fee: discovery -// may track the cheapest node all the way down to 1 sat/kB, but never to 0. -// -// This is the fee's counterpart to the floor discoveredLimit applies to the -// size limits, and it exists for the same reason: node_status is -// unauthenticated gossip and the rule is a bare minimum, so a single peer -// advertising 0 — misconfigured, or simply a node with nil policy settings, -// which teranode advertises as min_mining_tx_fee=0 — silently disabled fee -// enforcement for an entire arcade instance. Accepting zero-fee transactions is -// a deliberate operator decision, so accept_zero_fee is the only thing that may -// produce a 0 floor. -const minDiscoveredFeePerKB = 1 - // discoveredFeePerKB resolves the network-tracked fee floor: the lowest rate a -// fresh peer will accept, bounded below by minDiscoveredFeePerKB, or the -// built-in default when nothing fresh has been observed. It also returns the id -// of the peer that set the floor, so the change is explainable in a log line -// rather than only by querying the store. +// fresh peer actually advertised, or the built-in default when no fresh peer +// advertised one at all. It also returns the id of the peer that set the floor, +// so the value is explainable from a log line rather than only by querying the +// store. func discoveredFeePerKB(peers []store.PeerPolicy, ttl time.Duration, now time.Time) (uint64, string) { observed, peerID, ok := lowestObservedFeePerKB(peers, ttl, now) if !ok { return uint64(config.DefaultValidatorMinFeePerKB), "" } - if observed < minDiscoveredFeePerKB { - return minDiscoveredFeePerKB, peerID - } return observed, peerID } +// isFreshObservation reports whether a peer row is a live observation: re-heard +// within the TTL. +// +// A row with no LastSeen at all is stale, not fresh. An observation that cannot +// be dated cannot be shown to be current, and the TTL is the only thing that +// stops a departed peer from pinning the network policy forever — every backend +// can produce an undated row (a missing aerospike bin, a zero pebble timestamp, +// a postgres zero-value date), and treating one as perpetually fresh makes it +// immortal. Shared by both aggregators so they cannot drift on the question. +func isFreshObservation(p store.PeerPolicy, cutoff time.Time) bool { + return !p.LastSeen.Before(cutoff) +} + +// countUnadvertisedFees reports how many fresh peers advertised no fee at all. +// It exists for the log line: it is the number that explains why the floor is +// what it is, and its absence is what made a production report of a 1 sat/kB +// floor unanswerable without a store query. +func countUnadvertisedFees(peers []store.PeerPolicy, ttl time.Duration, now time.Time) int { + cutoff := now.Add(-ttl) + var n int + for _, p := range peers { + if isFreshObservation(p, cutoff) && p.MiningFeeSatoshis == 0 { + n++ + } + } + return n +} + // lowestObservedFeePerKB returns the minimum mining fee rate (in satoshis per // 1000 bytes) advertised by peers re-heard within ttl, and the id of the peer // advertising it. ok is false when no fresh observation exists. A peer's rate @@ -199,9 +213,6 @@ func discoveredFeePerKB(peers []store.PeerPolicy, ttl time.Duration, now time.Ti // rounds the enforced floor *below* what the peer requires (e.g. 1 sat / 1001 // bytes must map to 1 sat/kB, not 0, or arcade would accept fee=0 that no node // would). -// -// Callers should use discoveredFeePerKB rather than this directly: the raw -// minimum is unbounded below and must not reach the validator unfloored. func lowestObservedFeePerKB(peers []store.PeerPolicy, ttl time.Duration, now time.Time) (uint64, string, bool) { cutoff := now.Add(-ttl) var ( @@ -210,10 +221,24 @@ func lowestObservedFeePerKB(peers []store.PeerPolicy, ttl time.Duration, now tim found bool ) for _, p := range peers { + if p.MiningFeeSatoshis == 0 { + // 0 satoshis is not an observation of a node that mines for free, + // it is the absence of an advertisement: teranode reports + // min_mining_tx_fee=0 whenever its policy settings are nil, and the + // legacy BSV/kB path converts that 0.0 straight through. Counting + // it as a real rate makes one silent non-advertisement the network + // minimum for the whole instance — which is what production did, + // first as a 0 sat/kB floor and then, once the 0 was clamped rather + // than discarded, as 1 sat/kB while every peer in /health required + // 100. This is the same sentinel highestObservedLimit applies to + // the size limits. A network that really does mine for free is an + // operator decision: that is what accept_zero_fee is for. + continue + } if p.MiningFeeBytes == 0 { continue // avoid divide-by-zero on a malformed row } - if !p.LastSeen.IsZero() && p.LastSeen.Before(cutoff) { + if !isFreshObservation(p, cutoff) { continue // peer not re-heard within TTL } perKB := ceilFeePerKB(p.MiningFeeSatoshis, p.MiningFeeBytes) @@ -248,7 +273,7 @@ func highestObservedLimit(peers []store.PeerPolicy, ttl time.Duration, now time. if v == 0 { continue // peer did not advertise this limit } - if !p.LastSeen.IsZero() && p.LastSeen.Before(cutoff) { + if !isFreshObservation(p, cutoff) { continue // peer not re-heard within TTL } if !found || v > best { diff --git a/services/api_server/policy_refresher_test.go b/services/api_server/policy_refresher_test.go index 0fa567b..b3c7e9a 100644 --- a/services/api_server/policy_refresher_test.go +++ b/services/api_server/policy_refresher_test.go @@ -111,6 +111,54 @@ func TestLowestObservedFeePerKB(t *testing.T) { t.Fatalf("got (%d,%v), want (80,true)", got, ok) } }) + + t.Run("zero satoshis means not advertised, not free", func(t *testing.T) { + // The production shape: a node with nil policy settings announces + // min_mining_tx_fee=0, which is the absence of a fee, not an offer to + // mine for nothing. It must not become the network minimum. + peers := []store.PeerPolicy{ + {PeerID: "zero", MiningFeeSatoshis: 0, MiningFeeBytes: 1000, LastSeen: now}, + {PeerID: "honest", MiningFeeSatoshis: 100, MiningFeeBytes: 1000, LastSeen: now}, + } + got, peer, ok := lowestObservedFeePerKB(peers, ttl, now) + if !ok || got != 100 || peer != "honest" { + t.Fatalf("got (%d,%q,%v), want (100,\"honest\",true)", got, peer, ok) + } + }) + + t.Run("a network advertising no fee yields no observation", func(t *testing.T) { + peers := []store.PeerPolicy{ + {PeerID: "zero", MiningFeeSatoshis: 0, MiningFeeBytes: 1000, LastSeen: now}, + } + if got, _, ok := lowestObservedFeePerKB(peers, ttl, now); ok { + t.Fatalf("got (%d,true), want ok=false — nobody advertised a fee", got) + } + }) + + t.Run("an undated row is stale, not immortal", func(t *testing.T) { + // A row we cannot date cannot be shown to be current. Treating it as + // fresh lets a departed peer pin the network policy forever. + peers := []store.PeerPolicy{ + {PeerID: "undated", MiningFeeSatoshis: 1, MiningFeeBytes: 1000}, + {PeerID: "honest", MiningFeeSatoshis: 100, MiningFeeBytes: 1000, LastSeen: now}, + } + got, peer, ok := lowestObservedFeePerKB(peers, ttl, now) + if !ok || got != 100 || peer != "honest" { + t.Fatalf("got (%d,%q,%v), want (100,\"honest\",true)", got, peer, ok) + } + }) + + t.Run("a peer that really advertises 1 sat/kB is still tracked", func(t *testing.T) { + // Discarding 0 must not turn into a blanket floor: 1 sat/kB is a real + // advertisement and mainnet has peers making it. + peers := []store.PeerPolicy{ + {PeerID: "cheap", MiningFeeSatoshis: 1, MiningFeeBytes: 1000, LastSeen: now}, + } + got, peer, ok := lowestObservedFeePerKB(peers, ttl, now) + if !ok || got != 1 || peer != "cheap" { + t.Fatalf("got (%d,%q,%v), want (1,\"cheap\",true)", got, peer, ok) + } + }) } // TestHighestObservedLimit covers the size-limit counterpart of @@ -164,6 +212,19 @@ func TestHighestObservedLimit(t *testing.T) { } }) + t.Run("an undated row is stale, not immortal", func(t *testing.T) { + // Same rule the fee aggregator applies, and for the same reason: an + // observation that cannot be dated must not outlive the TTL. + peers := []store.PeerPolicy{ + {PeerID: "undated", MaxTxSizePolicy: 900_000_000}, + {PeerID: "modern", MaxTxSizePolicy: 20_000_000, LastSeen: now}, + } + got, ok := highestObservedLimit(peers, ttl, now, pickTx) + if !ok || got != 20_000_000 { + t.Fatalf("got (%d,%v), want (20000000,true)", got, ok) + } + }) + t.Run("script limit uses its own field", func(t *testing.T) { peers := []store.PeerPolicy{ {PeerID: "p1", MaxTxSizePolicy: 100_000_000, MaxScriptSizePolicy: 500_000, LastSeen: now}, @@ -425,14 +486,16 @@ func TestConfigDefaultsMatchValidatorDefaults(t *testing.T) { } // TestDiscoveredFeePerKB is the regression for a production report: /policy -// advertised miningFee 0 sat/kB while every endpoint in /health advertised 100. +// advertised a miningFee far below what every endpoint in /health advertised — +// first 0 sat/kB, then 1 once the 0 was clamped rather than discarded. // -// The rule is a bare minimum over unauthenticated gossip, so one peer at 0 — -// misconfigured, or a node with nil policy settings, which teranode advertises -// as min_mining_tx_fee=0 — silently disabled fee enforcement for the whole -// instance. The floor is the fee's counterpart to the one discoveredLimit -// applies to the size limits: discovery may track the cheapest node all the way -// to 1 sat/kB, but only an explicit accept_zero_fee may produce a 0 floor. +// Both readings were wrong in the same way. A peer advertising 0 has not +// offered to mine for free, it has advertised nothing: teranode reports +// min_mining_tx_fee=0 whenever its policy settings are nil, and the legacy +// BSV/kB path converts that 0.0 straight through. Such a row is discarded, on +// exactly the terms highestObservedLimit discards a 0 size limit. If no fresh +// peer advertised a fee at all, the built-in default applies; only an explicit +// accept_zero_fee may produce a 0 floor. func TestDiscoveredFeePerKB(t *testing.T) { now := time.Date(2026, 8, 17, 12, 0, 0, 0, time.UTC) ttl := 15 * time.Minute @@ -452,22 +515,22 @@ func TestDiscoveredFeePerKB(t *testing.T) { want: 100, wantPeer: "p2", }, { - name: "a single zero-fee peer cannot disable fee enforcement", + name: "a peer advertising no fee is discarded, not clamped", peers: []store.PeerPolicy{ {PeerID: "honest", MiningFeeSatoshis: 100, MiningFeeBytes: 1000, LastSeen: now}, {PeerID: "zero", MiningFeeSatoshis: 0, MiningFeeBytes: 1000, LastSeen: now}, }, - want: minDiscoveredFeePerKB, wantPeer: "zero", + want: 100, wantPeer: "honest", }, { - name: "a whole network at zero still floors at the minimum", + name: "a network where nobody advertised a fee falls back to the built-in default", peers: []store.PeerPolicy{ {PeerID: "zero", MiningFeeSatoshis: 0, MiningFeeBytes: 1000, LastSeen: now}, }, - want: minDiscoveredFeePerKB, wantPeer: "zero", + want: uint64(config.DefaultValidatorMinFeePerKB), wantPeer: "", }, { - name: "one satoshi per kB is above the floor and passes through", + name: "one satoshi per kB is a real advertisement and passes through", peers: []store.PeerPolicy{ {PeerID: "cheap", MiningFeeSatoshis: 1, MiningFeeBytes: 1000, LastSeen: now}, }, @@ -502,10 +565,10 @@ func TestDiscoveredFeePerKB(t *testing.T) { } } -// TestRefreshPolicyOnce_ZeroFeePeerDoesNotZeroTheFloor is the same guarantee +// TestRefreshPolicyOnce_UnadvertisedFeeDoesNotSetTheFloor is the same guarantee // end to end through the refresher and the validator, since that is where the // production symptom was visible: GET /policy reads the validator's floor. -func TestRefreshPolicyOnce_ZeroFeePeerDoesNotZeroTheFloor(t *testing.T) { +func TestRefreshPolicyOnce_UnadvertisedFeeDoesNotSetTheFloor(t *testing.T) { s := newPolicyServer(&mockStore{peerPolicies: []store.PeerPolicy{ freshPolicy("honest", 100, 100_000_000, 500_000), freshPolicy("zero", 0, 100_000_000, 500_000), @@ -513,9 +576,9 @@ func TestRefreshPolicyOnce_ZeroFeePeerDoesNotZeroTheFloor(t *testing.T) { s.refreshPolicyOnce(context.Background()) - if got := s.validator.MinFeePerKB(); got != minDiscoveredFeePerKB { - t.Errorf("intake fee floor = %d, want %d — a zero-fee peer must not "+ - "make arcade accept transactions no node will mine", got, minDiscoveredFeePerKB) + if got := s.validator.MinFeePerKB(); got != 100 { + t.Errorf("intake fee floor = %d, want 100 — a peer that advertised no "+ + "fee must not set the floor for the peers that did", got) } // The size limits still track normally. if got := s.validator.MaxTxSizePolicy(); got != 100_000_000 { @@ -523,6 +586,45 @@ func TestRefreshPolicyOnce_ZeroFeePeerDoesNotZeroTheFloor(t *testing.T) { } } +// TestRefreshPolicyOnce_NobodyAdvertisedAFeeUsesTheDefault covers the case the +// one above cannot: with no honest peer to fall back to, the built-in default +// applies rather than some value derived from the non-advertisements. +func TestRefreshPolicyOnce_NobodyAdvertisedAFeeUsesTheDefault(t *testing.T) { + s := newPolicyServer(&mockStore{peerPolicies: []store.PeerPolicy{ + freshPolicy("nil-policy-a", 0, 100_000_000, 500_000), + freshPolicy("nil-policy-b", 0, 100_000_000, 500_000), + }}) + + s.refreshPolicyOnce(context.Background()) + + if got := s.validator.MinFeePerKB(); got != config.DefaultValidatorMinFeePerKB { + t.Errorf("intake fee floor = %d, want %d (built-in default)", + got, config.DefaultValidatorMinFeePerKB) + } +} + +// TestRefreshPolicyOnce_MainnetRegression reproduces the exact production shape +// recorded in peer_policies on arcade-v2 mainnet: six datahub peers advertising +// 100 sat/kB alongside one node announcing no fee at all. Before this fix the +// single non-advertisement won the minimum and /policy told wallets to build +// transactions at 1 sat/kB while /health showed every peer requiring 100. +func TestRefreshPolicyOnce_MainnetRegression(t *testing.T) { + datahubs := []string{"dh-1", "dh-2", "dh-3", "dh-4", "dh-5", "dh-6"} + peers := make([]store.PeerPolicy, 0, len(datahubs)+1) + peers = append(peers, freshPolicy("nil-policy-node", 0, 0, 0)) + for _, id := range datahubs { + peers = append(peers, freshPolicy(id, 100, 100_000_000, 500_000)) + } + s := newPolicyServer(&mockStore{peerPolicies: peers}) + + s.refreshPolicyOnce(context.Background()) + + if got := s.validator.MinFeePerKB(); got != 100 { + t.Errorf("intake fee floor = %d, want 100 — /policy must advertise what "+ + "the datahub peers in /health actually require", got) + } +} + // TestRefreshPolicyOnce_AcceptZeroFeeStillPinsZero confirms the floor did not // take away the one legitimate route to a zero-fee intake: an operator opting // in explicitly, which is what accept_zero_fee is for. diff --git a/services/p2p_client/client.go b/services/p2p_client/client.go index 870844f..19bf4f0 100644 --- a/services/p2p_client/client.go +++ b/services/p2p_client/client.go @@ -351,7 +351,7 @@ func (c *Client) handleNodeStatus(ctx context.Context, msg teranodep2p.NodeStatu // does see nodes announcing cluster-internal names), set the floor for // everyone while appearing nowhere in /health. An operator looking at a // 0 sat/kB floor could see only peers advertising 100. - c.recordPeerPolicy(ctx, msg) + c.recordPeerPolicy(ctx, msg, normalized) } // recordPeerPolicy extracts the peer's advertised transaction policy from a @@ -363,8 +363,10 @@ func (c *Client) handleNodeStatus(ctx context.Context, msg teranodep2p.NodeStatu // // Called only after the peer's datahub URL has been registered, so every row it // writes belongs to a peer arcade can broadcast to — see the call site for why -// that matters. -func (c *Client) recordPeerPolicy(ctx context.Context, msg teranodep2p.NodeStatusMessage) { +// that matters. datahubURL is that registered URL, and labels the per-peer +// gauges so every input to the network policy is visible under the same URL +// GET /health lists it under. +func (c *Client) recordPeerPolicy(ctx context.Context, msg teranodep2p.NodeStatusMessage, datahubURL string) { var sats, byts, maxTxSize, maxScriptSize uint64 switch { case msg.FeePolicy != nil && msg.FeePolicy.MiningFee.Bytes > 0: @@ -410,16 +412,24 @@ func (c *Client) recordPeerPolicy(ctx context.Context, msg teranodep2p.NodeStatu zap.Uint64("max_script_size_policy", maxScriptSize)) } - if msg.BaseURL != "" { + // Label by the registered datahub URL, not msg.BaseURL. The store row this + // function writes — and therefore the network minimum GET /policy derives + // from it — is keyed off pickDatahubURL, which prefers PropagationURL. A + // peer announcing only a propagation URL used to set the fee floor while + // appearing in no gauge at all, which is exactly why a production floor of + // 1 sat/kB could not be explained from the outside: every visible series + // read 100. Using the registered URL also makes these series join to + // /health's datahub_urls[].url, which is normalized the same way. + if datahubURL != "" { // Normalize to satoshis per 1000 bytes for a comparable gauge. - metrics.P2PPeerMinMiningFee.WithLabelValues(msg.BaseURL).Set(float64(sats) * 1000 / float64(byts)) + metrics.P2PPeerMinMiningFee.WithLabelValues(datahubURL).Set(float64(sats) * 1000 / float64(byts)) // Only report a size limit the peer actually advertised: a gauge stuck // at 0 for a legacy peer would read as "accepts nothing". if pp.MaxTxSizePolicy > 0 { - metrics.P2PPeerMaxTxSizePolicy.WithLabelValues(msg.BaseURL).Set(float64(pp.MaxTxSizePolicy)) + metrics.P2PPeerMaxTxSizePolicy.WithLabelValues(datahubURL).Set(float64(pp.MaxTxSizePolicy)) } if pp.MaxScriptSizePolicy > 0 { - metrics.P2PPeerMaxScriptSizePolicy.WithLabelValues(msg.BaseURL).Set(float64(pp.MaxScriptSizePolicy)) + metrics.P2PPeerMaxScriptSizePolicy.WithLabelValues(datahubURL).Set(float64(pp.MaxScriptSizePolicy)) } } diff --git a/services/p2p_client/client_test.go b/services/p2p_client/client_test.go index e730d59..af3c242 100644 --- a/services/p2p_client/client_test.go +++ b/services/p2p_client/client_test.go @@ -12,10 +12,12 @@ import ( p2pclient "github.com/bsv-blockchain/go-teranode-p2p-client" teranodep2p "github.com/bsv-blockchain/teranode/services/p2p" + "github.com/prometheus/client_golang/prometheus/testutil" "go.uber.org/zap" "go.uber.org/zap/zaptest" "github.com/bsv-blockchain/arcade/config" + "github.com/bsv-blockchain/arcade/metrics" "github.com/bsv-blockchain/arcade/store" ) @@ -454,7 +456,7 @@ func TestRecordPeerPolicy_FeePolicy(t *testing.T) { PeerID: "peer-1", BaseURL: testPeerURL, FeePolicy: &teranodep2p.FeePolicy{MiningFee: teranodep2p.FeeAmount{Satoshis: 75, Bytes: 1000}}, - }) + }, testPeerURL) fees := w.feeSnapshot() if len(fees) != 1 { @@ -476,7 +478,7 @@ func TestRecordPeerPolicy_LegacyMinMiningTxFee(t *testing.T) { c.recordPeerPolicy(context.Background(), teranodep2p.NodeStatusMessage{ PeerID: "peer-2", MinMiningTxFee: &fee, - }) + }, testPeerURL) fees := w.feeSnapshot() if len(fees) != 1 { @@ -554,11 +556,37 @@ func TestPeerPolicyOnlyRecordedForRegisteredURLs(t *testing.T) { } } +// TestRecordPeerPolicy_GaugeCoversPropagationOnlyPeers is the regression for a +// production fee floor that could not be explained from outside the cluster. +// +// The per-peer gauges used to be labelled by msg.BaseURL, but the store row +// they accompany — and so the network minimum GET /policy derives from it — is +// keyed off pickDatahubURL, which prefers PropagationURL. A peer announcing +// only a propagation URL therefore set the floor for the whole instance while +// appearing in no gauge at all, leaving an operator looking at a floor of 1 +// alongside six visible peers all advertising 100. +func TestRecordPeerPolicy_GaugeCoversPropagationOnlyPeers(t *testing.T) { + const propURL = "https://prop-only.example/api/v1" + metrics.P2PPeerMinMiningFee.DeleteLabelValues(propURL) + + c, _ := newTestClient(t, newFakeTeraClient(testPeerID)) + c.recordPeerPolicy(context.Background(), teranodep2p.NodeStatusMessage{ + PeerID: "peer-prop-only", + PropagationURL: propURL, + FeePolicy: &teranodep2p.FeePolicy{MiningFee: teranodep2p.FeeAmount{Satoshis: 100, Bytes: 1000}}, + }, propURL) + + if got := testutil.ToFloat64(metrics.P2PPeerMinMiningFee.WithLabelValues(propURL)); got != 100 { + t.Errorf("gauge for a propagation-only peer = %v, want 100 — every peer "+ + "that can set the network fee floor must be visible in metrics", got) + } +} + // TestRecordPeerPolicy_NoFeeAdvertised verifies peers advertising no fee (old // nodes) are skipped rather than recorded with a zero fee. func TestRecordPeerPolicy_NoFeeAdvertised(t *testing.T) { c, w := newTestClient(t, newFakeTeraClient(testPeerID)) - c.recordPeerPolicy(context.Background(), teranodep2p.NodeStatusMessage{PeerID: "peer-4"}) + c.recordPeerPolicy(context.Background(), teranodep2p.NodeStatusMessage{PeerID: "peer-4"}, "") if fees := w.feeSnapshot(); len(fees) != 0 { t.Fatalf("expected no fee upsert for feeless peer, got %+v", fees) } @@ -575,7 +603,7 @@ func TestRecordPeerPolicy_RejectsMalformedLegacyFee(t *testing.T) { c.recordPeerPolicy(context.Background(), teranodep2p.NodeStatusMessage{ PeerID: "peer-bad", MinMiningTxFee: &f, - }) + }, "") if fees := w.feeSnapshot(); len(fees) != 0 { t.Errorf("fee=%v: expected no upsert for malformed fee, got %+v", f, fees) } @@ -595,7 +623,7 @@ func TestRecordPeerPolicy_RecordsSizeLimits(t *testing.T) { MaxTxSizePolicy: 100_000_000, MaxScriptSizePolicy: 500_000, }, - }) + }, testPeerURL) fees := w.feeSnapshot() if len(fees) != 1 { @@ -620,7 +648,7 @@ func TestRecordPeerPolicy_LegacyPeerLeavesSizesUnset(t *testing.T) { c.recordPeerPolicy(context.Background(), teranodep2p.NodeStatusMessage{ PeerID: "peer-legacy", MinMiningTxFee: &fee, - }) + }, testPeerURL) fees := w.feeSnapshot() if len(fees) != 1 { @@ -646,7 +674,7 @@ func TestRecordPeerPolicy_UnstorableSizeKeepsFee(t *testing.T) { MaxTxSizePolicy: math.MaxUint64, MaxScriptSizePolicy: math.MaxUint64, }, - }) + }, testPeerURL) fees := w.feeSnapshot() if len(fees) != 1 { diff --git a/store/store.go b/store/store.go index a388082..904d454 100644 --- a/store/store.go +++ b/store/store.go @@ -147,8 +147,16 @@ type StatusCensus struct { // mining fee is stored as satoshis-per-Bytes (the node_status FeePolicy.MiningFee // shape), e.g. {Satoshis: 100, Bytes: 1000} == 100 sat/kB. type PeerPolicy struct { - PeerID string - Network string + PeerID string + Network string + + // MiningFeeSatoshis is the peer's advertised rate over MiningFeeBytes. + // Zero means "not advertised", on exactly the same terms as the size + // limits below: teranode reports min_mining_tx_fee=0 whenever its policy + // settings are nil, and the legacy BSV/kB conversion carries that 0 + // through verbatim. Readers skip such rows rather than reading the 0 as + // "this peer mines for free" — a genuinely zero-fee network is configured + // with accept_zero_fee, never inferred from gossip. MiningFeeSatoshis uint64 MiningFeeBytes uint64