Conversation
IanJohnsons
requested review from
Waldz,
redhatua,
redmundas and
soffokl
as code owners
September 21, 2026 14:53
IanJohnsons
force-pushed
the
fix/session-validation-trace-owner
branch
from
September 21, 2026 15:02
d565729 to
f867d89
Compare
Contributor
Author
|
A test build of this PR is available as a pre-release on my fork, for anyone who wants to verify it on a node before merging: https://github.com/IanJohnsons/node/releases/tag/v1.39.6-fix6221
On my public-IP provider node, this code ran for about 11.5 hours with 6 paid sessions and 0 |
soffokl
approved these changes
Sep 30, 2026
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #6221 +/- ##
==========================================
+ Coverage 26.00% 26.13% +0.12%
==========================================
Files 544 544
Lines 31565 31588 +23
==========================================
+ Hits 8210 8256 +46
+ Misses 22509 22484 -25
- Partials 846 848 +2 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #6220
Provider nodes log
Failed to sign metrics event: authentication needed: password or unlockbecause some quality metrics are signed with an identity the node does not hold. This PR fixes the two sources found on a live provider node.The keystore returns
ErrLockedfor any address that is not in its unlocked map, including a foreign or empty one. That makes the message misleading: the provider identity is unlocked, but the metric's owner is someone else.Source 1:
Session validationtrace attributed to the consumer (one error per session)traceEventToMetricsEventderives the metric owner from the stage-name prefix: only stages starting withProviderare owned by the provider. The provider-side stage"Session validation"(core/service/session_manager.go) had no prefix. It was attributed to the consumer, and the provider tried to sign with the consumer's identity.Change (
f867d89):"Provider session validation"and update the expected sequences insession_manager_test.go.core/quality/trace_owner_test.go:IsProvider/TargetIdassertions for provider and consumer stages;TestTraceStageNames_HaveOwnerPrefix, which scans all non-testStartStage("…")literals and fails if one lacks aProvider/Consumerprefix. It fails on currentmasterfor"Session validation".Source 2:
public_ipNAT event published with an empty ID (one error per start, public-IP hosts only)handleNATStatusForPublicIP(cmd/di.go) publishesevent.BuildSuccessfulEvent("", "public_ip")during bootstrap when the outbound IP equals the public IP.It was added in
c53685e(0.46.2) for the localnat/status_tracker.go, which did not need an identity. That tracker was removed in7a77bce(0.67.0, "Throw away legacy status_tracker.go"). Since then, the only subscriber ofAppTopicTraversalis the quality sender, which turns the event into a MORQA metric with owner"". That metric cannot be signed and cannot be attributed to any provider.It affects only hosts with a public IP on an interface (VPS / dedicated servers). Nodes behind NAT never publish it.
Change (
7a3ee5f):identity.AppTopicIdentityUnlock, using the unlocked identity, the same patternquality.Sender.sendNATTypealready uses. The subscription is registered inBootstrapbeforeNode.Start(). The unlock happens in the service command after bootstrap, so it cannot be missed.cmd/di_nat_public_ip_test.go: the event carries the unlocked identity on a public IP (fails with the old empty ID), and nothing is published behind NAT.The only functional change is timing: the event is sent a few seconds later (after unlock), now owned and signed. With multiple unlocked identities, the existing de-duplication in
nat/event/sender.go(matchesLastEvent, which ignores the ID) still applies.Testing
Unit / CI checks (on top of
masterc45527a):go test ./cmd/ ./core/quality/... ./core/service/... ./nat/... ./identity/...: passmage CheckGoImports,CheckGoLint,CheckGoVet,CheckCopyright: passLive, on a public-IP provider node (Debian 13, x86_64). The patched build is
masterplus both commits (CGO_ENABLED=0,-trimpath), run through a systemd drop-in withsetcap cap_net_admin+ep, matchingdebian/postinst.A temporary debug build logged the owner of the failing batch at startup. This identified source 2:
A/B restart on the same host, 2026-09-21 (UTC):
Failed to sign metrics eventSessions on the patched build:
5 sessions + 1 restart → 0
Failed to sign metrics event. On the official build, the same host logged one error per session plus one per restart (e.g. 2026-09-21: 6 sessions + 2 restarts = 8 errors).Note for MORQA
Session validationtoProvider session validation. These events are nowIsProvider: trueand signed by the provider.public_ipNAT events are now owned by the provider identity instead of being ownerless.Any MORQA-side query or dashboard keyed on the old stage name or on ownerless NAT events may need updating.