π€ ajtowns reviewed a pull request: "refactor: Print verbose serialize compiler error messages"
(https://github.com/bitcoin/bitcoin/pull/29056#pullrequestreview-1783013964)
utACK fa45567e030272d34845e6b563a6b626b9eda739
(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
```
(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) {
...
(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
(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
(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
(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
(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
(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)
(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!
(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.
(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);
|
...
(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)
(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.
(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
(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?
(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?
(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
(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
...
(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.
(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.