💬 maflcko commented on pull request "test: p2p: check that connecting to ourself leads to disconnect":
(https://github.com/bitcoin/bitcoin/pull/30362#issuecomment-2199328998)
ACK 5d2fb14bafe4e80c0a482d99e5ebde07c477f000

💬 maflcko commented on pull request "scripted-diff: Use LogInfo/LogDebug over LogPrintf/LogPrint":
(https://github.com/bitcoin/bitcoin/pull/29641#issuecomment-2199403598)
> I suggest [#29256 (comment)](https://github.com/bitcoin/bitcoin/pull/29256#issuecomment-2197461775) instead.
>
> It may also make sense to finish consensus and work on the simplest possible consistent user-facing API and developer API before doing a mass migration.

I don't think the changes here conceptually conflict with any open pull request. There are discussions around the naming and functionality around `LogWarning` and `LogError`, but I don't think they are changed or touched in th
...

🤔 glozow reviewed a pull request: "optimization: Switch CTxMemPool::CalculateDescendants from set to vector to reduce transaction hash calculations"
(https://github.com/bitcoin/bitcoin/pull/30325#pullrequestreview-2150632999)
Seems like a reasonable thing to do, but this conflicts with / is already done in #28676. See the "Select transactions for blocks based on chunk feerate" commit there which gets rid of this call.

I think our current policy is to avoid making refactoring changes that would need to be redone or removed in cluster mempool.

✅ maflcko closed an issue: "Add "maxuploadtargettimeframe" to change the timeframe considered by "maxuploadtarget""
(https://github.com/bitcoin/bitcoin/issues/30176)

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