Bitcoin Core Github
43 subscribers
123K links
Download Telegram
💬 vasild commented on pull request "fuzz: Make FuzzedSock fuzz friendlier":
(https://github.com/bitcoin/bitcoin/pull/30211#discussion_r1636524185)
> ... will lose the trailing `N - M` bytes from the peek.

Fixed in https://github.com/bitcoin/bitcoin/pull/30273
💬 glozow commented on pull request "policy: bump TX_MAX_STANDARD_VERSION to 3":
(https://github.com/bitcoin/bitcoin/pull/29496#discussion_r1636524772)
done in #30272
💬 maflcko commented on pull request "doc: Mention EOL policy in release notes template":
(https://github.com/bitcoin/bitcoin/pull/30271#issuecomment-2163091600)
Feel free to close if this feels too redundant, or if someone from the security team wanted to create a similar pull. (I am not on the security team)
💬 glozow commented on pull request "policy: bump TX_MAX_STANDARD_VERSION to 3":
(https://github.com/bitcoin/bitcoin/pull/29496#discussion_r1636524914)
done in #30272
💬 vasild commented on pull request "fuzz: Make FuzzedSock fuzz friendlier":
(https://github.com/bitcoin/bitcoin/pull/30211#discussion_r1636524913)
Removed the optional in https://github.com/bitcoin/bitcoin/pull/30273.
👍 vasild approved a pull request: "fuzz: add I2P harness"
(https://github.com/bitcoin/bitcoin/pull/30230#pullrequestreview-2113132000)
ACK 193c748e44f8647a056121fc9cbb9c2efbcbfc49
💬 vasild commented on pull request "fuzz: add I2P harness":
(https://github.com/bitcoin/bitcoin/pull/30230#discussion_r1636531569)
> I think it's an acceptable tradeoff ...

Ok.

> ... advancing time would be a more accurate simulation. Maybe we leave this as a potential follow up ...

Right, further changes in `FuzzedSock` itself are kind of out of scope for this PR which resurrects the previously deleted test.

Thanks!
💬 sipa commented on pull request "Update minisketch subtree to eb37a9b8e79f9e49d73b96a49bf97a96d9eb676c":
(https://github.com/bitcoin/bitcoin/pull/30270#issuecomment-2163112473)
utACK 89464ad59cf11f68315ea3104236989e5b429d15
maflcko closed a pull request: "doc: Mention EOL policy in release notes template"
(https://github.com/bitcoin/bitcoin/pull/30271)
💬 maflcko commented on pull request "doc: Mention EOL policy in release notes template":
(https://github.com/bitcoin/bitcoin/pull/30271#issuecomment-2163134086)
Closing for now as up-for-grabs.
⚠️ okabri opened an issue: "Btc"
(https://github.com/bitcoin/bitcoin/issues/30274)
👍 ryanofsky approved a pull request: "Encapsulate warnings in generalized node::Warnings and remove globals"
(https://github.com/bitcoin/bitcoin/pull/30058#pullrequestreview-2113194341)
Code review ACK 4ca48b328ba601299fc5aa4efc9749420590a022. Just rebase and a change to internal commits (removing DoWarning earlier) since last review.
fanquake closed an issue: "Btc"
(https://github.com/bitcoin/bitcoin/issues/30274)
:lock: fanquake locked an issue: "Btc"
(https://github.com/bitcoin/bitcoin/issues/30274)
👍 ryanofsky approved a pull request: "p2p: For assumeutxo, download snapshot chain before background chain"
(https://github.com/bitcoin/bitcoin/pull/29519#pullrequestreview-2113216701)
Code review ACK ac547ea43beb1d66d2c11e7e8abb76dd1bfc2883. This is an improvement over the status quo in the normal case, though it seems like it introduces some odd behavior if snapshot block is not on the most work chain, as described in other comments. I think it would be nice to clean the code up more and clarify what is trying to do, instead of merging this change as it is. But it would also be ok to merge in its current form, if you prefer that.
🤔 BrandonOdiwuor reviewed a pull request: "assumeutxo: Check snapshot base block is not in invalid chain"
(https://github.com/bitcoin/bitcoin/pull/30267#pullrequestreview-2113239131)
Code Review ACK cbc604f6a7739bf22a5d811b6366453860d59f4c
💬 BrandonOdiwuor commented on pull request "assumeutxo: Check snapshot base block is not in invalid chain":
(https://github.com/bitcoin/bitcoin/pull/30267#discussion_r1636599996)
Nit: Would it be better to include the base block hash on the error message to help in debugging

```cpp
throw JSONRPCError(
RPC_INTERNAL_ERROR,
"The base block header (%s) is part of an invalid chain.", base_blockhash.ToString());
```
👍 ryanofsky approved a pull request: "Introduce Mining interface"
(https://github.com/bitcoin/bitcoin/pull/30200#pullrequestreview-2113260997)
Code review ACK 4f9c3369a453f7e5f0c12179dd629ee8fc042379. Only changes since last review are: restoring ApplyArgsManOptions function and inconclusive-not-best-prevblk check order, splitting up an internal commit, and improving some commit messages.
💬 maflcko commented on pull request "build: Bump clang minimum supported version to 16":
(https://github.com/bitcoin/bitcoin/pull/30263#issuecomment-2163209582)
Not sure what is wrong with Apple, but I don't have any insight into their XCode/Apple-Clang versioning and source code, so I think I'll just clarify that this needs Apple-Clang 16 (the meaning of which is unclear).
💬 maflcko commented on pull request "build: Bump clang minimum supported version to 16":
(https://github.com/bitcoin/bitcoin/pull/30263#issuecomment-2163230498)
Ref:

```
/Applications/Xcode_15.0.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/usr/include/c++/v1/map:1282:24: note: in instantiation of function template specialization 'std::__tree<std::__value_type<std::string_view, FuzzTarget>, std::__map_value_compare<std::string_view, std::__value_type<std::string_view, FuzzTarget>, std::less<std::string_view>>, std::allocator<std::__value_type<std::string_view, FuzzTarget>>>::__emplace_unique_key_args<std::string_view, co
...