💬 ajtowns commented on pull request "mempool / rpc: add getprioritisedtransactions, delete a mapDeltas entry when delta==0":
(https://github.com/bitcoin/bitcoin/pull/27501#discussion_r1220348795)
Could add a `-deprecatedrpc=prioritisetransactionresult` flag to allow access to the current format of result.

💬 ajtowns commented on pull request "mempool / rpc: add getprioritisedtransactions, delete a mapDeltas entry when delta==0":
(https://github.com/bitcoin/bitcoin/pull/27501#discussion_r1220330124)
This is `"transactionid"` in `getrawmempool` and `getmempoolancestors`, probably better to be consistent?

💬 ajtowns commented on pull request "mempool / rpc: add getprioritisedtransactions, delete a mapDeltas entry when delta==0":
(https://github.com/bitcoin/bitcoin/pull/27501#discussion_r1220349890)
Probably better to either expose it via the rpc, or not have the useless code at all?

💬 ajtowns commented on pull request "mempool / rpc: add getprioritisedtransactions, delete a mapDeltas entry when delta==0":
(https://github.com/bitcoin/bitcoin/pull/27501#discussion_r1220335611)
Provides a good excuse to map to an object rather than just a the delta itself, which in turn might make it easier to add other info in future without having a breaking change?

💬 ajtowns commented on pull request "doc: Clarify -datacarriersize, add -datacarriersize=2 tests":
(https://github.com/bitcoin/bitcoin/pull/27832#issuecomment-1579448029)
Concept ACK

💬 instagibbs commented on pull request "mempool / rpc: add getprioritisedtransactions, delete a mapDeltas entry when delta==0":
(https://github.com/bitcoin/bitcoin/pull/27501#discussion_r1220360880)
100% for object for future-proofing, either way

💬 theStack commented on pull request "Introduce 'getblockfileinfo' RPC command":
(https://github.com/bitcoin/bitcoin/pull/27770#issuecomment-1579485693)
FWIW, I was curious how hard it would be to implement `getblockfileinfo` in the functional test framework by parsing the block files: https://github.com/theStack/bitcoin/blob/97ea647ee9b186ff4d3be2ba5ac91b94e5371323/test/functional/test_framework/util.py#L559-L584 (branch https://github.com/theStack/bitcoin/tree/getblockfileinfo_python)

The result seems to work, but is still kind of ugly and hacky. To get a parsed block's height (without flooding the node with hundreds of `getblockheader` RPC
...

💬 theStack commented on pull request "Return EXIT_FAILURE on post-init fatal errors":
(https://github.com/bitcoin/bitcoin/pull/27708#issuecomment-1579532617)
> > Regarding the conclusion in the first commit (_"'StartShutdown' should only be used for user requested shutdowns. Internal errors that cause a shutdown should use 'AbortNode'."_), the following is probably another candidate where `AbortNode` is preferred over `StartShutdown`?
>
> Yeah absolutely. Good eye. If you compare it in-detail, that function is a plain copy of `AbortNode`: it (1) calls `SetMiscWarning`, (2) logs the error, (3) notifies the UI by calling `InitError` (noui listeners
...

💬 hebasto commented on pull request "guix: Clean up manifest":
(https://github.com/bitcoin/bitcoin/pull/27811#issuecomment-1579928821)
My Guix build:
```
c4f2c29c7b5e1ab211ea38358517cf6c2a1e154e5d2c0d036e7750a8f5f5555a guix-build-77414d50d6f7/output/aarch64-linux-gnu/SHA256SUMS.part
c902944cce64fe7f2cee1242f18b35dbe1c5ce515e921c08c9bfebb34d0c28ca guix-build-77414d50d6f7/output/aarch64-linux-gnu/bitcoin-77414d50d6f7-aarch64-linux-gnu-debug.tar.gz
3aad68ed102f2f1a18217b3d2b2b32ba51f266c03b971f05f8bebd160bbf7d75 guix-build-77414d50d6f7/output/aarch64-linux-gnu/bitcoin-77414d50d6f7-aarch64-linux-gnu.tar.gz
36737484c80cdfc10
...

👋 MarcoFalke's pull request is ready for review: "p2p: Stop relaying non-mempool txs"
(https://github.com/bitcoin/bitcoin/pull/27625)

💬 joostjager commented on pull request "[mempool] allow tx replacement by smaller witness":
(https://github.com/bitcoin/bitcoin/pull/24007#issuecomment-1579986129)
Regarding use cases: perhaps the potential coinjoin problem with annexes mentioned in https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2023-June/021736.html can be mitigated with this PR.

💬 achow101 commented on pull request "Use `int32_t` type for most transaction size/weight values":
(https://github.com/bitcoin/bitcoin/pull/23962#issuecomment-1580072467)
Silent merge conflict (with `-werror`)

```
../../../src/test/miniminer_tests.cpp: In member function ‘void miniminer_tests::miniminer_1p1c::test_method()’:
../../../src/test/miniminer_tests.cpp:145:66: error: narrowing conversion of ‘((const boost::operators_impl::dereferenceable<boost::multi_index::detail::hashed_index_iterator<boost::multi_index::detail::hashed_index_node<boost::multi_index::detail::hashed_index_node<boost::multi_index::detail::ordered_index_node<boost::multi_index::detai
...

👋 achow101's pull request is ready for review: "wallet: Load database records in a particular order"
(https://github.com/bitcoin/bitcoin/pull/24914)

💬 S3RK commented on pull request "wallet: Have the wallet store the key for automatically generated descriptors":
(https://github.com/bitcoin/bitcoin/pull/26728#issuecomment-1580076237)
tACK c79539fc20d902b5f9e098cc996c4e27c8e5b8c5

I found one odd behaviour mentioned in https://github.com/bitcoin/bitcoin/pull/26728#issuecomment-1489825161
Even if you create a wallet with the master branch you still have one opportunity to trigger upgrade procedure

Reproduction steps
* create new blank wallet with master branch
* import 6 descriptors derived from same HD key
* call to `getxpub` says there is no HD key
* reload wallet (this will trigger upgrade procedure)
* call to `g
...

💬 achow101 commented on pull request "mempool / rpc: add getprioritisedtransactions, delete a mapDeltas entry when delta==0":
(https://github.com/bitcoin/bitcoin/pull/27501#issuecomment-1580084709)
ACK 67b7fecacd0489809690982c89ba2d0acdca938c

🚀 achow101 merged a pull request: "mempool / rpc: add getprioritisedtransactions, delete a mapDeltas entry when delta==0"
(https://github.com/bitcoin/bitcoin/pull/27501)

💬 TheCharlatan commented on pull request "guix: Clean up manifest":
(https://github.com/bitcoin/bitcoin/pull/27811#discussion_r1221049461)
Is there a good reason for using `git-fetch` beyond being more consistent? Seems like this is just less efficient, because we get and hash the entire repository instead of just an archive. Probably all the other cases should be using `url-fetch` instead, or at least with the exception of the sourceware repository?

👍 dergoegge approved a pull request: "ci: enable AArch64 target in MSAN jobs"
(https://github.com/bitcoin/bitcoin/pull/27824#pullrequestreview-1467123893)
utACK 2ebeb421dd21a69344a32c681ccbfcf5e5c6dab9

💬 TheCharlatan commented on pull request "kernel: Remove shutdown from kernel library":
(https://github.com/bitcoin/bitcoin/pull/27711#issuecomment-1580365465)
Thank you @theuni and @ryanofsky for the great discussion so far.

Updated a6a3c3245303d05917c04460e71790e33241f3b5 -> f13880e83200ec824d7528061f96708d96bf9bd4 ([rmKernelShutdown_0](https://github.com/TheCharlatan/bitcoin/commits/rmKernelShutdown_0) -> [rmKernelShutdown_1](https://github.com/TheCharlatan/bitcoin/commits/rmKernelShutdown_1), [compare](https://github.com/TheCharlatan/bitcoin/compare/rmKernelShutdown_0..rmKernelShutdown_1)).

This update reworks the existing logic in this pull
...

👍 dergoegge approved a pull request: "fuzz: Fix mini_miner_selection running out of coin"
(https://github.com/bitcoin/bitcoin/pull/27806#pullrequestreview-1467317035)
Code review ACK bbb927739955b1588d94c5fd8ed73ddf5afde46c