Bitcoin Core Github
44 subscribers
122K links
Download Telegram
📝 fanquake locked a pull request: "."
(https://github.com/bitcoin/bitcoin/pull/28198)
<!--
*** 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/extracoin-core/gui
first. See CONTRIBUTING.md
-->

<!--
Please provide clear motivation for your patch and explain how it improves
extracoin Core user experience or extracoin Core developer experience
significantly:

* Any test improvements or new tests that
...
💬 MarcoFalke commented on pull request "blockstorage: XOR blocksdir *.dat files":
(https://github.com/bitcoin/bitcoin/pull/28052#issuecomment-1661738608)
Rebased, and made every commit a refactor, except for the one that has release notes and test changes :)
💬 fanquake commented on pull request "test: dedup file hashing using `sha256sum_file` helper":
(https://github.com/bitcoin/bitcoin/pull/27572#issuecomment-1661842143)
Concept ACK. Did you want to address the review comment, or just mark as resolved.
💬 MarcoFalke commented on pull request "lint: fix custom mypy cache dir setting":
(https://github.com/bitcoin/bitcoin/pull/28184#discussion_r1281660991)
If it is not needed, it may be better to remove it.
💬 theStack commented on pull request "test: dedup file hashing using `sha256sum_file` helper":
(https://github.com/bitcoin/bitcoin/pull/27572#discussion_r1281680165)
Thanks for reviewing! Decided to keep the name as it is, as sha256 falls into the SHA message digests category (see e.g. https://linux.die.net/man/1/shasum) and for the sake of the test concrete type of hash used doesn't matter anyway. All we care is detecting if the wallet file contents have been changed.
💬 hebasto commented on issue "ci: Future of macOS and Windows MSVC CI tasks":
(https://github.com/bitcoin/bitcoin/issues/28098#issuecomment-1661920835)
> Thus, someone would have to sponsor an amount of roughly 5kUSD/mo for those two tasks.

According to Cirrus CI stats for June / July 2023, the usage of resources was equivalent to compute credits/USD as follows:
- Windows -- 2275 / 1941
- Linux -- 2546 / 3160
- macOS -- 2053 / 3568

Therefore, future of Linux tasks should be considered as well.
💬 MarcoFalke commented on issue "ci: Future of macOS and Windows MSVC CI tasks":
(https://github.com/bitcoin/bitcoin/issues/28098#issuecomment-1661941813)
> Therefore, future of Linux tasks should be considered as well.

Have you seen https://github.com/bitcoin/bitcoin/pull/28161 ?
👍 willcl-ark approved a pull request: "test: cover addrv2 anchors by adding TorV3 to CAddress in messages.py"
(https://github.com/bitcoin/bitcoin/pull/27452#pullrequestreview-1558684193)
ACK ba8ab4fc54

Nice to have this additional coverage for TorV3 addresses 👍🏼

It's not intruduced by this PR, but I notice a mypy error in _feature_anchors.py_ where a tuple is assigned to `onion_conf.addr` which is initialized to `None` in _socks5.py_.

![image](https://github.com/bitcoin/bitcoin/assets/6606587/c44f86b1-15ab-4d86-9e89-5617866b8dad)

If you do end up re-touching this for some reason, you can fix the warning by specifying the type of `.addr` in _socks5.py_ (which will
...
💬 dergoegge commented on pull request "Relay own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/27509#issuecomment-1661944988)
> I don't like the idea of a separate tool because that's like saying "we can't get the current software to do it properly, so we will create another software".

"we can't get the current software to do it properly" that is kinda true w.r.t. fingerprinting, or it would at least require significant effort to avoid it completely. Fingerprinting negates the usefulness of this feature, because `"The peers that receive the transaction could deduce that this is initial transaction broadcast from the
...
👍 kristapsk approved a pull request: "test: dedup file hashing using `sha256sum_file` helper"
(https://github.com/bitcoin/bitcoin/pull/27572#pullrequestreview-1558692172)
ACK 2c0c6f44770403899bd8514ad7343356853bf38c
💬 Daniel600 commented on pull request "policy: Enable full-rbf by default":
(https://github.com/bitcoin/bitcoin/pull/28132#issuecomment-1661960440)
A clear and open method to research the adoption of full RBF would look something like this and could easily be done -

Create 20 trxs (larger numbers better) every block and after 30 seconds try replace them.
Run this test for at least a few hours preferably more than 24 hours or even a few days.
See results of how many were replaced.
Ignore trx results if trx are included in blocks before replace trxs are published.

Based on a test like this or something similar it would be reliable t
...
💬 MarcoFalke commented on pull request "Relay own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/27509#issuecomment-1661971280)
> I'd prefer a separate tool as we otherwise turn fingerprinting into a big maintenance issue. A separate tool guarantees that there is no internal state overlap which makes fingerprinting very hard (if not impossible).

Not sure why this requires a separate tool? I think a separate internal module (or internal library that is linked, or whatever) should be able to achieve the same, with the added benefit that users can use it in Bitcoin Core without having to fiddle with another binary.
💬 hebasto commented on pull request "ci: Move ASan USDT to persistent_worker":
(https://github.com/bitcoin/bitcoin/pull/28161#issuecomment-1661972486)
Concept ACK on moving CI tasks outside from Cirrus CI as a response to their new [limits](https://cirrus-ci.org/blog/2023/07/17/limiting-free-usage-of-cirrus-ci/).

Historically, the Bitcoin Core project started to use self-hosted CI in [April 2021](https://github.com/bitcoin/bitcoin/pull/21619):
> For testing purposes, a single task will be transformed to run on the DrahtBot infrastructure. If all goes well, the other tasks can be moved, too.

At the same time, there were some usage of the
...
💬 vasild commented on pull request "Relay own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/27509#discussion_r1281726530)
_moving discussion from https://github.com/bitcoin/bitcoin/pull/27509#issuecomment-1659008138_

> 1. `-sensitiverelay=1` and `-onlynet=ipv4` should probably throw an init error. In this configuration, sensitive stuff seems to be ignored and new wallet TXs are relayed via ipv4 peers like normal.

This check was supposed to prevent that but it was too strict. Apparently you have `listenonion=1` which means that later, after startup, we will connect to the Tor control and may set the Tor proxy
...
💬 dergoegge commented on pull request "Relay own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/27509#issuecomment-1661976345)
> I think a separate internal module (or internal library that is linked, or whatever) should be able to achieve the same, with the added benefit that users can use it in Bitcoin Core without having to fiddle with another binary.

Sure but that is the "significant effort" i was referring to. You would need to completely decouple this new logic (and its state) from the other full node networking and message processing logic.
🚀 fanquake merged a pull request: "test: dedup file hashing using `sha256sum_file` helper"
(https://github.com/bitcoin/bitcoin/pull/27572)
📝 glozow opened a pull request: "functional test: tx orphan handling"
(https://github.com/bitcoin/bitcoin/pull/28199)
I was doing some mutation testing (through reckless refactoring) locally and found some specific behaviors in orphan handling that weren't picked up by tests. Adding some of these test cases now can maybe help with reviewing refactors like #28031.

- Parent requests aren't sent immediately. A delay is added and the requests are filtered by AlreadyHaveTx before they are sent, which means you can't use fake orphans to probe precise arrival timing of a tx.
- Parent requests include all that are
...
💬 MarcoFalke commented on pull request "ci: Move ASan USDT to persistent_worker":
(https://github.com/bitcoin/bitcoin/pull/28161#issuecomment-1662036818)
> Tbh, I'd feel much more confident and comfortable knowing that some sponsors publicly pledged to cover all related costs.

Ok, I'll try to encourage the sponsor to publicly identify themselves, but I wonder what you worry about? If you are worried about the sponsor walking away, I can assure you that there are currently three (3) willing sponsors in line. Also, while it isn't free, running Linux is extremely cheap compared to Windows/macOS.

If you have any alternative ideas, it would be g
...
💬 hebasto commented on pull request "ci: Move ASan USDT to persistent_worker":
(https://github.com/bitcoin/bitcoin/pull/28161#issuecomment-1662057512)
> > Tbh, I'd feel much more confident and comfortable knowing that some sponsors publicly pledged to cover all related costs.
>
> Ok, I'll try to encourage the sponsor to publicly identify themselves

Sorry for being misunderstood. I don't want to force current sponsors to any additional actions.

> ... but I wonder what you worry about?

Sustainability.

> Also, while it isn't free, running Linux is extremely cheap compared to Windows/macOS.

I don't think so. In June, total Linux
...
👍 hebasto approved a pull request: "ci: Move ASan USDT to persistent_worker"
(https://github.com/bitcoin/bitcoin/pull/28161#pullrequestreview-1558764134)
ACK fa474397b5d4235efdfc5a5ddce2d637234548a7, I have reviewed the code and it looks OK.