💬 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
(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)
(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.
(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.
(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.
(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
...
(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.
(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)
(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
(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?
(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.
(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
(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
...
(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
...
💬 pinheadmz commented on pull request "refactor: Remove CAddressBookData::destdata":
(https://github.com/bitcoin/bitcoin/pull/27224#discussion_r1181656908)
I was momentarily confused by this because (IIUC) map keys are the `int64_t id` field of `RecentRequestEntry` but actually encoded into `std::string` by a few methods in `RecentRequestsTableModel`. So I'm wondering: Is there a reason we need to keep those IDs as strings? Is it like, at this point, going back to int64 would be a needless refactor?
(https://github.com/bitcoin/bitcoin/pull/27224#discussion_r1181656908)
I was momentarily confused by this because (IIUC) map keys are the `int64_t id` field of `RecentRequestEntry` but actually encoded into `std::string` by a few methods in `RecentRequestsTableModel`. So I'm wondering: Is there a reason we need to keep those IDs as strings? Is it like, at this point, going back to int64 would be a needless refactor?
💬 jarolrod commented on pull request "qt: 25.0rc2 translations update":
(https://github.com/bitcoin/bitcoin/pull/27517#issuecomment-1529845383)
Concept ACK, could use an update?
(https://github.com/bitcoin/bitcoin/pull/27517#issuecomment-1529845383)
Concept ACK, could use an update?
💬 RandyMcMillan commented on pull request "bumpfee: Allow the user to choose which output is change":
(https://github.com/bitcoin/bitcoin/pull/26467#issuecomment-1529846864)
Concept ACK
(https://github.com/bitcoin/bitcoin/pull/26467#issuecomment-1529846864)
Concept ACK
💬 RandyMcMillan commented on pull request "Add warnings for non-active addresses in receive tab and address book":
(https://github.com/bitcoin-core/gui/pull/723#issuecomment-1529850188)
Concept ACK
(https://github.com/bitcoin-core/gui/pull/723#issuecomment-1529850188)
Concept ACK
💬 ryanofsky commented on pull request "refactor: Remove CAddressBookData::destdata":
(https://github.com/bitcoin/bitcoin/pull/27224#discussion_r1181688010)
re: https://github.com/bitcoin/bitcoin/pull/27224#discussion_r1181656908
> So I'm wondering: Is there a reason we need to keep those IDs as strings? Is it like, at this point, going back to int64 would be a needless refactor?
Only reason that the [`CAddressBookData::receive_requests`](https://github.com/bitcoin/bitcoin/blob/be0325c6a62505d63bc07320b05e31618ef9bbb1/src/wallet/wallet.h#L232-L239) map containing the requests has string keys and string values is that the [`setAddressReceiveReq
...
(https://github.com/bitcoin/bitcoin/pull/27224#discussion_r1181688010)
re: https://github.com/bitcoin/bitcoin/pull/27224#discussion_r1181656908
> So I'm wondering: Is there a reason we need to keep those IDs as strings? Is it like, at this point, going back to int64 would be a needless refactor?
Only reason that the [`CAddressBookData::receive_requests`](https://github.com/bitcoin/bitcoin/blob/be0325c6a62505d63bc07320b05e31618ef9bbb1/src/wallet/wallet.h#L232-L239) map containing the requests has string keys and string values is that the [`setAddressReceiveReq
...
💬 pinheadmz commented on pull request "Add warnings for non-active addresses in receive tab and address book":
(https://github.com/bitcoin-core/gui/pull/723#issuecomment-1529982632)
> Concept ACK
Thanks Randy! I just rebased on master to fix merge conflict. If you'd like to see this move forward, you can help my reviewing the parent Bitcoin Core PR: https://github.com/bitcoin/bitcoin/pull/27216 🙏 ❤️
(https://github.com/bitcoin-core/gui/pull/723#issuecomment-1529982632)
> Concept ACK
Thanks Randy! I just rebased on master to fix merge conflict. If you'd like to see this move forward, you can help my reviewing the parent Bitcoin Core PR: https://github.com/bitcoin/bitcoin/pull/27216 🙏 ❤️
💬 pinheadmz commented on issue "v 0.17.0 win 7 64 bit missing recent transactions. ":
(https://github.com/bitcoin/bitcoin/issues/14541#issuecomment-1529990436)
@MarcoFalke I think [this comment](https://github.com/bitcoin/bitcoin/issues/14541#issuecomment-621434177) made us think it was not the wallet's fault but the p2p stalling issue. @VadimEv are you able to test again with the latest release of Bitcoin Core?
(https://github.com/bitcoin/bitcoin/issues/14541#issuecomment-1529990436)
@MarcoFalke I think [this comment](https://github.com/bitcoin/bitcoin/issues/14541#issuecomment-621434177) made us think it was not the wallet's fault but the p2p stalling issue. @VadimEv are you able to test again with the latest release of Bitcoin Core?