💬 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`?
(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)

(https://github.com/bitcoin/bitcoin/issues/27568)

✅ geenutts closed an issue: "Github"
(https://github.com/bitcoin/bitcoin/issues/27568)
(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
...
(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
(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)
(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")
(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
...
(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()) {
```
(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
(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.
(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?
(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`)?
(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).
(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).
(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).
💬 ajtowns commented on pull request "mempool / rpc: add getprioritisationmap, delete a mapDeltas entry when delta==0":
(https://github.com/bitcoin/bitcoin/pull/27501#discussion_r1184516693)
You're already looking up the tx here, so you could use `find()` instead and on success also return the total modified fee.
(https://github.com/bitcoin/bitcoin/pull/27501#discussion_r1184516693)
You're already looking up the tx here, so you could use `find()` instead and on success also return the total modified fee.
💬 ajtowns commented on pull request "mempool / rpc: add getprioritisationmap, delete a mapDeltas entry when delta==0":
(https://github.com/bitcoin/bitcoin/pull/27501#discussion_r1184514467)
"transaction txid" is redundant here
(https://github.com/bitcoin/bitcoin/pull/27501#discussion_r1184514467)
"transaction txid" is redundant here
💬 ajtowns commented on pull request "mempool / rpc: add getprioritisationmap, delete a mapDeltas entry when delta==0":
(https://github.com/bitcoin/bitcoin/pull/27501#discussion_r1184518158)
If you have txindex enabled, you could lookup the txid of any `in-mempool: false` transactions in the txindex here, and, if found, calculate how many confirmations the tx has, and report that. That seems like it would be fairly useful, though not sure it's actually worth the effort (compared to writing a script that just calls `getrawtransaction $txid 1 | jq .confirmations`).
(https://github.com/bitcoin/bitcoin/pull/27501#discussion_r1184518158)
If you have txindex enabled, you could lookup the txid of any `in-mempool: false` transactions in the txindex here, and, if found, calculate how many confirmations the tx has, and report that. That seems like it would be fairly useful, though not sure it's actually worth the effort (compared to writing a script that just calls `getrawtransaction $txid 1 | jq .confirmations`).
💬 cshintov commented on issue "Slow memory leak in v22.0?":
(https://github.com/bitcoin/bitcoin/issues/24542#issuecomment-1534133728)
Hi facing similar issue. I'm running an rpc node. My memory usage grows beyond 36GB. I only see this problem when setting `rpcthreads` parameter. Without `rpcthreads` the usage doesn't grow beyond 4 GB.
Any progress? Did you figure out why the leak?
(https://github.com/bitcoin/bitcoin/issues/24542#issuecomment-1534133728)
Hi facing similar issue. I'm running an rpc node. My memory usage grows beyond 36GB. I only see this problem when setting `rpcthreads` parameter. Without `rpcthreads` the usage doesn't grow beyond 4 GB.
Any progress? Did you figure out why the leak?
:lock: fanquake locked an issue: "Github"
(https://github.com/bitcoin/bitcoin/issues/27568)
(https://github.com/bitcoin/bitcoin/issues/27568)
👍1