Bitcoin Core Github
42 subscribers
125K links
Download Telegram
💬 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
```
💬 willcl-ark commented on pull request "Fee estimation: avoid serving stale fee estimate":
(https://github.com/bitcoin/bitcoin/pull/27622#discussion_r1219177648)
I know we only use it once in the test, but if you re-touch this file it could be nice to use a named variable for the 60 hours, to make it clear where it comes from? (e.g. `MAX_FILE_AGE` would shadow the name used in _fees.h_)
💬 hebasto commented on pull request "guix: Clean up manifest":
(https://github.com/bitcoin/bitcoin/pull/27811#issuecomment-1578197211)
Updated 86739705406edbe89bfb039a63fb33361990d673 -> 404a0e98dd964af4db338843f65ca3a63429db17 ([pr27811.02](https://github.com/hebasto/bitcoin/commits/pr27811.02) -> [pr27811.03](https://github.com/hebasto/bitcoin/commits/pr27811.03), [diff](https://github.com/hebasto/bitcoin/compare/pr27811.02..pr27811.03)):

- addressed @TheCharlatan's [comment](https://github.com/bitcoin/bitcoin/pull/27811#discussion_r1219147914)
💬 hebasto commented on pull request "guix: Clean up manifest":
(https://github.com/bitcoin/bitcoin/pull/27811#discussion_r1219192511)
> What did this do?

Effectively, [nothing](https://github.com/bitcoin/bitcoin/pull/27779#issuecomment-1572754794).
💬 MarcoFalke commented on issue "Intermittent failures in interface_usdt_mempool.py":
(https://github.com/bitcoin/bitcoin/issues/27380#issuecomment-1578204112)
Ok, closing for now. Let's reopen if this is an issue again.
MarcoFalke closed an issue: "Intermittent failures in interface_usdt_mempool.py"
(https://github.com/bitcoin/bitcoin/issues/27380)
💬 0xB10C commented on pull request "test: handle failed `check_equal()` assertions in bcc callback functions":
(https://github.com/bitcoin/bitcoin/pull/27831#issuecomment-1578237838)
Concept ACK. Thanks for picking this up! Seeing the asserts fail in the CI logs is needed and would have helped figuring out the reason for https://github.com/bitcoin/bitcoin/issues/27380.

(I think you meant to write `assert_equal()` instead of `check_equal()`)