feat(vcr): retry + DLQ for the OpenID4VCI credential-offer push - #4471
Open
reinkrul wants to merge 6 commits into
Open
feat(vcr): retry + DLQ for the OpenID4VCI credential-offer push#4471reinkrul wants to merge 6 commits into
reinkrul wants to merge 6 commits into
Conversation
Adds a persistent, retrying queue for OpenID4VCI credential offers that fail on the initial synchronous attempt, and gates the gRPC/DAG network fallback behind it instead of firing on the very first failure. Offer queue (vcr/issuer/offer_queue.go): - Bounded by a fixed 24h wall-clock retry window (tracked from the first failed attempt, surviving restarts via persisted state), not an attempt count - delivery here depends on a remote party's node being reachable, which a local retry cadence can't influence. - Increasing retry interval with jitter, same shape as the existing private-payload-fetch notifier (network/dag/notifier.go). - A pre-first-retry delay: the offer was already attempted once synchronously right before being queued, so the first retry waits rather than immediately re-attempting a very likely still-failing operation (retry-go's own delay only applies *between* attempts). - Once the window is exhausted, the offer is marked dead-lettered (persisted, not removed) and a give-up callback fires - the deferred equivalent of today's immediate fallback. Issue() changes (vcr/issuer/issuer.go): - A genuine delivery failure (OfferCredential itself failing) is now queued instead of immediately falling back to the network: publishing to the DAG is irreversible and network-wide replicated with no dedup, so it shouldn't pay that cost for a failure a retry might resolve. - "Unsupported" cases (no wallet/issuer identifier configured) are unchanged: immediate synchronous fallback, since retrying won't help. - issuer.Issue() no longer guarantees delivery-or-error by the time it returns for the queued case: a 200 means the credential was created and the synchronous attempt was made, not that it was delivered. Documented in the tracking issue's Impact Assessment. Also required: Issuer gained Start()/Shutdown() so the queue's persisted, not-yet-finished offers resume across node restarts, and its in-flight retries stop cleanly on shutdown instead of leaking goroutines. New e2e test (e2e-tests/openid4vci/offer-retry) exercises the real behavior end-to-end: issuing while the receiver is down returns immediately (queued), and the credential is delivered automatically once the receiver comes back - verified locally against a build of this branch, not just via unit tests. The 24h-exhaustion/give-up path is covered by unit tests only; e2e-testing an actual 24h wait isn't practical in CI. Related: #4469 Assisted by AI
13 tasks
Contributor
|
Coverage Impact This PR will not change total coverage. Modified Files with Diff Coverage (3)
🤖 Increase coverage with AI coding...🚦 See full report on Qlty Cloud » 🛟 Help
|
Start/Shutdown just delegate to offerQueue.Run()/Close(), which already document the resume/persist behavior; restate only that. offerQueueStore's doc previously implied its nilness was an independent operational fallback mode - in fact it's nil exactly when openidHandlerFn is nil (both gated by the same OpenID4VCI.Enabled check in vcr.go), and is only independently nilable to support constructing an issuer without a queue in tests. Assisted by AI
Previously only checked the credential arrived, which is consistent with either delivery path. Assert node A's transaction_count is unchanged before and after delivery, proving it went over OpenID4VCI (offerQueue never calls the DAG publisher while retrying) rather than the gRPC/DAG fallback. Verified locally: built the branch's image, ran the test end to end (exit 0), transaction_count stayed at 6 across the retry. Assisted by AI
…ered retry.Do() was missing WrapContextErrorWithLastError(true): when the 24h offerRetryWindow expires between attempts (the normal give-up path), it returned a bare context.DeadlineExceeded and discarded the actual last delivery error, so a dead-lettered job's persisted Error just said "context deadline exceeded" - useless for the diagnostics/requeue tooling planned in #4469 item 3. Verified against the pinned retry-go v4.7.0 source: without the option, Do() returns context.Cause(ctx) alone on the ctx.Done() branch of its unbounded-attempts loop. Also fixed the same class of loss in retry()'s pre-wait bail-out (when the deadline hits before retry.Do is even called): it now folds any previously-recorded job.Error into the returned error instead of overwriting it with a bare context error. Strengthened the existing "gives up once the retry window is exhausted" test to assert on Error content, which the prior version never checked - verified it fails without this fix (bare "context deadline exceeded"), passes with it. Assisted by AI
Close() only cancelled the context and returned immediately, without waiting for retry goroutines to observe cancellation and stop. A caller closing the underlying KV store right after Close() returns (e.g. vcr.Shutdown() closing issuerStore/verifierStore/store right after issuer.Shutdown()) could race an in-flight save() still using context.Background() for persistence I/O. Track in-flight retries with a sync.WaitGroup and have Close() wait for them, bounded by offerQueueShutdownGrace (5s) so a misbehaving attempt that ignores cancellation can't hang shutdown forever - the same timeout-bounded-wait shape as e.g. http.Server.Shutdown(ctx), just with the bound owned internally rather than exposed on Shutdown()'s signature: core.Runnable (core/engine.go:205), which every module's Shutdown() implements, takes no context, so changing Issuer.Shutdown()'s signature would be a much larger, cross-cutting change out of scope here. Removed the sleep-based synchronization hack from the existing Close() test (no longer needed now that Close() genuinely blocks) and added tests for the actual blocking behavior and the grace-period timeout. Verified with -race -count=10, clean. Assisted by AI
…log retries at Warn - Godoc on every offerJob field, including why the persisted key is Credential.ID.String() rather than a generated ID: idempotency (one credential can never have two persisted jobs) and it's the lookup key the planned admin requeue endpoint (#4469 item 3) needs anyway. - Expanded the context.Background()-vs-q.ctx comment on save/finish/all into its two actual reasons: reads must keep working independent of the queue's lifecycle (diagnostics), and writes must not be lost to the Close()/OnRetry shutdown race (q.ctx being cancelled mid-write would silently drop the last recorded Retries/Error state). - Bumped the per-retry-attempt log from Debug to Warn: every attempt that reaches the queue is a genuine delivery failure (unsupported or misconfigured wallet/issuer never reaches it - see issueUsingOpenID4VCI), so it deserves visibility, not Debug-level noise. Matches network/dag/notifier.go's OnRetry logging, the precedent this queue is modeled on, which logs every retry at Error/Warn rather than Debug. Assisted by AI
reinkrul
marked this pull request as ready for review
September 4, 2026 14:55
reinkrul
requested review from
Dirklectisch,
JorisHeadease,
gerardsn,
stevenvegt and
woutslakhorst
as code owners
September 4, 2026 14:55
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
1 participant
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.
Related: #4469
Problem
When pushing a credential offer to a wallet/issuer over OpenID4VCI fails, the node immediately falls back to the synchronous Nuts network (DAG) publish path. That fallback publishes the credential publicly-encrypted-for-participants over gRPC, which has its own delivery assumptions (e.g. PAL decryption via an exportable private key) that don't hold for every deployment — see #4469. A transient OpenID4VCI failure (wallet momentarily down, network blip) shouldn't immediately trigger that fallback.
Solution
Add a persistent, retrying offer queue (
vcr/issuer/offer_queue.go):Issue()schedules the offer on the queue and returns immediately, instead of synchronously falling back to the network publisher.avast/retry-go) for up to 24h (offerRetryWindow), persisted in a stoabs shelf so retries survive a node restart (Run()resumes pending jobs on startup).GivenUp: true) and only then does the node fall back to the network publisher (giveUpOffer).GetFailedOffers()exposes dead-lettered jobs for future diagnostics/requeue tooling (OpenID4VCI: make server-to-server credential-offer delivery reliable (retry, DLQ, base-URL setup) #4469 item 3).IssuergainedStart()/Shutdown()to run/stop the queue's background goroutines; wired intovcr.go's module lifecycle. Requires a persistent KV store (openid4vci-offer-queue), only allocated when OpenID4VCI is enabled.Does not depend on #4470 — verified standalone (cherry-picked onto
master, builds and tests pass in isolation).E2E test
e2e-tests/openid4vci/offer-retry: node B's OpenID4VCI endpoint is taken down before an authorization credential is issued from node A, confirming the offer is accepted (queued) rather than blocking; node B is then brought back up and the test polls until the credential is delivered via retry.Test plan
go test ./vcr/...(including-race,-count=15on the previously-flaky give-up subtest)go build ./...e2e-tests/openid4vci/offer-retry/run-test.sh, exit 0)Assisted by AI