Bitcoin Core Github
43 subscribers
122K links
Download Telegram
πŸ€” ajtowns reviewed a pull request: "refactor: Print verbose serialize compiler error messages"
(https://github.com/bitcoin/bitcoin/pull/29056#pullrequestreview-1783013964)
utACK fa45567e030272d34845e6b563a6b626b9eda739
πŸ’¬ ajtowns commented on pull request "refactor: Print verbose serialize compiler error messages":
(https://github.com/bitcoin/bitcoin/pull/29056#discussion_r1427470728)
Perhaps describe it as "Helper concept for basic byte types" ? "BasicByte" is a concept that applies to types, not a type itself? Could name the concept as `BasicByteType` since it applies to types?

Consider adding .clang-format fields:

```
BreakBeforeConceptDeclarations: Always
RequiresExpressionIndentation: OuterScope
```
πŸ’¬ ajtowns commented on pull request "refactor: Print verbose serialize compiler error messages":
(https://github.com/bitcoin/bitcoin/pull/29056#discussion_r1427481753)
Could consider:

```c++
template <class Stream>
concept WritableStream = requires(Stream& s, const Span<std::byte> src) { s.write(src); };

template <class T, class Stream>
concept Serializable = WritableStream<Stream> && requires(T a, Stream s) { a.Serialize(s); };

template <class Stream>
concept ReadableStream = requires(Stream& s, Span<std::byte> dst) { s.read(dst); };

template <class T, class Stream>
concept Unserializable = ReadableStream<Stream> && requires(T a, Stream s) {
...
πŸ’¬ DOD1000 commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1427576230)
Thank you for your interest, my dear friend. Can you explain further? Thank you
πŸ€” Mita23456 reviewed a pull request: "[26.x] Changes for rc3"
(https://github.com/bitcoin/bitcoin/pull/28872#pullrequestreview-1783218094)
So not doing at
πŸ€” Mita23456 reviewed a pull request: "[26.x] Changes for rc3"
(https://github.com/bitcoin/bitcoin/pull/28872#pullrequestreview-1783220557)
Hello
πŸ’¬ Mita23456 commented on pull request "[26.x] Changes for rc3":
(https://github.com/bitcoin/bitcoin/pull/28872#discussion_r1427615063)
Yep
πŸ’¬ Mita23456 commented on pull request "[26.x] Changes for rc3":
(https://github.com/bitcoin/bitcoin/pull/28872#discussion_r1427614919)
Ok
πŸ“ achow101 opened a pull request: "Update security.md contact for achow101"
(https://github.com/bitcoin/bitcoin/pull/29087)
πŸ’¬ sarthak13gupta commented on issue "Not able to build and compile on macos":
(https://github.com/bitcoin/bitcoin/issues/29083#issuecomment-1857419461)
> This is clearly a local issue / misconfiguration, not a problem with Core.

Changing the terminal config to running it without rosetta worked, Thanks Guys!
πŸ“ ajtowns opened a pull request: "tests: Don't depend on value of DEFAULT_PERMIT_BAREMULTISIG"
(https://github.com/bitcoin/bitcoin/pull/29088)
Update unit and functional tests so that they continue to work if the default for `-permitbaremultisig` is changed.
πŸ’¬ maflcko commented on pull request "tests: Don't depend on value of DEFAULT_PERMIT_BAREMULTISIG":
(https://github.com/bitcoin/bitcoin/pull/29088#issuecomment-1857486872)
```
elay_fee, std::string& reason);
| ^
test/script_p2sh_tests.cpp:29:43: error: argument name 'permit_baremultisig' in comment does not match parameter name 'permit_bare_multisig' [bugprone-argument-comment,-warnings-as-errors]
29 | IsStandardTx(tx, std::nullopt, /*permit_baremultisig=*/false, CFeeRate{DUST_RELAY_TX_FEE}, reason);
|
...
βœ… maflcko closed an issue: "Not able to build and compile on macos"
(https://github.com/bitcoin/bitcoin/issues/29083)
πŸ’¬ maflcko commented on issue "Not able to build and compile on macos":
(https://github.com/bitcoin/bitcoin/issues/29083#issuecomment-1857487522)
Good to hear. Closing for now.
πŸ’¬ maflcko commented on pull request "Update security.md contact for achow101":
(https://github.com/bitcoin/bitcoin/pull/29087#issuecomment-1857489114)
lgtm ACK e7d66509dc393400079eb885a497dd4e968eade6
πŸ’¬ maflcko commented on issue "Discussion: Upgrading to C++20":
(https://github.com/bitcoin/bitcoin/issues/23363#issuecomment-1857497397)
> Having `std::span<>` with the size as part of the type rather than a runtime thing could be nice for the various crypto things where we currently do things like `assert(key.size() == KEYLEN)`.

I guess this requires a helper method to safely convert to a fixed-size span, internally preferring a compile-time check, or falling back to an assert, if it isn't available?
πŸ’¬ maflcko commented on pull request "refactor: C++20: Use lambda implicit capture and std::rotl":
(https://github.com/bitcoin/bitcoin/pull/29085#issuecomment-1857506138)
Not sure. This needs a benchmark to check against slowdown, see https://github.com/bitcoin/bitcoin/pull/29057#issuecomment-1854487602

Also, C++20 is enabled for years, so I don't understand the other commit. I think you either forgot to compile locally, or your compiler doesn't emit any warnings that the changes are wrong?
πŸ’¬ maflcko commented on pull request "tests: Don't depend on value of DEFAULT_PERMIT_BAREMULTISIG":
(https://github.com/bitcoin/bitcoin/pull/29088#issuecomment-1857548697)
lgtm ACK 7b45744df33c6a4759eae1a3984f389cbac837c2
⚠️ ChrisMartl opened an issue: "Bitcoin's systemic flaw"
(https://github.com/bitcoin/bitcoin/issues/29089)
β€œBitcoin” doesn’t have a government (a coercion power), and thus must really on **market processes** i.e. on prices, which are determined by supply and demand. Consumer's wants and needs are signaled through prices, which are in turn satisfied by producers, who want to make a **profit**.

In this sense, it seems to be a β€œBitcoin” systemic flaw, that a valuable information like the β€œ**timechain**” is not provided to the consumer in exchange for Sats; i.e. no market, no price mechanism and thus no
...
πŸ’¬ brunoerg commented on pull request "fuzz: coinselection, improve `min_viable_change`/`change_output_size`":
(https://github.com/bitcoin/bitcoin/pull/28372#issuecomment-1857557432)
Force-pushed addressing https://github.com/bitcoin/bitcoin/pull/28372#issuecomment-1856559034 and https://github.com/bitcoin/bitcoin/pull/28372#discussion_r1427177813. Thanks @murchandamus and @furszy for the review!

Left it running for 10h before pushing and did not crash.