๐ฌ 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?
๐ฌ maflcko commented on issue "intermittent issue in wallet_backwards_compatibility.py: line 245, in run_test assert txs[3]["abandoned"] AssertionError":
(https://github.com/bitcoin/bitcoin/issues/29806#issuecomment-2082271022)
> The failure can be reproduced locally by wrapping `callbacks.TransactionAddedToMempool` and `callbacks.TransactionRemovedFromMempool` (in `validationinterface.cpp`) with a lambda introducing an `UninterruptibleSleep(40ms)`, and calling `self.sync_mempools()` after `node_master.abandontransaction(tx3_id)` in `wallet_backwards_compatibility.py`
Can you produce a diff for this please, so that it can be applied locally by reviewers?
(https://github.com/bitcoin/bitcoin/issues/29806#issuecomment-2082271022)
> The failure can be reproduced locally by wrapping `callbacks.TransactionAddedToMempool` and `callbacks.TransactionRemovedFromMempool` (in `validationinterface.cpp`) with a lambda introducing an `UninterruptibleSleep(40ms)`, and calling `self.sync_mempools()` after `node_master.abandontransaction(tx3_id)` in `wallet_backwards_compatibility.py`
Can you produce a diff for this please, so that it can be applied locally by reviewers?
๐ฌ brunoerg commented on pull request "fuzz: wallet, add target for `Crypter`":
(https://github.com/bitcoin/bitcoin/pull/28074#issuecomment-2082278225)
Up for grabs?
(https://github.com/bitcoin/bitcoin/pull/28074#issuecomment-2082278225)
Up for grabs?
๐ฌ laanwj commented on pull request "guix: build with glibc 2.31":
(https://github.com/bitcoin/bitcoin/pull/29987#discussion_r1582783359)
Good to get rid of these libc patches, especially the rv64 one.
(https://github.com/bitcoin/bitcoin/pull/29987#discussion_r1582783359)
Good to get rid of these libc patches, especially the rv64 one.
๐ฌ laanwj commented on issue "guix: re-enable exported symbol checking for RISC-V":
(https://github.com/bitcoin/bitcoin/issues/28095#issuecomment-2082291945)
Holding this off until #29987 is.merged.
(https://github.com/bitcoin/bitcoin/issues/28095#issuecomment-2082291945)
Holding this off until #29987 is.merged.
๐ฌ Sjors commented on pull request "rpc: Optimize serialization and enhance metadata of dumptxoutset output":
(https://github.com/bitcoin/bitcoin/pull/29612#issuecomment-2082344035)
> I can try to split it up if it makes review easier.
Doesn't matter that much, but it's probably to put the `kernel/chainparams.h` change in its own commit so e.g. @TheCharlatan can do a drive-by review.
Here's some new snapshot torrents (not sure if I'm seeding them correctly...).
Torrent for testnet: `magnet:?xt=urn:btih:787f5917029876c83301c49dc97bb69224597285&dn=utxo-testnet-2500000.dat&tr=udp%3A%2F%2Ftracker.bitcoin.sprovoost.nl%3A6969`
Torrent for signet: `magnet:?xt=urn:btih:
...
(https://github.com/bitcoin/bitcoin/pull/29612#issuecomment-2082344035)
> I can try to split it up if it makes review easier.
Doesn't matter that much, but it's probably to put the `kernel/chainparams.h` change in its own commit so e.g. @TheCharlatan can do a drive-by review.
Here's some new snapshot torrents (not sure if I'm seeding them correctly...).
Torrent for testnet: `magnet:?xt=urn:btih:787f5917029876c83301c49dc97bb69224597285&dn=utxo-testnet-2500000.dat&tr=udp%3A%2F%2Ftracker.bitcoin.sprovoost.nl%3A6969`
Torrent for signet: `magnet:?xt=urn:btih:
...
๐ฌ Sjors commented on pull request "rpc: Optimize serialization and enhance metadata of dumptxoutset output":
(https://github.com/bitcoin/bitcoin/pull/29612#discussion_r1582744045)
6ed6bffcbedb6a28c3578721cc0a08086a15e417: this doesn't seem kernel-worthy. `chaintype.h` is a better home for it, next to `ChainTypeToString`.
(https://github.com/bitcoin/bitcoin/pull/29612#discussion_r1582744045)
6ed6bffcbedb6a28c3578721cc0a08086a15e417: this doesn't seem kernel-worthy. `chaintype.h` is a better home for it, next to `ChainTypeToString`.