π achow101 merged a pull request: "test: wallet rescan with reorged parent + IsFromMe child in mempool"
(https://github.com/bitcoin/bitcoin/pull/29179)
(https://github.com/bitcoin/bitcoin/pull/29179)
π¬ achow101 commented on pull request "doc, test: test and explain service flag handling":
(https://github.com/bitcoin/bitcoin/pull/29213#issuecomment-1894299113)
ACK 74ebd4d1359edce82a134dfcd3da9840f8d206e2
(https://github.com/bitcoin/bitcoin/pull/29213#issuecomment-1894299113)
ACK 74ebd4d1359edce82a134dfcd3da9840f8d206e2
π¬ yancyribbens commented on pull request "test: Add algo assert to bnb_search_test":
(https://github.com/bitcoin/bitcoin/pull/29206#issuecomment-1894307538)
> I donβt think itβs useful to require that the best input set was produced by BnB
That's fine, however the tests in question are part of the `bnb_search_test`. So if it's not testing BnB, I feel like the tests should be moved to a different test. It's confusing imo to understand how BnB works if the tests cases labeled as `bnb_search_test` are not acutally testing `bnb_search`.
(https://github.com/bitcoin/bitcoin/pull/29206#issuecomment-1894307538)
> I donβt think itβs useful to require that the best input set was produced by BnB
That's fine, however the tests in question are part of the `bnb_search_test`. So if it's not testing BnB, I feel like the tests should be moved to a different test. It's confusing imo to understand how BnB works if the tests cases labeled as `bnb_search_test` are not acutally testing `bnb_search`.
π achow101 merged a pull request: "doc, test: test and explain service flag handling"
(https://github.com/bitcoin/bitcoin/pull/29213)
(https://github.com/bitcoin/bitcoin/pull/29213)
β οΈ Xaspr opened an issue: "Unable to sync blockchain on laptop: ERROR: ReadBlockFromDisk: Deserialize or I/O error"
(https://github.com/bitcoin/bitcoin/issues/29255)
### Is there an existing issue for this?
- [X] I have searched the existing issues
### Current behaviour
After installing Bitcoin Core 26.0 in Windows Pro 11 and starting IBD, Core crashes.
I know Windows might not be exactly ideal, but I like to run a pruned node on my work laptop as well. I didn't set pruning yet by the way, I started Core just with the standard settings.
Bitcoin Core crashes and the following error pops up in debug.log:
`ERROR: ReadBlockFromDisk: Deserialize or
...
(https://github.com/bitcoin/bitcoin/issues/29255)
### Is there an existing issue for this?
- [X] I have searched the existing issues
### Current behaviour
After installing Bitcoin Core 26.0 in Windows Pro 11 and starting IBD, Core crashes.
I know Windows might not be exactly ideal, but I like to run a pruned node on my work laptop as well. I didn't set pruning yet by the way, I started Core just with the standard settings.
Bitcoin Core crashes and the following error pops up in debug.log:
`ERROR: ReadBlockFromDisk: Deserialize or
...
π¬ maflcko commented on pull request "test: Add and use option for tx-version in MiniWallet methods":
(https://github.com/bitcoin/bitcoin/pull/28972#issuecomment-1894333952)
rebased for CI
(https://github.com/bitcoin/bitcoin/pull/28972#issuecomment-1894333952)
rebased for CI
π¬ jonatack commented on pull request "doc, test: test and explain service flag handling":
(https://github.com/bitcoin/bitcoin/pull/29213#issuecomment-1894340133)
Post-merge ACK 74ebd4d1359edce82a134dfcd3da9840f8d206e2
(https://github.com/bitcoin/bitcoin/pull/29213#issuecomment-1894340133)
Post-merge ACK 74ebd4d1359edce82a134dfcd3da9840f8d206e2
π¬ maflcko commented on pull request "kernel: Remove key module from kernel library":
(https://github.com/bitcoin/bitcoin/pull/29252#discussion_r1453887711)
It is possible to suppress a clang tidy check on a specific line of code
(https://github.com/bitcoin/bitcoin/pull/29252#discussion_r1453887711)
It is possible to suppress a clang tidy check on a specific line of code
π¬ murchandamus commented on pull request "test: Add algo assert to bnb_search_test":
(https://github.com/bitcoin/bitcoin/pull/29206#issuecomment-1894388979)
The presence of BnB ensures that these expected input set will be found. The presence of BnB does neither preclude another algorithm from finding the same solution, nor cause the BnB solution to be necessarily picked among two equivalent solutions produced by two different algorithms. Hence it does not make sense to require that the solution was produced by BnB, only that the best solution will be found.
(https://github.com/bitcoin/bitcoin/pull/29206#issuecomment-1894388979)
The presence of BnB ensures that these expected input set will be found. The presence of BnB does neither preclude another algorithm from finding the same solution, nor cause the BnB solution to be necessarily picked among two equivalent solutions produced by two different algorithms. Hence it does not make sense to require that the solution was produced by BnB, only that the best solution will be found.
π¬ hebasto commented on issue "Unable to sync blockchain on laptop: ERROR: ReadBlockFromDisk: Deserialize or I/O error":
(https://github.com/bitcoin/bitcoin/issues/29255#issuecomment-1894391840)
Does it happen with the previous versions: 25.1, 24.2?
Does it happen, when antivirus tools/software are disabled (or the datadir is added to the excluded folders)?
(https://github.com/bitcoin/bitcoin/issues/29255#issuecomment-1894391840)
Does it happen with the previous versions: 25.1, 24.2?
Does it happen, when antivirus tools/software are disabled (or the datadir is added to the excluded folders)?
π murchandamus converted_to_draft a pull request: "Fix waste calculation in SelectionResult"
(https://github.com/bitcoin/bitcoin/pull/28366)
PR #26152 moved waste calculation into SelectionResult to be able to correct the waste score on basis of the bump_fee_group_discount for overlapping ancestries. This left two functions with largely overlapping purpose, where one was simply a wrapper of the other. This PR cleans up the overlap, and fixes the double-meaning of `change_cost` where the `GetChange()` function assumed that no change was created when `change_cost` was set to 0. This behavior was exploited in a bunch of tests, but is pr
...
(https://github.com/bitcoin/bitcoin/pull/28366)
PR #26152 moved waste calculation into SelectionResult to be able to correct the waste score on basis of the bump_fee_group_discount for overlapping ancestries. This left two functions with largely overlapping purpose, where one was simply a wrapper of the other. This PR cleans up the overlap, and fixes the double-meaning of `change_cost` where the `GetChange()` function assumed that no change was created when `change_cost` was set to 0. This behavior was exploited in a bunch of tests, but is pr
...
π¬ murchandamus commented on pull request "Fix waste calculation in SelectionResult":
(https://github.com/bitcoin/bitcoin/pull/28366#issuecomment-1894414662)
Converting to draft until I get around to fixing the coin_selection benchmark.
(https://github.com/bitcoin/bitcoin/pull/28366#issuecomment-1894414662)
Converting to draft until I get around to fixing the coin_selection benchmark.
π¬ achow101 commented on pull request "snapshots: don't core dump when running -checkblockindex after `loadtxoutset`":
(https://github.com/bitcoin/bitcoin/pull/28791#issuecomment-1894420211)
ACK cdc6ac4126b31426261605a757c52ea2dbfb2a81
(https://github.com/bitcoin/bitcoin/pull/28791#issuecomment-1894420211)
ACK cdc6ac4126b31426261605a757c52ea2dbfb2a81
π¬ ismaelsadeeq 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_r1453480372)
```suggestion
/** Add fee and size of another FeeFrac to this one. */
void inline operator+=(const FeeFrac& other) noexcept
{
fee += other.fee;
size += other.size;
}
/** Subtrack fee and size of another FeeFrac from this one. */
```
(https://github.com/bitcoin/bitcoin/pull/29242#discussion_r1453480372)
```suggestion
/** Add fee and size of another FeeFrac to this one. */
void inline operator+=(const FeeFrac& other) noexcept
{
fee += other.fee;
size += other.size;
}
/** Subtrack fee and size of another FeeFrac from this one. */
```
π¬ ismaelsadeeq 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_r1453482107)
No need for this return here
```suggestion
}
```
(https://github.com/bitcoin/bitcoin/pull/29242#discussion_r1453482107)
No need for this return here
```suggestion
}
```
π¬ ismaelsadeeq 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_r1453501485)
nit: we can just use the empty constructor
```suggestion
FeeFrac empty{};
```
(https://github.com/bitcoin/bitcoin/pull/29242#discussion_r1453501485)
nit: we can just use the empty constructor
```suggestion
FeeFrac empty{};
```
π¬ ismaelsadeeq 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_r1453480679)
Reserve here?
```suggestion
diagram.clear();
diagram.reserve(chunks.size() + 1);
```
(https://github.com/bitcoin/bitcoin/pull/29242#discussion_r1453480679)
Reserve here?
```suggestion
diagram.clear();
diagram.reserve(chunks.size() + 1);
```
π¬ ismaelsadeeq 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_r1453487633)
Maybe add a docstring on what `BuildDiagramFromUnsortedChunks` does in and out params description that are doxygen compatible?
(https://github.com/bitcoin/bitcoin/pull/29242#discussion_r1453487633)
Maybe add a docstring on what `BuildDiagramFromUnsortedChunks` does in and out params description that are doxygen compatible?
π¬ ismaelsadeeq 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_r1453502118)
why can we have negative size ?
(https://github.com/bitcoin/bitcoin/pull/29242#discussion_r1453502118)
why can we have negative size ?
π¬ ismaelsadeeq 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_r1453612523)
use static cast?
```suggestion
FeeFrac package{txiter->GetModFeesWithAncestors(), static_cast<int32_t>(txiter->GetSizeWithAncestors())};
```
(https://github.com/bitcoin/bitcoin/pull/29242#discussion_r1453612523)
use static cast?
```suggestion
FeeFrac package{txiter->GetModFeesWithAncestors(), static_cast<int32_t>(txiter->GetSizeWithAncestors())};
```