💬 hebasto commented on pull request "test: Add missed header":
(https://github.com/bitcoin-core/gui/pull/729#issuecomment-1551033551)
The MSVC link issue was kind of intermittent. I guess, it was dependent on the actual order of processing of the source files.
(https://github.com/bitcoin-core/gui/pull/729#issuecomment-1551033551)
The MSVC link issue was kind of intermittent. I guess, it was dependent on the actual order of processing of the source files.
💬 MarcoFalke commented on pull request "test: Add missed header":
(https://github.com/bitcoin-core/gui/pull/729#issuecomment-1551036780)
Ok, interesting. Maybe with iwyu this kind of bug will be fixed eventually :smiling_face_with_tear:
(https://github.com/bitcoin-core/gui/pull/729#issuecomment-1551036780)
Ok, interesting. Maybe with iwyu this kind of bug will be fixed eventually :smiling_face_with_tear:
💬 fanquake commented on pull request "test: Add missed header":
(https://github.com/bitcoin-core/gui/pull/729#issuecomment-1551037870)
Yea. I guess the compiler is broken? Developers should not have to worry about putting files in the "right order" for it.
(https://github.com/bitcoin-core/gui/pull/729#issuecomment-1551037870)
Yea. I guess the compiler is broken? Developers should not have to worry about putting files in the "right order" for it.
👍 hebasto approved a pull request: "build: Bump minimum supported Clang to clang-10"
(https://github.com/bitcoin/bitcoin/pull/27682#pullrequestreview-1430199164)
ACK fa199ee614a7ed99c6caf329093a3573ea5a664b
(https://github.com/bitcoin/bitcoin/pull/27682#pullrequestreview-1430199164)
ACK fa199ee614a7ed99c6caf329093a3573ea5a664b
💬 jamesob commented on issue "Use muhash for assumeUTXO snapshot ":
(https://github.com/bitcoin/bitcoin/issues/27669#issuecomment-1551051475)
See the proposal for why I didn't use muhash to begin with: https://github.com/jamesob/assumeutxo-docs/tree/2019-04-proposal/proposal#how-will-users-and-reviewers-efficiently-verify-hashes-of-a-given-utxo-set
That said, the original rationale (supporting snapshot chunks) doesn't seem as relevant anymore, so this might be worth revisiting...
(https://github.com/bitcoin/bitcoin/issues/27669#issuecomment-1551051475)
See the proposal for why I didn't use muhash to begin with: https://github.com/jamesob/assumeutxo-docs/tree/2019-04-proposal/proposal#how-will-users-and-reviewers-efficiently-verify-hashes-of-a-given-utxo-set
That said, the original rationale (supporting snapshot chunks) doesn't seem as relevant anymore, so this might be worth revisiting...
💬 Sjors commented on issue "Proposal for a new mempool design":
(https://github.com/bitcoin/bitcoin/issues/27677#issuecomment-1551056767)
Nice! It's also worth noting that although this proposal introduces a new limit, for the cluster size, it could potentially supersede some other limits (e.g. `-limitancestorcount`, `-limitancestorsize`, `-limitdescendantcount`, `-limitdescendantsize`) if the maximum cluster size is large enough. Though if the limit is in vbytes, then someone assuming a limit in count might still hit a non backwards compatible policy limit.
(https://github.com/bitcoin/bitcoin/issues/27677#issuecomment-1551056767)
Nice! It's also worth noting that although this proposal introduces a new limit, for the cluster size, it could potentially supersede some other limits (e.g. `-limitancestorcount`, `-limitancestorsize`, `-limitdescendantcount`, `-limitdescendantsize`) if the maximum cluster size is large enough. Though if the limit is in vbytes, then someone assuming a limit in count might still hit a non backwards compatible policy limit.
💬 hebasto commented on pull request "build: Bump minimum supported Clang to clang-10":
(https://github.com/bitcoin/bitcoin/pull/27682#issuecomment-1551090163)
Guix builds:
```
4837adede6c57a98c0bc0ddaa178870a543c3f6ce9c3c8cd47841262937637c7 guix-build-fa199ee614a7/output/arm64-apple-darwin/SHA256SUMS.part
7ff35a219b09300f27a03b00196367ae9f17effab610e440a60322b0dd6a22e8 guix-build-fa199ee614a7/output/arm64-apple-darwin/bitcoin-fa199ee614a7-arm64-apple-darwin-unsigned.dmg
4dcf0408d81b0558f30055749334b966b8b1d2269da32238ff1f219e3e027bfc guix-build-fa199ee614a7/output/arm64-apple-darwin/bitcoin-fa199ee614a7-arm64-apple-darwin-unsigned.tar.gz
169fe
...
(https://github.com/bitcoin/bitcoin/pull/27682#issuecomment-1551090163)
Guix builds:
```
4837adede6c57a98c0bc0ddaa178870a543c3f6ce9c3c8cd47841262937637c7 guix-build-fa199ee614a7/output/arm64-apple-darwin/SHA256SUMS.part
7ff35a219b09300f27a03b00196367ae9f17effab610e440a60322b0dd6a22e8 guix-build-fa199ee614a7/output/arm64-apple-darwin/bitcoin-fa199ee614a7-arm64-apple-darwin-unsigned.dmg
4dcf0408d81b0558f30055749334b966b8b1d2269da32238ff1f219e3e027bfc guix-build-fa199ee614a7/output/arm64-apple-darwin/bitcoin-fa199ee614a7-arm64-apple-darwin-unsigned.tar.gz
169fe
...
📝 fanquake opened a pull request: "ci: remove `RUN_SECURITY_TESTS`"
(https://github.com/bitcoin/bitcoin/pull/27683)
We no-longer run any security/syymbol checks in the CI, and doubt we will in future (if we do, it'll be via Guix, where this var would be redundant in any case). The CI environment doesn't (exactly) match the release build environment (and is semi-regularly changing), and the binaries produced in the CI don't match how we build release binaries, so there is no point trying to run these checks, especially as we add more involved tests, i.e #26953.
(https://github.com/bitcoin/bitcoin/pull/27683)
We no-longer run any security/syymbol checks in the CI, and doubt we will in future (if we do, it'll be via Guix, where this var would be redundant in any case). The CI environment doesn't (exactly) match the release build environment (and is semi-regularly changing), and the binaries produced in the CI don't match how we build release binaries, so there is no point trying to run these checks, especially as we add more involved tests, i.e #26953.
💬 fanquake commented on pull request "macOS: Bump minimum required runtime version and prepare for building with upstream LLVM":
(https://github.com/bitcoin/bitcoin/pull/27676#issuecomment-1551101900)
> @fanquake What sets RUN_SECURITY_TESTS?
Nothing. symbol/security checks are no-longer run in the CI, because the CI environment is semi-regularly changing, and the binaries produced don't represent release binaries, meaning there are some tests that will never pass. `RUN_SECURITY_TESTS` is leftover from when we used run (some) tests; I've opened a PR to remove it (#27683). The current way to run the security/symbol checks is to perform a Guix build.
(https://github.com/bitcoin/bitcoin/pull/27676#issuecomment-1551101900)
> @fanquake What sets RUN_SECURITY_TESTS?
Nothing. symbol/security checks are no-longer run in the CI, because the CI environment is semi-regularly changing, and the binaries produced don't represent release binaries, meaning there are some tests that will never pass. `RUN_SECURITY_TESTS` is leftover from when we used run (some) tests; I've opened a PR to remove it (#27683). The current way to run the security/symbol checks is to perform a Guix build.
💬 fanquake commented on pull request "ci: Use `macos-ventura-xcode:14.1` image for "macOS native" task":
(https://github.com/bitcoin/bitcoin/pull/26388#issuecomment-1551106363)
> As https://github.com/bitcoin/bitcoin/pull/27676#issuecomment-1550459263, this seems to have removed RUN_SECURITY_TESTS for x86 too?
See #27683.
(https://github.com/bitcoin/bitcoin/pull/26388#issuecomment-1551106363)
> As https://github.com/bitcoin/bitcoin/pull/27676#issuecomment-1550459263, this seems to have removed RUN_SECURITY_TESTS for x86 too?
See #27683.
📝 hebasto opened a pull request: "Avoid lock order inversion in `Chainstate::ConnectTip` function "
(https://github.com/bitcoin/bitcoin/pull/27684)
Due to the synchronous call of `CValidationInterface::BlockChecked` a lock order inversion [happens](https://github.com/bitcoin/bitcoin/issues/19303#issuecomment-1514928912):
```
PeerManagerImpl::m_peer_mutex
|
V
Peer::TxRelay::m_tx_inventory_mutex
|
V
CTxMemPool::cs
|
V
PeerManagerImpl::m_peer_mutex
```
This PR breaks the last link.
The other possible solution is to move `CValidationInterface::BlockChecked` to a background thread (see https://github.com/bitcoin/bit
...
(https://github.com/bitcoin/bitcoin/pull/27684)
Due to the synchronous call of `CValidationInterface::BlockChecked` a lock order inversion [happens](https://github.com/bitcoin/bitcoin/issues/19303#issuecomment-1514928912):
```
PeerManagerImpl::m_peer_mutex
|
V
Peer::TxRelay::m_tx_inventory_mutex
|
V
CTxMemPool::cs
|
V
PeerManagerImpl::m_peer_mutex
```
This PR breaks the last link.
The other possible solution is to move `CValidationInterface::BlockChecked` to a background thread (see https://github.com/bitcoin/bit
...
💬 fanquake commented on pull request "macOS: Bump minimum required runtime version and prepare for building with upstream LLVM":
(https://github.com/bitcoin/bitcoin/pull/27676#issuecomment-1551127526)
The CI failure is unrelated, and will be fixed post-rebase. Guix currently fails to build. i.e:
```bash
time HOSTS="x86_64-apple-darwin arm64-apple-darwin" ./contrib/guix/guix-build
<snip>
Extracting libevent...
/home/ubuntu/sources/libevent-2.1.12-stable.tar.gz: OK
Preprocessing libevent...
Configuring libevent...
checking for a BSD-compatible install... /home/ubuntu/.guix-profile/bin/install -c
checking whether build environment is sane... yes
checking for x86_64-apple-darwin-strip..
...
(https://github.com/bitcoin/bitcoin/pull/27676#issuecomment-1551127526)
The CI failure is unrelated, and will be fixed post-rebase. Guix currently fails to build. i.e:
```bash
time HOSTS="x86_64-apple-darwin arm64-apple-darwin" ./contrib/guix/guix-build
<snip>
Extracting libevent...
/home/ubuntu/sources/libevent-2.1.12-stable.tar.gz: OK
Preprocessing libevent...
Configuring libevent...
checking for a BSD-compatible install... /home/ubuntu/.guix-profile/bin/install -c
checking whether build environment is sane... yes
checking for x86_64-apple-darwin-strip..
...
💬 hebasto commented on pull request "ci: remove `RUN_SECURITY_TESTS`":
(https://github.com/bitcoin/bitcoin/pull/27683#issuecomment-1551132082)
> We no-longer run any security/symbol checks in the CI, and doubt we will in future (if we do, it'll be via Guix, where this var would be redundant in any case).
Concept ACK on that.
(https://github.com/bitcoin/bitcoin/pull/27683#issuecomment-1551132082)
> We no-longer run any security/symbol checks in the CI, and doubt we will in future (if we do, it'll be via Guix, where this var would be redundant in any case).
Concept ACK on that.
💬 MarcoFalke commented on pull request "Avoid lock order inversion in `Chainstate::ConnectTip` function":
(https://github.com/bitcoin/bitcoin/pull/27684#discussion_r1196280935)
Can you explain why this is fine in the context of logical races, as well as performance-wise in the context of new thread(pools) getting spawned?
(https://github.com/bitcoin/bitcoin/pull/27684#discussion_r1196280935)
Can you explain why this is fine in the context of logical races, as well as performance-wise in the context of new thread(pools) getting spawned?
💬 hebasto commented on pull request "Avoid lock order inversion in `Chainstate::ConnectTip` function":
(https://github.com/bitcoin/bitcoin/pull/27684#discussion_r1196283047)
Is `MaybePunishNodeForBlock` performance critical?
(https://github.com/bitcoin/bitcoin/pull/27684#discussion_r1196283047)
Is `MaybePunishNodeForBlock` performance critical?
💬 DanM3rcurius commented on issue "Can't compile v24.0.1":
(https://github.com/bitcoin/bitcoin/issues/27680#issuecomment-1551144773)
Hi Hennadii,
I did, sorry i forgot to mention that.
Anything else that i miss?
sincerely,
Dan
Sent with [Proton Mail](https://proton.me/) secure email.
------- Original Message -------
On Wednesday, May 17th, 2023 at 00:21, Hennadii Stepanov ***@***.***> wrote:
>> Steps to reproduce
>>
>> Setup Raspi4 with Raspi OS installed on a bootable SSD
>> Install BDB 4.8
>> Clone bitcoin repo 24.0.1
>> configure
>
> Try to run ./autogen.sh before ./configure as it is documented [here](https://github.
...
(https://github.com/bitcoin/bitcoin/issues/27680#issuecomment-1551144773)
Hi Hennadii,
I did, sorry i forgot to mention that.
Anything else that i miss?
sincerely,
Dan
Sent with [Proton Mail](https://proton.me/) secure email.
------- Original Message -------
On Wednesday, May 17th, 2023 at 00:21, Hennadii Stepanov ***@***.***> wrote:
>> Steps to reproduce
>>
>> Setup Raspi4 with Raspi OS installed on a bootable SSD
>> Install BDB 4.8
>> Clone bitcoin repo 24.0.1
>> configure
>
> Try to run ./autogen.sh before ./configure as it is documented [here](https://github.
...
💬 MarcoFalke commented on issue "Can't compile v24.0.1":
(https://github.com/bitcoin/bitcoin/issues/27680#issuecomment-1551153545)
What are the exact steps to reproduce? Make sure to follow https://github.com/bitcoin/bitcoin/blob/24.x/doc/build-unix.md#to-build and not an external outdated random blog post from 2016. BDB isn't required for new installs, and in fact discouraged, unless you need it for a legacy wallet. New wallets use sqlite.
(https://github.com/bitcoin/bitcoin/issues/27680#issuecomment-1551153545)
What are the exact steps to reproduce? Make sure to follow https://github.com/bitcoin/bitcoin/blob/24.x/doc/build-unix.md#to-build and not an external outdated random blog post from 2016. BDB isn't required for new installs, and in fact discouraged, unless you need it for a legacy wallet. New wallets use sqlite.
💬 MarcoFalke commented on pull request "Avoid lock order inversion in `Chainstate::ConnectTip` function":
(https://github.com/bitcoin/bitcoin/pull/27684#discussion_r1196294868)
Bitcoin Core is performance critical and randomly (unspecified) spawning thread(pool)s may kill performance, cause OOM, etc ...
(https://github.com/bitcoin/bitcoin/pull/27684#discussion_r1196294868)
Bitcoin Core is performance critical and randomly (unspecified) spawning thread(pool)s may kill performance, cause OOM, etc ...
💬 DanM3rcurius commented on issue "Can't compile v24.0.1":
(https://github.com/bitcoin/bitcoin/issues/27680#issuecomment-1551160542)
Thank you for that info.
i will start from the beginning.
Sent from Proton Mail for iOS
On Wed, May 17, 2023 at 12:38, MacrabFalke ***@***.***> wrote:
> What are the exact steps to reproduce? Make sure to follow https://github.com/bitcoin/bitcoin/blob/24.x/doc/build-unix.md#to-build and not an external outdated random blog post from 2016. BDB isn't required for new installs, and in fact discouraged, unless you need it for a legacy wallet. New wallets use sqlite.
>
> —
> Reply to this email di
...
(https://github.com/bitcoin/bitcoin/issues/27680#issuecomment-1551160542)
Thank you for that info.
i will start from the beginning.
Sent from Proton Mail for iOS
On Wed, May 17, 2023 at 12:38, MacrabFalke ***@***.***> wrote:
> What are the exact steps to reproduce? Make sure to follow https://github.com/bitcoin/bitcoin/blob/24.x/doc/build-unix.md#to-build and not an external outdated random blog post from 2016. BDB isn't required for new installs, and in fact discouraged, unless you need it for a legacy wallet. New wallets use sqlite.
>
> —
> Reply to this email di
...
💬 DanM3rcurius commented on issue "Can't compile v24.0.1":
(https://github.com/bitcoin/bitcoin/issues/27680#issuecomment-1551166508)
Why i used the blog post, because it was linked from a beginner friendly tutorial for a raspnode setup. The bitcoin docs on github are difficult for novices like me to understand and execute.
Sent from Proton Mail for iOS
On Wed, May 17, 2023 at 12:38, MacrabFalke ***@***.***> wrote:
> What are the exact steps to reproduce? Make sure to follow https://github.com/bitcoin/bitcoin/blob/24.x/doc/build-unix.md#to-build and not an external outdated random blog post from 2016. BDB isn't required for
...
(https://github.com/bitcoin/bitcoin/issues/27680#issuecomment-1551166508)
Why i used the blog post, because it was linked from a beginner friendly tutorial for a raspnode setup. The bitcoin docs on github are difficult for novices like me to understand and execute.
Sent from Proton Mail for iOS
On Wed, May 17, 2023 at 12:38, MacrabFalke ***@***.***> wrote:
> What are the exact steps to reproduce? Make sure to follow https://github.com/bitcoin/bitcoin/blob/24.x/doc/build-unix.md#to-build and not an external outdated random blog post from 2016. BDB isn't required for
...