Skip to content

fix: sign provider quality metrics with the correct identity - #6221

Merged
soffokl merged 3 commits into
mysteriumnetwork:masterfrom
IanJohnsons:fix/session-validation-trace-owner
Sep 30, 2026
Merged

soffokl merged 3 commits into
mysteriumnetwork:masterfrom
IanJohnsons:fix/session-validation-trace-owner

Conversation

@IanJohnsons

@IanJohnsons IanJohnsons commented Sep 21, 2026 •

Copy link
Copy Markdown
Contributor

Fixes #6220

Provider nodes log Failed to sign metrics event: authentication needed: password or unlock because 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.

ERR core/quality/mysterium_morqa.go:212 > Failed to sign metrics event error="failed to sign metrics event: authentication needed: password or unlock"

The keystore returns ErrLocked for 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 validation trace attributed to the consumer (one error per session)

traceEventToMetricsEvent derives the metric owner from the stage-name prefix: only stages starting with Provider are 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):

  • Rename the stage to "Provider session validation" and update the expected sequences in session_manager_test.go.
  • Add core/quality/trace_owner_test.go:
    • owner / IsProvider / TargetId assertions for provider and consumer stages;
    • TestTraceStageNames_HaveOwnerPrefix, which scans all non-test StartStage("…") literals and fails if one lacks a Provider/Consumer prefix. It fails on current master for "Session validation".

Source 2: public_ip NAT event published with an empty ID (one error per start, public-IP hosts only)

handleNATStatusForPublicIP (cmd/di.go) publishes event.BuildSuccessfulEvent("", "public_ip") during bootstrap when the outbound IP equals the public IP.

It was added in c53685e (0.46.2) for the local nat/status_tracker.go, which did not need an identity. That tracker was removed in 7a77bce (0.67.0, "Throw away legacy status_tracker.go"). Since then, the only subscriber of AppTopicTraversal is 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):

  • Publish the event after identity.AppTopicIdentityUnlock, using the unlocked identity, the same pattern quality.Sender.sendNATType already uses. The subscription is registered in Bootstrap before Node.Start(). The unlock happens in the service command after bootstrap, so it cannot be missed.
  • Add 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 master c45527a):

  • go test ./cmd/ ./core/quality/... ./core/service/... ./nat/... ./identity/...: pass
  • mage CheckGoImports, CheckGoLint, CheckGoVet, CheckCopyright: pass
  • Both new regression tests verified to fail without their fix and pass with it

Live, on a public-IP provider node (Debian 13, x86_64). The patched build is master plus both commits (CGO_ENABLED=0, -trimpath), run through a systemd drop-in with setcap cap_net_admin+ep, matching debian/postinst.

A temporary debug build logged the owner of the failing batch at startup. This identified source 2:

Failed to sign metrics event ... event_types=["*metrics.Event_NatMappingPayload[provider=true target=]"] events=1 owner=

A/B restart on the same host, 2026-09-21 (UTC):

Time Binary Event
17:28:16 1.39.6 (official) start
17:29:06 1.39.6 (official) identity unlocked
17:29:34 1.39.6 (official) Failed to sign metrics event
17:31:15 patched start
17:31:47 patched identity unlocked, no error

Sessions on the patched build:

Session start (UTC) Result
17:37:43 (test via mymysterium) completed, no error
18:16:20 no error
18:53:52 no error
19:11:22 no error
19:13:08 no error

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

  • The stage name reported to MORQA changes from Session validation to Provider session validation. These events are now IsProvider: true and signed by the provider.
  • public_ip NAT 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.

@IanJohnsons
IanJohnsons force-pushed the fix/session-validation-trace-owner branch from d565729 to f867d89 Compare September 21, 2026 15:02
@IanJohnsons IanJohnsons changed the title fix: prefix provider-side Session validation trace stage fix: sign provider quality metrics with the correct identity Sep 21, 2026
@IanJohnsons

Copy link
Copy Markdown
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

  • myst_linux_amd64.deb / myst_linux_arm64.deb + SHA256SUMS
  • Official 1.39.6 (c45527a) plus the two commits of this PR, nothing else
  • Packaged with the repo's own bin/package_debian (fpm) pipeline with BUILD_STATIC=1, so it installs like the official package (same service, sudoers, setcap)
  • Version 1.39.6+fix6221, which sorts above the official 1.39.6+build…, so the updater won't downgrade it but will replace it with the next official release

On my public-IP provider node, this code ran for about 11.5 hours with 6 paid sessions and 0 Failed to sign metrics event errors.

@codecov-commenter

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 77.77778% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 26.13%. Comparing base (cf3f5aa) to head (2ec7e98).
⚠️ Report is 3 commits behind head on master.

Files with missing lines Patch % Lines
cmd/di.go 75.00% 2 Missing ⚠️
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.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@soffokl
soffokl merged commit c2f956c into mysteriumnetwork:master Sep 30, 2026
2 of 4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Provider signs quality metrics with a foreign or empty identity → "Failed to sign metrics event"

3 participants