💬 MarcoFalke commented on pull request "ci: A few fixes of `ccache` issues":
(https://github.com/bitcoin/bitcoin/pull/27084#issuecomment-1449717685)
I guess instead of `${CIRRUS_WORKING_DIR}/ccache_dir` you can also just write `ccache_dir`, as `CIRRUS_WORKING_DIR` is implicit?

💬 MarcoFalke commented on pull request "contrib: Improve verify-commits.py to work with maintainers leaving":
(https://github.com/bitcoin/bitcoin/pull/27058#issuecomment-1449721171)
Ok, fair enough. Also, I guess no one is forced to upgrade their script. Anyone is free to use the previous version of the python script as long as they want.

💬 glozow commented on pull request "26032 followups":
(https://github.com/bitcoin/bitcoin/pull/27180#discussion_r1121471409)
trailing whitespace here

💬 dergoegge commented on pull request "p2p, validation: Don't download witnesses for assumed-valid blocks when running in prune mode":
(https://github.com/bitcoin/bitcoin/pull/27050#issuecomment-1449815580)
@wtogami This PR does not introduce a new witnessless archival mode (which is what you are describing iiuc). It only makes pruned nodes skip downloading the witnesses up to the assume-valid point (since they're not validated and deleted shortly after anyway).

I would agree that witnessless archival nodes should not be called "pruned", but again that is not what this PR is introducing.

💬 glozow commented on pull request "Implement Mini version of BlockAssembler to calculate mining scores":
(https://github.com/bitcoin/bitcoin/pull/27021#discussion_r1121488980)
Marking as resolved as this has been implemented

💬 glozow commented on pull request "Implement Mini version of BlockAssembler to calculate mining scores":
(https://github.com/bitcoin/bitcoin/pull/27021#discussion_r1121487831)
I agree it might just be better to not reserve anything. We have no idea what the cluster size is going to be, so might as well let the stdlib magic do its work.

💬 glozow commented on pull request "Implement Mini version of BlockAssembler to calculate mining scores":
(https://github.com/bitcoin/bitcoin/pull/27021#discussion_r1121498816)
Disagree with txmempool getting this value from mini_miner, as that would create a circular dependency.
This kind of constant should live in src/kernel/mempool_options.h.

💬 glozow commented on pull request "Implement Mini version of BlockAssembler to calculate mining scores":
(https://github.com/bitcoin/bitcoin/pull/27021#discussion_r1121483332)
Can you explain why using `count` is better?

⚠️ stickies-v opened an issue: "libevent: event_enable_debug_logging() not to be used after creating of event base"
(https://github.com/bitcoin/bitcoin/issues/27182)
Raising this issue on behalf of @hebasto who identified the potential problem as well as dug into the docs.

The `logging` RPC method, through [`UpdateHTTPServerLogging()`](https://github.com/bitcoin/bitcoin/blob/cb40639bdf04bab31bcdceb3bf941e9bade8317a/src/httpserver.cpp#L410-L416) calls the libevent function `event_enable_debug_logging()`. The libevent documentation [states](https://libevent.org/libevent-book/Ref1_libsetup.html) that "You must make any changes to these settings before you ca
...

💬 MarcoFalke commented on pull request "guix: switch to some `minimal` versions of packages in our manifest":
(https://github.com/bitcoin/bitcoin/pull/27172#issuecomment-1449976413)
objdump diff:

```diff
$ git diff -- ./d_*
diff --git a/./d_5_objdump b/./d_e_objdump
index eb1d0fe..27afaec 100644
--- a/./d_5_objdump
+++ b/./d_e_objdump
@@ -1,5 +1,5 @@

-bitcoin-519ec2650e7a/bin/bitcoind: file format elf64-x86-64
+bitcoin-e81d1cdf216b/bin/bitcoind: file format elf64-x86-64


Disassembly of section .init:
@@ -1471666,7 +1471666,7 @@ Disassembly of section .text:
6e63a7: lea 0x7a4af2(%rip),%rsi # e8aea0 <stderr@GLIBC_2.2.5+0xb620>
...

💬 dergoegge commented on pull request "Implement Mini version of BlockAssembler to calculate mining scores":
(https://github.com/bitcoin/bitcoin/pull/27021#discussion_r1121528999)
The `mini_miner` target will always produce a cluster of 500 txs right now, because we always add outputs to `available_coins`. So my suggestion would be to let the fuzzer choose which outputs are added to `available_coins`.

```suggestion
for (uint32_t n{0}; n < num_outputs; ++n) {
if (fuzzed_data_provider.ConsumeBool()) {
available_coins.push_back(COutPoint{tx->GetHash(), n});
}
}
```

💬 dergoegge commented on pull request "Implement Mini version of BlockAssembler to calculate mining scores":
(https://github.com/bitcoin/bitcoin/pull/27021#discussion_r1121540528)
```suggestion
if (fuzzed_data_provider.ConsumeBool()) {
```

Shouldn't really make a difference since the fuzzer picks both bools.

💬 hebasto commented on pull request "ci: A few fixes of `ccache` issues":
(https://github.com/bitcoin/bitcoin/pull/27084#issuecomment-1449992308)
> I guess instead of `${CIRRUS_WORKING_DIR}/ccache_dir` you can also just write `ccache_dir`, as `CIRRUS_WORKING_DIR` is implicit?

While it works for the `folder` property of the `ccache_cache`, it seems the `CCACHE_DIR` variable is supposed to contain an absolute path.

✅ dergoegge closed a pull request: "refactor: Continue moving application data from CNode to Peer"
(https://github.com/bitcoin/bitcoin/pull/26621)

📝 dergoegge reopened a pull request: "refactor: Continue moving application data from CNode to Peer"
(https://github.com/bitcoin/bitcoin/pull/26621)
This PR picks up a subset of changes from #24970 and additionally moves `m_bip152_highbandwith{to,from}`, `nTimeOffset`, `nVersion`, `m_greates_common_version`.

💬 MarcoFalke commented on pull request "refactor: Continue moving application data from CNode to Peer":
(https://github.com/bitcoin/bitcoin/pull/26621#issuecomment-1450005818)
Cirrus CI won't pick up close/open, IIRC. If there was no review, your best option is to (force) push to trigger CI, if the GitHub trigger event was lost on the first try.

💬 dergoegge commented on pull request "Implement Mini version of BlockAssembler to calculate mining scores":
(https://github.com/bitcoin/bitcoin/pull/27021#discussion_r1121607436)
Alternatively, the `LIMITED_WHILE` condition could be changed.

💬 dergoegge commented on pull request "Implement Mini version of BlockAssembler to calculate mining scores":
(https://github.com/bitcoin/bitcoin/pull/27021#discussion_r1121611301)
Also, with this modification there is a crash, which I don't think is the modifications fault:

```
/wCqamFv0GgmkfHCTmPQeMXAul83pioRsGwGcWUbQCYRX/BcVADDAQm0wQAAAAAAAAAAAAAPAAAA
AAAAAA8AAAAAAAAAAAAAAGPQeMXAul83pioRsGwGcWUbKUAGcWUbQCYRX/BcVADDAQm0wQAAAAAA
AAAAAAAPAAAAAAAAAA8AAAAAAAAAAAAAAGPQeMXAul83pioRsGwGcWUbKUAmEV/wVFwAwwG0CQAA
AAAAAAAA//////+mKhGwbAZxZRtAJhFf8FRcAMMBCbTBAAAAAAAAAAAAAA///////////////2Zm
ZmZmZmZmZmZmZgAAAAAAAFw=
```

👍 darosior approved a pull request: "doc: Expand scantxoutset help text to cover tr() and miniscript"
(https://github.com/bitcoin/bitcoin/pull/27155)
ACK ca605f015dc8fbabbc6c0640d87429d6bf8f553f -- thanks for improving the doc.

💬 darosior commented on pull request "doc: Expand scantxoutset help text to cover tr() and miniscript":
(https://github.com/bitcoin/bitcoin/pull/27155#discussion_r1121613859)
nit: i don't think it's worth mentioning Miniscript here, and especially not as something external to descriptors.