💬 polespinasa commented on pull request "wallet, rpc: remove settxfee and paytxfee":
(https://github.com/bitcoin/bitcoin/pull/32138#discussion_r2308221628)
forgot to remove the line, will do next push
(https://github.com/bitcoin/bitcoin/pull/32138#discussion_r2308221628)
forgot to remove the line, will do next push
🚀 fanquake merged a pull request: "ci: return to using dash in CentOS job"
(https://github.com/bitcoin/bitcoin/pull/33261)
(https://github.com/bitcoin/bitcoin/pull/33261)
💬 polespinasa commented on pull request "rpc, logging: add backgroundvalidation to getblockchaininfo":
(https://github.com/bitcoin/bitcoin/pull/33259#discussion_r2308229142)
done
(https://github.com/bitcoin/bitcoin/pull/33259#discussion_r2308229142)
done
💬 polespinasa commented on pull request "rpc, logging: add backgroundvalidation to getblockchaininfo":
(https://github.com/bitcoin/bitcoin/pull/33259#discussion_r2308230173)
I like to see `false` if it's not in b validation.
Fixed the "estimate of"
(https://github.com/bitcoin/bitcoin/pull/33259#discussion_r2308230173)
I like to see `false` if it's not in b validation.
Fixed the "estimate of"
🚀 fanquake merged a pull request: "threading: reduce the scope of lock in getblocktemplate"
(https://github.com/bitcoin/bitcoin/pull/33264)
(https://github.com/bitcoin/bitcoin/pull/33264)
🚀 fanquake merged a pull request: "rpc: followups for 33106"
(https://github.com/bitcoin/bitcoin/pull/33189)
(https://github.com/bitcoin/bitcoin/pull/33189)
🚀 fanquake merged a pull request: "ci: use LLVM 21"
(https://github.com/bitcoin/bitcoin/pull/33258)
(https://github.com/bitcoin/bitcoin/pull/33258)
💬 polespinasa commented on pull request "rpc, logging: add backgroundvalidation to getblockchaininfo":
(https://github.com/bitcoin/bitcoin/pull/33259#discussion_r2308265935)
That's a good point, but that's actually what we are showing in the logs. Do we want to keep consistency between both? Update maybe also the logging?
```[background validation] UpdateTip: new best=00000000000000000007b796179f5abb7eed59f736e3418eb8381c29eb78527a height=744000 version=0x20400004 log2_work=93.616004 tx=747106639 date='2022-07-07T14:47:25Z' progress=0.605933 cache=358.0MiB(2606151txo)```
In `validation.cpp`
```c++
constexpr int BACKGROUND_LOG_INTERVAL = 2000;
if
...
(https://github.com/bitcoin/bitcoin/pull/33259#discussion_r2308265935)
That's a good point, but that's actually what we are showing in the logs. Do we want to keep consistency between both? Update maybe also the logging?
```[background validation] UpdateTip: new best=00000000000000000007b796179f5abb7eed59f736e3418eb8381c29eb78527a height=744000 version=0x20400004 log2_work=93.616004 tx=747106639 date='2022-07-07T14:47:25Z' progress=0.605933 cache=358.0MiB(2606151txo)```
In `validation.cpp`
```c++
constexpr int BACKGROUND_LOG_INTERVAL = 2000;
if
...
💬 enirox001 commented on pull request "test: Replace legacy wallet with MiniWallet in rpc_getblockstats.py":
(https://github.com/bitcoin/bitcoin/pull/33184#discussion_r2308376222)
Done
(https://github.com/bitcoin/bitcoin/pull/33184#discussion_r2308376222)
Done
💬 enirox001 commented on pull request "test: Replace legacy wallet with MiniWallet in rpc_getblockstats.py":
(https://github.com/bitcoin/bitcoin/pull/33184#discussion_r2308376554)
You're correct - the comments were misleading. I've updated them to accurately reflect that MiniWallet uses consistent fee behavior for all transactions. The test still validates `getblockstats` functionality across different block structures, which is the primary goal.
(https://github.com/bitcoin/bitcoin/pull/33184#discussion_r2308376554)
You're correct - the comments were misleading. I've updated them to accurately reflect that MiniWallet uses consistent fee behavior for all transactions. The test still validates `getblockstats` functionality across different block structures, which is the primary goal.
💬 enirox001 commented on pull request "test: Replace legacy wallet with MiniWallet in rpc_getblockstats.py":
(https://github.com/bitcoin/bitcoin/pull/33184#discussion_r2308376622)
You're correct that UTXO statistics don't depend on output destination. However, I'm keeping external outputs to maintain consistency with the original test's transaction structure. Changing to `send_self_transfer` would alter the generated test data and expected statistics values.
Since the goal is to replace the wallet implementation while preserving test behavior, `send_to` with external scripts is the most faithful translation of the original `sendtoaddress` calls.
(https://github.com/bitcoin/bitcoin/pull/33184#discussion_r2308376622)
You're correct that UTXO statistics don't depend on output destination. However, I'm keeping external outputs to maintain consistency with the original test's transaction structure. Changing to `send_self_transfer` would alter the generated test data and expected statistics values.
Since the goal is to replace the wallet implementation while preserving test behavior, `send_to` with external scripts is the most faithful translation of the original `sendtoaddress` calls.
💬 w0xlt commented on pull request "net: Prevent node from binding to the same `CService`":
(https://github.com/bitcoin/bitcoin/pull/33231#issuecomment-3234759432)
> IMO this warning is sufficient, but if you keep the error approach, consider removing the warning.
Thanks for the review @naiyoma .
The error will occur regardless, whether in master or in this PR.
This PR simply provides a clearer and more useful message.
Regarding removing the warning, I think the `bind` validation could be moved to occur before the onion validation. Good catch.
(https://github.com/bitcoin/bitcoin/pull/33231#issuecomment-3234759432)
> IMO this warning is sufficient, but if you keep the error approach, consider removing the warning.
Thanks for the review @naiyoma .
The error will occur regardless, whether in master or in this PR.
This PR simply provides a clearer and more useful message.
Regarding removing the warning, I think the `bind` validation could be moved to occur before the onion validation. Good catch.
✅ hebasto closed a pull request: "cmake, guix: Skip building tests in subtrees for releases"
(https://github.com/bitcoin/bitcoin/pull/32054)
(https://github.com/bitcoin/bitcoin/pull/32054)
🤔 hebasto reviewed a pull request: "guix: update time-machine to 5cb84f2013c5b1e48a7d0e617032266f1e6059e2"
(https://github.com/bitcoin/bitcoin/pull/33185#pullrequestreview-3166350833)
My Guix build on RISC-V hardware for some older version of this branch:
```
riscv64
62fa84fe6a59109f6f0e4e6bc99b818aceaa339c2928384379070fa0467b94a9 guix-build-663fb8bf44aa/output/aarch64-linux-gnu/SHA256SUMS.part
36d0d70798970aa734aa945523593acf9dd57fa461e0799f2c8b55d4c4881fdd guix-build-663fb8bf44aa/output/aarch64-linux-gnu/bitcoin-663fb8bf44aa-aarch64-linux-gnu-debug.tar.gz
3852c55e6cd103f3db42e7a111b2885574956aa38bed202bc0669e261dc52c59 guix-build-663fb8bf44aa/output/aarch64-linux-gn
...
(https://github.com/bitcoin/bitcoin/pull/33185#pullrequestreview-3166350833)
My Guix build on RISC-V hardware for some older version of this branch:
```
riscv64
62fa84fe6a59109f6f0e4e6bc99b818aceaa339c2928384379070fa0467b94a9 guix-build-663fb8bf44aa/output/aarch64-linux-gnu/SHA256SUMS.part
36d0d70798970aa734aa945523593acf9dd57fa461e0799f2c8b55d4c4881fdd guix-build-663fb8bf44aa/output/aarch64-linux-gnu/bitcoin-663fb8bf44aa-aarch64-linux-gnu-debug.tar.gz
3852c55e6cd103f3db42e7a111b2885574956aa38bed202bc0669e261dc52c59 guix-build-663fb8bf44aa/output/aarch64-linux-gn
...
💬 davidgumberg commented on pull request "net, pcp: handle multi-part responses and filter for default route while querying default gateway":
(https://github.com/bitcoin/bitcoin/pull/32159#discussion_r2308434775)
> Is it guaranteed that the reponse to `NLM_F_DUMP` is always multipart? Or do we need to check `nlmsg_flags` for `NLM_F_MULTI`, and if not, break after the first packet?
It seems that in the linux kernel, that is the case, a message type's (e.g. GETROUTE) `control` callback's `[.dumpit]`(https://github.com/torvalds/linux/blob/39f90c1967215375f7d87b81d14b0f3ed6b40c29/include/net/rtnetlink.h#L50) is [called](https://github.com/torvalds/linux/blob/39f90c1967215375f7d87b81d14b0f3ed6b40c29/net/n
...
(https://github.com/bitcoin/bitcoin/pull/32159#discussion_r2308434775)
> Is it guaranteed that the reponse to `NLM_F_DUMP` is always multipart? Or do we need to check `nlmsg_flags` for `NLM_F_MULTI`, and if not, break after the first packet?
It seems that in the linux kernel, that is the case, a message type's (e.g. GETROUTE) `control` callback's `[.dumpit]`(https://github.com/torvalds/linux/blob/39f90c1967215375f7d87b81d14b0f3ed6b40c29/include/net/rtnetlink.h#L50) is [called](https://github.com/torvalds/linux/blob/39f90c1967215375f7d87b81d14b0f3ed6b40c29/net/n
...
💬 fqlx commented on pull request "rpc: require integer verbosity; remove boolean 'verbose'":
(https://github.com/bitcoin/bitcoin/pull/33214#issuecomment-3234803555)
> I expect those to be widely used (not only in the internal tests, as you noticed in the repeated force pushes), so this may or may not be a widely breaking change (depending on how much they are used in active projects).
>
> There is no auto-generated schema or RPC interface, so those projects would have to fix them manually.
>
> No strong opinion, but maybe this can be closed for now, and revisited when there is a schema generator?
In my opinion, this is a significantly breaking chan
...
(https://github.com/bitcoin/bitcoin/pull/33214#issuecomment-3234803555)
> I expect those to be widely used (not only in the internal tests, as you noticed in the repeated force pushes), so this may or may not be a widely breaking change (depending on how much they are used in active projects).
>
> There is no auto-generated schema or RPC interface, so those projects would have to fix them manually.
>
> No strong opinion, but maybe this can be closed for now, and revisited when there is a schema generator?
In my opinion, this is a significantly breaking chan
...
💬 achow101 commented on pull request "doc: unify `datacarriersize` warning with release notes":
(https://github.com/bitcoin/bitcoin/pull/33224#issuecomment-3234820665)
I've done some experimenting with a test project on transifex to see what the actual effects this change would cause, and a potential workaround.
In this test project, I copied over a couple fully translated languages to observe how the translated strings change when the source is updated. After rebasing this PR branch and doing `cmake --build build --target translate`, I uploaded the resulting modified `bitcoin_en.xlf` file. This resulted in 122 (~10% of strings) being marked untranslated ag
...
(https://github.com/bitcoin/bitcoin/pull/33224#issuecomment-3234820665)
I've done some experimenting with a test project on transifex to see what the actual effects this change would cause, and a potential workaround.
In this test project, I copied over a couple fully translated languages to observe how the translated strings change when the source is updated. After rebasing this PR branch and doing `cmake --build build --target translate`, I uploaded the resulting modified `bitcoin_en.xlf` file. This resulted in 122 (~10% of strings) being marked untranslated ag
...
💬 achow101 commented on issue "Zero output not cleared":
(https://github.com/bitcoin/bitcoin/issues/33265#issuecomment-3234847341)
This seems like it's because the check for whether a transaction is "from" the wallet is if it a non-zero amount leaves the wallet. Since these outputs are 0 value, they are definitionally do not meet that criteria.
(https://github.com/bitcoin/bitcoin/issues/33265#issuecomment-3234847341)
This seems like it's because the check for whether a transaction is "from" the wallet is if it a non-zero amount leaves the wallet. Since these outputs are 0 value, they are definitionally do not meet that criteria.
💬 davidgumberg commented on pull request "net, pcp: handle multi-part responses and filter for default route while querying default gateway":
(https://github.com/bitcoin/bitcoin/pull/32159#discussion_r2308480089)
Relevant section of iproute2, if a response to getroute is not of type `RTM_NEWROUTE`, it's an error:
```c
static int iproute_get(int argc, char **argv)
// [... `Populate answer` ]
struct rtmsg *r = NLMSG_DATA(answer);
if (answer->nlmsg_type != RTM_NEWROUTE) {
fprintf(stderr, "Not a route?\n");
delete_json_obj();
free(answer);
return -1;
}
}
```
https://github.com/iproute2/iproute2/blob/3337e5013e168bcf4fab5f6518d1e4293a0a830b/ip/iproute.c#L2242-L2247
(https://github.com/bitcoin/bitcoin/pull/32159#discussion_r2308480089)
Relevant section of iproute2, if a response to getroute is not of type `RTM_NEWROUTE`, it's an error:
```c
static int iproute_get(int argc, char **argv)
// [... `Populate answer` ]
struct rtmsg *r = NLMSG_DATA(answer);
if (answer->nlmsg_type != RTM_NEWROUTE) {
fprintf(stderr, "Not a route?\n");
delete_json_obj();
free(answer);
return -1;
}
}
```
https://github.com/iproute2/iproute2/blob/3337e5013e168bcf4fab5f6518d1e4293a0a830b/ip/iproute.c#L2242-L2247
💬 davidgumberg commented on pull request "net, pcp: handle multi-part responses and filter for default route while querying default gateway":
(https://github.com/bitcoin/bitcoin/pull/32159#discussion_r2308514117)
Relevant (editorialized) code from iproute2:
```c
if (tb[RTA_DST]) {
// [...if the route has a specific destination, e.g.] ip route add 192.168.0.5 via 192.168.0.4
} else if (r->rtm_dst_len) {
// [if the route has prefix has a length]
snprintf(b1, sizeof(b1), "0/%d ", r->rtm_dst_len);
} else {
// [if rtm_dst_len == 0]
strncpy(b1, "default", sizeof(b1));
}
https://github.com/iproute2/iproute2/blob/3337e5013e168bcf4fab5f6518d1e4293a0a830b/ip/iproute.
...
(https://github.com/bitcoin/bitcoin/pull/32159#discussion_r2308514117)
Relevant (editorialized) code from iproute2:
```c
if (tb[RTA_DST]) {
// [...if the route has a specific destination, e.g.] ip route add 192.168.0.5 via 192.168.0.4
} else if (r->rtm_dst_len) {
// [if the route has prefix has a length]
snprintf(b1, sizeof(b1), "0/%d ", r->rtm_dst_len);
} else {
// [if rtm_dst_len == 0]
strncpy(b1, "default", sizeof(b1));
}
https://github.com/iproute2/iproute2/blob/3337e5013e168bcf4fab5f6518d1e4293a0a830b/ip/iproute.
...
💬 icedmoca commented on issue "signet: disk-space-DoS due to low mining difficulty":
(https://github.com/bitcoin/bitcoin/issues/33266#issuecomment-3234912770)
The most robust mitigation to nonce-grind disk-bloat attacks on low-difficulty Signets and derivative test networks is to elevate the effective difficulty target beyond the 2³² threshold, thereby rendering exhaustive nonce enumeration infeasible within realistic compute bounds, while simultaneously hardening node storage semantics by enforcing deduplication constraints on alternative block headers at a given height and chainwork level. In practice, this entails (i) tuning the consensus difficult
...
(https://github.com/bitcoin/bitcoin/issues/33266#issuecomment-3234912770)
The most robust mitigation to nonce-grind disk-bloat attacks on low-difficulty Signets and derivative test networks is to elevate the effective difficulty target beyond the 2³² threshold, thereby rendering exhaustive nonce enumeration infeasible within realistic compute bounds, while simultaneously hardening node storage semantics by enforcing deduplication constraints on alternative block headers at a given height and chainwork level. In practice, this entails (i) tuning the consensus difficult
...