Bitcoin Core Github
44 subscribers
120K links
Download Telegram
💬 maflcko commented on pull request "test: fix intermittent failure in rpc_invalidateblock.py":
(https://github.com/bitcoin/bitcoin/pull/32968#issuecomment-3069863625)
lgtm ACK 28416f367a5d720d89214aa8ef94013caa1d1a40

Can be tested via by moving the lines after ` self.nodes[0].reconsiderblock(block.hash)` down again and observing a failure.
👋 ishaanam's pull request is ready for review: "wallet, rpc: add v3 transaction creation and wallet support"
(https://github.com/bitcoin/bitcoin/pull/32896)
💬 ishaanam commented on pull request "wallet, rpc: add v3 transaction creation and wallet support":
(https://github.com/bitcoin/bitcoin/pull/32896#issuecomment-3069883620)
This PR is now ready for review.
🤔 sipa reviewed a pull request: "cluster mempool: add TxGraph work controls"
(https://github.com/bitcoin/bitcoin/pull/32263#pullrequestreview-3016660633)
Addressed comments, and also made some further simplifications (dropped the returning of amount of work performed by `MakeAcceptable` and `MakeAllAcceptable` in the preparation commit, as the new `DoWork()` doesn't use them anymore but invokes `Cluster::Relinearize` directly).
💬 sipa commented on pull request "cluster mempool: add TxGraph work controls":
(https://github.com/bitcoin/bitcoin/pull/32263#discussion_r2205126924)
I have added some additional comments.
💬 sipa commented on pull request "cluster mempool: add TxGraph work controls":
(https://github.com/bitcoin/bitcoin/pull/32263#discussion_r2205127512)
Done.
💬 sipa commented on pull request "cluster mempool: add TxGraph work controls":
(https://github.com/bitcoin/bitcoin/pull/32263#discussion_r2205127983)
Added some comments to clarify.
maflcko closed a pull request: "rpc: Support v3 raw transactions creation"
(https://github.com/bitcoin/bitcoin/pull/31936)
💬 maflcko commented on pull request "rpc: Support v3 raw transactions creation":
(https://github.com/bitcoin/bitcoin/pull/31936#issuecomment-3069899126)
Closing for now, due to inactivity. The given feedback hasn't bee addressed over a long time and at this point it may be best to focus review on https://github.com/bitcoin/bitcoin/pull/32896.
💬 josibake commented on pull request "kernel: Introduce initial C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#issuecomment-3069964449)



> > am not sure how I am to interpret your NACK here.
>
> Just as "I think this PR should not be merged in its current form." I definitely do agree with the approach of adding a C API.

FWIW, I read this as "Concept ACK, Approach NACK" (per https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#conceptual-review), which I think is a helpful distinction.
💬 maflcko commented on pull request "refactor: cleanup index logging":
(https://github.com/bitcoin/bitcoin/pull/32948#issuecomment-3069982232)
review ACK c18bf0bd9be632e54591493a5309e9ed4020f2e5 🚣

<details><summary>Show signature</summary>

Signature:

```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: review ACK c18bf0bd9be6
...
💬 purpleKarrot commented on pull request "kernel: Introduce initial C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#issuecomment-3069994277)
Yes, Concept ACK, Approach NACK

Thanks, @josibake.
💬 TheCharlatan commented on pull request "kernel: Introduce initial C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#issuecomment-3070013951)
Updated 267a7b3f321304f75e8c47e380da49ba9c64bc84 -> 1ffc1c9d94b16cdbfb92a26d0f0e75451efad4fe ([kernelApi_44](https://github.com/TheCharlatan/bitcoin/tree/kernelApi_44) -> [kernelApi_45](https://github.com/TheCharlatan/bitcoin/tree/kernelApi_45), [compare](https://github.com/TheCharlatan/bitcoin/compare/kernelApi_44..kernelApi_45))

* Enforce better function names in the API, which should make future discussions on their desired end format a bit easier.
* Dropped the macro check for gcc 4.
🚀 fanquake merged a pull request: "refactor: cleanup index logging"
(https://github.com/bitcoin/bitcoin/pull/32948)
🤔 mzumsande reviewed a pull request: "test: fix intermittent failure in rpc_invalidateblock.py"
(https://github.com/bitcoin/bitcoin/pull/32968#pullrequestreview-3016812338)
Code Review ACK 28416f367a5d720d89214aa8ef94013caa1d1a40
💬 m3dwards commented on pull request "test: use notarized v28.2 binaries and fix macOS detection":
(https://github.com/bitcoin/bitcoin/pull/32922#issuecomment-3070026124)
ACK 4bb4c865999bfa0b0cb5aa806cf62dbf982fcfe9

Tested that the correct binaries were self signed on ARM Mac. Also checked the SHA256 sums for 28.2.
📝 crypomen9 opened a pull request: "chore: rename misspelling variable"
(https://github.com/bitcoin/bitcoin/pull/32971)
### Description

This pull request refactors the variable name from `shuffled_indeces` to `shuffled_indices` for improved code readability and consistency with standard naming conventions.

### Impact

- No changes to logic or functionality.
- No new tests are required, as this is a non-functional refactor.
- This change improves the developer experience by making the codebase easier to read and maintain.
💬 anhilde commented on issue "Bitcoin Core v29.0 incorrectly enters IBD mode when only ~600 blocks behind, preventing normal sync":
(https://github.com/bitcoin/bitcoin/issues/32955#issuecomment-3070086012)
ok, I will look out for that the next time. I am pretty sure the node was up a few hours before I noticed that the block number had not changed. What is the normal time until a node must have resumed syncing again? Is there a timeframe where I can be sure something is wrong?
💬 sipa commented on issue "Bitcoin Core v29.0 incorrectly enters IBD mode when only ~600 blocks behind, preventing normal sync":
(https://github.com/bitcoin/bitcoin/issues/32955#issuecomment-3070117257)
It should literally *always* be syncing. If it doesn't pick up an announced block immediately (within seconds, at the very longest), there is something wrong.
💬 1440000bytes commented on pull request "Reduce minrelaytxfee to 100 sats/kvB":
(https://github.com/bitcoin/bitcoin/pull/32959#issuecomment-3070190437)
Better to admit that USD is still the currency used for payments and there isn't any difference from 1 USD in 2015 and 2025.

Maybe Bitcoin(protocol) is dependent on USD(US government)?
🤔 Sjors reviewed a pull request: "descriptors: MuSig2"
(https://github.com/bitcoin/bitcoin/pull/31244#pullrequestreview-3016814938)
Reviewed from 4af0dca096ca497a6b4e5314c9edea683efe620e to a53924bee321f9d01d053cf562ee3d9493e00529.

In a53924bee321f9d01d053cf562ee3d9493e00529 _descriptor: Parse musig() key expressions_: it might be slightly easier to follow if you extract `& has_hardened`, the `ParsePubkey` signature change and `FromString` changes into their own commit(s).