Conversation
Signed-off-by: Effi-S <effi.szt@gmail.com>
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 #2176
Problem
Auditor-side transaction validation (
AuditorCheck→common.RetrieveAuditTokens,token/core/common/auditor.go) can spuriouslyfail when a transaction references — as inputs — outputs of a very
recently-audited transaction. The SQL-backed token store returns a hard
"token not found for key [...]"the moment a requested output row ismissing, and those rows are only written asynchronously by the finality
listener (
token/services/ttx/finality/listener.go) once the network reportsthe referenced transaction as final. A closely-following transaction can
therefore reach audit validation before those rows are persisted.
See parent issue #2105 for the full analysis of the race.
Scope of this issue
This tracks only the cache fast-fail solution (Suggestion 2 in #2105,
refined in the review on #2140), not the retry/backoff mitigation that
landed in #2140.
The idea:
tokens.Servicealready maintains an in-memoryRequestsCache,populated at audit-approval time (
AuditApproveView.Call,token/services/ttx/auditor.go, viaCacheRequest) and removed inAppendValid(token/services/tokens/tokens.go). Its lifetime lines upexactly with the race window, and the cached value is byte-identical to what
the DB read returns (
owner_raw,token_type,quantity). Consulting thiscache on a DB miss lets audit validation resolve the in-flight token
immediately — fast — instead of relying on a multi-second backoff window
that is unbounded under load and only shrinks (never removes) the failure
probability.
Proposed shape:
tokens.Servicecache only on a miss.auditor = truefilter (Flags.Auditor) so a cached entryis only used where the DB read would have qualified.
token/core/commoncannot importtoken/services/tokensdirectly, so thisneeds a consumer-side interface plus an adapter wired in the SDK, and
tokens.Service(or a narrow view of it) plumbed intoAuditorServiceconstruction for both the fabtoken and zkatdlog drivers — a dependency that
does not exist in either construction chain today.
Known limits
The cache is in-process and best-effort (Ristretto's
Setresult isdiscarded), so a process restart or a second auditor replica falls straight
through it. This is why the cache is a complement to the backoff, not a
replacement: the backoff remains the outer net for whatever the cache does
not cover.
Impact
Removes the multi-second latency the backoff adds to the common case (a
just-audited output referenced on the same node), turning a slow retry into
an immediate cache hit, while further reducing the spurious-failure rate for
quickly-chained transactions.