📝 maflcko opened a pull request: " rpc: Fix race in loadtxoutset "
(https://github.com/bitcoin/bitcoin/pull/29262)
The tip may have advanced, also if it did not, there is no reason to
have two variables point to the same block.
Fixes https://github.com/bitcoin/bitcoin/pull/27596#discussion_r1344694600
(https://github.com/bitcoin/bitcoin/pull/29262)
The tip may have advanced, also if it did not, there is no reason to
have two variables point to the same block.
Fixes https://github.com/bitcoin/bitcoin/pull/27596#discussion_r1344694600
💬 real-or-random commented on pull request "Update libsecp256k1 subtree to current master":
(https://github.com/bitcoin/bitcoin/pull/29169#issuecomment-1896095271)
Hm, this is really not expected.
> Compiled with `gcc 10.2.1`.
You could see if gcc 10.5.0 makes a difference. AFAIU this is used in the official builds currently (until https://github.com/bitcoin/bitcoin/pull/27897 lands), and this was one of our considerations.
If that doesn't help, please benchmark libsecp256k1 with (https://github.com/bitcoin-core/secp256k1/commit/10e6d29b60c3931e327bc18e6c50cea78296b1ba) and without https://github.com/bitcoin-core/secp256k1/pull/1446 (https://githu
...
(https://github.com/bitcoin/bitcoin/pull/29169#issuecomment-1896095271)
Hm, this is really not expected.
> Compiled with `gcc 10.2.1`.
You could see if gcc 10.5.0 makes a difference. AFAIU this is used in the official builds currently (until https://github.com/bitcoin/bitcoin/pull/27897 lands), and this was one of our considerations.
If that doesn't help, please benchmark libsecp256k1 with (https://github.com/bitcoin-core/secp256k1/commit/10e6d29b60c3931e327bc18e6c50cea78296b1ba) and without https://github.com/bitcoin-core/secp256k1/pull/1446 (https://githu
...
💬 furszy commented on pull request "p2p: adaptive connections services flags":
(https://github.com/bitcoin/bitcoin/pull/28170#issuecomment-1896099957)
> > Hmm, good point. I would slightly rephrase, and extend, your first paragraph to state that the net limited buffer is connected to a user-customizable parameter, `-maxtipage`, with its default value is set to 24h.
>
> Yes, but `-maxtipage` is just an undocumented debug-only option meant for testing, nothing to be changed by actual users on mainnet.
For sure. Nothing to be changed. It was just good to mention the possible issue.
Still, on a slightly connected note; you would be surprise
...
(https://github.com/bitcoin/bitcoin/pull/28170#issuecomment-1896099957)
> > Hmm, good point. I would slightly rephrase, and extend, your first paragraph to state that the net limited buffer is connected to a user-customizable parameter, `-maxtipage`, with its default value is set to 24h.
>
> Yes, but `-maxtipage` is just an undocumented debug-only option meant for testing, nothing to be changed by actual users on mainnet.
For sure. Nothing to be changed. It was just good to mention the possible issue.
Still, on a slightly connected note; you would be surprise
...
💬 vasild commented on pull request "fuzz: extend ConsumeNetAddr() to return I2P and CJDNS addresses":
(https://github.com/bitcoin/bitcoin/pull/26859#issuecomment-1896105759)
This tests extension was stalled because it uncovered a bug. Now the bug is fixed via https://github.com/bitcoin/bitcoin/pull/27071. @dergoegge, @mzumsande, @jonatack, @chinggg - you did Concept ACK, maybe you want to review it?
(https://github.com/bitcoin/bitcoin/pull/26859#issuecomment-1896105759)
This tests extension was stalled because it uncovered a bug. Now the bug is fixed via https://github.com/bitcoin/bitcoin/pull/27071. @dergoegge, @mzumsande, @jonatack, @chinggg - you did Concept ACK, maybe you want to review it?
🚀 fanquake merged a pull request: "refactor: Allow std::span construction from CKey"
(https://github.com/bitcoin/bitcoin/pull/29133)
(https://github.com/bitcoin/bitcoin/pull/29133)
💬 tromp commented on issue "Witness scripts being abused to bypass datacarriersize limit (CVE-2023-50428)":
(https://github.com/bitcoin/bitcoin/issues/29187#issuecomment-1896118061)
> they put text (link) in many cases
Yes, some do. But many others don't, and those are the ones these measures are targetting.
(https://github.com/bitcoin/bitcoin/issues/29187#issuecomment-1896118061)
> they put text (link) in many cases
Yes, some do. But many others don't, and those are the ones these measures are targetting.
🚀 fanquake merged a pull request: "contrib: Update clang-format-diff"
(https://github.com/bitcoin/bitcoin/pull/29251)
(https://github.com/bitcoin/bitcoin/pull/29251)
💬 jamesob commented on pull request "rpc: Fix race in loadtxoutset":
(https://github.com/bitcoin/bitcoin/pull/29262#issuecomment-1896156373)
Approach ACK, thanks.
(https://github.com/bitcoin/bitcoin/pull/29262#issuecomment-1896156373)
Approach ACK, thanks.
💬 TheCharlatan commented on pull request "build: always set `-g -O2` in `CORE_CXXFLAGS`":
(https://github.com/bitcoin/bitcoin/pull/29205#issuecomment-1896171001)
Guix builds:
```
c65c40452ea0c2485c8a17a4d79f4befd2f3f5b66af5281a7833842990a236be guix-build-00c1e2aa4496/output/aarch64-linux-gnu/SHA256SUMS.part
c027a5f50f2b767a251953eded0cef07adf932f4905e5bd6cb1b19fd013a9f8d guix-build-00c1e2aa4496/output/aarch64-linux-gnu/bitcoin-00c1e2aa4496-aarch64-linux-gnu-debug.tar.gz
2baa2e82b8af6171b7861e6487fb3c1a07f0b0637e1518fb047d9f0773922781 guix-build-00c1e2aa4496/output/aarch64-linux-gnu/bitcoin-00c1e2aa4496-aarch64-linux-gnu.tar.gz
7452fa6211244d19607
...
(https://github.com/bitcoin/bitcoin/pull/29205#issuecomment-1896171001)
Guix builds:
```
c65c40452ea0c2485c8a17a4d79f4befd2f3f5b66af5281a7833842990a236be guix-build-00c1e2aa4496/output/aarch64-linux-gnu/SHA256SUMS.part
c027a5f50f2b767a251953eded0cef07adf932f4905e5bd6cb1b19fd013a9f8d guix-build-00c1e2aa4496/output/aarch64-linux-gnu/bitcoin-00c1e2aa4496-aarch64-linux-gnu-debug.tar.gz
2baa2e82b8af6171b7861e6487fb3c1a07f0b0637e1518fb047d9f0773922781 guix-build-00c1e2aa4496/output/aarch64-linux-gnu/bitcoin-00c1e2aa4496-aarch64-linux-gnu.tar.gz
7452fa6211244d19607
...
💬 TheCharlatan commented on pull request "kernel: Remove dependency on CScheduler":
(https://github.com/bitcoin/bitcoin/pull/28960#issuecomment-1896179668)
5755454fb5cc4067fc94e2682116e0fc5c9dfc58 -> 487b12e18ce007379a997da48b5ee97f459f6342 ([noGlobalSignals_2](https://github.com/TheCharlatan/bitcoin/tree/noGlobalSignals_2) -> [noGlobalSignals_3](https://github.com/TheCharlatan/bitcoin/tree/noGlobalSignals_3), [compare](https://github.com/TheCharlatan/bitcoin/compare/noGlobalSignals_2..noGlobalSignals_3))
* Addressed @maflcko's [comment](https://github.com/bitcoin/bitcoin/pull/28960#discussion_r1455548750), adding `Assert` to guard against early
...
(https://github.com/bitcoin/bitcoin/pull/28960#issuecomment-1896179668)
5755454fb5cc4067fc94e2682116e0fc5c9dfc58 -> 487b12e18ce007379a997da48b5ee97f459f6342 ([noGlobalSignals_2](https://github.com/TheCharlatan/bitcoin/tree/noGlobalSignals_2) -> [noGlobalSignals_3](https://github.com/TheCharlatan/bitcoin/tree/noGlobalSignals_3), [compare](https://github.com/TheCharlatan/bitcoin/compare/noGlobalSignals_2..noGlobalSignals_3))
* Addressed @maflcko's [comment](https://github.com/bitcoin/bitcoin/pull/28960#discussion_r1455548750), adding `Assert` to guard against early
...
👍 vasild approved a pull request: "fuzz: make FuzzedDataProvider usage deterministic"
(https://github.com/bitcoin/bitcoin/pull/29043#pullrequestreview-1827705262)
ACK 01960c53c7d71c70792abe19413315768dc2275a
I think it is worth improving this even if currently we only fuzz with clang and even if clang has not changed orders before or due to compilation flags - this may happen in the future.
A clang-tidy plugin to enforce this would be nice but I do not see its absence as a blocker for this PR.
(https://github.com/bitcoin/bitcoin/pull/29043#pullrequestreview-1827705262)
ACK 01960c53c7d71c70792abe19413315768dc2275a
I think it is worth improving this even if currently we only fuzz with clang and even if clang has not changed orders before or due to compilation flags - this may happen in the future.
A clang-tidy plugin to enforce this would be nice but I do not see its absence as a blocker for this PR.
💬 vasild commented on pull request "fuzz: make FuzzedDataProvider usage deterministic":
(https://github.com/bitcoin/bitcoin/pull/29043#discussion_r1456045072)
Unrelated to this PR, out of scope, but passing `max_pct` greater than `100` is pointless. `AddrManImpl::GetAddr_()` will behave as if `100` is passed. In addition to being useless and confusing this skews the input towards `100`. That is, it will treat `> 97%` of the possibilities as `100` (`(4097-100)/4097*100 = 97.6%`).
(https://github.com/bitcoin/bitcoin/pull/29043#discussion_r1456045072)
Unrelated to this PR, out of scope, but passing `max_pct` greater than `100` is pointless. `AddrManImpl::GetAddr_()` will behave as if `100` is passed. In addition to being useless and confusing this skews the input towards `100`. That is, it will treat `> 97%` of the possibilities as `100` (`(4097-100)/4097*100 = 97.6%`).
💬 vasild commented on pull request "fuzz: make FuzzedDataProvider usage deterministic":
(https://github.com/bitcoin/bitcoin/pull/29043#discussion_r1456013304)
nit: all these variables can be `const`
(https://github.com/bitcoin/bitcoin/pull/29043#discussion_r1456013304)
nit: all these variables can be `const`
💬 vasild commented on pull request "fuzz: make FuzzedDataProvider usage deterministic":
(https://github.com/bitcoin/bitcoin/pull/29043#discussion_r1456051965)
ditto and even worse because the range here is `0..2^64-1`
(https://github.com/bitcoin/bitcoin/pull/29043#discussion_r1456051965)
ditto and even worse because the range here is `0..2^64-1`
💬 joeyvee1986 commented on issue "Witness scripts being abused to bypass datacarriersize limit (CVE-2023-50428)":
(https://github.com/bitcoin/bitcoin/issues/29187#issuecomment-1896257073)
I'm all for this. The spam and constant duplicates have been slowing my
network down tremendously.
On Wed, Jan 17, 2024, 10:08 AM John Tromp ***@***.***> wrote:
> they put text (link) in many cases
>
> Yes, some do. But many others don't, and those are the ones these measures
> are targetting.
>
> —
> Reply to this email directly, view it on GitHub
> <https://github.com/bitcoin/bitcoin/issues/29187#issuecomment-1896118061>,
> or unsubscribe
> <https://github.com/notifications/uns
...
(https://github.com/bitcoin/bitcoin/issues/29187#issuecomment-1896257073)
I'm all for this. The spam and constant duplicates have been slowing my
network down tremendously.
On Wed, Jan 17, 2024, 10:08 AM John Tromp ***@***.***> wrote:
> they put text (link) in many cases
>
> Yes, some do. But many others don't, and those are the ones these measures
> are targetting.
>
> —
> Reply to this email directly, view it on GitHub
> <https://github.com/bitcoin/bitcoin/issues/29187#issuecomment-1896118061>,
> or unsubscribe
> <https://github.com/notifications/uns
...
👍 TheCharlatan approved a pull request: "test: Remove all-lint.py script"
(https://github.com/bitcoin/bitcoin/pull/29228#pullrequestreview-1827843026)
ACK fa2b95cf3f5148d27a8fd4fb3763ca1fc139bdd9
(https://github.com/bitcoin/bitcoin/pull/29228#pullrequestreview-1827843026)
ACK fa2b95cf3f5148d27a8fd4fb3763ca1fc139bdd9
💬 pablomartin4btc commented on pull request "assumeutxo, rpc: Improve EOF error when reading snapshot metadata in loadtxoutset":
(https://github.com/bitcoin/bitcoin/pull/28670#issuecomment-1896372688)
Updates:
- Switch the logic strategy to the previously presented alternative due to preference of reviewers @kevkevinpal & @furszy (thanks!);
- Added test coverage
(https://github.com/bitcoin/bitcoin/pull/28670#issuecomment-1896372688)
Updates:
- Switch the logic strategy to the previously presented alternative due to preference of reviewers @kevkevinpal & @furszy (thanks!);
- Added test coverage
💬 totient commented on issue "Witness scripts being abused to bypass datacarriersize limit (CVE-2023-50428)":
(https://github.com/bitcoin/bitcoin/issues/29187#issuecomment-1896400042)
> No matter what new restrictions are imposed, it's safe to assume that the economic reality of people wanting to inscribe data on the blockchain will persist and use whatever means is available and cheapest. There will always be means available such as spreading the data over multiple outputs or multiple txs, encoding the data as arbitrary taproot scripts, or encoding the data as fake public keys or key hashes. Some of these encodings will be detectable while others will not. Raising the cost o
...
(https://github.com/bitcoin/bitcoin/issues/29187#issuecomment-1896400042)
> No matter what new restrictions are imposed, it's safe to assume that the economic reality of people wanting to inscribe data on the blockchain will persist and use whatever means is available and cheapest. There will always be means available such as spreading the data over multiple outputs or multiple txs, encoding the data as arbitrary taproot scripts, or encoding the data as fake public keys or key hashes. Some of these encodings will be detectable while others will not. Raising the cost o
...
🤔 pablomartin4btc reviewed a pull request: "rpc: Fix race in loadtxoutset"
(https://github.com/bitcoin/bitcoin/pull/29262#pullrequestreview-1827979729)
ACK 5555d8db3313f893609eb0cf549bb597361d4466
(https://github.com/bitcoin/bitcoin/pull/29262#pullrequestreview-1827979729)
ACK 5555d8db3313f893609eb0cf549bb597361d4466
✅ theuni closed a pull request: "Replace non-standard CLZ builtins with c++20's bit_width"
(https://github.com/bitcoin/bitcoin/pull/29057)
(https://github.com/bitcoin/bitcoin/pull/29057)