💬 MarcoFalke commented on issue "Intermittent failures in interface_usdt_mempool.py":
(https://github.com/bitcoin/bitcoin/issues/27380#issuecomment-1578265398)
Can it be forced to "reproduce" on master by adding a typo to the error string in assert_equal?
(https://github.com/bitcoin/bitcoin/issues/27380#issuecomment-1578265398)
Can it be forced to "reproduce" on master by adding a typo to the error string in assert_equal?
💬 virtu commented on pull request "test: handle failed `check_equal()` assertions in bcc callback functions":
(https://github.com/bitcoin/bitcoin/pull/27831#issuecomment-1578274093)
> (I think you meant to write `assert_equal()` instead of `check_equal()`)
You're right. I don't think I can change it in the PR title, but I at least fixed it in the description.
(https://github.com/bitcoin/bitcoin/pull/27831#issuecomment-1578274093)
> (I think you meant to write `assert_equal()` instead of `check_equal()`)
You're right. I don't think I can change it in the PR title, but I at least fixed it in the description.
🚀 fanquake merged a pull request: "guix: remove cURL from build env"
(https://github.com/bitcoin/bitcoin/pull/27779)
(https://github.com/bitcoin/bitcoin/pull/27779)
💬 0xB10C commented on issue "Intermittent failures in interface_usdt_mempool.py":
(https://github.com/bitcoin/bitcoin/issues/27380#issuecomment-1578336531)
> Can it be forced to "reproduce" on master by adding a typo to the error string in assert_equal?
see https://github.com/0xB10C/bitcoin/commit/4091d621f16b6316eef5551a56c00bf0e2c0e541 and this CI task https://cirrus-ci.com/task/4909169740873728
(https://github.com/bitcoin/bitcoin/issues/27380#issuecomment-1578336531)
> Can it be forced to "reproduce" on master by adding a typo to the error string in assert_equal?
see https://github.com/0xB10C/bitcoin/commit/4091d621f16b6316eef5551a56c00bf0e2c0e541 and this CI task https://cirrus-ci.com/task/4909169740873728
💬 hebasto commented on pull request "guix: Clean up manifest":
(https://github.com/bitcoin/bitcoin/pull/27811#issuecomment-1578368801)
Rebased 404a0e98dd964af4db338843f65ca3a63429db17 -> 77414d50d6f71bfa1e7c04ec672167cf0cf0dedc ([pr27811.03](https://github.com/hebasto/bitcoin/commits/pr27811.03) -> [pr27811.04](https://github.com/hebasto/bitcoin/commits/pr27811.04)) on top of the just merged https://github.com/bitcoin/bitcoin/pull/27779.
(https://github.com/bitcoin/bitcoin/pull/27811#issuecomment-1578368801)
Rebased 404a0e98dd964af4db338843f65ca3a63429db17 -> 77414d50d6f71bfa1e7c04ec672167cf0cf0dedc ([pr27811.03](https://github.com/hebasto/bitcoin/commits/pr27811.03) -> [pr27811.04](https://github.com/hebasto/bitcoin/commits/pr27811.04)) on top of the just merged https://github.com/bitcoin/bitcoin/pull/27779.
💬 vasild commented on pull request "test: add end-to-end tests for CConnman and PeerManager":
(https://github.com/bitcoin/bitcoin/pull/26812#issuecomment-1578372952)
`f559067e27...4c867de996`: rebase due to conflicts
(https://github.com/bitcoin/bitcoin/pull/26812#issuecomment-1578372952)
`f559067e27...4c867de996`: rebase due to conflicts
💬 vasild commented on pull request "test: add end-to-end tests for CConnman and PeerManager":
(https://github.com/bitcoin/bitcoin/pull/26812#discussion_r1219375776)
I put the new e2e tests in a separate new file because this code (which I was trying to avoid duplicating) is gone after https://github.com/bitcoin/bitcoin/pull/27766
(https://github.com/bitcoin/bitcoin/pull/26812#discussion_r1219375776)
I put the new e2e tests in a separate new file because this code (which I was trying to avoid duplicating) is gone after https://github.com/bitcoin/bitcoin/pull/27766
💬 MarcoFalke commented on pull request "test: add end-to-end tests for CConnman and PeerManager":
(https://github.com/bitcoin/bitcoin/pull/26812#discussion_r1219413577)
Changing the fuzz input format with a run-time setting seems fragile and not intuitive when one wants to re-use fuzz inputs or reproduce runs.
(https://github.com/bitcoin/bitcoin/pull/26812#discussion_r1219413577)
Changing the fuzz input format with a run-time setting seems fragile and not intuitive when one wants to re-use fuzz inputs or reproduce runs.
💬 MarcoFalke commented on pull request "test: add end-to-end tests for CConnman and PeerManager":
(https://github.com/bitcoin/bitcoin/pull/26812#discussion_r1219414855)
Could this use `DataStream`, or drop it completely?
(https://github.com/bitcoin/bitcoin/pull/26812#discussion_r1219414855)
Could this use `DataStream`, or drop it completely?
💬 0xB10C commented on issue "Duplicate coinbase transaction space reservation in CreateNewBlock":
(https://github.com/bitcoin/bitcoin/issues/21950#issuecomment-1578535803)
I agree that reserving 4000 WU twice is confusing. However, I'm not familiar enough with the code to judge on which to remove.
(https://github.com/bitcoin/bitcoin/issues/21950#issuecomment-1578535803)
I agree that reserving 4000 WU twice is confusing. However, I'm not familiar enough with the code to judge on which to remove.
💬 vasild commented on pull request "p2p: Diversify automatic outbound connections with respect to networks":
(https://github.com/bitcoin/bitcoin/pull/27213#issuecomment-1578579892)
I am slowly coming back to this PR...
> we are still leaning towards bundling into a single clearnet ... - thoughts?
Ok :)
(https://github.com/bitcoin/bitcoin/pull/27213#issuecomment-1578579892)
I am slowly coming back to this PR...
> we are still leaning towards bundling into a single clearnet ... - thoughts?
Ok :)
📝 MarcoFalke opened a pull request: "test: Add -datacarriersize=2 tests"
(https://github.com/bitcoin/bitcoin/pull/27832)
Clarify with a test that `-datacarriersize` applies to the serialized size of the scriptPubKey, not the size of the pushed data. So for example,
* `-datacarriersize=2` will reject a `raw(6a01aa)`, even though only one byte is pushed
* `-datacarriersize=0` (or `-datacarrier=0`) will reject a `raw(6a)`, even though no byte is pushed
* `-datacarriersize=0` (or `-datacarrier=0`) will reject a `raw(6a00)`, even though zero bytes are pushed
(https://github.com/bitcoin/bitcoin/pull/27832)
Clarify with a test that `-datacarriersize` applies to the serialized size of the scriptPubKey, not the size of the pushed data. So for example,
* `-datacarriersize=2` will reject a `raw(6a01aa)`, even though only one byte is pushed
* `-datacarriersize=0` (or `-datacarrier=0`) will reject a `raw(6a)`, even though no byte is pushed
* `-datacarriersize=0` (or `-datacarrier=0`) will reject a `raw(6a00)`, even though zero bytes are pushed
💬 TheCharlatan commented on pull request "guix: Clean up manifest":
(https://github.com/bitcoin/bitcoin/pull/27811#issuecomment-1578679709)
Guix build:
```
find guix-build-$(git rev-parse --short=12 HEAD)/output/ -type f -print0 | env LC_ALL=C sort -z | xargs -r0 sha256sum
c4f2c29c7b5e1ab211ea38358517cf6c2a1e154e5d2c0d036e7750a8f5f5555a guix-build-77414d50d6f7/output/aarch64-linux-gnu/SHA256SUMS.part
c902944cce64fe7f2cee1242f18b35dbe1c5ce515e921c08c9bfebb34d0c28ca guix-build-77414d50d6f7/output/aarch64-linux-gnu/bitcoin-77414d50d6f7-aarch64-linux-gnu-debug.tar.gz
3aad68ed102f2f1a18217b3d2b2b32ba51f266c03b971f05f8bebd160bbf7d7
...
(https://github.com/bitcoin/bitcoin/pull/27811#issuecomment-1578679709)
Guix build:
```
find guix-build-$(git rev-parse --short=12 HEAD)/output/ -type f -print0 | env LC_ALL=C sort -z | xargs -r0 sha256sum
c4f2c29c7b5e1ab211ea38358517cf6c2a1e154e5d2c0d036e7750a8f5f5555a guix-build-77414d50d6f7/output/aarch64-linux-gnu/SHA256SUMS.part
c902944cce64fe7f2cee1242f18b35dbe1c5ce515e921c08c9bfebb34d0c28ca guix-build-77414d50d6f7/output/aarch64-linux-gnu/bitcoin-77414d50d6f7-aarch64-linux-gnu-debug.tar.gz
3aad68ed102f2f1a18217b3d2b2b32ba51f266c03b971f05f8bebd160bbf7d7
...
💬 vasild commented on pull request "test: add end-to-end tests for CConnman and PeerManager":
(https://github.com/bitcoin/bitcoin/pull/26812#discussion_r1219560287)
Are you suggesting to replace it with this:
```cpp
const std::string random_message_type{fuzzed_data_provider.ConsumeBytesAsString(CMessageHeader::COMMAND_SIZE)};
if (!LIMIT_TO_MESSAGE_TYPE.empty() && random_message_type != LIMIT_TO_MESSAGE_TYPE) {
continue;
}
```
(https://github.com/bitcoin/bitcoin/pull/26812#discussion_r1219560287)
Are you suggesting to replace it with this:
```cpp
const std::string random_message_type{fuzzed_data_provider.ConsumeBytesAsString(CMessageHeader::COMMAND_SIZE)};
if (!LIMIT_TO_MESSAGE_TYPE.empty() && random_message_type != LIMIT_TO_MESSAGE_TYPE) {
continue;
}
```
💬 vasild commented on pull request "test: add end-to-end tests for CConnman and PeerManager":
(https://github.com/bitcoin/bitcoin/pull/26812#discussion_r1219568001)
Good point, dropped it!
(https://github.com/bitcoin/bitcoin/pull/26812#discussion_r1219568001)
Good point, dropped it!
💬 Sjors commented on pull request "guix: Clean up manifest":
(https://github.com/bitcoin/bitcoin/pull/27811#issuecomment-1578700432)
My guix build matches @TheCharlatan
(https://github.com/bitcoin/bitcoin/pull/27811#issuecomment-1578700432)
My guix build matches @TheCharlatan
💬 hebasto commented on pull request "guix: Clean up manifest":
(https://github.com/bitcoin/bitcoin/pull/27811#issuecomment-1578703813)
I'll be able to provide my own Guix build hashes tomorrow only.
(https://github.com/bitcoin/bitcoin/pull/27811#issuecomment-1578703813)
I'll be able to provide my own Guix build hashes tomorrow only.
💬 ismaelsadeeq commented on pull request "Fee estimation: avoid serving stale fee estimate":
(https://github.com/bitcoin/bitcoin/pull/27622#discussion_r1219595571)
I agree it's better than the magic number, I have updated it.
(https://github.com/bitcoin/bitcoin/pull/27622#discussion_r1219595571)
I agree it's better than the magic number, I have updated it.
💬 ismaelsadeeq commented on pull request "Fee estimation: avoid serving stale fee estimate":
(https://github.com/bitcoin/bitcoin/pull/27622#discussion_r1219596235)
Fixed Thank you.
(https://github.com/bitcoin/bitcoin/pull/27622#discussion_r1219596235)
Fixed Thank you.
👍 instagibbs approved a pull request: "Fee estimation: avoid serving stale fee estimate"
(https://github.com/bitcoin/bitcoin/pull/27622#pullrequestreview-1465157911)
code review ACK 6d36f3edbe7549cfaadc48b43c09dd3e581e92ff
I'd really prefer we have a regtest-only option to skip the check to impact integration tests the least.
(https://github.com/bitcoin/bitcoin/pull/27622#pullrequestreview-1465157911)
code review ACK 6d36f3edbe7549cfaadc48b43c09dd3e581e92ff
I'd really prefer we have a regtest-only option to skip the check to impact integration tests the least.