Bitcoin Core Github
44 subscribers
121K links
Download Telegram
💬 maflcko commented on pull request "refactor: Allow std::span construction from CKey":
(https://github.com/bitcoin/bitcoin/pull/29133#issuecomment-1893861762)
I dropped the last commit, to not introduce new `std::span` usage for now. To verify the changes in this pull, the commit's diff can be used. (Fails to compile before, compiles after):

```diff
diff --git a/src/key.cpp b/src/key.cpp
index 2bd6396298..300c95e35a 100644
--- a/src/key.cpp
+++ b/src/key.cpp
@@ -385,7 +385,7 @@ bool CExtKey::Derive(CExtKey &out, unsigned int _nChild) const {
return key.Derive(out.key, out.chaincode, _nChild, chaincode);
}

-void CExtKey::SetSeed(Spa
...
💬 hebasto commented on pull request "ci, iwyu: Update mappings":
(https://github.com/bitcoin/bitcoin/pull/27710#issuecomment-1893868905)
https://github.com/bitcoin/bitcoin/pull/27710#discussion_r1452493222

> are you still working on this?

I don't think I will work on this PR in the nearest future.
hebasto closed a pull request: "ci, iwyu: Update mappings"
(https://github.com/bitcoin/bitcoin/pull/27710)
🤔 maflcko reviewed a pull request: "kernel: Remove dependency on CScheduler"
(https://github.com/bitcoin/bitcoin/pull/28960#pullrequestreview-1823257728)
Left some questions, feel free to ignore.
💬 maflcko commented on pull request "kernel: Remove dependency on CScheduler":
(https://github.com/bitcoin/bitcoin/pull/28960#discussion_r1453524696)
nit: Any reason to call it "main", when main.cpp has long been split up and renamed to validation.cpp? Could call it `validation_signals`, or leave it as is.
💬 maflcko commented on pull request "kernel: Remove dependency on CScheduler":
(https://github.com/bitcoin/bitcoin/pull/28960#discussion_r1453516899)
Wouldn't it be better to use existing `chainman` references to get the signals reference, instead of using unchecked pointer dereferences? (Same in all other places)
💬 maflcko commented on pull request "kernel: Remove dependency on CScheduler":
(https://github.com/bitcoin/bitcoin/pull/28960#discussion_r1453413538)
nit in the first commit: Could drop `signals` and use `active_chainstate.GetMainSignals()` instead?
💬 maflcko commented on pull request "kernel: Remove dependency on CScheduler":
(https://github.com/bitcoin/bitcoin/pull/28960#discussion_r1453405057)
first commit: Why check below, but not here, for nullptr?
💬 maflcko commented on pull request "kernel: Remove dependency on CScheduler":
(https://github.com/bitcoin/bitcoin/pull/28960#discussion_r1453382756)
in the first commit: Any reason to re-order this and remove the doc comment?
💬 vasild commented on pull request "ci: run test_bitcoin with DEBUG_LOG_OUT in RUN_UNIT_TESTS_SEQUENTIAL":
(https://github.com/bitcoin/bitcoin/pull/28736#issuecomment-1893896190)
`2b7888ec3e...57e8bc6b31`: rebase due to conflicts
💬 furszy commented on issue "Use of a wallet shouldn't be blocked in prune mode ("wallet loading failed... beyond pruned data")":
(https://github.com/bitcoin/bitcoin/issues/27188#issuecomment-1893908170)
@shovas, I have created #29183 to focus our efforts and delineate the working path in the clearest possible manner. It should help pushing the project forward.
💬 vasild commented on pull request "Avoid returning references to mutex guarded members":
(https://github.com/bitcoin/bitcoin/pull/28774#issuecomment-1893912633)
@achow101, this is now solely in the wallet. Maybe you want to take a look?
👍 TheCharlatan approved a pull request: "depends: add NM output to gen_id"
(https://github.com/bitcoin/bitcoin/pull/29249#pullrequestreview-1823792580)
ACK a1db512f069039530b9fb70f8c004a4f25c3e257

Guix build (x86)
```
580df4b97c2e045d93e98c2bf2fbccad05117aa54594ec8849969ae18dd0d865 guix-build-a1db512f0690/output/aarch64-linux-gnu/SHA256SUMS.part
d8ee47f067d42f087f1c2f01d0613d57c3f887eb2be746498b674b27671c2678 guix-build-a1db512f0690/output/aarch64-linux-gnu/bitcoin-a1db512f0690-aarch64-linux-gnu-debug.tar.gz
77655b1a8946a131c5bd0c5083d9faeae83d08d9d718ac43a79d1872c98a27e0 guix-build-a1db512f0690/output/aarch64-linux-gnu/bitcoin-a1db51
...
💬 fanquake commented on pull request "build: depends move macOS C(XX) FLAGS out of C & CXX":
(https://github.com/bitcoin/bitcoin/pull/29233#issuecomment-1893982810)
Guix Build (aarch64):
```bash
94d2fe95101baf9ba74e5c1be6a93708bd55b3700567c768e72ceb4af4951bb9 guix-build-cbc9bf11fe84/output/aarch64-linux-gnu/SHA256SUMS.part
3cacc8c3d726e78b1db8ea07c25186e589f32c991cd7fd45d4390135adf199f2 guix-build-cbc9bf11fe84/output/aarch64-linux-gnu/bitcoin-cbc9bf11fe84-aarch64-linux-gnu-debug.tar.gz
39f42920ea452ea67f0f8df3199c50d73660009368fccfab900204d27b8e50cc guix-build-cbc9bf11fe84/output/aarch64-linux-gnu/bitcoin-cbc9bf11fe84-aarch64-linux-gnu.tar.gz
dc1a2b
...
🚀 fanquake merged a pull request: "contrib: add macho branch protection check"
(https://github.com/bitcoin/bitcoin/pull/29170)
💬 TheCharlatan commented on pull request "kernel: Remove key module from kernel library":
(https://github.com/bitcoin/bitcoin/pull/29252#issuecomment-1894004822)
Updated 67058d4a82e222f770ede96dc948f758b0a8386c -> 2cb38d93a8fea16bf84b072a90ac023ef345134d ([kernelRmKey_0](https://github.com/TheCharlatan/bitcoin/tree/kernelRmKey_0) -> [kernelRmKey_1](https://github.com/TheCharlatan/bitcoin/tree/kernelRmKey_1), [compare](https://github.com/TheCharlatan/bitcoin/compare/kernelRmKey_0..kernelRmKey_1))

* Implemented @maflcko's [suggestion](https://github.com/bitcoin/bitcoin/pull/29252#issuecomment-1893335214), calling `ECC_Stop` instead of relaxing the `ECC_
...
💬 maflcko commented on pull request "kernel: Remove key module from kernel library":
(https://github.com/bitcoin/bitcoin/pull/29252#discussion_r1453627000)
Removing this will lead to compile errors when a `unique_ptr` member is added with a fwd decl, no?
💬 maflcko commented on pull request "kernel: Remove key module from kernel library":
(https://github.com/bitcoin/bitcoin/pull/29252#discussion_r1453627918)
need to remove include as well, no?
💬 TheCharlatan commented on pull request "kernel: Remove key module from kernel library":
(https://github.com/bitcoin/bitcoin/pull/29252#discussion_r1453636097)
Yes.
💬 Sjors commented on pull request "build: depends move macOS C(XX) FLAGS out of C & CXX":
(https://github.com/bitcoin/bitcoin/pull/29233#issuecomment-1894038057)
I was able to make and run a depends build on Intel macOS 14.2.1 for cbc9bf11fe84deb96daf9b97a8e7499979360db2.

I'm also baking a Guix build.