💬 maflcko commented on pull request "fixing-link-Update dependencies.md":
(https://github.com/bitcoin/bitcoin/pull/31822#issuecomment-2645783216)
I wouldn't mind the change and would be happy to review it, but given the two conflicts https://github.com/bitcoin/bitcoin/pull/31822#issuecomment-2643522687 (one which also rewords the file, and another one which removes BDB and the line changed in this pull request anyway), I'll be closing this for now. Thanks.
(https://github.com/bitcoin/bitcoin/pull/31822#issuecomment-2645783216)
I wouldn't mind the change and would be happy to review it, but given the two conflicts https://github.com/bitcoin/bitcoin/pull/31822#issuecomment-2643522687 (one which also rewords the file, and another one which removes BDB and the line changed in this pull request anyway), I'll be closing this for now. Thanks.
💬 tofutim commented on issue "Unable to cross compile on linux for macos (28.x branch)":
(https://github.com/bitcoin/bitcoin/issues/31027#issuecomment-2645915233)
> Update: re-downloaded Xcode and re-extracted the SDK, guix build is proceeding again
Did it work after re-extracting the SDK? I had no luck with 28.1. Trying 27.2 now.
(https://github.com/bitcoin/bitcoin/issues/31027#issuecomment-2645915233)
> Update: re-downloaded Xcode and re-extracted the SDK, guix build is proceeding again
Did it work after re-extracting the SDK? I had no luck with 28.1. Trying 27.2 now.
💬 fjahr commented on pull request "doc: swap CPPFLAGS for APPEND_CPPFLAGS":
(https://github.com/bitcoin/bitcoin/pull/31819#issuecomment-2645930017)
ACK ea687d202934ee9aa26912cda21993da219cd418
Just used `APPEND_CPPFLAGS` yesterday for some debugging, so it works :)
(https://github.com/bitcoin/bitcoin/pull/31819#issuecomment-2645930017)
ACK ea687d202934ee9aa26912cda21993da219cd418
Just used `APPEND_CPPFLAGS` yesterday for some debugging, so it works :)
💬 luke-jr commented on issue "Duplicate coinbase transaction space reservation in CreateNewBlock":
(https://github.com/bitcoin/bitcoin/issues/21950#issuecomment-2645974712)
I suppose the ideal would be to create an actual generation transaction that fills the reserved weight/sigops, so the block validity check picks up on any error.
But blocks are already way too big, so I don't consider this a priority. Better to play it safe. The pennies unclaimed are nothing compared to the loss of an entire invalid block.
(https://github.com/bitcoin/bitcoin/issues/21950#issuecomment-2645974712)
I suppose the ideal would be to create an actual generation transaction that fills the reserved weight/sigops, so the block validity check picks up on any error.
But blocks are already way too big, so I don't consider this a priority. Better to play it safe. The pennies unclaimed are nothing compared to the loss of an entire invalid block.
🤔 rishkwal reviewed a pull request: "tests: Add witness commitment if we have a witness transaction in `FullBlockTest.update_block()`"
(https://github.com/bitcoin/bitcoin/pull/31823#pullrequestreview-2604068209)
Hi, looking at `feature_block.py` — it doesn't seem to contain any SegWit-specific validation tests. They remain in `feature_segwit.py`, `p2p_segwit.py`, etc; although I'm not completely confident about the scope of this file.
I have a couple of questions(avoid if irrelevant):
1. Is there any test case that can be covered in the current implementation that needs this change to be implemented?
2. Wouldn't it be better to create a new test file like `feature_<soft_fork>.py` to test the consensu
...
(https://github.com/bitcoin/bitcoin/pull/31823#pullrequestreview-2604068209)
Hi, looking at `feature_block.py` — it doesn't seem to contain any SegWit-specific validation tests. They remain in `feature_segwit.py`, `p2p_segwit.py`, etc; although I'm not completely confident about the scope of this file.
I have a couple of questions(avoid if irrelevant):
1. Is there any test case that can be covered in the current implementation that needs this change to be implemented?
2. Wouldn't it be better to create a new test file like `feature_<soft_fork>.py` to test the consensu
...
🤔 hodlinator reviewed a pull request: "qa: Fix `wallet_multiwallet.py`"
(https://github.com/bitcoin/bitcoin/pull/31410#pullrequestreview-2604068753)
Code review e717fb5a429d9d00e008fa1eb2b85179050be8fe
### Guix cross-build failure
Attempted to run Guix build e717fb5a429d9d00e008fa1eb2b85179050be8fe cross-built for Windows, but it fails already on test "0" / line 133:
https://github.com/bitcoin/bitcoin/blob/e717fb5a429d9d00e008fa1eb2b85179050be8fe/test/functional/wallet_multiwallet.py#L129-L133
<details><summary>Log</summary>
```
$ BITCOIND=/c/Users/hodlinator/Downloads/bitcoind-e717fb5a429d.exe build/test/functional/wallet_mu
...
(https://github.com/bitcoin/bitcoin/pull/31410#pullrequestreview-2604068753)
Code review e717fb5a429d9d00e008fa1eb2b85179050be8fe
### Guix cross-build failure
Attempted to run Guix build e717fb5a429d9d00e008fa1eb2b85179050be8fe cross-built for Windows, but it fails already on test "0" / line 133:
https://github.com/bitcoin/bitcoin/blob/e717fb5a429d9d00e008fa1eb2b85179050be8fe/test/functional/wallet_multiwallet.py#L129-L133
<details><summary>Log</summary>
```
$ BITCOIND=/c/Users/hodlinator/Downloads/bitcoind-e717fb5a429d.exe build/test/functional/wallet_mu
...
💬 onlinesipahimithu commented on pull request "Add bitcoin-{node,gui} to release binaries for IPC":
(https://github.com/bitcoin/bitcoin/pull/31802#issuecomment-2646012270)
What is and how
(https://github.com/bitcoin/bitcoin/pull/31802#issuecomment-2646012270)
What is and how
🤔 onlinesipahimithu reviewed a pull request: "Add bitcoin-{node,gui} to release binaries for IPC"
(https://github.com/bitcoin/bitcoin/pull/31802#pullrequestreview-2604083934)
GOOD lesson from
(https://github.com/bitcoin/bitcoin/pull/31802#pullrequestreview-2604083934)
GOOD lesson from
💬 onlinesipahimithu commented on pull request "Remove the legacy wallet and BDB dependency":
(https://github.com/bitcoin/bitcoin/pull/28710#issuecomment-2646012743)
Helli how to develop
(https://github.com/bitcoin/bitcoin/pull/28710#issuecomment-2646012743)
Helli how to develop
⚠️ onlinesipahimithu opened an issue: "Hellobtc"
(https://github.com/bitcoin/bitcoin/issues/31828)
I trying for learn how works crypto
(https://github.com/bitcoin/bitcoin/issues/31828)
I trying for learn how works crypto
✅ pinheadmz closed an issue: "Hellobtc"
(https://github.com/bitcoin/bitcoin/issues/31828)
(https://github.com/bitcoin/bitcoin/issues/31828)
💬 eval-exec commented on pull request "Limit retries in GetRNDRRS to avoid infinite loop":
(https://github.com/bitcoin/bitcoin/pull/31826#discussion_r1947992696)
Agree, I prefer fallback to `GetRNSR`.
(https://github.com/bitcoin/bitcoin/pull/31826#discussion_r1947992696)
Agree, I prefer fallback to `GetRNSR`.
💬 eval-exec commented on pull request "Limit retries in GetRNDRRS to avoid infinite loop":
(https://github.com/bitcoin/bitcoin/pull/31826#issuecomment-2646035608)
> it would be better to instead investigate and fix the feature test or feature reporting.
Agree, it need time to invistigate this bug.
And I did some research, this CPU have bug on `rndrrs`:
> [04:28](https://oftc.irclog.whitequark.org/aarch64-laptops/2024-07-21#33392883) <HdkR> I found out yesterday that the bitcoin project uses rndrrs if it is available on aarch64 to fill its entropy pool. It spins in a loop until its pool is filled.
> [04:28](https://oftc.irclog.whitequark.org/aarch6
...
(https://github.com/bitcoin/bitcoin/pull/31826#issuecomment-2646035608)
> it would be better to instead investigate and fix the feature test or feature reporting.
Agree, it need time to invistigate this bug.
And I did some research, this CPU have bug on `rndrrs`:
> [04:28](https://oftc.irclog.whitequark.org/aarch64-laptops/2024-07-21#33392883) <HdkR> I found out yesterday that the bitcoin project uses rndrrs if it is available on aarch64 to fill its entropy pool. It spins in a loop until its pool is filled.
> [04:28](https://oftc.irclog.whitequark.org/aarch6
...
💬 eval-exec commented on pull request "Limit retries in GetRNDRRS to avoid infinite loop":
(https://github.com/bitcoin/bitcoin/pull/31826#discussion_r1947993250)
Maybe should fallback to `GetRNDR`?
(https://github.com/bitcoin/bitcoin/pull/31826#discussion_r1947993250)
Maybe should fallback to `GetRNDR`?
💬 sipa commented on pull request "Limit retries in GetRNDRRS to avoid infinite loop":
(https://github.com/bitcoin/bitcoin/pull/31826#discussion_r1947994381)
I think it would be even better to do a trial run inside `InitHardwareRand`, so this condition can be detected, and the feature can be considered unsupported (so future calls to GetRNDRS will not even bother).
(https://github.com/bitcoin/bitcoin/pull/31826#discussion_r1947994381)
I think it would be even better to do a trial run inside `InitHardwareRand`, so this condition can be detected, and the feature can be considered unsupported (so future calls to GetRNDRS will not even bother).
💬 theStack commented on pull request "contrib: fix `test_deterministic_coverage.sh` script for out-of-tree builds":
(https://github.com/bitcoin/bitcoin/pull/31588#issuecomment-2646119824)
@fjahr: Note that `GCOV_EXECUTABLE` is hardcoded in the script currently, so setting it as environment variable won't have any effect. Can you try again by modifying it directly in the file? It's not very user friendly that this is necessary, can add a commit that allows passing it via environment, if reviewers feel that this is still in scope of this PR.
(https://github.com/bitcoin/bitcoin/pull/31588#issuecomment-2646119824)
@fjahr: Note that `GCOV_EXECUTABLE` is hardcoded in the script currently, so setting it as environment variable won't have any effect. Can you try again by modifying it directly in the file? It's not very user friendly that this is necessary, can add a commit that allows passing it via environment, if reviewers feel that this is still in scope of this PR.
💬 purpleKarrot commented on something "":
(https://github.com/bitcoin/bitcoin/commit/332655cb52c8f8ef64b29b09e38ef5d61235ed21#r152324998)
Instead of setting so many different variables, why not use variables that are already set by CMake?
The `DESCRIPTION` that is passed to the `project()` command sets the variables `PROJECT_DESCRIPTION` and `BitcoinCore_DESCRIPTION`. Neither of the two variables are used anywhere, so maybe "Bitcoin Core" should be passed as the `DESCRIPTION` argument and then `PROJECT_DESCRIPTION` may be used in places where `PACKAGE_NAME` is used at the moment?
Likewise, the `VERSION` that is passed to `pr
...
(https://github.com/bitcoin/bitcoin/commit/332655cb52c8f8ef64b29b09e38ef5d61235ed21#r152324998)
Instead of setting so many different variables, why not use variables that are already set by CMake?
The `DESCRIPTION` that is passed to the `project()` command sets the variables `PROJECT_DESCRIPTION` and `BitcoinCore_DESCRIPTION`. Neither of the two variables are used anywhere, so maybe "Bitcoin Core" should be passed as the `DESCRIPTION` argument and then `PROJECT_DESCRIPTION` may be used in places where `PACKAGE_NAME` is used at the moment?
Likewise, the `VERSION` that is passed to `pr
...
💬 katesalazar commented on pull request "Updated MacOS icon to more closely fit Apple's design standards":
(https://github.com/bitcoin-core/gui/pull/852#issuecomment-2646204166)
As an ex macOS user, all I see here now is bytes.
(https://github.com/bitcoin-core/gui/pull/852#issuecomment-2646204166)
As an ex macOS user, all I see here now is bytes.
💬 sipa commented on pull request "Limit retries in GetRNDRRS to avoid infinite loop":
(https://github.com/bitcoin/bitcoin/pull/31826#discussion_r1948122100)
Nit: perhaps just say "Using RNDR as additional entropy source" here, instead of claiming that RNDRRS is used, but then disabled?
(https://github.com/bitcoin/bitcoin/pull/31826#discussion_r1948122100)
Nit: perhaps just say "Using RNDR as additional entropy source" here, instead of claiming that RNDRRS is used, but then disabled?
💬 sipa commented on pull request "Limit retries in GetRNDRRS to avoid infinite loop":
(https://github.com/bitcoin/bitcoin/pull/31826#discussion_r1948121952)
You'll also need to actually use this variable inside `GetRNDRRS()`, to fall back to `GetRNDR()` instead.
(https://github.com/bitcoin/bitcoin/pull/31826#discussion_r1948121952)
You'll also need to actually use this variable inside `GetRNDRRS()`, to fall back to `GetRNDR()` instead.