erigontech / erigontech/erigon
execution: a version-map dependency raised during system-tx finalization is not retryable
- Dominant language
- Go
- Stars
- 3.6k
- Forks
- 1.5k
- Avg merge
- 1d 16h
- Merged PRs (30d)
- 455
Description
## What
`FinalizedWrites` resolves account fields through the version map, and those
reads can raise `state.ErrDependency` — `read_paths.go` sets `s.dep` and panics
when a cell it previously recorded now resolves to a different writer. That
panic is the normal optimistic-concurrency signal, but only if something
converts it into a retryable abort.
Two call sites, one of which does:
| site | dependency handling |
|---|---|
| `execution/exec/txtask.go` (`finalizedWrites`) | recovers `ErrDependency` and returns `protocol.ErrExecAbortError{DependencyTxIndex: ibs.DepTxIndex()}`, so the apply loop re-executes the tx against its blocker |
| `execution/stagedsync/exec3_parallel.go:1877` (`finalizeSystemTx`) | none — the panic escapes |
The recover in `TxnExecutor.Execute` that normally performs this translation
wraps message execution only, so it does not cover either finalize call.
For the regular-transaction path an unconverted panic reached the worker's
generic recover and became `exec task panic: found dependency`, i.e. a
`BadBlock` rather than a retry. `finalizeSystemTx` runs in the apply loop, so the
panic escapes there instead, but the underlying problem is the same: a routine
concurrency signal is treated as a failure.
## Why it is worth closing
The exposure is not new — `ApplyVersionedWrites` earlier in the same function
also reads through the version map — so this predates the write-set clearing
work. It is now the only finalize path without the translation, which makes it
the kind of asymmetry that reads as intentional when it is not.
Reachability: the system transaction can run speculatively before every regular
transaction has settled (see the comment above `finalizeSystemTx` explaining why
it re-reads with an empty `ReadSet`), which is exactly the situation where a
recorded read can be superseded.
## Suggested direction
Give `finalizeSystemTx` the same translation, ideally by extracting the recover
so both call sites share one helper rather than keeping two copies. Whether the
apply loop should retry a system transaction the same way it retries a regular
one — or whether a dependency there should defer the whole block-end step —
needs deciding as part of the fix.
Came out of review of #22768, which added the translation on the
regular-transaction path.
Contributor guide
Assessment
This issue has not been assessed yet.