💬 TheCharlatan commented on pull request "kernel: De-globalize validation caches":
(https://github.com/bitcoin/bitcoin/pull/30141#issuecomment-2175538120)
Rebased 5b1576bbc664a07e00b5528e13c1928df9bc9b0c -> 63923c8da686da42f522771f338ea8f2a4f4e568 ([noGlobalScriptCache_2](https://github.com/TheCharlatan/bitcoin/tree/noGlobalScriptCache_2) -> [noGlobalScriptCache_3](https://github.com/TheCharlatan/bitcoin/tree/noGlobalScriptCache_3), [compare](https://github.com/TheCharlatan/bitcoin/compare/noGlobalScriptCache_2..noGlobalScriptCache_3))
* Fixed conflict with #30058
(https://github.com/bitcoin/bitcoin/pull/30141#issuecomment-2175538120)
Rebased 5b1576bbc664a07e00b5528e13c1928df9bc9b0c -> 63923c8da686da42f522771f338ea8f2a4f4e568 ([noGlobalScriptCache_2](https://github.com/TheCharlatan/bitcoin/tree/noGlobalScriptCache_2) -> [noGlobalScriptCache_3](https://github.com/TheCharlatan/bitcoin/tree/noGlobalScriptCache_3), [compare](https://github.com/TheCharlatan/bitcoin/compare/noGlobalScriptCache_2..noGlobalScriptCache_3))
* Fixed conflict with #30058
💬 theStack commented on pull request "contrib: add tool to convert compact-serialized UTXO set to SQLite database":
(https://github.com/bitcoin/bitcoin/pull/27432#issuecomment-2175547170)
@Sjors: Thanks for testing. Interesting, IIUC it's not possible to compress the serialization of a not-on-the-curve-pubkey, so I strongly assume there must be a bug somewhere in the `decompress_pubkey` routine. Will investigate in a bit, and put the PR into draft state until this is fixed.
> Ideally we'd test this script against every UTXO ever created, but extracting those efficiently requires its own raw block file parsing script :-)
Agree that would be very nice.
(https://github.com/bitcoin/bitcoin/pull/27432#issuecomment-2175547170)
@Sjors: Thanks for testing. Interesting, IIUC it's not possible to compress the serialization of a not-on-the-curve-pubkey, so I strongly assume there must be a bug somewhere in the `decompress_pubkey` routine. Will investigate in a bit, and put the PR into draft state until this is fixed.
> Ideally we'd test this script against every UTXO ever created, but extracting those efficiently requires its own raw block file parsing script :-)
Agree that would be very nice.
📝 theStack converted_to_draft a pull request: "contrib: add tool to convert compact-serialized UTXO set to SQLite database"
(https://github.com/bitcoin/bitcoin/pull/27432)
## Problem description
There is demand from users to get the UTXO set in form of a SQLite database (#24628). Bitcoin Core currently only supports dumping the UTXO set in a binary _compact-serialized_ format, which was crafted specifically for AssumeUTXO snapshots (see PR #16899), with the primary goal of being as compact as possible. Previous PRs tried to extend the `dumptxoutset` RPC with new formats, either in human-readable form (e.g. #18689, #24202), or most recently, directly as SQLite d
...
(https://github.com/bitcoin/bitcoin/pull/27432)
## Problem description
There is demand from users to get the UTXO set in form of a SQLite database (#24628). Bitcoin Core currently only supports dumping the UTXO set in a binary _compact-serialized_ format, which was crafted specifically for AssumeUTXO snapshots (see PR #16899), with the primary goal of being as compact as possible. Previous PRs tried to extend the `dumptxoutset` RPC with new formats, either in human-readable form (e.g. #18689, #24202), or most recently, directly as SQLite d
...
💬 alfonsoromanz commented on pull request "test: add validation for gettxout RPC response":
(https://github.com/bitcoin/bitcoin/pull/30226#discussion_r1644090971)
Done. Thanks
(https://github.com/bitcoin/bitcoin/pull/30226#discussion_r1644090971)
Done. Thanks
💬 alfonsoromanz commented on pull request "test: add validation for gettxout RPC response":
(https://github.com/bitcoin/bitcoin/pull/30226#discussion_r1644093301)
Yeah, I agree. I was following the same log structure used by other tests in this file. I changed it to `"Validating gettxout RPC response"`. Let me know if that's better. Thanks
(https://github.com/bitcoin/bitcoin/pull/30226#discussion_r1644093301)
Yeah, I agree. I was following the same log structure used by other tests in this file. I changed it to `"Validating gettxout RPC response"`. Let me know if that's better. Thanks
💬 0xB10C commented on issue "Porting bcc tools to libbpf":
(https://github.com/bitcoin/bitcoin/issues/30298#issuecomment-2175564019)
I've been extensively using libbpf (& the libbpf-rs Rust crate) with the Bitcoin Core USDT tracepoints in https://github.com/0xB10C/peer-observer. I would be happy to see the tools being ported and would help to review a port. That being said, the tools (`contrinb/tracing/*`) could also be extracted to a `tracing-tools` repository to be maintained (and tested against Bitcoin Core) there.
More important than the tools is IMO an elegant way of getting rid of the bcc Python dependency, which ha
...
(https://github.com/bitcoin/bitcoin/issues/30298#issuecomment-2175564019)
I've been extensively using libbpf (& the libbpf-rs Rust crate) with the Bitcoin Core USDT tracepoints in https://github.com/0xB10C/peer-observer. I would be happy to see the tools being ported and would help to review a port. That being said, the tools (`contrinb/tracing/*`) could also be extracted to a `tracing-tools` repository to be maintained (and tested against Bitcoin Core) there.
More important than the tools is IMO an elegant way of getting rid of the bcc Python dependency, which ha
...
💬 Sjors commented on pull request "Support self-hosted Cirrus workers on forks":
(https://github.com/bitcoin/bitcoin/pull/29274#discussion_r1644119042)
It's indeed a custom variable like `NO_ARM`. I moved the explanation of `NO_BRANCH` up in the documentation. Leaving only the explanation of `skip` vs. `only_if` here, which I also reworded.
(https://github.com/bitcoin/bitcoin/pull/29274#discussion_r1644119042)
It's indeed a custom variable like `NO_ARM`. I moved the explanation of `NO_BRANCH` up in the documentation. Leaving only the explanation of `skip` vs. `only_if` here, which I also reworded.
💬 Sjors commented on pull request "Support self-hosted Cirrus workers on forks":
(https://github.com/bitcoin/bitcoin/pull/29274#discussion_r1644123429)
Updated.
It serves the same purpose as `NO_BRANCH` on Cirrus, expect that I couldn't find a way to make that behaviour opt-in on Github CI.
(https://github.com/bitcoin/bitcoin/pull/29274#discussion_r1644123429)
Updated.
It serves the same purpose as `NO_BRANCH` on Cirrus, expect that I couldn't find a way to make that behaviour opt-in on Github CI.
💬 Sjors commented on pull request "Support self-hosted Cirrus workers on forks":
(https://github.com/bitcoin/bitcoin/pull/29274#issuecomment-2175605000)
Rebased after #30193, which removes "noble" as a machine type. I previously already removed `NO_NOBLE`, see https://github.com/bitcoin/bitcoin/pull/29274#issuecomment-2134764402. The rebase merely impacts the documented list of persistent workers, which this PR moves.
Addressed review comments.
(https://github.com/bitcoin/bitcoin/pull/29274#issuecomment-2175605000)
Rebased after #30193, which removes "noble" as a machine type. I previously already removed `NO_NOBLE`, see https://github.com/bitcoin/bitcoin/pull/29274#issuecomment-2134764402. The rebase merely impacts the documented list of persistent workers, which this PR moves.
Addressed review comments.
💬 Sjors commented on pull request "Support self-hosted Cirrus workers on forks":
(https://github.com/bitcoin/bitcoin/pull/29274#discussion_r1644128922)
I moved 063ea4e0da9e1c06cc0f4aec97a03abac66ae59f to be right after 8259a13954878fd6188bb218c3b7320385516596 and expanded the commit message to explain why they don't use the same mechanism.
(https://github.com/bitcoin/bitcoin/pull/29274#discussion_r1644128922)
I moved 063ea4e0da9e1c06cc0f4aec97a03abac66ae59f to be right after 8259a13954878fd6188bb218c3b7320385516596 and expanded the commit message to explain why they don't use the same mechanism.
📝 m3dwards opened a pull request: "ci: remove unused bcc variable from workflow"
(https://github.com/bitcoin/bitcoin/pull/30299)
Fixes https://github.com/bitcoin/bitcoin/pull/30193#discussion_r1636804002
(https://github.com/bitcoin/bitcoin/pull/30299)
Fixes https://github.com/bitcoin/bitcoin/pull/30193#discussion_r1636804002
💬 m3dwards commented on pull request "ci: move ASan job to GitHub Actions from Cirrus CI":
(https://github.com/bitcoin/bitcoin/pull/30193#discussion_r1644131995)
https://github.com/bitcoin/bitcoin/pull/30299
(https://github.com/bitcoin/bitcoin/pull/30193#discussion_r1644131995)
https://github.com/bitcoin/bitcoin/pull/30299
💬 Sjors commented on pull request "Support self-hosted Cirrus workers on forks":
(https://github.com/bitcoin/bitcoin/pull/29274#discussion_r1644133891)
Improved the comment, see below.
(https://github.com/bitcoin/bitcoin/pull/29274#discussion_r1644133891)
Improved the comment, see below.
🚀 fanquake merged a pull request: "Revert "contrib: macdeploy: monkey-patch gen-sdk to be deterministic""
(https://github.com/bitcoin/bitcoin/pull/30282)
(https://github.com/bitcoin/bitcoin/pull/30282)
💬 maflcko commented on pull request "Support self-hosted Cirrus workers on forks":
(https://github.com/bitcoin/bitcoin/pull/29274#discussion_r1644151691)
Well, what problem is this trying to solve, given that it introduces new problems?
(https://github.com/bitcoin/bitcoin/pull/29274#discussion_r1644151691)
Well, what problem is this trying to solve, given that it introduces new problems?
💬 maflcko commented on pull request "Support self-hosted Cirrus workers on forks":
(https://github.com/bitcoin/bitcoin/pull/29274#discussion_r1644154081)
An alternative would be for forks to just supply an x86 machine. The CI system should detect this itself and invoke qemu-aarch64
(https://github.com/bitcoin/bitcoin/pull/29274#discussion_r1644154081)
An alternative would be for forks to just supply an x86 machine. The CI system should detect this itself and invoke qemu-aarch64
💬 Sjors commented on pull request "Support self-hosted Cirrus workers on forks":
(https://github.com/bitcoin/bitcoin/pull/29274#discussion_r1644156898)
Is that new behavior? Let me test that again...
(https://github.com/bitcoin/bitcoin/pull/29274#discussion_r1644156898)
Is that new behavior? Let me test that again...
💬 Sjors commented on pull request "Support self-hosted Cirrus workers on forks":
(https://github.com/bitcoin/bitcoin/pull/29274#discussion_r1644159505)
I guess I never tested this, because the documentation just says:
```
# - arm64: For an aarch64 machine, recommended to have 2 CPUs and 8 GB of memory.
```
If the performance difference isn't too terrible, then I can just drop the commit and mention this in the documentation.
(https://github.com/bitcoin/bitcoin/pull/29274#discussion_r1644159505)
I guess I never tested this, because the documentation just says:
```
# - arm64: For an aarch64 machine, recommended to have 2 CPUs and 8 GB of memory.
```
If the performance difference isn't too terrible, then I can just drop the commit and mention this in the documentation.
💬 maflcko commented on pull request "Support self-hosted Cirrus workers on forks":
(https://github.com/bitcoin/bitcoin/pull/29274#discussion_r1644160436)
You'll have to run something like `podman run --rm --privileged docker.io/multiarch/qemu-user-static --reset -p yes` and then register the type as `arm64`, but I'd say it should work.
(https://github.com/bitcoin/bitcoin/pull/29274#discussion_r1644160436)
You'll have to run something like `podman run --rm --privileged docker.io/multiarch/qemu-user-static --reset -p yes` and then register the type as `arm64`, but I'd say it should work.
💬 paplorinc commented on pull request "test: Validate oversized transactions or without inputs":
(https://github.com/bitcoin/bitcoin/pull/29862#discussion_r1644161127)
We only have 3 matching lines, so extracting them would result in slightly more complicated code.
If the reviewers insist, I don't mind doing it, see https://github.com/bitcoin/bitcoin/pull/29862#discussion_r1643367751
(https://github.com/bitcoin/bitcoin/pull/29862#discussion_r1644161127)
We only have 3 matching lines, so extracting them would result in slightly more complicated code.
If the reviewers insist, I don't mind doing it, see https://github.com/bitcoin/bitcoin/pull/29862#discussion_r1643367751