💬 naumenkogs commented on pull request "net: Add new permission `forceinbound` to evict a random unprotected connection if all slots are otherwise full":
(https://github.com/bitcoin/bitcoin/pull/27600#discussion_r1387612308)
311902f2cf94ee0e11ed34a1373db42dbc218b20
should this be exposed in peer info?
(https://github.com/bitcoin/bitcoin/pull/27600#discussion_r1387612308)
311902f2cf94ee0e11ed34a1373db42dbc218b20
should this be exposed in peer info?
💬 naumenkogs commented on pull request "net: Add new permission `forceinbound` to evict a random unprotected connection if all slots are otherwise full":
(https://github.com/bitcoin/bitcoin/pull/27600#discussion_r1387598983)
I'd suggest expanding this comment to explain what exactly happened.
(https://github.com/bitcoin/bitcoin/pull/27600#discussion_r1387598983)
I'd suggest expanding this comment to explain what exactly happened.
💬 naumenkogs commented on pull request "net: Add new permission `forceinbound` to evict a random unprotected connection if all slots are otherwise full":
(https://github.com/bitcoin/bitcoin/pull/27600#discussion_r1387600158)
This still remains default, no?
(https://github.com/bitcoin/bitcoin/pull/27600#discussion_r1387600158)
This still remains default, no?
💬 naumenkogs commented on pull request "multiprocess compatibility updates":
(https://github.com/bitcoin/bitcoin/pull/28721#issuecomment-1803337313)
ACK 3b70f7b6156cb110c47a6e482791cf337bb6ad6d
(https://github.com/bitcoin/bitcoin/pull/28721#issuecomment-1803337313)
ACK 3b70f7b6156cb110c47a6e482791cf337bb6ad6d
💬 glozow commented on pull request "refactor: Simplify CTxMempool/BlockAssembler fields, remove some external mapTx access":
(https://github.com/bitcoin/bitcoin/pull/28391#issuecomment-1803462865)
reACK 2be5e799ba623f969f5ffc59667a5bc6799df290
(https://github.com/bitcoin/bitcoin/pull/28391#issuecomment-1803462865)
reACK 2be5e799ba623f969f5ffc59667a5bc6799df290
🚀 glozow merged a pull request: "refactor: Miniminer package linearization followups"
(https://github.com/bitcoin/bitcoin/pull/28808)
(https://github.com/bitcoin/bitcoin/pull/28808)
💬 glozow commented on pull request "refactor: Miniminer package linearization followups":
(https://github.com/bitcoin/bitcoin/pull/28808#issuecomment-1803470344)
@kevkevinpal (nit) in the future, it's best to delete the help text before you open the PR as it gets pulled into the merge commit
(https://github.com/bitcoin/bitcoin/pull/28808#issuecomment-1803470344)
@kevkevinpal (nit) in the future, it's best to delete the help text before you open the PR as it gets pulled into the merge commit
💬 MatthewLM commented on pull request "Use LE hex-encoded representations in script ASM for pushed values <= 4 bytes":
(https://github.com/bitcoin/bitcoin/pull/28824#issuecomment-1803471329)
In the dart library coinlib (https://pub.dev/packages/coinlib) I followed the same approach of encoding everything in HEX with integers encoded as LE. However, 0 and -1 are encoded as 0 and -1. See the code here: https://github.com/peercoin/coinlib/blob/master/coinlib/lib/src/scripts/operations.dart
I did this to avoid changing the ASM too much. However, I'm happy to move towards using `0x` for hex and presenting decimal for smaller data of up-to 32bits if this is introduced in Bitcoin. This
...
(https://github.com/bitcoin/bitcoin/pull/28824#issuecomment-1803471329)
In the dart library coinlib (https://pub.dev/packages/coinlib) I followed the same approach of encoding everything in HEX with integers encoded as LE. However, 0 and -1 are encoded as 0 and -1. See the code here: https://github.com/peercoin/coinlib/blob/master/coinlib/lib/src/scripts/operations.dart
I did this to avoid changing the ASM too much. However, I'm happy to move towards using `0x` for hex and presenting decimal for smaller data of up-to 32bits if this is introduced in Bitcoin. This
...
💬 maflcko commented on pull request "Show transactions as not fully confirmed during background validation":
(https://github.com/bitcoin/bitcoin/pull/28616#discussion_r1387766074)
Maybe submit this in a separate pull for review?
(https://github.com/bitcoin/bitcoin/pull/28616#discussion_r1387766074)
Maybe submit this in a separate pull for review?
💬 dergoegge commented on pull request "fuzz: Minor improvements to tx_package_eval target":
(https://github.com/bitcoin/bitcoin/pull/28825#discussion_r1387708805)
Maybe put this in `test/util/script.h`?
(https://github.com/bitcoin/bitcoin/pull/28825#discussion_r1387708805)
Maybe put this in `test/util/script.h`?
📝 fanquake locked a pull request: "[doc]: uppercase title for "Assumeutxo""
(https://github.com/bitcoin/bitcoin/pull/28827)
uppercase title from `# assumeutxo` to `# Assumeutxo`
(https://github.com/bitcoin/bitcoin/pull/28827)
uppercase title from `# assumeutxo` to `# Assumeutxo`
💬 dergoegge commented on issue "fuzz: Fix timeouts":
(https://github.com/bitcoin/bitcoin/issues/28812#issuecomment-1803538946)
> I can add steps to reproduce, if you want.
That would be great.
(https://github.com/bitcoin/bitcoin/issues/28812#issuecomment-1803538946)
> I can add steps to reproduce, if you want.
That would be great.
💬 fanquake commented on pull request "build: Patch Qt to handle minimum macOS version properly":
(https://github.com/bitcoin/bitcoin/pull/28775#issuecomment-1803550311)
Another thing to note, is that the code being patched here, has been deleted entirely upstream: https://github.com/qt/qtbase/commit/329db8b64f17c8ef013c586cea1f1c5b49c4a4b7.
> macOS: Remove fallback defines for MAC_OS_X_VERSION_MIN_REQUIRED
> Availability.h from the platform SDK should take care of this these days.
So this points to there still being an underlying issue, and workaround will "break" if/when we start using newer versions of Qt.
(https://github.com/bitcoin/bitcoin/pull/28775#issuecomment-1803550311)
Another thing to note, is that the code being patched here, has been deleted entirely upstream: https://github.com/qt/qtbase/commit/329db8b64f17c8ef013c586cea1f1c5b49c4a4b7.
> macOS: Remove fallback defines for MAC_OS_X_VERSION_MIN_REQUIRED
> Availability.h from the platform SDK should take care of this these days.
So this points to there still being an underlying issue, and workaround will "break" if/when we start using newer versions of Qt.
🚀 fanquake merged a pull request: "test: Add missing wait for version to be sent in add_outbound_p2p_connection"
(https://github.com/bitcoin/bitcoin/pull/28822)
(https://github.com/bitcoin/bitcoin/pull/28822)
💬 maflcko commented on pull request "fuzz: call lookup functions before calling `Ban`":
(https://github.com/bitcoin/bitcoin/pull/27935#issuecomment-1803592254)
> I did some experiments and I decided to keep the lookup functions. Without it, I still get discrepances even avoiding the comparison when we call Ban with an invalid subnet/netaddr.
Would be good to explain this a bit more, to make sure this is not a bug.
(https://github.com/bitcoin/bitcoin/pull/27935#issuecomment-1803592254)
> I did some experiments and I decided to keep the lookup functions. Without it, I still get discrepances even avoiding the comparison when we call Ban with an invalid subnet/netaddr.
Would be good to explain this a bit more, to make sure this is not a bug.
💬 fanquake commented on pull request "doc: update docs for `CHECK_ATOMIC` macro":
(https://github.com/bitcoin/bitcoin/pull/28777#issuecomment-1803597897)
> Could this moment be now?
Feel free to open an issue for broader discussion. Given the overhead of supporting it is low, I don't see a pressing reason to do so. There is also a difference between us officially dropping support, and purposefully breaking the ability to compile for certain targets.
(https://github.com/bitcoin/bitcoin/pull/28777#issuecomment-1803597897)
> Could this moment be now?
Feel free to open an issue for broader discussion. Given the overhead of supporting it is low, I don't see a pressing reason to do so. There is also a difference between us officially dropping support, and purposefully breaking the ability to compile for certain targets.
📝 fanquake opened a pull request: "ci: win64 task does use boost:process"
(https://github.com/bitcoin/bitcoin/pull/28829)
It passes `--enable-external-signer`.
(https://github.com/bitcoin/bitcoin/pull/28829)
It passes `--enable-external-signer`.
💬 maflcko commented on issue "fuzz: Fix timeouts":
(https://github.com/bitcoin/bitcoin/issues/28812#issuecomment-1803615609)
Steps to reproduce flamegraph:
* Compile fuzz tests normally (libFuzzer recommended)
* `perf record`, for example via `FUZZ=mocked_descriptor_parse perf record -g --call-graph dwarf -F 1001 ./src/test/fuzz/fuzz -runs=2 /tmp/fuzz_input.dat` (`-runs` can be increased to discount an expensive global setup initialization, `--call-graph` and frequency can be modified depending on your system)
* Load the data as a flamegraph: `hotspot ./perf.data`
(https://github.com/bitcoin/bitcoin/issues/28812#issuecomment-1803615609)
Steps to reproduce flamegraph:
* Compile fuzz tests normally (libFuzzer recommended)
* `perf record`, for example via `FUZZ=mocked_descriptor_parse perf record -g --call-graph dwarf -F 1001 ./src/test/fuzz/fuzz -runs=2 /tmp/fuzz_input.dat` (`-runs` can be increased to discount an expensive global setup initialization, `--call-graph` and frequency can be modified depending on your system)
* Load the data as a flamegraph: `hotspot ./perf.data`
💬 maflcko commented on issue "fuzz: Fix timeouts":
(https://github.com/bitcoin/bitcoin/issues/28812#issuecomment-1803617915)
cc @darosior about `mocked_descriptor_parse`
(https://github.com/bitcoin/bitcoin/issues/28812#issuecomment-1803617915)
cc @darosior about `mocked_descriptor_parse`
💬 darosior commented on issue "fuzz: Fix timeouts":
(https://github.com/bitcoin/bitcoin/issues/28812#issuecomment-1803618906)
Yeah i was looking into it. It's surprising we are spending so long inside the dup key check. Maybe we re-introduced the issue fixed by https://github.com/bitcoin/bitcoin/pull/25540.
(https://github.com/bitcoin/bitcoin/issues/28812#issuecomment-1803618906)
Yeah i was looking into it. It's surprising we are spending so long inside the dup key check. Maybe we re-introduced the issue fixed by https://github.com/bitcoin/bitcoin/pull/25540.