π¬ maflcko commented on issue "test: wallet_reorgsrestore.py failure under valgrind":
(https://github.com/bitcoin/bitcoin/issues/32493#issuecomment-2879940657)
Does it always happen, or did this start at some point in the git history?
(https://github.com/bitcoin/bitcoin/issues/32493#issuecomment-2879940657)
Does it always happen, or did this start at some point in the git history?
π laanwj approved a pull request: "cmake: Add application manifests when cross-compiling for Windows"
(https://github.com/bitcoin/bitcoin/pull/32396#pullrequestreview-2839952072)
re-ACK c0f41523973e33a8cd104a8ebf4906a4d99f3eed
(https://github.com/bitcoin/bitcoin/pull/32396#pullrequestreview-2839952072)
re-ACK c0f41523973e33a8cd104a8ebf4906a4d99f3eed
π¬ fanquake commented on pull request "test: fix two intermittent failures in wallet_basic.py":
(https://github.com/bitcoin/bitcoin/pull/32483#issuecomment-2880021915)
@achow101 note that you've ACK'd a commit has not in this PR, but for an equivalent change.
(https://github.com/bitcoin/bitcoin/pull/32483#issuecomment-2880021915)
@achow101 note that you've ACK'd a commit has not in this PR, but for an equivalent change.
β
fanquake closed an issue: "failure in wallet_basic.py --descriptors"
(https://github.com/bitcoin/bitcoin/issues/27249)
(https://github.com/bitcoin/bitcoin/issues/27249)
β
fanquake closed an issue: "qa: Failure in wallet_basic.py spendzeroconfchange test"
(https://github.com/bitcoin/bitcoin/issues/32456)
(https://github.com/bitcoin/bitcoin/issues/32456)
π fanquake merged a pull request: "test: fix two intermittent failures in wallet_basic.py"
(https://github.com/bitcoin/bitcoin/pull/32483)
(https://github.com/bitcoin/bitcoin/pull/32483)
π¬ l0rinc commented on pull request "refactor: reenable `implicit-integer-sign-change` check for `serialize.h`":
(https://github.com/bitcoin/bitcoin/pull/32296#discussion_r2088818525)
Nice, I like that a lot more - addressed the reformatting and unification in separate commits, let me know if you think this is better.
Pushed a rebase separately to simplify review from GitHub as well (I know you do range-diff), and to make sure the latest CI is run against it.
(https://github.com/bitcoin/bitcoin/pull/32296#discussion_r2088818525)
Nice, I like that a lot more - addressed the reformatting and unification in separate commits, let me know if you think this is better.
Pushed a rebase separately to simplify review from GitHub as well (I know you do range-diff), and to make sure the latest CI is run against it.
π¬ fanquake commented on pull request "test: fix two intermittent failures in wallet_basic.py":
(https://github.com/bitcoin/bitcoin/pull/32483#issuecomment-2880037334)
Backported to `29.x` in #32292.
(https://github.com/bitcoin/bitcoin/pull/32483#issuecomment-2880037334)
Backported to `29.x` in #32292.
β οΈ Sjors opened an issue: "Dropping unused legacy wallet code"
(https://github.com/bitcoin-core/gui/issues/873)
Tracking pull request(s) that remove any remaining legacy wallet code, either here or in the main repo:
- https://github.com/bitcoin/bitcoin/pull/32459
(https://github.com/bitcoin-core/gui/issues/873)
Tracking pull request(s) that remove any remaining legacy wallet code, either here or in the main repo:
- https://github.com/bitcoin/bitcoin/pull/32459
π¬ luke-jr commented on pull request "[DRAFT] Expose AssumeUTXO Load Snapshot Functionality To The GUI":
(https://github.com/bitcoin-core/gui/pull/870#issuecomment-2880052845)
Seems like this would be more appropriate for the first-run intro page?
(https://github.com/bitcoin-core/gui/pull/870#issuecomment-2880052845)
Seems like this would be more appropriate for the first-run intro page?
π¬ Sjors commented on pull request "qt: drop unused watch-only functionality":
(https://github.com/bitcoin/bitcoin/pull/32459#issuecomment-2880054651)
Doesn't seem worth moving, and this PR also touches the interface, but I opened a tracking issue on the GUI repo that points here. There's probably more than what I found here.
(https://github.com/bitcoin/bitcoin/pull/32459#issuecomment-2880054651)
Doesn't seem worth moving, and this PR also touches the interface, but I opened a tracking issue on the GUI repo that points here. There's probably more than what I found here.
π¬ maflcko commented on pull request "refactor: reenable `implicit-integer-sign-change` check for `serialize.h`":
(https://github.com/bitcoin/bitcoin/pull/32296#discussion_r2088837960)
nit in the second commit: Would be good to just set this to the final version `template<typename Stream> void Serialize(Stream& s, int8_t a ) { ser_writedata8(s, uint8_t(a)); }` from the beginning. Otherwise there will be three style-fixup commits on top of each other, making review harder and the git log and git blame depth tedious to follow.
(https://github.com/bitcoin/bitcoin/pull/32296#discussion_r2088837960)
nit in the second commit: Would be good to just set this to the final version `template<typename Stream> void Serialize(Stream& s, int8_t a ) { ser_writedata8(s, uint8_t(a)); }` from the beginning. Otherwise there will be three style-fixup commits on top of each other, making review harder and the git log and git blame depth tedious to follow.
π¬ maflcko commented on issue "Dropping unused legacy wallet code":
(https://github.com/bitcoin-core/gui/issues/873#issuecomment-2880070206)
No objection, but not sure a tracking issue is needed for a single, simple pull request?
(https://github.com/bitcoin-core/gui/issues/873#issuecomment-2880070206)
No objection, but not sure a tracking issue is needed for a single, simple pull request?
π¬ TheCharlatan commented on pull request "kernel: Separate UTXO set access from validation functions":
(https://github.com/bitcoin/bitcoin/pull/32317#discussion_r2088847208)
Sweet, will apply this!
(https://github.com/bitcoin/bitcoin/pull/32317#discussion_r2088847208)
Sweet, will apply this!
π¬ Sjors commented on issue "Dropping unused legacy wallet code":
(https://github.com/bitcoin-core/gui/issues/873#issuecomment-2880140236)
I don't think I found all the code, regular GUI contributors might find more.
(https://github.com/bitcoin-core/gui/issues/873#issuecomment-2880140236)
I don't think I found all the code, regular GUI contributors might find more.
π¬ maflcko commented on issue "Dropping unused legacy wallet code":
(https://github.com/bitcoin-core/gui/issues/873#issuecomment-2880157568)
I think they can just open a new issue or pull request, if they find something? I guess it can't hurt to open/close this issue as a notification, but absent any bug or any concrete work to do, I am not sure what needs to be done here and when this can be closed.
(https://github.com/bitcoin-core/gui/issues/873#issuecomment-2880157568)
I think they can just open a new issue or pull request, if they find something? I guess it can't hurt to open/close this issue as a notification, but absent any bug or any concrete work to do, I am not sure what needs to be done here and when this can be closed.
π l0rinc opened a pull request: "merkle: preβreserve leaves to prevent reallocs with odd vtx count"
(https://github.com/bitcoin/bitcoin/pull/32497)
### Summary
`ComputeMerkleRoot` duplicates the last hash when the input size is odd:
https://github.com/bitcoin/bitcoin/blob/39b6c139bd6be33699af781f1d71f6fed303d468/src/consensus/merkle.cpp#L54-L56
If the caller gave it a `std::vector` whose capacity equals its size (the common case when the vector was created with `resize()`), that single `push_back` forces a reallocation (doubling its size).
We pay this penalty on every Merkleβroot calculation even though we know the final size in advance
...
(https://github.com/bitcoin/bitcoin/pull/32497)
### Summary
`ComputeMerkleRoot` duplicates the last hash when the input size is odd:
https://github.com/bitcoin/bitcoin/blob/39b6c139bd6be33699af781f1d71f6fed303d468/src/consensus/merkle.cpp#L54-L56
If the caller gave it a `std::vector` whose capacity equals its size (the common case when the vector was created with `resize()`), that single `push_back` forces a reallocation (doubling its size).
We pay this penalty on every Merkleβroot calculation even though we know the final size in advance
...
π¬ ismaelsadeeq commented on pull request "interfaces: refactor: move `Mining` and `BlockTemplate` implementation to miner":
(https://github.com/bitcoin/bitcoin/pull/32378#issuecomment-2880169560)
@Sjors I've taken your suggestion in this push [0d774ab59..62fc42d475](https://github.com/bitcoin/bitcoin/compare/51344920ebe1772d8dd3bece789a6510d774ab59..62fc42d475df4f23bd93313f95ee7b7eb0d4683f)
If the `GetTip` method from the mining interface isn't going to be used by the TP, it can be deleted as well?
(https://github.com/bitcoin/bitcoin/pull/32378#issuecomment-2880169560)
@Sjors I've taken your suggestion in this push [0d774ab59..62fc42d475](https://github.com/bitcoin/bitcoin/compare/51344920ebe1772d8dd3bece789a6510d774ab59..62fc42d475df4f23bd93313f95ee7b7eb0d4683f)
If the `GetTip` method from the mining interface isn't going to be used by the TP, it can be deleted as well?
π¬ Sjors commented on issue "Dropping unused legacy wallet code":
(https://github.com/bitcoin-core/gui/issues/873#issuecomment-2880178818)
In that case I can close this once https://github.com/bitcoin/bitcoin/pull/32459 is merged, unless there's another issue found by then.
(https://github.com/bitcoin-core/gui/issues/873#issuecomment-2880178818)
In that case I can close this once https://github.com/bitcoin/bitcoin/pull/32459 is merged, unless there's another issue found by then.
π¬ l0rinc commented on pull request "refactor: reenable `implicit-integer-sign-change` check for `serialize.h`":
(https://github.com/bitcoin/bitcoin/pull/32296#discussion_r2088900114)
we could, but I would be mixing changes in the same commit - the commit message wouldn't represent the behavior. Or you mean I should just squash the last few changes as "(un)serialization consistency"?
(https://github.com/bitcoin/bitcoin/pull/32296#discussion_r2088900114)
we could, but I would be mixing changes in the same commit - the commit message wouldn't represent the behavior. Or you mean I should just squash the last few changes as "(un)serialization consistency"?