✅ waketraindev closed an issue: "Ability to retrieve peer info by peer id"
(https://github.com/bitcoin/bitcoin/issues/33478)
(https://github.com/bitcoin/bitcoin/issues/33478)
✅ waketraindev closed an issue: "No way to clear command history in RPC console or reset the console without restarting the node"
(https://github.com/bitcoin-core/gui/issues/897)
(https://github.com/bitcoin-core/gui/issues/897)
💬 sdaftuar commented on pull request "Cluster mempool":
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2512154527)
Neat idea-- however it looks like I also use this in `CTxMemPool::GetFeerateDiagram()`, where we want the size to be accurate.
I do like getting rid of the rounding error, but perhaps that would be best accomplished by changing the internal units of things like the blockminfeerate to be in terms of fee-per-weight, in a followup PR?
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2512154527)
Neat idea-- however it looks like I also use this in `CTxMemPool::GetFeerateDiagram()`, where we want the size to be accurate.
I do like getting rid of the rounding error, but perhaps that would be best accomplished by changing the internal units of things like the blockminfeerate to be in terms of fee-per-weight, in a followup PR?
💬 sdaftuar commented on pull request "Cluster mempool":
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2512156434)
I'm planning to take @glozow's suggestion above (https://github.com/bitcoin/bitcoin/pull/33629#issuecomment-3499901560).
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2512156434)
I'm planning to take @glozow's suggestion above (https://github.com/bitcoin/bitcoin/pull/33629#issuecomment-3499901560).
💬 sipa commented on pull request "Cluster mempool":
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2512170504)
Fair enough, and that's easier to do more granuarly then all-at-once guaranteeing that all uses of `FeePerVSize` are size-independent right now.
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2512170504)
Fair enough, and that's easier to do more granuarly then all-at-once guaranteeing that all uses of `FeePerVSize` are size-independent right now.
💬 instagibbs commented on pull request "Cluster mempool":
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2512171225)
https://github.com/glozow/bitcoin/commit/85895de30cc09db2b1f6be5f1057f79a68a6ccc9#r170215881
note that the suggested branch has at least one behavior change
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2512171225)
https://github.com/glozow/bitcoin/commit/85895de30cc09db2b1f6be5f1057f79a68a6ccc9#r170215881
note that the suggested branch has at least one behavior change
💬 hebasto commented on pull request "build: add `-W*-whitespace`":
(https://github.com/bitcoin/bitcoin/pull/32482#issuecomment-3514168708)
> > Thanks. I've pulled those two in here.
>
> Looking at the CI. Those commits are not going to work as implemented (essentially anywhere we are using Clang).
Here is a branch with the fixed commit: https://github.com/hebasto/bitcoin/commits/pr32482/1110.1/.
(https://github.com/bitcoin/bitcoin/pull/32482#issuecomment-3514168708)
> > Thanks. I've pulled those two in here.
>
> Looking at the CI. Those commits are not going to work as implemented (essentially anywhere we are using Clang).
Here is a branch with the fixed commit: https://github.com/hebasto/bitcoin/commits/pr32482/1110.1/.
💬 sdaftuar commented on pull request "Cluster mempool":
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2512204607)
I believe we've always thought about feerate in our RPCs/user interface as being fee-per-vsize, so I'd be reluctant to make that change as a one-off here; if we don't follow through and make fee-per-weight the norm, then I think our interface would feel more confusing for users.
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2512204607)
I believe we've always thought about feerate in our RPCs/user interface as being fee-per-vsize, so I'd be reluctant to make that change as a one-off here; if we don't follow through and make fee-per-weight the norm, then I think our interface would feel more confusing for users.
💬 sdaftuar commented on pull request "Cluster mempool":
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2512241874)
@instagibbs Good catch, thanks. (We should add a test for that usage of submitpackage!)
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2512241874)
@instagibbs Good catch, thanks. (We should add a test for that usage of submitpackage!)
💬 sdaftuar commented on pull request "Cluster mempool":
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2512245466)
Done.
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2512245466)
Done.
💬 sdaftuar commented on pull request "Cluster mempool":
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2512246538)
Changed to 1700.
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2512246538)
Changed to 1700.
💬 sdaftuar commented on pull request "Cluster mempool":
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2512250023)
Took this change (and the related one below). Planning to add a commit to the followup PR to get rid of the usage of `mapTx.modify()` here as well, now that the mempool doesn't keep any indices tied to feerate.
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2512250023)
Took this change (and the related one below). Planning to add a commit to the followup PR to get rid of the usage of `mapTx.modify()` here as well, now that the mempool doesn't keep any indices tied to feerate.
💬 sdaftuar commented on pull request "Cluster mempool":
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2512251764)
Done.
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2512251764)
Done.
💬 sdaftuar commented on pull request "Cluster mempool":
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2512254194)
Done in e6591f3ed
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2512254194)
Done in e6591f3ed
💬 sdaftuar commented on pull request "Cluster mempool":
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2512255248)
Redid this and `CalculateDescendantData` as you suggested.
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2512255248)
Redid this and `CalculateDescendantData` as you suggested.
💬 sdaftuar commented on pull request "Cluster mempool":
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2512255792)
Should be fixed now in that commit.
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2512255792)
Should be fixed now in that commit.
💬 sdaftuar commented on pull request "Cluster mempool":
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2512256794)
Done.
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2512256794)
Done.
💬 sdaftuar commented on pull request "Cluster mempool":
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2512270361)
I rewrote the commit message -- should be a bit better now.
(Also, it's worth noting that mini_miner is only used by the wallet, and we may want the wallet's behavior to change more slowly than with the next release, as it will take time for new software to be widely deployed.)
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2512270361)
I rewrote the commit message -- should be a bit better now.
(Also, it's worth noting that mini_miner is only used by the wallet, and we may want the wallet's behavior to change more slowly than with the next release, as it will take time for new software to be widely deployed.)
💬 sdaftuar commented on pull request "Cluster mempool":
(https://github.com/bitcoin/bitcoin/pull/33629#issuecomment-3514262272)
I believe I've responded to all of @sipa's comments, with the exception of the open issue around where to lock the mempool in `validation.cpp`.
(https://github.com/bitcoin/bitcoin/pull/33629#issuecomment-3514262272)
I believe I've responded to all of @sipa's comments, with the exception of the open issue around where to lock the mempool in `validation.cpp`.
💬 sipa commented on pull request "doc: corrected lockunspent rpc quoting":
(https://github.com/bitcoin/bitcoin/pull/31275#issuecomment-3514327614)
I wanted to verify if all invocations of `HelpExampleRpc` produce valid JSON, so I tested by adding a `UniValue::read` inside the function. The following RPCs all fail it still (with this PR):
* `getblockfrompeer`
* `importmempool`
* `getindexinfo`
* `addnode`
* `addconnection`
* `sendmsgtopeer`
* `createwalletdescriptor`
* `restorewallet`
* `listlabels`
* `loadwallet`
* `unloadwallet`
I think this means we either need to:
* Get rid of the `curl` example codes in the RPC help text
...
(https://github.com/bitcoin/bitcoin/pull/31275#issuecomment-3514327614)
I wanted to verify if all invocations of `HelpExampleRpc` produce valid JSON, so I tested by adding a `UniValue::read` inside the function. The following RPCs all fail it still (with this PR):
* `getblockfrompeer`
* `importmempool`
* `getindexinfo`
* `addnode`
* `addconnection`
* `sendmsgtopeer`
* `createwalletdescriptor`
* `restorewallet`
* `listlabels`
* `loadwallet`
* `unloadwallet`
I think this means we either need to:
* Get rid of the `curl` example codes in the RPC help text
...