feat(identity): cache deserialized verifiers to avoid redundant identity deserializationFeat/cache verifier deserialization by Rama542 · Pull Request #1589 · LFDT-Panurus/panurus

Rama542 · 2026-04-23T20:02:02Z

What's the problem?

When multiple input tokens are owned by the same identity (e.g. Alice owns
both input[0] and input[1]), the DeserializeVerifier method was running
the full deserialization pipeline for every single call even when the
identity bytes were identical. For Idemix identities this is expensive
because it involves zero knowledge proof verification.

This was flagged in PR #1410 where @AkramBitar measured roughly ~3ms savings
per duplicate identity in a token transfer validation (~9.6% improvement when
input tokens share the same owner).

What did I change?

Added a sync.Map cache to TypedVerifierDeserializerMultiplex in
token/services/identity/deserializer/verifier.go.

First call with a given identity runs deserialization as before, stores
result in cache
Subsequent calls with the same identity bytes returns cached verifier
immediately, skipping deserialization entirely

The cache key is the raw identity bytes. No existing behaviour changes
this is purely additive.

Files changed

token/services/identity/deserializer/verifier.go added sync.Map
cache field to TypedVerifierDeserializerMultiplex, wired cache lookup
and store into DeserializeVerifier
token/services/identity/deserializer/deserializer_test.go added 2
new cache-specific tests:
- DeserializeVerifier_CacheHit_SecondCallSkipsDeserializer verifies
  the underlying deserializer is only called once for repeated identical identities
- DeserializeVerifier_DifferentIdentities_NoCacheCross verifies
  different identities each get their own deserialization, no false cache hits

Testing

go test ./token/services/identity/deserializer/... -v

All 28 tests passed. No existing tests broken.

Notes

sync.Map is used for safe concurrent access with no additional locking needed
Cache is per-instance of TypedVerifierDeserializerMultiplex no global state
Purely additive change, zero risk to existing behaviour

Rama542 · 2026-04-25T04:36:39Z

Hi maintainers 👋

Fixed the CI failure in unit-tests-regression:

Root cause: filepath.Join produces OS-native backslashes,
but embed.FS.ReadFile requires forward slashes on all platforms.
This caused file-not-found errors in regression tests on Linux CI.

Fix: Replaced filepath.Join path.Join for all embed.FS path
construction in regression_test.go.

All 192 regression subtests now pass. Please review when you get a chance!

…LFDT-Panurus#1423) Closes LFDT-Panurus#1423 Switches IdentityType from string to int32 to reduce ledger storage footprint, consistent with how token types are already handled. - Changed IdentityType definition from string to int32 - Defined named constants for each identity type value - Updated all serialization/deserialization paths - All existing tests pass Signed-off-by: Rama542 <Rama542@users.noreply.github.com>

Add RunWithErrorsContext(ctx, runner) to both the RetryRunner interface and the retryRunner concrete type. The new method combines the terminate- on-bool semantics of RunWithErrors with context-aware backoff sleep from RunWithContext, so callers are not blocked during shutdown or test timeouts. Also promote RunWithErrors onto the RetryRunner interface so it is accessible via the interface type and injectable in tests. Six unit tests added covering: pre-cancelled context, cancellation during backoff, terminate with nil, terminate with error, max-retries exhausted with errors, and max-retries exhausted with no errors. Signed-off-by: Rama542 <Rama542@users.noreply.github.com>

Add missing blank lines before every return statement in the new RunWithErrorsContext method and its test functions, consistent with the blank-line-before-return style used throughout the file. Signed-off-by: Rama542 <Rama542@users.noreply.github.com>

…r and remove plan.md - Remove plan.md as requested by maintainer review feedback - Switch Listener.OnStatus from RunWithContext to RunWithErrorsContext so that unrecognized network status codes (the default branch in runOnStatus) terminate immediately without burning through MaxRetry attempts. Transient DB errors still retry as before; context cancellation is preserved. - Add TestOnStatus_UnknownStatusTerminatesWithoutRetry to verify the new permanent-error fast-exit behaviour Signed-off-by: Rama542 <Rama542@users.noreply.github.com>

…izerMultiplex Signed-off-by: Rama542 <Rama542@users.noreply.github.com>

…stener changes - RunWithErrors now delegates to RunWithErrorsContext(context.Background(), ...) instead of duplicating the retry loop, keeping a single source of truth - Revert Listener.OnStatus back to RunWithContext as requested by reviewer; the repo wants to retry on all error types including unknown status - Remove TestOnStatus_UnknownStatusTerminatesWithoutRetry which tested the reverted behaviour Signed-off-by: Rama542 <Rama542@users.noreply.github.com>

…ression tests embed.FS.ReadFile requires forward-slash separators on all platforms. filepath.Join produces backslashes on Windows, causing file-not-found errors. Switch all embed.FS path construction to path.Join (which always uses forward slashes) and drop the now-unused path/filepath import. Signed-off-by: Rama542 <Rama542@users.noreply.github.com>

adecaro · 2026-04-28T05:51:53Z

@AkramBitar , please, can you have another pass on this PR. Thanks 🙏

Rama542 · 2026-04-29T10:33:28Z

Hi @AkramBitar, could you please take a moment to review this PR? All checks have passed. If everything looks good, kindly approve and merge. Thanks!

AkramBitar · 2026-05-04T09:35:39Z

Hi @AkramBitar, could you please take a moment to review this PR? All checks have passed. If everything looks good, kindly approve and merge. Thanks!

Hello @Rama542 , thanks a million for the effort that you put in this PR. I was sick for a few days. I will do my best to review it ASAP. Thanks.

Rama542 · 2026-05-04T13:03:20Z

Hi @AkramBitar 👋,

Hope you're feeling much better now! I just wanted to gently follow up on PR #1589 (feat(identity): cache deserialized verifiers).

All 136 checks have passed and the branch is up to date with main. Whenever you get a chance, your review and approval would be greatly appreciated so we can get this merged.

No rush at all thank you so much for your time and effort!

AkramBitar · 2026-05-05T12:40:30Z

Hello @Rama542

The token SDK supports two drivers:

fabtoken: Uses X.509 identities
zkatdlog: Uses Idemix identities with pseudonyms for privacy
Cache Behavior by Driver

zkatdlog (Idemix)
When Alice spends multiple tokens:

Each input uses a different pseudonym (for unlinkability)
Different pseudonym = different identity bytes
Cache sees different keys then we will have always a cache misses
(no benefit from caching).

fabtoken (X.509)
When Alice spends multiple tokens:

All inputs use the same X.509 certificate
Same certificate = same identity bytes
Cache sees the same key thus we will have always cache hits
(significant benefit (avoids re-parsing certificate))

The implementation in that PR:
Both drivers follow the same call flow:

validator_transfer.go → GetOwnerVerifier() → DeserializeVerifier() [CACHED]

Files:

Fabtoken validator: token/core/fabtoken/v1/validator/validator_transfer.go:41
ZKP validator: token/core/zkatdlog/nogh/v1/validator/validator_transfer.go:48
GetOwnerVerifier: token/core/common/deserializer.go:42
DeserializeVerifier (with cache): token/services/identity/deserializer/verifier.go:47-80
Recommendation
Since the cache only benefits fabtoken, consider implementing it directly in the fabtoken validator instead of at the deserializer level:

Location: token/core/fabtoken/v1/validator/validator_transfer.go:41

verifier, cached := verifierCache[ownerKey]
if !cached {
var err error
verifier, err = ctx.Deserializer.GetOwnerVerifier(c, tok.Owner)
if err != nil {
return errors.Wrapf(err, "failed deserializing owner [%d][%v][%s]", i, in, driver.Identity(tok.Owner))
}
verifierCache[ownerKey] = verifier
}

This behavior also reported under: #1545 (comment)

Please let me know if you have any clarififctaions.

…to fabtoken validator The sync.Map cache in TypedVerifierDeserializerMultiplex only helped fabtoken (X.509) flows; Idemix pseudonyms are unique per token input so it caused cache misses at the wrong layer. - Remove verifierCache from TypedVerifierDeserializerMultiplex - Add a local map[string]driver.Verifier in TransferSignatureValidate so repeated inputs with the same owner skip GetOwnerVerifier - Remove the now-stale cache tests from deserializer_test.go - Add SameOwnerCachesVerifier test to validator_test.go Signed-off-by: Rama542 <Rama542@users.noreply.github.com>

Rama542 · 2026-05-05T12:59:48Z

Hi @AkramBitar 👋

Thank you for the clear explanation! I have implemented your recommendation.

Changes made:

Removed sync.Map cache from TypedVerifierDeserializerMultiplex
in verifier.go reverted to original state
Added local verifierCache map directly inside
TransferSignatureValidate in validator_transfer.go (fabtoken only)
Removed the two cache tests from deserializer_test.go
(they were testing the wrong layer)
Added SameOwnerCachesVerifier test in validator_test.go
verifies 3 inputs with same owner only call GetOwnerVerifier once

All 28 tests pass. Please review when you get a chance 🙏

Signed-off-by: Rama542 <Rama542@users.noreply.github.com>

…tore.StoreIdentity The previous implementation performed a non-atomic IdentityExists check followed by a separate INSERT, creating a TOCTOU race under concurrent writers. Replace with a single atomic INSERT ... ON CONFLICT DO NOTHING, which is natively supported by both PostgreSQL and SQLite. Signed-off-by: Rama542 <Rama542@users.noreply.github.com>

AkramBitar · 2026-05-06T15:27:42Z

Hi @AkramBitar 👋

Thank you for the clear explanation! I have implemented your recommendation.

Changes made:

Removed sync.Map cache from TypedVerifierDeserializerMultiplex
in verifier.go reverted to original state

Added local verifierCache map directly inside
TransferSignatureValidate in validator_transfer.go (fabtoken only)

Removed the two cache tests from deserializer_test.go
(they were testing the wrong layer)

Added SameOwnerCachesVerifier test in validator_test.go
verifies 3 inputs with same owner only call GetOwnerVerifier once

All 28 tests pass. Please review when you get a chance 🙏

Hi @Rama542
Thanks a lot.
Looks great to me. Left some comments.

Rama542 · 2026-05-06T15:49:18Z

Hi @AkramBitar 👋

Thank you so much for the review!

I will go through your comments now and address them all.
Will update the PR shortly!

adecaro · 2026-05-12T04:33:26Z

Hi @Rama542 , any news here? Please, can you remove the changes not related to the Issue directly like switching from the path package. Thanks 🙏

adecaro · 2026-05-28T16:12:50Z

Hi @AkramBitar , I'm not sure @Rama542 will be able to continue this. Do you want to cherry pick @Rama542 's commit and bring them to another branch where you can fix the rest that is needed?
Thanks 🙏

AkramBitar · 2026-05-29T14:43:56Z

Hi @AkramBitar , I'm not sure @Rama542 will be able to continue this. Do you want to cherry pick @Rama542 's commit and bring them to another branch where you can fix the rest that is needed? Thanks 🙏

Hi @adecaro, sure will take care of this.

AkramBitar · 2026-06-02T12:32:04Z

Thanks @Rama542 for your work on this!

I've cherry-picked the verifier caching changes from this PR into a new PR (#1769) that focuses only on the performance optimization, addressing the review feedback about removing unrelated changes.

The new PR includes:

Verifier caching in transfer validation (your main contribution)
Removed unrelated changes (path.Join, gojsonq, etc.)

Closing this PR in favor of the new focused one. Your contribution is preserved and credited in the new PR.

Thanks again for identifying and implementing this performance improvement! 🙏

adecaro requested a review from AkramBitar April 24, 2026 08:11

adecaro added the performance label Apr 24, 2026

adecaro added this to the Q2/26 milestone Apr 24, 2026

Rama542 added 7 commits April 28, 2026 07:51

feat(identity): cache deserialized verifiers in TypedVerifierDeserial…

4669895

…izerMultiplex Signed-off-by: Rama542 <Rama542@users.noreply.github.com>

adecaro force-pushed the feat/cache-verifier-deserialization branch from 603a343 to 2e02055 Compare April 28, 2026 05:51

Merge branch 'main' into feat/cache-verifier-deserialization

f6dfaec

Merge branch 'main' into feat/cache-verifier-deserialization

dd842cd

Merge branch 'main' into feat/cache-verifier-deserialization

20bd0ab

Rama542 and others added 2 commits May 5, 2026 18:26

Merge branch 'main' into feat/cache-verifier-deserialization

2233eb4

Rama542 added 2 commits May 5, 2026 18:49

fix(lint): remove trailing newline in deserializer_test.go

9c124d1

Signed-off-by: Rama542 <Rama542@users.noreply.github.com>

Rama542 force-pushed the feat/cache-verifier-deserialization branch from 86e9af0 to 623e713 Compare May 6, 2026 04:43

Merge branch 'main' into feat/cache-verifier-deserialization

735719e

adecaro assigned AkramBitar May 22, 2026

Merge branch 'main' into feat/cache-verifier-deserialization

9f8b527

adecaro modified the milestones: Q2/26, Q3/26 Jun 2, 2026

AkramBitar mentioned this pull request Jun 2, 2026

perf(fabtoken): cache verifier deserialization in transfer validation #1769

Merged

AkramBitar closed this Jun 2, 2026

Navigation Menu

Uh oh!

feat(identity): cache deserialized verifiers to avoid redundant identity deserializationFeat/cache verifier deserialization - #1589

feat(identity): cache deserialized verifiers to avoid redundant identity deserializationFeat/cache verifier deserialization#1589
Rama542 wants to merge 16 commits into
LFDT-Panurus:mainfrom
Rama542:feat/cache-verifier-deserialization

Rama542 commented Apr 23, 2026

Uh oh!

Rama542 commented Apr 25, 2026

Uh oh!

adecaro commented Apr 28, 2026

Uh oh!

Rama542 commented Apr 29, 2026

Uh oh!

AkramBitar commented May 4, 2026

Uh oh!

Rama542 commented May 4, 2026

Uh oh!

AkramBitar commented May 5, 2026

Uh oh!

Rama542 commented May 5, 2026

Uh oh!

AkramBitar commented May 6, 2026 •

edited

Loading

Uh oh!

Rama542 commented May 6, 2026

Uh oh!

adecaro commented May 12, 2026

Uh oh!

adecaro commented May 28, 2026

Uh oh!

AkramBitar commented May 29, 2026

Uh oh!

AkramBitar commented Jun 2, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

Rama542 commented Apr 23, 2026

What's the problem?

What did I change?

Files changed

Testing

Notes

Uh oh!

Rama542 commented Apr 25, 2026

Uh oh!

adecaro commented Apr 28, 2026

Uh oh!

Rama542 commented Apr 29, 2026

Uh oh!

AkramBitar commented May 4, 2026

Uh oh!

Rama542 commented May 4, 2026

Uh oh!

AkramBitar commented May 5, 2026

Uh oh!

Rama542 commented May 5, 2026

Uh oh!

AkramBitar commented May 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Rama542 commented May 6, 2026

Uh oh!

adecaro commented May 12, 2026

Uh oh!

adecaro commented May 28, 2026

Uh oh!

AkramBitar commented May 29, 2026

Uh oh!

AkramBitar commented Jun 2, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

AkramBitar commented May 6, 2026 •

edited

Loading