Auditor nodes call IsMine() on every output identity, causing LocalMembership lock contention under high load

[EvanYan1024]

Summary

On an auditor node under c=800 stress (~520 TPS sustained, 19k unique wallets, x509 identities, zkatdlog driver), LocalMembership.refreshAndGet accumulates 82-88% of all mutex wait time (Go mutex profile). Looking at the call chain, the root cause appears to be that tokens.Service.Parse calls auth.IsMine() on every transaction output regardless of node role, and on an auditor the result is always false but the LocalMembership lookup is still performed.

Question for maintainers: is this intentional? Should the auditor path skip IsMine(), or is the Mine flag on TokenToAppend semantically required for auditor records?

Call chain

P2P.handleMessage
  → AuditView.Call
    → AuditApproveView.Call
      → tokens.Service.CacheRequest
        → extractActions                       // tokens.go:364
          → Parse                              // tokens.go:397
            → auth.IsMine()                    // tokens.go:438 (per output)
              → role.Lookup
                → LocalMembership.refreshAndGet

At tokens.go:438 Parse computes mine for every output:

ownerWalletID, ids, mine := auth.IsMine(ctx, &output.Token)

At line 450:

if !mine && !auditorFlag && !issuerFlag {
    continue
}

For an auditor auditorFlag is always true, so mine does not affect keep/discard. The only later use of mine is the TokenToAppend.Flags.Mine field (line 475), which on an auditor is always false.

Evidence

Setup: 1 auditor + 1 institution + 19,996 x509 wallets, c=800, 60s. zkatdlog driver, identity_type=Owner cache fully warmed at start.

mutex profile breakdown (cumulative):

Path	Share
LocalMembership.refreshAndGet	88.3%
↳ via GetIdentityInfo	64.2%
↳ via GetIdentifier	24.1%
auditor.Service.Append → auditdb.ReleaseLocks	10.5%
other	~1%

Per-60s-window counters (with negative-caching shim added locally — without the shim every call takes write Lock):

• ~150,000 IsMine calls per 60s
• ~6,000 unique owner labels seen → all written to negative cache
• 0 positive resolutions on the auditor (localIdentitiesByName / localIdentitiesByIdentity never matched a user-wallet label)
• 100% of slow-path entries either find the label in the negative cache during double-check, or do a PG storedIdentityConfigurations read followed by writing a negative entry

So every IsMine call from the audit hot path is essentially "look up a user-wallet label that this auditor will never have locally → confirm absent → record absence". The lookup result is unused on the auditor (line 450 already kept it via auditorFlag).

audit_view p50 duration is ~1.5s under c=800, with audit_view inflight peaking at 16,943 goroutines (vs audit-pool size 120) — large queue piled up on the LocalMembership write Lock during slow-path entries.

Possible fixes (asking for guidance)

(A) Skip IsMine when auditorFlag is set, in Parse:

var ownerWalletID string
var ids []string
mine := false
if !auditorFlag {
    ownerWalletID, ids, mine = auth.IsMine(ctx, &output.Token)
}

Smallest possible patch. Predicated on Mine: false being acceptable on an auditor's TokenToAppend records. Does anything read Flags.Mine on auditor-owned records?

(B) Auditor short-circuit in Authorization.IsMine:
Return false immediately when the local node is purely an auditor (no owner wallets and not an issuer). Less invasive at the call site but changes IsMine semantics for one node role.

(C) Different audit path that doesn't go through Parse:
More invasive — give the auditor view a dedicated parser that doesn't compute ownership flags.

Environment

• fabric-token-sdk: v0.10.0
• Driver: zkatdlog (nogh v1)
• Identity type: x509
• Go: 1.25
• Workload: ring transfers across 19,996 wallets, c=800 concurrent, 60s window

Happy to share full mutex profiles, OTel histogram dumps, or test patches if any of (A)/(B)/(C) sounds promising.

[EvanYan1024]

For context on what "negative-caching shim" refers to in the evidence section above, here is the patch we applied locally to make the cumulative mutex profile interpretable: Built-by-Sign#8

It adds an RLock fast path + a negative-result cache to LocalMembership.refreshAndGet. Without it, every IsMine call on an auditor takes the write lock and reads storedIdentityConfigurations from PG; the upstream LocalMembership.refreshAndGet at v0.10.0 (token/services/identity/membership/lm.go#L738-L782) does exactly that.

So the numbers reported in the parent issue are with that shim already in place — the architectural concern (auditor calling IsMine at all) sits on top of the mechanism-level fix. Happy to upstream the shim as a separate PR if that helps independent of how the IsMine question is decided.

[EvanYan1024]

Follow-up: the negative-cache shim has an unbounded-growth downside, and a more targeted fix (option B)

On the shim (PR #8) we mentioned above

The RLock + negative-cache shim removes the lock contention, but the negative cache (localIdentitiesNegative) is unbounded at runtime: it gets one entry per distinct owner label the node has ever seen and is only cleared on Load() (i.e. restart). Its size is bounded by the number of distinct wallet identities seen, not by transaction volume, so it plateaus — but on an auditor that audits the whole network the plateau approaches "every wallet in the system", with the key being the raw x509 owner identity. So the shim trades lock contention for steady memory growth on long-running auditors.

That tradeoff is itself an argument that the auditor shouldn't be doing the lookup at all — which brings us back to the original question.

Proposed fix (a safer variant of option B)

Short-circuit IsMine for a pure auditor — a node that holds an auditor wallet and no owner wallets — decided once at construction in NewTMSAuthorization:

// NewTMSAuthorization
pureAuditor := false
if amIAnAuditor {
    if ids, err := walletService.OwnerWalletIDs(context.Background()); err == nil {
        pureAuditor = len(ids) == 0
    }
}

// WalletBasedAuthorization.IsMine
func (w *WalletBasedAuthorization) IsMine(ctx context.Context, tok *token2.Token) (string, []string, bool) {
    if w.pureAuditor {
        return "", nil, false
    }
    // ... unchanged
}

Why this over option (A) (the if !auditorFlag guard in Parse):

• Safe for a combined auditor+owner node. auditorFlag = AmIAnAuditor() only means "I hold an auditor wallet"; it does not imply "I own nothing". A node that both audits and owns would, under (A), store Flags.Mine=false (→ TokenRecord.Owner=false) for tokens it genuinely owns, breaking its owner balance/queries. (A)'s keep/discard line is dominated by auditorFlag, so the damage is invisible there — it only shows up in the stored Owner flag. Gating on "no owner wallets" avoids this entirely.
• Central — covers every IsMine caller, not just the Parse hot path.
• Behavior-preserving on a pure auditor: IsMine is already always false there (0 positive resolutions in our profile), so this just skips the always-false lookup and the negative-cache growth it causes.

Does this implementation look acceptable to you? Happy to open a PR if so.

[adecaro]

Hi @EvanYan1024 , sorry for the late reply on this.
This is something I knew and I was still wondering how to address it. The main issue is that we don't know if a node with the auditor role has also other roles that might require that additional check. For example, in some tests we have the auditor being also the issuer. An issuer can issue to itself becoming therefore the owner of the tokens.
For instance, if there are only auditor wallets in the TMS, we know that check is useless. If any wallet is added, we can enable the check. What do you think?

Please, submit this other fix you mentioned. Thanks much 🙏