💬 stickies-v commented on pull request "Package validation: accept packages of size 1":
(https://github.com/bitcoin/bitcoin/pull/31096#discussion_r1808495896)
I had similar thoughts, yes. I think the current description is correct but less helpful than it could be (even without this PR).
Suggested alternative:
"The package must solely consist of a child transaction and all, some or none of its unconfirmed parents. None of the parents may depend on each other.\n"
(https://github.com/bitcoin/bitcoin/pull/31096#discussion_r1808495896)
I had similar thoughts, yes. I think the current description is correct but less helpful than it could be (even without this PR).
Suggested alternative:
"The package must solely consist of a child transaction and all, some or none of its unconfirmed parents. None of the parents may depend on each other.\n"
💬 fanquake commented on pull request "Add multiprocess binaries to release build":
(https://github.com/bitcoin/bitcoin/pull/30975#issuecomment-2426256506)
See [cb3d8038b61440dff10316afd238603bd9f60924](https://github.com/fanquake/bitcoin/commit/cb3d8038b61440dff10316afd238603bd9f60924) for how I'd rework e76fdae837cea1384ea7fff231dd54ec5fbb0b19. Note that the `_multiprocess` suffix could be dropped from the `i686` job, however, we should have atleast one job building with `NO_MULTIPROCESS`, so maybe that could be reused for that with `_no_multiprocess`.
(https://github.com/bitcoin/bitcoin/pull/30975#issuecomment-2426256506)
See [cb3d8038b61440dff10316afd238603bd9f60924](https://github.com/fanquake/bitcoin/commit/cb3d8038b61440dff10316afd238603bd9f60924) for how I'd rework e76fdae837cea1384ea7fff231dd54ec5fbb0b19. Note that the `_multiprocess` suffix could be dropped from the `i686` job, however, we should have atleast one job building with `NO_MULTIPROCESS`, so maybe that could be reused for that with `_no_multiprocess`.
👍 rkrux approved a pull request: "doc: replace `-?` with `-h` for bench_bitcoin help"
(https://github.com/bitcoin/bitcoin/pull/31118#pullrequestreview-2381587440)
tACK https://github.com/bitcoin/bitcoin/commit/33a28e252a7349c0aa284005aee97873b965fcfe
Thanks @l0rinc for addressing the change quickly. Rebuilt and checked for bitcoin-cli as well, it's updated there too.
```
➜ bitcoin git:(l0rinc/bench-help) ✗ bitcoincli -h
Bitcoin Core RPC client version v28.99.0-33a28e252a73
Usage: bitcoin-cli [options] <command> [params] Send command to Bitcoin Core
or: bitcoin-cli [options] -named <command> [name=value]... Send command to Bitcoin Core
...
(https://github.com/bitcoin/bitcoin/pull/31118#pullrequestreview-2381587440)
tACK https://github.com/bitcoin/bitcoin/commit/33a28e252a7349c0aa284005aee97873b965fcfe
Thanks @l0rinc for addressing the change quickly. Rebuilt and checked for bitcoin-cli as well, it's updated there too.
```
➜ bitcoin git:(l0rinc/bench-help) ✗ bitcoincli -h
Bitcoin Core RPC client version v28.99.0-33a28e252a73
Usage: bitcoin-cli [options] <command> [params] Send command to Bitcoin Core
or: bitcoin-cli [options] -named <command> [name=value]... Send command to Bitcoin Core
...
✅ fanquake closed a pull request: "cmake: Switch to crc32c upstream build system"
(https://github.com/bitcoin/bitcoin/pull/30905)
(https://github.com/bitcoin/bitcoin/pull/30905)
💬 fanquake commented on pull request "cmake: Switch to crc32c upstream build system":
(https://github.com/bitcoin/bitcoin/pull/30905#issuecomment-2426263301)
Closing, as the consensus between myself, @hebasto, @theuni and @TheCharlatan was to not make this change (for now, or not until we've adjusted upstream further).
(https://github.com/bitcoin/bitcoin/pull/30905#issuecomment-2426263301)
Closing, as the consensus between myself, @hebasto, @theuni and @TheCharlatan was to not make this change (for now, or not until we've adjusted upstream further).
💬 laanwj commented on pull request "guix: Enable CET for `glibc` package":
(https://github.com/bitcoin/bitcoin/pull/31121#issuecomment-2426337364)
i've built both this PR and the commit it's based on using guix, and the resulting x86_64 bitcoind binaries have the same size. There are subtle differences in the assembly, though:
```bash
objdump -dC --dwarf=follow-links a38603456e9a/bitcoind > a38603456e9a/bitcoind.s
objdump -dC --dwarf=follow-links 4d3da08d1b9d/bitcoind > 4d3da08d1b9d/bitcoind.s
diff -du a38603456e9a/bitcoind.s 4d3da08d1b9d/bitcoind.s > diff
```
```
--- a38603456e9a/bitcoind.s 2024-10-21 12:52:23.822751727 +0200
+++
...
(https://github.com/bitcoin/bitcoin/pull/31121#issuecomment-2426337364)
i've built both this PR and the commit it's based on using guix, and the resulting x86_64 bitcoind binaries have the same size. There are subtle differences in the assembly, though:
```bash
objdump -dC --dwarf=follow-links a38603456e9a/bitcoind > a38603456e9a/bitcoind.s
objdump -dC --dwarf=follow-links 4d3da08d1b9d/bitcoind > 4d3da08d1b9d/bitcoind.s
diff -du a38603456e9a/bitcoind.s 4d3da08d1b9d/bitcoind.s > diff
```
```
--- a38603456e9a/bitcoind.s 2024-10-21 12:52:23.822751727 +0200
+++
...
👍 laanwj approved a pull request: "guix: Enable CET for `glibc` package"
(https://github.com/bitcoin/bitcoin/pull/31121#pullrequestreview-2381714750)
ACK 4d3da08d1b9d07acb43420899e0d16fad2437fb0
(https://github.com/bitcoin/bitcoin/pull/31121#pullrequestreview-2381714750)
ACK 4d3da08d1b9d07acb43420899e0d16fad2437fb0
👍 laanwj approved a pull request: "doc: replace `-?` with `-h` for bench_bitcoin help"
(https://github.com/bitcoin/bitcoin/pull/31118#pullrequestreview-2381742913)
Code review ACK 33a28e252a7349c0aa284005aee97873b965fcfe
(https://github.com/bitcoin/bitcoin/pull/31118#pullrequestreview-2381742913)
Code review ACK 33a28e252a7349c0aa284005aee97873b965fcfe
💬 laanwj commented on pull request "test: use context managers and add file existence checks in feature_fee_estimation.py":
(https://github.com/bitcoin/bitcoin/pull/31030#issuecomment-2426407768)
Concept ACK on using the context managers
(https://github.com/bitcoin/bitcoin/pull/31030#issuecomment-2426407768)
Concept ACK on using the context managers
💬 laanwj commented on pull request "rpc: net: follow-ups for #30062":
(https://github.com/bitcoin/bitcoin/pull/30183#discussion_r1808615537)
yes i am a bit divide about this, i understand you don't want to invalidate ACKs last-minute, but on the other hand re-reviewing after a trivial change is super easy, and if we're touching this anyway it's good to make sure it's the best possible documentation
(https://github.com/bitcoin/bitcoin/pull/30183#discussion_r1808615537)
yes i am a bit divide about this, i understand you don't want to invalidate ACKs last-minute, but on the other hand re-reviewing after a trivial change is super easy, and if we're touching this anyway it's good to make sure it's the best possible documentation
💬 brunoerg commented on pull request "rpc: net: follow-ups for #30062":
(https://github.com/bitcoin/bitcoin/pull/30183#discussion_r1808622065)
No problem, so I'm gonna address it here.
(https://github.com/bitcoin/bitcoin/pull/30183#discussion_r1808622065)
No problem, so I'm gonna address it here.
📝 hodlinator opened a pull request: "util: Remove RandAddSeedPerfmon"
(https://github.com/bitcoin/bitcoin/pull/31124)
`RegQueryValueExA(HKEY_PERFORMANCE_DATA, ...)` sometimes hangs *bitcoind.exe* on Windows during startup, at least on CI.
We have other sources of entropy to seed randomness with on Windows, so should be alright removing this. Might resurrect if less drastic fix is found.
Hopefully sufficient to fix #30390.
<!--
*** Please remove the following help text before submitting: ***
Pull requests without a rationale and clear improvement may be closed
immediately.
GUI-related pull reque
...
(https://github.com/bitcoin/bitcoin/pull/31124)
`RegQueryValueExA(HKEY_PERFORMANCE_DATA, ...)` sometimes hangs *bitcoind.exe* on Windows during startup, at least on CI.
We have other sources of entropy to seed randomness with on Windows, so should be alright removing this. Might resurrect if less drastic fix is found.
Hopefully sufficient to fix #30390.
<!--
*** Please remove the following help text before submitting: ***
Pull requests without a rationale and clear improvement may be closed
immediately.
GUI-related pull reque
...
💬 hodlinator commented on pull request "Windows bitcoind stall debugging [NOMERGE, DRAFT]":
(https://github.com/bitcoin/bitcoin/pull/30956#issuecomment-2426460077)
PR: #31124
(https://github.com/bitcoin/bitcoin/pull/30956#issuecomment-2426460077)
PR: #31124
💬 hodlinator commented on pull request "util: Remove RandAddSeedPerfmon":
(https://github.com/bitcoin/bitcoin/pull/31124#issuecomment-2426479455)
For brief context, in 3 of 3 checked Windows stack dumps, the main callstack where it hangs has been:
```
ntdll.dll!NtReleaseSemaphore() Unknown
KERNELBASE.dll!ReleaseSemaphore() Unknown
WmiApRpl.dll!WmiReverseGuard::LeaveRead(long) Unknown
WmiApRpl.dll!WmiReverseMemoryExt<class WmiReverseGuard>::Read(void *,unsigned long,unsigned long *,unsigned long) Unknown
WmiApRpl.dll!WmiAdapterWrapper::GetValidity(unsigned long) Unknown
WmiApRpl.dll!WmiAdapterWrapper::CollectObjects(unsi
...
(https://github.com/bitcoin/bitcoin/pull/31124#issuecomment-2426479455)
For brief context, in 3 of 3 checked Windows stack dumps, the main callstack where it hangs has been:
```
ntdll.dll!NtReleaseSemaphore() Unknown
KERNELBASE.dll!ReleaseSemaphore() Unknown
WmiApRpl.dll!WmiReverseGuard::LeaveRead(long) Unknown
WmiApRpl.dll!WmiReverseMemoryExt<class WmiReverseGuard>::Read(void *,unsigned long,unsigned long *,unsigned long) Unknown
WmiApRpl.dll!WmiAdapterWrapper::GetValidity(unsigned long) Unknown
WmiApRpl.dll!WmiAdapterWrapper::CollectObjects(unsi
...
💬 sipa commented on pull request "util: Remove RandAddSeedPerfmon":
(https://github.com/bitcoin/bitcoin/pull/31124#discussion_r1808672524)
You may want to also modify the "// Dynamic environment data (performance monitoring, ...)" line in `SeedPeriodic()` in random.cpp.
(https://github.com/bitcoin/bitcoin/pull/31124#discussion_r1808672524)
You may want to also modify the "// Dynamic environment data (performance monitoring, ...)" line in `SeedPeriodic()` in random.cpp.
💬 sipa commented on pull request "util: Remove RandAddSeedPerfmon":
(https://github.com/bitcoin/bitcoin/pull/31124#issuecomment-2426496088)
Concept ACK
(https://github.com/bitcoin/bitcoin/pull/31124#issuecomment-2426496088)
Concept ACK
💬 fanquake commented on pull request "util: Remove RandAddSeedPerfmon":
(https://github.com/bitcoin/bitcoin/pull/31124#issuecomment-2426561701)
Concept ACK - can also remove `winreg.h`.
(https://github.com/bitcoin/bitcoin/pull/31124#issuecomment-2426561701)
Concept ACK - can also remove `winreg.h`.
💬 fjahr commented on pull request "miner: Reorg Testnet4 minimum difficulty blocks":
(https://github.com/bitcoin/bitcoin/pull/31117#issuecomment-2426574794)
CI is broken because BIP94 is also active on regtest and this change breaks some functional tests that are not related to the issue. I don't think I need to work on fixing those tests unless I get an indication that people think this is actually interesting to merge.
We are getting some report that the `getblocktemplate` sometimes hangs on a small-ish debian 12 server. I have not been able to reproduce this locally or on a small-ish ubuntu server. Would be great if anyone is interested in tes
...
(https://github.com/bitcoin/bitcoin/pull/31117#issuecomment-2426574794)
CI is broken because BIP94 is also active on regtest and this change breaks some functional tests that are not related to the issue. I don't think I need to work on fixing those tests unless I get an indication that people think this is actually interesting to merge.
We are getting some report that the `getblocktemplate` sometimes hangs on a small-ish debian 12 server. I have not been able to reproduce this locally or on a small-ish ubuntu server. Would be great if anyone is interested in tes
...
💬 laanwj commented on pull request "util: Remove RandAddSeedPerfmon":
(https://github.com/bitcoin/bitcoin/pull/31124#issuecomment-2426608538)
Concept ACK as explained in https://github.com/bitcoin/bitcoin/pull/30956#issuecomment-2425157662 - querying all performance counters has a potentially high overhead for the system.
(https://github.com/bitcoin/bitcoin/pull/31124#issuecomment-2426608538)
Concept ACK as explained in https://github.com/bitcoin/bitcoin/pull/30956#issuecomment-2425157662 - querying all performance counters has a potentially high overhead for the system.
💬 hodlinator commented on pull request "util: Remove RandAddSeedPerfmon":
(https://github.com/bitcoin/bitcoin/pull/31124#discussion_r1808780747)
Adjusted in latest push (5cdca2f9d704f2f57ed49beaa764d31c8c55ca77), new comment:
```C++
// Dynamic environment data (clocks, resource usage, ...)
```
(https://github.com/bitcoin/bitcoin/pull/31124#discussion_r1808780747)
Adjusted in latest push (5cdca2f9d704f2f57ed49beaa764d31c8c55ca77), new comment:
```C++
// Dynamic environment data (clocks, resource usage, ...)
```
💬 pinheadmz commented on issue "estimateSmartFee error: "Insufficient data or no feerate found"":
(https://github.com/bitcoin/bitcoin/issues/31116#issuecomment-2426620801)
@qk-santi ok sounds like you have a clear understanding and the behavior is still unexpected. I would check two more things:
- Check your bitcoin.conf for anything that might be affecting mempool policy (rbf, relayfee, whitelist, etc)
- Enable estimate fee logging `bitcoin-cli logging '["estimatefee"]'` and watch debug.log for `[estiamtefee]` messages
(https://github.com/bitcoin/bitcoin/issues/31116#issuecomment-2426620801)
@qk-santi ok sounds like you have a clear understanding and the behavior is still unexpected. I would check two more things:
- Check your bitcoin.conf for anything that might be affecting mempool policy (rbf, relayfee, whitelist, etc)
- Enable estimate fee logging `bitcoin-cli logging '["estimatefee"]'` and watch debug.log for `[estiamtefee]` messages