🚀 fanquake merged a pull request: "guix: remove test-security/symbol-check scripts"
(https://github.com/bitcoin/bitcoin/pull/31818)
(https://github.com/bitcoin/bitcoin/pull/31818)
✅ fanquake closed an issue: "contrib: Autoconf fragments left in test-*-check scripts"
(https://github.com/bitcoin/bitcoin/issues/31698)
(https://github.com/bitcoin/bitcoin/issues/31698)
💬 naiyoma commented on pull request "rpc: print P2WSH and P2SH redem Script in getrawtransaction":
(https://github.com/bitcoin/bitcoin/pull/31252#discussion_r1951583490)
the value of `have_undo` is always going to be 0 when calling` getrawtransaction` hence the reason why this only works for `getblock..2`
(https://github.com/bitcoin/bitcoin/pull/31252#discussion_r1951583490)
the value of `have_undo` is always going to be 0 when calling` getrawtransaction` hence the reason why this only works for `getblock..2`
💬 naiyoma commented on pull request "rpc: print P2WSH and P2SH redem Script in getrawtransaction":
(https://github.com/bitcoin/bitcoin/pull/31252#discussion_r1952236280)
This is not a suggestion to replace your current approach, but rather an alternative way. I have an implementation here ->[https://github.com/naiyoma/bitcoin/pull/1/commits/ff82e551404e55c75b1950e01e4f4626f5ad452e](https://github.com/naiyoma/bitcoin/pull/1/commits/ff82e551404e55c75b1950e01e4f4626f5ad452e) that shows how to decompile outside of `if(have_undo) `. While it needs more testing, it successfully works for both `getrawtransaction `and `getblock..2`. You might find some ideas there for
...
(https://github.com/bitcoin/bitcoin/pull/31252#discussion_r1952236280)
This is not a suggestion to replace your current approach, but rather an alternative way. I have an implementation here ->[https://github.com/naiyoma/bitcoin/pull/1/commits/ff82e551404e55c75b1950e01e4f4626f5ad452e](https://github.com/naiyoma/bitcoin/pull/1/commits/ff82e551404e55c75b1950e01e4f4626f5ad452e) that shows how to decompile outside of `if(have_undo) `. While it needs more testing, it successfully works for both `getrawtransaction `and `getblock..2`. You might find some ideas there for
...
💬 naiyoma commented on pull request "rpc: print P2WSH and P2SH redem Script in getrawtransaction":
(https://github.com/bitcoin/bitcoin/pull/31252#discussion_r1952168882)
this outputs `redeemScript` for both P2SH and P2WSH, but it should be
P2SH `redeemScript` and P2WSH `witnessScript`
(https://github.com/bitcoin/bitcoin/pull/31252#discussion_r1952168882)
this outputs `redeemScript` for both P2SH and P2WSH, but it should be
P2SH `redeemScript` and P2WSH `witnessScript`
💬 Sjors commented on pull request "guix: Notarize MacOS app bundle and codesign all MacOS and Windows binaries":
(https://github.com/bitcoin/bitcoin/pull/31407#issuecomment-2653056523)
I'm unable to apply the code signatures. E.g. for arm64:
```
HOSTS="arm64-apple-darwin" ./contrib/guix/guix-codesign
Checking that we can connect to the guix-daemon...
Hint: If this hangs, you may want to try turning your guix-daemon off and on
again.
INFO: Codesigning 096525e92cc2 for platform triple arm64-apple-darwin:
...using reference timestamp: 1733177891
...from worktree directory: '/home/sjors/bitcoin'
...bind-mounted in container to: '/bitcoin
...
(https://github.com/bitcoin/bitcoin/pull/31407#issuecomment-2653056523)
I'm unable to apply the code signatures. E.g. for arm64:
```
HOSTS="arm64-apple-darwin" ./contrib/guix/guix-codesign
Checking that we can connect to the guix-daemon...
Hint: If this hangs, you may want to try turning your guix-daemon off and on
again.
INFO: Codesigning 096525e92cc2 for platform triple arm64-apple-darwin:
...using reference timestamp: 1733177891
...from worktree directory: '/home/sjors/bitcoin'
...bind-mounted in container to: '/bitcoin
...
👍 vasild approved a pull request: "multiprocess: Add libmultiprocess git subtree"
(https://github.com/bitcoin/bitcoin/pull/31741#pullrequestreview-2611252684)
ACK c6658d675d0bb945f53dc62f7e9b95c7bba973bb
> `CAPNP_EXECUTABLE` and `CAPNPC_CXX_EXECUTABLE` ... `multiprocess.md` should mention them as well
:+1: I guess in the "Alternately ..." paragraph.
(https://github.com/bitcoin/bitcoin/pull/31741#pullrequestreview-2611252684)
ACK c6658d675d0bb945f53dc62f7e9b95c7bba973bb
> `CAPNP_EXECUTABLE` and `CAPNPC_CXX_EXECUTABLE` ... `multiprocess.md` should mention them as well
:+1: I guess in the "Alternately ..." paragraph.
💬 Sjors commented on pull request "test: clarify timewarp grace period griefing attack":
(https://github.com/bitcoin/bitcoin/pull/31725#discussion_r1952264789)
> more stricter equality.
Stricter than what? I don't fully remember what the original test was trying to do here. But we know the exact value to expect.
`curtime` is equal to `nTime` of the template, which is controlled by `UpdateTime()` in `node/miner.cpp`. It starts with the actual current time, but then increases if needed to account for the MTP+1 rule and timewarp - and no more than that.
(https://github.com/bitcoin/bitcoin/pull/31725#discussion_r1952264789)
> more stricter equality.
Stricter than what? I don't fully remember what the original test was trying to do here. But we know the exact value to expect.
`curtime` is equal to `nTime` of the template, which is controlled by `UpdateTime()` in `node/miner.cpp`. It starts with the actual current time, but then increases if needed to account for the MTP+1 rule and timewarp - and no more than that.
💬 Sjors commented on pull request "test: clarify timewarp grace period griefing attack":
(https://github.com/bitcoin/bitcoin/pull/31725#discussion_r1952265557)
Indeed, fixed.
(https://github.com/bitcoin/bitcoin/pull/31725#discussion_r1952265557)
Indeed, fixed.
💬 Sjors commented on pull request "test: clarify timewarp grace period griefing attack":
(https://github.com/bitcoin/bitcoin/pull/31725#discussion_r1952265961)
Changed the log statement to `<=`
(https://github.com/bitcoin/bitcoin/pull/31725#discussion_r1952265961)
Changed the log statement to `<=`
🚀 fanquake merged a pull request: "build: set build type and per-build-type flags as early as possible"
(https://github.com/bitcoin/bitcoin/pull/31711)
(https://github.com/bitcoin/bitcoin/pull/31711)
💬 fanquake commented on pull request "guix: Notarize MacOS app bundle and codesign all MacOS and Windows binaries":
(https://github.com/bitcoin/bitcoin/pull/31407#discussion_r1952324188)
Do these new dependencies change what is needed for a no-substitutes/boostrap build?
(https://github.com/bitcoin/bitcoin/pull/31407#discussion_r1952324188)
Do these new dependencies change what is needed for a no-substitutes/boostrap build?
👍 hodlinator approved a pull request: "ci: Test cross-built Windows executables on Windows natively"
(https://github.com/bitcoin/bitcoin/pull/31176#pullrequestreview-2611426260)
ACK 18274b677500204503a4c3853cfcd9af75531021
### Concept
Since few/no end users will be running Bitcoin Core under *Wine*, it makes sense to test cross-built binaries (similar to the Guix-produced release binaries) natively on Windows.
Left over mentions of Wine stem from eaafa23cbd83b7bda4b28779138c62446bbdea2a and 21704f6334d2a4bd140c6e3260c4bfa3f3157bad. The former seems like it could be more of a mingw issue, but the latter workaround could possibly be removed we we want to stop sup
...
(https://github.com/bitcoin/bitcoin/pull/31176#pullrequestreview-2611426260)
ACK 18274b677500204503a4c3853cfcd9af75531021
### Concept
Since few/no end users will be running Bitcoin Core under *Wine*, it makes sense to test cross-built binaries (similar to the Guix-produced release binaries) natively on Windows.
Left over mentions of Wine stem from eaafa23cbd83b7bda4b28779138c62446bbdea2a and 21704f6334d2a4bd140c6e3260c4bfa3f3157bad. The former seems like it could be more of a mingw issue, but the latter workaround could possibly be removed we we want to stop sup
...
🚀 fanquake merged a pull request: "depends: Make default `host` and `build` comparable"
(https://github.com/bitcoin/bitcoin/pull/30584)
(https://github.com/bitcoin/bitcoin/pull/30584)
💬 laanwj commented on pull request "random: Check `GetRNDRRS` is supported in `InitHardwareRand` to avoid infinite loop":
(https://github.com/bitcoin/bitcoin/pull/31826#issuecomment-2653350018)
Checked on amazon r7g.xlarge (Graviton 3):
```
2025-02-12T10:07:35Z Bitcoin Core version v28.99.0-585aba6eec85 (release build)
2025-02-12T10:07:35Z Using the 'arm_shani(1way,2way)' SHA256 implementation
2025-02-12T10:07:35Z Using RNDR and RNDRRS as additional entropy sources
```
Also verified that both `GetRNDR` and `GetRNDRRS` are still being called.
As baseline i also built and tested on rpi5 (ARM64 which has no support for RNG). It is still correctly not detected, no invalid instruct
...
(https://github.com/bitcoin/bitcoin/pull/31826#issuecomment-2653350018)
Checked on amazon r7g.xlarge (Graviton 3):
```
2025-02-12T10:07:35Z Bitcoin Core version v28.99.0-585aba6eec85 (release build)
2025-02-12T10:07:35Z Using the 'arm_shani(1way,2way)' SHA256 implementation
2025-02-12T10:07:35Z Using RNDR and RNDRRS as additional entropy sources
```
Also verified that both `GetRNDR` and `GetRNDRRS` are still being called.
As baseline i also built and tested on rpi5 (ARM64 which has no support for RNG). It is still correctly not detected, no invalid instruct
...
🤔 vasild reviewed a pull request: "Have createNewBlock() wait for tip, make rpc handle shutdown during long poll and wait methods"
(https://github.com/bitcoin/bitcoin/pull/31785#pullrequestreview-2611342025)
Approach ACK be15f2e769987b1df2cf1c8f327c5aafe064dbb3
(https://github.com/bitcoin/bitcoin/pull/31785#pullrequestreview-2611342025)
Approach ACK be15f2e769987b1df2cf1c8f327c5aafe064dbb3
💬 vasild 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#discussion_r1952373326)
Given that "no tip" is expected here (unlikely but is not a "programming logic error"), maybe avoid `CHECK_NONFATAL()` and use `JSONRPCError`:
```cpp
std::optional<BlockRef> old_block{miner.getTip()};
std::optional<BlockRef> new_block;
if (IsRPCRunning()) {
const uint256 h{old_block.has_value() ? old_block->hash : uint256::ZERO};
new_block = timeout > 0 ? miner.waitTipChanged(h, std::chrono::milliseconds(timeout)) : miner.waitTipChanged(h);
}
if (!new_block.has_value()) {
...
(https://github.com/bitcoin/bitcoin/pull/31785#discussion_r1952373326)
Given that "no tip" is expected here (unlikely but is not a "programming logic error"), maybe avoid `CHECK_NONFATAL()` and use `JSONRPCError`:
```cpp
std::optional<BlockRef> old_block{miner.getTip()};
std::optional<BlockRef> new_block;
if (IsRPCRunning()) {
const uint256 h{old_block.has_value() ? old_block->hash : uint256::ZERO};
new_block = timeout > 0 ? miner.waitTipChanged(h, std::chrono::milliseconds(timeout)) : miner.waitTipChanged(h);
}
if (!new_block.has_value()) {
...
💬 vasild 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#discussion_r1952303154)
```suggestion
* @retval std::nullopt if the given `timeout` passes or the node is shut down before the tip is connected (there is no tip during startup).
```
(https://github.com/bitcoin/bitcoin/pull/31785#discussion_r1952303154)
```suggestion
* @retval std::nullopt if the given `timeout` passes or the node is shut down before the tip is connected (there is no tip during startup).
```
💬 vasild 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#discussion_r1952318566)
Just before the lock and return there was a period when both `m_tip_block_mutex` and `cs_main` were unlocked, so `tip_hash` may be stale here. So this could return an inconsistent hash+height - the hash of one block and the height of another. The previous code was ok because it retrieved both the hash and the height from `chainman().ActiveChain().Tip()` under `cs_main`. I think this will fix it:
```diff
- LOCK(::cs_main);
- return BlockRef{*Assume(tip_hash), Assume(chainman().
...
(https://github.com/bitcoin/bitcoin/pull/31785#discussion_r1952318566)
Just before the lock and return there was a period when both `m_tip_block_mutex` and `cs_main` were unlocked, so `tip_hash` may be stale here. So this could return an inconsistent hash+height - the hash of one block and the height of another. The previous code was ok because it retrieved both the hash and the height from `chainman().ActiveChain().Tip()` under `cs_main`. I think this will fix it:
```diff
- LOCK(::cs_main);
- return BlockRef{*Assume(tip_hash), Assume(chainman().
...
💬 vasild 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#discussion_r1952397601)
Similar here:
```cpp
std::optional<BlockRef> block{miner.getTip()};
const auto deadline{std::chrono::steady_clock::now() + 1ms * timeout};
while (IsRPCRunning() && (!block.has_value() || block->hash != hash)) {
const uint256 h{block.has_value() ? block->hash : uint256::ZERO};
if (timeout) {
const auto now{std::chrono::steady_clock::now()};
if (now >= deadline) break;
const MillisecondsDouble remaining{deadline - now};
block = miner.waitTipChang
...
(https://github.com/bitcoin/bitcoin/pull/31785#discussion_r1952397601)
Similar here:
```cpp
std::optional<BlockRef> block{miner.getTip()};
const auto deadline{std::chrono::steady_clock::now() + 1ms * timeout};
while (IsRPCRunning() && (!block.has_value() || block->hash != hash)) {
const uint256 h{block.has_value() ? block->hash : uint256::ZERO};
if (timeout) {
const auto now{std::chrono::steady_clock::now()};
if (now >= deadline) break;
const MillisecondsDouble remaining{deadline - now};
block = miner.waitTipChang
...