💬 l0rinc commented on pull request "http: replace WorkQueue and single threads handling for ThreadPool":
(https://github.com/bitcoin/bitcoin/pull/33689#discussion_r2477491505)
nit: for consistency
```suggestion
for (size_t i{1}; i <= num_tasks; ++i) {
```
(https://github.com/bitcoin/bitcoin/pull/33689#discussion_r2477491505)
nit: for consistency
```suggestion
for (size_t i{1}; i <= num_tasks; ++i) {
```
💬 l0rinc commented on pull request "http: replace WorkQueue and single threads handling for ThreadPool":
(https://github.com/bitcoin/bitcoin/pull/33689#discussion_r2477455551)
do we really need this extra complexity here?
(https://github.com/bitcoin/bitcoin/pull/33689#discussion_r2477455551)
do we really need this extra complexity here?
💬 l0rinc commented on pull request "refactor: Return uint64_t from GetSerializeSize":
(https://github.com/bitcoin/bitcoin/pull/33724#discussion_r2478779605)
These two are used in consensus code as well, that's why I mentioned them. And they're part of some template metaprogramming magic which should likely be aligned here. Will leave it up to you of course.
(https://github.com/bitcoin/bitcoin/pull/33724#discussion_r2478779605)
These two are used in consensus code as well, that's why I mentioned them. And they're part of some template metaprogramming magic which should likely be aligned here. Will leave it up to you of course.
🚀 fanquake merged a pull request: "test: add valid tx test with minimum-sized ECDSA signature (8 bytes DER-encoded)"
(https://github.com/bitcoin/bitcoin/pull/32924)
(https://github.com/bitcoin/bitcoin/pull/32924)
💬 00w1 commented on issue "Remove *petertodd.net DNS seed for testnet and mainnet":
(https://github.com/bitcoin/bitcoin/issues/33736#issuecomment-3468946970)
<img width="948" height="289" alt="Image" src="https://github.com/user-attachments/assets/080212eb-ada9-496e-9c85-d87a9306e2e5" />
(https://github.com/bitcoin/bitcoin/issues/33736#issuecomment-3468946970)
<img width="948" height="289" alt="Image" src="https://github.com/user-attachments/assets/080212eb-ada9-496e-9c85-d87a9306e2e5" />
🚀 fanquake merged a pull request: "test: Use same rpc timeout for authproxy and cli"
(https://github.com/bitcoin/bitcoin/pull/33698)
(https://github.com/bitcoin/bitcoin/pull/33698)
💬 dergoegge commented on issue "Remove *petertodd.net DNS seed for testnet and mainnet":
(https://github.com/bitcoin/bitcoin/issues/33736#issuecomment-3468958855)
> floppy profile pic in screenshot
<img width="272" height="186" alt="Image" src="https://github.com/user-attachments/assets/88349a98-b1b5-432d-a2af-9a21cb5372ce" />
(https://github.com/bitcoin/bitcoin/issues/33736#issuecomment-3468958855)
> floppy profile pic in screenshot
<img width="272" height="186" alt="Image" src="https://github.com/user-attachments/assets/88349a98-b1b5-432d-a2af-9a21cb5372ce" />
💬 l0rinc commented on pull request "refactor: Return uint64_t from GetSerializeSize":
(https://github.com/bitcoin/bitcoin/pull/33724#discussion_r2478798725)
Sure, but the smallest change here that would fix the bug would be to upcast the `4` multiplier to 64 bits as mentioned in https://github.com/bitcoin/bitcoin/pull/33724#discussion_r2474414992.
I don't mind doing some combination of partial migration and not just the smallest possible diff, but I mentioned the outliers, will let you decide which ones are relevant and which ones to do separately. I don't particularly care about the order.
(https://github.com/bitcoin/bitcoin/pull/33724#discussion_r2478798725)
Sure, but the smallest change here that would fix the bug would be to upcast the `4` multiplier to 64 bits as mentioned in https://github.com/bitcoin/bitcoin/pull/33724#discussion_r2474414992.
I don't mind doing some combination of partial migration and not just the smallest possible diff, but I mentioned the outliers, will let you decide which ones are relevant and which ones to do separately. I don't particularly care about the order.
💬 davidgumberg commented on pull request "p2p: Drop unsolicited CMPCTBLOCK from non-HB peer":
(https://github.com/bitcoin/bitcoin/pull/32606#issuecomment-3468964102)
> > the second peer can endlessly send CMPCTBLOCK's
>
> If we're concerned about that, seems better to fix it directly?
>
> ```diff
> --- a/src/net_processing.cpp
> +++ b/src/net_processing.cpp
> @@ -4412,6 +4412,11 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type,
> while (range_flight.first != range_flight.second) {
> if (range_flight.first->second.first == pfrom.GetId()) {
> requested_block_from_this_peer = tr
...
(https://github.com/bitcoin/bitcoin/pull/32606#issuecomment-3468964102)
> > the second peer can endlessly send CMPCTBLOCK's
>
> If we're concerned about that, seems better to fix it directly?
>
> ```diff
> --- a/src/net_processing.cpp
> +++ b/src/net_processing.cpp
> @@ -4412,6 +4412,11 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type,
> while (range_flight.first != range_flight.second) {
> if (range_flight.first->second.first == pfrom.GetId()) {
> requested_block_from_this_peer = tr
...
💬 l0rinc commented on pull request "refactor: Return uint64_t from GetSerializeSize":
(https://github.com/bitcoin/bitcoin/pull/33724#discussion_r2478801132)
https://github.com/bitcoin/bitcoin/pull/33724#discussion_r2476885390 explains it well, I can get behind both as long as we're consistent
(https://github.com/bitcoin/bitcoin/pull/33724#discussion_r2478801132)
https://github.com/bitcoin/bitcoin/pull/33724#discussion_r2476885390 explains it well, I can get behind both as long as we're consistent
💬 00w1 commented on issue "Remove *petertodd.net DNS seed for testnet and mainnet":
(https://github.com/bitcoin/bitcoin/issues/33736#issuecomment-3468967896)
> > floppy profile pic in screenshot
>
> <img alt="Image" width="272" height="186" src="https://private-user-images.githubusercontent.com/8077169/507760105-88349a98-b1b5-432d-a2af-9a21cb5372ce.png?jwt=eyJ0eXAiOiJKV1QiLCJhbGciOiJIUzI1NiJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3NjE4NDI1NDQsIm5iZiI6MTc2MTg0MjI0NCwicGF0aCI6Ii84MDc3MTY5LzUwNzc2MDEwNS04ODM0OWE5OC1iMWI1LTQzMmQtYTJhZi05YTIxY2I1MzcyY2UucG5nP1gtQW16LUFsZ29yaXRobT1BV1M0LUhNQUMtU
...
(https://github.com/bitcoin/bitcoin/issues/33736#issuecomment-3468967896)
> > floppy profile pic in screenshot
>
> <img alt="Image" width="272" height="186" src="https://private-user-images.githubusercontent.com/8077169/507760105-88349a98-b1b5-432d-a2af-9a21cb5372ce.png?jwt=eyJ0eXAiOiJKV1QiLCJhbGciOiJIUzI1NiJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3NjE4NDI1NDQsIm5iZiI6MTc2MTg0MjI0NCwicGF0aCI6Ii84MDc3MTY5LzUwNzc2MDEwNS04ODM0OWE5OC1iMWI1LTQzMmQtYTJhZi05YTIxY2I1MzcyY2UucG5nP1gtQW16LUFsZ29yaXRobT1BV1M0LUhNQUMtU
...
💬 maflcko commented on pull request "refactor: Return uint64_t from GetSerializeSize":
(https://github.com/bitcoin/bitcoin/pull/33724#discussion_r2478816900)
> These two are used in consensus code as well,
No, they are not. Only one of them is used in psbt, which is not consensus.
One way to check if a function is used somewhere is to `=delete` it and compile.
The following diff compiles:
```diff
diff --git a/src/psbt.h b/src/psbt.h
index f0de079f7b..ec235ed2fc 100644
--- a/src/psbt.h
+++ b/src/psbt.h
@@ -212,7 +212,7 @@ void DeserializeMuSig2ParticipantPubkeys(Stream& s, SpanReader& skey, std::map<C
std::vector<unsigned char>
...
(https://github.com/bitcoin/bitcoin/pull/33724#discussion_r2478816900)
> These two are used in consensus code as well,
No, they are not. Only one of them is used in psbt, which is not consensus.
One way to check if a function is used somewhere is to `=delete` it and compile.
The following diff compiles:
```diff
diff --git a/src/psbt.h b/src/psbt.h
index f0de079f7b..ec235ed2fc 100644
--- a/src/psbt.h
+++ b/src/psbt.h
@@ -212,7 +212,7 @@ void DeserializeMuSig2ParticipantPubkeys(Stream& s, SpanReader& skey, std::map<C
std::vector<unsigned char>
...
💬 maflcko commented on pull request "ci: Add missing python3-dev package for riscv64":
(https://github.com/bitcoin/bitcoin/pull/33746#issuecomment-3468999171)
(force pushed for Alpine)
(https://github.com/bitcoin/bitcoin/pull/33746#issuecomment-3468999171)
(force pushed for Alpine)
💬 maflcko commented on pull request "refactor: Return uint64_t from GetSerializeSize":
(https://github.com/bitcoin/bitcoin/pull/33724#discussion_r2478845769)
Dropped the `blockstorage.cpp` changes
(https://github.com/bitcoin/bitcoin/pull/33724#discussion_r2478845769)
Dropped the `blockstorage.cpp` changes
✅ glozow closed an issue: "Remove *petertodd.net DNS seed for testnet and mainnet"
(https://github.com/bitcoin/bitcoin/issues/33736)
(https://github.com/bitcoin/bitcoin/issues/33736)
💬 mzumsande commented on pull request "fuzz: Add fuzz target for block index tree and related validation events":
(https://github.com/bitcoin/bitcoin/pull/31533#discussion_r2478861359)
fixed
(https://github.com/bitcoin/bitcoin/pull/31533#discussion_r2478861359)
fixed
💬 mzumsande commented on pull request "fuzz: Add fuzz target for block index tree and related validation events":
(https://github.com/bitcoin/bitcoin/pull/31533#discussion_r2478862391)
done, thanks
(https://github.com/bitcoin/bitcoin/pull/31533#discussion_r2478862391)
done, thanks
💬 mzumsande commented on pull request "fuzz: Add fuzz target for block index tree and related validation events":
(https://github.com/bitcoin/bitcoin/pull/31533#issuecomment-3469069615)
[ab8bc7a](https://github.com/bitcoin/bitcoin/commit/ab8bc7a1f877ad9e80a112dae6a12ff6ddbce076) to [48d1151](https://github.com/bitcoin/bitcoin/commit/48d11516da6351358080493fa95ea6252ce08525):
Rebased, addressed remaining feedback, including resetting `m_cached_finished_ibd` reset to cleanup. Moved out of draft.
(https://github.com/bitcoin/bitcoin/pull/31533#issuecomment-3469069615)
[ab8bc7a](https://github.com/bitcoin/bitcoin/commit/ab8bc7a1f877ad9e80a112dae6a12ff6ddbce076) to [48d1151](https://github.com/bitcoin/bitcoin/commit/48d11516da6351358080493fa95ea6252ce08525):
Rebased, addressed remaining feedback, including resetting `m_cached_finished_ibd` reset to cleanup. Moved out of draft.
👋 mzumsande's pull request is ready for review: "fuzz: Add fuzz target for block index tree and related validation events"
(https://github.com/bitcoin/bitcoin/pull/31533)
(https://github.com/bitcoin/bitcoin/pull/31533)
💬 maflcko commented on pull request "fuzz: Add fuzz target for block index tree and related validation events":
(https://github.com/bitcoin/bitcoin/pull/31533#discussion_r2478881137)
from the llm:
ReceivedBlockTransaction -> ReceivedBlockTransactions [the comment refers to the ReceivedBlockTransactions function; singular "ReceivedBlockTransaction" is incorrect and may confuse readers]
making no assumptions what must -> making no assumptions about what must [missing preposition "about" makes the phrase ungrammatical and harder to parse]
(https://github.com/bitcoin/bitcoin/pull/31533#discussion_r2478881137)
from the llm:
ReceivedBlockTransaction -> ReceivedBlockTransactions [the comment refers to the ReceivedBlockTransactions function; singular "ReceivedBlockTransaction" is incorrect and may confuse readers]
making no assumptions what must -> making no assumptions about what must [missing preposition "about" makes the phrase ungrammatical and harder to parse]