Bitcoin Core Github
44 subscribers
122K links
Download Telegram
💬 MarcoFalke commented on pull request "refactor: Extract util/fs from util/system":
(https://github.com/bitcoin/bitcoin/pull/27254#discussion_r1135396391)
```suggestion
#include <cstdint>
#include <cstdio>
```

nit
💬 MarcoFalke commented on pull request "refactor: Extract util/fs from util/system":
(https://github.com/bitcoin/bitcoin/pull/27254#discussion_r1135397641)
unrelated change?
💬 fanquake commented on pull request "refactor: Extract util/fs from util/system":
(https://github.com/bitcoin/bitcoin/pull/27254#discussion_r1135402195)
nit: missing newline
💬 TheCharlatan commented on pull request "refactor: Extract util/fs from util/system":
(https://github.com/bitcoin/bitcoin/pull/27254#discussion_r1135404419)
This is from clang format diff. Should I drop these kind of changes in the future? There are also some other not strictly related include order fixes.
💬 fanquake commented on pull request "refactor: Extract util/fs from util/system":
(https://github.com/bitcoin/bitcoin/pull/27254#discussion_r1135405169)
Pretty sure this can just be merged into the includes below.
💬 fanquake commented on pull request "refactor: Extract util/fs from util/system":
(https://github.com/bitcoin/bitcoin/pull/27254#discussion_r1135406000)
I think include fixing is fine. However theres no need to include unrelated clang-format changes.
💬 MarcoFalke commented on pull request "refactor: Extract util/fs from util/system":
(https://github.com/bitcoin/bitcoin/pull/27254#discussion_r1135406385)
You'll need to set `-U0` for the context to avoid unrelated changes, no?
💬 fanquake commented on pull request "refactor: Split util/system into exception, shell, and fs-specific files":
(https://github.com/bitcoin/bitcoin/pull/25152#issuecomment-1467936449)
Going to close this for now, given that it's being split out, and there's a [thumbs up from the author above](https://github.com/bitcoin/bitcoin/pull/25152#issuecomment-1458326647).
fanquake closed a pull request: "refactor: Split util/system into exception, shell, and fs-specific files"
(https://github.com/bitcoin/bitcoin/pull/25152)
💬 TheCharlatan commented on pull request "refactor: Extract util/fs from util/system":
(https://github.com/bitcoin/bitcoin/pull/27254#discussion_r1135417534)
I'm formatting each commit with `git diff -U0 HEAD~1.. | ./contrib/devtools/clang-format-diff.py -p1 -i -v`. I think I must have included this unrelated change somewhere. Will fix.
💬 0xB10C commented on pull request "Add feerate histogram to getmempoolinfo":
(https://github.com/bitcoin/bitcoin/pull/21422#issuecomment-1467960000)
> > Could a tracepoints approach, similar to #26531, work?
>
> I need to research that in detail but it looks promising. Thank you for sharing it here!

I don't think tracepoints are suited for this use-case. Tracepoints allow you to react to a specific event. Here, you request / pull data when a consumer needs it.

> So what would be your approach to do it externally then? Especially if you want to get this information more often.

You can query the `getrawmempool` RPC with the verbose
...
💬 TheCharlatan commented on pull request "refactor: Extract util/fs from util/system":
(https://github.com/bitcoin/bitcoin/pull/27254#issuecomment-1467983104)
Thanks for the review!

Updated b67efe778d4e4ad359fb8dccea7f92e17d1b1f95 -> ea278094e9937bf96157161941d38a0c08a0aa4e ([splitSystemFs_0](https://github.com/TheCharlatan/bitcoin/commits/splitSystemFs_0) -> [splitSystemFs_1](https://github.com/TheCharlatan/bitcoin/commits/splitSystemFs_1), [compare](https://github.com/TheCharlatan/bitcoin/compare/splitSystemFs_0..splitSystemFs_1)) to address review comments.
💬 willcl-ark commented on pull request "wallet: GetEffectiveBalance":
(https://github.com/bitcoin/bitcoin/pull/26365#issuecomment-1467991659)
Going to close this as there are other, cleaner and better-maintained version(s) of it being worked on now.
willcl-ark closed a pull request: "wallet: GetEffectiveBalance"
(https://github.com/bitcoin/bitcoin/pull/26365)
💬 virtu commented on pull request "mempool: Add mempool tracepoints":
(https://github.com/bitcoin/bitcoin/pull/26531#discussion_r1135459251)
Last time I checked event names could not be reused, so the package rejection code would have to use a new event.

On the bright side, having separate rejection events might help differentiating between them and avoid conflating regular vs. package-based rejections.
📝 darosior opened a pull request: "MiniTapscript: port Miniscript to Tapscript"
(https://github.com/bitcoin/bitcoin/pull/27255)
Miniscript was targeting P2WSH, and as such can currently only be used in `wsh()` descriptors. This pull request introduces support for Tapscript in Miniscript and makes Miniscript available inside `tr()` descriptors. It adds support for both watching *and* signing TapMiniscript descriptors.

The main changes to Miniscript for Tapscript are the following:
- A new `multi_a` fragment is introduced with the same semantics as `multi`. Like in other descriptors `multi` and `multi_a` can exclusivel
...
💬 glozow commented on pull request "mempool: Add mempool tracepoints":
(https://github.com/bitcoin/bitcoin/pull/26531#discussion_r1135469419)
Mm good to know, thanks!
💬 michaelfolkson commented on pull request "MiniTapscript: port Miniscript to Tapscript":
(https://github.com/bitcoin/bitcoin/pull/27255#issuecomment-1468032095)
Concept ACK, an important piece in opening up Taproot scripting.

> The design of Miniscript for Tapscript is the result of discussions between various people over the past year(s).

Your PR description is great but perhaps you can link to a few of those discussions for extra background reading for those that are interested? e.g. https://gist.github.com/sipa/06c5c844df155d4e5044c2c8cac9c05e

Also just checking if this PR should be in Draft or not? Are there ongoing open discussions on anyt
...
💬 ryanofsky commented on pull request "refactor: Extract util/fs from util/system":
(https://github.com/bitcoin/bitcoin/pull/27254#issuecomment-1468037295)
Agree these functions should not be combined into existing `src/fs.h` since they are doing higher level things than accessing the filesystem. But instead of having two `fs.h` files in different locations, I would move `src/fs.h` to `src/util/fs.h` and move these functions to `src/util/fs_misc.h` or another place like that.
💬 kristapsk commented on pull request "Add feerate histogram to getmempoolinfo":
(https://github.com/bitcoin/bitcoin/pull/21422#issuecomment-1468044559)
@0xB10C In general I agree it's better to keep Core simple if things can be done with some simple workarounds using existing APIs, but I feel this is something that could make some use in Qt GUI too, and for that reason it makes sense to implement inside Core codebase.

> IIRC there was also an out-of-band push to review this PR as "rumor has it that a well-known wallet is currently using Bitcoin Knots solely because this feature isn't available in this RPC interface (https://github.com/bitcoi
...