Bitcoin Core Github
44 subscribers
121K links
Download Telegram
💬 jadijadi commented on pull request "Showing Local Addresses in Node Window":
(https://github.com/bitcoin-core/gui/pull/626#discussion_r1609379939)
changed to a normal loop and checking for .begin().
💬 fjahr commented on pull request "rpc: Optimize serialization and enhance metadata of dumptxoutset output":
(https://github.com/bitcoin/bitcoin/pull/29612#discussion_r1609402573)
Sorry, I must have overlooked that if you already mentioned it before when I changed the type. I will address it if I have to retouch.
🚀 fanquake merged a pull request: "Update libsecp256k1 subtree to current master"
(https://github.com/bitcoin/bitcoin/pull/30120)
💬 stratospher commented on pull request "test: improve robustness of connect_nodes()":
(https://github.com/bitcoin/bitcoin/pull/30118#issuecomment-2124125832)
reACK 6629d1d.
💬 naiyoma commented on pull request "cli: restrict multiple exclusive argument usage in bitcoin-cli":
(https://github.com/bitcoin/bitcoin/pull/30148#issuecomment-2124129963)

> This is very close to @Brotcrunsher's original commit. At the very least they should be a co-author.

I've added the co-author.
📝 maflcko opened a pull request: "doc: Correct pull request prefix for scripts and tools"
(https://github.com/bitcoin/bitcoin/pull/30150)
`script` is confusing, because in the context of Bitcoin, it usually means Bitcoin script (c.f. `CScript` in `script.h`, or pull requests such as https://github.com/bitcoin/bitcoin/pull/27122 using the prefix).

This could be fixed by renaming it to `scripts` (with a plural `s` at the end), however, looking at the current usage `contrib` and `cli` seem more common (https://github.com/bitcoin/bitcoin/pull/29687, https://github.com/bitcoin/bitcoin/pull/26953, https://github.com/bitcoin/bitcoin/p
...
💬 maflcko commented on pull request "script: Fix errors in verify-binaries/verify.py OS platform parsing, update test.py & docs":
(https://github.com/bitcoin/bitcoin/pull/30147#issuecomment-2124140010)
@BenWestgate I believe this is a typo in the documentation, fixed in https://github.com/bitcoin/bitcoin/pull/30150. (You can change the title yourself to `contrib:` and the bot will pick it up at some point.)
💬 jadijadi commented on pull request "Showing Local Addresses in Node Window":
(https://github.com/bitcoin-core/gui/pull/626#issuecomment-2124149228)
> This PR still have a couple of unaddressed comments: [#626 (comment)](https://github.com/bitcoin-core/gui/pull/626#discussion_r915334998) and [#626 (comment)](https://github.com/bitcoin-core/gui/pull/626#discussion_r1274077350).

Dear @hebasto , addressed all issues, updated the code & replied to queries. Thanks for the follow up.
👍 fanquake approved a pull request: "doc: Correct pull request prefix for scripts and tools"
(https://github.com/bitcoin/bitcoin/pull/30150#pullrequestreview-2070467640)
ACK fa3e1151a28345edff8f371283745bdd647f9a74
👍 fanquake approved a pull request: "ci: Add mising -Wno-error=maybe-uninitialized to armhf task"
(https://github.com/bitcoin/bitcoin/pull/30144#pullrequestreview-2070470661)
ACK fa73431dd4709754c34a4d5ad1c940ff9e628cf3 - many fixed with 13.x
💬 maflcko commented on pull request "contrib: Fixup verify-binaries OS platform parsing":
(https://github.com/bitcoin/bitcoin/pull/30147#issuecomment-2124165256)
Merge commits in pull requests are discouraged in this repo, please squash your commits according to https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#squashing-commits
🚀 fanquake merged a pull request: "ci: Add mising -Wno-error=maybe-uninitialized to armhf task"
(https://github.com/bitcoin/bitcoin/pull/30144)
💬 fanquake commented on pull request "build: LLD based macOS toolchain":
(https://github.com/bitcoin/bitcoin/pull/21778#discussion_r1609553954)
Clang actually expects this all the time, so just dsymutil is correct in depends as well (so we can't copy the bin as llvm-dsymutil) (this was the qt related issue I had seen before).
💬 fanquake commented on pull request "build: LLD based macOS toolchain":
(https://github.com/bitcoin/bitcoin/pull/21778#issuecomment-2124214837)
Guix build (aarch64):
```bash
00becde2dd12878e3b9f50f27899a6a8b752343dade7c71781632715c3001473 guix-build-e8c25e8a35e3/output/aarch64-linux-gnu/SHA256SUMS.part
a685b9cee54014e74639be1e8db2d55b7c008fdb3b31c1c708c364a49b56759a guix-build-e8c25e8a35e3/output/aarch64-linux-gnu/bitcoin-e8c25e8a35e3-aarch64-linux-gnu-debug.tar.gz
d61228158409802e5aef11c39a0da5653a6c7e870d5f500483c32c75f319e8b6 guix-build-e8c25e8a35e3/output/aarch64-linux-gnu/bitcoin-e8c25e8a35e3-aarch64-linux-gnu.tar.gz
49447a
...
💬 fanquake commented on pull request "lint/contrib/doc updates":
(https://github.com/bitcoin/bitcoin/pull/30084#discussion_r1609567748)
I agree, this change doesn't look like an improvement.
💬 maflcko commented on pull request "test: Assumeutxo: snapshots with less work should not be loaded":
(https://github.com/bitcoin/bitcoin/pull/29428#discussion_r1609570161)
style nit: The code should be self-explanatory, so could remove this comment? (This should also re-trigger the false positive failed CI)
💬 laanwj commented on pull request "net: Replace libnatpmp with built-in PCP implementation":
(https://github.com/bitcoin/bitcoin/pull/30043#issuecomment-2124254747)
> The only TODO that is left to do is the Qt settings migration

Avoiding this complexity is another reason to add a NAT-PMP fallback, then we'd be right to keep the option named the same and only change the descriptions.
💬 brunoerg commented on pull request "net: add ASMap info in `getrawaddrman` RPC":
(https://github.com/bitcoin/bitcoin/pull/30062#issuecomment-2124274889)
Force-pushed:

- Changed `mapped_as` and `source_mapped_as` to optional.
- Changed description of the fields as suggested by @fjahr.
- Reordered these new fields (https://github.com/bitcoin/bitcoin/pull/30062#discussion_r1607941908)
💬 brunoerg commented on pull request "net: add ASMap info in `getrawaddrman` RPC":
(https://github.com/bitcoin/bitcoin/pull/30062#discussion_r1609596900)
Done
👍 willcl-ark approved a pull request: "doc: Correct pull request prefix for scripts and tools"
(https://github.com/bitcoin/bitcoin/pull/30150#pullrequestreview-2070615158)
ACK fa3e1151a28345edff8f371283745bdd647f9a74

I'm half tempted to bikeshed this to using `tools`, but this also seems totally fine to me :)
💬 maflcko commented on pull request "doc: Correct pull request prefix for scripts and tools":
(https://github.com/bitcoin/bitcoin/pull/30150#issuecomment-2124282415)
> I'm half tempted to bikeshed this to using `tools`, but this also seems totally fine to me :)

The bot will understand it, so you are free to use it. :)