💬 desi-bitcoiner commented on pull request "datacarriersize: Match more datacarrying":
(https://github.com/bitcoin/bitcoin/pull/28408#issuecomment-1878334784)
I hope the following information helps in adding clarity to the thought process of folks who advocate otherwise:
"Satoshi made only 1 mistake, which I believe is the root of the confusion among some Bitcoiners who defend spam.
He used the same name bitcoin - to define two different elements: the asset, and the network.
Although they are interdependent, they possess very distinct properties.
The network is decentralized, permission-less, and resistant to censorship - not the asset.
...
(https://github.com/bitcoin/bitcoin/pull/28408#issuecomment-1878334784)
I hope the following information helps in adding clarity to the thought process of folks who advocate otherwise:
"Satoshi made only 1 mistake, which I believe is the root of the confusion among some Bitcoiners who defend spam.
He used the same name bitcoin - to define two different elements: the asset, and the network.
Although they are interdependent, they possess very distinct properties.
The network is decentralized, permission-less, and resistant to censorship - not the asset.
...
💬 maflcko commented on issue "ubsan: misaligned-pointer-use in crc32c/src/crc32c_arm64.cc":
(https://github.com/bitcoin/bitcoin/issues/29178#issuecomment-1878395056)
```
# git bisect bad
228d6a2969e4fcee573c9df7aad31550eab9c8d4 is the first bad commit
commit 228d6a2969e4fcee573c9df7aad31550eab9c8d4
Author: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com>
Date: Mon Nov 20 13:37:44 2023 +0000
build: Fix regression in "ARMv8 CRC32 intrinsics" test
The `vmull_p64` is a part of the Crypto extensions from the ACLE. They
are optional extensions, so they get enabled with a `+crypto` for
architecture flags.
configur
...
(https://github.com/bitcoin/bitcoin/issues/29178#issuecomment-1878395056)
```
# git bisect bad
228d6a2969e4fcee573c9df7aad31550eab9c8d4 is the first bad commit
commit 228d6a2969e4fcee573c9df7aad31550eab9c8d4
Author: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com>
Date: Mon Nov 20 13:37:44 2023 +0000
build: Fix regression in "ARMv8 CRC32 intrinsics" test
The `vmull_p64` is a part of the Crypto extensions from the ACLE. They
are optional extensions, so they get enabled with a `+crypto` for
architecture flags.
configur
...
💬 fanquake commented on pull request "refactor: Make CWalletTx sync state type-safe":
(https://github.com/bitcoin/bitcoin/pull/21206#discussion_r1442693589)
This is part of #29042.
(https://github.com/bitcoin/bitcoin/pull/21206#discussion_r1442693589)
This is part of #29042.
💬 brunoerg commented on pull request "p2p: Allow whitelisting outgoing connections":
(https://github.com/bitcoin/bitcoin/pull/27114#discussion_r1442715945)
I think we can avoid passing them in `Initialize` by setting them in `AddWhitelistPermissionFlags`.
(https://github.com/bitcoin/bitcoin/pull/27114#discussion_r1442715945)
I think we can avoid passing them in `Initialize` by setting them in `AddWhitelistPermissionFlags`.
👍 TheCharlatan approved a pull request: "rpc: Remove deprecated -rpcserialversion"
(https://github.com/bitcoin/bitcoin/pull/28890#pullrequestreview-1805703512)
lgtm ACK fa46cc22bc696e6845915ae91d6b68e36bf4c242
> If there is a use-case, likely a per-RPC flag can be added, if needed.
(https://github.com/bitcoin/bitcoin/pull/28890#pullrequestreview-1805703512)
lgtm ACK fa46cc22bc696e6845915ae91d6b68e36bf4c242
> If there is a use-case, likely a per-RPC flag can be added, if needed.
💬 fanquake commented on pull request "util: remove Boost posix_time usage from GetTime*":
(https://github.com/bitcoin/bitcoin/pull/21110#issuecomment-1878453838)
> As std::chrono::parse is available now,
I'm not sure if that is true? See discussion in https://github.com/bitcoin/bitcoin/pull/29081#issuecomment-1857683898. Happy for you to submit a PR in any case.
(https://github.com/bitcoin/bitcoin/pull/21110#issuecomment-1878453838)
> As std::chrono::parse is available now,
I'm not sure if that is true? See discussion in https://github.com/bitcoin/bitcoin/pull/29081#issuecomment-1857683898. Happy for you to submit a PR in any case.
🚀 fanquake merged a pull request: "rpc: Remove deprecated -rpcserialversion"
(https://github.com/bitcoin/bitcoin/pull/28890)
(https://github.com/bitcoin/bitcoin/pull/28890)
💬 maflcko commented on pull request "rpc,rest,zmq: faster getblock, NotifyBlock and rest_block by reading raw block":
(https://github.com/bitcoin/bitcoin/pull/26415#discussion_r1442732727)
rpc ser-flags were removed
(https://github.com/bitcoin/bitcoin/pull/26415#discussion_r1442732727)
rpc ser-flags were removed
👍 fanquake approved a pull request: "build: Fix check whether `-latomic` needed"
(https://github.com/bitcoin/bitcoin/pull/29177#pullrequestreview-1805731949)
ACK f8ca1357c8205ceff732dcfb0d2bad79b40b611b
(https://github.com/bitcoin/bitcoin/pull/29177#pullrequestreview-1805731949)
ACK f8ca1357c8205ceff732dcfb0d2bad79b40b611b
🚀 fanquake merged a pull request: "build: Fix check whether `-latomic` needed"
(https://github.com/bitcoin/bitcoin/pull/29177)
(https://github.com/bitcoin/bitcoin/pull/29177)
💬 maflcko commented on pull request "build: Bump clang minimum supported version to 15":
(https://github.com/bitcoin/bitcoin/pull/29165#discussion_r1442736791)
Thanks, fixed.
(https://github.com/bitcoin/bitcoin/pull/29165#discussion_r1442736791)
Thanks, fixed.
💬 maflcko commented on pull request "build: Bump clang minimum supported version to 15":
(https://github.com/bitcoin/bitcoin/pull/29165#issuecomment-1878479956)
rebased
(https://github.com/bitcoin/bitcoin/pull/29165#issuecomment-1878479956)
rebased
💬 maflcko commented on issue "fuzz: Fix timeouts":
(https://github.com/bitcoin/bitcoin/issues/28812#issuecomment-1878490116)
So currently, I think, the remaining ones are:
* https://github.com/bitcoin/bitcoin/pull/28226#issuecomment-1837969022
* https://github.com/bitcoin-core/qa-assets/issues/158
* https://github.com/bitcoin/bitcoin/pull/28657#issuecomment-1874012428
* and possibly https://github.com/bitcoin/bitcoin/issues/28812#issuecomment-1855815358 ?
(https://github.com/bitcoin/bitcoin/issues/28812#issuecomment-1878490116)
So currently, I think, the remaining ones are:
* https://github.com/bitcoin/bitcoin/pull/28226#issuecomment-1837969022
* https://github.com/bitcoin-core/qa-assets/issues/158
* https://github.com/bitcoin/bitcoin/pull/28657#issuecomment-1874012428
* and possibly https://github.com/bitcoin/bitcoin/issues/28812#issuecomment-1855815358 ?
💬 fanquake commented on pull request "ci: Use DEBUG=1 in depends for MSAN jobs":
(https://github.com/bitcoin/bitcoin/pull/27495#issuecomment-1878505456)
Pushed again as oss-fuzz closed the equivalent issue as flaky/not-reproducible, however it definitely still happens:
```bash
2024-01-05T1Uninitialized bytes in strlen at offset 113 inside [0xe0900000a05c, 114)
==35245==WARNING: MemorySanitizer: use-of-uninitialized-value
#0 0xaaaad75d2440 in sqlite3Strlen30 /ci_container_base/depends/work/build/aarch64-unknown-linux-gnu/sqlite/3380500-6ac42047a53/sqlite3.c:32670:28
#1 0xaaaad75fd278 in mkFullPathname /ci_container_base/depends/work/
...
(https://github.com/bitcoin/bitcoin/pull/27495#issuecomment-1878505456)
Pushed again as oss-fuzz closed the equivalent issue as flaky/not-reproducible, however it definitely still happens:
```bash
2024-01-05T1Uninitialized bytes in strlen at offset 113 inside [0xe0900000a05c, 114)
==35245==WARNING: MemorySanitizer: use-of-uninitialized-value
#0 0xaaaad75d2440 in sqlite3Strlen30 /ci_container_base/depends/work/build/aarch64-unknown-linux-gnu/sqlite/3380500-6ac42047a53/sqlite3.c:32670:28
#1 0xaaaad75fd278 in mkFullPathname /ci_container_base/depends/work/
...
👍 hebasto approved a pull request: "build: Bump clang minimum supported version to 15"
(https://github.com/bitcoin/bitcoin/pull/29165#pullrequestreview-1805775946)
ACK fac4cf24588c281b12f9f64aa6f0a4dda1a645b7.
(https://github.com/bitcoin/bitcoin/pull/29165#pullrequestreview-1805775946)
ACK fac4cf24588c281b12f9f64aa6f0a4dda1a645b7.
💬 dergoegge commented on pull request "fuzz: set `nMaxOutboundLimit` in connman target":
(https://github.com/bitcoin/bitcoin/pull/29172#discussion_r1442763249)
Maybe call `CConnan::Init` instead of introducing this new test only setter?
(https://github.com/bitcoin/bitcoin/pull/29172#discussion_r1442763249)
Maybe call `CConnan::Init` instead of introducing this new test only setter?
💬 dergoegge commented on pull request "fuzz: set `nMaxOutboundLimit` in connman target":
(https://github.com/bitcoin/bitcoin/pull/29172#discussion_r1442764742)
```suggestion
const uint64_t max_outbound_limit{fuzzed_data_provider.ConsumeIntegral<uint64_t>()};
```
this will result in 0 some of the time.
(https://github.com/bitcoin/bitcoin/pull/29172#discussion_r1442764742)
```suggestion
const uint64_t max_outbound_limit{fuzzed_data_provider.ConsumeIntegral<uint64_t>()};
```
this will result in 0 some of the time.
💬 TheCharlatan commented on pull request "crypto: remove use of BUILD_BITCOIN_INTERNAL macro in sha256":
(https://github.com/bitcoin/bitcoin/pull/29180#issuecomment-1878520710)
Re comment https://github.com/bitcoin/bitcoin/pull/29180#pullrequestreview-1804912723
> Now it is gated with the DISABLE_OPTIMIZED_SHA256 one. Is it intentional?
That code seems to be unused currently if `BUILD_BITCOIN_INTERNAL` is defined, so this seems like an improvement to me.
(https://github.com/bitcoin/bitcoin/pull/29180#issuecomment-1878520710)
Re comment https://github.com/bitcoin/bitcoin/pull/29180#pullrequestreview-1804912723
> Now it is gated with the DISABLE_OPTIMIZED_SHA256 one. Is it intentional?
That code seems to be unused currently if `BUILD_BITCOIN_INTERNAL` is defined, so this seems like an improvement to me.
💬 theuni commented on pull request "crypto: remove use of BUILD_BITCOIN_INTERNAL macro in sha256":
(https://github.com/bitcoin/bitcoin/pull/29180#issuecomment-1878521297)
> compiles regardless of the BUILD_BITCOIN_INTERNAL macro. Now it is gated with the DISABLE_OPTIMIZED_SHA256 one. Is it intentional?
Thanks for pointing this out. Yes it was intentional as I believe this is currently wonky in master.
Note that libbitcoinconsensus is the only user of `LIBBITCOIN_CRYPTO_BASE`. And because it never calls `SHA256AutoDetect` it will never actually use `sha256_sse4::Transform`. It's therefore useless to include sse4 in `LIBBITCOIN_CRYPTO_BASE`.
I'll push anot
...
(https://github.com/bitcoin/bitcoin/pull/29180#issuecomment-1878521297)
> compiles regardless of the BUILD_BITCOIN_INTERNAL macro. Now it is gated with the DISABLE_OPTIMIZED_SHA256 one. Is it intentional?
Thanks for pointing this out. Yes it was intentional as I believe this is currently wonky in master.
Note that libbitcoinconsensus is the only user of `LIBBITCOIN_CRYPTO_BASE`. And because it never calls `SHA256AutoDetect` it will never actually use `sha256_sse4::Transform`. It's therefore useless to include sse4 in `LIBBITCOIN_CRYPTO_BASE`.
I'll push anot
...
💬 theuni commented on pull request "rpc: Remove deprecated -rpcserialversion":
(https://github.com/bitcoin/bitcoin/pull/28890#issuecomment-1878533182)
Post-merge utACK fa46cc22bc696e6845915ae91d6b68e36bf4c242. Nice cleanup :)
(https://github.com/bitcoin/bitcoin/pull/28890#issuecomment-1878533182)
Post-merge utACK fa46cc22bc696e6845915ae91d6b68e36bf4c242. Nice cleanup :)