💬 laanwj commented on pull request "build: Switch to Qt 6":
(https://github.com/bitcoin/bitcoin/pull/30997#discussion_r1939445814)
```
# collection of types that map to MIME and file types
```
(https://github.com/bitcoin/bitcoin/pull/30997#discussion_r1939445814)
```
# collection of types that map to MIME and file types
```
💬 laanwj commented on pull request "build: Switch to Qt 6":
(https://github.com/bitcoin/bitcoin/pull/30997#discussion_r1939481222)
```python
'api-ms-win-core-synch-l1-2-0.dll', # Synchronization Primitives API
'api-ms-win-core-winrt-l1-1-0.dll', # Windows Runtime API
'api-ms-win-core-winrt-string-l1-1-0.dll', # WinRT String API
'AUTHZ.dll', # Windows Authorization Framework
'comdlg32.dll', # Common Dialog Box Library
'd3d11.dll', # Direct3D 11 API
'd3d12.dll', # Direct3D 12 API
'd3d9.dll', # Direct3D 9 API
'dwmapi.dll', # desktop window manager
'DWrite.dll', # DirectX Typography Services
'dxgi.dll', # DirectX Gr
...
(https://github.com/bitcoin/bitcoin/pull/30997#discussion_r1939481222)
```python
'api-ms-win-core-synch-l1-2-0.dll', # Synchronization Primitives API
'api-ms-win-core-winrt-l1-1-0.dll', # Windows Runtime API
'api-ms-win-core-winrt-string-l1-1-0.dll', # WinRT String API
'AUTHZ.dll', # Windows Authorization Framework
'comdlg32.dll', # Common Dialog Box Library
'd3d11.dll', # Direct3D 11 API
'd3d12.dll', # Direct3D 12 API
'd3d9.dll', # Direct3D 9 API
'dwmapi.dll', # desktop window manager
'DWrite.dll', # DirectX Typography Services
'dxgi.dll', # DirectX Gr
...
💬 hebasto commented on pull request "RFC: Riscv bare metal CI job":
(https://github.com/bitcoin/bitcoin/pull/31425#discussion_r1939486828)
I imagine that expanding this build scenario might enable building `boost` and `libevent` in depends, no?
If so, the Boost check should have another option to be disabled.
Additionally, mind considering this comment: https://github.com/bitcoin/bitcoin/blob/1172bc4157eefe80d1aaf0b56459857ec651e535/cmake/module/AddBoostIfNeeded.cmake#L7-L8
which, in turn, was based on this legacy code:https://github.com/bitcoin/bitcoin/blob/32efe850438ef22e2de39e562af557872a402c31/configure.ac#L1247-L1251
...
(https://github.com/bitcoin/bitcoin/pull/31425#discussion_r1939486828)
I imagine that expanding this build scenario might enable building `boost` and `libevent` in depends, no?
If so, the Boost check should have another option to be disabled.
Additionally, mind considering this comment: https://github.com/bitcoin/bitcoin/blob/1172bc4157eefe80d1aaf0b56459857ec651e535/cmake/module/AddBoostIfNeeded.cmake#L7-L8
which, in turn, was based on this legacy code:https://github.com/bitcoin/bitcoin/blob/32efe850438ef22e2de39e562af557872a402c31/configure.ac#L1247-L1251
...
💬 Sjors commented on pull request "Add waitNext() to BlockTemplate interface":
(https://github.com/bitcoin/bitcoin/pull/31283#discussion_r1939503331)
@vasild it's possible to start the node and quickly shut it down. If that happens then `m_tip_block` remains `null`.
If you call `createNewBlock()` during that time I'm not sure what happens. I think it's best to add an extra guard there. The Template Provider calls `waitTipChanged` before calling `createNewBlock()`, but not every interface user might do that.
With that in place you can't get here.
(https://github.com/bitcoin/bitcoin/pull/31283#discussion_r1939503331)
@vasild it's possible to start the node and quickly shut it down. If that happens then `m_tip_block` remains `null`.
If you call `createNewBlock()` during that time I'm not sure what happens. I think it's best to add an extra guard there. The Template Provider calls `waitTipChanged` before calling `createNewBlock()`, but not every interface user might do that.
With that in place you can't get here.
💬 brunoerg commented on pull request "miniscript: account for all `StringType` variants in `Miniscriptdescriptor::ToString()`":
(https://github.com/bitcoin/bitcoin/pull/31734#issuecomment-2631212572)
Concept ACK
(https://github.com/bitcoin/bitcoin/pull/31734#issuecomment-2631212572)
Concept ACK
💬 hebasto commented on pull request "build: Switch to Qt 6":
(https://github.com/bitcoin/bitcoin/pull/30997#discussion_r1939513224)
Right. From https://formulae.brew.sh/formula/qt:
> Also known as: **qt6**, **qt@6**
(https://github.com/bitcoin/bitcoin/pull/30997#discussion_r1939513224)
Right. From https://formulae.brew.sh/formula/qt:
> Also known as: **qt6**, **qt@6**
📝 Sjors opened a pull request: "Have createNewBlock() ensure m_tip_block is set"
(https://github.com/bitcoin/bitcoin/pull/31785)
This allows the proposed `waitNext()` method in https://github.com/bitcoin/bitcoin/pull/31283 to safely assume `TipBlock()` isn't `null`, not even during a scenario of early shutdown.
(https://github.com/bitcoin/bitcoin/pull/31785)
This allows the proposed `waitNext()` method in https://github.com/bitcoin/bitcoin/pull/31283 to safely assume `TipBlock()` isn't `null`, not even during a scenario of early shutdown.
💬 Sjors commented on pull request "Add waitNext() to BlockTemplate interface":
(https://github.com/bitcoin/bitcoin/pull/31283#discussion_r1939522785)
Opened #31785 for this check, though I could include it here.
(https://github.com/bitcoin/bitcoin/pull/31283#discussion_r1939522785)
Opened #31785 for this check, though I could include it here.
✅ fanquake closed an issue: "multiprocess: build failure on Alpine with depends & `DEBUG=1`"
(https://github.com/bitcoin/bitcoin/issues/31455)
(https://github.com/bitcoin/bitcoin/issues/31455)
💬 l0rinc commented on pull request "Have createNewBlock() ensure m_tip_block is set":
(https://github.com/bitcoin/bitcoin/pull/31785#discussion_r1939533024)
If it's not too difficult, can we reproduce this with a test?
(https://github.com/bitcoin/bitcoin/pull/31785#discussion_r1939533024)
If it's not too difficult, can we reproduce this with a test?
💬 Sjors commented on pull request "Have createNewBlock() ensure m_tip_block is set":
(https://github.com/bitcoin/bitcoin/pull/31785#issuecomment-2631258942)
The Stratum v2 Template Provider that I implemented first calls `mining.waitTipChanged(uint256::ZERO)` before calling `mining.createNewBlock()`. This ensures that now matter we order the node & IPC startup sequence, it won't prematurely try to make a block.
See https://github.com/Sjors/bitcoin/pull/49 (`src/sv2/template_provider.cpp` in `Sv2TemplateProvider::ThreadSv2Handler()`).
But other implementers may not realise this, so having `createNewBlock()` return nothing when called too early
...
(https://github.com/bitcoin/bitcoin/pull/31785#issuecomment-2631258942)
The Stratum v2 Template Provider that I implemented first calls `mining.waitTipChanged(uint256::ZERO)` before calling `mining.createNewBlock()`. This ensures that now matter we order the node & IPC startup sequence, it won't prematurely try to make a block.
See https://github.com/Sjors/bitcoin/pull/49 (`src/sv2/template_provider.cpp` in `Sv2TemplateProvider::ThreadSv2Handler()`).
But other implementers may not realise this, so having `createNewBlock()` return nothing when called too early
...
💬 Sjors commented on pull request "Have createNewBlock() ensure m_tip_block is set":
(https://github.com/bitcoin/bitcoin/pull/31785#discussion_r1939540523)
This depends on the node initialization and shutdown sequence. I'm not sure how to reproduce that in a test. And we might change that sequence in a way that this never happens in the first place.
(https://github.com/bitcoin/bitcoin/pull/31785#discussion_r1939540523)
This depends on the node initialization and shutdown sequence. I'm not sure how to reproduce that in a test. And we might change that sequence in a way that this never happens in the first place.
💬 maflcko commented on pull request "Have createNewBlock() ensure m_tip_block is set":
(https://github.com/bitcoin/bitcoin/pull/31785#discussion_r1939572655)
Would be good to update the docs, because `CreateNewBlock` never returns null and always returns a block.
(https://github.com/bitcoin/bitcoin/pull/31785#discussion_r1939572655)
Would be good to update the docs, because `CreateNewBlock` never returns null and always returns a block.
👍 danielabrozzoni approved a pull request: "test: deduplicates p2p_tx_download constants"
(https://github.com/bitcoin/bitcoin/pull/31758#pullrequestreview-2590273881)
tACK 2b94e1abbfdef058546221d63f53b0b58eea22ea
The code looks good to me, I think renaming `MAX_GETDATA_IN_FLIGHT` to `MAX_PEER_TX_REQUEST_IN_FLIGHT` makes it easier to refer back to txdownloadman
(https://github.com/bitcoin/bitcoin/pull/31758#pullrequestreview-2590273881)
tACK 2b94e1abbfdef058546221d63f53b0b58eea22ea
The code looks good to me, I think renaming `MAX_GETDATA_IN_FLIGHT` to `MAX_PEER_TX_REQUEST_IN_FLIGHT` makes it easier to refer back to txdownloadman
📝 0xf58ce opened a pull request: "list 6467600: 334knUNdf1FSCDvrYsmK3N4uGv3nJnj7a3"
(https://github.com/bitcoin/bitcoin/pull/31786)
<!--
*** Please remove the following help text before submitting: ***
Pull requests without a rationale and clear improvement may be closed
immediately.
GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->
<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:
* Any test improvements or new tests that improv
...
(https://github.com/bitcoin/bitcoin/pull/31786)
<!--
*** Please remove the following help text before submitting: ***
Pull requests without a rationale and clear improvement may be closed
immediately.
GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->
<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:
* Any test improvements or new tests that improv
...
✅ fanquake closed a pull request: "list 6467600: 334knUNdf1FSCDvrYsmK3N4uGv3nJnj7a3"
(https://github.com/bitcoin/bitcoin/pull/31786)
(https://github.com/bitcoin/bitcoin/pull/31786)
📝 fanquake locked a pull request: "list 6467600: 334knUNdf1FSCDvrYsmK3N4uGv3nJnj7a3"
(https://github.com/bitcoin/bitcoin/pull/31786)
<!--
*** Please remove the following help text before submitting: ***
Pull requests without a rationale and clear improvement may be closed
immediately.
GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->
<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:
* Any test improvements or new tests that improv
...
(https://github.com/bitcoin/bitcoin/pull/31786)
<!--
*** Please remove the following help text before submitting: ***
Pull requests without a rationale and clear improvement may be closed
immediately.
GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->
<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:
* Any test improvements or new tests that improv
...
💬 hebasto commented on pull request "cmake: Fix `-pthread` flags in summary":
(https://github.com/bitcoin/bitcoin/pull/31724#discussion_r1939606312)
> I agree, but in that situation we would be back to printing generator expressions in the flags summary...
Unfortunately, it happens not only with unknown future changes but also with older yet supported CMake versions, such as 3.25, where `$<$<NOT:$<COMPILE_LANG_AND_ID:CUDA,NVIDIA>>:-pthread>` is printed.
(https://github.com/bitcoin/bitcoin/pull/31724#discussion_r1939606312)
> I agree, but in that situation we would be back to printing generator expressions in the flags summary...
Unfortunately, it happens not only with unknown future changes but also with older yet supported CMake versions, such as 3.25, where `$<$<NOT:$<COMPILE_LANG_AND_ID:CUDA,NVIDIA>>:-pthread>` is printed.
👍 willcl-ark approved a pull request: "lint: Call more checks from test_runner"
(https://github.com/bitcoin/bitcoin/pull/31653#pullrequestreview-2590369181)
tACK faf8fc5487d409eeff7b7b260eabb6929a7b7a5f
Makes sense to move this into test_runner.
Worked correctly for me, although I did forget that we currently can't run the linter (and local CI) in a worktree, which I was using.
I'll try and open a PR to fix that shortly.
(https://github.com/bitcoin/bitcoin/pull/31653#pullrequestreview-2590369181)
tACK faf8fc5487d409eeff7b7b260eabb6929a7b7a5f
Makes sense to move this into test_runner.
Worked correctly for me, although I did forget that we currently can't run the linter (and local CI) in a worktree, which I was using.
I'll try and open a PR to fix that shortly.
💬 maflcko commented on pull request "Pass custom DEP_OPTS and CONFIG_FLAGS to guix-build":
(https://github.com/bitcoin/bitcoin/pull/31763#issuecomment-2631420290)
It seems a bit confusing to leak the env into guix, which may then cause non-determinism, compiling the same source commit id. It seems simpler to just create a commit with your modifications to the config.
(https://github.com/bitcoin/bitcoin/pull/31763#issuecomment-2631420290)
It seems a bit confusing to leak the env into guix, which may then cause non-determinism, compiling the same source commit id. It seems simpler to just create a commit with your modifications to the config.
💬 maflcko commented on pull request "lint: Call more checks from test_runner":
(https://github.com/bitcoin/bitcoin/pull/31653#issuecomment-2631425896)
> can't run the linter (and local CI) in a worktree
Jup, see https://github.com/bitcoin/bitcoin/issues/30028 for the CI part.
(https://github.com/bitcoin/bitcoin/pull/31653#issuecomment-2631425896)
> can't run the linter (and local CI) in a worktree
Jup, see https://github.com/bitcoin/bitcoin/issues/30028 for the CI part.