💬 Eunovo commented on pull request "wallet: fix unrelated parent conflict doesn't cause child tx to be marked as conflict":
(https://github.com/bitcoin/bitcoin/pull/29680#discussion_r1625155299)
@josibake Deleting the nullptr constructor worked.
(https://github.com/bitcoin/bitcoin/pull/29680#discussion_r1625155299)
@josibake Deleting the nullptr constructor worked.
💬 Eunovo commented on pull request "wallet: fix unrelated parent conflict doesn't cause child tx to be marked as conflict":
(https://github.com/bitcoin/bitcoin/pull/29680#issuecomment-2146288055)
Thanks for the reviews @josibake and @furszy I have implemented your suggested changes
(https://github.com/bitcoin/bitcoin/pull/29680#issuecomment-2146288055)
Thanks for the reviews @josibake and @furszy I have implemented your suggested changes
📝 Eunovo converted_to_draft a pull request: "wallet: fix unrelated parent conflict doesn't cause child tx to be marked as conflict"
(https://github.com/bitcoin/bitcoin/pull/29680)
This PR implements a fix for the issue described in https://github.com/bitcoin/bitcoin/issues/29435.
The problem is that the wallet is unable to abandon transactions that have unrelated parent conflicts. The solution implemented here, augments the mempool transaction `REPLACED` signal with the double-spending transaction which the wallet stores and watches for in Block notifications. A map is added to the wallet to track conflicting tx ids and their child transactions. The entry is erased wh
...
(https://github.com/bitcoin/bitcoin/pull/29680)
This PR implements a fix for the issue described in https://github.com/bitcoin/bitcoin/issues/29435.
The problem is that the wallet is unable to abandon transactions that have unrelated parent conflicts. The solution implemented here, augments the mempool transaction `REPLACED` signal with the double-spending transaction which the wallet stores and watches for in Block notifications. A map is added to the wallet to track conflicting tx ids and their child transactions. The entry is erased wh
...
🚀 fanquake merged a pull request: "depends: Update Boost download link"
(https://github.com/bitcoin/bitcoin/pull/30217)
(https://github.com/bitcoin/bitcoin/pull/30217)
✅ maflcko closed an issue: "Improve/simplify node sync for pruned nodes"
(https://github.com/bitcoin/bitcoin/issues/30220)
(https://github.com/bitcoin/bitcoin/issues/30220)
💬 maflcko commented on issue "Improve/simplify node sync for pruned nodes":
(https://github.com/bitcoin/bitcoin/issues/30220#issuecomment-2146978830)
This is possible to implement, but may be controversial, depending on how it is implemented. You can learn more about the topic by reading the various discussions around assumeutxo.
Usually the issue tracker is used to track technical issues related to the Bitcoin Core code base.
General bitcoin questions and/or support requests are best directed to the [Bitcoin StackExchange](https://bitcoin.stackexchange.com) or the `#bitcoin` IRC channel on Libera Chat, or one of the Bitcoin subreddits,
...
(https://github.com/bitcoin/bitcoin/issues/30220#issuecomment-2146978830)
This is possible to implement, but may be controversial, depending on how it is implemented. You can learn more about the topic by reading the various discussions around assumeutxo.
Usually the issue tracker is used to track technical issues related to the Bitcoin Core code base.
General bitcoin questions and/or support requests are best directed to the [Bitcoin StackExchange](https://bitcoin.stackexchange.com) or the `#bitcoin` IRC channel on Libera Chat, or one of the Bitcoin subreddits,
...
💬 hebasto commented on pull request "feefrac: 128-bit multiply support in MSVC":
(https://github.com/bitcoin/bitcoin/pull/29758#issuecomment-2147006298)
I've add a [benchmark](https://github.com/hebasto/bitcoin/commit/0f5abaf980122cce1bd0d405d17db3bfab273afa) for the `FeeRateCompare` function, which calls `Mul` twice.
Here are results on Windows 11, `Release` configuration, which implies `/O2 /Oi` compile flags:
- master branch @ 61de64df6790077857faba84796bb874b59c5d15:
```
> .\build_msvc\master\bench_bitcoin.exe -filter=FeefracMultipication
| ns/op | op/s | err% | total | benchmark
|---------------
...
(https://github.com/bitcoin/bitcoin/pull/29758#issuecomment-2147006298)
I've add a [benchmark](https://github.com/hebasto/bitcoin/commit/0f5abaf980122cce1bd0d405d17db3bfab273afa) for the `FeeRateCompare` function, which calls `Mul` twice.
Here are results on Windows 11, `Release` configuration, which implies `/O2 /Oi` compile flags:
- master branch @ 61de64df6790077857faba84796bb874b59c5d15:
```
> .\build_msvc\master\bench_bitcoin.exe -filter=FeefracMultipication
| ns/op | op/s | err% | total | benchmark
|---------------
...
💬 maflcko commented on pull request "guix: Use LTO to build releases":
(https://github.com/bitcoin/bitcoin/pull/25391#issuecomment-2147129097)
> Is this safe to do in light of https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105469 ?
Also https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113359 (in combination with https://github.com/bitcoin/bitcoin/pull/29881)
Would be nice to extend the motivation (pull request description) a bit about the benefits and risks.
(https://github.com/bitcoin/bitcoin/pull/25391#issuecomment-2147129097)
> Is this safe to do in light of https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105469 ?
Also https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113359 (in combination with https://github.com/bitcoin/bitcoin/pull/29881)
Would be nice to extend the motivation (pull request description) a bit about the benefits and risks.
💬 TheCharlatan commented on pull request "indexes: Don't wipe indexes again when continuing a prior reindex":
(https://github.com/bitcoin/bitcoin/pull/30132#issuecomment-2147147524)
Re https://github.com/bitcoin/bitcoin/pull/30132#pullrequestreview-2094875458
> But I also think it was a mistake in https://github.com/bitcoin/bitcoin/pull/29817 to add the BlockManagerOpts::reindex option, and think it would be better to have a less confusing set of options. The following change built on top of this PR could provide a simpler alternative: https://github.com/bitcoin/bitcoin/commit/9c643e7ddd82523b84f65b614d3e6c1f640b23c7. Feel free to use any of these changes, or they could
...
(https://github.com/bitcoin/bitcoin/pull/30132#issuecomment-2147147524)
Re https://github.com/bitcoin/bitcoin/pull/30132#pullrequestreview-2094875458
> But I also think it was a mistake in https://github.com/bitcoin/bitcoin/pull/29817 to add the BlockManagerOpts::reindex option, and think it would be better to have a less confusing set of options. The following change built on top of this PR could provide a simpler alternative: https://github.com/bitcoin/bitcoin/commit/9c643e7ddd82523b84f65b614d3e6c1f640b23c7. Feel free to use any of these changes, or they could
...
📝 fanquake opened a pull request: "[27.x] rc2 or final"
(https://github.com/bitcoin/bitcoin/pull/30222)
Backports:
* https://github.com/bitcoin/bitcoin/pull/30216
* https://github.com/bitcoin/bitcoin/pull/30217
I don't think either of these changes warrants an `rc2` cycle.
Depending on what else is added, this could turn into final.
(https://github.com/bitcoin/bitcoin/pull/30222)
Backports:
* https://github.com/bitcoin/bitcoin/pull/30216
* https://github.com/bitcoin/bitcoin/pull/30217
I don't think either of these changes warrants an `rc2` cycle.
Depending on what else is added, this could turn into final.
💬 fanquake commented on pull request "depends: Update Boost download link":
(https://github.com/bitcoin/bitcoin/pull/30217#issuecomment-2147211651)
Backported in #30222.
(https://github.com/bitcoin/bitcoin/pull/30217#issuecomment-2147211651)
Backported in #30222.
💬 fanquake commented on pull request "build: Fix building `fuzz` binary on on SunOS / illumos":
(https://github.com/bitcoin/bitcoin/pull/30216#issuecomment-2147211876)
Backported to 27.x in #30222.
(https://github.com/bitcoin/bitcoin/pull/30216#issuecomment-2147211876)
Backported to 27.x in #30222.
💬 hebasto commented on pull request "feefrac: 128-bit multiply support in MSVC":
(https://github.com/bitcoin/bitcoin/pull/29758#discussion_r1625782590)
> on MSVC one needs to use the `_mul128` or `_mulh` intrinsics instead
From my research, it follows that [`__mulh`](https://learn.microsoft.com/en-us/cpp/intrinsics/mulh) seems more performant.
This code:
```suggestion
// On 64-bit MSVC, use __mulh intrinsic for wide multiplication.
return {__mulh(a, b), std::bit_cast<uint64_t>(a) * b};
```
gives the following numbers for the [benchmark](https://github.com/bitcoin/bitcoin/pull/29758#issuecomment-2147006298):
```
> bu
...
(https://github.com/bitcoin/bitcoin/pull/29758#discussion_r1625782590)
> on MSVC one needs to use the `_mul128` or `_mulh` intrinsics instead
From my research, it follows that [`__mulh`](https://learn.microsoft.com/en-us/cpp/intrinsics/mulh) seems more performant.
This code:
```suggestion
// On 64-bit MSVC, use __mulh intrinsic for wide multiplication.
return {__mulh(a, b), std::bit_cast<uint64_t>(a) * b};
```
gives the following numbers for the [benchmark](https://github.com/bitcoin/bitcoin/pull/29758#issuecomment-2147006298):
```
> bu
...
👍 maflcko approved a pull request: "Lint: Support running individual lint checks"
(https://github.com/bitcoin/bitcoin/pull/30219#pullrequestreview-2095977332)
lgtm
(https://github.com/bitcoin/bitcoin/pull/30219#pullrequestreview-2095977332)
lgtm
💬 maflcko commented on pull request "Lint: Support running individual lint checks":
(https://github.com/bitcoin/bitcoin/pull/30219#discussion_r1625752646)
```suggestion
( cd ./test/lint/test_runner/ && RUST_BACKTRACE=1 cargo run -- --help )
```
(https://github.com/bitcoin/bitcoin/pull/30219#discussion_r1625752646)
```suggestion
( cd ./test/lint/test_runner/ && RUST_BACKTRACE=1 cargo run -- --help )
```
💬 stickies-v commented on pull request "indexes: Don't wipe indexes again when continuing a prior reindex":
(https://github.com/bitcoin/bitcoin/pull/30132#issuecomment-2147379245)
> The following change built on top of this PR could provide a simpler alternative: [9c643e7](https://github.com/bitcoin/bitcoin/commit/9c643e7ddd82523b84f65b614d3e6c1f640b23c7).
I like this suggestion. I think the naming is more clear in terms of the actual effect each variable has. I also think not having two `reindex` options is an improvement, the flow is more clear to me now.
I don't think `wipe_block_tree_db` (and a couple of other blockman options) should ultimately be a `Chainstate
...
(https://github.com/bitcoin/bitcoin/pull/30132#issuecomment-2147379245)
> The following change built on top of this PR could provide a simpler alternative: [9c643e7](https://github.com/bitcoin/bitcoin/commit/9c643e7ddd82523b84f65b614d3e6c1f640b23c7).
I like this suggestion. I think the naming is more clear in terms of the actual effect each variable has. I also think not having two `reindex` options is an improvement, the flow is more clear to me now.
I don't think `wipe_block_tree_db` (and a couple of other blockman options) should ultimately be a `Chainstate
...
👍 marcofleon approved a pull request: "fuzz: Make FuzzedSock fuzz friendlier"
(https://github.com/bitcoin/bitcoin/pull/30211#pullrequestreview-2096248644)
Tested ACK a7fceda68bb62fe3d9060fcf52e33b2f64a2acf9
In `i2p` the private key needs to be at least 387 bytes. When this key is generated (using FuzzedSock) the bytes were being broken up such that the length of the saved key was always less than 387. Having `Recv` return the length of a byte vector, instead of a single byte, when `MSG_PEEK` is enabled fixes this and allows the fuzzer to continue through the I2P target (https://github.com/bitcoin/bitcoin/issues/28803).
(https://github.com/bitcoin/bitcoin/pull/30211#pullrequestreview-2096248644)
Tested ACK a7fceda68bb62fe3d9060fcf52e33b2f64a2acf9
In `i2p` the private key needs to be at least 387 bytes. When this key is generated (using FuzzedSock) the bytes were being broken up such that the length of the saved key was always less than 387. Having `Recv` return the length of a byte vector, instead of a single byte, when `MSG_PEEK` is enabled fixes this and allows the fuzzer to continue through the I2P target (https://github.com/bitcoin/bitcoin/issues/28803).
👍 brunoerg approved a pull request: "fuzz: Make FuzzedSock fuzz friendlier"
(https://github.com/bitcoin/bitcoin/pull/30211#pullrequestreview-2096272288)
utACK 22d0f1a27ef7733b51b3c2138a8d01713df8f248
(https://github.com/bitcoin/bitcoin/pull/30211#pullrequestreview-2096272288)
utACK 22d0f1a27ef7733b51b3c2138a8d01713df8f248
💬 apulsifer commented on issue "LevelDB read failure: Corruption: block checksum mismatch":
(https://github.com/bitcoin/bitcoin/issues/30159#issuecomment-2147462839)
Update: After running continuously since 05-23 (no reboots or restarting bitcoind), one of the servers failed this morning. So it seems the data corruption bug occurs even when bitcoind is running continuously, although at a much lower rate.
Started bitcoind.service.
2024-05-23T11:23:40Z Bitcoin Core version v25.2.0 (release build)
2024-05-23T11:23:40Z InitParameterInteraction: parameter interaction: -blocksonly=1 -> setting -whitelistrelay=0
2024-05-23T11:23:40Z Using the 'arm_shani(1way,
...
(https://github.com/bitcoin/bitcoin/issues/30159#issuecomment-2147462839)
Update: After running continuously since 05-23 (no reboots or restarting bitcoind), one of the servers failed this morning. So it seems the data corruption bug occurs even when bitcoind is running continuously, although at a much lower rate.
Started bitcoind.service.
2024-05-23T11:23:40Z Bitcoin Core version v25.2.0 (release build)
2024-05-23T11:23:40Z InitParameterInteraction: parameter interaction: -blocksonly=1 -> setting -whitelistrelay=0
2024-05-23T11:23:40Z Using the 'arm_shani(1way,
...
💬 maflcko commented on pull request "Lint: Support running individual lint checks":
(https://github.com/bitcoin/bitcoin/pull/30219#issuecomment-2147517803)
ACK 0fcbfdb7ad172e518a10dd6e5be4cb6bb1158784
(https://github.com/bitcoin/bitcoin/pull/30219#issuecomment-2147517803)
ACK 0fcbfdb7ad172e518a10dd6e5be4cb6bb1158784