💬 glozow commented on pull request "p2p: Don't process mutated blocks":
(https://github.com/bitcoin/bitcoin/pull/29412#discussion_r1499609287)
e2d1eb2e2001c31b43154df47ab5be215a26774f

this is the same thing as
```suggestion
tx = self.wallet.create_self_transfer()["tx"]
```

💬 glozow commented on pull request "p2p: Don't process mutated blocks":
(https://github.com/bitcoin/bitcoin/pull/29412#discussion_r1499613815)
in e2d1eb2e2001c31b43154df47ab5be215a26774f
does this need a `with p2p_lock`?

✅ maflcko closed a pull request: "test: Use test framework utils in functional tests"
(https://github.com/bitcoin/bitcoin/pull/28528)

💬 maflcko commented on pull request "test: Use test framework utils in functional tests":
(https://github.com/bitcoin/bitcoin/pull/28528#issuecomment-1959945805)
Closing for now due to inactivity. I think it is pretty clear that a change like this won't be going in in one go.

If someone wants to try this again, I'd recommend to try a smaller portion and see if reviewers like it. If not, then maybe it isn't worth changing?

💬 LarryRuane commented on pull request "test: test_bitcoin: allow -testdatadir=<datadir>":
(https://github.com/bitcoin/bitcoin/pull/26564#issuecomment-1959949768)
>This contradicts the PR summary:

Thanks for the review! You're right, good catch. I updated the PR description.

The behavior is correct, we don't want to remove the `.lock` file at the start of the run, because we've just _acquired_ the lock, and want to keep it while we delete the test's data directory that might be left over from a previous run. That's why there's a `datadir` at the same level as `.lock` (so we can delete `datadir` without also deleting `.lock`). This behavior was sugge
...

💬 instagibbs commented on pull request "Mempool util: Add RBF diagram checks for single chunks against clusters of size 2":
(https://github.com/bitcoin/bitcoin/pull/29242#discussion_r1499641315)
done

💬 instagibbs commented on pull request "Mempool util: Add RBF diagram checks for single chunks against clusters of size 2":
(https://github.com/bitcoin/bitcoin/pull/29242#discussion_r1499641781)
removed, since I stripped out the sorting function from `BuildDiagramFromUnsortedChunks`

🤔 ryanofsky reviewed a pull request: "assumeutxo: Get rid of faked nTx and nChainTx values"
(https://github.com/bitcoin/bitcoin/pull/29370#pullrequestreview-1896432015)
Thanks for the review. Tried to answer some questions below and I will work on the suggestions

💬 ryanofsky commented on pull request "assumeutxo: Get rid of faked nTx and nChainTx values":
(https://github.com/bitcoin/bitcoin/pull/29370#discussion_r1499608826)
re: https://github.com/bitcoin/bitcoin/pull/29370#discussion_r1499266054

> Just to clarify, the `is_active` check is just a belt-and-suspender, since `pindex == snap_base` implies `is_active`?

It's meant to be a real check. I should probably add a comment about it. The idea is that on the snapshot chain, the snapshot block should be in `setBlockIndexCandidates` even though earlier transactions may not be downloaded.

I don't think `pindex == snap_base` implies anything about `is_active`,
...

💬 ryanofsky commented on pull request "assumeutxo: Get rid of faked nTx and nChainTx values":
(https://github.com/bitcoin/bitcoin/pull/29370#discussion_r1499641472)
re: https://github.com/bitcoin/bitcoin/pull/29370#discussion_r1499519079

> Why would the _background_ chainstate have the _snapshot_ base? The comment seems wrong and the second branch of the if condition is never taken, no?

I think the comment is just saying that if pindex is an ancestor of the snapshot base, it should be in setBlockIndexCandidates. It's possible the snapshot base could be in there too, but the code is only checking for it to be there if the chain is active, or if all tra
...

✅ maflcko closed a pull request: "assumeutxo, rpc: Add 'start' parameter to loadtxoutset"
(https://github.com/bitcoin/bitcoin/pull/28659)

💬 maflcko commented on pull request "assumeutxo, rpc: Add 'start' parameter to loadtxoutset":
(https://github.com/bitcoin/bitcoin/pull/28659#issuecomment-1959967361)
Closing for now. This has been a placeholder for months now, with no code to review or merge, and the CI failing anyway. Feel free to open a new pull request once and if there is something to review. If you have questions, you can leave them in the existing thread: https://github.com/bitcoin/bitcoin/issues/28620

👍 cbergqvist approved a pull request: "test: test_bitcoin: allow -testdatadir=<datadir>"
(https://github.com/bitcoin/bitcoin/pull/26564#pullrequestreview-1896496278)
ACK 7b81dea. One follow-up suggestion.

💬 cbergqvist commented on pull request "test: test_bitcoin: allow -testdatadir=<datadir>":
(https://github.com/bitcoin/bitcoin/pull/26564#discussion_r1499651040)
Suggestion: change to something like `"Test directory (not deleted until next run): "`

💬 maflcko commented on pull request "assumeutxo state and locking cleanup":
(https://github.com/bitcoin/bitcoin/pull/28608#issuecomment-1959970631)
Not sure what the status here is? It would be good to rebase, so that reviewers can take a look, or close, so that it can be grabbed up, if there is need.

💬 ariard commented on pull request "[WIP] Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#issuecomment-1959971463)
> interesting what is the lowest performance linux host assumed for decentralization of the tx-relay network.
assume always 24/7 internet and on a vpcu instance, not baremetal.

Running this branch mainet on a 2 vcpu instance, with the following performance characteristics:
```
processor : 0
vendor_id : AuthenticAMD
cpu family : 25
model : 1
model name : AMD EPYC 7543 32-Core Processor
stepping : 1
microcode : 0xa0011d1
cpu MHz : 2
...

💬 maflcko commented on pull request "miniscript: convert non-critical asserts to Assumes":
(https://github.com/bitcoin/bitcoin/pull/28678#issuecomment-1959974038)
Are you still working on this?

💬 instagibbs commented on pull request "Mempool util: Add RBF diagram checks for single chunks against clusters of size 2":
(https://github.com/bitcoin/bitcoin/pull/29242#discussion_r1499665791)
@sdaftuar that matches my understanding.

>Removing the sorting step in this function (moving it to the caller). The function can then take Span<const FeeFrac> as input too, and perhaps in a further iteration (in a later PR) this function could then be turned into one that actually performs chunking too.

Took this suggestion

💬 maflcko commented on pull request "kernel: Remove dependency on CScheduler":
(https://github.com/bitcoin/bitcoin/pull/28960#issuecomment-1959978642)
Added to 28.0, since it looks rfm, but is probably waiting on the branch-off?

💬 maflcko commented on pull request "RPC: add new `listmempooltransactions`":
(https://github.com/bitcoin/bitcoin/pull/29016#issuecomment-1959989869)
Are you still working on this? Looks like there are outstanding questions.

✅ maflcko closed a pull request: "wallet: move lock at the top of ReleaseWallet"
(https://github.com/bitcoin/bitcoin/pull/29155)