💬 maflcko commented on issue "Add "maxuploadtargettimeframe" to change the timeframe considered by "maxuploadtarget"":
(https://github.com/bitcoin/bitcoin/issues/30176#issuecomment-2199441498)
Closing for now, per previous comment.

🤔 glozow reviewed a pull request: "fuzz: Improve stability for txorphan and mini_miner harnesses"
(https://github.com/bitcoin/bitcoin/pull/30306#pullrequestreview-2150663280)
utACK e009bf681c0e38a6451afa594ba3c7c8861f61c3

Didn't check stability, but using txid seems fine.

💬 glozow commented on pull request "kernel: remove mempool_persist":
(https://github.com/bitcoin/bitcoin/pull/30344#issuecomment-2199482117)
lgtm, iiuc you'd want #30141 merged before this one?

💬 TheCharlatan commented on pull request "kernel: remove mempool_persist":
(https://github.com/bitcoin/bitcoin/pull/30344#issuecomment-2199500416)
Re https://github.com/bitcoin/bitcoin/pull/30344#issuecomment-2199482117

> lgtm, iiuc you'd want https://github.com/bitcoin/bitcoin/pull/30141 merged before this one?

Mmh, I don't think this was said anywhere? Looks like the two are not really related either.

✅ maflcko closed a pull request: "Correct Error Code in OP_IF/OP_NOTIF Empty Stack Check"
(https://github.com/bitcoin/bitcoin/pull/30359)

💬 maflcko commented on pull request "Correct Error Code in OP_IF/OP_NOTIF Empty Stack Check":
(https://github.com/bitcoin/bitcoin/pull/30359#issuecomment-2199513724)
Closing for now. At a minimum for changes like these, a unit test will have to be added. Also, the existing tests will need to pass. You'll have to run `make check && ./test/functional/test_runner.py` locally (before opening a pull request).

💬 willcl-ark commented on issue "Stuck in Endless Pre-Syncing Headers Loop":
(https://github.com/bitcoin/bitcoin/issues/26391#issuecomment-2199514099)
Do we want to keep this open to address [this](https://github.com/bitcoin/bitcoin/issues/26391#issuecomment-1292366671) comment?:

> Perhaps time is better spent on better reporting to the user, in the form of targetted warnings in logs (or even failure to start) when there appears to be a long invalid high-PoW chain out there.

Otherwise I think we can probably close this as stale.

💬 paplorinc commented on pull request "optimization: Moved repeated `-printpriority` fetching out of AddToBlock":
(https://github.com/bitcoin/bitcoin/pull/30324#discussion_r1660659790)
I didn't change the external option, just the internal name - as requested here https://github.com/bitcoin/bitcoin/pull/30324#discussion_r1652212085

✅ paplorinc closed a pull request: "optimization: Switch CTxMemPool::CalculateDescendants from set to vector to reduce transaction hash calculations"
(https://github.com/bitcoin/bitcoin/pull/30325)

💬 paplorinc commented on pull request "optimization: Switch CTxMemPool::CalculateDescendants from set to vector to reduce transaction hash calculations":
(https://github.com/bitcoin/bitcoin/pull/30325#issuecomment-2199548732)
Thanks for checking @glozow.
Closing, invalidated by https://github.com/bitcoin/bitcoin/pull/28676/files#diff-c065d4cd2398ad0dbcef393c5dfc53f465bf44723348892395fffd2fb3bac522R377

💬 Sjors commented on pull request "refactor: add coinbase constraints to BlockAssembler::Options":
(https://github.com/bitcoin/bitcoin/pull/30356#discussion_r1660679371)
I agree something like "block requestor" is more generic than "pool", but also more confusing.

> weight and bytes seem to be used interchangeably here and in the function definition

These bytes all go in the output which doesn't a witness discount, so each byte counts as 4 weight units. I think we should prefer the term "bytes", but make sure it's used correctly since we use weight units for internal accounting.

👍 dergoegge approved a pull request: "test: Add Compact Block Encoding test `ReceiveWithExtraTransactions` covering non-empty `extra_txn`"
(https://github.com/bitcoin/bitcoin/pull/30237#pullrequestreview-2150770839)
Code review ACK 55eea003af24169c883e1761beb997e151845225

💬 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());
```

💬 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...

💬 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());
```

📝 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
...

💬 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.

💬 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

💬 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

💬 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());
```

👍 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