💬 jonatack commented on pull request "net: option to disallow v1 connection on ipv4 and ipv6 peers":
(https://github.com/bitcoin/bitcoin/pull/30951#discussion_r1779484571)
```suggestion
} else if {
(LookupHost(host, true).has_value()) {
```
(https://github.com/bitcoin/bitcoin/pull/30951#discussion_r1779484571)
```suggestion
} else if {
(LookupHost(host, true).has_value()) {
```
👍 itornaza approved a pull request: "docs: Add instructions on how to self-sign bitcoin-core binaries for macOS"
(https://github.com/bitcoin/bitcoin/pull/30982#pullrequestreview-2335026045)
Concept ACK.
I can also verify that the steps described in the pr work properly on macOS Sequoia 15.0 (24A335).
> I think it might be good to also include instructions for signing a downloaded `Bitcoin-Qt.app`, as I would think that might be what a lot (majority?) of macOS users are downloading.
In my view, users who download the app directly can always be referenced to the standard Apple procedure for installing unknown developer apps https://support.apple.com/guide/mac-help/open-a-mac
...
(https://github.com/bitcoin/bitcoin/pull/30982#pullrequestreview-2335026045)
Concept ACK.
I can also verify that the steps described in the pr work properly on macOS Sequoia 15.0 (24A335).
> I think it might be good to also include instructions for signing a downloaded `Bitcoin-Qt.app`, as I would think that might be what a lot (majority?) of macOS users are downloading.
In my view, users who download the app directly can always be referenced to the standard Apple procedure for installing unknown developer apps https://support.apple.com/guide/mac-help/open-a-mac
...
💬 maflcko commented on pull request "tinyformat: refactor: increase compile-time checks and don't throw for tfm::format_error":
(https://github.com/bitcoin/bitcoin/pull/30928#discussion_r1779498782)
I am not sure about "partial validation". Either the string is fully validated, or not at all. Prepending a valid string to an invalid string doesn't make it valid.
(https://github.com/bitcoin/bitcoin/pull/30928#discussion_r1779498782)
I am not sure about "partial validation". Either the string is fully validated, or not at all. Prepending a valid string to an invalid string doesn't make it valid.
💬 rebroad commented on pull request "p2p: When close to the tip, download blocks in parallel from additional peers to prevent stalling":
(https://github.com/bitcoin/bitcoin/pull/29664#issuecomment-2380661215)
Rather than the crude 10 minute timeout - why not actually check if the block is being downloaded from the chosen peer - i.e. work out the throughput and make a decision based on this (compared to typical throughput from other peers)?
(https://github.com/bitcoin/bitcoin/pull/29664#issuecomment-2380661215)
Rather than the crude 10 minute timeout - why not actually check if the block is being downloaded from the chosen peer - i.e. work out the throughput and make a decision based on this (compared to typical throughput from other peers)?
💬 l0rinc commented on pull request "tinyformat: refactor: increase compile-time checks and don't throw for tfm::format_error":
(https://github.com/bitcoin/bitcoin/pull/30928#discussion_r1779584565)
Weren't we already doing partial validation?
Doing a full one for these examples wouldn't be *that* difficult - do you suggest we do that instead?
(https://github.com/bitcoin/bitcoin/pull/30928#discussion_r1779584565)
Weren't we already doing partial validation?
Doing a full one for these examples wouldn't be *that* difficult - do you suggest we do that instead?
💬 l0rinc commented on pull request "tinyformat: refactor: increase compile-time checks and don't throw for tfm::format_error":
(https://github.com/bitcoin/bitcoin/pull/30928#discussion_r1779656652)
Got it, thanks for clarifying!
(https://github.com/bitcoin/bitcoin/pull/30928#discussion_r1779656652)
Got it, thanks for clarifying!
💬 naiyoma commented on pull request "test: addrman: tried 3 times and never a success so `isTerrible=true`":
(https://github.com/bitcoin/bitcoin/pull/30445#issuecomment-2380717731)
Concept ACK: additional test coverage to check that an address is marked as "terrible" after 3 or more unsuccessful connection attempts.
(https://github.com/bitcoin/bitcoin/pull/30445#issuecomment-2380717731)
Concept ACK: additional test coverage to check that an address is marked as "terrible" after 3 or more unsuccessful connection attempts.
💬 l0rinc commented on pull request "tinyformat: refactor: increase compile-time checks and don't throw for tfm::format_error":
(https://github.com/bitcoin/bitcoin/pull/30928#discussion_r1779666783)
Alternatively, if that's not desired, what if we replaced the dynamic widths with explicit `PadLeft`/`PadRight` calls, e.g.
```C++
result += tfm::format("<-> type net v mping ping send recv txn blk hb %s%s%s ",
PadRight("addrp", m_max_addr_processed_length),
PadRight("addrl", m_max_addr_rate_limited_length),
PadRight("age", m_max_age_length));
```
and
```C++
result += tfm::format("%s %s%s\n",
...
(https://github.com/bitcoin/bitcoin/pull/30928#discussion_r1779666783)
Alternatively, if that's not desired, what if we replaced the dynamic widths with explicit `PadLeft`/`PadRight` calls, e.g.
```C++
result += tfm::format("<-> type net v mping ping send recv txn blk hb %s%s%s ",
PadRight("addrp", m_max_addr_processed_length),
PadRight("addrl", m_max_addr_rate_limited_length),
PadRight("age", m_max_age_length));
```
and
```C++
result += tfm::format("%s %s%s\n",
...
📝 Jitphanu-051 opened a pull request: "Initial commit"
(https://github.com/bitcoin/bitcoin/pull/30995)
<!--
*** 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/30995)
<!--
*** 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
...
✅ hebasto closed a pull request: "Initial commit"
(https://github.com/bitcoin/bitcoin/pull/30995)
(https://github.com/bitcoin/bitcoin/pull/30995)
📝 hebasto locked a pull request: "."
(https://github.com/bitcoin/bitcoin/pull/30995)
<!--
*** 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/30995)
<!--
*** 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
...
💬 l0rinc commented on pull request "tinyformat: refactor: increase compile-time checks and don't throw for tfm::format_error":
(https://github.com/bitcoin/bitcoin/pull/30928#discussion_r1779720533)
do we need to keep the `strprintf("%s%s"` on line 563?
(https://github.com/bitcoin/bitcoin/pull/30928#discussion_r1779720533)
do we need to keep the `strprintf("%s%s"` on line 563?
🤔 glozow reviewed a pull request: "test: switch MiniWallet padding unit from weight to vsize"
(https://github.com/bitcoin/bitcoin/pull/30718#pullrequestreview-2335386388)
ACK 7d1c8d0aa3ebe30d7633caf9bc4765927e4d6e08, lmk if you want to update the second commit
(https://github.com/bitcoin/bitcoin/pull/30718#pullrequestreview-2335386388)
ACK 7d1c8d0aa3ebe30d7633caf9bc4765927e4d6e08, lmk if you want to update the second commit
💬 glozow commented on pull request "test: switch MiniWallet padding unit from weight to vsize":
(https://github.com/bitcoin/bitcoin/pull/30718#discussion_r1779726643)
a few more 1000s not changed:
L104 `assert_greater_than_or_equal(tx_v3_child_almost_heavy["tx"].get_vsize() + tx_v3_child_almost_heavy_rbf["tx"].get_vsize(), 1000)`
L194 `assert_greater_than(node.getmempoolentry(tx_v3_child_large["txid"])["vsize"], 1000)`
(https://github.com/bitcoin/bitcoin/pull/30718#discussion_r1779726643)
a few more 1000s not changed:
L104 `assert_greater_than_or_equal(tx_v3_child_almost_heavy["tx"].get_vsize() + tx_v3_child_almost_heavy_rbf["tx"].get_vsize(), 1000)`
L194 `assert_greater_than(node.getmempoolentry(tx_v3_child_large["txid"])["vsize"], 1000)`
💬 RandyMcMillan commented on pull request "doc/build-osx.md:brew relinking note":
(https://github.com/bitcoin/bitcoin/pull/30993#discussion_r1779729009)
good point - i will move it to the end of the "brew install" list and make a more general instruction.
-thanks
(https://github.com/bitcoin/bitcoin/pull/30993#discussion_r1779729009)
good point - i will move it to the end of the "brew install" list and make a more general instruction.
-thanks
💬 torkelrogstad commented on pull request "Extend signetchallenge to set target block spacing":
(https://github.com/bitcoin/bitcoin/pull/29365#issuecomment-2380864234)
Concept ACK
(https://github.com/bitcoin/bitcoin/pull/29365#issuecomment-2380864234)
Concept ACK
📝 torkelrogstad opened a pull request: "doc: update signet documentation related to build directories"
(https://github.com/bitcoin/bitcoin/pull/30996)
While setting up my own signet I noticed that the binary paths in the documentation for this is out of date, after build artifacts moved to the `build` directory. This PR mimics what happened in #30741
(https://github.com/bitcoin/bitcoin/pull/30996)
While setting up my own signet I noticed that the binary paths in the documentation for this is out of date, after build artifacts moved to the `build` directory. This PR mimics what happened in #30741
💬 l0rinc commented on pull request "validation: write chainstate to disk every hour":
(https://github.com/bitcoin/bitcoin/pull/30611#issuecomment-2380875224)
> running different benchmarks won't necessarily give us a clearer picture.
It could get confusing if they're not pointing in the same direction (which seems to be the case here).
----
I reran the previous full IBD on the previous setup with HDD, but this time until 860000 with 30gb memory.
<details>
<summary>benchmark</summary>
```bash
hyperfine --runs 3 \
--export-json /mnt/ibd-30611-full.json \
--parameter-list COMMIT 33adc7521cc8bb24b941d959022b084002ba7c60,391c87640d78d98
...
(https://github.com/bitcoin/bitcoin/pull/30611#issuecomment-2380875224)
> running different benchmarks won't necessarily give us a clearer picture.
It could get confusing if they're not pointing in the same direction (which seems to be the case here).
----
I reran the previous full IBD on the previous setup with HDD, but this time until 860000 with 30gb memory.
<details>
<summary>benchmark</summary>
```bash
hyperfine --runs 3 \
--export-json /mnt/ibd-30611-full.json \
--parameter-list COMMIT 33adc7521cc8bb24b941d959022b084002ba7c60,391c87640d78d98
...
💬 mzumsande commented on pull request "net: option to disallow v1 connection on ipv4 and ipv6 peers":
(https://github.com/bitcoin/bitcoin/pull/30951#issuecomment-2380879313)
> True. Lets dive a bit deeper - what are the reasons to be concerned about somebody inspecting the traffic to my node?
Some thoughts off the top my head:
- You don't want others (e.g. network admins, internet providers, etc.) to know that you are running a bitcoin node at all. Maybe it's not allowed in your network / country etc., maybe for other reasons such as personal security.
- Transaction privacy
In both of these cases, if you accept inbounds they could just connect to you as an a
...
(https://github.com/bitcoin/bitcoin/pull/30951#issuecomment-2380879313)
> True. Lets dive a bit deeper - what are the reasons to be concerned about somebody inspecting the traffic to my node?
Some thoughts off the top my head:
- You don't want others (e.g. network admins, internet providers, etc.) to know that you are running a bitcoin node at all. Maybe it's not allowed in your network / country etc., maybe for other reasons such as personal security.
- Transaction privacy
In both of these cases, if you accept inbounds they could just connect to you as an a
...
💬 theStack commented on pull request "test: switch MiniWallet padding unit from weight to vsize":
(https://github.com/bitcoin/bitcoin/pull/30718#discussion_r1779753414)
Thanks, updated!
(https://github.com/bitcoin/bitcoin/pull/30718#discussion_r1779753414)
Thanks, updated!