💬 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
💬 josibake commented on pull request "rpc, p2p: allow `disconnectnode` with subnet":
(https://github.com/bitcoin/bitcoin/pull/26576#discussion_r1184671009)
The release notes could be a bit more detailed, maybe mentioning that this aligns `DisconnectNode` and the `disconnectnode` rpc. It's nice to have at least a summary of the motivation in the release notes.
(https://github.com/bitcoin/bitcoin/pull/26576#discussion_r1184671009)
The release notes could be a bit more detailed, maybe mentioning that this aligns `DisconnectNode` and the `disconnectnode` rpc. It's nice to have at least a summary of the motivation in the release notes.
🤔 josibake reviewed a pull request: "rpc, p2p: allow `disconnectnode` with subnet"
(https://github.com/bitcoin/bitcoin/pull/26576#pullrequestreview-1412513149)
crACK https://github.com/bitcoin/bitcoin/pull/26576/commits/23f4c2cb452d25f61dada898d5cc4c74f72e0145
(https://github.com/bitcoin/bitcoin/pull/26576#pullrequestreview-1412513149)
crACK https://github.com/bitcoin/bitcoin/pull/26576/commits/23f4c2cb452d25f61dada898d5cc4c74f72e0145
💬 josibake commented on pull request "rpc, p2p: allow `disconnectnode` with subnet":
(https://github.com/bitcoin/bitcoin/pull/26576#discussion_r1184667474)
it would be great to add an example for subnets in the help section below.
(https://github.com/bitcoin/bitcoin/pull/26576#discussion_r1184667474)
it would be great to add an example for subnets in the help section below.
💬 TheCharlatan commented on pull request "refactor, kernel: Decouple ArgsManager from blockstorage":
(https://github.com/bitcoin/bitcoin/pull/27125#discussion_r1184686191)
Seems like it's a bug: https://github.com/include-what-you-use/include-what-you-use/issues/648
Since it sounds like it's going to be fixed in an upcoming version, I'll remove the `memory` include.
(https://github.com/bitcoin/bitcoin/pull/27125#discussion_r1184686191)
Seems like it's a bug: https://github.com/include-what-you-use/include-what-you-use/issues/648
Since it sounds like it's going to be fixed in an upcoming version, I'll remove the `memory` include.
💬 willcl-ark commented on issue "Coinstats index corrupted after invalidateblock and clean shutdown":
(https://github.com/bitcoin/bitcoin/issues/27558#issuecomment-1534309409)
Thanks @mzumsande, I can confirm that that patch does fix the described issue for me too.
(https://github.com/bitcoin/bitcoin/issues/27558#issuecomment-1534309409)
Thanks @mzumsande, I can confirm that that patch does fix the described issue for me too.
💬 MarcoFalke commented on pull request "refactor, kernel: Decouple ArgsManager from blockstorage":
(https://github.com/bitcoin/bitcoin/pull/27125#discussion_r1184783154)
Looks like this fix is part of the branch we are using, so this seems a separate bug?
(https://github.com/bitcoin/bitcoin/pull/27125#discussion_r1184783154)
Looks like this fix is part of the branch we are using, so this seems a separate bug?
💬 dergoegge commented on pull request "net: call getaddrinfo() in detachable thread to prevent stalling":
(https://github.com/bitcoin/bitcoin/pull/27557#discussion_r1184786408)
Afaict this won't work here, because the thread might not finish and then the pool would just get clogged (see comment about abandoning the thread).
Performance should also not really matter here since this doesn't happen that often and startup time for a thread vs. time of a DNS lookup is probably negligible.
(https://github.com/bitcoin/bitcoin/pull/27557#discussion_r1184786408)
Afaict this won't work here, because the thread might not finish and then the pool would just get clogged (see comment about abandoning the thread).
Performance should also not really matter here since this doesn't happen that often and startup time for a thread vs. time of a DNS lookup is probably negligible.
💬 TheCharlatan commented on pull request "refactor, kernel: Decouple ArgsManager from blockstorage":
(https://github.com/bitcoin/bitcoin/pull/27125#discussion_r1184822728)
Huh, yeah, seems like it. But I guess the issue is similar to what is happening here?
(https://github.com/bitcoin/bitcoin/pull/27125#discussion_r1184822728)
Huh, yeah, seems like it. But I guess the issue is similar to what is happening here?
💬 TheCharlatan commented on pull request "refactor, kernel: Decouple ArgsManager from blockstorage":
(https://github.com/bitcoin/bitcoin/pull/27125#discussion_r1184831690)
I'm not sure if it is possible for lambda's to shadow a function that they call. Can you show me what you mean?
(https://github.com/bitcoin/bitcoin/pull/27125#discussion_r1184831690)
I'm not sure if it is possible for lambda's to shadow a function that they call. Can you show me what you mean?