💬 sipa commented on pull request "util: add VecDeque":
(https://github.com/bitcoin/bitcoin/pull/30161#discussion_r1629971528)
Done, though naive self-assignment isn't actually broken here. It's just test code though, so better to be more obviously correct.

💬 sipa commented on pull request "util: add VecDeque":
(https://github.com/bitcoin/bitcoin/pull/30161#issuecomment-2153072603)
I've added some doxygen comments on `VecDeque`.

💬 sipa commented on pull request "net_processing: make any misbehavior trigger immediate discouragement":
(https://github.com/bitcoin/bitcoin/pull/29575#issuecomment-2153089008)
@sr-gi Done, in a new commit, for both `MaybePunishNodeForBlock` and `MaybePunishNodeForTx`.

💬 theuni commented on pull request "build: add `-Wundef`":
(https://github.com/bitcoin/bitcoin/pull/29876#issuecomment-2153158803)
Any reason this is still in draft? I agree it seems useful do enable before the CMake switch.

💬 pinheadmz commented on pull request "json-rpc 2.0 followups: docs, tests, cli":
(https://github.com/bitcoin/bitcoin/pull/30238#discussion_r1630032080)
hmmm I'm struggling to get a link to survive the lint test

💬 instagibbs commented on pull request "Ephemeral Anchors, take 2":
(https://github.com/bitcoin/bitcoin/pull/30239#issuecomment-2153175605)
going to put this into draft until https://github.com/bitcoin/bitcoin/pull/29496 is merged, since dust checks are completely off when `-acceptnonstdtxn=1`. which is required for TRUC currently.

📝 instagibbs converted_to_draft a pull request: "Ephemeral Anchors, take 2"
(https://github.com/bitcoin/bitcoin/pull/30239)
A replacement for https://github.com/bitcoin/bitcoin/pull/29001

Now that we have 1P1C relay, TRUC transactions and sibling eviction, it makes sense to retarget this feature more narrowly by not introducing a new output type for now, and simple focusing on the feature of allowing temporary dust in the mempool.

Users of this can immediately use dust outputs as:
1. Single keyed anchor (can be shared by multiple parties)
2. Single unkeyed anchor, ala P2WSH(OP_TRUE), P2SH(OP_TRUE)

Which is
...

💬 cbergqvist commented on pull request "wallet: Migrate legacy wallets to descriptor wallets without requiring BDB":
(https://github.com/bitcoin/bitcoin/pull/26596#discussion_r1630046789)
If the code is unable to communicate why it is like it is, it's good to explain the otherwise surprising state of it. It would hopefully limit the amount of questions like my prior one from https://github.com/bitcoin/bitcoin/pull/26596#pullrequestreview-2089208116:
> `LegacyScriptPubKeyMan::NewKeyPool()` only touches data from `LegacyDataSPKM`, should it be moved to that class as public (or protected and then exposed in the subclass?).

But I guess you're fairly confident it will be deleted
...

💬 sr-gi commented on pull request "net_processing: make any misbehavior trigger immediate discouragement":
(https://github.com/bitcoin/bitcoin/pull/29575#issuecomment-2153187235)
ACK [6eecba4](https://github.com/bitcoin/bitcoin/pull/29575/commits/6eecba475efd025eb011400af58621ad5823994e)

💬 instagibbs commented on pull request "util: add VecDeque":
(https://github.com/bitcoin/bitcoin/pull/30161#issuecomment-2153187763)
reACK https://github.com/bitcoin/bitcoin/pull/30161/commits/ee253ca7dea9bed01d4c1800760477ef06310df8

reviewed via `git range-diff master e4ecb82 ee253ca`

💬 sr-gi commented on pull request "refactor: move m_is_inbound out of CNodeState":
(https://github.com/bitcoin/bitcoin/pull/30233#discussion_r1630056430)
https://github.com/bitcoin/bitcoin/pull/29575#issuecomment-2152662655

Will rebase on top of that

💬 instagibbs commented on pull request "locks: introduce mutex for tx download, flush rejection filters on UpdatedBlockTip":
(https://github.com/bitcoin/bitcoin/pull/30111#issuecomment-2153199835)
reACK https://github.com/bitcoin/bitcoin/pull/30111/commits/a9a6de5a7c8c411519622a32bd4b998df5d7d883

Only relevant change is suggested commit message fix

reviewed via `git range-diff master ef8de26 a9a6de5`

💬 cbergqvist commented on pull request "wallet: Migrate legacy wallets to descriptor wallets without requiring BDB":
(https://github.com/bitcoin/bitcoin/pull/26596#discussion_r1630064452)
Thanks for taking the time to clarify this matter!

💬 pinheadmz commented on pull request "json-rpc 2.0 followups: docs, tests, cli":
(https://github.com/bitcoin/bitcoin/pull/30238#discussion_r1630091414)
ok passed ci with `/doc/...`

👍 hebasto approved a pull request: "util: add VecDeque"
(https://github.com/bitcoin/bitcoin/pull/30161#pullrequestreview-2103018546)
re-ACK ee253ca7dea9bed01d4c1800760477ef06310df8, code deduplication using `emplace_{back,front}` is really nice.

👍 cbergqvist approved a pull request: "wallet: Migrate legacy wallets to descriptor wallets without requiring BDB"
(https://github.com/bitcoin/bitcoin/pull/26596#pullrequestreview-2103034416)
ACK a8f8a2d5d0e2328cbe2e569c7e12f653d18f456d

(Ran tests on 31632d18ea122def82820770f14d9aa009726114 but only comments changed since then).
Functional tests including `--extended` passed (a few automatically skipped).
`make check` passed.

👍 cbergqvist approved a pull request: "util: add BitSet"
(https://github.com/bitcoin/bitcoin/pull/30160#pullrequestreview-2103029469)
ACK c99063e902c810dde1742696b3140610120d391c

(Ran `git range-diff 3bc131a~2..3bc131a c99063e~2..c99063e`).

💬 cbergqvist commented on pull request "util: add BitSet":
(https://github.com/bitcoin/bitcoin/pull/30160#discussion_r1630102455)
Why not use the ability to put the descriptions inside the `static_assert()`s here and elsewhere?
```suggestion
static_assert(std::is_integral_v<I> && std::is_unsigned_v<I> && std::numeric_limits<I>::radix == 2,
"Only binary, unsigned, integer, types allowed.");
```

💬 sipa commented on pull request "util: add BitSet":
(https://github.com/bitcoin/bitcoin/pull/30160#discussion_r1630120206)
I think the condition is sufficiently self-descriptive that this isn't really needed (the compiler error emitted will report the line causing it anyway). And I still like to have a comment on all top-level statements/definitions inside a class, even if it's trivial, just for organization purposes.

💬 murchandamus commented on issue "fuzz: Fix timeouts":
(https://github.com/bitcoin/bitcoin/issues/28812#issuecomment-2153274374)
I stumbled upon a fuzz seed that timed out in CI for `package_rbf`. Since the seed appears to be too big to paste, I provide the output of `$ base64 95a3b1b42d43ac7190f24ff116b2cd6515e7a566 > /tmp/base64-95a3b1b4-oom.txt`:
[base64-95a3b1b4-oom.txt](https://github.com/user-attachments/files/15685350/base64-95a3b1b4-oom.txt)