💬 vasild commented on pull request "util: avoid using thread_local variable that has a destructor":
(https://github.com/bitcoin/bitcoin/pull/30095#discussion_r1599981699)
My understanding is that it is safe to use `thread_local` on FreeBSD for variables that do not have a destructor even in the presence of the above bug (or bugs if they are two separate bugs).

👍 ryanofsky approved a pull request: "kernel: Remove batchpriority from kernel library"
(https://github.com/bitcoin/bitcoin/pull/30083#pullrequestreview-2055370796)
Code review ACK d4b17c7d46ad8e2833ade99d5b4c9741c913e84d, just added suggested comment since last review

💬 glozow commented on pull request "rpc: move UniValue in blockToJSON":
(https://github.com/bitcoin/bitcoin/pull/30094#issuecomment-2110260609)
backport for 26.x in #29899

👋 glozow's pull request is ready for review: "[26.x] archive 26.1 release notes + backports"
(https://github.com/bitcoin/bitcoin/pull/29899)

💬 naiyoma commented on pull request "test: Refactor fee/feerate calculations in feature_fee_estimation.py to use Decimal instead of float":
(https://github.com/bitcoin/bitcoin/pull/29566#discussion_r1600064674)
Thanks for the breakdown, seems ideal to also use satoshi_round here and to use round down for min and round up for max, helps with greater precision

💬 ajtowns commented on issue "contrib/signet/miner: miner won't consider --nbits parameter":
(https://github.com/bitcoin/bitcoin/issues/30091#issuecomment-2110275325)
> Another thing, it doesn't seem to exist an option to control the miner's internal "timer". That is, suppose I want to mine every minute instead of every 10 minutes.

The nbits options exist to control the miner's internal timer -- if you were to mine every minute instead of every 10 minutes, that will increase the difficulty until you run out of hashpower, at which point you'll be running your computer at full power 24/7 and end up getting blocks every 10 minutes. The "no matter how much has
...

💬 sdaftuar commented on pull request "test: add conflicting topology test case":
(https://github.com/bitcoin/bitcoin/pull/30066#discussion_r1600064969)
Looks good, thanks!

💬 sdaftuar commented on pull request "test: add conflicting topology test case":
(https://github.com/bitcoin/bitcoin/pull/30066#issuecomment-2110283778)
re-utACK

💬 rkrux commented on pull request "Fee Estimation: Ignore all transactions with an in-block child":
(https://github.com/bitcoin/bitcoin/pull/30079#discussion_r1600088583)
Oh nevermind, reviewing this PR gave me the answer: https://github.com/bitcoin/bitcoin/pull/29982

💬 naiyoma commented on pull request "test: Refactor fee/feerate calculations in feature_fee_estimation.py to use Decimal instead of float":
(https://github.com/bitcoin/bitcoin/pull/29566#discussion_r1600089403)
resolved

💬 rkrux commented on pull request "Fee Estimation: Ignore all transactions with an in-block child":
(https://github.com/bitcoin/bitcoin/pull/30079#discussion_r1600099780)
Hmm I see, as per this comment it says "not dependent on any other transactions in the **mempool**" - does this also apply to other transactions in the **block**?

https://github.com/bitcoin/bitcoin/blob/7fcf4e99791ca5be0b068ac03a81a50ece11dba3/src/policy/fees.cpp#L613

👍 hebasto approved a pull request: "kernel: Streamline util library"
(https://github.com/bitcoin/bitcoin/pull/29015#pullrequestreview-2055510319)
ACK 850c73e250fe0b248cae80d26561da7047696e3d.

Thanks for the `test/util/check-deps.sh` script!

💬 hebasto commented on pull request "kernel: Streamline util library":
(https://github.com/bitcoin/bitcoin/pull/29015#discussion_r1600117347)
Duplicated lines?

💬 hebasto commented on pull request "kernel: Streamline util library":
(https://github.com/bitcoin/bitcoin/pull/29015#discussion_r1600118750)
```suggestion
**Dependency graph**. Arrows show linker symbol dependencies. *Crypto* lib depends on nothing. *Util* lib is depended on by everything. *Kernel* lib depends only on consensus, crypto and util.
```

💬 hebasto commented on pull request "kernel: Streamline util library":
(https://github.com/bitcoin/bitcoin/pull/29015#discussion_r1600120289)
nit: Sort these lines?

💬 ryanofsky commented on pull request "Stratum v2 Noise Protocol":
(https://github.com/bitcoin/bitcoin/pull/29346#issuecomment-2110376536)
> Maybe I could start by having the `getblocktemplate` RPC use that new interface?

That's a good idea, nice way the interface could be used from the start.

> And then interface implementation would take care of getting the chain manager and mempool (via the node interface?).

Right, you could look at the interfaces::Chain for an example of how to initialize the interface. Store the interface in NodeContext like:

https://github.com/bitcoin/bitcoin/blob/7fcf4e99791ca5be0b068ac03a81a50ec
...

💬 hebasto commented on pull request "kernel: Remove batchpriority from kernel library":
(https://github.com/bitcoin/bitcoin/pull/30083#issuecomment-2110387585)
Concept ACK.

💬 Eunovo commented on pull request "wallet: fix unrelated parent conflict doesn't cause child tx to be marked as conflict":
(https://github.com/bitcoin/bitcoin/pull/29680#issuecomment-2110388198)
> Maybe this is starting to be more complicated than just having a struct per each reason? But I'd still argue this is a better approach in that it keeps all the logic for mempool removal reasons in one place and avoids duplicating code on each struct.

Same thing I was thinking. Using the class right now looks like it will make things more complicated. Maybe we should leave the class for a future change where it becomes necessary? @josibake @ryanofsky

👍 hebasto approved a pull request: "kernel: Remove batchpriority from kernel library"
(https://github.com/bitcoin/bitcoin/pull/30083#pullrequestreview-2055622041)
ACK d4b17c7d46ad8e2833ade99d5b4c9741c913e84d, I have reviewed the code and it looks OK.

💬 ryanofsky commented on pull request "wallet: fix unrelated parent conflict doesn't cause child tx to be marked as conflict":
(https://github.com/bitcoin/bitcoin/pull/29680#issuecomment-2110436529)
> Seems easily addressed with a constructor, no? Something like:

Yes, but those are manual constraints that you are writing by hand rather than automatic constraints expressed implicitly in the data definition. Depending on the constructors it may also only provide runtime checking rather than compile-time checking like in your example. And if the struct is mutable could allow invalid representations of state after construction.

I don't know what is best in this particular case, I would ju
...