💬 martinus commented on pull request "util: implement `noexcept` move assignment & move ctor for `prevector`":
(https://github.com/bitcoin/bitcoin/pull/27334#discussion_r1192934995)
I think it's enough to have `other._size = 0;` because then `other.is_direct()` will return false and no free happens
(https://github.com/bitcoin/bitcoin/pull/27334#discussion_r1192934995)
I think it's enough to have `other._size = 0;` because then `other.is_direct()` will return false and no free happens
💬 Sjors commented on pull request "p2p: Increase tx relay rate":
(https://github.com/bitcoin/bitcoin/pull/27630#issuecomment-1546551585)
> randomly drop half of `m_recently_announced_invs` if it exceeds some maximum size
I just realized this would act as a shredder on clusters. Some peers would only get the parent (that's fine), but (for n > 2 clusters) most would get children for which they don't have the parent yet.
So any kind of load distribution between peers would have to be done per cluster. Perhaps by randomly dropping clusters until the queue is at an acceptable size. Or by coordinating the distribution globally f
...
(https://github.com/bitcoin/bitcoin/pull/27630#issuecomment-1546551585)
> randomly drop half of `m_recently_announced_invs` if it exceeds some maximum size
I just realized this would act as a shredder on clusters. Some peers would only get the parent (that's fine), but (for n > 2 clusters) most would get children for which they don't have the parent yet.
So any kind of load distribution between peers would have to be done per cluster. Perhaps by randomly dropping clusters until the queue is at an acceptable size. Or by coordinating the distribution globally f
...
💬 real-or-random commented on pull request "Introduce field element and group element classes to test_framework/key.py":
(https://github.com/bitcoin/bitcoin/pull/26222#discussion_r1192949775)
This one too. I suggest iterating over the all doc comments, also to make "group element" vs "point" consistent. (`__repr__` uses group element.)
(https://github.com/bitcoin/bitcoin/pull/26222#discussion_r1192949775)
This one too. I suggest iterating over the all doc comments, also to make "group element" vs "point" consistent. (`__repr__` uses group element.)
💬 real-or-random commented on pull request "Introduce field element and group element classes to test_framework/key.py":
(https://github.com/bitcoin/bitcoin/pull/26222#discussion_r1192949389)
outdated comment
(https://github.com/bitcoin/bitcoin/pull/26222#discussion_r1192949389)
outdated comment
💬 joostjager commented on pull request "rpc: allow submitpackage to be called outside of regtest":
(https://github.com/bitcoin/bitcoin/pull/27609#issuecomment-1546603600)
>covers Lightning since AFAIK you won't make commitment transactions that spend each other
That's true. For anchor commitments I think it is the case that all outputs except the anchors (main balance outputs and htlcs) are timelocked CSV 1, so they aren't even available until after confirmation.
(https://github.com/bitcoin/bitcoin/pull/27609#issuecomment-1546603600)
>covers Lightning since AFAIK you won't make commitment transactions that spend each other
That's true. For anchor commitments I think it is the case that all outputs except the anchors (main balance outputs and htlcs) are timelocked CSV 1, so they aren't even available until after confirmation.
💬 joostjager commented on pull request "rpc: allow submitpackage to be called outside of regtest":
(https://github.com/bitcoin/bitcoin/pull/27609#issuecomment-1546604107)
>The worst case is the miner accepts transactions they actually should reject.
If I understood the various threads here and in other prs correctly, I think the consequence of this is that the mempool will be sub-optimal and lead to a block template that earns fewer fees for the miner exposing the endpoint?
If this would happen, I suppose the miner can always close the endpoint again and wait for the next release that is more strict on the "child-with-parents tree only" packages (while also
...
(https://github.com/bitcoin/bitcoin/pull/27609#issuecomment-1546604107)
>The worst case is the miner accepts transactions they actually should reject.
If I understood the various threads here and in other prs correctly, I think the consequence of this is that the mempool will be sub-optimal and lead to a block template that earns fewer fees for the miner exposing the endpoint?
If this would happen, I suppose the miner can always close the endpoint again and wait for the next release that is more strict on the "child-with-parents tree only" packages (while also
...
👋 MarcoFalke's pull request is ready for review: "ci: Run iwyu on all src files"
(https://github.com/bitcoin/bitcoin/pull/27571)
(https://github.com/bitcoin/bitcoin/pull/27571)
💬 brunoerg commented on pull request "fuzz: wallet, add target for `fees`":
(https://github.com/bitcoin/bitcoin/pull/27647#discussion_r1192971964)
In fact, there is no need to create it for each iteration, gonna change it. Thanks.
(https://github.com/bitcoin/bitcoin/pull/27647#discussion_r1192971964)
In fact, there is no need to create it for each iteration, gonna change it. Thanks.
💬 brunoerg commented on pull request "fuzz: wallet, add target for `fees`":
(https://github.com/bitcoin/bitcoin/pull/27647#issuecomment-1546633637)
Force-pushed addressing @MarcoFalke's review.
- Changed `wallet` to be static
- Changed format according to `clang-format`
(https://github.com/bitcoin/bitcoin/pull/27647#issuecomment-1546633637)
Force-pushed addressing @MarcoFalke's review.
- Changed `wallet` to be static
- Changed format according to `clang-format`
💬 sipa commented on pull request "Introduce field element and group element classes to test_framework/key.py":
(https://github.com/bitcoin/bitcoin/pull/26222#discussion_r1192972870)
Done.
(https://github.com/bitcoin/bitcoin/pull/26222#discussion_r1192972870)
Done.
💬 sipa commented on pull request "Introduce field element and group element classes to test_framework/key.py":
(https://github.com/bitcoin/bitcoin/pull/26222#discussion_r1192972961)
Done.
(https://github.com/bitcoin/bitcoin/pull/26222#discussion_r1192972961)
Done.
💬 sipa commented on pull request "Introduce field element and group element classes to test_framework/key.py":
(https://github.com/bitcoin/bitcoin/pull/26222#issuecomment-1546636677)
I've made a number of changes.
* (Done earlier) Made the `GE` class represent infinity explicitly.
* The new classes / code are in a new module `test_framework.secp256k1`, which removes the implementation details from `test_framework.key`, and also feels a bit better namespace-wise (`secp256k1.GE` is probably more informative to an unfamiliar reviewer than just `GE`, at least indicating it has something to do with elliptic curve cryptography).
* Dropped a number of unused functions / operat
...
(https://github.com/bitcoin/bitcoin/pull/26222#issuecomment-1546636677)
I've made a number of changes.
* (Done earlier) Made the `GE` class represent infinity explicitly.
* The new classes / code are in a new module `test_framework.secp256k1`, which removes the implementation details from `test_framework.key`, and also feels a bit better namespace-wise (`secp256k1.GE` is probably more informative to an unfamiliar reviewer than just `GE`, at least indicating it has something to do with elliptic curve cryptography).
* Dropped a number of unused functions / operat
...
💬 kroese commented on issue "v25.0 testing":
(https://github.com/bitcoin/bitcoin/issues/27621#issuecomment-1546651567)
I just downloaded v24rc3 for testing, but when I do:
`gpg --verify SHA256SUMS.asc SHA256SUMS`
it fails (exitcode 2). Normally on official releases this always worked fine.
So is it normal that release candidates are not signed? Or what could be the problem?
(https://github.com/bitcoin/bitcoin/issues/27621#issuecomment-1546651567)
I just downloaded v24rc3 for testing, but when I do:
`gpg --verify SHA256SUMS.asc SHA256SUMS`
it fails (exitcode 2). Normally on official releases this always worked fine.
So is it normal that release candidates are not signed? Or what could be the problem?
💬 martinus commented on pull request "init: add MALLOC_ARENA_MAX=1 to systemd":
(https://github.com/bitcoin/bitcoin/pull/27642#issuecomment-1546658892)
Now that #25325 has been merged, this might be less of an issue. This pool allocator significantly reduces the number of allocations used for `CCoinsMap`. (by about a factor of 2000).
(https://github.com/bitcoin/bitcoin/pull/27642#issuecomment-1546658892)
Now that #25325 has been merged, this might be less of an issue. This pool allocator significantly reduces the number of allocations used for `CCoinsMap`. (by about a factor of 2000).
💬 sangaman commented on pull request "init: add MALLOC_ARENA_MAX=1 to systemd":
(https://github.com/bitcoin/bitcoin/pull/27642#issuecomment-1546666636)
> Now that #25325 has been merged, this might be less of an issue. This pool allocator significantly reduces the number of allocations used for `CCoinsMap`. (by about a factor of 2000).
Thanks! I'll try that branch out and see if resolves the underlying issue without this env var, in which case I'd agree this PR is probably not necessary.
(https://github.com/bitcoin/bitcoin/pull/27642#issuecomment-1546666636)
> Now that #25325 has been merged, this might be less of an issue. This pool allocator significantly reduces the number of allocations used for `CCoinsMap`. (by about a factor of 2000).
Thanks! I'll try that branch out and see if resolves the underlying issue without this env var, in which case I'd agree this PR is probably not necessary.
💬 TheCharlatan commented on pull request "kernel: Remove interface_ui, util/system from kernel library":
(https://github.com/bitcoin/bitcoin/pull/27636#issuecomment-1546714913)
Thank you for the suggestions @ryanofsky!
Updated 0b159b9256c214a289135ca7bfaf70e48ab28e20 -> 63e915af5a437d36abdd79a1b90fed2395086589 ([chainstateRmKernelUiInterface_0](https://github.com/TheCharlatan/bitcoin/commits/chainstateRmKernelUiInterface_0) -> [chainstateRmKernelUiInterface_1](https://github.com/TheCharlatan/bitcoin/commits/chainstateRmKernelUiInterface_1), [compare](https://github.com/TheCharlatan/bitcoin/compare/chainstateRmKernelUiInterface_0..chainstateRmKernelUiInterface_1)).
...
(https://github.com/bitcoin/bitcoin/pull/27636#issuecomment-1546714913)
Thank you for the suggestions @ryanofsky!
Updated 0b159b9256c214a289135ca7bfaf70e48ab28e20 -> 63e915af5a437d36abdd79a1b90fed2395086589 ([chainstateRmKernelUiInterface_0](https://github.com/TheCharlatan/bitcoin/commits/chainstateRmKernelUiInterface_0) -> [chainstateRmKernelUiInterface_1](https://github.com/TheCharlatan/bitcoin/commits/chainstateRmKernelUiInterface_1), [compare](https://github.com/TheCharlatan/bitcoin/compare/chainstateRmKernelUiInterface_0..chainstateRmKernelUiInterface_1)).
...
⚠️ kroese opened an issue: "Allow getblockfrompeer to use any peer"
(https://github.com/bitcoin/bitcoin/issues/27652)
### Please describe the feature you'd like to see added.
When you use `getblockfrompeer` you have to specify an index of a specific peer. But it would be much easier if it allowed you to use ANY peer.
For example, if you run a pruned node, and your first peer happens to be also pruned, the call will fail. If the call tried all nodes, until it finds a peer that has the block, it wouldn't require you that implement any logic for that.
### Is your feature related to a problem, if so please des
...
(https://github.com/bitcoin/bitcoin/issues/27652)
### Please describe the feature you'd like to see added.
When you use `getblockfrompeer` you have to specify an index of a specific peer. But it would be much easier if it allowed you to use ANY peer.
For example, if you run a pruned node, and your first peer happens to be also pruned, the call will fail. If the call tried all nodes, until it finds a peer that has the block, it wouldn't require you that implement any logic for that.
### Is your feature related to a problem, if so please des
...
💬 kroese commented on issue "Allow getblockfrompeer to use any peer":
(https://github.com/bitcoin/bitcoin/issues/27652#issuecomment-1546731833)
Maybe @Sjors can look into this?
(https://github.com/bitcoin/bitcoin/issues/27652#issuecomment-1546731833)
Maybe @Sjors can look into this?
💬 furszy commented on issue "Allow getblockfrompeer to use any peer":
(https://github.com/bitcoin/bitcoin/issues/27652#issuecomment-1546733957)
> For example, if you run a pruned node, and your first peer happens to be also pruned, the call will fail. If the call tried all nodes, until it finds a peer that has the block, it wouldn't require you that implement any logic for that.
Based on your words, guess that you are calling `getblockfrompeer` with a fixed index of `1`?
You could instead use `getpeerinfo` and filter the result by service. Only requesting the block to another full node.
(https://github.com/bitcoin/bitcoin/issues/27652#issuecomment-1546733957)
> For example, if you run a pruned node, and your first peer happens to be also pruned, the call will fail. If the call tried all nodes, until it finds a peer that has the block, it wouldn't require you that implement any logic for that.
Based on your words, guess that you are calling `getblockfrompeer` with a fixed index of `1`?
You could instead use `getpeerinfo` and filter the result by service. Only requesting the block to another full node.
💬 kroese commented on issue "Allow getblockfrompeer to use any peer":
(https://github.com/bitcoin/bitcoin/issues/27652#issuecomment-1546736533)
> Based on your words, guess that you are calling `getblockfrompeer` with a fixed index of `1`?
> You could instead use `getpeerinfo` and filter the result by service. Only requesting the block to another full node.
I'm aware you could workaround this issue by implementing a bunch of manual logic on the client-side. But even if you skip the pruned nodes and this full node couldn't provide you the block, it would be handy if the call would just try the next one.
(https://github.com/bitcoin/bitcoin/issues/27652#issuecomment-1546736533)
> Based on your words, guess that you are calling `getblockfrompeer` with a fixed index of `1`?
> You could instead use `getpeerinfo` and filter the result by service. Only requesting the block to another full node.
I'm aware you could workaround this issue by implementing a bunch of manual logic on the client-side. But even if you skip the pruned nodes and this full node couldn't provide you the block, it would be handy if the call would just try the next one.