Bitcoin Core Github
42 subscribers
126K links
Download Telegram
👋 brunoerg's pull request is ready for review: "fuzz: wallet: add target for spkm migration"
(https://github.com/bitcoin/bitcoin/pull/29694)
💬 brunoerg commented on pull request "fuzz: wallet: add target for spkm migration":
(https://github.com/bitcoin/bitcoin/pull/29694#issuecomment-2511884253)
Force-pushed changing the fuzz target to address the `ApplyMigrationData` function. Now, after migrating the spk to descriptor, it creates some transactions and apply the migrated data.

Note that the CI failure is expected (see #31374):
```sh
[09:11:48.112] Run spkm_migration with args ['/ci_container_base/ci/scratch/build-x86_64-pc-linux-gnu/src/test/fuzz/fuzz', '-max_total_time=60']INFO: Running with entropic power schedule (0xFF, 100).
[09:11:48.112] INFO: Seed: 2346481497
[09:11:48.1
...
💬 hodlinator commented on pull request "util: Improve documentation and negation of args":
(https://github.com/bitcoin/bitcoin/pull/31212#discussion_r1866096570)
We are currently using the nullness of `m_config_path` to signal something different, initialized/not, although I think it should probably be more explicit. See https://github.com/bitcoin/bitcoin/pull/31212#issuecomment-2470510859, and this code:

https://github.com/bitcoin/bitcoin/blob/2ac863bdf974d7b4b4e8314876d02270400b6240/src/common/args.cpp#L761-L772

We could possibly do `optional<optional<path>> m_config_path` to flag both initialization and negation... but I'm not sure I want to go
...
📝 furszy opened a pull request: "[RFC] descriptors: inference process, do not return unparsable multisig descriptors"
(https://github.com/bitcoin/bitcoin/pull/31404)
---- Pushing it under RFC to gather some feedback ----

The internal script-to-descriptor inference procedure shouldn't return invalid
descriptors or ones that fail the string-to-descriptor parsing process. These
two procedures should always support seamless roundtrips.

This inlines `InferScript`/`InferDescriptor` to the actual descriptors
`ParseScript` logic for the multisig path, ensuring only valid descriptors are
produced.

Examples where this presents an issue:
1) Because the l
...
💬 edilmedeiros commented on issue "release: ship codesigned MacOS arm64 binaries":
(https://github.com/bitcoin/bitcoin/issues/29749#issuecomment-2511949059)
This is still an issue in v28. Would be great to at least include a readme file in the tarball stating that the binaries should be signed after download.
💬 sipa commented on pull request "[RFC] descriptors: inference process, do not return unparsable multisig descriptors":
(https://github.com/bitcoin/bitcoin/pull/31404#discussion_r1866127089)
Can you introduce a constant for this number 3 (if it doesn't exist already)?
💬 sipa commented on pull request "[RFC] descriptors: inference process, do not return unparsable multisig descriptors":
(https://github.com/bitcoin/bitcoin/pull/31404#discussion_r1866126191)
The limit is 520, not 512.
💬 willcl-ark commented on issue "release: ship codesigned MacOS arm64 binaries":
(https://github.com/bitcoin/bitcoin/issues/29749#issuecomment-2511980210)
@edilmedeiros notarization (and codesigning) is being actively worked on by @achow101. It's my understanding (and I could be wrong here) that, if we get the notarization working, we can apply it retroactively to other binaries which would resolve this issue in a preferable way.
💬 furszy commented on pull request "[RFC] descriptors: inference process, do not return unparsable multisig descriptors":
(https://github.com/bitcoin/bitcoin/pull/31404#discussion_r1866150428)
upps, fixed. Thanks
💬 furszy commented on pull request "[RFC] descriptors: inference process, do not return unparsable multisig descriptors":
(https://github.com/bitcoin/bitcoin/pull/31404#discussion_r1866152366)
Sure. Introduced it in the first commit.
danielabrozzoni closed an issue: "Unable to compile for test coverage on Nixos 24.05"
(https://github.com/bitcoin/bitcoin/issues/31087)
💬 hebasto commented on pull request "ci: Test cross-built Windows executables on Windows natively":
(https://github.com/bitcoin/bitcoin/pull/31176#issuecomment-2512046722)
Rebased.

@maflcko comments have been addressed.
💬 hebasto commented on pull request "ci: Test cross-built Windows executables on Windows natively":
(https://github.com/bitcoin/bitcoin/pull/31176#discussion_r1866162497)
Thanks! [Done](https://github.com/bitcoin/bitcoin/pull/31176#issuecomment-2512046722).
💬 hebasto commented on pull request "ci: Test cross-built Windows executables on Windows natively":
(https://github.com/bitcoin/bitcoin/pull/31176#discussion_r1866164093)
Thanks! [Done](https://github.com/bitcoin/bitcoin/pull/31176#issuecomment-2512046722).

However, a few functional tests fail...
💬 l0rinc commented on pull request "util: Improve documentation and negation of args":
(https://github.com/bitcoin/bitcoin/pull/31212#discussion_r1866191667)
optional of optional sounds like a `std::variant` - but I'm fine with doing these in a separate PR, of course
👍 ryanofsky approved a pull request: "test: enable running independent functional test sub-tests"
(https://github.com/bitcoin/bitcoin/pull/30991#pullrequestreview-2473279752)
Code review ACK 409d0d629378c3e23388ed31516376ad1ae536b5. This seems like a good step towards making it easy to run independent tests quickly. I think ideally there would be some naming convention or @ annotation added to test methods that can run independently, so the test framework could provide more functionality like being able to list test methods, being able to show command lines to quickly reproduce problems when tests fails, and calling test methods automatically instead of requiring ind
...
🚀 ryanofsky merged a pull request: "test: enable running independent functional test sub-tests"
(https://github.com/bitcoin/bitcoin/pull/30991)
👍 hebasto approved a pull request: "util: Drop boost posix_time in ParseISO8601DateTime"
(https://github.com/bitcoin/bitcoin/pull/31391#pullrequestreview-2473326256)
ACK faf70cc9941ce2b0ce4fd48ecfdbe28194adb8ba, I have reviewed the code and it looks OK.
💬 ryanofsky commented on pull request "Add multiprocess binaries to release build":
(https://github.com/bitcoin/bitcoin/pull/30975#issuecomment-2512207207)
> I disabled ASan + LSan + UBsan again, see previous failure log: https://github.com/bitcoin/bitcoin/actions/runs/11970857809/job/33374462331?pr=30975

I think https://github.com/chaincodelabs/libmultiprocess/pull/121 should fix this, but I want to test the fix a little more and it will require another depends bump like 90b405516f7f3be522ced3e0c4d23b3892df0661 to fix the problem in CI.

If there are more bugs feel free to post to https://github.com/chaincodelabs/libmultiprocess/issues/new or
...