💬 sipa commented on pull request "Limit retries in GetRNDRRS to avoid infinite loop":
(https://github.com/bitcoin/bitcoin/pull/31826#discussion_r1947836644)
When this fails, do we want to fall back to `GetRNSR` instead, rather than returning whatever arbitrary value was left in the register?
(https://github.com/bitcoin/bitcoin/pull/31826#discussion_r1947836644)
When this fails, do we want to fall back to `GetRNSR` instead, rather than returning whatever arbitrary value was left in the register?
💬 Christewart commented on pull request "test: Add `leaf_version` parameter to `taproot_tree_helper()`":
(https://github.com/bitcoin/bitcoin/pull/29371#issuecomment-2645752238)
Rebased
(https://github.com/bitcoin/bitcoin/pull/29371#issuecomment-2645752238)
Rebased
💬 Christewart commented on pull request "consensus: Consistently encode and decode `OP_1NEGATE` similar to other small ints in Script":
(https://github.com/bitcoin/bitcoin/pull/29589#issuecomment-2645753696)
Rebased
(https://github.com/bitcoin/bitcoin/pull/29589#issuecomment-2645753696)
Rebased
💬 maflcko commented on pull request "build, ci: Fix linking `bitcoin-chainstate.exe` to `bitcoinkernel.dll` on Windows":
(https://github.com/bitcoin/bitcoin/pull/31158#issuecomment-2645774491)
> Give `cs_main` hidden visiblility and add a `get_csmain()` getter function for bitcon-chainstate to use. That would work but would mean that bitcoin-chainstate's code would diverge from the rest of the codebase. (assuming we didn't do a global replacement, which would be a HUGE hammer)
(Probably out-of-scope for this pull, but there is already a getter for cs_main: `chainman.GetMutex()`, which can be used today, and can make it easier to turn cs_main from a global variable into a member fie
...
(https://github.com/bitcoin/bitcoin/pull/31158#issuecomment-2645774491)
> Give `cs_main` hidden visiblility and add a `get_csmain()` getter function for bitcon-chainstate to use. That would work but would mean that bitcoin-chainstate's code would diverge from the rest of the codebase. (assuming we didn't do a global replacement, which would be a HUGE hammer)
(Probably out-of-scope for this pull, but there is already a getter for cs_main: `chainman.GetMutex()`, which can be used today, and can make it easier to turn cs_main from a global variable into a member fie
...
✅ maflcko closed a pull request: "fixing-link-Update dependencies.md"
(https://github.com/bitcoin/bitcoin/pull/31822)
(https://github.com/bitcoin/bitcoin/pull/31822)
💬 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).