Resolve guix non-determinism with emplace_back instead of push

achow101 commented at 6:39 am on July 10, 2025: member

For some reason, building x86_64-w64-mingw32 on x86_64 and aarch64 results in a single instruction difference which can be traced down to prevector.h:174. The ultimate caller of this is the copy constructor for a prevector that ends up being called by std::vector::push_back in walletmodel.cpp:183. By replacing the push_back with an emplace_back, somehow this non-determinism goes away.

Closes #32923

DrahtBot commented at 6:39 am on July 10, 2025: contributor

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/32930.

Reviews

See the guideline for information on the review process.

Type	Reviewers
ACK	l0rinc
Concept ACK	Sjors
Stale ACK	maflcko

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

maflcko commented at 6:44 am on July 10, 2025: member

lgtm ACK 3add4b365c90c0943f18dd26d4a18c1e2ee5860e

Haven’t tested a guix build, but the changes lgtm

achow101 commented at 6:44 am on July 10, 2025: member

x86_64 guix build:

0$ find guix-build-3add4b365c90/output/ -type f -print0 | env LC_ALL=C sort -z | xargs -r0 sha256sum
15d3ec518a66bcff88ae4032a352119945ecae1cad0337bdadc84315dd9405a2c  guix-build-3add4b365c90/output/dist-archive/bitcoin-3add4b365c90.tar.gz
29a317eb639f20191d13df01c11cd71d6da0f2593c9b6048f882ef40e0a080cc1  guix-build-3add4b365c90/output/x86_64-w64-mingw32/SHA256SUMS.part
3083fb802296373e8d0faa4fdda4ce072f0f95f9443e9ba0682cb5da6dc55d7b6  guix-build-3add4b365c90/output/x86_64-w64-mingw32/bitcoin-3add4b365c90-win64-codesigning.tar.gz
41aa4834f17fee7b6cb7b5c72878eb6f6b3875d23ab35337c02f92768657fa257  guix-build-3add4b365c90/output/x86_64-w64-mingw32/bitcoin-3add4b365c90-win64-debug.zip
5663b6fb28095c5c6ba331675c257a0ce3ef7d92f4c04e2ff96a093e030322e88  guix-build-3add4b365c90/output/x86_64-w64-mingw32/bitcoin-3add4b365c90-win64-setup-unsigned.exe
6ad8ab977a123d56642aef2291a4b9d0d662be9cdbbcc304c079b898e5f37fb33  guix-build-3add4b365c90/output/x86_64-w64-mingw32/bitcoin-3add4b365c90-win64-unsigned.zip

Sjors commented at 7:10 am on July 10, 2025: member

Concept ACK

I’ll spin up a new aarch64 guix machine to test if this actually makes determinism go away. Even if it doesn’t, the change itself is fine since it removes a line, but you’d have to change the commit message.

3add4b365c90c0943f18dd26d4a18c1e2ee5860e builds and passes the tests locally for me on Apple Silicon macOS 15.5. Not sure what’s up with CI.

x86_64 guix hashes:

05d3ec518a66bcff88ae4032a352119945ecae1cad0337bdadc84315dd9405a2c  guix-build-3add4b365c90/output/dist-archive/bitcoin-3add4b365c90.tar.gz
19a317eb639f20191d13df01c11cd71d6da0f2593c9b6048f882ef40e0a080cc1  guix-build-3add4b365c90/output/x86_64-w64-mingw32/SHA256SUMS.part
2083fb802296373e8d0faa4fdda4ce072f0f95f9443e9ba0682cb5da6dc55d7b6  guix-build-3add4b365c90/output/x86_64-w64-mingw32/bitcoin-3add4b365c90-win64-codesigning.tar.gz
31aa4834f17fee7b6cb7b5c72878eb6f6b3875d23ab35337c02f92768657fa257  guix-build-3add4b365c90/output/x86_64-w64-mingw32/bitcoin-3add4b365c90-win64-debug.zip
4663b6fb28095c5c6ba331675c257a0ce3ef7d92f4c04e2ff96a093e030322e88  guix-build-3add4b365c90/output/x86_64-w64-mingw32/bitcoin-3add4b365c90-win64-setup-unsigned.exe
5ad8ab977a123d56642aef2291a4b9d0d662be9cdbbcc304c079b898e5f37fb33  guix-build-3add4b365c90/output/x86_64-w64-mingw32/bitcoin-3add4b365c90-win64-unsigned.zip

maflcko commented at 7:21 am on July 10, 2025: member

Congrats, you are triggering another compiler bug. See 9999dbc1bd171931f02266d7c1a5cfd39f49238e for more details.

in src/qt/walletmodel.cpp:182 in 3add4b365c outdated

178@@ -179,8 +179,7 @@ WalletModel::SendCoinsReturn WalletModel::prepareTransaction(WalletModelTransact
179             setAddress.insert(rcp.address);
180             ++nAddresses;
181 
182-            CRecipient recipient{DecodeDestination(rcp.address.toStdString()), rcp.amount, rcp.fSubtractFeeFromAmount};
183-            vecSend.push_back(recipient);
184+            vecSend.emplace_back(DecodeDestination(rcp.address.toStdString()), rcp.amount, rcp.fSubtractFeeFromAmount);

maflcko commented at 7:24 am on July 10, 2025:

0            vecSend.emplace_back(CRecipient {DecodeDestination(rcp.address.toStdString()), rcp.amount, rcp.fSubtractFeeFromAmount});

I don’t have an apple, but I guess this may be the patch to work around the xcode compiler bug. I don’t know if that still works around the non-determinism.

As a completely different alternative, you could also try #32597 (review) to remove the std::string construction from the header file and maybe take some load off of the optimize passes or linker?

DrahtBot added the label CI failed on Jul 10, 2025

Resolve guix non-determinism with emplace_back instead of push_back

For some reason, building x86_64-w64-mingw32 on x86_64 and aarch64
results in a single instruction difference which can be traced down to
prevector.h:174. The ultimate caller of this is the copy constructor for
a prevector that ends up being called by std::vector::push_back in
walletmodel.cpp:183. By replacing the push_back with an emplace_back,
somehow this non-determinism goes away.

f43571010e

achow101 force-pushed on Jul 10, 2025

achow101 commented at 8:11 pm on July 10, 2025: member

guix builds of f43571010e38

x86_64

0$ find guix-build-f43571010e38/output/ -type f -print0 | env LC_ALL=C sort -z | xargs -r0 sha256sum
1bb9e6435df82cfe01c78e046ef39cea62a3b0ad0deafa66e1d2a15257bc21eeb  guix-build-f43571010e38/output/dist-archive/bitcoin-f43571010e38.tar.gz
27d3bc9459a51abcb2d9d36b69fc135cbb4940c092a9f8df63032c6e9f659a958  guix-build-f43571010e38/output/x86_64-w64-mingw32/SHA256SUMS.part
334af63c893a315745364d1f4dd36bbc1521eef0cad42ea964ba1d0dc5cbcf738  guix-build-f43571010e38/output/x86_64-w64-mingw32/bitcoin-f43571010e38-win64-codesigning.tar.gz
420929256891cacbeb58f75644257631273c4096597a9e4e366c61807ce87eade  guix-build-f43571010e38/output/x86_64-w64-mingw32/bitcoin-f43571010e38-win64-debug.zip
58d39a4e45bc66dad9c9becd16ab11a77abc976a0091d370b26977d5194ed9535  guix-build-f43571010e38/output/x86_64-w64-mingw32/bitcoin-f43571010e38-win64-setup-unsigned.exe
6969f416e0004ecba38e4dbec9bb60e0d5c07f21aae6d72f49480bb85426725ab  guix-build-f43571010e38/output/x86_64-w64-mingw32/bitcoin-f43571010e38-win64-unsigned.zip

aarch64

0bb9e6435df82cfe01c78e046ef39cea62a3b0ad0deafa66e1d2a15257bc21eeb  guix-build-f43571010e38/output/dist-archive/bitcoin-f43571010e38.tar.gz
17d3bc9459a51abcb2d9d36b69fc135cbb4940c092a9f8df63032c6e9f659a958  guix-build-f43571010e38/output/x86_64-w64-mingw32/SHA256SUMS.part
234af63c893a315745364d1f4dd36bbc1521eef0cad42ea964ba1d0dc5cbcf738  guix-build-f43571010e38/output/x86_64-w64-mingw32/bitcoin-f43571010e38-win64-codesigning.tar.gz
320929256891cacbeb58f75644257631273c4096597a9e4e366c61807ce87eade  guix-build-f43571010e38/output/x86_64-w64-mingw32/bitcoin-f43571010e38-win64-debug.zip
48d39a4e45bc66dad9c9becd16ab11a77abc976a0091d370b26977d5194ed9535  guix-build-f43571010e38/output/x86_64-w64-mingw32/bitcoin-f43571010e38-win64-setup-unsigned.exe
5969f416e0004ecba38e4dbec9bb60e0d5c07f21aae6d72f49480bb85426725ab  guix-build-f43571010e38/output/x86_64-w64-mingw32/bitcoin-f43571010e38-win64-unsigned.zip

in src/qt/walletmodel.cpp:182 in f43571010e

178@@ -179,8 +179,7 @@ WalletModel::SendCoinsReturn WalletModel::prepareTransaction(WalletModelTransact
179             setAddress.insert(rcp.address);
180             ++nAddresses;
181 
182-            CRecipient recipient{DecodeDestination(rcp.address.toStdString()), rcp.amount, rcp.fSubtractFeeFromAmount};
183-            vecSend.push_back(recipient);
184+            vecSend.emplace_back(CRecipient{DecodeDestination(rcp.address.toStdString()), rcp.amount, rcp.fSubtractFeeFromAmount});

l0rinc commented at 8:22 pm on July 10, 2025:

do we need to create a temp copy or would this also work?

0            vecSend.emplace_back(DecodeDestination(rcp.address.toStdString()), rcp.amount, rcp.fSubtractFeeFromAmount);

achow101 commented at 8:24 pm on July 10, 2025:

#32930 (comment) and #32930 (review)

l0rinc commented at 8:24 pm on July 10, 2025:

Ah, it was already the case before, my bad

l0rinc commented at 8:25 pm on July 10, 2025: contributor

code review ACK f43571010e3853e83a21aa4774b1c8da47b5d961

DrahtBot requested review from maflcko on Jul 10, 2025

DrahtBot requested review from Sjors on Jul 10, 2025

DrahtBot removed the label CI failed on Jul 10, 2025

Resolve guix non-determinism with emplace_back instead of push_back #32930

Code Coverage & Benchmarks

Reviews