💬 kevkevinpal commented on pull request "test: Added coverage to Block not found error using gettxoutsetinfo":
(https://github.com/bitcoin/bitcoin/pull/30340#discussion_r1657352656)
thanks updated in [6809ffd](https://github.com/bitcoin/bitcoin/pull/30340/commits/6809ffd8a6c589c515af48da2dd8367e4c6c2c26)

also the earlier comment I addressed below https://github.com/bitcoin/bitcoin/pull/30340#issuecomment-2195027831

💬 darosior commented on pull request "fuzz: a new target for the coins database":
(https://github.com/bitcoin/bitcoin/pull/28216#issuecomment-2195058547)
Status here is i'm looking into some determinism issues pointed out by Niklas: https://gnusha.org/bitcoin-core-dev/2024-06-03.log

👍 theuni approved a pull request: "kernel: De-globalize validation caches"
(https://github.com/bitcoin/bitcoin/pull/30141#pullrequestreview-2145715522)
utACK modulo the non-copyable comment.

Looks good. Left some comments with a few nice-to-have's. I'm not sure how much trouble it'd be to pass in the nonces (and whether it's 100% safe to reuse them), so that might make more sense as a follow-up.

💬 theuni commented on pull request "kernel: De-globalize validation caches":
(https://github.com/bitcoin/bitcoin/pull/30141#discussion_r1657330024)
This function can be const.

💬 theuni commented on pull request "kernel: De-globalize validation caches":
(https://github.com/bitcoin/bitcoin/pull/30141#discussion_r1657322116)
Move this into `ValidationCache`? It seems unused anywhere else.

💬 theuni commented on pull request "kernel: De-globalize validation caches":
(https://github.com/bitcoin/bitcoin/pull/30141#discussion_r1657354399)
Same comment here, would be nice to pass this in. Perhaps one nonce to rule them all? I don't think there'd be any loss of security.

💬 theuni commented on pull request "kernel: De-globalize validation caches":
(https://github.com/bitcoin/bitcoin/pull/30141#discussion_r1657370573)
Similar to @stickies-v's comment in a previous commit, since we're going to be passing around references and pointers to this, it'd be a good idea to make it non-copyable to avoid accidental copies.

💬 theuni commented on pull request "kernel: De-globalize validation caches":
(https://github.com/bitcoin/bitcoin/pull/30141#discussion_r1657337310)
Might be useful to pass the nonce in rather than generating it here to make the order of rng/ValidationCache instantiation explicit.

Presumably it'd also make for easier testing with a deterministic seed.

💬 theuni commented on pull request "kernel: De-globalize validation caches":
(https://github.com/bitcoin/bitcoin/pull/30141#discussion_r1657394662)
Oh, I guess the mutex takes care of that for us. Still, wouldn't hurt to be explicit.

💬 murchandamus commented on pull request "Refactor BnB tests":
(https://github.com/bitcoin/bitcoin/pull/29532#discussion_r1622850893)
Thanks, fixed

💬 murchandamus commented on pull request "Refactor BnB tests":
(https://github.com/bitcoin/bitcoin/pull/29532#discussion_r1657289988)
Made all four tests output messages that indicate what went wrong in the success case.

💬 murchandamus commented on pull request "Refactor BnB tests":
(https://github.com/bitcoin/bitcoin/pull/29532#discussion_r1629892844)
Removed the static keyword and the clear as suggested.

💬 murchandamus commented on pull request "Refactor BnB tests":
(https://github.com/bitcoin/bitcoin/pull/29532#discussion_r1657397165)
Amended all the functions to directly create OutputGroups instead.

💬 murchandamus commented on pull request "Refactor BnB tests":
(https://github.com/bitcoin/bitcoin/pull/29532#discussion_r1622850804)
I have removed `nInput` from the function parameters of `MakeCoin(…)`.

💬 murchandamus commented on pull request "Refactor BnB tests":
(https://github.com/bitcoin/bitcoin/pull/29532#discussion_r1622850394)
I was able to remove all except for

```cpp
#include <consensus/amount.h>
#include <wallet/coinselection.h>
#include <wallet/test/wallet_test_fixture.h>

#include <boost/test/unit_test.hpp>
```

I’m not familiar with IOWY and could not turn up anything with a websearch, what is that?

💬 murchandamus commented on pull request "Refactor BnB tests":
(https://github.com/bitcoin/bitcoin/pull/29532#discussion_r1622920206)
Thanks, fixed

💬 murchandamus commented on pull request "Refactor BnB tests":
(https://github.com/bitcoin/bitcoin/pull/29532#discussion_r1622920679)
Thanks, fixed

👍 1alexbc1 approved a pull request: "test: Added coverage to Block not found error using gettxoutsetinfo"
(https://github.com/bitcoin/bitcoin/pull/30340#pullrequestreview-2145858736)
Amadeevantv yimelaja

💬 ariard commented on issue "Brainstorm: Transaction issuer-selected policy limits":
(https://github.com/bitcoin/bitcoin/issues/29454#issuecomment-2195109286)
> The problem with policy limits is they're all based on asking miners to ignore transactions that are potentially profitable. This is similar in spirit to opt-in RBF. And as we've seen, the supermajority of hash power eventually decided to ignore the opt-in flag and just do RBF all the time. If push came to shove, it wouldn't be surprising to run into the same class of problem with policy limits.

> Much better to focus on solutions like replace-by-fee-rate that rely on better aligning miner
...

💬 furszy commented on pull request "Refactor BnB tests":
(https://github.com/bitcoin/bitcoin/pull/29532#discussion_r1657420121)
> I’m not familiar with IOWY and could not turn up anything with a websearch, what is that?

Upps, the acronym is IWYU from include-what-you-use. I wrote it fast as IOWY "include only what you use".
Should stop using acronyms..