💬 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});
}
}
```
(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.
(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.
(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)
(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`.
(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.
(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.
(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=
```
(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.
(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.
(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.
(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.
(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
(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.
(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.
💬 Sjors commented on pull request "guix: switch to some `minimal` versions of packages in our manifest":
(https://github.com/bitcoin/bitcoin/pull/27172#issuecomment-1450156518)
tACK 2c9eb4afe1f583aafa552b2711b149f17ef8320f
I get the same Guix hashes. Lightly tested on macOS.
(https://github.com/bitcoin/bitcoin/pull/27172#issuecomment-1450156518)
tACK 2c9eb4afe1f583aafa552b2711b149f17ef8320f
I get the same Guix hashes. Lightly tested on macOS.
🚀 fanquake merged a pull request: "Update translations for 25.0 soft translation string freeze"
(https://github.com/bitcoin/bitcoin/pull/27169)
(https://github.com/bitcoin/bitcoin/pull/27169)
👍 hebasto approved a pull request: "http: Track active requests and wait for last to finish - 2nd attempt"
(https://github.com/bitcoin/bitcoin/pull/26742)
ACK 60978c8080ec13ff4571c8a89e742517b2aca692
(https://github.com/bitcoin/bitcoin/pull/26742)
ACK 60978c8080ec13ff4571c8a89e742517b2aca692
💬 hebasto commented on pull request "http: Track active requests and wait for last to finish - 2nd attempt":
(https://github.com/bitcoin/bitcoin/pull/26742#discussion_r1121744890)
nit: IWYU suggests to s/`#include <event2/http_struct.h>`/`#include <event2/event.h>`/.
(https://github.com/bitcoin/bitcoin/pull/26742#discussion_r1121744890)
nit: IWYU suggests to s/`#include <event2/http_struct.h>`/`#include <event2/event.h>`/.
💬 Sjors commented on pull request "Deduplicate bitcoind and bitcoin-qt init code":
(https://github.com/bitcoin/bitcoin/pull/27150#issuecomment-1450219330)
Light review ACK 802cc1e
I'm not super familiar with the init code, but the changes look sane to me. I tested some of the error messages on macOS.
Regarding d172b5c, any hints on how to produce a fatal error?
(https://github.com/bitcoin/bitcoin/pull/27150#issuecomment-1450219330)
Light review ACK 802cc1e
I'm not super familiar with the init code, but the changes look sane to me. I tested some of the error messages on macOS.
Regarding d172b5c, any hints on how to produce a fatal error?
💬 instagibbs commented on pull request "doc: Expand scantxoutset help text to cover tr() and miniscript":
(https://github.com/bitcoin/bitcoin/pull/27155#discussion_r1121805808)
hmm true, will remove
(https://github.com/bitcoin/bitcoin/pull/27155#discussion_r1121805808)
hmm true, will remove