💬 MarcoFalke commented on pull request "test: bugfix, synchronize indexes synchronously":
(https://github.com/bitcoin/bitcoin/pull/28026#issuecomment-1623118058)
> I mean, I'm also ok if we simply rise the timeout but, at the same time, I don't think that the background threads creation adds any meaningful test coverage here.

Right, it doesn't add any meaningful coverage. Though, I think the main point is that we should try to avoid adding test-only code to "real" code where possible. I am fine with anything, though.

🤔 S3RK reviewed a pull request: "Bump unconfirmed ancestor transactions to target feerate"
(https://github.com/bitcoin/bitcoin/pull/26152#pullrequestreview-1512188451)
Continue tests review

💬 S3RK commented on pull request "Bump unconfirmed ancestor transactions to target feerate":
(https://github.com/bitcoin/bitcoin/pull/26152#discussion_r1251575760)
nit: since you create new wallet for every test and never reuse any addresses do you really need to generate blocks? I tried to delete and the tests pass

💬 S3RK commented on pull request "Bump unconfirmed ancestor transactions to target feerate":
(https://github.com/bitcoin/bitcoin/pull/26152#discussion_r1251588782)
nit: here and other tests add `self.assert_spends_only_parent`

💬 S3RK commented on pull request "Bump unconfirmed ancestor transactions to target feerate":
(https://github.com/bitcoin/bitcoin/pull/26152#discussion_r1251589066)
nit: add `self.assert_spends_only_parent`

💬 S3RK commented on pull request "Bump unconfirmed ancestor transactions to target feerate":
(https://github.com/bitcoin/bitcoin/pull/26152#discussion_r1254037039)
nit: why there is a multiplier here?

💬 S3RK commented on pull request "Bump unconfirmed ancestor transactions to target feerate":
(https://github.com/bitcoin/bitcoin/pull/26152#discussion_r1251595920)
Here and other tests.I'm not sure the magic number is needed. I tried setting the multiplier to 1 and the tests still pass. Maybe just check that resulting fee rate is exactly equal to target fee rate?

💬 S3RK commented on pull request "Bump unconfirmed ancestor transactions to target feerate":
(https://github.com/bitcoin/bitcoin/pull/26152#discussion_r1254028212)
This could be extended to a set of parent txs to verify cases with multiple parents

💬 S3RK commented on pull request "Bump unconfirmed ancestor transactions to target feerate":
(https://github.com/bitcoin/bitcoin/pull/26152#discussion_r1254048054)
nit: sffo is not needed

💬 S3RK commented on pull request "Bump unconfirmed ancestor transactions to target feerate":
(https://github.com/bitcoin/bitcoin/pull/26152#discussion_r1254044504)
Isn't `test_preset_input_cpfp` testing this?

⚠️ megumin9 opened an issue: "The destructor of CCheckQueueControl may throw a exception "
(https://github.com/bitcoin/bitcoin/issues/28033)
In src/checkqueue.h, the destructor of CCheckQueueControl will call Wait, which may throw a condition_error exception in 'void condition_variable::wait(unique_lock<mutex>& m)'. This will cause a crash, Is it intended? If not, the exception from Wait can be caught as follows to avoid the crash.

if (!fDone)
Wait();
if (pqueue != nullptr) {
LEAVE_CRITICAL_SECTION(pqueue->ControlMutex);
}

--->

try {
if (!fDone)
Wait();
if (pqueue != nullptr) {
LEAVE_CRITICAL_SECTION(
...

💬 MarcoFalke commented on issue "The destructor of CCheckQueueControl may throw a exception ":
(https://github.com/bitcoin/bitcoin/issues/28033#issuecomment-1623201981)
What is `condition_error`?

💬 MarcoFalke commented on issue "The destructor of CCheckQueueControl may throw a exception ":
(https://github.com/bitcoin/bitcoin/issues/28033#issuecomment-1623240274)
Closing for now, but can be reopened when the outstanding questions have been answered.

✅ MarcoFalke closed an issue: "The destructor of CCheckQueueControl may throw a exception "
(https://github.com/bitcoin/bitcoin/issues/28033)

💬 MarcoFalke commented on issue "Unable to send funds":
(https://github.com/bitcoin/bitcoin/issues/28001#issuecomment-1623243536)
Closing for now. Leave a comment if this is still an issue.

✅ MarcoFalke closed an issue: "Unable to send funds"
(https://github.com/bitcoin/bitcoin/issues/28001)

💬 fanquake commented on issue "Assertion failed: (data.size() > node.nSendOffset), function SocketSendData, file net.cpp, line 837":
(https://github.com/bitcoin/bitcoin/issues/27963#issuecomment-1623245433)
@FelixWeis were you able to perform a bisect here?

✅ fanquake closed a pull request: "Gitignore auto-generated Secp256k1 files"
(https://github.com/bitcoin/bitcoin/pull/28032)

💬 willcl-ark commented on issue "depends build fails macOS intel":
(https://github.com/bitcoin/bitcoin/issues/27977#issuecomment-1623264853)
Seems to work just fine out of the box on my macbook with clang16? Although technically this is not a totally fresh install of macos...

<details>
<summary> log </summary>

```sh
will@MBP in …/src/bitcoin/depends on  master [$?⇡]
₿ make NO_QT=1 NO_WALLET=1 NO_ZMQ=1 NO_UPNP=1 NO_NATPMP=1 -j 4
Building native_ds_store...
running build
running build_py
creating build
creating build/lib
creating build/lib/ds_store
copying ds_store/store.py -> build/lib/ds_store
copying ds_store/__ini
...

💬 fanquake commented on issue "depends build fails macOS intel":
(https://github.com/bitcoin/bitcoin/issues/27977#issuecomment-1623269757)
Haven't looked into the issue here yet, but note that these packages will be removed as part of #27099.