💬 TheCharlatan commented on pull request "kernel: Move block tree db open to block manager":
(https://github.com/bitcoin/bitcoin/pull/30965#discussion_r1909193453)
Good thinking ahead! Yeah, my idea was to use something along those lines too. Maybe we could start annotating some of the functions here to make it easier for the developer to see which exceptions it throws? Though my understanding is that such annotations have been deliberately left out of the code so far (https://stackoverflow.com/questions/1055387/throw-keyword-in-functions-signature).

On the other hand it would be nice if we could agree on more coherent error handling for constructors, b
...

💬 TheCharlatan commented on pull request "kernel: Move block tree db open to block manager":
(https://github.com/bitcoin/bitcoin/pull/30965#issuecomment-2580871894)
Updated 384c73d4fcda4af7c2b4bfb945a248cb93dba47d -> b776e40554c8a315d832f3996d14ff2e3ae7b8cb ([blockmanDB_6](https://github.com/TheCharlatan/bitcoin/tree/blockmanDB_6) -> [blockmanDB_7](https://github.com/TheCharlatan/bitcoin/tree/blockmanDB_7), [compare](https://github.com/TheCharlatan/bitcoin/compare/blockmanDB_6..blockmanDB_7))

* Addressed @ryanofsky's [comment](https://github.com/bitcoin/bitcoin/pull/30965#discussion_r1909124834), ran clang-format over commits.

💬 ismaelsadeeq commented on pull request "mining: bugfix: Fix duplicate coinbase tx weight reservation":
(https://github.com/bitcoin/bitcoin/pull/31384#discussion_r1909199419)
I did this to address https://github.com/bitcoin/bitcoin/pull/31384#discussion_r1903268301

Let me know if (`LARGE_TXS_COUNT` - 1) + `NORMAL_TXS_COUNT` is preferable.

💬 ismaelsadeeq commented on pull request "mining: bugfix: Fix duplicate coinbase tx weight reservation":
(https://github.com/bitcoin/bitcoin/pull/31384#discussion_r1909201069)
I've taken your suggestion @Sjors.

💬 ismaelsadeeq commented on pull request "mining: bugfix: Fix duplicate coinbase tx weight reservation":
(https://github.com/bitcoin/bitcoin/pull/31384#discussion_r1909201883)
I've taken this with a little modification.

👋 brunoerg's pull request is ready for review: "descriptor: check whitespace in pubkeys within fragments"
(https://github.com/bitcoin/bitcoin/pull/31603)

💬 brunoerg commented on pull request "descriptor: check whitespace in pubkeys within fragments":
(https://github.com/bitcoin/bitcoin/pull/31603#issuecomment-2580884607)
Ready for review!

Thanks, @furszy for the comment. I've changed the approched and described it on description.

💬 ismaelsadeeq commented on pull request "mining: bugfix: Fix duplicate coinbase tx weight reservation":
(https://github.com/bitcoin/bitcoin/pull/31384#discussion_r1909208594)
> This changes the meaning of `getmininginfo` fields unexpectedly, and forces us to add in the coinbase reserved size over and over. Seems like it would be better to include it here once.

> We could either adjust the value in the RPC code, or as @luke-jr suggests restore the original behavior here.

I've reverted and included it but clarify the `getmininginfo` help text.

> I agree we should not change the `weight`, `currentblockweight` and `sigops` fields in `getblocktemplate` and `getmi
...

💬 ismaelsadeeq commented on pull request "mining: bugfix: Fix duplicate coinbase tx weight reservation":
(https://github.com/bitcoin/bitcoin/pull/31384#discussion_r1909208803)
Done.

💬 ismaelsadeeq commented on pull request "mining: bugfix: Fix duplicate coinbase tx weight reservation":
(https://github.com/bitcoin/bitcoin/pull/31384#issuecomment-2580905232)
Thanks for reviewing @pinheadmz @luke-jr @Sjors

I've recently foreced pushed from 2fb833e69627a8f9b1c4b14b60d1df9903adf210 to 7ab1344eb13228998bcead7328d5cc0a905971d4 [diff](https://github.com/bitcoin/bitcoin/compare/2fb833e69627a8f9b1c4b14b60d1df9903adf210..7ab1344eb13228998bcead7328d5cc0a905971d4)

Changes were:

1. Reverting https://github.com/bitcoin/bitcoin/commit/76a95522b322c1a8ab1594ca8bf5ac99ab379c46 but improving the help text.
2. Use policy `DEFAULT_MAX_BLOCK_WEIGHT` instead
...

💬 theuni commented on pull request "refactor: modernize recent `ByteType` usages and read/write functions":
(https://github.com/bitcoin/bitcoin/pull/31601#issuecomment-2580925856)
> > And doing that all over would be a recipe for never-ending churn
>
> Isn't that what we're doing regularly though? Endlessly making the legacy codebase a tiny bit more readable, a tiny bit more modern, a tiny bit more maintainable and consistent?

Not change for change's sake, no. Please consider this from the POV of reviewers and maintainers. A constant stream of changes like that is exhausting. It would distract from larger improvements.

I think @maflcko's comment above is a good d
...

📝 mzumsande opened a pull request: "wallet: fix rescanning inconsistency"
(https://github.com/bitcoin/bitcoin/pull/31629)
If the chain advances during a rescan, ScanForWalletTransactions would previously process the new blocks without adjusting `m_last_processed_block`, which would leave the wallet in an inconsistent state temporarily, and could lead to crashes in the GUI reported in #31474.
Fix this by not rescanning blocks beyond `m_last_processed_block` - for all blocks beyond that height, there will be pending BlockConnected notifications that will process them after the rescan is finished.

This means that
...

💬 mzumsande commented on issue "Potential crash (assert) rescanning wallet":
(https://github.com/bitcoin/bitcoin/issues/31474#issuecomment-2580943009)
See #31629, I took your version, just added some doc changes.

💬 l0rinc commented on pull request "refactor: modernize recent `ByteType` usages and read/write functions":
(https://github.com/bitcoin/bitcoin/pull/31601#issuecomment-2580961020)
This PR was a follow-up from the comments on Marco's change.
The main changes are Russ's template simplification and my suggestion to const the writes to obviate which we're modifying.

💬 sipa commented on pull request "validation: write chainstate to disk every hour":
(https://github.com/bitcoin/bitcoin/pull/30611#issuecomment-2580988597)
ACK b7f3b6fd96ca2d5d1f9538affef682f778bd07e8

In the log message `[bench] FlushStateToDisk: write coins cache to disk (598202 coins, 86527KiB) started`, the number of coins and the size is the total cache size now, including non-dirty entries. I guess it should be changed to just account for the dirty ones, as well as in the "Flushing large UTXO set to disk" warning message.

💬 andrewtoth commented on pull request "validation: write chainstate to disk every hour":
(https://github.com/bitcoin/bitcoin/pull/30611#issuecomment-2581015366)
> it should be changed to just account for the dirty ones

That would be a follow-up from #28233.

💬 ismaelsadeeq commented on pull request "cluster mempool: introduce TxGraph":
(https://github.com/bitcoin/bitcoin/pull/31363#discussion_r1909271584)
optional nit when their is need to retouch

In "txgraph: (feature) add initial version" 0c8dc2323eb1ec34357a807f0860cf0a08a63a75
you mean `QualityLevel::ACCEPTABLE` right we could just reference the enum like you were doing `txgraph.cpp`?
```suggestion
* The transactions will be returned in a topologically-valid order of acceptable quality (QualityLevel::ACCEPTABLE).
```

💬 ismaelsadeeq commented on pull request "cluster mempool: introduce TxGraph":
(https://github.com/bitcoin/bitcoin/pull/31363#discussion_r1909270566)
nit when retouching

In " txgraph: (feature) add staging support" 972df15af28ba6787f096162f776b2973342e674
```suggestion
/** Get the feerate of the chunk which transaction arg is in the main graph. Returns the
```

💬 ismaelsadeeq commented on pull request "cluster mempool: introduce TxGraph":
(https://github.com/bitcoin/bitcoin/pull/31363#discussion_r1909277721)
In "clusterlin: add FixLinearization function + fuzz test" 1c5f1c4601e0a895258cfe073125361f7a6ea012
Is it safe to get the `ClusterIndex` when j is 0, we will subtract 1 from `uint32_t` which will result to a large positive number?

📝 achow101 opened a pull request: "doc: Archive 28.1 release notes"
(https://github.com/bitcoin/bitcoin/pull/31630)