💬 willcl-ark commented on pull request "Fee estimation: avoid serving stale fee estimate":
(https://github.com/bitcoin/bitcoin/pull/27622#discussion_r1219177648)
I know we only use it once in the test, but if you re-touch this file it could be nice to use a named variable for the 60 hours, to make it clear where it comes from? (e.g. `MAX_FILE_AGE` would shadow the name used in _fees.h_)
(https://github.com/bitcoin/bitcoin/pull/27622#discussion_r1219177648)
I know we only use it once in the test, but if you re-touch this file it could be nice to use a named variable for the 60 hours, to make it clear where it comes from? (e.g. `MAX_FILE_AGE` would shadow the name used in _fees.h_)
💬 hebasto commented on pull request "guix: Clean up manifest":
(https://github.com/bitcoin/bitcoin/pull/27811#issuecomment-1578197211)
Updated 86739705406edbe89bfb039a63fb33361990d673 -> 404a0e98dd964af4db338843f65ca3a63429db17 ([pr27811.02](https://github.com/hebasto/bitcoin/commits/pr27811.02) -> [pr27811.03](https://github.com/hebasto/bitcoin/commits/pr27811.03), [diff](https://github.com/hebasto/bitcoin/compare/pr27811.02..pr27811.03)):
- addressed @TheCharlatan's [comment](https://github.com/bitcoin/bitcoin/pull/27811#discussion_r1219147914)
(https://github.com/bitcoin/bitcoin/pull/27811#issuecomment-1578197211)
Updated 86739705406edbe89bfb039a63fb33361990d673 -> 404a0e98dd964af4db338843f65ca3a63429db17 ([pr27811.02](https://github.com/hebasto/bitcoin/commits/pr27811.02) -> [pr27811.03](https://github.com/hebasto/bitcoin/commits/pr27811.03), [diff](https://github.com/hebasto/bitcoin/compare/pr27811.02..pr27811.03)):
- addressed @TheCharlatan's [comment](https://github.com/bitcoin/bitcoin/pull/27811#discussion_r1219147914)
💬 hebasto commented on pull request "guix: Clean up manifest":
(https://github.com/bitcoin/bitcoin/pull/27811#discussion_r1219188992)
Thanks! [Updated](https://github.com/bitcoin/bitcoin/pull/27811#issuecomment-1578197211).
(https://github.com/bitcoin/bitcoin/pull/27811#discussion_r1219188992)
Thanks! [Updated](https://github.com/bitcoin/bitcoin/pull/27811#issuecomment-1578197211).
💬 hebasto commented on pull request "guix: Clean up manifest":
(https://github.com/bitcoin/bitcoin/pull/27811#discussion_r1219192511)
> What did this do?
Effectively, [nothing](https://github.com/bitcoin/bitcoin/pull/27779#issuecomment-1572754794).
(https://github.com/bitcoin/bitcoin/pull/27811#discussion_r1219192511)
> What did this do?
Effectively, [nothing](https://github.com/bitcoin/bitcoin/pull/27779#issuecomment-1572754794).
💬 MarcoFalke commented on issue "Intermittent failures in interface_usdt_mempool.py":
(https://github.com/bitcoin/bitcoin/issues/27380#issuecomment-1578204112)
Ok, closing for now. Let's reopen if this is an issue again.
(https://github.com/bitcoin/bitcoin/issues/27380#issuecomment-1578204112)
Ok, closing for now. Let's reopen if this is an issue again.
✅ MarcoFalke closed an issue: "Intermittent failures in interface_usdt_mempool.py"
(https://github.com/bitcoin/bitcoin/issues/27380)
(https://github.com/bitcoin/bitcoin/issues/27380)
💬 0xB10C commented on pull request "test: handle failed `check_equal()` assertions in bcc callback functions":
(https://github.com/bitcoin/bitcoin/pull/27831#issuecomment-1578237838)
Concept ACK. Thanks for picking this up! Seeing the asserts fail in the CI logs is needed and would have helped figuring out the reason for https://github.com/bitcoin/bitcoin/issues/27380.
(I think you meant to write `assert_equal()` instead of `check_equal()`)
(https://github.com/bitcoin/bitcoin/pull/27831#issuecomment-1578237838)
Concept ACK. Thanks for picking this up! Seeing the asserts fail in the CI logs is needed and would have helped figuring out the reason for https://github.com/bitcoin/bitcoin/issues/27380.
(I think you meant to write `assert_equal()` instead of `check_equal()`)
💬 0xB10C commented on issue "Intermittent failures in interface_usdt_mempool.py":
(https://github.com/bitcoin/bitcoin/issues/27380#issuecomment-1578262246)
> Since the failure hasn't been observed since https://github.com/bitcoin/bitcoin/pull/27177 was merged, there's a good chance it fixed the issue.
For future reference, adding a bit more details on this what we think was happening here. We couldn't actually verify it as we haven't been able to reproduce this locally and the assertions in the callback aren't logged in the CI logs (fixed in #27831).
The `mempool:rejected` interface test constructs a zero-fee transaction and sends it to the
...
(https://github.com/bitcoin/bitcoin/issues/27380#issuecomment-1578262246)
> Since the failure hasn't been observed since https://github.com/bitcoin/bitcoin/pull/27177 was merged, there's a good chance it fixed the issue.
For future reference, adding a bit more details on this what we think was happening here. We couldn't actually verify it as we haven't been able to reproduce this locally and the assertions in the callback aren't logged in the CI logs (fixed in #27831).
The `mempool:rejected` interface test constructs a zero-fee transaction and sends it to the
...
💬 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