💬 laanwj commented on pull request "net: Replace libnatpmp with built-in PCP implementation":
(https://github.com/bitcoin/bitcoin/pull/30043#issuecomment-2117004469)
Added a (mostly untested for now) Windows implementation of `QueryDefaultGateway`, if someone could test this it'd be very helpful. i will try in WINE later.
(https://github.com/bitcoin/bitcoin/pull/30043#issuecomment-2117004469)
Added a (mostly untested for now) Windows implementation of `QueryDefaultGateway`, if someone could test this it'd be very helpful. i will try in WINE later.
💬 josibake commented on pull request "Fee Estimation: Ignore all transactions with an in-block child":
(https://github.com/bitcoin/bitcoin/pull/30079#discussion_r1604550196)
ah, my bad. I had even read that comment that @glozow linked to earlier but it didn't click for me that by the time we get to the block we've already processed transactions and excluded the child transaction. I think a comment explaining what you just explained here would help a lot, maybe at the call site of `removeParentTxs`?
(https://github.com/bitcoin/bitcoin/pull/30079#discussion_r1604550196)
ah, my bad. I had even read that comment that @glozow linked to earlier but it didn't click for me that by the time we get to the block we've already processed transactions and excluded the child transaction. I think a comment explaining what you just explained here would help a lot, maybe at the call site of `removeParentTxs`?
💬 glozow commented on pull request "locks: introduce mutex for tx download, flush rejection filters on UpdatedBlockTip":
(https://github.com/bitcoin/bitcoin/pull/30111#discussion_r1604554237)
Done
(https://github.com/bitcoin/bitcoin/pull/30111#discussion_r1604554237)
Done
👍 TheCharlatan approved a pull request: "kernel: Streamline util library"
(https://github.com/bitcoin/bitcoin/pull/29015#pullrequestreview-2062665306)
Re-ACK 1c26d10b23bb7a7d0204b4a8b44a50fad89f2a2a
(https://github.com/bitcoin/bitcoin/pull/29015#pullrequestreview-2062665306)
Re-ACK 1c26d10b23bb7a7d0204b4a8b44a50fad89f2a2a
✅ laanwj closed a pull request: "[PoC] qt, depends: Add wayland support without build-time nor fixed run-time deps"
(https://github.com/bitcoin/bitcoin/pull/29959)
(https://github.com/bitcoin/bitcoin/pull/29959)
💬 laanwj commented on pull request "[PoC] qt, depends: Add wayland support without build-time nor fixed run-time deps":
(https://github.com/bitcoin/bitcoin/pull/29959#issuecomment-2117037519)
Closing for now. It exists as a PoC, if anyone is truly interested in wayland support in bitcoin core's binary releases, and interested in helping test and review, let me know. Right now, it seems something people mostly ask about just to ask about.
(https://github.com/bitcoin/bitcoin/pull/29959#issuecomment-2117037519)
Closing for now. It exists as a PoC, if anyone is truly interested in wayland support in bitcoin core's binary releases, and interested in helping test and review, let me know. Right now, it seems something people mostly ask about just to ask about.
💬 willcl-ark commented on pull request "[PoC] qt, depends: Add wayland support without build-time nor fixed run-time deps":
(https://github.com/bitcoin/bitcoin/pull/29959#issuecomment-2117042080)
Thanks @laanwj
Perhaps we close #19950 with the same rationale?
(https://github.com/bitcoin/bitcoin/pull/29959#issuecomment-2117042080)
Thanks @laanwj
Perhaps we close #19950 with the same rationale?
💬 laanwj commented on pull request "[PoC] qt, depends: Add wayland support without build-time nor fixed run-time deps":
(https://github.com/bitcoin/bitcoin/pull/29959#issuecomment-2117053619)
> Perhaps we close https://github.com/bitcoin/bitcoin/issues/19950 with the same rationale?
i'm not sure. Pretty sure it's something that people will open new issues about endlessly and then forget about them. Might as well leave this one open so there's context.
(https://github.com/bitcoin/bitcoin/pull/29959#issuecomment-2117053619)
> Perhaps we close https://github.com/bitcoin/bitcoin/issues/19950 with the same rationale?
i'm not sure. Pretty sure it's something that people will open new issues about endlessly and then forget about them. Might as well leave this one open so there's context.
💬 laanwj commented on pull request "rpc: introduce getversion RPC":
(https://github.com/bitcoin/bitcoin/pull/30112#issuecomment-2117084796)
> I'm partial to getnodeversion or getclientversion, since RPC is implicitly versioned by the major version of the client/node
The problem with those, imo, is that `client` and `node` are both words associated with general networking. They might as well return P2P protocol version information. If this call is intended to return RPC versions and capabilities, then that should probably be explicit in the name and documentation. But maybe i'm misunderstanding the intent.
> If in the future RP
...
(https://github.com/bitcoin/bitcoin/pull/30112#issuecomment-2117084796)
> I'm partial to getnodeversion or getclientversion, since RPC is implicitly versioned by the major version of the client/node
The problem with those, imo, is that `client` and `node` are both words associated with general networking. They might as well return P2P protocol version information. If this call is intended to return RPC versions and capabilities, then that should probably be explicit in the name and documentation. But maybe i'm misunderstanding the intent.
> If in the future RP
...
💬 glozow commented on pull request "refactor prep for package rbf":
(https://github.com/bitcoin/bitcoin/pull/30072#discussion_r1604604969)
Agree there's room for improvement clarity-wise.
Perhaps a good approach is to have each flag control 1 part of the validation logic (even if 1 flag implies another flag)
- whether you call `LimitMempoolSize()` within `Finalize()`
- whether all conflicts should be rejected immediately
- whether `CheckFeeRate` should be skipped in `PreChecks`
- whether sibling eviction can be considered
- whether package RBF can be considered
The `ATMPArgs` constructors should decide the flags based on
...
(https://github.com/bitcoin/bitcoin/pull/30072#discussion_r1604604969)
Agree there's room for improvement clarity-wise.
Perhaps a good approach is to have each flag control 1 part of the validation logic (even if 1 flag implies another flag)
- whether you call `LimitMempoolSize()` within `Finalize()`
- whether all conflicts should be rejected immediately
- whether `CheckFeeRate` should be skipped in `PreChecks`
- whether sibling eviction can be considered
- whether package RBF can be considered
The `ATMPArgs` constructors should decide the flags based on
...
🤔 glozow reviewed a pull request: "test: expand LimitOrphan and EraseForPeer coverage"
(https://github.com/bitcoin/bitcoin/pull/30082#pullrequestreview-2058508184)
lgtm
(https://github.com/bitcoin/bitcoin/pull/30082#pullrequestreview-2058508184)
lgtm
💬 glozow commented on pull request "test: expand LimitOrphan and EraseForPeer coverage":
(https://github.com/bitcoin/bitcoin/pull/30082#discussion_r1601941249)
Note you've only jumped 1 second here
(https://github.com/bitcoin/bitcoin/pull/30082#discussion_r1601941249)
Note you've only jumped 1 second here
💬 glozow commented on pull request "test: expand LimitOrphan and EraseForPeer coverage":
(https://github.com/bitcoin/bitcoin/pull/30082#discussion_r1601942742)
Would be nice if the constants were moved to txorphanage.h so we don't need the magic number here. Could be useful in other tests as well.
(https://github.com/bitcoin/bitcoin/pull/30082#discussion_r1601942742)
Would be nice if the constants were moved to txorphanage.h so we don't need the magic number here. Could be useful in other tests as well.
👍 hebasto approved a pull request: "kernel: Streamline util library"
(https://github.com/bitcoin/bitcoin/pull/29015#pullrequestreview-2062749982)
re-ACK 1c26d10b23bb7a7d0204b4a8b44a50fad89f2a2a.
(https://github.com/bitcoin/bitcoin/pull/29015#pullrequestreview-2062749982)
re-ACK 1c26d10b23bb7a7d0204b4a8b44a50fad89f2a2a.
💬 hebasto commented on pull request "[PoC] qt, depends: Add wayland support without build-time nor fixed run-time deps":
(https://github.com/bitcoin/bitcoin/pull/29959#issuecomment-2117111606)
@laanwj
> Closing for now. It exists as a PoC, if anyone is truly interested in wayland support in bitcoin core's binary releases, and interested in helping test and review, let me know.
I'm sorry for not paying attention to this great PR. I just don't have time to focus on it at the moment. It is about to be changed soon :)
(https://github.com/bitcoin/bitcoin/pull/29959#issuecomment-2117111606)
@laanwj
> Closing for now. It exists as a PoC, if anyone is truly interested in wayland support in bitcoin core's binary releases, and interested in helping test and review, let me know.
I'm sorry for not paying attention to this great PR. I just don't have time to focus on it at the moment. It is about to be changed soon :)
💬 laanwj commented on issue "stringop-overflow warning with GCC 14":
(https://github.com/bitcoin/bitcoin/issues/30114#issuecomment-2117118213)
Looks like it derives that `.begin()` is the same as `.end()` which would bring the size to `-1`. This would require `CPubKey::GetLen` to return 0, which is possible if the first byte is invalid. Maybe it'd make sense to make it return `1` in that case because there's always the first byte?
(https://github.com/bitcoin/bitcoin/issues/30114#issuecomment-2117118213)
Looks like it derives that `.begin()` is the same as `.end()` which would bring the size to `-1`. This would require `CPubKey::GetLen` to return 0, which is possible if the first byte is invalid. Maybe it'd make sense to make it return `1` in that case because there's always the first byte?
💬 vasild commented on pull request "net: Replace libnatpmp with built-in PCP implementation":
(https://github.com/bitcoin/bitcoin/pull/30043#discussion_r1604635967)
Here is a standalone program to get the default gateway using a netlink socket:
<details>
<summary>netlink_get_default_route.cc</summary>
```cc
#include <arpa/inet.h>
#include <assert.h>
#include <errno.h>
#include <net/if.h>
#include <netinet/in.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <sys/socket.h>
#include <sys/types.h>
#include <unistd.h>
#ifdef __linux__
#include <linux/rtnetlink.h>
#elif defined(__FreeBSD__)
#include <netlink/netlink.h
...
(https://github.com/bitcoin/bitcoin/pull/30043#discussion_r1604635967)
Here is a standalone program to get the default gateway using a netlink socket:
<details>
<summary>netlink_get_default_route.cc</summary>
```cc
#include <arpa/inet.h>
#include <assert.h>
#include <errno.h>
#include <net/if.h>
#include <netinet/in.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <sys/socket.h>
#include <sys/types.h>
#include <unistd.h>
#ifdef __linux__
#include <linux/rtnetlink.h>
#elif defined(__FreeBSD__)
#include <netlink/netlink.h
...
💬 laanwj commented on pull request "[PoC] qt, depends: Add wayland support without build-time nor fixed run-time deps":
(https://github.com/bitcoin/bitcoin/pull/29959#issuecomment-2117145929)
Thank you. To be clear that wasn't aimed at you. i know you're busy enough. Just at the wider FOSS community, who are always full of talk about the next big thing but when push comes to shove... In any case we can pick this up at some point. #29923 is basically complete so this would be the next step in the particular progression.
(https://github.com/bitcoin/bitcoin/pull/29959#issuecomment-2117145929)
Thank you. To be clear that wasn't aimed at you. i know you're busy enough. Just at the wider FOSS community, who are always full of talk about the next big thing but when push comes to shove... In any case we can pick this up at some point. #29923 is basically complete so this would be the next step in the particular progression.
💬 stickies-v commented on pull request "rpc: avoid copying into UniValue":
(https://github.com/bitcoin/bitcoin/pull/30115#issuecomment-2117153581)
Concept ACK
> Indeed, I believe c-i should be doing that check.
It does: https://github.com/bitcoin/bitcoin/blob/2f53f2273da020d7fabd7c65a1bc7e69a31249b2/src/.clang-tidy#L6
(https://github.com/bitcoin/bitcoin/pull/30115#issuecomment-2117153581)
Concept ACK
> Indeed, I believe c-i should be doing that check.
It does: https://github.com/bitcoin/bitcoin/blob/2f53f2273da020d7fabd7c65a1bc7e69a31249b2/src/.clang-tidy#L6
💬 wiz commented on pull request "Testnet4 including PoW difficulty adjustment fix":
(https://github.com/bitcoin/bitcoin/pull/29775#issuecomment-2117166802)
testnet4 faucet up at https://mempool.space/testnet4/faucet
feel free to donate coins to `tb1q0dzcgv7scppjxsnwlzpkt02vlmc5rtr40wyjgr`
(https://github.com/bitcoin/bitcoin/pull/29775#issuecomment-2117166802)
testnet4 faucet up at https://mempool.space/testnet4/faucet
feel free to donate coins to `tb1q0dzcgv7scppjxsnwlzpkt02vlmc5rtr40wyjgr`