💬 paplorinc commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1660680256)
The flgas seem like an implementation detail to me which we might not want to expose (we could have stored it in two booleans as well and the outside behavior shouldn't change), so usages such as:
```C++
flags = it->second.GetFlags();
```
could become something like:
```C++
setDirty(it->second.IsDirty());
setFresh(it->second.IsFresh());
```
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1660680256)
The flgas seem like an implementation detail to me which we might not want to expose (we could have stored it in two booleans as well and the outside behavior shouldn't change), so usages such as:
```C++
flags = it->second.GetFlags();
```
could become something like:
```C++
setDirty(it->second.IsDirty());
setFresh(it->second.IsFresh());
```
💬 Sjors commented on pull request "refactor: add coinbase constraints to BlockAssembler::Options":
(https://github.com/bitcoin/bitcoin/pull/30356#discussion_r1660681030)
This probably needs to be `DEFAULT_BLOCK_MAX_WEIGHT / 4`, let me check the spec as well...
(https://github.com/bitcoin/bitcoin/pull/30356#discussion_r1660681030)
This probably needs to be `DEFAULT_BLOCK_MAX_WEIGHT / 4`, let me check the spec as well...
💬 paplorinc commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1660684697)
I'll leave it up to you of course, but the instances I found would be simpler as well:
```C++
BOOST_CHECK_EQUAL(n1.second.GetFlags(), CCoinsCacheEntry::DIRTY);
```
would become
```C++
BOOST_CHECK(n1.second.IsDirty());
```
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1660684697)
I'll leave it up to you of course, but the instances I found would be simpler as well:
```C++
BOOST_CHECK_EQUAL(n1.second.GetFlags(), CCoinsCacheEntry::DIRTY);
```
would become
```C++
BOOST_CHECK(n1.second.IsDirty());
```
📝 anonployed opened a pull request: "Update README: Enhance Formatting and Clarity"
(https://github.com/bitcoin/bitcoin/pull/30366)
This PR updates the README file to improve its formatting and clarity. The changes made are aimed at enhancing both the user and developer experience by making the documentation more accessible and easier to understand.
- **Improved Link Formatting**: Updated the links to remove visible `https://`, ensuring a cleaner and more professional appearance.
- **Corrected Minor Typos**: Fixed minor typographical errors to improve readability.
- **Enhanced Readability**: Made small adjustments to t
...
(https://github.com/bitcoin/bitcoin/pull/30366)
This PR updates the README file to improve its formatting and clarity. The changes made are aimed at enhancing both the user and developer experience by making the documentation more accessible and easier to understand.
- **Improved Link Formatting**: Updated the links to remove visible `https://`, ensuring a cleaner and more professional appearance.
- **Corrected Minor Typos**: Fixed minor typographical errors to improve readability.
- **Enhanced Readability**: Made small adjustments to t
...
💬 Sjors commented on pull request "refactor: add coinbase constraints to BlockAssembler::Options":
(https://github.com/bitcoin/bitcoin/pull/30356#issuecomment-2199576083)
@TheCharlatan happy to put the constants somewhere else... see https://github.com/Sjors/bitcoin/pull/47 for an attempt at introducing `libbitcoin_net`, which might reduce the dependencies of a future `libbitcoin_sv2` to `libbitcoin_net`, `libbitcoin_crypto` and `libbitcoin_util`. I got stuck there on where to put `XOnlyPubKey` and a few other things.
A similar problem might exist for `CBlockTemplate` if I actually tried to make `libbitcoin_sv2`, which I haven't done yet.
(https://github.com/bitcoin/bitcoin/pull/30356#issuecomment-2199576083)
@TheCharlatan happy to put the constants somewhere else... see https://github.com/Sjors/bitcoin/pull/47 for an attempt at introducing `libbitcoin_net`, which might reduce the dependencies of a future `libbitcoin_sv2` to `libbitcoin_net`, `libbitcoin_crypto` and `libbitcoin_util`. I got stuck there on where to put `XOnlyPubKey` and a few other things.
A similar problem might exist for `CBlockTemplate` if I actually tried to make `libbitcoin_sv2`, which I haven't done yet.
💬 Sjors commented on pull request "refactor: add coinbase constraints to BlockAssembler::Options":
(https://github.com/bitcoin/bitcoin/pull/30356#discussion_r1660691098)
See also: https://github.com/stratum-mining/sv2-spec/blob/main/07-Template-Distribution-Protocol.md#71-coinbaseoutputdatasize-client---server
(https://github.com/bitcoin/bitcoin/pull/30356#discussion_r1660691098)
See also: https://github.com/stratum-mining/sv2-spec/blob/main/07-Template-Distribution-Protocol.md#71-coinbaseoutputdatasize-client---server
💬 paplorinc commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1660694276)
ok, thanks for checking
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1660694276)
ok, thanks for checking
💬 paplorinc commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1660700180)
as mentioned before, after https://github.com/bitcoin/bitcoin/pull/28280/commits/cf5bfda650bd57636c7dd13f0ca03910d2aebe2e we can simplify these asserts
```C++
BOOST_CHECK(n2.second.IsFresh());
```
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1660700180)
as mentioned before, after https://github.com/bitcoin/bitcoin/pull/28280/commits/cf5bfda650bd57636c7dd13f0ca03910d2aebe2e we can simplify these asserts
```C++
BOOST_CHECK(n2.second.IsFresh());
```
👍 brunoerg approved a pull request: "test: Added coverage to Block not found error using gettxoutsetinfo"
(https://github.com/bitcoin/bitcoin/pull/30340#pullrequestreview-2150831199)
crACK 8ec24bdad89e2a72c394060ba5661a91f374b874
(https://github.com/bitcoin/bitcoin/pull/30340#pullrequestreview-2150831199)
crACK 8ec24bdad89e2a72c394060ba5661a91f374b874
✅ fanquake closed a pull request: "Update README: Enhance Formatting and Clarity"
(https://github.com/bitcoin/bitcoin/pull/30366)
(https://github.com/bitcoin/bitcoin/pull/30366)
✅ willcl-ark closed an issue: "build: `libbitcoin_util` unexpected(?)/undocumented dependencies"
(https://github.com/bitcoin/bitcoin/issues/28548)
(https://github.com/bitcoin/bitcoin/issues/28548)
💬 willcl-ark commented on issue "build: `libbitcoin_util` unexpected(?)/undocumented dependencies":
(https://github.com/bitcoin/bitcoin/issues/28548#issuecomment-2199628181)
Seems that we can close this now that #29015 has been merged.
Let me know if this is a mistake and this shouldn't be closed until `libbitcoin_util` doesn't depend on `libbitcoin_crypto`.
(https://github.com/bitcoin/bitcoin/issues/28548#issuecomment-2199628181)
Seems that we can close this now that #29015 has been merged.
Let me know if this is a mistake and this shouldn't be closed until `libbitcoin_util` doesn't depend on `libbitcoin_crypto`.
💬 dergoegge commented on pull request "fuzz: FuzzedSock::Recv() don't lose bytes from MSG_PEEK read":
(https://github.com/bitcoin/bitcoin/pull/30273#issuecomment-2199646499)
CI failure looks unrelated to me
(https://github.com/bitcoin/bitcoin/pull/30273#issuecomment-2199646499)
CI failure looks unrelated to me
💬 Fi3 commented on pull request "Stratum v2 Noise Protocol":
(https://github.com/bitcoin/bitcoin/pull/29346#issuecomment-2199647556)
@itornaza If I remember correctly anything needed to implement sv2 noise is contained in the [sv2 spec](https://github.com/stratum-mining/sv2-spec/blob/main/04-Protocol-Security.md) and the [noise spec](https://noiseprotocol.org/noise.html) are a little bit dispersive. But I implemented it on SRI more then one year ago so take it with a grain of salt.
(https://github.com/bitcoin/bitcoin/pull/29346#issuecomment-2199647556)
@itornaza If I remember correctly anything needed to implement sv2 noise is contained in the [sv2 spec](https://github.com/stratum-mining/sv2-spec/blob/main/04-Protocol-Security.md) and the [noise spec](https://noiseprotocol.org/noise.html) are a little bit dispersive. But I implemented it on SRI more then one year ago so take it with a grain of salt.
💬 paplorinc commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1660708747)
nit:
```suggestion
if (!it->second.coin.IsSpent()) {
/* BatchWrite must unset all unspent entries when will_erase=false. */
throw std::logic_error("Not all unspent flagged entries were unset");
}
const auto next_entry{it->second.Next(/*clear_flags=*/true)};
cachedCoinsUsage -= it->second.coin.DynamicMemoryUsage();
cacheCoins.erase(it->first);
```
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1660708747)
nit:
```suggestion
if (!it->second.coin.IsSpent()) {
/* BatchWrite must unset all unspent entries when will_erase=false. */
throw std::logic_error("Not all unspent flagged entries were unset");
}
const auto next_entry{it->second.Next(/*clear_flags=*/true)};
cachedCoinsUsage -= it->second.coin.DynamicMemoryUsage();
cacheCoins.erase(it->first);
```
💬 paplorinc commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1660730018)
nit:
untested, but modern compilers will likely [decide the need for inlining themselves
](https://en.cppreference.com/w/cpp/language/inline#:~:text=Since%20inline%20substitution)
> Since inline substitution is unobservable in the standard semantics, compilers are free to use inline substitution for any function that's not marked inline, and are free to generate function calls to any function marked inline.
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1660730018)
nit:
untested, but modern compilers will likely [decide the need for inlining themselves
](https://en.cppreference.com/w/cpp/language/inline#:~:text=Since%20inline%20substitution)
> Since inline substitution is unobservable in the standard semantics, compilers are free to use inline substitution for any function that's not marked inline, and are free to generate function calls to any function marked inline.
💬 paplorinc commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#issuecomment-2199661794)
utACK 670084adc53e3d661ea5b4a19743659609a42d5e, will run the benchmarks in the following weeks, thanks for addressing the suggestions.
(https://github.com/bitcoin/bitcoin/pull/28280#issuecomment-2199661794)
utACK 670084adc53e3d661ea5b4a19743659609a42d5e, will run the benchmarks in the following weeks, thanks for addressing the suggestions.
👍 brunoerg approved a pull request: "fuzz: FuzzedSock::Recv() don't lose bytes from MSG_PEEK read"
(https://github.com/bitcoin/bitcoin/pull/30273#pullrequestreview-2150885622)
utACK 4d81b4de339efbbb68c9785203b699e6e12ecd83
(https://github.com/bitcoin/bitcoin/pull/30273#pullrequestreview-2150885622)
utACK 4d81b4de339efbbb68c9785203b699e6e12ecd83
💬 pablomartin4btc commented on pull request "cli: improve error message on multiwallet and add validation to cli-side commands":
(https://github.com/bitcoin/bitcoin/pull/26990#discussion_r1660756564)
Ok, thanks, will do.
(https://github.com/bitcoin/bitcoin/pull/26990#discussion_r1660756564)
Ok, thanks, will do.
🚀 fanquake merged a pull request: "test: p2p: check that connecting to ourself leads to disconnect"
(https://github.com/bitcoin/bitcoin/pull/30362)
(https://github.com/bitcoin/bitcoin/pull/30362)