💬 hebasto commented on pull request "build: Pass sanitize flags to instrument `libsecp256k1` code":
(https://github.com/bitcoin/bitcoin/pull/28875#discussion_r1458717675)
Sure. Done.
(https://github.com/bitcoin/bitcoin/pull/28875#discussion_r1458717675)
Sure. Done.
💬 fanquake commented on pull request "[26.x] more backports":
(https://github.com/bitcoin/bitcoin/pull/29209#issuecomment-1900132316)
Lets add #29127, so if we decide to, we can test macOS notarization: https://github.com/bitcoin/bitcoin/issues/15774#issuecomment-1899228781.
(https://github.com/bitcoin/bitcoin/pull/29209#issuecomment-1900132316)
Lets add #29127, so if we decide to, we can test macOS notarization: https://github.com/bitcoin/bitcoin/issues/15774#issuecomment-1899228781.
💬 josibake commented on pull request "Silent Payments: Implement BIP352":
(https://github.com/bitcoin/bitcoin/pull/28122#issuecomment-1900134591)
Rebased https://github.com/bitcoin/bitcoin/commit/0299dfb0607b85813c7226690772ab3be198b0df -> https://github.com/bitcoin/bitcoin/commit/6a9ec4771d691bf4143f3574dd129da641c6ee74
* Fixes a silent merge conflict introduced by changing `CKey` to use `std::byte*` instead of `unsigned char`
(https://github.com/bitcoin/bitcoin/pull/28122#issuecomment-1900134591)
Rebased https://github.com/bitcoin/bitcoin/commit/0299dfb0607b85813c7226690772ab3be198b0df -> https://github.com/bitcoin/bitcoin/commit/6a9ec4771d691bf4143f3574dd129da641c6ee74
* Fixes a silent merge conflict introduced by changing `CKey` to use `std::byte*` instead of `unsigned char`
👍 fanquake approved a pull request: "depends: Update libmultiprocess library to fix C++20 macos build error"
(https://github.com/bitcoin/bitcoin/pull/29276#pullrequestreview-1832088849)
ACK 5dfd24581a7c5497601966a5371d8c33eabcee8a - thanks. Checked this fixes the macOS build. Also tested the build on aarch64.
(https://github.com/bitcoin/bitcoin/pull/29276#pullrequestreview-1832088849)
ACK 5dfd24581a7c5497601966a5371d8c33eabcee8a - thanks. Checked this fixes the macOS build. Also tested the build on aarch64.
💬 RicYashiroLee commented on pull request "set `DEFAULT_PERMIT_BAREMULTISIG` to false":
(https://github.com/bitcoin/bitcoin/pull/28217#issuecomment-1900148350)
> Concept NACK.
>
> Bitcoin moves value* through time + space.
>
> *Value is subjective. (Value, in Bitcoin, is subjectively determined by the fee-rate of the tx.)
>
> Bitcoin is a cypherpunk system. Bitcoin is based on crypto-anarchy. This is a colorful way of saying that Bitcoin is about freedom, including free markets.
>
> Bitcoin's anti-DoS mechanism is not about Bitcoin Core sending political messages to dictate to the economy what it should or should not do.
>
> Bitcoin's an
...
(https://github.com/bitcoin/bitcoin/pull/28217#issuecomment-1900148350)
> Concept NACK.
>
> Bitcoin moves value* through time + space.
>
> *Value is subjective. (Value, in Bitcoin, is subjectively determined by the fee-rate of the tx.)
>
> Bitcoin is a cypherpunk system. Bitcoin is based on crypto-anarchy. This is a colorful way of saying that Bitcoin is about freedom, including free markets.
>
> Bitcoin's anti-DoS mechanism is not about Bitcoin Core sending political messages to dictate to the economy what it should or should not do.
>
> Bitcoin's an
...
💬 fanquake commented on pull request "depends: add NM output to gen_id":
(https://github.com/bitcoin/bitcoin/pull/29249#issuecomment-1900188495)
@TheCharlatan note that you've ACK'd what I assume was the prior commit hash here
(https://github.com/bitcoin/bitcoin/pull/29249#issuecomment-1900188495)
@TheCharlatan note that you've ACK'd what I assume was the prior commit hash here
💬 josibake commented on pull request "Silent Payments: Implement BIP352":
(https://github.com/bitcoin/bitcoin/pull/28122#issuecomment-1900189106)
Updated https://github.com/bitcoin/bitcoin/commit/6a9ec4771d691bf4143f3574dd129da641c6ee74 -> https://github.com/bitcoin/bitcoin/pull/28122/commits/6e7c29528eb4a7c39bf8290b7c622e30bf07b45d ([implement-bip352-00](https://github.com/josibake/bitcoin/tree/implement-bip352-00) -> [implement-bip352-01](https://github.com/josibake/bitcoin/tree/implement-bip352-01), [compare](https://github.com/josibake/bitcoin/compare/implement-bip352-00..implement-bip352-01))
* Adds a test case for taproot inputs wi
...
(https://github.com/bitcoin/bitcoin/pull/28122#issuecomment-1900189106)
Updated https://github.com/bitcoin/bitcoin/commit/6a9ec4771d691bf4143f3574dd129da641c6ee74 -> https://github.com/bitcoin/bitcoin/pull/28122/commits/6e7c29528eb4a7c39bf8290b7c622e30bf07b45d ([implement-bip352-00](https://github.com/josibake/bitcoin/tree/implement-bip352-00) -> [implement-bip352-01](https://github.com/josibake/bitcoin/tree/implement-bip352-01), [compare](https://github.com/josibake/bitcoin/compare/implement-bip352-00..implement-bip352-01))
* Adds a test case for taproot inputs wi
...
💬 TheCharlatan commented on pull request "kernel: Remove dependency on CScheduler":
(https://github.com/bitcoin/bitcoin/pull/28960#issuecomment-1900227084)
Thank you for the review and detailed comments @ryanofsky!
Rebased e3592ee55756c08bda3075b54cbff719d3fb964f -> 2ef630f3e49f11cbae7abe2dc31c2a908bb5e6ba ([noGlobalSignals_4](https://github.com/TheCharlatan/bitcoin/tree/noGlobalSignals_4) -> [noGlobalSignals_5](https://github.com/TheCharlatan/bitcoin/tree/noGlobalSignals_5), [compare](https://github.com/TheCharlatan/bitcoin/compare/noGlobalSignals_4..noGlobalSignals_5))
Updated 2ef630f3e49f11cbae7abe2dc31c2a908bb5e6ba -> aad39765ad69565dcbdb
...
(https://github.com/bitcoin/bitcoin/pull/28960#issuecomment-1900227084)
Thank you for the review and detailed comments @ryanofsky!
Rebased e3592ee55756c08bda3075b54cbff719d3fb964f -> 2ef630f3e49f11cbae7abe2dc31c2a908bb5e6ba ([noGlobalSignals_4](https://github.com/TheCharlatan/bitcoin/tree/noGlobalSignals_4) -> [noGlobalSignals_5](https://github.com/TheCharlatan/bitcoin/tree/noGlobalSignals_5), [compare](https://github.com/TheCharlatan/bitcoin/compare/noGlobalSignals_4..noGlobalSignals_5))
Updated 2ef630f3e49f11cbae7abe2dc31c2a908bb5e6ba -> aad39765ad69565dcbdb
...
💬 willcl-ark commented on issue "macOS App Notarization & Stapling":
(https://github.com/bitcoin/bitcoin/issues/15774#issuecomment-1900233704)
> Given that MacOS is just phoning home all the time, I think we should move ahead with notarizing our MacOS releases to improve the installation experience. Although stapling sounds like a good idea, at this point, I sincerely doubt that Apple will change their tooling to stop phoning home for stapled apps, but maybe we should do so in the hopes that they change their minds.
In this case notarizing MacOS releases seems the most reasonable course of action to me too. Of course we'd like the a
...
(https://github.com/bitcoin/bitcoin/issues/15774#issuecomment-1900233704)
> Given that MacOS is just phoning home all the time, I think we should move ahead with notarizing our MacOS releases to improve the installation experience. Although stapling sounds like a good idea, at this point, I sincerely doubt that Apple will change their tooling to stop phoning home for stapled apps, but maybe we should do so in the hopes that they change their minds.
In this case notarizing MacOS releases seems the most reasonable course of action to me too. Of course we'd like the a
...
💬 TheCharlatan commented on pull request "kernel: Remove dependency on CScheduler":
(https://github.com/bitcoin/bitcoin/pull/28960#discussion_r1458838935)
> It might also be worth asking whether the signals parameter actually should be required.
It is used in `CTxMemPool` methods, so should be required, no? Or are you saying these classes should be given the option to not be sending signals in the first place? In that case, I don't see what the difference between the mempool and the other validation classes is. The signals could be optional in the chainman too.
(https://github.com/bitcoin/bitcoin/pull/28960#discussion_r1458838935)
> It might also be worth asking whether the signals parameter actually should be required.
It is used in `CTxMemPool` methods, so should be required, no? Or are you saying these classes should be given the option to not be sending signals in the first place? In that case, I don't see what the difference between the mempool and the other validation classes is. The signals could be optional in the chainman too.
📝 hebasto opened a pull request: "depends: Ensure definitions are passed when building SQLite with `DEBUG=1`"
(https://github.com/bitcoin/bitcoin/pull/29282)
The SQLite build system overrides the `CFLAGS` when is configured with the `--enable-debug` option.
Amends https://github.com/bitcoin/bitcoin/pull/25987.
Otherwise, for example in OSS-Fuzz, the testing binaries diverge from release ones.
(https://github.com/bitcoin/bitcoin/pull/29282)
The SQLite build system overrides the `CFLAGS` when is configured with the `--enable-debug` option.
Amends https://github.com/bitcoin/bitcoin/pull/25987.
Otherwise, for example in OSS-Fuzz, the testing binaries diverge from release ones.
💬 ryanofsky commented on pull request "kernel: Remove dependency on CScheduler":
(https://github.com/bitcoin/bitcoin/pull/28960#discussion_r1458922669)
re: https://github.com/bitcoin/bitcoin/pull/28960#discussion_r1458838935
> The signals could be optional in the chainman too.
Yes, absolutely. The question is in cases where libbitcoinkernel applications don't care about receiving signals (if maybe just they are just validating a block), should they be forced to declare `ValidationSignals validation_signals{std::make_unique<util::ImmediateTaskRunner>()};` and pass a `validation_signals` parameter to the ChainstateManager object that won't
...
(https://github.com/bitcoin/bitcoin/pull/28960#discussion_r1458922669)
re: https://github.com/bitcoin/bitcoin/pull/28960#discussion_r1458838935
> The signals could be optional in the chainman too.
Yes, absolutely. The question is in cases where libbitcoinkernel applications don't care about receiving signals (if maybe just they are just validating a block), should they be forced to declare `ValidationSignals validation_signals{std::make_unique<util::ImmediateTaskRunner>()};` and pass a `validation_signals` parameter to the ChainstateManager object that won't
...
💬 sipa commented on issue "Post startup stalling":
(https://github.com/bitcoin/bitcoin/issues/29281#issuecomment-1900333798)
Discussed this briefly with @mzumsande.
The stalling detection logic is ineffective here, because that only triggers when the 1024-block wide download window cannot move, and as you're less than 1024 blocks behind, the window is already as far as it can go.
We should probably have a separate criterion for stalling detection in this case.
(https://github.com/bitcoin/bitcoin/issues/29281#issuecomment-1900333798)
Discussed this briefly with @mzumsande.
The stalling detection logic is ineffective here, because that only triggers when the 1024-block wide download window cannot move, and as you're less than 1024 blocks behind, the window is already as far as it can go.
We should probably have a separate criterion for stalling detection in this case.
💬 TheCharlatan commented on pull request "depends: add NM output to gen_id":
(https://github.com/bitcoin/bitcoin/pull/29249#issuecomment-1900352612)
Yeah, also built on the wrong commit, so:
Re-ACK 6ec2813cd88d5f0b955d746e4711a8ad256ea47f
Guix build (x86):
```
67d1d5dd27bc7b2df6b5d77265a9be0c5e3d7cf0bd003b8ec177010c18526814 guix-build-6ec2813cd88d/output/aarch64-linux-gnu/SHA256SUMS.part
59d2f605151251ee05e9c8b043ead799254c0b61da513df8ba4b9adb8c3dd619 guix-build-6ec2813cd88d/output/aarch64-linux-gnu/bitcoin-6ec2813cd88d-aarch64-linux-gnu-debug.tar.gz
d299e058786d3db9e145ea692b4e90b9ce8a60ec16d101e475e8b2927b53f5ac guix-build-6ec28
...
(https://github.com/bitcoin/bitcoin/pull/29249#issuecomment-1900352612)
Yeah, also built on the wrong commit, so:
Re-ACK 6ec2813cd88d5f0b955d746e4711a8ad256ea47f
Guix build (x86):
```
67d1d5dd27bc7b2df6b5d77265a9be0c5e3d7cf0bd003b8ec177010c18526814 guix-build-6ec2813cd88d/output/aarch64-linux-gnu/SHA256SUMS.part
59d2f605151251ee05e9c8b043ead799254c0b61da513df8ba4b9adb8c3dd619 guix-build-6ec2813cd88d/output/aarch64-linux-gnu/bitcoin-6ec2813cd88d-aarch64-linux-gnu-debug.tar.gz
d299e058786d3db9e145ea692b4e90b9ce8a60ec16d101e475e8b2927b53f5ac guix-build-6ec28
...
💬 1440000bytes commented on pull request "set `DEFAULT_PERMIT_BAREMULTISIG` to false":
(https://github.com/bitcoin/bitcoin/pull/28217#issuecomment-1900364519)
> This isn't Twitter, if you want to participate here make an effort to read everything through and through.
Exactly, this isn't twitter so a detailed explanation with the ACK helps everyone and adds more weight to your comment.
(https://github.com/bitcoin/bitcoin/pull/28217#issuecomment-1900364519)
> This isn't Twitter, if you want to participate here make an effort to read everything through and through.
Exactly, this isn't twitter so a detailed explanation with the ACK helps everyone and adds more weight to your comment.
👍 TheCharlatan approved a pull request: "refactor: remove CTxMemPool::queryHashes()"
(https://github.com/bitcoin/bitcoin/pull/29260#pullrequestreview-1832517919)
ACK a30af58d26ec98350ae273ad10793a12ad71cff6
(https://github.com/bitcoin/bitcoin/pull/29260#pullrequestreview-1832517919)
ACK a30af58d26ec98350ae273ad10793a12ad71cff6
💬 moonsettler commented on pull request "Implement OP_CHECKTEMPLATEVERIFY":
(https://github.com/bitcoin/bitcoin/pull/29280#issuecomment-1900407815)
Concept ACK!
(https://github.com/bitcoin/bitcoin/pull/29280#issuecomment-1900407815)
Concept ACK!
💬 moonsettler commented on pull request "Implement OP_CHECKSIGFROMSTACK(VERIFY)":
(https://github.com/bitcoin/bitcoin/pull/29270#issuecomment-1900408113)
Concept ACK!
(https://github.com/bitcoin/bitcoin/pull/29270#issuecomment-1900408113)
Concept ACK!
💬 moonsettler commented on pull request "Add OP_INTERNALKEY for Tapscript":
(https://github.com/bitcoin/bitcoin/pull/29269#issuecomment-1900408304)
Concept ACK!
(https://github.com/bitcoin/bitcoin/pull/29269#issuecomment-1900408304)
Concept ACK!
🚀 fanquake merged a pull request: "depends: add NM output to gen_id"
(https://github.com/bitcoin/bitcoin/pull/29249)
(https://github.com/bitcoin/bitcoin/pull/29249)