🚀 achow101 merged a pull request: "random: Check `GetRNDRRS` is supported in `InitHardwareRand` to avoid infinite loop"
(https://github.com/bitcoin/bitcoin/pull/31826)
(https://github.com/bitcoin/bitcoin/pull/31826)
💬 davidgumberg commented on pull request "guix: Notarize MacOS app bundle and codesign all MacOS and Windows binaries":
(https://github.com/bitcoin/bitcoin/pull/31407#issuecomment-2667434518)
> > [bitcoin-096525e92cc2-arm64-apple-darwin.zip](https://github.com/user-attachments/files/18781660/bitcoin-096525e92cc2-arm64-apple-darwin.zip)
>
> Tested this on a Mac M1. I could download it flawlessly through Firefox and perform most of IBD.
>
> I tried downloading it through Safari and likewise other reviewers i encountered an error:
>
> The system language was set to French but i guess what matters here is the error ID: -47.
I don't know if this had the same cause as what you
...
(https://github.com/bitcoin/bitcoin/pull/31407#issuecomment-2667434518)
> > [bitcoin-096525e92cc2-arm64-apple-darwin.zip](https://github.com/user-attachments/files/18781660/bitcoin-096525e92cc2-arm64-apple-darwin.zip)
>
> Tested this on a Mac M1. I could download it flawlessly through Firefox and perform most of IBD.
>
> I tried downloading it through Safari and likewise other reviewers i encountered an error:
>
> The system language was set to French but i guess what matters here is the error ID: -47.
I don't know if this had the same cause as what you
...
✅ ariard closed a pull request: "Halt processing of unrequested transactions v2"
(https://github.com/bitcoin/bitcoin/pull/30572)
(https://github.com/bitcoin/bitcoin/pull/30572)
💬 ariard commented on pull request "Halt processing of unrequested transactions v2":
(https://github.com/bitcoin/bitcoin/pull/30572#issuecomment-2667460145)
Forwarded an update on the mailing list entitled “Transaction Relay V2” to give an overview on the topic.
I’ll go to close this draft PR, not because I think the problem has been solved, neither because I don’t think it’s a serious problem (I believe it’s a serious problem) though because I prefer to go to maintain my own full-node in Rust and fix the pointed transaction-relay issues there. So apart of reviewing the code changes for the consensus logic and finishing Erlay deployment, I don’t
...
(https://github.com/bitcoin/bitcoin/pull/30572#issuecomment-2667460145)
Forwarded an update on the mailing list entitled “Transaction Relay V2” to give an overview on the topic.
I’ll go to close this draft PR, not because I think the problem has been solved, neither because I don’t think it’s a serious problem (I believe it’s a serious problem) though because I prefer to go to maintain my own full-node in Rust and fix the pointed transaction-relay issues there. So apart of reviewing the code changes for the consensus logic and finishing Erlay deployment, I don’t
...
✅ ariard closed an issue: "[brainstorm] replacement cache to optimize miners block templates"
(https://github.com/bitcoin/bitcoin/issues/28699)
(https://github.com/bitcoin/bitcoin/issues/28699)
💬 ariard commented on issue "[brainstorm] replacement cache to optimize miners block templates":
(https://github.com/bitcoin/bitcoin/issues/28699#issuecomment-2667464456)
Closing this issue as I do not plan to contribute to Bitcoin Core anymore and I prefer to go to maintain my own full-node in Rust built on top of `libbitcoinkernel`, it is mature enough.
(https://github.com/bitcoin/bitcoin/issues/28699#issuecomment-2667464456)
Closing this issue as I do not plan to contribute to Bitcoin Core anymore and I prefer to go to maintain my own full-node in Rust built on top of `libbitcoinkernel`, it is mature enough.
💬 sipa commented on pull request "p2p: improve TxOrphanage denial of service bounds and increase -maxorphantxs":
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r1960805496)
In commit "[bench] TxOrphanage::EraseForBlock"
Better not to have years listed than outdated/wrong ones.
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r1960805496)
In commit "[bench] TxOrphanage::EraseForBlock"
Better not to have years listed than outdated/wrong ones.
💬 sipa commented on pull request "p2p: improve TxOrphanage denial of service bounds and increase -maxorphantxs":
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r1960809301)
In commit "[txorphanage] add per-peer iterator list and announcements accounting"
Nit: I may have instigated this, but given the `m_peer_orphanage_info.find(peer)` call, it's *O(log n)* really (in the number of peers, not *O(1)*).
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r1960809301)
In commit "[txorphanage] add per-peer iterator list and announcements accounting"
Nit: I may have instigated this, but given the `m_peer_orphanage_info.find(peer)` call, it's *O(log n)* really (in the number of peers, not *O(1)*).
💬 sipa commented on pull request "p2p: improve TxOrphanage denial of service bounds and increase -maxorphantxs":
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r1960932480)
Regarding https://github.com/bitcoin/bitcoin/pull/31829/commits/2771b69e461230feb7761b0afff41b44bd3ba34f#r1949554535
With `GetGlobalMaxUsage()` computing memory limits on-the-fly, is this comment still relevant?
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r1960932480)
Regarding https://github.com/bitcoin/bitcoin/pull/31829/commits/2771b69e461230feb7761b0afff41b44bd3ba34f#r1949554535
With `GetGlobalMaxUsage()` computing memory limits on-the-fly, is this comment still relevant?
💬 davidgumberg commented on pull request "fuzz: a new target for the coins database":
(https://github.com/bitcoin/bitcoin/pull/28216#discussion_r1960953920)
Why explicit conversion with `!!`? Doesn't assert cause contextual conversion to bool of the expression?
E.g.
https://godbolt.org/z/5s9688Thn
(https://github.com/bitcoin/bitcoin/pull/28216#discussion_r1960953920)
Why explicit conversion with `!!`? Doesn't assert cause contextual conversion to bool of the expression?
E.g.
https://godbolt.org/z/5s9688Thn
💬 achow101 commented on pull request "guix: use GCC 13 to build releases":
(https://github.com/bitcoin/bitcoin/pull/29881#issuecomment-2667525906)
ACK 0c1b29a05777256c5ee686fff60f281dfeae289c
```
b381ff87a34e45f693abee5554dd0680cb7879d66787f8aba45fb9dd149e8e78 guix-build-0c1b29a05777/output/aarch64-linux-gnu/SHA256SUMS.part
d3cb534b26ee5edd0c5d459cfffd3ebb46235da21ce2c614fda92d56f4b6ba98 guix-build-0c1b29a05777/output/aarch64-linux-gnu/bitcoin-0c1b29a05777-aarch64-linux-gnu-debug.tar.gz
b6b73fe6c3ae291e4c3d702a1343e44f3c143b50a566f1a0412dbfcf45fe31e2 guix-build-0c1b29a05777/output/aarch64-linux-gnu/bitcoin-0c1b29a05777-aarch64-lin
...
(https://github.com/bitcoin/bitcoin/pull/29881#issuecomment-2667525906)
ACK 0c1b29a05777256c5ee686fff60f281dfeae289c
```
b381ff87a34e45f693abee5554dd0680cb7879d66787f8aba45fb9dd149e8e78 guix-build-0c1b29a05777/output/aarch64-linux-gnu/SHA256SUMS.part
d3cb534b26ee5edd0c5d459cfffd3ebb46235da21ce2c614fda92d56f4b6ba98 guix-build-0c1b29a05777/output/aarch64-linux-gnu/bitcoin-0c1b29a05777-aarch64-linux-gnu-debug.tar.gz
b6b73fe6c3ae291e4c3d702a1343e44f3c143b50a566f1a0412dbfcf45fe31e2 guix-build-0c1b29a05777/output/aarch64-linux-gnu/bitcoin-0c1b29a05777-aarch64-lin
...
🚀 achow101 merged a pull request: "guix: use GCC 13 to build releases"
(https://github.com/bitcoin/bitcoin/pull/29881)
(https://github.com/bitcoin/bitcoin/pull/29881)
💬 davidgumberg commented on pull request "fuzz: a new target for the coins database":
(https://github.com/bitcoin/bitcoin/pull/28216#issuecomment-2667661548)
I let the fuzz binary run with `FUZZ=coins_db` for about an hour and nothing crashed.
And, I generated some corpora (https://github.com/davidgumberg/qa-assets/tree/coins_db_corpora) for ~20 minutes by doing
```console
$ ./build_fuzz/test/fuzz/test_runner.py -g qa-assets/fuzz_corpora/ coins_db
```
and used the deterministic fuzz coverage tool from #31836 but it found instability:
```console
$ RUST_BACKTRACE=1 cargo run --manifest-path ./contrib/devtools/deterministic-fuzz-covera
...
(https://github.com/bitcoin/bitcoin/pull/28216#issuecomment-2667661548)
I let the fuzz binary run with `FUZZ=coins_db` for about an hour and nothing crashed.
And, I generated some corpora (https://github.com/davidgumberg/qa-assets/tree/coins_db_corpora) for ~20 minutes by doing
```console
$ ./build_fuzz/test/fuzz/test_runner.py -g qa-assets/fuzz_corpora/ coins_db
```
and used the deterministic fuzz coverage tool from #31836 but it found instability:
```console
$ RUST_BACKTRACE=1 cargo run --manifest-path ./contrib/devtools/deterministic-fuzz-covera
...
💬 purpleKarrot commented on pull request "build: Make config warnings fatal if -DWCONFIGURE_ERROR=ON":
(https://github.com/bitcoin/bitcoin/pull/31665#issuecomment-2667723356)
Concept NACK.
Please don't add nonstandard cmake variables (here: `WCONFIGURE_ERROR=ON`) for things that have builtin support in CMake. Just issue warnings with the [`message`](https://cmake.org/cmake/help/latest/command/message.html) command:
```cmake
message(WARNING "There is some potential issue on the system, like a new version of a dependency that has not been tested yet.")
message(AUTHOR_WARNING "There is a potention issue in cmake code, like suspicious arguments to a custom functi
...
(https://github.com/bitcoin/bitcoin/pull/31665#issuecomment-2667723356)
Concept NACK.
Please don't add nonstandard cmake variables (here: `WCONFIGURE_ERROR=ON`) for things that have builtin support in CMake. Just issue warnings with the [`message`](https://cmake.org/cmake/help/latest/command/message.html) command:
```cmake
message(WARNING "There is some potential issue on the system, like a new version of a dependency that has not been tested yet.")
message(AUTHOR_WARNING "There is a potention issue in cmake code, like suspicious arguments to a custom functi
...
💬 maflcko commented on pull request "fuzz: a new target for the coins database":
(https://github.com/bitcoin/bitcoin/pull/28216#discussion_r1961110876)
Wouldn't it be shorter and clearer to write `assert(!!coins_view_cursor == is_db);`?
(https://github.com/bitcoin/bitcoin/pull/28216#discussion_r1961110876)
Wouldn't it be shorter and clearer to write `assert(!!coins_view_cursor == is_db);`?
💬 maflcko commented on pull request "refactor: Remove redundant and confusing calls to IsArgSet":
(https://github.com/bitcoin/bitcoin/pull/31896#discussion_r1961129692)
Not sure. It is not immediately actionable and probably opinionated or unclear, so best to remove. Though, I don't want to do it here. Maybe a follow-up pull could do it?
(https://github.com/bitcoin/bitcoin/pull/31896#discussion_r1961129692)
Not sure. It is not immediately actionable and probably opinionated or unclear, so best to remove. Though, I don't want to do it here. Maybe a follow-up pull could do it?
💬 Sjors commented on pull request "mining: drop unused -nFees and sigops from CBlockTemplate":
(https://github.com/bitcoin/bitcoin/pull/31897#discussion_r1961163057)
I forgot to drop this line in my previous push.
(https://github.com/bitcoin/bitcoin/pull/31897#discussion_r1961163057)
I forgot to drop this line in my previous push.
💬 maflcko commented on pull request "scripted-diff: Type-safe settings retrieval":
(https://github.com/bitcoin/bitcoin/pull/31260#discussion_r1961169935)
> Note this change would also be a change in behavior because `-addresstype=` could no longer be used to unset previous value
Why would that be? I haven't tested this, but from reading the code and the test, I get the impression that setting a value to an empty string will set the value to an empty string and not the default value.
Though, maybe I am confused with negation, which, according to the docs also claims to set a string to an empty string, even though I had the impression that it
...
(https://github.com/bitcoin/bitcoin/pull/31260#discussion_r1961169935)
> Note this change would also be a change in behavior because `-addresstype=` could no longer be used to unset previous value
Why would that be? I haven't tested this, but from reading the code and the test, I get the impression that setting a value to an empty string will set the value to an empty string and not the default value.
Though, maybe I am confused with negation, which, according to the docs also claims to set a string to an empty string, even though I had the impression that it
...
💬 Sjors commented on pull request "mining: drop unused -nFees and sigops from CBlockTemplate":
(https://github.com/bitcoin/bitcoin/pull/31897#issuecomment-2667869182)
`block.vtx` contains a dummy coinbase, `vTxFees` and `vTxSigOpsCost` contain dummy fees and dummy sigop costs for the coinbase. That seems more straight forward to me than having `vTxFees` be shorter and then having to document why these things have different lengths.
> Since the coinbase transaction does not have fees
That's almost a philosophical question, because miners can (and sometimes accidentally did) claim less fees than allowed. In a way such a coinbase has negative fees, which i
...
(https://github.com/bitcoin/bitcoin/pull/31897#issuecomment-2667869182)
`block.vtx` contains a dummy coinbase, `vTxFees` and `vTxSigOpsCost` contain dummy fees and dummy sigop costs for the coinbase. That seems more straight forward to me than having `vTxFees` be shorter and then having to document why these things have different lengths.
> Since the coinbase transaction does not have fees
That's almost a philosophical question, because miners can (and sometimes accidentally did) claim less fees than allowed. In a way such a coinbase has negative fees, which i
...
💬 Sjors commented on pull request "Have createNewBlock() wait for tip, make rpc handle shutdown during long poll and wait methods":
(https://github.com/bitcoin/bitcoin/pull/31785#issuecomment-2667879598)
> I think a dedicated PR like https://github.com/bitcoin/bitcoin/pull/30635 would be a better place to update the RPC interface, if that is what we want to do.
I'll rebase the latter PR on top of this, and then reduce the scope here.
(https://github.com/bitcoin/bitcoin/pull/31785#issuecomment-2667879598)
> I think a dedicated PR like https://github.com/bitcoin/bitcoin/pull/30635 would be a better place to update the RPC interface, if that is what we want to do.
I'll rebase the latter PR on top of this, and then reduce the scope here.