💬 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.

💬 Sjors commented on pull request "doc: Expand scantxoutset help text to cover tr() and miniscript":
(https://github.com/bitcoin/bitcoin/pull/27155#issuecomment-1450040369)
Concept ACK. Note that the help already refers to `doc/descriptors.md` so it doesn't have to be super thorough.

💬 dergoegge commented on pull request "net, refactor: net_processing, add `ProcessCompactBlockTxns`":
(https://github.com/bitcoin/bitcoin/pull/26969#discussion_r1121638358)
```suggestion
void ProcessBlockTxns(CNode& pfrom, const Peer& peer, const BlockTransactions& block_transactions)
```

Just create a new CNetMsgMaker, no need to pass it.

💬 TheCharlatan commented on pull request "build: Include qt sources for parsing with extract_strings.py":
(https://github.com/bitcoin/bitcoin/pull/22764#issuecomment-1450067557)
Concept ACK

💬 theStack commented on issue "make check errors on big endian OpenBSD 7.2":
(https://github.com/bitcoin/bitcoin/issues/26492#issuecomment-1450097418)
> @theStack @grubles can you confirm if this is still an issue, i.e with master or not, and on which systems/OS? Note that you can also use `--enable-suppress-external-warnings` to avoid the Boost build spam.

I was never able to reproduce this issue. On the emulated SPARC64 machine (see https://github.com/bitcoin/bitcoin/issues/26492#issuecomment-1314149488) also all tests passed.