💬 laanwj commented on pull request "depends: Remove Qt build-time dependencies":
(https://github.com/bitcoin/bitcoin/pull/29923#issuecomment-2082089346)
Rebased for merge conflict and to apply fixup commits.
(https://github.com/bitcoin/bitcoin/pull/29923#issuecomment-2082089346)
Rebased for merge conflict and to apply fixup commits.
👍 maflcko approved a pull request: "fuzz: don't allow adding duplicate transactions to the mempool"
(https://github.com/bitcoin/bitcoin/pull/29990#pullrequestreview-2027780727)
lgtm ACK cc15c5bfd1eb3903de246c124702a7c66c687008
I guess this isn't due to a fuzz crash, because the only thing that's off is `cachedInnerUsage` (and friends), which are not checked in this fuzz target?
(https://github.com/bitcoin/bitcoin/pull/29990#pullrequestreview-2027780727)
lgtm ACK cc15c5bfd1eb3903de246c124702a7c66c687008
I guess this isn't due to a fuzz crash, because the only thing that's off is `cachedInnerUsage` (and friends), which are not checked in this fuzz target?
💬 laanwj commented on pull request "depends: Fix build of Qt for 32-bit platforms with recent glibc":
(https://github.com/bitcoin/bitcoin/pull/29985#issuecomment-2082091570)
Force-pushed to apply @hebasto's comment to make the patch apply cleanly.
(https://github.com/bitcoin/bitcoin/pull/29985#issuecomment-2082091570)
Force-pushed to apply @hebasto's comment to make the patch apply cleanly.
💬 virtu commented on pull request "contrib: Add asmap-tool":
(https://github.com/bitcoin/bitcoin/pull/28793#issuecomment-2082138376)
ACK [6abe772](https://github.com/bitcoin/bitcoin/commit/6abe772a17e09fe96e68cd3311280d5a30f6378b)
Tested decoding and encoding some [demo ASMaps](https://github.com/fjahr/asmap-data).
Also made sure encoding yields a reproducible ordering by decoding, `shuf`'ling, and re-encoded an ASMap and making sure the resulting encoding was identical to the original one.
(https://github.com/bitcoin/bitcoin/pull/28793#issuecomment-2082138376)
ACK [6abe772](https://github.com/bitcoin/bitcoin/commit/6abe772a17e09fe96e68cd3311280d5a30f6378b)
Tested decoding and encoding some [demo ASMaps](https://github.com/fjahr/asmap-data).
Also made sure encoding yields a reproducible ordering by decoding, `shuf`'ling, and re-encoded an ASMap and making sure the resulting encoding was identical to the original one.
👋 Sjors's pull request is ready for review: "doc: add LLVM instruction for macOS < 13"
(https://github.com/bitcoin/bitcoin/pull/29934)
(https://github.com/bitcoin/bitcoin/pull/29934)
💬 Sjors commented on pull request "doc: add LLVM instruction for macOS < 13":
(https://github.com/bitcoin/bitcoin/pull/29934#issuecomment-2082144574)
I dropped the `@14` from the default instruction, but added a hint to try `@17` if that fails. That worked when I tested and I don't expect Homebrew formulas for older versions to change much moving forward, nor llvm 17 itself.
(https://github.com/bitcoin/bitcoin/pull/29934#issuecomment-2082144574)
I dropped the `@14` from the default instruction, but added a hint to try `@17` if that fails. That worked when I tested and I don't expect Homebrew formulas for older versions to change much moving forward, nor llvm 17 itself.
💬 Sjors commented on pull request "build: Bump clang minimum supported version to 15":
(https://github.com/bitcoin/bitcoin/pull/29165#issuecomment-2082152250)
@maflcko the updated wording in #29934 is such this PR won't conflict with it.
(https://github.com/bitcoin/bitcoin/pull/29165#issuecomment-2082152250)
@maflcko the updated wording in #29934 is such this PR won't conflict with it.
💬 maflcko commented on pull request "doc: add LLVM instruction for macOS < 13":
(https://github.com/bitcoin/bitcoin/pull/29934#issuecomment-2082159132)
utACK 22574046c90c0662f3aa9b1baea074aff54f92a9
(https://github.com/bitcoin/bitcoin/pull/29934#issuecomment-2082159132)
utACK 22574046c90c0662f3aa9b1baea074aff54f92a9
💬 Sjors commented on issue "Add IPv6 pinhole support using UPnP / NAT-PMP":
(https://github.com/bitcoin/bitcoin/issues/17012#issuecomment-2082162255)
> The good part is that it's just a matter of sending one fixed-size binary UDP packet to the default gateway and parsing the result.
Nice!
(https://github.com/bitcoin/bitcoin/issues/17012#issuecomment-2082162255)
> The good part is that it's just a matter of sending one fixed-size binary UDP packet to the default gateway and parsing the result.
Nice!
💬 laanwj commented on issue "Add IPv6 pinhole support using UPnP / NAT-PMP":
(https://github.com/bitcoin/bitcoin/issues/17012#issuecomment-2082215034)
> Nice!
If you'd like to test, i have a branch here: https://github.com/laanwj/bitcoin/tree/2024-04-pcp-pinhole
It is a PoC that replaces [bitcoind.cpp](https://github.com/laanwj/bitcoin/blob/2024-04-pcp-pinhole/src/bitcoind.cpp) with a program that (on Linux only for now):
- Enumerates local publicly routable IPv6 addresses
- Gets the default gateway to get the PCP endpoint
- Requests pinholes for 100 seconds to port 1234 on all addreses, and prints the result
i've tried it on two r
...
(https://github.com/bitcoin/bitcoin/issues/17012#issuecomment-2082215034)
> Nice!
If you'd like to test, i have a branch here: https://github.com/laanwj/bitcoin/tree/2024-04-pcp-pinhole
It is a PoC that replaces [bitcoind.cpp](https://github.com/laanwj/bitcoin/blob/2024-04-pcp-pinhole/src/bitcoind.cpp) with a program that (on Linux only for now):
- Enumerates local publicly routable IPv6 addresses
- Gets the default gateway to get the PCP endpoint
- Requests pinholes for 100 seconds to port 1234 on all addreses, and prints the result
i've tried it on two r
...
💬 0xB10C commented on pull request "tracing: cast block_connected duration to µs":
(https://github.com/bitcoin/bitcoin/pull/29877#issuecomment-2082221803)
> Any reason not to just stick to nanoseconds and harden it against future accidents?
I think updating the documentation and using `Ticks<std::chrono::nanoseconds>` would work just as well, yes..
(https://github.com/bitcoin/bitcoin/pull/29877#issuecomment-2082221803)
> Any reason not to just stick to nanoseconds and harden it against future accidents?
I think updating the documentation and using `Ticks<std::chrono::nanoseconds>` would work just as well, yes..
💬 sdaftuar commented on pull request "fuzz: don't allow adding duplicate transactions to the mempool":
(https://github.com/bitcoin/bitcoin/pull/29990#issuecomment-2082230525)
> I guess this isn't due to a fuzz crash, because the only thing that's off is `cachedInnerUsage` (and friends), which are not checked in this fuzz target?
Correct, I didn't see any fuzz crash on master, but this issue was leading to crashes in my cluster mempool branch, where the inconsistency manifested itself sooner.
(https://github.com/bitcoin/bitcoin/pull/29990#issuecomment-2082230525)
> I guess this isn't due to a fuzz crash, because the only thing that's off is `cachedInnerUsage` (and friends), which are not checked in this fuzz target?
Correct, I didn't see any fuzz crash on master, but this issue was leading to crashes in my cluster mempool branch, where the inconsistency manifested itself sooner.
📝 luchenhan opened a pull request: "chore: fix some typos"
(https://github.com/bitcoin/bitcoin/pull/29992)
<!--
*** Please remove the following help text before submitting: ***
Pull requests without a rationale and clear improvement may be closed
immediately.
GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->
<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:
* Any test improvements or new tests that improv
...
(https://github.com/bitcoin/bitcoin/pull/29992)
<!--
*** Please remove the following help text before submitting: ***
Pull requests without a rationale and clear improvement may be closed
immediately.
GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->
<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:
* Any test improvements or new tests that improv
...
✅ fanquake closed a pull request: "chore: fix some typos"
(https://github.com/bitcoin/bitcoin/pull/29992)
(https://github.com/bitcoin/bitcoin/pull/29992)
💬 fanquake commented on pull request "chore: fix some typos":
(https://github.com/bitcoin/bitcoin/pull/29992#issuecomment-2082247128)
Please send changes to the minisketch subtree to the upstream repository. See the notes on subtrees in the [developer-notes](https://github.com/bitcoin/bitcoin/blob/master/doc/developer-notes.md#subtrees).
The change to the Boost macro is also in upstream code.
(https://github.com/bitcoin/bitcoin/pull/29992#issuecomment-2082247128)
Please send changes to the minisketch subtree to the upstream repository. See the notes on subtrees in the [developer-notes](https://github.com/bitcoin/bitcoin/blob/master/doc/developer-notes.md#subtrees).
The change to the Boost macro is also in upstream code.
👍 hebasto approved a pull request: "depends: Fix build of Qt for 32-bit platforms with recent glibc"
(https://github.com/bitcoin/bitcoin/pull/29985#pullrequestreview-2027951391)
re-ACK 2e266f33b5d2be5c233c2c692481f75785714fa1.
(https://github.com/bitcoin/bitcoin/pull/29985#pullrequestreview-2027951391)
re-ACK 2e266f33b5d2be5c233c2c692481f75785714fa1.
🤔 virtu reviewed a pull request: "tracing: network connection tracepoints"
(https://github.com/bitcoin/bitcoin/pull/25832#pullrequestreview-2027891049)
ACK [996029b](https://github.com/bitcoin/bitcoin/pull/25832/commits/996029b1fdc131b5a65c34b4898a515d523268d3) (modulo comment and [this](https://github.com/bitcoin/bitcoin/pull/25832/files/996029b1fdc131b5a65c34b4898a515d523268d3#diff-adf830d8b5ab19af24c3f9be2cb2c8a14fa49455d22613c50adfd5e3cd583013))
Successfully ran `log_p2p_connections.bt` over the weekend on both x86_64/clang and aarch64/gcc.
Extended functional test (`interface_usdt_net.py`) equally passed on both machines.
(https://github.com/bitcoin/bitcoin/pull/25832#pullrequestreview-2027891049)
ACK [996029b](https://github.com/bitcoin/bitcoin/pull/25832/commits/996029b1fdc131b5a65c34b4898a515d523268d3) (modulo comment and [this](https://github.com/bitcoin/bitcoin/pull/25832/files/996029b1fdc131b5a65c34b4898a515d523268d3#diff-adf830d8b5ab19af24c3f9be2cb2c8a14fa49455d22613c50adfd5e3cd583013))
Successfully ran `log_p2p_connections.bt` over the weekend on both x86_64/clang and aarch64/gcc.
Extended functional test (`interface_usdt_net.py`) equally passed on both machines.
💬 virtu commented on pull request "tracing: network connection tracepoints":
(https://github.com/bitcoin/bitcoin/pull/25832#discussion_r1582727992)
```suggestion
OUTBOUND conn to 127.0.0.1:15287: id=0, type=block-relay-only, network=0, total_out=1
INBOUND conn from 127.0.0.1:45324: id=1, type=inbound, network=0, total_in=1
MISBEHAVING conn id=1, score_before=0, score_increase=20, message='getdata message size = 50001', threshold_exceeded=false
CLOSED conn to 127.0.0.1:15287: id=0, type=block-relay-only, network=0, established=1231006505
EVICTED conn to 127.0.0.1:45324: id=1, type=inbound, network=0, established=1612312312
```
(https://github.com/bitcoin/bitcoin/pull/25832#discussion_r1582727992)
```suggestion
OUTBOUND conn to 127.0.0.1:15287: id=0, type=block-relay-only, network=0, total_out=1
INBOUND conn from 127.0.0.1:45324: id=1, type=inbound, network=0, total_in=1
MISBEHAVING conn id=1, score_before=0, score_increase=20, message='getdata message size = 50001', threshold_exceeded=false
CLOSED conn to 127.0.0.1:15287: id=0, type=block-relay-only, network=0, established=1231006505
EVICTED conn to 127.0.0.1:45324: id=1, type=inbound, network=0, established=1612312312
```
💬 ismaelsadeeq commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1582768678)
I learned that we can RBF a transaction that gets into the mempool using carveout. I wanted to test that you wouldn't RBF that transaction using a package.
Even if the arrangement attempts to RBF or not, I think the error in the suggestion is how it would fail. We would set `m_allow_carveout` to false while evaluating the first parent transaction and fail its evaluation due to a `TX_MEMPOOL_POLICY` violation of `too-long-mempool-chain`. hence, all the descendants would fail due to missing
...
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1582768678)
I learned that we can RBF a transaction that gets into the mempool using carveout. I wanted to test that you wouldn't RBF that transaction using a package.
Even if the arrangement attempts to RBF or not, I think the error in the suggestion is how it would fail. We would set `m_allow_carveout` to false while evaluating the first parent transaction and fail its evaluation due to a `TX_MEMPOOL_POLICY` violation of `too-long-mempool-chain`. hence, all the descendants would fail due to missing
...
💬 willcl-ark commented on pull request "rename bitcoin.conf in dist tarball":
(https://github.com/bitcoin/bitcoin/pull/29903#issuecomment-2082267922)
@josibake @laanwj I'm happy to close this (and the issue) based on discussion above and in agreement with @pinheadmz [comment](https://github.com/bitcoin/bitcoin/issues/29139#issuecomment-2068038387) on the issue, if you both agree?
(https://github.com/bitcoin/bitcoin/pull/29903#issuecomment-2082267922)
@josibake @laanwj I'm happy to close this (and the issue) based on discussion above and in agreement with @pinheadmz [comment](https://github.com/bitcoin/bitcoin/issues/29139#issuecomment-2068038387) on the issue, if you both agree?