Bitcoin Core Github
42 subscribers
125K links
Download Telegram
💬 mzumsande commented on pull request "Relay own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/27509#discussion_r1218631408)
I'm a bit worried about this priority: What would happen if we, for some reason, aren't able to make successful sensitive relay connections (e.g. we think that we can connect to Tor but misconfigured something, or maybe we can connect but don't have any addresses for these networks in our addrman).
Wouldn't we try again and again (since this is the first priority and `m_sensitive_relay_connections_to_open` is never reduced) and never attempt to make an outbound connection of another type?
📝 Brotcrunsher opened a pull request: "Supporting parameter "h" and "?" in -netinfo."
(https://github.com/bitcoin/bitcoin/pull/27830)
The main -help argument supports these two as well, so we might as well behave the same here.
💬 ariard commented on pull request "Remove -mempoolfullrbf option":
(https://github.com/bitcoin/bitcoin/pull/26525#issuecomment-1577688541)
Note for the clarity, I think effectively both a loosening (e.g nVersion=3 where TX_MAX_STANDARD_VERSION allows nVersion=3 transaction propagation) and a tightening (e.g some nSequence field usage being banned from propagation) can be used to commit a double-spend.

With a loosening, if the victim full-node is not upgraded, they won't accept your new nVersion=3 transactions, and you can feed the rest of the upgraded network with your nVersion=3 double-spend.

With a tightening, if the victim
...
💬 0xB10C commented on issue "Duplicate coinbase transaction space reservation in CreateNewBlock":
(https://github.com/bitcoin/bitcoin/issues/21950#issuecomment-1577702096)
> And actually it looks like ALL of F2Pool's recent blocks are actually > 3,996,000 in weight. They may be the only pool using their own software to create block templates?

I'm fairly certain they use Bitcoin Core. You can control the max block weight with `-blockmaxweight` (default is 3996000 WU). Also, F2Pool is known for running with custom patches on top of Bitcoin Core to maximize what they can: e.g. they reduced the number of sigops reserved for the coinbase. This [bit them twice](https
...
💬 pinheadmz commented on issue "Duplicate coinbase transaction space reservation in CreateNewBlock":
(https://github.com/bitcoin/bitcoin/issues/21950#issuecomment-1577730448)
@0xB10C thanks for these great insights. What do you think about the issue? The `4000` is reserved in two places:

https://github.com/bitcoin/bitcoin/blob/f4a8269dfc144cc918570bdb870aa5143a11c1fe/src/node/miner.cpp#L97

https://github.com/bitcoin/bitcoin/blob/f4a8269dfc144cc918570bdb870aa5143a11c1fe/src/policy/policy.h#L23

Should we remove one of these?
💬 ajtowns commented on pull request "Remove -mempoolfullrbf option":
(https://github.com/bitcoin/bitcoin/pull/26525#issuecomment-1577762400)
> Note for the clarity, I think effectively both a loosening (e.g nVersion=3 where TX_MAX_STANDARD_VERSION allows nVersion=3 transaction propagation) and a tightening (e.g some nSequence field usage being banned from propagation) can be used to commit a double-spend.

Any mempool acceptance policy difference between two nodes can be used to prevent your node from seeing transactions the other node accepts. If you outright reject tx A, then spending an output from that tx will suffice. If you a
...
💬 jarolrod commented on pull request "guix: Update `python-lief` package to 0.13.1":
(https://github.com/bitcoin/bitcoin/pull/27813#issuecomment-1577862296)
Guix hashes

```
f3006576f4882414f9bbe9e37dbf54567bf24cff5252e487260a79d7f411c0bd guix-build-e3792a660abf/output/aarch64-linux-gnu/SHA256SUMS.part
61e90a272ab6d534ade60c46da90809f5d03017d31fed4b24225780db8c4aa4d guix-build-e3792a660abf/output/aarch64-linux-gnu/bitcoin-e3792a660abf-aarch64-linux-gnu-debug.tar.gz
a6e23c73e89076f5cb8c712effd6dd5796fb1e9bfd720b9310b829128e7a94f1 guix-build-e3792a660abf/output/aarch64-linux-gnu/bitcoin-e3792a660abf-aarch64-linux-gnu.tar.gz
8531ad1072f660de83
...
💬 hebasto commented on pull request "Use `int32_t` type for most transaction size/weight values":
(https://github.com/bitcoin/bitcoin/pull/23962#discussion_r1219057335)
@ryanofsky

I agree with the suggested changes. In addition, it seems reasonable to combine them with making signed `nCountWithAncestors` and return types of the related getter methods as well. I'd leave such changes for a follow up. Or do you prefer to incorporate them in here?
💬 vasild commented on pull request "Relay own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/27509#discussion_r1219111950)
The guidelines allow both and I prefer this because: https://github.com/vasild/bitcoin/wiki/if-on-the-same-line
💬 Sjors commented on pull request "rpc: fix data optionality for RPC calls.":
(https://github.com/bitcoin/bitcoin/pull/27829#discussion_r1219120387)
I find this wording confusing. Do you mean to say that the user can combine data and address entries? Or that you can provide data or an address directly without putting "data" or "address" in front of it?

Examples can be helpful. Maybe say "see send RPC for examples" and then expand the `send` RPC docs with said examples.
💬 vasild commented on pull request "Relay own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/27509#discussion_r1219127718)
This chain of peers is desired / demanded to consider the transaction successfully propagated. Lets say we are connected to 30 peers (some inbound, some outbound). If somebody of them gets the transaction (from somebody else, not from us) he will tell us about it due to the normal flooding mechanism because he will not be aware we have it. If all our 30 peers stay silent about the transaction, it means that they did not get it which is a sign it propagated only to some part of the network or did
...
💬 Sjors commented on pull request "ci: label docker images and prune dangling images selectively":
(https://github.com/bitcoin/bitcoin/pull/27793#issuecomment-1578129056)
Concept ACK. Haven't tested.

It would also be nice to have an incantation (e.g. in the help) to kill dangling images after CI failure / interruption (if you don't plan on running it again).
💬 vasild commented on pull request "Relay own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/27509#issuecomment-1578149945)
> > Tor-to-IPv4 exit nodes,
>
> I was more asking about this, not as much from a vulnerability standpoint but more like, the bottleneck created by all TXs going through a specific subset of network nodes first. I think your envelope math is reassuring though - I didn't realize there were so many Tor nodes already

Well, ok, we don't know how many of those Tor or I2P nodes are also connected to clearnet. For this secondary propagation (after sensitive relay has fired the transaction) however
...
💬 TheCharlatan commented on pull request "guix: Clean up manifest":
(https://github.com/bitcoin/bitcoin/pull/27811#discussion_r1219147914)
Would it make sense to add `((gnu packages linux) #:select (util-linux linux-libre-headers-5.15))`?
💬 Sjors commented on pull request "guix: Clean up manifest":
(https://github.com/bitcoin/bitcoin/pull/27811#discussion_r1219151936)
641897a83dc9d40b618efbae67c3beb90a1f34f8: What did this do?
📝 virtu opened a pull request: "test: handle failed `check_equal()` assertions in bcc callback functions"
(https://github.com/bitcoin/bitcoin/pull/27831)
Address #27380 (and similar future issues) by handling failed `check_equal()` assertions in bcc callback functions

### Problem

Exceptions are not propagated in ctype callback functions used by bcc. This means an AssertionError exception raised by `check_equal()` to signal a failed assertion is not getting caught and properly logged. Instead, the error is logged to stdout and execution of the callback stops.

The current workaround to check whether all `check_equal()` assertions in a c
...
💬 virtu commented on issue "Intermittent failures in interface_usdt_mempool.py":
(https://github.com/bitcoin/bitcoin/issues/27380#issuecomment-1578194840)
Since the failure hasn't been observed since https://github.com/bitcoin/bitcoin/pull/27177 was merged, there's a good chance it fixed the issue.

However, it still seems like a good idea to address the issue of missing `check_equal()` exception information in the logs, so I created https://github.com/bitcoin/bitcoin/pull/27831 to make sure all tracepoint tests properly log failed `check_equal()` assertions.
👍 willcl-ark approved a pull request: "Fee estimation: avoid serving stale fee estimate"
(https://github.com/bitcoin/bitcoin/pull/27622#pullrequestreview-1464599043)
ACK 451ab8b513

Left a few comments on the test in case you touch that file again, but nothing blocking.

Not loading very old fee estimates makes good sense to me to further avoid creating stuck transactions.
💬 willcl-ark commented on pull request "Fee estimation: avoid serving stale fee estimate":
(https://github.com/bitcoin/bitcoin/pull/27622#discussion_r1219184270)
```suggestion
# Verify that the estimates are not the same if new blocks were produced in the flush interval
```