💬 instagibbs commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1598711331)
picked a constant and used it

💬 instagibbs commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1598711398)
removed a series of these ignored args

💬 instagibbs commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1598711480)
Agreed, it's perhaps an older badly done version of #30066, removed

💬 maflcko commented on issue "RFC: "Insufficient review" tag for closed PRs":
(https://github.com/bitcoin/bitcoin/issues/29839#issuecomment-2108095272)
> Another idea would be to have a/the bot to automatically tag the PR with `insufficient review` if it gets stalled for a few weeks, before closing it, both to facilitate closing them for lack of activity but also to try to attract review effort for it.

Not sure if this can be done automatically, because evaluating whether review is sufficient or not may involve a broader understanding. For example, the existing review may have been sufficient, but a minor change was pushed and no one yet did
...

💬 instagibbs commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1598711567)
I turned off maxfeerate, jacked up the child feerates, and made constants for both

💬 Sjors commented on pull request "Silent payment index (for light wallets and consistency check)":
(https://github.com/bitcoin/bitcoin/pull/28241#discussion_r1598714122)
Exactly, with the caveat that you can use multiples of 16.

💬 theuni commented on pull request "depends: set AR & RANLIB for CMake":
(https://github.com/bitcoin/bitcoin/pull/30078#discussion_r1598717348)
Thanks, I think this is the way to go.

👍 theuni approved a pull request: "depends: set AR & RANLIB for CMake"
(https://github.com/bitcoin/bitcoin/pull/30078#pullrequestreview-2053123361)
utACK 019ad7327c397094d7648b55503bf5373b108a57. I didn't test, but I tried this approach on a few deps and it seemed to work as expected.

💬 setavenger commented on pull request "Silent payment index (for light wallets and consistency check)":
(https://github.com/bitcoin/bitcoin/pull/28241#discussion_r1598724027)
The [tweaks endpoint](https://github.com/setavenger/blindbit-oracle?tab=readme-ov-file#endpoints) with the `dustLimit` parameter. Any value should work I believe.

💬 ryanofsky commented on pull request "refactor: interfaces, make 'createTransaction' less error-prone ":
(https://github.com/bitcoin-core/gui/pull/807#discussion_r1598730244)
In commit "refactor: move CreatedTransactionResult to an util file" (6d59564646cd6ec087c9cab20f9d5707ec5dd4f9)

Would suggest maybe moving this struct to `wallet/types.h` instead of introducing a new header. That file is meant to hold wallet types that are used outside of the wallet library. (For context see https://github.com/bitcoin/bitcoin/pull/29415#discussion_r1543379055 which talks about wallet/types.h, node/types.h, and common/types.h files).

Or if you would prefer to use a separate
...

💬 tdb3 commented on pull request "cli: Detect port errors in rpcconnect and rpcport":
(https://github.com/bitcoin/bitcoin/pull/29521#discussion_r1598738005)
Thanks all for the suggestions. Pushed an update.
Since locally I have x86_64 (and not arm), I tried configuring/compiling with `--enable-werror` added to `./configure` (to minimize excess CI churn). Interestingly enough, this didn't result in the same warning as seen on arm CI.

💬 Sjors commented on pull request "Silent payment index (for light wallets and consistency check)":
(https://github.com/bitcoin/bitcoin/pull/28241#discussion_r1598741307)
I'll try that with 992 once I rebuilt the database. Is `dustLimit` `>= 992` or `> 992`?

💬 theuni commented on pull request "refactor: reserve memory allocation for transaction outputs":
(https://github.com/bitcoin/bitcoin/pull/30093#discussion_r1598744518)
A few suggestions:
- Looks to me like `txNew.vout` is always empty here, so I'm not sure why we'd bother to add `txNew.vout.size()` unless I'm missing something ?
- There's an additional likely insert for change. Since the reserved size here is exact, that could trigger a reallocation up to the next power-of-two. Makes sense to make this `vecSend.size()+1` just in case.
- I think the txouts could be `std::move`d (in both places) instead to save a copy similar to #30094?
- Any reason not `.re
...

🤔 theuni reviewed a pull request: "refactor: reserve memory allocation for transaction outputs"
(https://github.com/bitcoin/bitcoin/pull/30093#pullrequestreview-2053171781)
Concept ACK

👍 ryanofsky approved a pull request: "kernel: Remove batchpriority from kernel library"
(https://github.com/bitcoin/bitcoin/pull/30083#pullrequestreview-2053188813)
Code review ACK 04ffe4061da2d0135f206032e167703772b5da78, this is an obvious improvement

💬 ryanofsky commented on pull request "kernel: Remove batchpriority from kernel library":
(https://github.com/bitcoin/bitcoin/pull/30083#discussion_r1598754079)
In commit "kernel: Remove batchpriority from kernel library" (04ffe4061da2d0135f206032e167703772b5da78)

I think it might be useful to keep the `// End scope of ImportNow` comment here. If someone is adding new code at this point, it's possible they might want to add it with importing set to false, or importing set to true, so the comment would be a reminder that the value will change at this point.

💬 instagibbs commented on pull request "test: expand LimitOrphan and EraseForPeer coverage":
(https://github.com/bitcoin/bitcoin/pull/30082#discussion_r1598772376)
nope, changed

💬 instagibbs commented on pull request "test: expand LimitOrphan and EraseForPeer coverage":
(https://github.com/bitcoin/bitcoin/pull/30082#discussion_r1598772485)
done

💬 instagibbs commented on pull request "test: expand LimitOrphan and EraseForPeer coverage":
(https://github.com/bitcoin/bitcoin/pull/30082#discussion_r1598773210)
probably a good idea, though you might have better opinions on orphanage interface. I tightened the time testing a bit more by setting mocktime at beginning, then checking boundary conditions via subsequent setting of the time.

💬 glozow commented on pull request "p2p: index TxOrphanage by wtxid, allow entries with same txid":
(https://github.com/bitcoin/bitcoin/pull/30000#issuecomment-2108249084)
Ah, p2p_invalid_tx.py failed because `assert_debug_log` didn't match anymore (those should be unit tests!)