💬 pinheadmz commented on issue "Add RPC to set transaction comment":
(https://github.com/bitcoin/bitcoin/issues/12594#issuecomment-1533608333)
@GSPP `txid` by definition is a unique identifier for a transaction. Comments can be added to new transactions in some RPCs like `sendtoaddress` and those comments are visible in the GUI. What do you need? The ability to edit a transaction comment in the GUI?

👍 furszy approved a pull request: "Introduce `MockableDatabase` for wallet unit tests"
(https://github.com/bitcoin/bitcoin/pull/26715#pullrequestreview-1411697889)
ACK 33e2b82

💬 furszy commented on pull request "wallet: Pass through transaction locktime and preset input sequences and scripts to CreateTransaction":
(https://github.com/bitcoin/bitcoin/pull/25273#discussion_r1181279440)
tiny nit:
just because I'm using this in another PR:
```c++
change_pos == -1 ? std::nullopt : std::make_optional(change_pos)
```

💬 hebasto commented on pull request "qt: 25.0rc2 translations update":
(https://github.com/bitcoin/bitcoin/pull/27517#issuecomment-1533660835)
Updated.

🤔 achow101 reviewed a pull request: "Wallet: estimate the size of signed inputs using descriptors"
(https://github.com/bitcoin/bitcoin/pull/26567#pullrequestreview-1411720143)
The `MultiSigningProvider` seems like a fine approach.

💬 achow101 commented on pull request "Wallet: estimate the size of signed inputs using descriptors":
(https://github.com/bitcoin/bitcoin/pull/26567#discussion_r1184204984)
In 9e8df2de1750b823492c18a924fefbd7d474819e "descriptor: introduce a method to get the satisfaction size"

Is `MultiADescriptor` supposed to be missing a `MaxSatisfactionWeight`?

⚠️ geenutts opened an issue: "Github"
(https://github.com/bitcoin/bitcoin/issues/27568)

✅ geenutts closed an issue: "Github"
(https://github.com/bitcoin/bitcoin/issues/27568)

💬 mzumsande commented on issue "Coinstats index corrupted after invalidateblock and clean shutdown":
(https://github.com/bitcoin/bitcoin/issues/27558#issuecomment-1533742759)
A more detailed explanation: The indexes have a `Rewind()` function to reverse the effects of blocks (which calls `CustomRewind` for the coinstats index). However, once the index is synced, `Rewind` will only be invoked through `BlockConnected()` validationinterface signals, not when blocks are disconnected. So when `invalidateblock` is called, the state of the coinstatsindex will still point to the old block (unless a new block is connected before the node is stopped).

After restarting the n
...

💬 achow101 commented on pull request "rpc: add `descriptorprocesspsbt` rpc":
(https://github.com/bitcoin/bitcoin/pull/25796#issuecomment-1533778952)
ACK 9bf2d303295594b8327ce0d67b9c1de98701f80a

🚀 achow101 merged a pull request: "prune, import: allow pruning to work during loadblock import"
(https://github.com/bitcoin/bitcoin/pull/24957)

💬 theStack commented on pull request "mempool / rpc: add getprioritisationmap, delete a mapDeltas entry when delta==0":
(https://github.com/bitcoin/bitcoin/pull/27501#discussion_r1184370245)
tiny nit: should keep the list alphabetically sorted (that is, add the new RPC between "getpeerinfo" and "getrawmempool")

🤔 theStack reviewed a pull request: "mempool / rpc: add getprioritisationmap, delete a mapDeltas entry when delta==0"
(https://github.com/bitcoin/bitcoin/pull/27501#pullrequestreview-1412001811)
Concept ACK

Was a bit surprised to see that the fee-delta is returned in BTC as unit rather than Satoshis, I assume our RPC guidelines (saying _"**Always** use `AmountFromValue` and `ValueFromAmount` when inputting or outputting monetary values"_) have higher priority over the idea to simply use the same unit used for `prioritisetransaction`? Probably miners would be thankful to not need multiply the result by 100000000 first in order to delete mapDelta entries (and it would also make the tes
...

💬 theStack commented on pull request "mempool / rpc: add getprioritisationmap, delete a mapDeltas entry when delta==0":
(https://github.com/bitcoin/bitcoin/pull/27501#discussion_r1184372400)
nit, could get rid of extra variable:
```suggestion
for (const auto& [txid, delta_exists_pair] : mempool.GetPrioritisationMap()) {
```

💬 theStack commented on pull request "mempool / rpc: add getprioritisationmap, delete a mapDeltas entry when delta==0":
(https://github.com/bitcoin/bitcoin/pull/27501#discussion_r1184382264)
nit: `COIN` is defined in messages.py and should be imported from there

👍 jarolrod approved a pull request: "qt: 25.0rc2 translations update"
(https://github.com/bitcoin/bitcoin/pull/27517#pullrequestreview-1412184995)
ACK 20c076d0567da56637e69a26a8cc4e7d99124ebd

Made sure we're adhering to the translation process and ran the script. I sanity tested loading different translations within the gui to make sure nothing is wildly off.

💬 ajtowns commented on pull request "mempool / rpc: add getprioritisationmap, delete a mapDeltas entry when delta==0":
(https://github.com/bitcoin/bitcoin/pull/27501#discussion_r1184496189)
`map` is a C++ concept, this is returning a json object. Wouldn't avoiding the implementation baggage by calling it something like `getprioritisetransactions` be better?

💬 ajtowns commented on pull request "mempool / rpc: add getprioritisationmap, delete a mapDeltas entry when delta==0":
(https://github.com/bitcoin/bitcoin/pull/27501#discussion_r1184508118)
Shouldn't these messages be returned as an RPC result?

Shouldn't the message here also show the resulting total `delta`, and perhaps whether the tx was already in the mempool (`nTransactionsUpdated > 0`)?

💬 ajtowns commented on pull request "mempool / rpc: add getprioritisationmap, delete a mapDeltas entry when delta==0":
(https://github.com/bitcoin/bitcoin/pull/27501#discussion_r1184502562)
Having `struct delta_info { uint256 txid, CAmount delta, bool in_mempool }` and returning a `vector<delta_info>` might make this a little quicker (using `reserve()` upfront would avoid memory allocations on each iteration).

💬 ajtowns commented on pull request "mempool / rpc: add getprioritisationmap, delete a mapDeltas entry when delta==0":
(https://github.com/bitcoin/bitcoin/pull/27501#discussion_r1184510850)
These should be `"fee_delta"` and `"in_mempool"` -- hyphens are annoying because they often get interpreted as a subtraction, so it's easier to say `jq .[].fee_delta` than `jq .[]."fee-delta"`. Also more consistent with other code (cf #24528).