Age | Commit message (Collapse) | Author |
|
3f05a1068d10ffe0f2859cd20c5fc9bc8efa1c70 remove repeated word in note (sunerok)
Pull request description:
Fix typo.
ACKs for top commit:
maflcko:
ACK 3f05a1068d10ffe0f2859cd20c5fc9bc8efa1c70
danielabrozzoni:
ACK 3f05a1068d10ffe0f2859cd20c5fc9bc8efa1c70
Tree-SHA512: 709d96ed18608c0ea788b4f0696abad79ab1b81c4f266487d16bbe4cfca5b99b8f7f9a58f830866db9695aa3aebcc6442098b1533d85507729af99709a53d26a
|
|
flags usage
60055f1abc4b4ad5f66a2fcf2e61c65efc777036 test: replace deprecated secp256k1 context flags usage (Sebastian Falbesoner)
Pull request description:
The flags `SECP256K1_CONTEXT_{SIGN,VERIFY}` have been marked as deprecated since libsecp256k1 version 0.2 (released in December 2022), with the recommendation to use SECP256K1_CONTEXT_NONE instead, see https://github.com/bitcoin-core/secp256k1/pull/1126 and https://github.com/bitcoin-core/secp256k1/blob/1988855079fa8161521b86515e77965120fdc734/CHANGELOG.md?plain=1#L132. Note that in contrast to other deprecated functions/variables, these defines don't have a deprecated attribute and hence don't lead to a compiler warning (see https://github.com/bitcoin-core/secp256k1/pull/1126#discussion_r922105271), so they are not easily detected.
ACKs for top commit:
TheCharlatan:
ACK 60055f1abc4b4ad5f66a2fcf2e61c65efc777036
ismaelsadeeq:
utACK 60055f1abc4b4ad5f66a2fcf2e61c65efc777036
tdb3:
light CR and test ACK 60055f1abc4b4ad5f66a2fcf2e61c65efc777036
Tree-SHA512: d93cf49e018a58469620c0d2f50242141f22dabc70afb2a7cd64e416f4f55588714510ae5a877376dd1e6b6f7494261969489af4b18a1c9dff0d0dfdf93f1fa8
|
|
221809b81cfcecb04050915eebacffda2599da42 headerssync: Update headerssync configuration (Ava Chow)
c2707446f745015d279af663e181219757ad6eb7 params: Update assumevalid and minimum chainwork (Ava Chow)
255d4514d3cd9f545f1d3eca5bbda8d8c90ee351 params: Update chainTxData (Ava Chow)
6a5bdae3225117651708aa430e04b6da58387cf2 params: Update assumed blockchain and chainstate sizes (Ava Chow)
Pull request description:
Update chainparams and headerssync parameters for the pre-28.x branching, per https://github.com/bitcoin/bitcoin/blob/master/doc/release-process.md#before-branch-off
ACKs for top commit:
fjahr:
re-ACK 221809b81cfcecb04050915eebacffda2599da42
Sjors:
re-ACK 221809b81cfcecb04050915eebacffda2599da42
glozow:
ACK 221809b81cfcecb04050915eebacffda2599da42
marcofleon:
ACK 221809b81cfcecb04050915eebacffda2599da42
Tree-SHA512: 5106d59f46dbe167fffa339519e52975ae5bfd7e52202d76ec058da0d4e8bf87355e90678f7ace7c8c402a2f7264050a0355680b9f727c7962ff60e8fcdb3a90
|
|
activate on regtest, lower nPowTargetTimespan to 144 and add test
59ff17e5af4e382cbe16f183767beef1bdcd9131 miner: adjust clock to timewarp rule (Sjors Provoost)
e929054e12210353812f440c685a23329e7040f7 Add timewarp attack mitigation test (Sjors Provoost)
e85f386c4b157b7d1ac16aface9bd2c614e62b46 consensus: enable BIP94 on regtest (Sjors Provoost)
dd154b05689c60fad45df0df6d31cec12e09ab21 consensus: lower regtest nPowTargetTimespan to 144 (Sjors Provoost)
Pull request description:
Because #30647 reduced the timewarp attack threshold from 7200s to 600s, our miner code will fail to propose a block template (on testnet4) if the last block of the previous period has a timestamp two hours in the future. This PR fixes that and also adds a test.
The non-test changes in the last commit should be in v28, otherwise miners have to patch it themselves. If necessary I can split that out into a separate PR, but I prefer to get the tests in as well.
In order to add the test, we activate BIP94 on regtest.
In order for the test to run faster, we reduce its difficulty retarget period to 144, the same number that's already used for softfork activation logic. Regtest does not actually adjust its difficulty, so this change has no effect (except for `getnetworkhashps`, see commit).
An alternative approach would be to run this test on testnet4, by hardcoding its first 2015 in the test suite. But since the timewarp mitigation is a serious candidate for a future mainnet softfork, it seems better to just deploy it on regtest.
The next commits add a test and fix the miner code.
The `MAX_TIMEWARP` constant is moved to `consensus.h` so both validation and miner code have access to it.
ACKs for top commit:
achow101:
ACK 59ff17e5af4e382cbe16f183767beef1bdcd9131
fjahr:
ACK 59ff17e5af4e382cbe16f183767beef1bdcd9131
glozow:
ACK 59ff17e5af4e382cbe16f183767beef1bdcd9131
Tree-SHA512: 50af9fdcba9b0d5c57e1efd5feffd870bd11b5318f1f8b0aabf684657f2d33ab108d5f00b1475fe0d38e8e0badc97249ef8dda20c7f47fcc1698bc1008798830
|
|
|
|
fa899fb7aa8a14acecadd8936ad5824fa0f697ff fuzz: Speed up utxo_snapshot fuzz target (MarcoFalke)
fa386642b4dfd88f74488c288c7886494d69f4ed fuzz: Speed up utxo_snapshot by lazy re-init (MarcoFalke)
fa645c7a861ffa83a53a459263b6a620defe31f9 fuzz: Remove unused DataStream object (MarcoFalke)
fae8c73d9e4eba4603447bb52b6e3e760fbf15f8 test: Disallow fee_estimator construction in ChainTestingSetup (MarcoFalke)
Pull request description:
Two commits to speed up unit and fuzz tests.
Can be tested by running the fuzz target and looking at the time it took, or by looking at the flamegraph. For example:
```
FUZZ=utxo_snapshot perf record -g --call-graph dwarf ./src/test/fuzz/fuzz -runs=100
hotspot ./perf.data
ACKs for top commit:
TheCharlatan:
Re-ACK fa899fb7aa8a14acecadd8936ad5824fa0f697ff
marcofleon:
Re ACK fa899fb7aa8a14acecadd8936ad5824fa0f697ff
brunoerg:
ACK fa899fb7aa8a14acecadd8936ad5824fa0f697ff
Tree-SHA512: d3a771bb12d7ef491eee61ca47325dd1cea5c20b6ad42554babf13ec98d03bef8e7786159d077e59cc7ab8112495037b0f6e55edae65b871c7cf1708687cf717
|
|
The flags SECP256K1_CONTEXT_{SIGN,VERIFY} have been deprecated since
libsecp256k1 version 0.2 (released in December 2022), with the
recommendation to use SECP256K1_CONTEXT_NONE instead.
|
|
|
|
|
|
This currently has no effect due to fPowNoRetargeting,
except for the getnetworkhashps when called with -1.
It will when the next commit enforces the timewarp attack mitigation on regtest.
|
|
This speeds up the fuzz target, which allows "valid" inputs. It does not
affect the "INVALID" fuzz target.
|
|
600s from 7200s
16e95bda86302af20cfb314a2c0252256d01f750 Move maximum timewarp attack threshold back to 600s from 7200s (Matt Corallo)
Pull request description:
In 6bfa26048dbafb91e9ca63ea8d3960271e798098 the testnet4 timewarp attack fix block time variation was increased from the Great Consensus Cleanup value of 600s to 7200s on the thesis that this allows miners to always create blocks with the current time. Sadly, doing so does allow for some nonzero inflation, even if not a huge amount.
While it could be that some hardware ignores the timestamp provided to it over Stratum and forces the block header timestamp to the current time, I'm not aware of any such hardware, and it would also likely suffer from random invalid blocks due to relying on NTP anyway, making its existence highly unlikely.
This leaves the only concern being pools, but most of those rely on work generated by Bitcoin Core (in one way or another, though when spy mining possibly not), and it seems likely that they will also not suffer any lost work. While its possible that a pool does generate invalid work due to spy mining or otherwise custom logic, it seems unlikely that a substantial portion of hashrate would do so, making the difference somewhat academic (any pool that screws this up will only do so once and the network would come out just fine).
Further, while we may end up deciding these assumptions were invalid and we should instead use 7200s, it seems prudent to try with the value we "want" on testnet4, giving us the ability to learn if the compatibility concerns are an issue before we go to mainnet.
ACKs for top commit:
fjahr:
tACK 16e95bda86302af20cfb314a2c0252256d01f750
achow101:
ACK 16e95bda86302af20cfb314a2c0252256d01f750
murchandamus:
crACK 16e95bda86302af20cfb314a2c0252256d01f750
Tree-SHA512: ae46d03b728b6e23cb6ace64c9813bc01c01e38dd7f159cf0fab53b331ef84b3b811edab225453ccdfedb53b242f55b0efd69829782657490fe393d24dacbeb2
|
|
6ed424f2db609f9f39ec1d1da2077c7616f3a0c2 wallet: fix, detect blank legacy wallets in IsLegacy (furszy)
Pull request description:
Blank legacy wallets do not have active SPKM. They can only be
detected by checking the descriptors' flag or the db format.
This enables the migration of blank legacy wallets in the GUI.
To test this:
1) Create a blank legacy wallet.
2) Try to migrate it using the GUI's toolbar "Migrate Wallet" button.
-> In master: The button will be disabled because `CWallet::IsLegacy()` returns false for blank legacy wallet.
-> In this PR: the button will be enabled, allowing the migration of legacy wallets.
ACKs for top commit:
achow101:
ACK 6ed424f2db609f9f39ec1d1da2077c7616f3a0c2
tdb3:
ACK 6ed424f2db609f9f39ec1d1da2077c7616f3a0c2
glozow:
ACK 6ed424f2db609f9f39ec1d1da2077c7616f3a0c2
Tree-SHA512: c06c4c4c2e546ccb033287b9aa3aee4ca36b47aeb2fac6fbed5de774b65caef9c818fc8dfdaac6ce78839b2d5d642a5632a5b44c5e889ea169ced80ed50501a7
|
|
|
|
|
|
|
|
|
|
The re-init is expensive, so skip it if there is no need.
Also, add an even faster fuzz target utxo_snapshot_invalid, which does
not need any re-init at all.
|
|
f550a8e035b4603787273ea250f403f6f0be453f Rename ReleaseWallet to FlushAndDeleteWallet (furszy)
64e736d79efc7201768244fc297084f70c0bebc1 wallet: WaitForDeleteWallet, do not expect thread safety (Ryan Ofsky)
8872b4a6ca91a83bf8d5a118fb808c043b9e879d wallet: rename UnloadWallet to WaitForDeleteWallet (furszy)
5d15485aafefdc759ba97e039bb1b9ccac267358 wallet: unload, notify GUI as soon as possible (furszy)
Pull request description:
Coming from #29073.
Applied ryanofsky suggested changes on https://github.com/bitcoin/bitcoin/issues/29073#issuecomment-2274237242 with few modifications coming from https://github.com/bitcoin/bitcoin/pull/18338#issuecomment-605060348.
The only point I did not tackle from https://github.com/bitcoin/bitcoin/pull/18338#issuecomment-605060348 is:
> * Move log print and flush out of ReleaseWallet into CWallet destructor
Because it would mean every `CWallet` object would flush data to disk during destruction. Which is not necessary for wallet tool utilities and unit tests.
ACKs for top commit:
achow101:
ACK f550a8e035b4603787273ea250f403f6f0be453f
ryanofsky:
Code review ACK f550a8e035b4603787273ea250f403f6f0be453f. Just a simple rename since last review
ismaelsadeeq:
Re-ACK f550a8e035b4603787273ea250f403f6f0be453f
Tree-SHA512: e2eb69bf36883c514f601f4838ae6a41113996b9559abf8dc2b46e16bbcdad401195ac0f2b9d1fb55a10e78bb8ea9953788a168c80474e3f101350d208cb3bd2
|
|
To better describe the function's behavior.
And add wallet name to logprint.
|
|
|
|
Multiple threads could try to delete the wallet at the same time.
|
|
And update function's documentation.
|
|
Releases wallet shared pointers prior to doing the
final settings update and prevent GUI races trying
to access a wallet that is no longer loaded.
|
|
1610643c8b37a9f674b236cfa79abf8f8aaf1410 chainparams: add mainnet assumeutxo param at height 840_000 (Sjors Provoost)
Pull request description:
This adds snapshot parameters for mainnet block 840,000.
You can generate the snapshot yourself using `./contrib/devtools/utxo_snapshot.sh` or download my torrent:
* torrent: `magnet:?xt=urn:btih:596c26cc709e213fdfec997183ff67067241440c&dn=utxo-840000.dat&tr=udp%3A%2F%2Ftracker.bitcoin.sprovoost.nl%3A6969`
It would be a good idea to test:
1. That you can produce the same snapshot file, sha256 sum:
```
dc4bb43d58d6a25e91eae93eb052d72e3318bd98ec62a5d0c11817cefbba177b utxo-840000.dat
```
2. That the snapshot works
ACKs for top commit:
fjahr:
re-ACK 1610643c8b37a9f674b236cfa79abf8f8aaf1410
achow101:
ACK 1610643c8b37a9f674b236cfa79abf8f8aaf1410
theStack:
Tested ACK 1610643c8b37a9f674b236cfa79abf8f8aaf1410
mzumsande:
tested ACK 1610643c8b37a9f674b236cfa79abf8f8aaf1410
willcl-ark:
tACK 1610643c8b37a9f674b236cfa79abf8f8aaf1410
Tree-SHA512: 581d8e86379bb044324f04f8559dd0a8946b6e2b145d5f25b38727b30b8cf13d6ac3c8777ff06554d3cf1a072809f7b5fbd693239868578f25dceafe5ba5f57c
|
|
in RPC help texts
9b297555207b4ea54bc0051f09c7084797aa9def Deduplicate list of chain strings in RPC help texts (Martin Saposnic)
Pull request description:
As mentioned in issue https://github.com/bitcoin/bitcoin/issues/30645:
Many command line parameter and RPC help texts currently contain the list of chain/network names hardcoded ("main, test, testnet4, signet, regtest"), which is error-prone as it can easily happen to miss an instance if the list ever changes again.
This PR deduplicates the list of possible chain/network strings in RPC/parameter help texts, and it creates a macro `LIST_CHAIN_NAMES` in src/chainparamsbase.h. In the future, there is only 1 place where that list of possible values lives, so maintainability is improved and errors are avoided.
All three places where this change impacts:
```
./bitcoin-cli --help
./bitcoin-cli help getblockchaininfo
./bitcoin-cli help getmininginfo
```
They all return the correct string `"main, test, testnet4, signet, regtest"`
See https://github.com/bitcoin/bitcoin/pull/30642#discussion_r1714711575
ACKs for top commit:
maflcko:
lgtm ACK 9b297555207b4ea54bc0051f09c7084797aa9def
achow101:
ACK 9b297555207b4ea54bc0051f09c7084797aa9def
MarnixCroes:
ACK 9b297555207b4ea54bc0051f09c7084797aa9def
theStack:
ACK 9b297555207b4ea54bc0051f09c7084797aa9def
danielabrozzoni:
ACK 9b297555207b4ea54bc0051f09c7084797aa9def
Tree-SHA512: 1e961bcbe40b0f17a87a2437eb4ba1bb89468fd1b5a39599d72a00ef75cb4009e7d2f05d0a621bb904fecf681c55b8a219fcfe4d44d5d27f27cdda20882b1323
|
|
The diff is produced by running `make -C src translate`.
|
|
8f2522d242961ceb9e79672aa43e856863a1a6dd gui: Use menu for wallet migration (Ava Chow)
d56a450bf5172e2c3f4b9a2786e71268019e1277 gui: Use wallet name for wallet migration rather than WalletModel (Ava Chow)
c3918583dd5fcd9001136da2192e02e092128901 gui: don't remove wallet manually before migration (furszy)
bfba63880fbb1108b73540faeb0620ba24b8cdd0 gui: Consolidate wallet display name to GUIUtil function (Ava Chow)
28fc562f2692af4f37f918d4ae31c4d115e03aee wallet, interfaces: Include database format in listWalletDir (Ava Chow)
Pull request description:
Currently the Migrate Wallet menu item can only be used to migrate the currently loaded wallet. This is not suitable for the future when legacy wallets can no longer be loaded at all, but should still be able to be migrated. This PR changes that menu item into a menu list like Open Wallet and lets users migrate any legacy wallet in their wallet directory regardless of the wallets loaded.
One issue I ran into was dealing with encrypted wallets. Ideally, we would detect whether a wallet is encrypted, and prompt the user for their passphrase at that time. However, that's actually difficult to do in the GUI since migration will unload the wallet if it was already loaded, and reload it without connecting it to any signals or interfaces. Only then can it detect whether a wallet is encrypted, but then there is no `WalletModel` or even an `interfaces::Wallet` that the GUI could use to unlock it via a callback.
To deal with this, I've opted to just add a button to the migration dialog box that has the user enter their passphrase first, along with instructional text to use that button if their wallet was encrypted. If the user enters the wrong passphrase or clicked the other button that does not prompt for the passphrase, migration will fail with a message indicating that the passphrase was incorrect.
ACKs for top commit:
hebasto:
ACK 8f2522d242961ceb9e79672aa43e856863a1a6dd.
furszy:
ACK 8f2522d
Tree-SHA512: a0e3b70dbfcacb89617956510ebcea94cad8617a987c68fe39fa16ac1721190b7cf7afc156c39b9032920cfb67b5d4ca28791681f5021d92d16acc691387afa1
|
|
Once legacy wallets can no longer be loaded, we need to be able to
migrate them without loading. Thus we should use a menu that lists the
wallets in the wallet directory instead of an action which migrates the
currently loaded wallet.
|
|
|
|
In 6bfa26048dbafb91e9ca63ea8d3960271e798098 the testnet4 timewarp
attack fix block time variation was increased from the Great
Consensus Cleanup value of 600s to 7200s on the thesis that this
allows miners to always create blocks with the current time. Sadly,
doing so does allow for some nonzero inflation, even if not a huge
amount.
While it could be that some hardware ignores the timestamp provided
to it over Stratum and forces the block header timestamp to the
current time, I'm not aware of any such hardware, and it would also
likely suffer from random invalid blocks due to relying on NTP
anyway, making its existence highly unlikely.
This leaves the only concern being pools, but most of those rely on
work generated by Bitcoin Core (in one way or another, though when
spy mining possibly not), and it seems likely that they will also
not suffer any lost work. While its possible that a pool does
generate invalid work due to spy mining or otherwise custom logic,
it seems unlikely that a substantial portion of hashrate would do
so, making the difference somewhat academic (any pool that screws
this up will only do so once and the network would come out just
fine).
Further, while we may end up deciding these assumptions were
invalid and we should instead use 7200s, it seems prudent to try
with the value we "want" on testnet4, giving us the ability to
learn if the compatibility concerns are an issue before we go to
mainnet.
|
|
|
|
To prepare for migrating wallets that are not loaded, when migration
occurs in the GUI, it should not rely on a WalletModel existing.
Co-authored-by: furszy <matiasfurszyfer@protonmail.com>
|
|
|
|
Instead of having the code for the wallet display name being copy and
pasted, use a GUIUtil function to get that for us.
|
|
|
|
RPC/init help texts
701530045553f2b9671a3fffea301bf4dc954514 doc: add missing "testnet4" network string in RPC/init help texts (Sebastian Falbesoner)
Pull request description:
The following bitcoind parameters / RPC calls still missed the "testnet4" network string:
- `-chain=` parameter
- `getblockchaininfo` RPC, `"chain"` result
- `getmininginfo` RPC, `"chain"` result
The occurences were found via `$ git grep \".*main.*test.*\"`.
ACKs for top commit:
maflcko:
review ACK 701530045553f2b9671a3fffea301bf4dc954514
glozow:
ACK 701530045553f2b9671a3fffea301bf4dc954514
tdb3:
ACK 701530045553f2b9671a3fffea301bf4dc954514
BrandonOdiwuor:
ACK 701530045553f2b9671a3fffea301bf4dc954514
danielabrozzoni:
ACK 701530045553f2b9671a3fffea301bf4dc954514
Tree-SHA512: 99bf5c2b4cf28651feaff2fc7d4669961012dfa8379d8522251540ae1b8fc77d1761b75395903b527580530f42a3c1fd2dd2d8dba4ffbc9b6e55cb357c3a271b
|
|
fa6fe432075df5e0eceb1ccd85038159cc820ccc net: Clarify that m_addr_local is only set once (MarcoFalke)
Pull request description:
The function is supposed to be only called once when the version msg arrives (a single time). Calling it twice would be an internal logic bug. However, the `LogError` in this function has many issues:
* If the error happens in tests, as is the case for the buggy fuzz test, it will go unnoticed
* It is dead code, unless a bug is introduced to execute it
Fix all issues by using `Assume(!m_addr_local.IsValid())` instead. Idea taken from https://github.com/bitcoin/bitcoin/pull/30364#discussion_r1680530382
ACKs for top commit:
achow101:
ACK fa6fe432075df5e0eceb1ccd85038159cc820ccc
mzumsande:
utACK fa6fe432075df5e0eceb1ccd85038159cc820ccc
glozow:
ACK fa6fe432075df5e0eceb1ccd85038159cc820ccc
Tree-SHA512: 8c1e8c524768f4f36cc50110ae54ee423e057a963ff78f736f3bf92df1ce5af28e3e0149153780897944e1d5c22ddbca9dac9865d9f4d44afffa152bc8559405
|
|
It is expensive to construct, and only one test uses it.
Fix both issues by disallowing the construction and moving it to the
single test that uses it.
|
|
Blank legacy wallets do not have active SPKM. They can
only be detected by checking the descriptors' flag or
the db format.
This enables the migration of blank legacy wallets in
the GUI.
|
|
The following bitcoind parameters / RPC calls missed the "testnet4"
network string:
- `-chain=` parameter
- `getblockchaininfo` RPC, "chain" result
- `getmininginfo` RPC, "chain" result
|
|
CCoinsViewCache::FetchCoin
204ca67bba263018374fe86d7a6867362d09536f Reduce cache lookups in CCoinsViewCache::FetchCoin (Lőrinc)
Pull request description:
Enhanced efficiency and readability of `CCoinsViewCache::FetchCoin` by replacing separate `find()` and `emplace()` calls with a single `try_emplace()`, reducing map lookups and potential insertions.
`AssembleBlock` shows `FetchCoin` as one of its bottlenecks:
<img src="https://github.com/bitcoin/bitcoin/assets/1841944/79c7f480-aac2-46da-9ac9-526a02a8eafa">
These changes result in a modest performance improvement:
> ./src/bench/bench_bitcoin --filter='AssembleBlock' --min-time=10000
before:
| ns/op | op/s | err% | total | benchmark
|--------------------:|--------------------:|--------:|----------:|:----------
| 156,160.70 | 6,403.66 | 0.6% | 10.91 | `AssembleBlock`
after:
| ns/op | op/s | err% | total | benchmark
|--------------------:|--------------------:|--------:|----------:|:----------
| 152,971.97 | 6,537.15 | 0.2% | 10.95 | `AssembleBlock`
Further benchmarks: https://github.com/bitcoin/bitcoin/pull/30326#issuecomment-2188378721
ACKs for top commit:
sipa:
utACK 204ca67bba263018374fe86d7a6867362d09536f
achow101:
ACK 204ca67bba263018374fe86d7a6867362d09536f
andrewtoth:
re-ACK 204ca67bba263018374fe86d7a6867362d09536f
Tree-SHA512: 65743a5d4edd442672a59d7b3de38fe197c61270a5c8df65712413904559f360fc58b512234558c7e5169ffb4dda3b2d2f7ded92ad5b04ca999828986b251066
|
|
86b38529d5014612c3e7bb59fdc4dad3bff2aa64 qa: a fuzz target for the block index database (Antoine Poinsot)
Pull request description:
This introduces a small fuzz target for `CBlockTreeDB` which asserts a few invariants by using an in-memory LevelDb.
ACKs for top commit:
achow101:
ACK 86b38529d5014612c3e7bb59fdc4dad3bff2aa64
TheCharlatan:
Re-ACK 86b38529d5014612c3e7bb59fdc4dad3bff2aa64
maflcko:
review ACK 86b38529d5014612c3e7bb59fdc4dad3bff2aa64 🥒
brunoerg:
utACK 86b38529d5014612c3e7bb59fdc4dad3bff2aa64
Tree-SHA512: ab75b4ae1c7e0a4b15f8a6ceffdf509fbc79833e6ea073ecef68558d53b83663d1b30362aaa2d77c22b8890a572f5b1d4b1c5abbca483c8c8f9b1fb5b276a59a
|
|
401cc4ec70d67ba2aa0e078d2fab214e1c40742c fuzz: improve scriptpubkeyman target (brunoerg)
Pull request description:
Fixes #30541
This PR aims to improve `scriptpubkeyman` target to avoid timeouts. The input provided in #30541 takes too much time to run because it basically calls only `MarkUnusedAddresses` (300 times * number of spks). The following changes were made to improve it:
- Reduce keypool size.
- When calling `MarkUnusedAddresses`, do it with one of the spks per iteration.
- Remove the specific `AddDescriptorKey` call since it is already covered with `AddWalletDescriptor`.
- Limit number of iterations to a reasonable value.
ACKs for top commit:
maflcko:
lgtm ACK 401cc4ec70d67ba2aa0e078d2fab214e1c40742c
achow101:
ACK 401cc4ec70d67ba2aa0e078d2fab214e1c40742c
Tree-SHA512: 941812bc6d991dd03675a2974ce1b839494ca7f6e6d8a22c689d4bf4fed2dac5491246998f19cb15dbff516fdd8eeda27e7628c3206d45f57dc292bc05624a5c
|
|
15aa7d023688700a47997b92108de95f2d864f5a gui, qt: brintToFront workaround for Wayland (pablomartin4btc)
Pull request description:
There are known issues around handling windows focus in `Wayland` ([this one specific](https://bugs.kde.org/show_bug.cgi?id=462574) in KDE but also in [gnome](https://gitlab.gnome.org/GNOME/gnome-shell/-/issues/730)).
The idea is that the workaround will be executed if `bitcoin-qt` is running using `Wayland` platform (e.g.: `QT_QPA_PLATFORM=wayland ./src/qt/bitcoin-qt -regtest`), since the workaround behaviour looks like re-opening the window again (which I tried to fix by moving the window to the original position and/ or re-setting the original geometry without success) while in `X11` (not sure in Mac) the current `GUIUtil::brintToFront` actually sets the focus to the desired window, keeping its original position as expected, and I didn't want to change that (`X11` behaviour).
The solution was [initially discussed](https://github.com/bitcoin-core/gui/pull/817#issuecomment-2256158902) with hebasto in #817.
ACKs for top commit:
hebasto:
ACK 15aa7d023688700a47997b92108de95f2d864f5a.
Tree-SHA512: 141d6cc4a618026e551627b9f4cc284285980db02a54a7b19c7de91e8c5adccf0c1d67380625146b5413e58c59f39c9e944ed5ba68cb8644f67647518918b6f7
|
|
default wallets and generated backup files
6b2dcba07670f04f32c0dc3a2c86fd805c85f12d wallet: List sqlite wallets with empty string name (Ava Chow)
3ddbdd1815c676a88345b3b0e55a551d2a569e28 wallet: Ignore .bak files when listing wallet files (Ava Chow)
Pull request description:
When the default wallet is migrated, we do not rename the wallet so we end up having a descriptor wallet with the empty string as its name and the wallet.dat file in the root of the walletdir. This is supposed to be an unsupported configuration and there is no other way to achieve this (other than file copying), but the wallet loading code does not disallow loading such wallets. However `listwalletdir` does not currently list the default wallet if it is sqlite. This is confusing to users, so change `listwalletdir` to include these wallets.
Additionally, the migration of the default wallet, and of any plain wallet files in the walletdir, produces a backup file in the walletdir itself. Since these backups are a BDB file, `listwalletdir` will detect them as being another wallet that we could open, but this is erroneous and could lead to confusion and potentially funds loss if both the backup and the migrated wallet are in use simultaneously. To reduce the likelihood of this issue, don't list these wallets in `listwalletdir`.
***
Possibly we could have more stringent checks on loading to resolve these issues, but I'm concerned that that will just confuse users and gratuitously break things that already worked.
Since the original intent was to disallow default wallets for sqlite/descriptors, a possible alternative would be to prevent people from loading such wallets and change migration to rename those wallets. However, given that this behavior with migrating default wallets has existed since default wallet migration was fixed, I think that making such a change would be confusing and break things for no good reason. Although perhaps we should still do the renaming.
For the backups, we could also change loading to refuse to load any wallet named with `.bak` (or `.legacy.bak`) as such wallets can still be loaded by giving the path to them directly, which some users may do to "restore" the backup. However restricting what can be loaded based on filename seems a little heavyhanded. It wouldn't be funds loss though since the correct way to restore the backup is with `restorewallet`.
ACKs for top commit:
fjahr:
Code review ACK 6b2dcba07670f04f32c0dc3a2c86fd805c85f12d
furszy:
Code ACK 6b2dcba076
glozow:
ACK 6b2dcba07670f04f32c0dc3a2c86fd805c85f12d
Tree-SHA512: 0b033f6ed55830f8a054afea3fb2cf1fa82a94040053ebfaf123bda36c99f45d3f01a2aec4ed02fed9c61bb3d320b047ed892d7f6644b5a356a7bc5974b10cff
|
|
055bc05792ff5d5b084563044818ebec12bfd748 policy/feerate.h: avoid constraint self-dependency (Matt Whitlock)
138f8671569f7ebb8c84e9d80c44cddeda9e3845 add missing #include <cstdint> for GCC 15 (Matt Whitlock)
Pull request description:
#30612 with changes made.
GCC 15 introduces three build failures:
* Two are related to missing includes. You can't use `uint16_t` et al. without including `<cstdint>`.
* The third is harder to understand but easy to fix. GCC changed something about the way templates are instantiated when checking type constraints, and now there is a dependency loop while checking `std::optional<CFeeRate>`. This manifests as the following compile-time mess:
```
In file included from /usr/lib/gcc/x86_64-pc-linux-gnu/15/include/g++-v15/format:48,
from /usr/lib/gcc/x86_64-pc-linux-gnu/15/include/g++-v15/bits/chrono_io.h:39,
from /usr/lib/gcc/x86_64-pc-linux-gnu/15/include/g++-v15/chrono:3362,
from ./util/time.h:9,
from ./primitives/block.h:12,
from ./blockencodings.h:8,
from blockencodings.cpp:5:
/usr/lib/gcc/x86_64-pc-linux-gnu/15/include/g++-v15/type_traits: In substitution of 'template<class _Up> requires !(is_same_v<std::optional<_Tp>, typename std::remove_cvref<_It2>::type>) && (is_constructible_v<_Tp, const _Up&>) && (__construct_from_contained_value<_Up, typename std::remove_cv< <template-parameter-1-1> >::type>) constexpr std::optional<CFeeRate>::optional(const std::optional<_Tp>&) [with _Up = CFeeRate]':
/usr/lib/gcc/x86_64-pc-linux-gnu/15/include/g++-v15/type_traits:1140:25: required by substitution of 'template<class _Tp, class ... _Args> using std::__is_constructible_impl = std::__bool_constant<__is_constructible(_Tp, _Args ...)> [with _Tp = CFeeRate; _Args = {std::optional<CFeeRate>&}]'
1140 | = __bool_constant<__is_constructible(_Tp, _Args...)>;
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/usr/lib/gcc/x86_64-pc-linux-gnu/15/include/g++-v15/type_traits:1145:12: required from 'struct std::is_constructible<CFeeRate, std::optional<CFeeRate>&>'
1145 | struct is_constructible
| ^~~~~~~~~~~~~~~~
/usr/lib/gcc/x86_64-pc-linux-gnu/15/include/g++-v15/type_traits:178:35: required by substitution of 'template<class ... _Bn> std::__detail::__first_t<std::integral_constant<bool, false>, typename std::enable_if<(!(bool)(_Bn::value)), void>::type ...> std::__detail::__or_fn(int) [with _Bn = {std::is_constructible<CFeeRate, std::optional<CFeeRate>&>, std::is_convertible<std::optional<CFeeRate>&, CFeeRate>, std::is_constructible<CFeeRate, std::optional<CFeeRate> >, std::is_convertible<std::optional<CFeeRate>, CFeeRate>, std::is_constructible<CFeeRate, const std::optional<CFeeRate>&>, std::is_convertible<const std::optional<CFeeRate>&, CFeeRate>, std::is_constructible<CFeeRate, const std::optional<CFeeRate> >, std::is_convertible<const std::optional<CFeeRate>, CFeeRate>}]'
178 | __enable_if_t<!bool(_Bn::value)>...>;
| ^~~~~
/usr/lib/gcc/x86_64-pc-linux-gnu/15/include/g++-v15/type_traits:196:41: required from 'struct std::__or_<std::is_constructible<CFeeRate, std::optional<CFeeRate>&>, std::is_convertible<std::optional<CFeeRate>&, CFeeRate>, std::is_constructible<CFeeRate, std::optional<CFeeRate> >, std::is_convertible<std::optional<CFeeRate>, CFeeRate>, std::is_constructible<CFeeRate, const std::optional<CFeeRate>&>, std::is_convertible<const std::optional<CFeeRate>&, CFeeRate>, std::is_constructible<CFeeRate, const std::optional<CFeeRate> >, std::is_convertible<const std::optional<CFeeRate>, CFeeRate> >'
196 | : decltype(__detail::__or_fn<_Bn...>(0))
| ~~~~~~~~~~~~~~~~~~~~~~~~~^~~
/usr/lib/gcc/x86_64-pc-linux-gnu/15/include/g++-v15/optional:824:45: required from 'constexpr const bool std::optional<CFeeRate>::__construct_from_contained_value<CFeeRate, CFeeRate>'
824 | = !__converts_from_optional<_Tp, _From>::value;
| ^~~~~
/usr/lib/gcc/x86_64-pc-linux-gnu/15/include/g++-v15/optional:884:7: required by substitution of 'template<class _Up> requires !(is_same_v<std::optional<_Tp>, typename std::remove_cvref<_It2>::type>) && (is_constructible_v<_Tp, const _Up&>) && (__construct_from_contained_value<_Up, typename std::remove_cv< <template-parameter-1-1> >::type>) constexpr std::optional<CFeeRate>::optional(const std::optional<_Tp>&) [with _Up = CFeeRate]'
884 | && __construct_from_contained_value<_Up>
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
./validation.h:164:41: required from here
164 | return MempoolAcceptResult(state);
| ^
/usr/lib/gcc/x86_64-pc-linux-gnu/15/include/g++-v15/optional:886:2: required by the constraints of 'template<class _Tp> template<class _Up> requires !(is_same_v<std::optional<_Tp>, typename std::remove_cvref<_It2>::type>) && (is_constructible_v<_Tp, const _Up&>) && (__construct_from_contained_value<_Up, typename std::remove_cv< <template-parameter-1-1> >::type>) constexpr std::optional<_Tp>::optional(const std::optional<_From>&)'
/usr/lib/gcc/x86_64-pc-linux-gnu/15/include/g++-v15/optional:884:14: error: satisfaction of atomic constraint '__construct_from_contained_value<_Up, typename std::remove_cv< <template-parameter-1-1> >::type> [with _Tp = _Tp; _Up = _Up]' depends on itself
884 | && __construct_from_contained_value<_Up>
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
```
It is easiest to solve this by changing the `static_assert` in the explicit `CFeeRate` constructor to a SFINAE by using a type constraint on the function template parameter.
We already [downstreamed](https://github.com/gentoo/gentoo/pull/38015) these fixes in Gentoo.
ACKs for top commit:
stickies-v:
ACK 055bc05792ff5d5b084563044818ebec12bfd748
Tree-SHA512: ce9cb27bcd9b0f4bbc80951e45cf7127112dcb7f9937bcb0167b362026d35beecb1255354746de0aac82e03c41eaccbe26acbfe0ddff2ee1e5a8634673f4f4ba
|
|
b0ec8716bf27335686471e0ae4c6a34f9a08f33c chainparams: Handle Testnet4 in GetNetworkForMagic (Fabian Jahr)
Pull request description:
Found during testing: The recently introduced `GetNetworkForMagic()` doesn't handle Testnet4 yet.
ACKs for top commit:
tdb3:
cr ACK b0ec8716bf27335686471e0ae4c6a34f9a08f33c
maflcko:
review ACK b0ec8716bf27335686471e0ae4c6a34f9a08f33c
theStack:
ACK b0ec8716bf27335686471e0ae4c6a34f9a08f33c
willcl-ark:
crACK b0ec8716bf27335686471e0ae4c6a34f9a08f33c
Tree-SHA512: 77cd0a6791529e5c5dfdb25cff3eff77224be9058d7cf4a8b3544651eb44c5e8ee90c5abfb9751ab0e11c5aa2d8477831dbf1868c4d5909481e0979e4db1eb28
|
|
fa5755b0a8536b844fdccfecf386c1baab24f1c9 doc: rpc: Use "output script" consistently (2/2) (MarcoFalke)
Pull request description:
Small follow-up to https://github.com/bitcoin/bitcoin/pull/30408 to fixup the RPCs that were forgotten.
ACKs for top commit:
theStack:
lgtm ACK fa5755b0a8536b844fdccfecf386c1baab24f1c9
Tree-SHA512: f1fc0aabb59017da216d6fe0f08a2274336d04db332ad6ce3d9608cd6f03667be1c76423f24a489ac8e7d536011a129dca752ab64b4621b7bc1d4d53f68602e4
|
|
In file included from /usr/lib/gcc/x86_64-pc-linux-gnu/15/include/g++-v15/format:48,
from /usr/lib/gcc/x86_64-pc-linux-gnu/15/include/g++-v15/bits/chrono_io.h:39,
from /usr/lib/gcc/x86_64-pc-linux-gnu/15/include/g++-v15/chrono:3362,
from ./util/time.h:9,
from ./primitives/block.h:12,
from ./blockencodings.h:8,
from blockencodings.cpp:5:
/usr/lib/gcc/x86_64-pc-linux-gnu/15/include/g++-v15/type_traits: In substitution of 'template<class _Up> requires !(is_same_v<std::optional<_Tp>, typename std::remove_cvref<_It2>::type>) && (is_constructible_v<_Tp, const _Up&>) && (__construct_from_contained_value<_Up, typename std::remove_cv< <template-parameter-1-1> >::type>) constexpr std::optional<CFeeRate>::optional(const std::optional<_Tp>&) [with _Up = CFeeRate]':
/usr/lib/gcc/x86_64-pc-linux-gnu/15/include/g++-v15/type_traits:1140:25: required by substitution of 'template<class _Tp, class ... _Args> using std::__is_constructible_impl = std::__bool_constant<__is_constructible(_Tp, _Args ...)> [with _Tp = CFeeRate; _Args = {std::optional<CFeeRate>&}]'
1140 | = __bool_constant<__is_constructible(_Tp, _Args...)>;
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/usr/lib/gcc/x86_64-pc-linux-gnu/15/include/g++-v15/type_traits:1145:12: required from 'struct std::is_constructible<CFeeRate, std::optional<CFeeRate>&>'
1145 | struct is_constructible
| ^~~~~~~~~~~~~~~~
/usr/lib/gcc/x86_64-pc-linux-gnu/15/include/g++-v15/type_traits:178:35: required by substitution of 'template<class ... _Bn> std::__detail::__first_t<std::integral_constant<bool, false>, typename std::enable_if<(!(bool)(_Bn::value)), void>::type ...> std::__detail::__or_fn(int) [with _Bn = {std::is_constructible<CFeeRate, std::optional<CFeeRate>&>, std::is_convertible<std::optional<CFeeRate>&, CFeeRate>, std::is_constructible<CFeeRate, std::optional<CFeeRate> >, std::is_convertible<std::optional<CFeeRate>, CFeeRate>, std::is_constructible<CFeeRate, const std::optional<CFeeRate>&>, std::is_convertible<const std::optional<CFeeRate>&, CFeeRate>, std::is_constructible<CFeeRate, const std::optional<CFeeRate> >, std::is_convertible<const std::optional<CFeeRate>, CFeeRate>}]'
178 | __enable_if_t<!bool(_Bn::value)>...>;
| ^~~~~
/usr/lib/gcc/x86_64-pc-linux-gnu/15/include/g++-v15/type_traits:196:41: required from 'struct std::__or_<std::is_constructible<CFeeRate, std::optional<CFeeRate>&>, std::is_convertible<std::optional<CFeeRate>&, CFeeRate>, std::is_constructible<CFeeRate, std::optional<CFeeRate> >, std::is_convertible<std::optional<CFeeRate>, CFeeRate>, std::is_constructible<CFeeRate, const std::optional<CFeeRate>&>, std::is_convertible<const std::optional<CFeeRate>&, CFeeRate>, std::is_constructible<CFeeRate, const std::optional<CFeeRate> >, std::is_convertible<const std::optional<CFeeRate>, CFeeRate> >'
196 | : decltype(__detail::__or_fn<_Bn...>(0))
| ~~~~~~~~~~~~~~~~~~~~~~~~~^~~
/usr/lib/gcc/x86_64-pc-linux-gnu/15/include/g++-v15/optional:824:45: required from 'constexpr const bool std::optional<CFeeRate>::__construct_from_contained_value<CFeeRate, CFeeRate>'
824 | = !__converts_from_optional<_Tp, _From>::value;
| ^~~~~
/usr/lib/gcc/x86_64-pc-linux-gnu/15/include/g++-v15/optional:884:7: required by substitution of 'template<class _Up> requires !(is_same_v<std::optional<_Tp>, typename std::remove_cvref<_It2>::type>) && (is_constructible_v<_Tp, const _Up&>) && (__construct_from_contained_value<_Up, typename std::remove_cv< <template-parameter-1-1> >::type>) constexpr std::optional<CFeeRate>::optional(const std::optional<_Tp>&) [with _Up = CFeeRate]'
884 | && __construct_from_contained_value<_Up>
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
./validation.h:164:41: required from here
164 | return MempoolAcceptResult(state);
| ^
/usr/lib/gcc/x86_64-pc-linux-gnu/15/include/g++-v15/optional:886:2: required by the constraints of 'template<class _Tp> template<class _Up> requires !(is_same_v<std::optional<_Tp>, typename std::remove_cvref<_It2>::type>) && (is_constructible_v<_Tp, const _Up&>) && (__construct_from_contained_value<_Up, typename std::remove_cv< <template-parameter-1-1> >::type>) constexpr std::optional<_Tp>::optional(const std::optional<_From>&)'
/usr/lib/gcc/x86_64-pc-linux-gnu/15/include/g++-v15/optional:884:14: error: satisfaction of atomic constraint '__construct_from_contained_value<_Up, typename std::remove_cv< <template-parameter-1-1> >::type> [with _Tp = _Tp; _Up = _Up]' depends on itself
884 | && __construct_from_contained_value<_Up>
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
|