Repository navigation
fix(setup): publish setup files atomically and keep the first one saved - #34
Merged
Merged
Conversation
save_setup truncated and rewrote the setup file in place, so a concurrent load could read a partial file and panic. It now writes a temporary file and renames it into place. setup generated a new setup when it found no file and then overwrote whatever another process had saved meanwhile. Processes that started together each kept their own setup, and a later call could load another process's. setup now links its file into place only if none exists, and otherwise loads and returns the saved setup. tests/setup_persistence.rs runs eight concurrent setup calls on an empty setup directory and checks they all return the saved setup.
Temporary names were <file>.<pid>.<counter>.tmp, opened with File::create. Two processes with the same PID sharing a cache directory (containers, shared home directories) could truncate and interleave one temporary file, or truncate the file another process had just linked into place. Temporary files now carry 128 random bits and are opened with create_new, retrying on a name clash. A drop guard removes the temporary file unless it was renamed into place, including when a write panics. On Unix, the parent directory is synced after publishing. load_setup reports a file removed after its existence check as not found, and setup tries to save its own setup again when the file saved first disappears before it can load it, instead of panicking.
quangvdao
marked this pull request as ready for review
October 29, 2026 21:09
This was referenced Nov 2, 2026
moodlezoup
pushed a commit
to a16z/jolt
that referenced
this pull request
Nov 2, 2026
* chore(deps): bump dory-pcs to 0.5.0 Picks up atomic first-writer-wins setup files (a16z/dory#34), the prepared-point cache prefix check (a16z/dory#35) and cyclotomic GT exponentiation in the verifier (a16z/dory#36). Refreshes the two scheme.rs comments that described the old dory behavior. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * chore(fuzz): bump dory-pcs to 0.5.0 in fuzz lockfiles Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.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.
Summary
This fixes the disk-persisted setup race in #24. That issue is closed, but the race is still on
main.Two problems, both in
src/setup.rsandsrc/lib.rs:save_setuptruncates the setup file and rewrites it in place. A concurrentload_setupcan read a partial file and panic.setup()finds no file, it generates a new random setup and saves it, overwriting any file another process saved in the meantime. Processes that start together each keep their own setup. A laterload_setupthen returns another process's setup, which doesn't match. Prover/verifier pairs built this way fail to verify.Jolt works around both problems by holding its own lock around
setup(urs_lock::lock_urs_cache). Other users ofdisk-persistencehave no such lock, and neither do dory's own tests. Under plaincargo test, test threads share one setup file, so tests fail intermittently. nextest gives each test its own process, which makes failures rarer. Cold-start processes still race on the shared file: in ten cold nextest runs on this sandbox, one failed two integration tests that don't touch the cache. That failure did not reproduce, and I didn't capture its output, so treat this as likely rather than proven.Changes
save_setupwrites a temporary sibling file (<name>.<32 random hex digits>.tmp), callssync_all, thenrenames it into place. A reader sees either the old file or the new one, never a partial one.create_new(retrying on a name clash), so two writers never share one, even when processes in different containers or on different hosts share the cache directory and reuse PIDs..tmpfile.pub(crate) save_setup_if_absentpublishes the temporary file withhard_link, which fails if the destination exists. So the first process to save wins, and the others learn they lost. If the filesystem doesn't support hard links and no file exists yet, it falls back torename.setup()usessave_setup_if_absent. If another process saved first,setup()loads and returns that saved setup, so every caller ends up with the same setup. If that file is removed beforesetup()can load it,setup()tries to save its own setup again (up to 8 attempts) instead of panicking.load_setupreports a file removed between its existence check and its open as "not found", sosetup()handles it like a missing file.tests/setup_persistence.rs(its own test binary,arkworks+disk-persistence) points HOME at an empty temp dir. It runs eightsetup::<BN254>(6)calls at once behind a barrier, then checks that each returned setup equals the saved one and that no.tmpfile is left behind.CHANGELOG.md: three entries under Unreleased → Fixed.No public API changes and no new dependencies. The random suffix comes from
rand_core::OsRng, already a dependency. I did not use thetempfilecrate, to keep the dependency set and the MSRV 1.75 lockfile unchanged.Validation
On a Linux sandbox, against
mainat 5246da5:main, both with parallel threads and with--test-threads=1. It passes with this change.cargo fmt --check;zk, and with--no-default-features;cargo docwith-D warnings;cargo nextestfor both feature sets (86 and 143 passed);cargo fmt --check, both clippy configurations with-D warnings,cargo docwith-D warnings,tests/setup_persistence.rs, and thesetuptests intest_arkworkspass. Windows is not built in CI or locally; the Unix-only directory sync is behindcfg(unix).cargo test --features backends,parallel,cache,disk-persistence,zkstill fails on this branch. The cause is a separate problem: the prepared-point cache returns points from an earlier setup (fixed in fix(cache): serve prepared points only for the points they came from #35). With both PRs applied, plaincargo testpassed:--test-threads=1;🤖 Generated with Claude Code