record delivered-but-unsettled requests as delivered_pending_settlement#1406
Draft
qdanik wants to merge 1 commit into
Draft
record delivered-but-unsettled requests as delivered_pending_settlement#1406qdanik wants to merge 1 commit into
qdanik wants to merge 1 commit into
Conversation
…ending_settlement, not failed (gonka-ai#1387)
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.
Summary
A gateway request whose winner streamed a full response to the client was accounted as
failedwithwinner_nonce=0whenever the settlement handshake couldn't finalize (validators halted → state signature unverifiable), so dashboards anddevshard_gateway_critical_user_failures_totalcounted a paid, delivered request as a critical failure. This PR stops dropping the executor's work on a transient validator halt, so such requests settle and are accounted assuccess, and closes two settlement-liveness gaps found along the way. Fixes #1387.Root cause
On an unverifiable validator state signature,
Session.processResponsereturnedErrInvalidStateSigearly — before queuing the executor's receipt (MsgConfirmStart) and finish (MsgFinishInference) and before settingoutcome.finished. So the executor's already-produced finish was dropped, the nonce never finished, and the request fell intofinishRaceOutcome's failure branch (reached on the dominant route viadoneCh → winningInflightTerminalFailure → forceTreatAsFailure), which hardcodedfailed/0. Separately,settleDevshardOnChaindeactivated the devshard beforeFinalize, so aFinalizethat fell short of quorum left the escrow inactive-and-unsettled with no retry.What changes
processResponseno longer drops the executor's finish on an unverifiable state signature. The invalid/unexpected-key signature is logged and skipped (not counted toward the settlement quorum); the receipt and finish are still queued andoutcome.finishedis still set. TheErrStateHashMismatchdrop (real state divergence) is kept as the dividing line. As a result the Gateway in-flight long chat during validator halt: client success vs request outcome failed #1387 halt now resolves to a normalsuccess, and a latent liveness bug is fixed too: previously one transient bad state signature aborted the entireFinalize.The
delivered_pending_settlementoutcome becomes a narrow fallback. Since the finish is no longer dropped, the label now only covers the remaining case — the winner streamed clean content but produced no applied finish, so the nonce never finished.winnerDeliveredPendingSettlementrequireserr == nil && contentChunks > 0 && !isFailedStreamAttempt, guarded by!IsNonceFinishedand!ErrStateHashMismatch;resolveTerminalDecisionrecords the real winner nonce and this outcome across accounting, the gateway request metric, and the settled log. A genuinely broken winner (transport error, empty/error stream) still recordsfailed/0. This relabels only accounting/metrics — escrow and the returned error are untouched.Settlement is retried instead of abandoned on a transient quorum shortfall.
settleDevshardOnChainnow always callsFinalize(itsPhaseSettlementbranch re-collects a short quorum, so a retry after a validator returns can complete instead of re-broadcasting the same short proof). AsettlementRetryset on the Gateway (devshard id → deadline epoch) is populated on a retire failure (deadlineepoch+1) and cleared on a successful settle (centralized insettleDevshardOnChain, so admin/auto/rotation/retry paths all clear);retrySettlementsruns each rotation tick — outside the pre-PoC prepare window and while PoC is inactive — re-attempting each pending settlement under a bounded timeout until it succeeds orcurrentEpochpasses the deadline, after which the on-chain reaper takes over.Safety
applyFinishInference,applyConfirmStart); malformed txs are dropped byApplyLocalBestEffort; and on-chain settlement independently re-verifies the state root, signatures, quorum, and theSettledflag (VerifyDevshardSettlement). No unverified signature is ever stored, so the change is quorum-neutral-or-stricter locally.MsgSettleDevshardEscrowchecksescrow.Settledbefore any payout, so a "broadcast landed but the gateway read an error → re-broadcast" cannot double-pay — the re-broadcast is rejected.Known limitations (not blockers)
settlementRetryset is in-memory; a gateway restart mid-retry loses it (the devshard stays persisted-inactive and is not retried). Funds are still protected by the on-chain reaper; persisting the set is a follow-up.PhaseFinalizing,Finalizereturns "already in progress" and cannot recover on retry — pre-existing, unchanged here.escrow.EpochIndex+1) and get rejected until the deadline — harmless log noise (funds safe); clearing on a terminal chain error is a follow-up (avoids error-string matching).Escrow economics (for reviewers)
If the executor never returns, its dropped finish is unrecoverable (there is no cross-host re-pick): the nonce goes unpaid to the executor and the client is refunded at devshard finalize; if settlement slips past the
E+1window, the reaper pays the group validators and the client loses the refund. This PR makes the transient-halt case settle normally (executor paid); permanent non-return still falls back to the reaper.Testing
go build,gofmt,go vet ./...clean;user,types, andcmd/devshardctlpackages green. New/updated tests: the apply path (bad sig + receipt + finish → nonce finishes, txs queued; rejected key not in quorum), the narrowed predicate (8 cases), forced-route delivered →delivered_pending_settlement, broken winner →failed, and the retry reconciler (success-after-failures clears, expiry past deadline, no-op when settlement disabled, earliest-deadline kept). End-to-end retry recovery and post-upgrade settlement are covered by Testermint.