⚠️ Sandra-Amina-Boss opened an issue: "Sm"
(https://github.com/bitcoin/bitcoin/issues/31224)
https://github.com/bnb-chain/greenfield-whitepaper/tree/main
(https://github.com/bitcoin/bitcoin/issues/31224)
https://github.com/bnb-chain/greenfield-whitepaper/tree/main
✅ fanquake closed an issue: "Sm"
(https://github.com/bitcoin/bitcoin/issues/31224)
(https://github.com/bitcoin/bitcoin/issues/31224)
:lock: fanquake locked an issue: "Sm"
(https://github.com/bitcoin/bitcoin/issues/31224)
(https://github.com/bitcoin/bitcoin/issues/31224)
💬 marcofleon commented on pull request "fuzz: Fix difficulty target generation in `p2p_headers_presync`":
(https://github.com/bitcoin/bitcoin/pull/31213#discussion_r1829926285)
If I wanted to do a lower target based on `MinimumChainWork`, I think I could do
```
target = (~work + 1) / work
```
where work would be `MinimumChainWork / 1600` or maybe a bit less to be safe.
(https://github.com/bitcoin/bitcoin/pull/31213#discussion_r1829926285)
If I wanted to do a lower target based on `MinimumChainWork`, I think I could do
```
target = (~work + 1) / work
```
where work would be `MinimumChainWork / 1600` or maybe a bit less to be safe.
👋 glozow's pull request is ready for review: "TxDownloadManager followups"
(https://github.com/bitcoin/bitcoin/pull/31190)
(https://github.com/bitcoin/bitcoin/pull/31190)
💬 achow101 commented on pull request "Update secp256k1 subtree to v0.6.0":
(https://github.com/bitcoin/bitcoin/pull/31216#issuecomment-2458061763)
> Should we disable the musig module in this PR and enable it only when it is needed?
Done
(https://github.com/bitcoin/bitcoin/pull/31216#issuecomment-2458061763)
> Should we disable the musig module in this PR and enable it only when it is needed?
Done
💬 glozow commented on pull request "BlockAssembler: return selected packages vsize and feerate":
(https://github.com/bitcoin/bitcoin/pull/30391#discussion_r1829943882)
Nonblocking, I can understand it's a larger change to use it.
(https://github.com/bitcoin/bitcoin/pull/30391#discussion_r1829943882)
Nonblocking, I can understand it's a larger change to use it.
💬 achow101 commented on pull request "wallet: Be able to receive and spend inputs involving MuSig2 aggregate keys":
(https://github.com/bitcoin/bitcoin/pull/29675#discussion_r1829944320)
Good catch! Fixed.
(https://github.com/bitcoin/bitcoin/pull/29675#discussion_r1829944320)
Good catch! Fixed.
📝 secp512k2 opened a pull request: "doc: Fix grammatical errors in multisig-tutorial.md"
(https://github.com/bitcoin/bitcoin/pull/31225)
This pull request fixes grammatical errors in the 'multisig-tutorial.md' document.
Corrections:
1. Incorrect Phrase "As can been seen":
- Before:
There are discussions about eliminating this redundancy, as can been seen in the issue #17190 (https://github.com/bitcoin/bitcoin/issues/17190).
- After:
There are discussions about eliminating this redundancy, as can be seen in the issue #17190 (https://github.com/bitcoin/bitcoin/issues/17190).
2. Clarity Improveme
...
(https://github.com/bitcoin/bitcoin/pull/31225)
This pull request fixes grammatical errors in the 'multisig-tutorial.md' document.
Corrections:
1. Incorrect Phrase "As can been seen":
- Before:
There are discussions about eliminating this redundancy, as can been seen in the issue #17190 (https://github.com/bitcoin/bitcoin/issues/17190).
- After:
There are discussions about eliminating this redundancy, as can be seen in the issue #17190 (https://github.com/bitcoin/bitcoin/issues/17190).
2. Clarity Improveme
...
💬 hebasto commented on pull request "build: Unify `-logsourcelocations` format":
(https://github.com/bitcoin/bitcoin/pull/30811#issuecomment-2458093729)
My Guix build:
```
aarch64
67b7e95e0f1d45debad8a8cac4c2d349e3e01efcf62e0b109345896f296758d9 guix-build-788c1324f3d8/output/aarch64-linux-gnu/SHA256SUMS.part
cd3c67eaa5f6fadf0a73cdb1aa03563d720fc7fdae1b59aa495a0eda9d2695af guix-build-788c1324f3d8/output/aarch64-linux-gnu/bitcoin-788c1324f3d8-aarch64-linux-gnu-debug.tar.gz
5c1eb4ff827336a9d113497459981e213844b87e232c33fc00317ce20302f761 guix-build-788c1324f3d8/output/aarch64-linux-gnu/bitcoin-788c1324f3d8-aarch64-linux-gnu.tar.gz
dd78923f
...
(https://github.com/bitcoin/bitcoin/pull/30811#issuecomment-2458093729)
My Guix build:
```
aarch64
67b7e95e0f1d45debad8a8cac4c2d349e3e01efcf62e0b109345896f296758d9 guix-build-788c1324f3d8/output/aarch64-linux-gnu/SHA256SUMS.part
cd3c67eaa5f6fadf0a73cdb1aa03563d720fc7fdae1b59aa495a0eda9d2695af guix-build-788c1324f3d8/output/aarch64-linux-gnu/bitcoin-788c1324f3d8-aarch64-linux-gnu-debug.tar.gz
5c1eb4ff827336a9d113497459981e213844b87e232c33fc00317ce20302f761 guix-build-788c1324f3d8/output/aarch64-linux-gnu/bitcoin-788c1324f3d8-aarch64-linux-gnu.tar.gz
dd78923f
...
👍 hebasto approved a pull request: "Update secp256k1 subtree to v0.6.0"
(https://github.com/bitcoin/bitcoin/pull/31216#pullrequestreview-2416632014)
ACK 97235c446e9986ecca09c2a4b78d6c6239853fdb, verified by updating the secp256k1 subtree locally.
(https://github.com/bitcoin/bitcoin/pull/31216#pullrequestreview-2416632014)
ACK 97235c446e9986ecca09c2a4b78d6c6239853fdb, verified by updating the secp256k1 subtree locally.
💬 murchandamus commented on pull request "Remove the legacy wallet and BDB dependency":
(https://github.com/bitcoin/bitcoin/pull/28710#issuecomment-2458206441)
It looks like all three dependencies got merged, is this ready for review?
(https://github.com/bitcoin/bitcoin/pull/28710#issuecomment-2458206441)
It looks like all three dependencies got merged, is this ready for review?
💬 achow101 commented on pull request "Remove the legacy wallet and BDB dependency":
(https://github.com/bitcoin/bitcoin/pull/28710#issuecomment-2458297336)
> It looks like all three dependencies got merged, is this ready for review?
Currently it is still dependent on #30328 but I suppose it doesn't have to be.
(https://github.com/bitcoin/bitcoin/pull/28710#issuecomment-2458297336)
> It looks like all three dependencies got merged, is this ready for review?
Currently it is still dependent on #30328 but I suppose it doesn't have to be.
🤔 tdb3 reviewed a pull request: "net, init: derive default onion port if a user specified a -port"
(https://github.com/bitcoin/bitcoin/pull/31223#pullrequestreview-2417053413)
Approach ACK
Seemed to work nicely with two node instances run as follows:
```
build/src/bitcoind -port=11000 -rpcport=11100 -datadir=/mnt/tmp/n1/
```
```
build/src/bitcoind -port=12000 -rpcport=12100 -datadir=/mnt/tmp/n2/
```
```
ss -tulpn
...
... 127.0.0.1:11100 0.0.0.0:* users:(("bitcoind",pid=192480,fd=12))
... 127.0.0.1:11001 0.0.0.0:* users:(("bitcoind",pid=192480,fd=29))
... 127.0.0.1:12100
...
(https://github.com/bitcoin/bitcoin/pull/31223#pullrequestreview-2417053413)
Approach ACK
Seemed to work nicely with two node instances run as follows:
```
build/src/bitcoind -port=11000 -rpcport=11100 -datadir=/mnt/tmp/n1/
```
```
build/src/bitcoind -port=12000 -rpcport=12100 -datadir=/mnt/tmp/n2/
```
```
ss -tulpn
...
... 127.0.0.1:11100 0.0.0.0:* users:(("bitcoind",pid=192480,fd=12))
... 127.0.0.1:11001 0.0.0.0:* users:(("bitcoind",pid=192480,fd=29))
... 127.0.0.1:12100
...
💬 tdb3 commented on pull request "net, init: derive default onion port if a user specified a -port":
(https://github.com/bitcoin/bitcoin/pull/31223#discussion_r1830257430)
`std::optional::value_or()` might work nicely here.
e.g.
```diff
diff --git a/src/torcontrol.cpp b/src/torcontrol.cpp
index d56216a7cb0..b518cf1bb93 100644
--- a/src/torcontrol.cpp
+++ b/src/torcontrol.cpp
@@ -715,5 +715,5 @@ CService DefaultOnionServiceTarget(std::optional<uint16_t> port)
{
struct in_addr onion_service_target;
onion_service_target.s_addr = htonl(INADDR_LOOPBACK);
- return {onion_service_target, port.has_value() ? port.value() : BaseParams().OnionServ
...
(https://github.com/bitcoin/bitcoin/pull/31223#discussion_r1830257430)
`std::optional::value_or()` might work nicely here.
e.g.
```diff
diff --git a/src/torcontrol.cpp b/src/torcontrol.cpp
index d56216a7cb0..b518cf1bb93 100644
--- a/src/torcontrol.cpp
+++ b/src/torcontrol.cpp
@@ -715,5 +715,5 @@ CService DefaultOnionServiceTarget(std::optional<uint16_t> port)
{
struct in_addr onion_service_target;
onion_service_target.s_addr = htonl(INADDR_LOOPBACK);
- return {onion_service_target, port.has_value() ? port.value() : BaseParams().OnionServ
...
💬 tdb3 commented on pull request "test: cover edge case for lockunspent rpc":
(https://github.com/bitcoin/bitcoin/pull/31209#issuecomment-2458574314)
> Not sure about this. This seems to be adding coverage for code that shouldn't exist in the first place.
>
> The integer is passed to `COutPoint`, which only takes unsigned integers, so parsing the integer as such would be more consistent. The check would then be redundant and could be removed.
Good catch. It seems a bit odd that the RPC is trying to interpret the `vout` arg as a *signed* int in the first place (`const int nOutput = o.find_value("vout").getInt<int>();`), then check for ne
...
(https://github.com/bitcoin/bitcoin/pull/31209#issuecomment-2458574314)
> Not sure about this. This seems to be adding coverage for code that shouldn't exist in the first place.
>
> The integer is passed to `COutPoint`, which only takes unsigned integers, so parsing the integer as such would be more consistent. The check would then be redundant and could be removed.
Good catch. It seems a bit odd that the RPC is trying to interpret the `vout` arg as a *signed* int in the first place (`const int nOutput = o.find_value("vout").getInt<int>();`), then check for ne
...
💬 vasild commented on pull request "Fee Estimation via Fee rate Forecasters":
(https://github.com/bitcoin/bitcoin/pull/30157#issuecomment-2458703251)
Concept ACK
> Allow node users to be self-sovereign by using their node's estimates, reducing reliance on third-party fee estimations.
> having users feel the need to use external fee estimation services that they could equally have served to them by their own node, feels sub-optimal
Indeed, I do that :face_with_head_bandage:
(https://github.com/bitcoin/bitcoin/pull/30157#issuecomment-2458703251)
Concept ACK
> Allow node users to be self-sovereign by using their node's estimates, reducing reliance on third-party fee estimations.
> having users feel the need to use external fee estimation services that they could equally have served to them by their own node, feels sub-optimal
Indeed, I do that :face_with_head_bandage:
💬 naumenkogs commented on pull request "Package validation: accept packages of size 1":
(https://github.com/bitcoin/bitcoin/pull/31096#issuecomment-2458962465)
@stickies-v i think you might be looking at the wrong commit. The hash you acked is outdated.
(https://github.com/bitcoin/bitcoin/pull/31096#issuecomment-2458962465)
@stickies-v i think you might be looking at the wrong commit. The hash you acked is outdated.
📝 hebasto opened a pull request: "cmake: Fix `IF_CHECK_PASSED` option handling"
(https://github.com/bitcoin/bitcoin/pull/31231)
`IF_CHECK_PASSED` is a multi-value keyword, resulting in a list value. Convert it to a string before applying any `string()` command.
Split from https://github.com/bitcoin/bitcoin/pull/30861.
No current CMake code is affected by this bug.
(https://github.com/bitcoin/bitcoin/pull/31231)
`IF_CHECK_PASSED` is a multi-value keyword, resulting in a list value. Convert it to a string before applying any `string()` command.
Split from https://github.com/bitcoin/bitcoin/pull/30861.
No current CMake code is affected by this bug.
💬 naumenkogs commented on pull request "Package validation: accept packages of size 1":
(https://github.com/bitcoin/bitcoin/pull/31096#discussion_r1830626118)
I've failed to suggest a simple code-change to avoid double-validation of a single tx.
The code is really about multi-tx. Even if we add a bunch of conditions around `auto multi_submission_result`, stuff below (`individual_results_nonfinal`) makes you think about multi-tx again.
The least thing I'm worried about is double-validation on itself.
I'm worried about code comprehension complexity: reading multi-tx code against a single-tx scenario.
At this point I agree with Gloria to handle
...
(https://github.com/bitcoin/bitcoin/pull/31096#discussion_r1830626118)
I've failed to suggest a simple code-change to avoid double-validation of a single tx.
The code is really about multi-tx. Even if we add a bunch of conditions around `auto multi_submission_result`, stuff below (`individual_results_nonfinal`) makes you think about multi-tx again.
The least thing I'm worried about is double-validation on itself.
I'm worried about code comprehension complexity: reading multi-tx code against a single-tx scenario.
At this point I agree with Gloria to handle
...