💬 Salarja commented on issue "depends: capnp build ignores config_opts":
(https://github.com/bitcoin/bitcoin/issues/32068#issuecomment-2746310181)
https://github.com/bitcoin/bitcoin/issues/32068
(https://github.com/bitcoin/bitcoin/issues/32068#issuecomment-2746310181)
https://github.com/bitcoin/bitcoin/issues/32068
💬 yancyribbens commented on pull request "contrib: document asmap-tool commands more thoroughly":
(https://github.com/bitcoin/bitcoin/pull/32110#issuecomment-2746323958)
This was pretty interesting to read more about. It looks like ASmap can provide protection from certain types of threat actors, such as those that can allocate a large number of addresses from a single RIR effectively creating a type of Sybil attack. However, it seems to me there's another attack vector, such as a botnet that infects numerous systems across many RIR's that this wouldn't be effective against.
(https://github.com/bitcoin/bitcoin/pull/32110#issuecomment-2746323958)
This was pretty interesting to read more about. It looks like ASmap can provide protection from certain types of threat actors, such as those that can allocate a large number of addresses from a single RIR effectively creating a type of Sybil attack. However, it seems to me there's another attack vector, such as a botnet that infects numerous systems across many RIR's that this wouldn't be effective against.
💬 sipa commented on pull request "contrib: document asmap-tool commands more thoroughly":
(https://github.com/bitcoin/bitcoin/pull/32110#discussion_r2009180426)
I don't think it defeats the purpose. It's just removing information one normally does not care about.
I'd just say "However, it does interfere with the ability to compute meaningful diffs".
(https://github.com/bitcoin/bitcoin/pull/32110#discussion_r2009180426)
I don't think it defeats the purpose. It's just removing information one normally does not care about.
I'd just say "However, it does interfere with the ability to compute meaningful diffs".
💬 sipa commented on pull request "contrib: document asmap-tool commands more thoroughly":
(https://github.com/bitcoin/bitcoin/pull/32110#discussion_r2009179891)
Typo: `files are`, or `a file is`
(https://github.com/bitcoin/bitcoin/pull/32110#discussion_r2009179891)
Typo: `files are`, or `a file is`
💬 sipa commented on pull request "contrib: document asmap-tool commands more thoroughly":
(https://github.com/bitcoin/bitcoin/pull/32110#discussion_r2009180269)
Instead of accuracy, maybe say "This procedure is lossy because it loses information about which ranges were unassigned"? Arguably, no accuracy is lost, because all real IP addresses are still mapped to their correct AS.
(https://github.com/bitcoin/bitcoin/pull/32110#discussion_r2009180269)
Instead of accuracy, maybe say "This procedure is lossy because it loses information about which ranges were unassigned"? Arguably, no accuracy is lost, because all real IP addresses are still mapped to their correct AS.
💬 sipa commented on pull request "contrib: document asmap-tool commands more thoroughly":
(https://github.com/bitcoin/bitcoin/pull/32110#discussion_r2009180669)
Sure it can be, it's just less meaningful.
(https://github.com/bitcoin/bitcoin/pull/32110#discussion_r2009180669)
Sure it can be, it's just less meaningful.
💬 sipa commented on pull request "contrib: document asmap-tool commands more thoroughly":
(https://github.com/bitcoin/bitcoin/pull/32110#discussion_r2009180746)
Same here.
(https://github.com/bitcoin/bitcoin/pull/32110#discussion_r2009180746)
Same here.
💬 sipa commented on pull request "contrib: document asmap-tool commands more thoroughly":
(https://github.com/bitcoin/bitcoin/pull/32110#issuecomment-2746342015)
@yancyribbens
> However, it seems to me there's another attack vector, such as a botnet that infects numerous systems across many RIR's that this wouldn't be effective against.
Of course, but the theory is that getting access to many *reachable* IP addresses in many different network is far more expensive than in one or a few.
(https://github.com/bitcoin/bitcoin/pull/32110#issuecomment-2746342015)
@yancyribbens
> However, it seems to me there's another attack vector, such as a botnet that infects numerous systems across many RIR's that this wouldn't be effective against.
Of course, but the theory is that getting access to many *reachable* IP addresses in many different network is far more expensive than in one or a few.
💬 yancyribbens commented on issue "Migrate from BTC/kvB to sat/vB on RPC and startup options":
(https://github.com/bitcoin/bitcoin/issues/32093#issuecomment-2746347346)
> I would recommend that we continue to use sat/kvB internally, maybe sat/kwu in the long-term, as we do only want to use integers but do want more precision than integer sat/vB
In rust-bitcoin land, the library uses sat/kwu as the default basis, and I think it would be rather nice if core and rust-bitcoin used the same default metric for FeeRate. So +1 for sat/kwu long-term idea.
(https://github.com/bitcoin/bitcoin/issues/32093#issuecomment-2746347346)
> I would recommend that we continue to use sat/kvB internally, maybe sat/kwu in the long-term, as we do only want to use integers but do want more precision than integer sat/vB
In rust-bitcoin land, the library uses sat/kwu as the default basis, and I think it would be rather nice if core and rust-bitcoin used the same default metric for FeeRate. So +1 for sat/kwu long-term idea.
💬 sipa commented on pull request "cluster mempool: introduce TxGraph":
(https://github.com/bitcoin/bitcoin/pull/31363#discussion_r2009188901)
This works:
```c++
TxGraph::Ref ref;
...
ref = txgraph.AddTransaction(fee, size):
```
So think of an empty Ref as an equivalent to an `std::unique_ptr` to `nullptr` or so. It's potentially useful just so you can have a variable declared as a member in a class or so without immediately initializing it.
It even goes further in that this works even through subclasses:
```
class TxMempoolEntry : public TxGraph::Ref { ... }
TxMemoolEntry entry;
...
entry = txgraph.AddTransaction
...
(https://github.com/bitcoin/bitcoin/pull/31363#discussion_r2009188901)
This works:
```c++
TxGraph::Ref ref;
...
ref = txgraph.AddTransaction(fee, size):
```
So think of an empty Ref as an equivalent to an `std::unique_ptr` to `nullptr` or so. It's potentially useful just so you can have a variable declared as a member in a class or so without immediately initializing it.
It even goes further in that this works even through subclasses:
```
class TxMempoolEntry : public TxGraph::Ref { ... }
TxMemoolEntry entry;
...
entry = txgraph.AddTransaction
...
💬 yancyribbens commented on issue "Migrate from BTC/kvB to sat/vB on RPC and startup options":
(https://github.com/bitcoin/bitcoin/issues/32093#issuecomment-2746348315)
Also, there's a long-standing issue where rust-bitcoin is off by one when compared to core because of the difference in metrics: https://github.com/rust-bitcoin/rust-bitcoin/issues/3806
(https://github.com/bitcoin/bitcoin/issues/32093#issuecomment-2746348315)
Also, there's a long-standing issue where rust-bitcoin is off by one when compared to core because of the difference in metrics: https://github.com/rust-bitcoin/rust-bitcoin/issues/3806
💬 frankomosh commented on issue "Enable PCP by default?":
(https://github.com/bitcoin/bitcoin/issues/31663#issuecomment-2746348967)
> And if you do enable it does the port mapping works from `bitcoind`?
unfortunately my ISP cannot allow me to change it, so I can't conclusively perform this specific test
(https://github.com/bitcoin/bitcoin/issues/31663#issuecomment-2746348967)
> And if you do enable it does the port mapping works from `bitcoind`?
unfortunately my ISP cannot allow me to change it, so I can't conclusively perform this specific test
💬 sipa commented on pull request "cluster mempool: introduce TxGraph":
(https://github.com/bitcoin/bitcoin/pull/31363#discussion_r2009189848)
I tried to encapsulate it in its own class, but it's hard to give it the same performance (which is relevant in the context of huge reorgs) without making the interface rather involved (needs a "representative" type that contains a pointer to the `PartitionData` object, separate from the element data type), and even then, the union-find version in `Trim()` in 31553 would need its own separate implementation still.
Going to leave it for now, we can think about cleanups later.
(https://github.com/bitcoin/bitcoin/pull/31363#discussion_r2009189848)
I tried to encapsulate it in its own class, but it's hard to give it the same performance (which is relevant in the context of huge reorgs) without making the interface rather involved (needs a "representative" type that contains a pointer to the `PartitionData` object, separate from the element data type), and even then, the union-find version in `Trim()` in 31553 would need its own separate implementation still.
Going to leave it for now, we can think about cleanups later.
💬 yancyribbens commented on pull request "fuzz: wallet: fix crypter target":
(https://github.com/bitcoin/bitcoin/pull/32118#issuecomment-2746352554)
nit: the commit message is pretty sparse. Could add a "why" section to the commit.
(https://github.com/bitcoin/bitcoin/pull/32118#issuecomment-2746352554)
nit: the commit message is pretty sparse. Could add a "why" section to the commit.
💬 willcl-ark commented on issue "b-msghand invoked oom-killer: Master (v28.99) crashing during IBD":
(https://github.com/bitcoin/bitcoin/issues/31561#issuecomment-2746371802)
OK well, I've been unable to reproduce using docker limited to 8g or ram with swap disabled:
```bash
$ docker run --memory="8g" --memory-swap="8g" --volume="/tmp/31561:/home/bitcoin/.bitcoin" -it bitcoin/bitcoin:master -txindex=1
```
Synced just fine to block 868650 where I shut down manually. The docker image is at commit 2db00278ea57
(https://github.com/bitcoin/bitcoin/issues/31561#issuecomment-2746371802)
OK well, I've been unable to reproduce using docker limited to 8g or ram with swap disabled:
```bash
$ docker run --memory="8g" --memory-swap="8g" --volume="/tmp/31561:/home/bitcoin/.bitcoin" -it bitcoin/bitcoin:master -txindex=1
```
Synced just fine to block 868650 where I shut down manually. The docker image is at commit 2db00278ea57
💬 pinheadmz commented on issue "b-msghand invoked oom-killer: Master (v28.99) crashing during IBD":
(https://github.com/bitcoin/bitcoin/issues/31561#issuecomment-2746382688)
Ok I'll try to reproduce and if I cant ill close.
(https://github.com/bitcoin/bitcoin/issues/31561#issuecomment-2746382688)
Ok I'll try to reproduce and if I cant ill close.
💬 fjahr commented on pull request "test: Add encodable PUSHDATA1 examples to feature_taproot":
(https://github.com/bitcoin/bitcoin/pull/32114#issuecomment-2746424182)
Concept ACK
Not sure about having the documentation to add new spenders mixed within this particular set of spenders. What if I want to add new spenders Maybe it would be better in a comment block at the top of the test or in the docstring of `spenders_taproot_active`?
(https://github.com/bitcoin/bitcoin/pull/32114#issuecomment-2746424182)
Concept ACK
Not sure about having the documentation to add new spenders mixed within this particular set of spenders. What if I want to add new spenders Maybe it would be better in a comment block at the top of the test or in the docstring of `spenders_taproot_active`?
💬 theStack commented on pull request "test: add missing segwitv1 test cases to `script_standard_tests`":
(https://github.com/bitcoin/bitcoin/pull/31340#issuecomment-2746468990)
@tdb3 @hodlinator @rkrux: Thanks for your detailed reviews, and sorry for the late reply.
> Why is this being removed/changed?
>
> https://github.com/bitcoin/bitcoin/blob/22ef95dbe3e467039e6cd18988e66557d94041d1/src/test/script_standard_tests.cpp#L271-L276
>
> As I understand it we were creating a taproot output (v1) using a compressed pubkey with the type-prefix prepended, meaning it was 33 bytes instead of the standard 32. Maybe you could add it back with a clearer comment?
> @hod
...
(https://github.com/bitcoin/bitcoin/pull/31340#issuecomment-2746468990)
@tdb3 @hodlinator @rkrux: Thanks for your detailed reviews, and sorry for the late reply.
> Why is this being removed/changed?
>
> https://github.com/bitcoin/bitcoin/blob/22ef95dbe3e467039e6cd18988e66557d94041d1/src/test/script_standard_tests.cpp#L271-L276
>
> As I understand it we were creating a taproot output (v1) using a compressed pubkey with the type-prefix prepended, meaning it was 33 bytes instead of the standard 32. Maybe you could add it back with a clearer comment?
> @hod
...
💬 theStack commented on pull request "test: add missing segwitv1 test cases to `script_standard_tests`":
(https://github.com/bitcoin/bitcoin/pull/31340#discussion_r2009258041)
Done.
(https://github.com/bitcoin/bitcoin/pull/31340#discussion_r2009258041)
Done.
💬 santochibtc commented on issue "Enable PCP by default?":
(https://github.com/bitcoin/bitcoin/issues/31663#issuecomment-2746474104)
not working with a router Arcadyan PRV3399B-B-LT with UPnP enabled
```
2025-03-23T21:09:32Z [net] portmap: gateway [IPv4]: x.x.x.x
2025-03-23T21:09:32Z [net] pcp: Requesting port mapping for addr 0.0.0.0 port 8333 from gateway x.x.x.x
2025-03-23T21:09:32Z [net] pcp: Internal address after connect: x.x.x.x
2025-03-23T21:09:32Z [net:warning] pcp: Could not receive response: Connection refused (111)
2025-03-23T21:09:32Z [net] portmap: Could not determine IPv6 default gateway
```
(https://github.com/bitcoin/bitcoin/issues/31663#issuecomment-2746474104)
not working with a router Arcadyan PRV3399B-B-LT with UPnP enabled
```
2025-03-23T21:09:32Z [net] portmap: gateway [IPv4]: x.x.x.x
2025-03-23T21:09:32Z [net] pcp: Requesting port mapping for addr 0.0.0.0 port 8333 from gateway x.x.x.x
2025-03-23T21:09:32Z [net] pcp: Internal address after connect: x.x.x.x
2025-03-23T21:09:32Z [net:warning] pcp: Could not receive response: Connection refused (111)
2025-03-23T21:09:32Z [net] portmap: Could not determine IPv6 default gateway
```