Repository navigation
Lock and remap Dart native builds for reproducibility - #1996
Conversation
Coverage Report for CI Build 39431882955Coverage remained the same at 87.471%Details
Uncovered ChangesNo uncovered changes found. Coverage RegressionsNo coverage regressions found. Coverage Stats
💛 - Coveralls |
|
@spacebear21 some context that didn't fit in the description. What has been checked. Unpinned entries in the shipped lock. The staged manifest still declares Git dependencies stay unlocked. The hook treats a checkout containing Which commits are new. The first two are ethicnology's from #1815, with one comment corrected. The last two are mostly rewritten. The lockfile is generated at staging (prepare-publish.sh#L44-L48) instead of tracked, because #1952 pins the wrapper to the release commit when staging and a tracked lock can't name that commit. Jolah1's review of #1815 was already addressed before the rework, so the marker file, absolute Follow-ups. The byte-identical comparison from #1815 ( To run staging against a commit you haven't pushed, from the repo root: CARGO_NET_GIT_FETCH_WITH_CLI=true GIT_CONFIG_COUNT=1 \
GIT_CONFIG_KEY_0="url.$PWD.insteadOf" \
GIT_CONFIG_VALUE_0="https://github.com/payjoin/rust-payjoin.git" \
nix develop .#dart -c ./payjoin-ffi/dart/contrib/prepare-publish.sh /tmp/payjoin-pubedit: context ripped from local claude session but I figured it's better to dump it as an ai-ism than to sit on it |
|
This is my plan for producing reproducible shared native libraries for all language bindings, while unblocking BBM asap by building on top of this approach. This PR locks the Dart package's dependencies, but it ships a lockfile full of stale entries, breaks whenever the core crate is bumped but not yet published, and never checks that a consumer's build actually reproduces. These need to be addressed before we make a new ffi release so that BBM can stop relying on their in-house fork. |
87d0e5e to
a38b2d6
Compare
payjoin-ffi asked for payjoin-test-utils 0.0.1 by version. Inside the workspace the root [patch.crates-io] redirects that to the local crate, but the Dart wrapper consumes payjoin-ffi as a git dependency, where the workspace patches do not apply. Cargo then resolves the July 2025 crates.io release of payjoin-test-utils and pulls its whole graph into any lockfile generated for the wrapper: payjoin 0.24, ring 0.16, rustls 0.21 and 80 other entries that are never compiled, because the dependency is optional and only _test-utils enables it. A security audit of such a lockfile flags them all the same. Use a path dependency, as payjoin, payjoin-cli and payjoin-mailroom already do. A git consumer then resolves the crate from the same checkout as payjoin-ffi. payjoin-ffi is not published to crates.io, so the path dependency does not block a publish, and check-invariants.sh already skips path dependencies. Both lockfiles stay unchanged, since the workspace already resolved the dependency to the local crate.
The Dart package carries a .cargo/config.toml that patches payjoin-ffi to workspace paths. Cargo discovers that file by walking up from the crate, so every command run under payjoin-ffi/dart silently inherits the overlay, and the hook could only tell a checkout from a published archive by the same file's existence without ever saying so. Rename it to config.local.toml, which Cargo does not pick up on its own, and have the build hook pass it explicitly with --config. The presence of the file stays the signal for a workspace build, but it is now the hook that decides when the overlay applies. Pass the absolute path: Cargo resolves a relative --config path against its own working directory, which is the package root, and errors out when nothing is there. Nothing here can move to an environment variable. hooks_runner spawns build hooks with an allowlisted environment, so a project specific name never reaches the hook.
The native wrapper records absolute paths from the machine that built it: the package root, the hook output directory, and the Cargo and Rustup homes all end up in the artifact. Two consumers building the same release therefore produce different bytes, which makes the library impossible to verify. Pass --remap-path-prefix for each of those roots so the recorded paths depend on the release rather than on the machine. Emit them through CARGO_ENCODED_RUSTFLAGS rather than RUSTFLAGS. Cargo splits RUSTFLAGS on whitespace, and the output directory lives under the consumer's project: a path such as /Users/jane/My App would become two arguments and fail the build. Flags already in CARGO_ENCODED_RUSTFLAGS are kept. Plain RUSTFLAGS is not inherited, because no hooks_runner release passes it to a build hook. Cargo takes rustflags from exactly one source, and this variable comes before every Cargo config file, so the hook replaces build.rustflags and target.*.rustflags from the consumer's config for this build. Say so where the variable is set. Directory URIs render with a trailing separator, and rustc substitutes the prefix literally, so drop it: otherwise native/src/lib.rs would be recorded as /payjoin/packagenative/src/lib.rs. Co-authored-by: spacebear <git@spacebear.dev>
Consumers of the published package compile the native wrapper themselves, and until now resolved its dependency graph fresh on whatever day they built. That picks up releases we never tested, as yoke-derive 0.8.3 showed when it started requiring Rust 1.87 without declaring it. prepare-publish.sh now writes native/Cargo.lock into the staged package after pinning payjoin-ffi to the release commit. It seeds the file from Cargo-recent.lock, so every crate a consumer builds keeps the version CI tested and cargo resolves only the wrapper and the crates it now takes from the pinned revision, payjoin-ffi and payjoin-test-utils. The payjoin core crate still comes from crates.io at the version payjoin-ffi requires. The lockfile cannot be committed instead: it would have to name the commit that contains it. The build hook passes --locked outside the workspace, so a consumer build fails rather than silently re-resolving. .pubignore stops withholding the lockfile. payjoin-ffi and payjoin-test-utils come from the pinned revision, but the release crates they require, payjoin and payjoin-mailroom, come from crates.io. Between a version bump of one of those and its crates.io release, the staging cargo update would fail on every pull request. When a required version is missing from crates.io, staging patches that crate to the same pinned revision and writes an UNPUBLISHED_CORE marker. Only a 404 from the crates.io API counts as missing; any other failure stops staging, so an outage during a release cannot produce a stage publish.sh refuses. The crates come from sibling_deps in the release tooling. cargo info cannot do the check from inside the repository, since it reports the local workspace crate. publish.sh refuses a stage with that marker. Co-authored-by: Dan Gould <d@ngould.dev> Co-authored-by: spacebear <git@spacebear.dev>
The contributing guide did not say how the hook tells a repository build from a consumer build, why that cannot be an environment variable, or what it does to keep build paths out of the artifact. Describe the marker file and the remapped prefixes, and note in the release steps where the shipped lockfile comes from and that staging needs the pinned commit on GitHub. Tell consumers in the README that hosted releases build their shipped lockfile with --locked, that a git dependency on this repository is a development build, and that the hook's CARGO_ENCODED_RUSTFLAGS makes Cargo ignore rustflags from their Cargo config for this build. Describe the UNPUBLISHED_CORE marker in the release steps. Co-authored-by: Dan Gould <d@ngould.dev> Co-authored-by: spacebear <git@spacebear.dev>
The hook remaps build paths so two consumers of one release get the same native library, and the staged lock fixes the dependency graph, but no job ever built the package twice to confirm the result. Bull Bitcoin verifies its APKs by rebuilding them, so a regression would first surface as a failed verification downstream. Add contrib/consumer, a minimal Dart program that depends on the package by path, and a reproducible-linux job in dart.yml that takes the package verify-pub-package staged. The job runs the consumer twice on the host under Flutter 3.44.9, whose Dart SDK is the one Bull Bitcoin builds with, and compares the two libraries with cmp. The two builds use separate stage copies, app directories and HOME, and with it separate Cargo, rustup and pub homes. The second build sits under a path with a space in it. The job inherits the workflow's triggers: pull requests that touch payjoin-ffi/**, which covers the Dart package and its hook, and the workflow_call from bindings-release.yml, so a release tag cannot publish unless the build reproduces.
a38b2d6 to
22dad4c
Compare
|
Pushed again with some changes:
Android should be verified with Bull's |
|
Thanks for the work on this matter |
DanGould
left a comment
DanGould
left a comment
There was a problem hiding this comment.
ACK 22dad4c
@spacebear21 is testing the integration with BullBitcoin directly to see that all works & is reproducible through the downstream implementation
|
Thanks guys for your work Just to give you an idea of the timeline: if it's ready by this Friday, I can include it in the upcoming 6.15.0 release. Otherwise, we'll ship it in the following release. No rush at all! |
xstoicunicornx
left a comment
xstoicunicornx
left a comment
There was a problem hiding this comment.
cACK 22dad4c
Producing reproducible builds for Dart will help users of this library produce reproducible builds for their own wallets.
|
@ethicnology Dart release is ready https://pub.dev/packages/payjoin/versions/0.25.1 I tested it on my machine and it looks like it produces reproducible builds for bbm when pointing at this. |
|
Thanks so much, just in time for 6.15.0 |
This picks up #1815 from @ethicnology, rebased on master and reworked for the release flow that landed since. It supersedes #1815.
Wallets that ship the Dart package need reproducible builds, so that anyone can rebuild a release and get the same bytes. The native wrapper build gets in the way of that in two places. It resolves its dependency graph fresh on every machine, and it embeds local paths such as the Cargo registry, the rustup toolchain and the package directory in the library. Fresh resolution also broke Dart builds on Rust 1.85 when yoke-derive 0.8.3 was released (#1885).
This PR ships
native/Cargo.lockin the published archive and has the build hook use it with--locked. The hook also remaps those paths throughCARGO_ENCODED_RUSTFLAGS, so they no longer end up in the library.The difference from #1815 is where that lockfile comes from. Since #1952,
prepare-publish.shpins the staged wrapper to the release commit, and a lockfile committed to the repo can't name the commit that contains it. Soprepare-publish.shnow generates the staged lockfile, seeded fromCargo-recent.lock. On a staged package, every third-party crate in the build graph (154 packages in total) keeps the version fromCargo-recent.lock. Only payjoin, payjoin-ffi and the wrapper itself are resolved fresh, since they now come from the pinned git revision.These parts of #1815 are unchanged: the overlay rename and marker file, the path remapping, the lockfile checker and its tests, and the hook docs. This PR does not yet show that two builds come out byte-identical.
check_reproducible.sh,smoke_consumer.shand the two-build procedure do that check, and they are left for a follow-up that also runs them in CI.The locked build applies to the published package. Depending on this repository through a git reference still builds through the development overlay, without
--locked.Staging now fetches rust-payjoin at the release commit, so the "Verify pub.dev package" job needs that commit to be fetchable from GitHub.
@spacebear21 I'm marking progress here while recognizing this isn't done yet and needs more iteration. Feel free to new pr/overwrite this one.
Disclosure: co-authored by Claude Code, working from @ethicnology's commits.