💬 chippsmith commented on issue "v25.0 testing":
(https://github.com/bitcoin/bitcoin/issues/27621#issuecomment-1558495158)
I downloaded the binaries on a System76 machine running Pop!OS 22.04 LTS. I ran all the tests in the testing guide(except the last one about verifying binaries) and got expected results. More detailed notes can be found here. https://github.com/chippsmith/bitcoinnotes/tree/main/bitcoin25.0rc.
(https://github.com/bitcoin/bitcoin/issues/27621#issuecomment-1558495158)
I downloaded the binaries on a System76 machine running Pop!OS 22.04 LTS. I ran all the tests in the testing guide(except the last one about verifying binaries) and got expected results. More detailed notes can be found here. https://github.com/chippsmith/bitcoinnotes/tree/main/bitcoin25.0rc.
⚠️ CryptoManiac opened an issue: "Validation of malformed address fails with peculiar message"
(https://github.com/bitcoin/bitcoin/issues/27723)
### Is there an existing issue for this?
- [X] I have searched the existing issues
### Current behaviour
Open RPC console and execute this command:
`validateaddress bc1qqrq69gfzvvqcxs6rgg3crqjzcw369sxzyp3v9sspursx9gmzyv32x7xa5z`
Following message is the result:
```
Internal bug detected: 'isValid == error_msg.empty()'
rpc/misc.cpp:75 (operator())
You may report this issue here: https://github.com/bitcoin/bitcoin/issues
(code -1)
```
### Expected behaviour
Well it should tel
...
(https://github.com/bitcoin/bitcoin/issues/27723)
### Is there an existing issue for this?
- [X] I have searched the existing issues
### Current behaviour
Open RPC console and execute this command:
`validateaddress bc1qqrq69gfzvvqcxs6rgg3crqjzcw369sxzyp3v9sspursx9gmzyv32x7xa5z`
Following message is the result:
```
Internal bug detected: 'isValid == error_msg.empty()'
rpc/misc.cpp:75 (operator())
You may report this issue here: https://github.com/bitcoin/bitcoin/issues
(code -1)
```
### Expected behaviour
Well it should tel
...
💬 de-served commented on issue "Bitcoin ignores datadir and blocksdir parameter in .conf":
(https://github.com/bitcoin/bitcoin/issues/27246#issuecomment-1558537251)
> My recommendation for you personally would be to configure the GUI to use the default datadir and place your `bitcoin.conf` file in `C:\Users\USERNAME\AppData\Roaming\Bitcoin` instead of
It's already here... for a "debug" what's happening:
`
datadir =W:/BitcoinCore/DataDir_/_User
blocksdir =W:/BitcoinCore/BlocksDir_/_User
walletdir =W:/BitcoinCore/WalletDir_/_User
debuglogfile=H:/_Logs_/BitcoinCore/_debug.default-appdata.no_params.log
[main]
datadir =W:/BitcoinCore/Data
...
(https://github.com/bitcoin/bitcoin/issues/27246#issuecomment-1558537251)
> My recommendation for you personally would be to configure the GUI to use the default datadir and place your `bitcoin.conf` file in `C:\Users\USERNAME\AppData\Roaming\Bitcoin` instead of
It's already here... for a "debug" what's happening:
`
datadir =W:/BitcoinCore/DataDir_/_User
blocksdir =W:/BitcoinCore/BlocksDir_/_User
walletdir =W:/BitcoinCore/WalletDir_/_User
debuglogfile=H:/_Logs_/BitcoinCore/_debug.default-appdata.no_params.log
[main]
datadir =W:/BitcoinCore/Data
...
💬 MarcoFalke commented on pull request "test: Wallet interactions with rescanning wallet":
(https://github.com/bitcoin/bitcoin/pull/26700#discussion_r1201575251)
This will race against the thread `t`, which may have already concluded before this point
(https://github.com/bitcoin/bitcoin/pull/26700#discussion_r1201575251)
This will race against the thread `t`, which may have already concluded before this point
💬 MarcoFalke commented on issue "Validation of malformed address fails with a peculiar message":
(https://github.com/bitcoin/bitcoin/issues/27723#issuecomment-1558620137)
Thank you for the bug report. I think what happens is:
* The function `ConvertBits<5, 8, false>` fails, however
* the function `bech32::LocateErrors` also fails to locate any errors.
Thus the `CHECK_NONFATAL(isValid == error_msg.empty())` added in commit 8f0b64fb513e8c6cdd1f115856100a4ef5afe23e will be hit.
(https://github.com/bitcoin/bitcoin/issues/27723#issuecomment-1558620137)
Thank you for the bug report. I think what happens is:
* The function `ConvertBits<5, 8, false>` fails, however
* the function `bech32::LocateErrors` also fails to locate any errors.
Thus the `CHECK_NONFATAL(isValid == error_msg.empty())` added in commit 8f0b64fb513e8c6cdd1f115856100a4ef5afe23e will be hit.
💬 MarcoFalke commented on pull request "wallet: Load database records in a particular order":
(https://github.com/bitcoin/bitcoin/pull/24914#discussion_r1201618467)
You can just use `DataStream` for that, which comes with compile-time checks to enforce that both values are in fact never read, and thus can be omitted completely.
Also, in the same commit, is there a reason why you are changing `DataStream ssKey` to `CDataStream ssKey`? That seems like a step backward, no?
(https://github.com/bitcoin/bitcoin/pull/24914#discussion_r1201618467)
You can just use `DataStream` for that, which comes with compile-time checks to enforce that both values are in fact never read, and thus can be omitted completely.
Also, in the same commit, is there a reason why you are changing `DataStream ssKey` to `CDataStream ssKey`? That seems like a step backward, no?
💬 denavila commented on pull request "Deniability - a tool to automatically improve coin ownership privacy":
(https://github.com/bitcoin-core/gui/pull/733#issuecomment-1558676764)
> Has that idea been discussed somewhere already?
The idea was published quite a while ago, but I learned about it just recently.
We looked around to see if there were existing discussions or implementations, but we didn't find anything on bitcoin-dev or the repo here.
There's an old implementation in a Bitcoin fork (Drivechain) which I used as a starting point, but ended up with a pretty different approach and implementation.
We were thinking of perhaps writing down the spec into a Pro
...
(https://github.com/bitcoin-core/gui/pull/733#issuecomment-1558676764)
> Has that idea been discussed somewhere already?
The idea was published quite a while ago, but I learned about it just recently.
We looked around to see if there were existing discussions or implementations, but we didn't find anything on bitcoin-dev or the repo here.
There's an old implementation in a Bitcoin fork (Drivechain) which I used as a starting point, but ended up with a pretty different approach and implementation.
We were thinking of perhaps writing down the spec into a Pro
...
📝 willcl-ark opened a pull request: "build: disable boost multi index safe mode in debug mode"
(https://github.com/bitcoin/bitcoin/pull/27724)
Fixes #27586
Disable boost multi index safe mode by default when configuring with
--enable-debug.
This option can cause transactions to take a long time to be accepted
into the mempool under certain conditions; iterator destruction takes
O(n) time vs O(1) as they are stored in a singly linked list. See
27586 and the [boost docs](https://www.boost.org/doc/libs/1_58_0/boost/multi_index/detail/safe_mode.hpp) for more information.
Adds a new configure option `--enable-boostmultiindexsaf
...
(https://github.com/bitcoin/bitcoin/pull/27724)
Fixes #27586
Disable boost multi index safe mode by default when configuring with
--enable-debug.
This option can cause transactions to take a long time to be accepted
into the mempool under certain conditions; iterator destruction takes
O(n) time vs O(1) as they are stored in a singly linked list. See
27586 and the [boost docs](https://www.boost.org/doc/libs/1_58_0/boost/multi_index/detail/safe_mode.hpp) for more information.
Adds a new configure option `--enable-boostmultiindexsaf
...
💬 hebasto commented on pull request "build: disable boost multi index safe mode in debug mode":
(https://github.com/bitcoin/bitcoin/pull/27724#issuecomment-1558736106)
Somewhat related: #27353.
(https://github.com/bitcoin/bitcoin/pull/27724#issuecomment-1558736106)
Somewhat related: #27353.
💬 willcl-ark commented on pull request "build: disable boost multi index safe mode in debug mode":
(https://github.com/bitcoin/bitcoin/pull/27724#issuecomment-1558738915)
The approach taken here was influenced by discussion in https://github.com/bitcoin/bitcoin/issues/24709#issuecomment-1556705320
I've been unable to instigate the congested behaviour on two nodes overnight, one with default mempool and one with 500MB mempool, but hopefully will manage to do so shortly and will reply here if so.
In any case, it seems overkill to enable these safe mode checks in debug builds (which are often tested on mainnet), and a more sensible approach to limit to CI task
...
(https://github.com/bitcoin/bitcoin/pull/27724#issuecomment-1558738915)
The approach taken here was influenced by discussion in https://github.com/bitcoin/bitcoin/issues/24709#issuecomment-1556705320
I've been unable to instigate the congested behaviour on two nodes overnight, one with default mempool and one with 500MB mempool, but hopefully will manage to do so shortly and will reply here if so.
In any case, it seems overkill to enable these safe mode checks in debug builds (which are often tested on mainnet), and a more sensible approach to limit to CI task
...
💬 fanquake commented on pull request "build: disable boost multi index safe mode in debug mode":
(https://github.com/bitcoin/bitcoin/pull/27724#issuecomment-1558740250)
Concept NACK - I don't think adding configure flags for specific Boost defined is a good approach. Will follow up in the other threads.
(https://github.com/bitcoin/bitcoin/pull/27724#issuecomment-1558740250)
Concept NACK - I don't think adding configure flags for specific Boost defined is a good approach. Will follow up in the other threads.
💬 willcl-ark commented on pull request "build: disable boost multi index safe mode in debug mode":
(https://github.com/bitcoin/bitcoin/pull/27724#issuecomment-1558764043)
> Concept NACK - I don't think adding configure flags for specific Boost defines is a good approach. Will follow up in the other threads.
OK. Looking forward to your thoughts, as I don't know how the same effect could be achieved in any neater or more maintainable way.
One alternative could be to manually append to the CI CPP flags, if that's all we wanted.
(https://github.com/bitcoin/bitcoin/pull/27724#issuecomment-1558764043)
> Concept NACK - I don't think adding configure flags for specific Boost defines is a good approach. Will follow up in the other threads.
OK. Looking forward to your thoughts, as I don't know how the same effect could be achieved in any neater or more maintainable way.
One alternative could be to manually append to the CI CPP flags, if that's all we wanted.
💬 MarcoFalke commented on pull request "build: disable boost multi index safe mode in debug mode":
(https://github.com/bitcoin/bitcoin/pull/27724#issuecomment-1558819623)
> One alternative could be to manually append to the CI CPP flags, if that's all we wanted.
Or just a configure setting to append CPP flags to the existing ones, instead of replacing the default ones?
Concept ACK
(https://github.com/bitcoin/bitcoin/pull/27724#issuecomment-1558819623)
> One alternative could be to manually append to the CI CPP flags, if that's all we wanted.
Or just a configure setting to append CPP flags to the existing ones, instead of replacing the default ones?
Concept ACK
💬 fanquake commented on pull request "build: disable boost multi index safe mode in debug mode":
(https://github.com/bitcoin/bitcoin/pull/27724#issuecomment-1558827148)
> Or just a configure setting to append CPP flags to the existing ones,
There's no need for a new setting. Just use `CPPFLAGS="-DWHATEVER"`. This has worked since #24391.
(https://github.com/bitcoin/bitcoin/pull/27724#issuecomment-1558827148)
> Or just a configure setting to append CPP flags to the existing ones,
There's no need for a new setting. Just use `CPPFLAGS="-DWHATEVER"`. This has worked since #24391.
💬 MarcoFalke commented on pull request "build: disable boost multi index safe mode in debug mode":
(https://github.com/bitcoin/bitcoin/pull/27724#discussion_r1201881964)
I wonder if this should be moved to the debug fuzz task to avoid only fuzzing a debug version of boost in all fuzz configs?
(https://github.com/bitcoin/bitcoin/pull/27724#discussion_r1201881964)
I wonder if this should be moved to the debug fuzz task to avoid only fuzzing a debug version of boost in all fuzz configs?
💬 willcl-ark commented on pull request "build: disable boost multi index safe mode in debug mode":
(https://github.com/bitcoin/bitcoin/pull/27724#discussion_r1201884441)
I like that. If anything it makes more sense to me to fuzz the non debug version...
(https://github.com/bitcoin/bitcoin/pull/27724#discussion_r1201884441)
I like that. If anything it makes more sense to me to fuzz the non debug version...
💬 MarcoFalke commented on pull request "build: disable boost multi index safe mode in debug mode":
(https://github.com/bitcoin/bitcoin/pull/27724#discussion_r1201886659)
ideally both are fuzzed :)
(https://github.com/bitcoin/bitcoin/pull/27724#discussion_r1201886659)
ideally both are fuzzed :)
👍 MarcoFalke approved a pull request: "build: disable boost multi index safe mode in debug mode"
(https://github.com/bitcoin/bitcoin/pull/27724#pullrequestreview-1439114903)
lgtm, but the description needs to be adjusted?
(https://github.com/bitcoin/bitcoin/pull/27724#pullrequestreview-1439114903)
lgtm, but the description needs to be adjusted?
⚠️ 137718 opened an issue: "Do you want to rebuild the block database now?"
(https://github.com/bitcoin/bitcoin/issues/27725)
Do you want to rebuild the block database now?
(https://github.com/bitcoin/bitcoin/issues/27725)
Do you want to rebuild the block database now?
✅ fanquake closed an issue: "Do you want to rebuild the block database now?"
(https://github.com/bitcoin/bitcoin/issues/27725)
(https://github.com/bitcoin/bitcoin/issues/27725)
👍 ajtowns approved a pull request: "Parallel compact block downloads, take 3"
(https://github.com/bitcoin/bitcoin/pull/27626#pullrequestreview-1438918136)
ACK 3b57b2b8d958871bec9a33953be051a75d8bf07c
Despite thinking this is an improvement on what we currently do and that it's ready for merge, there's two things I'm not 100% comfortable with, and that I think would be good to improve in a followup. (1) I think this puts more demand on outbound peers than it should, as per [previous comments](https://github.com/bitcoin/bitcoin/pull/27626#discussion_r1200426349). (2) I think the multimap stuff makes this hard to understand, and it would be good t
...
(https://github.com/bitcoin/bitcoin/pull/27626#pullrequestreview-1438918136)
ACK 3b57b2b8d958871bec9a33953be051a75d8bf07c
Despite thinking this is an improvement on what we currently do and that it's ready for merge, there's two things I'm not 100% comfortable with, and that I think would be good to improve in a followup. (1) I think this puts more demand on outbound peers than it should, as per [previous comments](https://github.com/bitcoin/bitcoin/pull/27626#discussion_r1200426349). (2) I think the multimap stuff makes this hard to understand, and it would be good t
...