Age | Commit message (Collapse) | Author |
|
fae885b98fa1da702d1b36f4bf99209595816984 ci: Run one task with all tests on credits (MarcoFalke)
Pull request description:
This should allow to detect any obvious issues in the tests within 10 minutes of opening a pull request, regardless of the current scheduling load on the Cirrus CI community cluster.
ACKs for top commit:
fanquake:
ACK fae885b98fa1da702d1b36f4bf99209595816984
Tree-SHA512: 384651364648cd38a126f64a5cc650dd396735c05f7d8b5d5c9a8ee7b3b98e36f5dfb3b1c0563ba4d9f554b5e0e82e13aa38e78092f11856a82fc76270fe6f3e
|
|
fadfae42f1be240af3198565068b409a62e698a0 ci: Remove unused busybox workaround (MarcoFalke)
fac424fce722fd0a2084d5792ceed200c773158b ci: Create named symbol for BINS_SCRATCH_DIR (MarcoFalke)
Pull request description:
It can be re-added when it is needed again. But it may be more likely that the other workarounds can be removed as well, in a follow-up.
ACKs for top commit:
fanquake:
ACK fadfae42f1be240af3198565068b409a62e698a0
Tree-SHA512: 39984d02ddcba907a5b780fd9cb4dab7f1abbc32468968b4862d226de2e4f95c71708c7361db97347e1ac65196dc40d379e68f34bef37d42b5464d343e7466d8
|
|
109cbb819ddd20220a255791c40261f746260f42 doc: Add release notes for #26618 (Aurèle Oulès)
b13902d2e45ad1e73f79e221ffc16ce26b7c3ba5 rpc: Prevent unloading a wallet when rescanning (Aurèle Oulès)
Pull request description:
Fixes #26463.
This PR prevents a user from unloading a wallet if it is currently rescanning.
To test:
```bash
./src/bitcoin-cli -testnet -named createwallet wallet_name=wo disable_private_keys=true
./src/bitcoin-cli -testnet -rpcwallet=wo importdescriptors '[{
"desc": "addr(mmcuW74MyJUZuLnWXGQLoNXPrS9RbFz6gD)#tpnrahgc",
"timestamp": 0,
"active": false,
"internal": false,
"next": 0
}]'
./src/bitcoin-cli -testnet unloadwallet wo
error code: -4
error message:
Wallet is currently rescanning. Abort existing rescan or wait.
ACKs for top commit:
achow101:
ACK 109cbb819ddd20220a255791c40261f746260f42
w0xlt:
ACK https://github.com/bitcoin/bitcoin/pull/26618/commits/109cbb819ddd20220a255791c40261f746260f42
kouloumos:
ACK 109cbb819ddd20220a255791c40261f746260f42
promag:
ACK 109cbb819ddd20220a255791c40261f746260f42
Tree-SHA512: 15fdddf4cf9f3fa08f52069fe4a25a76e04a55bb2586b031bfb0393dce7f175dcdb52823e132a7dff6a894539beeb980a1aad2a792790be036be6977628149b2
|
|
|
|
in `setban`
abccb27466c5a240a59f3f7c86f2bc60d51e9fee test: add coverage for absolute timestamp in `setban` (brunoerg)
b99f1f20f773eb55c870a033b3f2e8f13d55d0c8 p2p, rpc: don't allow past absolute timestamp in `setban` (brunoerg)
Pull request description:
We shouldn't allow call `setban` with past absolute timestamp. First, because doesn't make sense to ban a node until ~ past ~. Besides that, it could make an unnecessary write to the DB since `BanMan::Ban` calls `DumpBanlist` and it calls `SweepBanned` which will remove this new ban (because of the timestamp) of the array.
ACKs for top commit:
1440000bytes:
ACK https://github.com/bitcoin/bitcoin/pull/26822/commits/abccb27466c5a240a59f3f7c86f2bc60d51e9fee
Tree-SHA512: 6d0cadf99fc4f78d77d3bafd6f5c85ac56e243ebc376210fdb2bee751e7b139ec7d6f5f346317fd0b10051b685f2d0ee1d8e40f4bc10f4dbdbbddd5e1ee84de5
|
|
The find workaround is no longer needed after commit
d3d547c545021d8339db666d36e48f10ea478f9c
|
|
Also, create the dir a bit earlier, right after it was put in the PATH.
|
|
fa4e98c77f8c6b693b88c1a6bb811650a6267fdb ci: Fix ci_native_fuzz_msan CONTAINER_NAME (MarcoFalke)
Pull request description:
This avoids a duplicate name with the other msan task, which will lead to errors when running locally:
> Error: creating container storage: the container name "ci_native_msan" is already in use by 77350e26f9c36abbb601140cd0b485ead093ff118803c720ca8b10f6bdfa37d2. You have to remove that container to be able to reuse that name: that name is already in use
ACKs for top commit:
fanquake:
ACK fa4e98c77f8c6b693b88c1a6bb811650a6267fdb
hebasto:
ACK fa4e98c77f8c6b693b88c1a6bb811650a6267fdb, I've verified that there are no other duplicated `CONTAINER_NAME`'s values.
Tree-SHA512: f1b28b21302c0947912d642c12c2ccad236af6824fd27e68341baddedec24087af738f3226028a0eeb6e0fc7e9f90713fc680855eeb07adc113c4f6e8b03a545
|
|
|
|
|
|
|
|
for multiple test instances
c6119f478876f245ca5c65dd05da4cdc5be0e91f tests: Use unique port for ZMQ tests (Andrew Chow)
Pull request description:
The ZMQ interface tests should use unique ports as we do for the p2p and rpc ports so that multiple instances of the test can be run at the same time.
Without this, the test may hang until killed, or fail.
ACKs for top commit:
MarcoFalke:
ACK c6119f478876f245ca5c65dd05da4cdc5be0e91f
Tree-SHA512: 2ca3ed2f35e5a83d7ab83740674fed362a8d146dc751156cfe100133a591347cd1ac9d164046f1744d65451a57c52cb22d3bb2161105f421f8f655c4a2512c59
|
|
2427468f27ab5db0872b5edfc069cdf702e03d19 doc: Update Boost version in doc/dependencies.md (Hennadii Stepanov)
e8b4201ba2fd7745edc26543704d2b4a51dbb151 build: Update Boost to 1.81.0 in depends (Hennadii Stepanov)
Pull request description:
Required for https://github.com/bitcoin/bitcoin/pull/25696, also see https://github.com/bitcoin/bitcoin/pull/25696#discussion_r1005600540.
Guix build:
```
0aade9c6a91d8550dea6cb07d4657c299a78d4434c1e78ef487e30e53239da64 guix-build-e8b4201ba2fd/output/aarch64-linux-gnu/SHA256SUMS.part
f21964d25f96bca46ced3286b00e7e605d6517a6de7b00aa7ebf3bb6ee63d0d2 guix-build-e8b4201ba2fd/output/aarch64-linux-gnu/bitcoin-e8b4201ba2fd-aarch64-linux-gnu-debug.tar.gz
65e991407232714455a4bb5fb072b3b7f58f8f4696ca78b6756aae6e6497540b guix-build-e8b4201ba2fd/output/aarch64-linux-gnu/bitcoin-e8b4201ba2fd-aarch64-linux-gnu.tar.gz
92535c8b7ead8f3319858e55c7d076b3db174efd10b4b8dce8efbbd5e5b98819 guix-build-e8b4201ba2fd/output/arm-linux-gnueabihf/SHA256SUMS.part
9cd199415fbfe6ee6fdf5c57def2591327c97e7e31c4c7b323a738761fe6a285 guix-build-e8b4201ba2fd/output/arm-linux-gnueabihf/bitcoin-e8b4201ba2fd-arm-linux-gnueabihf-debug.tar.gz
e13dacef38e68bb30283fda82bbf73864e5977b4cc6f82be41476246c7f51e90 guix-build-e8b4201ba2fd/output/arm-linux-gnueabihf/bitcoin-e8b4201ba2fd-arm-linux-gnueabihf.tar.gz
e4c57c126927c2b40a3bd7c697e8b524140f989094c2a9ff216e1c03e90810da guix-build-e8b4201ba2fd/output/arm64-apple-darwin/SHA256SUMS.part
90b15bd6e8eab3c541a2a057402cf41630e5c72d07bf62b3d2d4d54698b3f03b guix-build-e8b4201ba2fd/output/arm64-apple-darwin/bitcoin-e8b4201ba2fd-arm64-apple-darwin-unsigned.dmg
848c7928d633f19912f340fa6df8b77756d291991dbd849b3d95761bcd445bad guix-build-e8b4201ba2fd/output/arm64-apple-darwin/bitcoin-e8b4201ba2fd-arm64-apple-darwin-unsigned.tar.gz
9880bfc57770372478c56b22b513d91d25a799591a345069e04c3f3a88bfc9ec guix-build-e8b4201ba2fd/output/arm64-apple-darwin/bitcoin-e8b4201ba2fd-arm64-apple-darwin.tar.gz
e61cb6337dc1480292300fcd45e9b128c567222911cac3a9209955433e1f413b guix-build-e8b4201ba2fd/output/dist-archive/bitcoin-e8b4201ba2fd.tar.gz
fd685ffcd1c5e32c25a84638fd47c5e14d0247dcf7d51e5317fea356f8c8347a guix-build-e8b4201ba2fd/output/powerpc64-linux-gnu/SHA256SUMS.part
b710e9123684e459732553b1d6df2912257ac43b315db7f2e9288d95c76ce2b6 guix-build-e8b4201ba2fd/output/powerpc64-linux-gnu/bitcoin-e8b4201ba2fd-powerpc64-linux-gnu-debug.tar.gz
ea7878ea044dbfaf9c022de5bf7532bf00b028c8582d24ce67bec917f34869e9 guix-build-e8b4201ba2fd/output/powerpc64-linux-gnu/bitcoin-e8b4201ba2fd-powerpc64-linux-gnu.tar.gz
737d9e054356ea50c7efe964a7b223eb81a7fcc10b235647ffcb588784e7e212 guix-build-e8b4201ba2fd/output/powerpc64le-linux-gnu/SHA256SUMS.part
46de2537ea90ddaced620a7dbfa1379b2d0cc80ae87d6225fef94a7196ff5166 guix-build-e8b4201ba2fd/output/powerpc64le-linux-gnu/bitcoin-e8b4201ba2fd-powerpc64le-linux-gnu-debug.tar.gz
7e10be7f494bd4635477919ca9b9968b62054e0448f2963d1c25f0215361feae guix-build-e8b4201ba2fd/output/powerpc64le-linux-gnu/bitcoin-e8b4201ba2fd-powerpc64le-linux-gnu.tar.gz
cfb90022ea23526d8ca26259a06d2ec06a278e8fbcb05d6ba37fa3ef33ba1f77 guix-build-e8b4201ba2fd/output/riscv64-linux-gnu/SHA256SUMS.part
ba45d57e19758682991fc51cc04c0e13e7e586b74ae1adae0273cd15bdd5cdb2 guix-build-e8b4201ba2fd/output/riscv64-linux-gnu/bitcoin-e8b4201ba2fd-riscv64-linux-gnu-debug.tar.gz
970941bee4c342b7b23065ce515d5e340b3aed989210a569faa33f638a3575f9 guix-build-e8b4201ba2fd/output/riscv64-linux-gnu/bitcoin-e8b4201ba2fd-riscv64-linux-gnu.tar.gz
3943c224a3fa0554ca75292f301ae02514c7ec100d9e13e185ed6ad4db932194 guix-build-e8b4201ba2fd/output/x86_64-apple-darwin/SHA256SUMS.part
4c5f14af1961e33336423a2021b5da76b550ae1bc2284003d6009b1ced940eea guix-build-e8b4201ba2fd/output/x86_64-apple-darwin/bitcoin-e8b4201ba2fd-x86_64-apple-darwin-unsigned.dmg
3ab7fd5623b414c4e0504c2943b84048524374150e9626b3a0b6931adc46109a guix-build-e8b4201ba2fd/output/x86_64-apple-darwin/bitcoin-e8b4201ba2fd-x86_64-apple-darwin-unsigned.tar.gz
00c4c2da392eaee439984ab2bd614d2df6b48ba775350b2c6d48c3061e7667c5 guix-build-e8b4201ba2fd/output/x86_64-apple-darwin/bitcoin-e8b4201ba2fd-x86_64-apple-darwin.tar.gz
df20f8a3c8836ca5b7bac4c5104a31f4be76da75a67846d69cc8cbdf42339743 guix-build-e8b4201ba2fd/output/x86_64-linux-gnu/SHA256SUMS.part
68f72876397bfbf6cde73b8303bd081235407211d684cdb78fc932ffb0752942 guix-build-e8b4201ba2fd/output/x86_64-linux-gnu/bitcoin-e8b4201ba2fd-x86_64-linux-gnu-debug.tar.gz
769556d6a4a8baed5dfb81ab63afd1c0999101a5afeb2f7a1bab9b94ce6635bc guix-build-e8b4201ba2fd/output/x86_64-linux-gnu/bitcoin-e8b4201ba2fd-x86_64-linux-gnu.tar.gz
4b8487e9b16e0ec6645cb63ddd25149011797af14d596ed31a23130f201d8f57 guix-build-e8b4201ba2fd/output/x86_64-w64-mingw32/SHA256SUMS.part
6fbad9caf9da6041f0d119be293d0895b50535604f86dda45a7b8345fb581a39 guix-build-e8b4201ba2fd/output/x86_64-w64-mingw32/bitcoin-e8b4201ba2fd-win64-debug.zip
42e84737cac5ec2d2052e5632f68bc6d211d5f02a171cb7347c067025edce13f guix-build-e8b4201ba2fd/output/x86_64-w64-mingw32/bitcoin-e8b4201ba2fd-win64-setup-unsigned.exe
9205f92b1ecfcfa649552e27bb0942611dfac2a5f99c326b7de89bbeb46ad564 guix-build-e8b4201ba2fd/output/x86_64-w64-mingw32/bitcoin-e8b4201ba2fd-win64-unsigned.tar.gz
098d98161393e663da794cc336de8a4676fd2c8bc14dbc8b1cac1aaf6b4098f5 guix-build-e8b4201ba2fd/output/x86_64-w64-mingw32/bitcoin-e8b4201ba2fd-win64.zip
```
ACKs for top commit:
fanquake:
ACK 2427468f27ab5db0872b5edfc069cdf702e03d19
Tree-SHA512: 31d268e4608ab75f843c0cb1874d060cea762ebbd09b220a7f84054fb837f60b41b8452c181a4de7aa3f8aa27fd584d0a84e770209e02adfb978d2b5f625d7a6
|
|
|
|
Werror=free-nonheap-object in AssumeCalculateMemPoolAncestors
faa86eeb418ac5a28e7c4aa6cd13f607e151fad8 refactor: Work around Werror=free-nonheap-object in AssumeCalculateMemPoolAncestors (MarcoFalke)
Pull request description:
This works around the s390x gcc bug mentioned in https://github.com/bitcoin/bitcoin/issues/26820
ACKs for top commit:
achow101:
ACK faa86eeb418ac5a28e7c4aa6cd13f607e151fad8
Tree-SHA512: 041d5daa157ea1856b0a8027181085d70624f5f8822049ace9963e90c653bbb8c91d1f16b8a5bf460687eb4ed13f1db72e3885a511aadbad6dede93d9f9ccd6d
|
|
AssumeCalculateMemPoolAncestors
|
|
watchonly/solvable wallets
730e14a317ae45fe871c8d6f44a51936756bbbea test: wallet: check that labels are migrated to watchonly wallet (Sebastian Falbesoner)
d5f4ae7fac0bceb0c9ad939b9a4fbdb85da0bf95 wallet: fully migrate address book entries for watchonly/solvable wallets (Sebastian Falbesoner)
Pull request description:
Currently `migratewallet` migrates the address book (i.e. labels and purposes) for watchonly and solvable wallets only in RAM, but doesn't persist them on disk. Fix this by adding another loop for both of the special wallet types after which writes the corresponding NAME and PURPOSE entries to the database in a single batch. Also adds a corresponding test that checks if labels were migrated correctly for a watchonly wallet.
ACKs for top commit:
achow101:
ACK 730e14a317ae45fe871c8d6f44a51936756bbbea
furszy:
code ACK 730e14a3, left a non-blocking nit.
aureleoules:
ACK 730e14a317ae45fe871c8d6f44a51936756bbbea
Tree-SHA512: 159487e11e858924ef762e0190ccaea185bdff239e3d2280c8d63c4ac2649ec71714dc4d53dec644f03488f91c3b4bbbbf3434dad23bc0fcecb6657f353ea766
|
|
instead of c style casts
f2fc03ec856d7d19a20c482514350cced38f9504 refactor: use braced init for integer constants instead of c style casts (Pasta)
Pull request description:
See https://github.com/bitcoin/bitcoin/pull/23810 for more context. This is broken out from that PR, as it is less breaking, and should be trivial to review and merge.
EDIT: Long term, the intention is to remove all C-style casts, as they can dangerously introduce reinterpret_casts. This is one step which removes a number of trivially removable C-style casts
ACKs for top commit:
aureleoules:
ACK f2fc03ec856d7d19a20c482514350cced38f9504
Tree-SHA512: 2fd11b92c9147e3f970ec3e130e3b3dce70e707ff02950a8c697d4b111ddcbbfa16915393db20cfc8f384bc76f13241c9b994a187987fcecd16a61f8cc0af14c
|
|
fac810bb0a524e79014882f8fc694efade35de9f test: Fix feature_startupnotify intermittent issue (MarcoFalke)
Pull request description:
Might fix #25644
ACKs for top commit:
aureleoules:
ACK fac810bb0a524e79014882f8fc694efade35de9f
brunoerg:
ACK fac810bb0a524e79014882f8fc694efade35de9f
Tree-SHA512: 870bf65da8120b6897d02e3bb70eea018d4761396abe64c3533bbc5237e65be9f77d35f62cd5d08cf7132dd53b504bf58229c33e18833c191495ad229c84d7c2
|
|
|
|
e6864fa157d75d8ae2b2f56620b019bde2355a24 contrib: remove builder keys (fanquake)
Pull request description:
This has been superseded by adding a builder-keys/ directory in
guix.sigs, where the presence of keys, and validity of signatures
is checked. Preventing issues like missing keys or invalid signatures.
New (or exisiting) Guix builders can add their key in the next PR
they open adding attestations.
Related to issues like #26566, #26563.
Also follows up with the comment here: https://github.com/bitcoin/bitcoin/pull/26565#issuecomment-1326053939.
ACKs for top commit:
hebasto:
ACK e6864fa157d75d8ae2b2f56620b019bde2355a24, modulo s/update/remove/ in the PR tittle.
Tree-SHA512: 095b4cf12ed0baeaf0ee7b8edcb3e2647e9c0f812e8fd63915ddb454f81dacc9c2d2b409de2773b7adb5ff643893d614d8aad1bc44c26da648e1bbbe19e11e05
|
|
cross-chain legacy wallet restore
21ad4e26ec320dcecc8961888bc82d0bb72d5ed3 test: add coverage for cross-chain wallet restore (Sebastian Falbesoner)
8c7222bda3f7136f312a6e57b76d6a2d0a114f68 wallet: fix GUI crash on cross-chain legacy wallet restore (Sebastian Falbesoner)
Pull request description:
Restoring a wallet backup from another chain should result in a dedicated error message (we have _"Wallet files should not be reused across chains. Restart bitcoind with -walletcrosschain to override."_ for that). Unfortunately this is currently not the case for legacy wallet restores, as in the course of cleaning up the newly created wallet directory a `filesystem_error` exception is thrown due to the directory not being empty; the wallet database did indeed load successfully (otherwise we wouldn't know that the chain doesn't match) and hence BDB-related files and directories are already created in the wallet directory.
For bitcoind, this leads to a very confusing error message:
```
$ ./src/bitcoin-cli restorewallet test123 ~/.bitcoin/regtest/wallets/regtest_wallet/wallet.dat
error code: -1
error message: filesystem error: in remove: Directory not empty ["/home/thestack/.bitcoin/wallets/test123"]
```
Even worse, the GUI crashes in such a scenario:
```
libc++abi: terminating with uncaught exception of type std::__1::__fs::filesystem::filesystem_error: filesystem error: in remove: Directory not empty ["/home/thestack/.bitcoin/wallets/foobar"]
Abort trap (core dumped)
```
Fix this by simply deleting the whole folder via `fs::remove_all`. With this, the expected error message appears both for the `restorewallet` RPC call and in the GUI (as a message-box):
```
$ ./src/bitcoin-cli restorewallet test123 ~/.bitcoin/regtest/wallets/regtest_wallet/wallet.dat
error code: -4
error message:
Wallet loading failed. Wallet files should not be reused across chains. Restart bitcoind with -walletcrosschain to override.
```
ACKs for top commit:
achow101:
ACK 21ad4e26ec320dcecc8961888bc82d0bb72d5ed3
aureleoules:
ACK 21ad4e26ec320dcecc8961888bc82d0bb72d5ed3
furszy:
utACK 21ad4e26
Tree-SHA512: 313f6494c2fbe823bff9b975cb2d9410bb518977a1e59a5159ee9836bc012947fa50b56be0e41b1a2f50d9c0c7f4fddfdf4fbe479d8a59a6ee44bb389c804abc
|
|
585c6722128537f772043ef4c87238e283669b8a compat: use STDIN_FILENO over 0 (fanquake)
Pull request description:
This is already used throughout this file, and is self-documenting.
ACKs for top commit:
john-moffett:
ACK 585c6722128537f772043ef4c87238e283669b8a
achow101:
ACK 585c6722128537f772043ef4c87238e283669b8a
hebasto:
ACK 585c6722128537f772043ef4c87238e283669b8a, I have reviewed the code and it looks OK, I agree it can be merged.
kristapsk:
utACK 585c6722128537f772043ef4c87238e283669b8a
aureleoules:
ACK 585c6722128537f772043ef4c87238e283669b8a
Tree-SHA512: c0114ae896ba5404be70b804ee9f454d213f1d789c8f5a578c422dd15a308a214e6851fee76c0ec736a212bc86fb33ec17af1b22e5d23422c375ca4458251356
|
|
entries are set to 0, not removed
f537127271b1f22ee4651915b7b9266e0df72841 doc: fix: prevHeights entries are set to 0, not removed (stickies-v)
Pull request description:
In [`CalculateSequenceLocks`](https://github.com/bitcoin/bitcoin/blob/a035b6a0c418d0b720707df69559028bd662fa70/src/consensus/tx_verify.h#L69) no items are removed from `prevHeights`, they are just set to 0:
https://github.com/bitcoin/bitcoin/blob/a035b6a0c418d0b720707df69559028bd662fa70/src/consensus/tx_verify.cpp#L69-L73
This PR updates the docs to reflect the actual implementation. Seems to have been wrongly documented since introduction in #7184 already ([implementation](https://github.com/bitcoin/bitcoin/pull/7184/files#diff-34d21af3c614ea3cee120df276c9c4ae95053830d7f1d3deaf009a4625409ad2R742-R749) and [documentation](https://github.com/bitcoin/bitcoin/pull/7184/files#diff-34d21af3c614ea3cee120df276c9c4ae95053830d7f1d3deaf009a4625409ad2R712-R713))
ACKs for top commit:
hebasto:
ACK f537127271b1f22ee4651915b7b9266e0df72841
Tree-SHA512: 3661501660f6832b2116fd83466ffe95a60b341c14cb09a37489e2a587bea3290b0528690120a0f644c3eea02177aa1fb8968258482fa43b0303e016abb17418
|
|
interface methods
55696a0ac30bcfbd555f71cbc8eac23b725f7dcf wallet: remove `mempool_sequence` from `transactionRemovedFromMempool` (w0xlt)
bf19069c53501231a2f3ba59afa067913ec4d3b2 wallet: remove `mempool_sequence` from `transactionAddedToMempool` (w0xlt)
Pull request description:
This PR removes `mempool_sequence` from `transactionRemovedFromMempool` and `transactionAddedToMempool`.
`mempool_sequence` is not used in these methods, only in ZMQ notifications.
ACKs for top commit:
instagibbs:
ACK https://github.com/bitcoin/bitcoin/pull/26752/commits/55696a0ac30bcfbd555f71cbc8eac23b725f7dcf
Tree-SHA512: 621e89230bcb6edfed83e2758601a2b093822fc2dc4e9bfb00487e340f2bc4c5ac3bf6df3ca00b7fe55bb3df15858820f2bf698f403d2e48b915dd9eb47b63e0
|
|
to make tests independent
b942c94d153f83b77ef5d603211252d9abadde95 test: Change coinselection parameter location to make tests independent (yancy)
Pull request description:
the `subtract_fee_outputs` param is expected to be `true` for all subsequent tests. It should be defined outside of a single test so that if it's removed or changed, all subsequent tests won't fail. Currently if you remove this [test](https://github.com/bitcoin/bitcoin/blob/master/src/wallet/test/coinselector_tests.cpp#L304:L325) the following [test](https://github.com/bitcoin/bitcoin/blob/master/src/wallet/test/coinselector_tests.cpp#L327:L345) fails. This change makes the tests independent.
ACKs for top commit:
achow101:
ACK b942c94d153f83b77ef5d603211252d9abadde95
aureleoules:
ACK b942c94d153f83b77ef5d603211252d9abadde95.
rajarshimaitra:
tACK b942c94d153f83b77ef5d603211252d9abadde95
theStack:
ACK b942c94d153f83b77ef5d603211252d9abadde95
Tree-SHA512: 461e19d15351318102ef9f96c68442365d8ca238c48ad7aefe23e8532b33b91dadf6c7840c7894574bccede6da162a55ad7a6f6a330d61a11ce804e68ddc5e9c
|
|
'AvailableCoins' function.
3a4f8bc24271d05765e9bf1e26558a28ab2e6b81 bench: add benchmark for wallet 'AvailableCoins' function. (furszy)
Pull request description:
#### Rationale
`AvailableCoins` is part of several important flows for the wallet; from RPC commands that create transactions like `fundrawtransaction`, `send`, `walletcreatefundedpsbt`, get the available balance, list the available coins with `listunspent` etc. to GUI connected processes that perform the same or similar actions: tx creation, available balance calculation, present the spendable coins in the coin control dialog.
As we are improving this process in #24699, #25005 and there are more structural changes coming on the way. This benchmark aims to ensure us that, at least, there are no regressions (obviously performance improvements are great but, at least for me, this heads into the direction of having a base metric to compare future structural changes).
#### Implementation Notes
There are 5 new benchmarks, one per wallet supported output type (LEGACY, P2SH_SEGWIT, BECH32, BECH32M), plus a multi-output-type wallet benchmark which contains outputs from all the descriptor types.
The test, by default, fills-up the wallet with 1k transactions, 2k outputs. Mainly to not consume much time if the user just want to verify that no substantial regressions were introduced. But, my expectation for those who are focused on this process is to use a much higher number locally to really note the differences across commits.
ACKs for top commit:
achow101:
ACK 3a4f8bc24271d05765e9bf1e26558a28ab2e6b81
hernanmarino:
ACK 3a4f8bc24271d05765e9bf1e26558a28ab2e6b81
aureleoules:
ACK 3a4f8bc24271d05765e9bf1e26558a28ab2e6b81
Tree-SHA512: d0bb4c165f1efa181b47cb31561e6217eff9135bcd1b6761a7292f9018e456d13d18a1b886c2e2268d35c52f9e1fd8e0f252972424e5c5f00c280620b79c5a1b
|
|
fa5cbf2290dab08296cabbd973e9adefc883f3fc ci: Properly set COMMIT_RANGE in lint task (MarcoFalke)
Pull request description:
Currently the variable holds (apart from the commits in the pull request) all commits to master since the pull was opened.
This is problematic, because already merged commits are linted in unrelated pulls, leading to:
* Wasted resources. For example, currently the lint task may take 9 minutes, when it should take 1. See https://cirrus-ci.com/task/6032782770569216?logs=lint#L1449
* False failures. For example, when a "wrong" commit is in master it can lead to some pulls failing unrelatedly, and others not.
Now that the CI has the `/merge` commit (since commit fad7281d7842f337932cf44e703fdd631230ddd6), `COMMIT_RANGE` can simply be set to `HEAD~..HEAD` to only hold the changes in the pull.
ACKs for top commit:
fanquake:
ACK fa5cbf2290dab08296cabbd973e9adefc883f3fc
Tree-SHA512: e85fca4ca9d2615ddd2544403485e06885769a3f70bca297e23eefda2a1d28f47c5271f6adfa6ce0e5e972335c78098b76e0db4b109f59d0986bf508cef7528f
|
|
subprocesses as for `all-lint.py`
f6eadaa413f20c1853cf1bea5141560cd5e4b15a Use same Python executable for subprocesses as for all-lint.py (Kristaps Kaupe)
Pull request description:
Before this all linters were ran by `/usr/bin/env python3`, no matter what was used to run `test/lint/all-lint.py`. This change allows to use non-default Python executable for `test/lint/all-lint.py` and then all subprocesses will also use same Python interpreter (for example, `python3.10 ./test/lint/all-lint.py`). See https://github.com/bitcoin/bitcoin/issues/26792#issuecomment-1369558866 as use case.
ACKs for top commit:
fanquake:
ACK f6eadaa413f20c1853cf1bea5141560cd5e4b15a - did not test
Tree-SHA512: 4da3b5581a0dd8ab9a6387829495019091a93a7ceaf2135d65d40a1983fd11a0b92b20891ef30d2a132abb0a690cd9b2f7eb5fcc38df06a340394ef449d640af
|
|
This is already used throughout this file, and is self-documenting.
|
|
f84e445dee5f4c5d65cb702958701ff7b38ebdce doc: Correct linked Microsoft URLs (Suriyaa Sundararuban)
Pull request description:
Update Microsoft-related links.
Top commit has no ACKs.
Tree-SHA512: 40c7b25a96772259fb04da1946d52f6aac9562262aef472ae75807bfbd246de47d72118140a12f7553037b94b89f95d69dea6ce30e611ac3d71a32d102355150
|
|
FreeBSD
22e9afe40d987f4f90bc8469f9475df138fe6261 use sha256 command instead of sha256sum on FreeBSD (Murray Nesbitt)
Pull request description:
The FreeBSD version of `sha256sum` takes different arguments than the GNU version.
The `sha256_check` function in `contrib/install_db4.sh` has code specific to FreeBSD, however it doesn't get reached because while the `sha256sum` command does exist on FreeBSD, it is incompatible and results in an error:
```
sha256sum: option requires an argument -- c
usage: sha256sum [-pqrtx] [-c file] [-s string] [files ...]
```
This change moves the FreeBSD-specific code before the check for the `sha256sum` command.
Fixes: #26774
Top commit has no ACKs.
Tree-SHA512: 2485e2e7d8fdca3b072b29fb22bbdfd69e520740537b331b33c64cc645b63da712cfa63a23bdf039bbc92a6558fc7bf03323a51784bf601ff360ff0ef59506c8
|
|
fundrawtransaction's input_weights field
927b8d4e0cddd89e1f71093c10bd697c25b7a7d8 rpc: Correct RPCHelpMan for fundrawtransaction's input_weights field (jdjkelly@gmail.com)
Pull request description:
`input_weights` is incorrectly documented as a fixed length JSON array, but it is actually a JSON array of JSON objects - this commit changes `input_weights` to use `RPCArg::Type::OBJ`
The behavior of `input_weights` as an object exists as a functional test in [wallet_fundrawtransaction.py](https://github.com/bitcoin/bitcoin/blob/master/test/functional/wallet_fundrawtransaction.py).
ACKs for top commit:
achow101:
ACK 927b8d4e0cddd89e1f71093c10bd697c25b7a7d8
Tree-SHA512: 384f5e16be36dba670d64d96f16f1fde2d0d51357e1094ae13eb71d004af0f4dc8bac965b4d2d724ccf64fb671faad37b73055152a9882af24f65dfceaf1e5fb
|
|
fa818e103c0ddb515f29ae9ce8de44931e12e69e txmempool: Remove unused clear() member function (MarcoFalke)
Pull request description:
Seems odd to have code in Bitcoin Core that is unused.
Moreover the function was broken (see https://github.com/bitcoin/bitcoin/pull/24145) and is brittle, as there is nothing that prevents similar bugs from re-appearing.
Fix both issues by replacing it with C++11 member initializers.
ACKs for top commit:
glozow:
ACK fa818e103c0ddb515f29ae9ce8de44931e12e69e
Tree-SHA512: e79e44cac7d5a84d9ecc8e3f3b0b9a50e1e3ebec358b20ba5dac175ef07d1fbe338a20f83ee80f746f7c726c79e77f8be49e14bca57a41063da8a5302123c3a9
|
|
|
|
messages
76dc547ee7b05864e7b1b6c55fc0301d47aa3a15 gui: create tx, launch error dialog if backend throws runtime_error (furszy)
f4d79477ff0946b0bd340ade9251fa38e3b95dd7 wallet: coin selection, add duplicated inputs checks (furszy)
0aa065b14e67592d5be8f46ebbe5d59a083ff0a5 wallet: return accurate error messages from Coin Selection (furszy)
7e8340ab1a970a14e180b1fcf420b46a5657b062 wallet: make SelectCoins flow return util::Result (furszy)
e5e147fe97f706e82bc51358f8bdc355f355be57 wallet: refactor eight consecutive 'AttemptSelection' calls into a loop (furszy)
Pull request description:
Work decoupled from #25806, which cleanup and improves the Coin Selection flow further.
Adding the capability to propagate specific error messages from the Coin Selection process to the user.
Instead of always returning the general "Insufficient funds" message which is not always accurate to what happened internally.
Letting us instruct the user how to proceed under certain circumstances.
The following error messages were added:
1) If the selection result exceeds the maximum transaction weight,
we now will return:
-> "The inputs size exceeds the maximum weight. Please try sending
a smaller amount or manually consolidating your wallet's UTXOs".
2) If the user pre-selected inputs and disallowed the automatic coin
selection process (no other inputs are allowed), we now will
return:
-> "The preselected coins total amount does not cover the transaction
target. Please allow other inputs to be automatically selected or include
more coins manually".
3) The double-counted preset inputs during Coin Selection error will now
throw an "internal bug detected" message instead of crashing the node.
The essence of this work comes from several comments:
1. https://github.com/bitcoin/bitcoin/pull/26560#discussion_r1037395665
2. https://github.com/bitcoin/bitcoin/pull/25729#discussion_r940619491
3. https://github.com/bitcoin/bitcoin/pull/25269#pullrequestreview-1135240825
4. https://github.com/bitcoin/bitcoin/issues/23144 (which is connected to #24845)
ACKs for top commit:
ishaanam:
crACK 76dc547ee7b05864e7b1b6c55fc0301d47aa3a15
achow101:
ACK 76dc547ee7b05864e7b1b6c55fc0301d47aa3a15
aureleoules:
ACK 76dc547ee7b05864e7b1b6c55fc0301d47aa3a15
theStack:
ACK 76dc547ee7b05864e7b1b6c55fc0301d47aa3a15 :city_sunrise:
Tree-SHA512: 9de30792d7a5849cae77747aa978e70390b66ee9d082779a56088a024f82e725b0af050e6603aece0ac8229f6d73bc471ba97b4ab69dc7eddf419f5f56ae89a5
|
|
|
|
The ZMQ interface tests should use unique ports as we do for the p2p and
rpc ports so that multiple instances of the test can be run at the same
time.
|
|
ancestors
47c4b1f52ab8d95d7deef83050bad49d1e3e5990 mempool: log/halt when CalculateMemPoolAncestors fails unexpectedly (stickies-v)
5481f65849313ff947f38433b1ac28285a7f7694 mempool: add AssumeCalculateMemPoolAncestors helper function (stickies-v)
f911bdfff95eba3793fffaf71a31cc8bfc6f80c9 mempool: use util::Result for CalculateMemPoolAncestors (stickies-v)
66e028f7399b6511f9b73b1cef54b6a6ac38a024 mempool: use util::Result for CalculateAncestorsAndCheckLimits (stickies-v)
Pull request description:
Upon reviewing the documentation for `CTxMemPool::CalculateMemPoolAncestors`, I noticed `setAncestors` was meant to be an `out` parameter but actually is an `in,out` parameter, as can be observed by adding `assert(setAncestors.empty());` as the first line in the function and running `make check`. This PR fixes this unexpected behaviour and introduces refactoring improvements to make intents and effects of the code more clear.
## Unexpected behaviour
This behaviour occurs only in the package acceptance path, currently only triggered by `testmempoolaccept` and `submitpackage` RPCs.
In `MemPoolAccept::AcceptMultipleTransactions()`, we first call `PreChecks()` and then `SubmitPackage()` with the same `Workspace ws` reference. `PreChecks` leaves `ws.m_ancestors` in a potentially non-empty state, before it is passed on to `MemPoolAccept::SubmitPackage`. `SubmitPackage` is the only place where `setAncestors` isn't guaranteed to be empty before calling `CalculateMemPoolAncestors`. The most straightforward fix is to just forcefully clear `setAncestors` at the beginning of CalculateMemPoolAncestors, which is done in the first bugfix commit.
## Improvements
### Return value instead of out-parameters
This PR updates the function signatures for `CTxMemPool::CalculateMemPoolAncestors` and `CTxMemPool::CalculateAncestorsAndCheckLimits` to use a `util::Result` return type and eliminate both the `setAncestors` `in,out`-parameter as well as the error string. It simplifies the code and makes the intent and effects more explicit.
### Observability
There are 7 instances where we currently call `CalculateMemPoolAncestors` without actually checking if the function succeeded because we assume that it can't fail, such as in [miner.cpp](https://github.com/bitcoin/bitcoin/blob/69b10212ea5370606c7a5aa500a70c36b4cbb58f/src/node/miner.cpp#L399). This PR adds a new wrapper `AssumeCalculateMemPoolAncestors` function that logs such unexpected failures, or in case of debug builds even halts the program. It's not crucial to the objective, more of an observability improvement that seems sensible to add on here.
ACKs for top commit:
achow101:
ACK 47c4b1f52ab8d95d7deef83050bad49d1e3e5990
w0xlt:
ACK https://github.com/bitcoin/bitcoin/pull/26289/commits/47c4b1f52ab8d95d7deef83050bad49d1e3e5990
glozow:
ACK 47c4b1f52ab8d95d7deef83050bad49d1e3e5990
furszy:
light code review ACK 47c4b1f5
aureleoules:
ACK 47c4b1f52ab8d95d7deef83050bad49d1e3e5990
Tree-SHA512: d908dad00d1a5645eb865c4877cc0bae74b9cd3332a3641eb4a285431aef119f9fc78172d38b55c592168a73dae83242e6af3348815f7b37cbe2d448a3a58648
|
|
|
|
update dependencies
1e5e87cec32260a6dfd03f11040aed8b7bb4064e script: update python linter dependencies (Jon Atack)
459cb637aca80f744a8399e84bc78fab60de0b5c script, test: fix python linter E275 errors with flake8 5.0.4 (Jon Atack)
Pull request description:
It is helpful to be able to run the python linter locally to review PRs and check local diffs and work. Fix the errors raised by `./test/lint/lint-python.py` when run locally with flake8 5.0.4, which enforces rule E275 more strictly than previous versions, and update our python linter CI dependencies.
ACKs for top commit:
kristapsk:
ACK 1e5e87cec32260a6dfd03f11040aed8b7bb4064e
Tree-SHA512: c7da09396144b9fed4ce6405c0763b2e3e5651d49a5220b053da465aceca09f754fb70a8ab9647278ce2028dde803bea461a3cd93fb39868ead39d06187cd8af
|
|
|
|
|
|
04609284ad5e0b72651f2d4b43263461ada40816 rpc: Improve error when wallet is already loaded (Aurèle Oulès)
Pull request description:
Currently, trying to load a descriptor (sqlite) wallet that is already loaded throws the following error:
> error code: -4
> error message:
> Wallet file verification failed. SQLiteDatabase: Unable to obtain an exclusive lock on the database, is it being used by another instance of Bitcoin Core?
I don't think it is very clear what it means for a user.
While a legacy wallet would throw:
> error code: -35
> error message:
> Wallet file verification failed. Refusing to load database. Data file '/home/user/.bitcoin/signet/wallets/test_wallet/wallet.dat' is already loaded.
This PR changes the error message for both types of wallet to:
> error code: -35
> error message:
> Wallet file verification failed. Wallet "test_wallet" is already loaded.
ACKs for top commit:
achow101:
ACK 04609284ad5e0b72651f2d4b43263461ada40816
hernanmarino:
ACK 0460928
theStack:
Tested ACK 04609284ad5e0b72651f2d4b43263461ada40816
Tree-SHA512: a8f3d5133bfaef7417a6c05d160910ea08f32ac62bfdf7f5ec305ff5b62e9113b55f385abab4d5a4ad711aabcb1eb7ef746eb41f841b196e8fb5393ab3ccc01e
|
|
coverage
9622fe64b8785430c71d4abc8637075026dc690c test: move coins result test to wallet_tests.cpp (furszy)
f69347d0588647ff9a4e986c7be987827a0417f4 test: extend and simplify availablecoins_tests (furszy)
212ccdf2c2b70d973b18ae78f0158ec5f0c3bbb4 wallet: AvailableCoins, add arg to include/skip locked coins (furszy)
Pull request description:
Negative PR with extended test coverage :).
1) Cleaned duplicated code and added coverage for the 'AvailableCoins' incremental result.
2) The class `AvailableCoinsTestingSetup` inside `availablecoins_tests.cpp` is a plain copy
of `ListCoinsTestingSetup` that is inside `wallet_tests.cpp`.
So, deleted the file and moved the `BasicOutputTypesTest` test case to `wallet_tests.cpp`.
3) Added arg to include/skip locked coins from the `AvailableCoins` result. This is needed for point (1) as otherwise the wallet will spend the coins that we recently created due its closeness to the recipient amount.
Note: this last point comes from #25659 where I'm using the same functionality to clean/speedup another flow as well.
ACKs for top commit:
achow101:
ACK 9622fe64b8785430c71d4abc8637075026dc690c
theStack:
ACK 9622fe64b8785430c71d4abc8637075026dc690c
aureleoules:
reACK 9622fe64b8785430c71d4abc8637075026dc690c, nice cleanup!
Tree-SHA512: 1ed9133120bfe8815455d1ad317bb0ff96e11a0cc34ee8098716ab9b001749168fa649212b2fa14b330c1686cb1f29039ff1f88ae306db68881b0428c038f388
|
|
81d4a2b14ff65fe07085ef2a967a466015370ce3 refactor: Move feerate comparison invariant outside of the loop (yancy)
365aca40453995163bbd17231251512f9f9a103b refactor: Simplify feerate comparison statement (yancy)
Pull request description:
This is a small nit, however I think it's more understandable to write:
`utxo_pool.at(0).fee > utxo_pool.at(0).long_term_fee`
vs
`(utxo_pool.at(0).fee - utxo_pool.at(0).long_term_fee) > 0`
ACKs for top commit:
Xekyo:
ACK 81d4a2b14ff65fe07085ef2a967a466015370ce3
achow101:
ACK 81d4a2b14ff65fe07085ef2a967a466015370ce3
aureleoules:
ACK 81d4a2b14ff65fe07085ef2a967a466015370ce3
Tree-SHA512: 3e89377989c36716b53114fe40178261671dde5688075fab1c21ec173ac310f8c84ed6af90354d7c329176cb7262dfcaa7191fd19847d3b7147a9a10c3e31176
|
|
parameter and rename
f496528556a67107d3d75d9c2ae345f7f4565d77 walletdb: refactor: drop unused `FindWalletTx` parameter and rename (Sebastian Falbesoner)
Pull request description:
Since commit 3340dbadd38f5624642cf0e14dddbe6f83a3863b ("Remove -zapwallettxes"), the `FindWalletTx` helper is only needed to read tx hashes, so drop the other parameter and rename the method accordingly.
ACKs for top commit:
S3RK:
code review ACK f496528556a67107d3d75d9c2ae345f7f4565d77
achow101:
ACK f496528556a67107d3d75d9c2ae345f7f4565d77
vincenzopalazzo:
ACK https://github.com/bitcoin/bitcoin/pull/26702/commits/f496528556a67107d3d75d9c2ae345f7f4565d77
Tree-SHA512: ead85bc724462f9e920f9d7fe89679931361187579ffd6e63427c8bf5305cd5f71da24ed84f3b1bd22a12be46b5abec13f11822e71a3e1a63bf6cf49de950ab5
|
|
input_weights is incorrectly documented as a fixed length JSON array,
but it is actually a JSON array of JSON objects - this commit changes
input_weights to use RPCArg::Type::OBJ
|
|
|
|
fabb6af8508a9c0d05d2e9f0006b0b1d3f38119a ci: Remove duplicate CC and CXX from tsan task (MarcoFalke)
fa5d9a0e246e94eb0d0e9a75f63ff1b09ec93c03 Revert "ci: Use clang-15 in tsan task" (MarcoFalke)
faa835e7e564536fb0589ec94d3f85042a1f9cd8 Revert "test: Drop no longer needed `race:epoll_ctl` TSan suppression" (MarcoFalke)
Pull request description:
Looks like there are still bugs in clang-15, so we need to roll back all the way to the previously used version (clang-13).
ACKs for top commit:
hebasto:
ACK fabb6af8508a9c0d05d2e9f0006b0b1d3f38119a, I have reviewed the code and it looks OK, I agree it can be merged.
Tree-SHA512: d62203049847ab9095ee3fc89e18bdd721d1d9d5a7ef7a9f524c80e6be58d1d9f6aa2f14533df1ea77eb59597fba6fa9b987b17eb03b2c3f7cb577ab59cd59c0
|