✅ fanquake closed a pull request: "Update README: Enhance Formatting and Clarity"
(https://github.com/bitcoin/bitcoin/pull/30366)
(https://github.com/bitcoin/bitcoin/pull/30366)
✅ willcl-ark closed an issue: "build: `libbitcoin_util` unexpected(?)/undocumented dependencies"
(https://github.com/bitcoin/bitcoin/issues/28548)
(https://github.com/bitcoin/bitcoin/issues/28548)
💬 willcl-ark commented on issue "build: `libbitcoin_util` unexpected(?)/undocumented dependencies":
(https://github.com/bitcoin/bitcoin/issues/28548#issuecomment-2199628181)
Seems that we can close this now that #29015 has been merged.
Let me know if this is a mistake and this shouldn't be closed until `libbitcoin_util` doesn't depend on `libbitcoin_crypto`.
(https://github.com/bitcoin/bitcoin/issues/28548#issuecomment-2199628181)
Seems that we can close this now that #29015 has been merged.
Let me know if this is a mistake and this shouldn't be closed until `libbitcoin_util` doesn't depend on `libbitcoin_crypto`.
💬 dergoegge commented on pull request "fuzz: FuzzedSock::Recv() don't lose bytes from MSG_PEEK read":
(https://github.com/bitcoin/bitcoin/pull/30273#issuecomment-2199646499)
CI failure looks unrelated to me
(https://github.com/bitcoin/bitcoin/pull/30273#issuecomment-2199646499)
CI failure looks unrelated to me
💬 Fi3 commented on pull request "Stratum v2 Noise Protocol":
(https://github.com/bitcoin/bitcoin/pull/29346#issuecomment-2199647556)
@itornaza If I remember correctly anything needed to implement sv2 noise is contained in the [sv2 spec](https://github.com/stratum-mining/sv2-spec/blob/main/04-Protocol-Security.md) and the [noise spec](https://noiseprotocol.org/noise.html) are a little bit dispersive. But I implemented it on SRI more then one year ago so take it with a grain of salt.
(https://github.com/bitcoin/bitcoin/pull/29346#issuecomment-2199647556)
@itornaza If I remember correctly anything needed to implement sv2 noise is contained in the [sv2 spec](https://github.com/stratum-mining/sv2-spec/blob/main/04-Protocol-Security.md) and the [noise spec](https://noiseprotocol.org/noise.html) are a little bit dispersive. But I implemented it on SRI more then one year ago so take it with a grain of salt.
💬 paplorinc commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1660708747)
nit:
```suggestion
if (!it->second.coin.IsSpent()) {
/* BatchWrite must unset all unspent entries when will_erase=false. */
throw std::logic_error("Not all unspent flagged entries were unset");
}
const auto next_entry{it->second.Next(/*clear_flags=*/true)};
cachedCoinsUsage -= it->second.coin.DynamicMemoryUsage();
cacheCoins.erase(it->first);
```
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1660708747)
nit:
```suggestion
if (!it->second.coin.IsSpent()) {
/* BatchWrite must unset all unspent entries when will_erase=false. */
throw std::logic_error("Not all unspent flagged entries were unset");
}
const auto next_entry{it->second.Next(/*clear_flags=*/true)};
cachedCoinsUsage -= it->second.coin.DynamicMemoryUsage();
cacheCoins.erase(it->first);
```
💬 paplorinc commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1660730018)
nit:
untested, but modern compilers will likely [decide the need for inlining themselves
](https://en.cppreference.com/w/cpp/language/inline#:~:text=Since%20inline%20substitution)
> Since inline substitution is unobservable in the standard semantics, compilers are free to use inline substitution for any function that's not marked inline, and are free to generate function calls to any function marked inline.
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1660730018)
nit:
untested, but modern compilers will likely [decide the need for inlining themselves
](https://en.cppreference.com/w/cpp/language/inline#:~:text=Since%20inline%20substitution)
> Since inline substitution is unobservable in the standard semantics, compilers are free to use inline substitution for any function that's not marked inline, and are free to generate function calls to any function marked inline.
💬 paplorinc commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#issuecomment-2199661794)
utACK 670084adc53e3d661ea5b4a19743659609a42d5e, will run the benchmarks in the following weeks, thanks for addressing the suggestions.
(https://github.com/bitcoin/bitcoin/pull/28280#issuecomment-2199661794)
utACK 670084adc53e3d661ea5b4a19743659609a42d5e, will run the benchmarks in the following weeks, thanks for addressing the suggestions.
👍 brunoerg approved a pull request: "fuzz: FuzzedSock::Recv() don't lose bytes from MSG_PEEK read"
(https://github.com/bitcoin/bitcoin/pull/30273#pullrequestreview-2150885622)
utACK 4d81b4de339efbbb68c9785203b699e6e12ecd83
(https://github.com/bitcoin/bitcoin/pull/30273#pullrequestreview-2150885622)
utACK 4d81b4de339efbbb68c9785203b699e6e12ecd83
💬 pablomartin4btc commented on pull request "cli: improve error message on multiwallet and add validation to cli-side commands":
(https://github.com/bitcoin/bitcoin/pull/26990#discussion_r1660756564)
Ok, thanks, will do.
(https://github.com/bitcoin/bitcoin/pull/26990#discussion_r1660756564)
Ok, thanks, will do.
🚀 fanquake merged a pull request: "test: p2p: check that connecting to ourself leads to disconnect"
(https://github.com/bitcoin/bitcoin/pull/30362)
(https://github.com/bitcoin/bitcoin/pull/30362)
💬 pablomartin4btc commented on pull request "cli: improve error message on multiwallet and add validation to cli-side commands":
(https://github.com/bitcoin/bitcoin/pull/26990#discussion_r1660761271)
Ok, I'll work on the full error message verification in the test and will consider your preference on the error documentation. Thanks again!
(https://github.com/bitcoin/bitcoin/pull/26990#discussion_r1660761271)
Ok, I'll work on the full error message verification in the test and will consider your preference on the error documentation. Thanks again!
💬 apulsifer commented on issue "LevelDB read failure: Corruption: block checksum mismatch":
(https://github.com/bitcoin/bitcoin/issues/30159#issuecomment-2199690757)
Update: 3 of the 10 servers experienced data corruption yesterday. This makes me suspect that the problem is not completely random, but depends on contents of the blocks. A replay of the blocks (best blocks and orphan blocks) generated from 2024-06-28 to 2024-06-31 might make good test data.
(https://github.com/bitcoin/bitcoin/issues/30159#issuecomment-2199690757)
Update: 3 of the 10 servers experienced data corruption yesterday. This makes me suspect that the problem is not completely random, but depends on contents of the blocks. A replay of the blocks (best blocks and orphan blocks) generated from 2024-06-28 to 2024-06-31 might make good test data.
⚠️ maflcko opened an issue: "fuzz: mini_miner_selection: ASSERT: mock_template_txids.size() <= blocktemplate->block.vtx.size()"
(https://github.com/bitcoin/bitcoin/issues/30367)
https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=69957
https://github.com/bitcoin/bitcoin/blob/2f6dca4d1c01ee47275a4292f128d714736837a1/src/test/fuzz/mini_miner.cpp#L192
(https://github.com/bitcoin/bitcoin/issues/30367)
https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=69957
https://github.com/bitcoin/bitcoin/blob/2f6dca4d1c01ee47275a4292f128d714736837a1/src/test/fuzz/mini_miner.cpp#L192
💬 maflcko commented on issue "fuzz: mini_miner_selection: ASSERT: mock_template_txids.size() <= blocktemplate->block.vtx.size()":
(https://github.com/bitcoin/bitcoin/issues/30367#issuecomment-2199772546)
```
$ echo 'AyABAAAAACAg/wAAAAAA/yD/AQAAAAAg/yCu/w==' | base64 --decode > /tmp/oss-fuzz-crash
$ FUZZ=mini_miner_selection ./src/test/fuzz/fuzz /tmp/oss-fuzz-crash
(https://github.com/bitcoin/bitcoin/issues/30367#issuecomment-2199772546)
```
$ echo 'AyABAAAAACAg/wAAAAAA/yD/AQAAAAAg/yCu/w==' | base64 --decode > /tmp/oss-fuzz-crash
$ FUZZ=mini_miner_selection ./src/test/fuzz/fuzz /tmp/oss-fuzz-crash
💬 Sjors commented on pull request "refactor: add coinbase constraints to BlockAssembler::Options":
(https://github.com/bitcoin/bitcoin/pull/30356#issuecomment-2199791658)
I renamed `coinbase_output_max_additional_size` to `coinbase_max_additional_weight`. With the current [Stratum v2 spec](https://github.com/stratum-mining/sv2-spec/blob/main/07-Template-Distribution-Protocol.md#71-coinbaseoutputdatasize-client---server) the latter is calculated as follows:
`coinbase_max_additional_weight = (coinbase_output_max_additional_size + 100 + 0 + 2) * 4`
- `coinbase_output_max_additional_size * 4`: Coinbase outputs are not part of the witness so their weight is simp
...
(https://github.com/bitcoin/bitcoin/pull/30356#issuecomment-2199791658)
I renamed `coinbase_output_max_additional_size` to `coinbase_max_additional_weight`. With the current [Stratum v2 spec](https://github.com/stratum-mining/sv2-spec/blob/main/07-Template-Distribution-Protocol.md#71-coinbaseoutputdatasize-client---server) the latter is calculated as follows:
`coinbase_max_additional_weight = (coinbase_output_max_additional_size + 100 + 0 + 2) * 4`
- `coinbase_output_max_additional_size * 4`: Coinbase outputs are not part of the witness so their weight is simp
...
💬 Sjors commented on pull request "refactor: add coinbase constraints to BlockAssembler::Options":
(https://github.com/bitcoin/bitcoin/pull/30356#discussion_r1660833557)
I ended up renaming `coinbase_output_max_additional_size` to `coinbase_max_additional_weight` to account for these issues, see below.
(https://github.com/bitcoin/bitcoin/pull/30356#discussion_r1660833557)
I ended up renaming `coinbase_output_max_additional_size` to `coinbase_max_additional_weight` to account for these issues, see below.
⚠️ fanquake opened an issue: "ci: failure in p2p_handshake.py"
(https://github.com/bitcoin/bitcoin/issues/30368)
https://github.com/bitcoin/bitcoin/actions/runs/9741796701/job/26881733806#step:7:6601
```bash
node0 2024-07-01T10:31:12.195996Z [msghand] [net_processing.cpp:3664] [ProcessMessage] [net] received: sendheaders (0 bytes) peer=19
test 2024-07-01T10:32:32.120000Z TestFramework (ERROR): Assertion failed
Traceback (most recent call last):
File "/home/runner/work/_temp/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/test
...
(https://github.com/bitcoin/bitcoin/issues/30368)
https://github.com/bitcoin/bitcoin/actions/runs/9741796701/job/26881733806#step:7:6601
```bash
node0 2024-07-01T10:31:12.195996Z [msghand] [net_processing.cpp:3664] [ProcessMessage] [net] received: sendheaders (0 bytes) peer=19
test 2024-07-01T10:32:32.120000Z TestFramework (ERROR): Assertion failed
Traceback (most recent call last):
File "/home/runner/work/_temp/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/test
...
✅ maflcko closed an issue: "ci: Move more tasks to GHA?"
(https://github.com/bitcoin/bitcoin/issues/30304)
(https://github.com/bitcoin/bitcoin/issues/30304)
💬 maflcko commented on issue "ci: Move more tasks to GHA?":
(https://github.com/bitcoin/bitcoin/issues/30304#issuecomment-2199810756)
Closing for now, but the discussion can continue. It should be possible and easy to switch more tasks over, starting with the ones mentioned in the starting post. This can be done, if people think it improves the UX on forks, or has other benefits. I just wanted to raise the discussion to explain that it is possible, if people find it useful, but I won't be working on it myself.
(https://github.com/bitcoin/bitcoin/issues/30304#issuecomment-2199810756)
Closing for now, but the discussion can continue. It should be possible and easy to switch more tasks over, starting with the ones mentioned in the starting post. This can be done, if people think it improves the UX on forks, or has other benefits. I just wanted to raise the discussion to explain that it is possible, if people find it useful, but I won't be working on it myself.