💬 glozow commented on issue "Voting on Priority Projects for 28.0":
(https://github.com/bitcoin/bitcoin/issues/29465#issuecomment-1959579543)
- cluster mempool
- package relay
(https://github.com/bitcoin/bitcoin/issues/29465#issuecomment-1959579543)
- cluster mempool
- package relay
💬 gdiscord commented on issue "Guix build script incorrectly reporting there is no Mac SDK":
(https://github.com/bitcoin/bitcoin/issues/29449#issuecomment-1959584098)
The part it comes from is indeed at the configure stage.
Depends build complete successfully, then ./autogen.sh excuted, after which ./configure is called, then the error occurs:
This is the command I ran:
env HOSTS="x86_64-linux-gnu x86_64-w64-mingw32" SOURCES_PATH="$HOME/depends-SOURCES_PATH" BASE_CACHE="$HOME/depends-BASE_CACHE" SDK_PATH="$HOME/SDKs" FORCE_DIRTY_WORKTREE=1 V=1 ./contrib/guix/guix-build
here's the last several lines leading to the error
--------
checking whether to b
...
(https://github.com/bitcoin/bitcoin/issues/29449#issuecomment-1959584098)
The part it comes from is indeed at the configure stage.
Depends build complete successfully, then ./autogen.sh excuted, after which ./configure is called, then the error occurs:
This is the command I ran:
env HOSTS="x86_64-linux-gnu x86_64-w64-mingw32" SOURCES_PATH="$HOME/depends-SOURCES_PATH" BASE_CACHE="$HOME/depends-BASE_CACHE" SDK_PATH="$HOME/SDKs" FORCE_DIRTY_WORKTREE=1 V=1 ./contrib/guix/guix-build
here's the last several lines leading to the error
--------
checking whether to b
...
💬 Sjors commented on pull request "Double -dbache maximum to 64GB":
(https://github.com/bitcoin/bitcoin/pull/28358#issuecomment-1959590198)
We could expose whether the program is compiled for 32 or 64 bit in the test framework... but I just added a regex for the error message instead.
(https://github.com/bitcoin/bitcoin/pull/28358#issuecomment-1959590198)
We could expose whether the program is compiled for 32 or 64 bit in the test framework... but I just added a regex for the error message instead.
💬 fanquake commented on issue "Guix build script incorrectly reporting there is no Mac SDK":
(https://github.com/bitcoin/bitcoin/issues/29449#issuecomment-1959592450)
> Building main branch.
> checking for openssl/crypto.h... no
configure: error: libcrypto headers missing
This is impossible given the output you are showing here. Where did you get the source from. What is the commit hash? etc.
(https://github.com/bitcoin/bitcoin/issues/29449#issuecomment-1959592450)
> Building main branch.
> checking for openssl/crypto.h... no
configure: error: libcrypto headers missing
This is impossible given the output you are showing here. Where did you get the source from. What is the commit hash? etc.
💬 instagibbs commented on pull request "p2p: Don't process mutated blocks":
(https://github.com/bitcoin/bitcoin/pull/29412#discussion_r1499355541)
have you thought about having this function return an optional error string so unit tests can check expected failure reason?
(https://github.com/bitcoin/bitcoin/pull/29412#discussion_r1499355541)
have you thought about having this function return an optional error string so unit tests can check expected failure reason?
💬 instagibbs commented on pull request "p2p: Don't process mutated blocks":
(https://github.com/bitcoin/bitcoin/pull/29412#discussion_r1499351311)
Asserting the stripped serialized size right after is nice for self-documentation/correctness of test, and maybe another case of a witness tx which witness size is not 64?
(https://github.com/bitcoin/bitcoin/pull/29412#discussion_r1499351311)
Asserting the stripped serialized size right after is nice for self-documentation/correctness of test, and maybe another case of a witness tx which witness size is not 64?
💬 gdiscord commented on issue "Guix build script incorrectly reporting there is no Mac SDK":
(https://github.com/bitcoin/bitcoin/issues/29449#issuecomment-1959597424)
fyi: I've also tried doing a gitian build, and though the build was not successful( expected that because I have not taken the time to properly configure gitian descriptors) , this particular error did not occur.
Will revisit gitian build at some point.
Now my main goal is to achieve a successful guix build.
(https://github.com/bitcoin/bitcoin/issues/29449#issuecomment-1959597424)
fyi: I've also tried doing a gitian build, and though the build was not successful( expected that because I have not taken the time to properly configure gitian descriptors) , this particular error did not occur.
Will revisit gitian build at some point.
Now my main goal is to achieve a successful guix build.
💬 instagibbs commented on pull request "[WIP] p2p: opportunistically accept 1-parent-1-child packages":
(https://github.com/bitcoin/bitcoin/pull/28970#issuecomment-1959598347)
ready for un-draft?
(https://github.com/bitcoin/bitcoin/pull/28970#issuecomment-1959598347)
ready for un-draft?
💬 gdiscord commented on issue "Guix build script incorrectly reporting there is no Mac SDK":
(https://github.com/bitcoin/bitcoin/issues/29449#issuecomment-1959618269)
Pulled the main branch, but I've experimented a lot with the code.
I can do a clean bitcoin pull and build it untouched to see what happens.
(https://github.com/bitcoin/bitcoin/issues/29449#issuecomment-1959618269)
Pulled the main branch, but I've experimented a lot with the code.
I can do a clean bitcoin pull and build it untouched to see what happens.
💬 sdaftuar commented on pull request "policy: enable sibling eviction for v3 transactions":
(https://github.com/bitcoin/bitcoin/pull/29306#issuecomment-1959678050)
ACK apart from @instagibbs' nit here: https://github.com/bitcoin/bitcoin/pull/29306#discussion_r1498102589
(https://github.com/bitcoin/bitcoin/pull/29306#issuecomment-1959678050)
ACK apart from @instagibbs' nit here: https://github.com/bitcoin/bitcoin/pull/29306#discussion_r1498102589
💬 luke-jr commented on pull request "rpc: getdescriptorinfo also returns normalized descriptor":
(https://github.com/bitcoin/bitcoin/pull/29396#issuecomment-1959724149)
>For the case of hardened derivation path...
...it's documented herein as working :\
(https://github.com/bitcoin/bitcoin/pull/29396#issuecomment-1959724149)
>For the case of hardened derivation path...
...it's documented herein as working :\
💬 sdaftuar commented on pull request "Mempool util: Add RBF diagram checks for single chunks against clusters of size 2":
(https://github.com/bitcoin/bitcoin/pull/29242#discussion_r1499465556)
I think the slope of AB can be negative, because a user could use the `prioritisetransaction` to give a transaction a negative fee?
(https://github.com/bitcoin/bitcoin/pull/29242#discussion_r1499465556)
I think the slope of AB can be negative, because a user could use the `prioritisetransaction` to give a transaction a negative fee?
💬 instagibbs commented on pull request "Mempool util: Add RBF diagram checks for single chunks against clusters of size 2":
(https://github.com/bitcoin/bitcoin/pull/29242#discussion_r1499472337)
one weird trick! will remove the comment
(https://github.com/bitcoin/bitcoin/pull/29242#discussion_r1499472337)
one weird trick! will remove the comment
🤔 sipa reviewed a pull request: "Mempool util: Add RBF diagram checks for single chunks against clusters of size 2"
(https://github.com/bitcoin/bitcoin/pull/29242#pullrequestreview-1893687911)
Partial review.
(https://github.com/bitcoin/bitcoin/pull/29242#pullrequestreview-1893687911)
Partial review.
💬 sipa commented on pull request "Mempool util: Add RBF diagram checks for single chunks against clusters of size 2":
(https://github.com/bitcoin/bitcoin/pull/29242#discussion_r1497886010)
We have `Shuffle()` in random.h, which is more efficient than `std::shuffle` (but only works with `FastRandomContext`).
(https://github.com/bitcoin/bitcoin/pull/29242#discussion_r1497886010)
We have `Shuffle()` in random.h, which is more efficient than `std::shuffle` (but only works with `FastRandomContext`).
💬 sipa commented on pull request "Mempool util: Add RBF diagram checks for single chunks against clusters of size 2":
(https://github.com/bitcoin/bitcoin/pull/29242#discussion_r1497917895)
Nit: no need to reimplement `FeeFrac` addition:
```
diagram.push_back(last + chunk);
```
(https://github.com/bitcoin/bitcoin/pull/29242#discussion_r1497917895)
Nit: no need to reimplement `FeeFrac` addition:
```
diagram.push_back(last + chunk);
```
💬 sipa commented on pull request "Mempool util: Add RBF diagram checks for single chunks against clusters of size 2":
(https://github.com/bitcoin/bitcoin/pull/29242#discussion_r1497874754)
The correctness of the result of `BuildDiagramFromUnsortedChunks` very much depends on the assumption that the chunks can be reordered w.r.t. one another, which seems to be the case in the current PR, but will very much not be the case going forward in a general cluster mempool world.
I would suggest either:
* Removing the sorting step in this function (moving it to the caller). The function can then take `Span<const FeeFrac>` as input too, and perhaps in a further iteration (in a later PR)
...
(https://github.com/bitcoin/bitcoin/pull/29242#discussion_r1497874754)
The correctness of the result of `BuildDiagramFromUnsortedChunks` very much depends on the assumption that the chunks can be reordered w.r.t. one another, which seems to be the case in the current PR, but will very much not be the case going forward in a general cluster mempool world.
I would suggest either:
* Removing the sorting step in this function (moving it to the caller). The function can then take `Span<const FeeFrac>` as input too, and perhaps in a further iteration (in a later PR)
...
💬 mzumsande commented on pull request "net: call `Select` with reachable networks in `ThreadOpenConnections`":
(https://github.com/bitcoin/bitcoin/pull/29436#issuecomment-1959775549)
> the performance goes down by a factor of ~5 for me
If this is correct and can be reproduced by others, I think that something should be done about it. If this PR would increase performance for a rare and transient case (right after the users switched their onlynet parameters) but decrease performance for each `Select()` call in the typical scenario (addrman has mostly addrs from reachable networks), the overall impact on performance of this PR would be clearly negative.
One possibility mi
...
(https://github.com/bitcoin/bitcoin/pull/29436#issuecomment-1959775549)
> the performance goes down by a factor of ~5 for me
If this is correct and can be reproduced by others, I think that something should be done about it. If this PR would increase performance for a rare and transient case (right after the users switched their onlynet parameters) but decrease performance for each `Select()` call in the typical scenario (addrman has mostly addrs from reachable networks), the overall impact on performance of this PR would be clearly negative.
One possibility mi
...
💬 sdaftuar commented on pull request "Mempool util: Add RBF diagram checks for single chunks against clusters of size 2":
(https://github.com/bitcoin/bitcoin/pull/29242#discussion_r1499507266)
> The correctness of the result of `BuildDiagramFromUnsortedChunks` very much depends on the assumption that the chunks can be reordered w.r.t. one another, which seems to be the case in the current PR, but will very much not be the case going forward in a general cluster mempool world.
No objection to your suggested change here, but I don't follow this comment -- in a general cluster mempool world, unless we were to bound chunk sizes to something smaller than a cluster size (resulting in the
...
(https://github.com/bitcoin/bitcoin/pull/29242#discussion_r1499507266)
> The correctness of the result of `BuildDiagramFromUnsortedChunks` very much depends on the assumption that the chunks can be reordered w.r.t. one another, which seems to be the case in the current PR, but will very much not be the case going forward in a general cluster mempool world.
No objection to your suggested change here, but I don't follow this comment -- in a general cluster mempool world, unless we were to bound chunk sizes to something smaller than a cluster size (resulting in the
...
⚠️ RHavar opened an issue: "rpc method removeprunedfunds should take an array of txids"
(https://github.com/bitcoin/bitcoin/issues/29466)
### Please describe the feature you'd like to see added.
Currently removeprunedfunds only takes a single txid, however the underlying `CWallet::RemoveTxs` takes an array. I am trying to clean up some wallets, and each individual removeprunedfunds call is very slow. So it'd be a lot better to just be able to pass in all the transactions at once
### Is your feature related to a problem, if so please describe it.
_No response_
### Describe the solution you'd like
_No response_
### Describe a
...
(https://github.com/bitcoin/bitcoin/issues/29466)
### Please describe the feature you'd like to see added.
Currently removeprunedfunds only takes a single txid, however the underlying `CWallet::RemoveTxs` takes an array. I am trying to clean up some wallets, and each individual removeprunedfunds call is very slow. So it'd be a lot better to just be able to pass in all the transactions at once
### Is your feature related to a problem, if so please describe it.
_No response_
### Describe the solution you'd like
_No response_
### Describe a
...
👍 hebasto approved a pull request: "lint: Check for missing bitcoin-config.h includes"
(https://github.com/bitcoin/bitcoin/pull/29408#pullrequestreview-1896313003)
ACK fa58ae74ea67485495dbc2d003adfbca68d6869b, tested on Ubuntu 23.10 -- it catches bugs properly. I didn't review rust code changes.
(https://github.com/bitcoin/bitcoin/pull/29408#pullrequestreview-1896313003)
ACK fa58ae74ea67485495dbc2d003adfbca68d6869b, tested on Ubuntu 23.10 -- it catches bugs properly. I didn't review rust code changes.