Bitcoin Core Github
44 subscribers
121K links
Download Telegram
💬 achow101 commented on pull request "test: Add test for `sendmany` rpc that uses `subtractfeefrom` parameter":
(https://github.com/bitcoin/bitcoin/pull/26733#issuecomment-1529706820)
ACK 057057a2d7e23c2e29cbfd29a5124b3947a33757
🚀 fanquake merged a pull request: "test: add coverage for `-bantime`"
(https://github.com/bitcoin/bitcoin/pull/26604)
💬 michaelfolkson commented on pull request "Remove BIP35 mempool p2p message":
(https://github.com/bitcoin/bitcoin/pull/27426#issuecomment-1529711107)
@0xB10C: Thanks for this, great analysis.

> I'm generally in favor of slimming down and removing the message (and also the BIP-37 implementation) at some point.

I read that as a longer term Concept ACK for removal but you'd like a deprecation release schedule rather than removing it immediately in say the next major version release. Presumably something like a deprecation warning in the next major version release before actual deprecation in the following major version release could work f
...
🚀 achow101 merged a pull request: "test: Add test for `sendmany` rpc that uses `subtractfeefrom` parameter"
(https://github.com/bitcoin/bitcoin/pull/26733)
💬 fanquake commented on pull request "ci: use LLVM/clang-16 in native_asan job":
(https://github.com/bitcoin/bitcoin/pull/27360#issuecomment-1529712915)
> You could remove the reference to this PR

Yep, dropped.
💬 MarcoFalke commented on pull request "ci: Add test coverage job":
(https://github.com/bitcoin/bitcoin/pull/27547#issuecomment-1529712995)
I think it is good to have coverage reports, but I am not sure if it is good to overlay them in the files tab. It might be too invasive? Maybe just a comment with a link is fine?

For the other commits: It would be good for reviewers to explain and motivate them, and explain why they are fine to do and what their tradeoffs are (especially not deleting the gcda files?)
🚀 fanquake merged a pull request: "test: Remove modinv python util helper function"
(https://github.com/bitcoin/bitcoin/pull/27538)
💬 MarcoFalke commented on pull request "ci: Add test coverage job":
(https://github.com/bitcoin/bitcoin/pull/27547#issuecomment-1529714362)
Also, if the other commits are self-motivated and stand-alone, they can be submitted/reviewed/tested in a separate pull
🚀 fanquake merged a pull request: "[23.x] Additional backports for 23.x"
(https://github.com/bitcoin/bitcoin/pull/27475)
💬 fanquake commented on issue "Proposed Timeline for Legacy Wallet and BDB removal":
(https://github.com/bitcoin/bitcoin/issues/20160#issuecomment-1529718871)
> I think the milestones will need to be edited in OP to be moved back by one release?

Moved all the remaining milestones back by a release.
💬 fanquake commented on pull request "lint: stop ignoring LIEF imports":
(https://github.com/bitcoin/bitcoin/pull/27507#issuecomment-1529721582)
> contrib/devtools/security-check.py:246: error: "object" has no attribute "header" [attr-defined]

The code is working as expected. This seems to either be a bug in mypy, or an issue with the LIEF stubs. Am following up.
💬 fanquake commented on issue "build: Explore building with musl as libc":
(https://github.com/bitcoin/bitcoin/issues/18110#issuecomment-1529727292)
I think the intial route we'll take for static builds will be to use glibc.
💬 aureleoules commented on pull request "ci: Add test coverage job":
(https://github.com/bitcoin/bitcoin/pull/27547#issuecomment-1529730669)
> I think it is good to have coverage reports, but I am not sure if it is good to overlay them in the files tab. It might be too invasive? Maybe just a comment with a link is fine?

You can toggle the annotations in the Files tab client-side using the `a` key.
Or we can fully disable the annotations feature with a custom `codecov.yml` config file but since they are togglable I think it's fine.

It would be nice if Drahtbot could link to the Codecov report also.

> For the other commits: I
...
💬 willcl-ark commented on pull request "Remove BIP35 mempool p2p message":
(https://github.com/bitcoin/bitcoin/pull/27426#issuecomment-1529767111)
Thanks for your statistics on the usage of this message @0xB10C, they show a compelling amount of use for the BIP35 protocol still.

As David Harding suggested on the mailing list, which I agree with, removing an actively-used feature in absence of an alternative would be undesirable, and therefore will close this PR for now.
willcl-ark closed a pull request: "Remove BIP35 mempool p2p message"
(https://github.com/bitcoin/bitcoin/pull/27426)
💬 RandyMcMillan commented on pull request "rpc,rest,zmq: faster getblock, NotifyBlock and rest_block by reading raw block":
(https://github.com/bitcoin/bitcoin/pull/26415#issuecomment-1529782191)
Concept ACK
💬 RandyMcMillan commented on issue "ci: failure in Docker build step":
(https://github.com/bitcoin/bitcoin/issues/27492#issuecomment-1529795351)
Have you tried adding --no-cache flag to the docker build command?
💬 instagibbs commented on pull request "policy: nVersion=3 and Package RBF":
(https://github.com/bitcoin/bitcoin/pull/25038#discussion_r1181639284)
I don't think this is necessary. `get_utxos` already filters for mature coinbases, and you're making 60 of those above.
📝 brunoerg opened a pull request: "fuzz: addrman, add coverage for `network` field in `Select()`"
(https://github.com/bitcoin/bitcoin/pull/27549)
This PR adds fuzz coverage for `network` field in `Select()`, there was only a call to `Select` without passing a network.
https://marcofalke.github.io/b-c-cov/fuzz.coverage/src/addrman.cpp.gcov.html
💬 ryanofsky commented on pull request "Add fee_est tool for debugging fee estimation code":
(https://github.com/bitcoin/bitcoin/pull/10443#issuecomment-1529830672)
@josie thanks for looking at this. I don't think I understand what the idea behind an approach NACK would be based on what you are saying, though.

It sounds more like you're asking for an additional feature which would be easy to implement on top of the first commit, not pointing to a drawback of the approach. If you want to gather live fee estimates, the first commit outputs a structured [jsonl](https://jsonlines.org/) log of fee estimation events, so it would be easy to add the actual fee e
...