💬 fjahr commented on pull request "assumeutxo: net_processing changes":
(https://github.com/bitcoin/bitcoin/pull/24008#issuecomment-1550427767)
> I wrote:
>
> > The first commit tends to confuses me
>
> Could use some thoughts on that comment (from anyone really).

@Sjors I found it hard to follow that message, to be honest :D Could you maybe try to formulate more clearly what is bothering you in particular and what questions you need to be answered? It might help if you reference the specific phases you are thinking of based on `doc/design/assumeutxo.md`. I think this is the most confusing part when discussing this.

💬 hebasto commented on issue "Can't compile v24.0.1":
(https://github.com/bitcoin/bitcoin/issues/27680#issuecomment-1550432466)
> ### Steps to reproduce
>
> Setup Raspi4 with Raspi OS installed on a bootable SSD
> Install BDB 4.8
> Clone bitcoin repo 24.0.1
> configure

Try to run `./autogen.sh` before `./configure` as it is documented [here](https://github.com/bitcoin/bitcoin/blob/24.x/doc/build-unix.md#to-build).

💬 LarryRuane commented on pull request "net: Allow inbound whitebind connections to more aggressively evict peers when slots are full":
(https://github.com/bitcoin/bitcoin/pull/27600#discussion_r1195757487)
Does RVO even apply here? I thought it applies to complex structures, a large vector being the canonical example, where we don't want to copy every element. But here we're just returning a `NodeId` which is a `int64_t`, wrapped by `std::optional`. I don't think there's any savings by moving instead of copying one of these.

💬 theuni commented on pull request "macOS: Bump minimum required runtime version and prepare for building with upstream LLVM":
(https://github.com/bitcoin/bitcoin/pull/27676#issuecomment-1550459263)
Pushed a test for `fixup_chains`, but I don't think it ever ran. @fanquake What sets `RUN_SECURITY_TESTS`?

⚠️ ddykeman1 opened an issue: "Mac osx 12.6.5 "
(https://github.com/bitcoin/bitcoin/issues/27681)
### Is there an existing issue for this?

- [X] I have searched the existing issues

### Current behaviour

I installed the new program it ran for 3 hrs and then I checked and said fatal error said to check logs there is no logs showing for this problem I uninstalled it and reinstalled a well as redownload it with the same out come no different
Did take a pic as it started and crashed
![IMG_20230516_184337383](https://github.com/bitcoin/bitcoin/assets/52932653/7aa98bec-7da5-4bb8-a385-a0794e4
...

💬 theuni commented on pull request "ci: Use `macos-ventura-xcode:14.1` image for "macOS native" task":
(https://github.com/bitcoin/bitcoin/pull/26388#issuecomment-1550484640)
As [seen here](https://github.com/bitcoin/bitcoin/pull/27676#issuecomment-1550459263), this seems to have removed `RUN_SECURITY_TESTS`? Ping @hebasto

💬 Xekyo commented on pull request "Implement Mini version of BlockAssembler to calculate mining scores":
(https://github.com/bitcoin/bitcoin/pull/27021#issuecomment-1550491250)
Super, thanks for the review, @theStack. As I already have a follow-up PR with #26152, since this PR has three ACKs now, and all the open comments are nits referring to the tests, I would like to include those changes as a new commit in the follow-up #26152.

What do you think, @glozow?

💬 theStack commented on issue "Use muhash for assumeUTXO snapshot ":
(https://github.com/bitcoin/bitcoin/issues/27669#issuecomment-1550522541)
Had similar thoughts that using muhash instead would be nice in order to avoid the chain rollback for AssumeUTXO hash reviewers. Not sure if this is a huge problem, but one funny side-effect of this would be that for a chainstate with _n_ coins, there are in theory _n!_ different serializations possible that all would pass as valid (compared to just one for `hash_serialized_2`), since the order of coins doesn't matter for muhash.

> For good measure we should also modify `gettxoutsetinfo` so
...

💬 Xekyo commented on pull request "Implement Mini version of BlockAssembler to calculate mining scores":
(https://github.com/bitcoin/bitcoin/pull/27021#issuecomment-1550595711)
> ACK [6b605b9](https://github.com/bitcoin/bitcoin/commit/6b605b91c1faf2c7f7cc0c9d39b4fcfd66dc2965) modulo `miniminer_overlap` test.
>
> Not really blocking, I'm planning to go deeper later. And probably add some explanatory comments and code simplifications. I think that has a readability barrier that will be a maintenance issue moving forward.

Perhaps we can address that in #26152 as well

💬 Xekyo commented on pull request "Bump unconfirmed ancestor transactions to target feerate":
(https://github.com/bitcoin/bitcoin/pull/26152#issuecomment-1550602703)
Rebased on #27021, need to rebase on master next, then will incorporate the follow-up nits from #27021

🤔 ishaanam reviewed a pull request: "wallet: improve IBD sync time by skipping block scanning prior birth time"
(https://github.com/bitcoin/bitcoin/pull/27469#pullrequestreview-1429618571)
Approach ACK, this looks good so far but I haven't reviewed the first commit yet.

💬 ishaanam commented on pull request "wallet: improve IBD sync time by skipping block scanning prior birth time":
(https://github.com/bitcoin/bitcoin/pull/27469#discussion_r1195808323)
nit:
```suggestion
/** Updates wallet birth time if 'new_birth_time' is lower than m_birth_time*/
```

💬 ishaanam commented on pull request "wallet: improve IBD sync time by skipping block scanning prior birth time":
(https://github.com/bitcoin/bitcoin/pull/27469#discussion_r1195809999)
Why is this check also done here if it is already done in `FirstKeyTimeChanged`?

💬 ajtowns commented on pull request "net_processing: Drop m_recently_announced_invs bloom filter":
(https://github.com/bitcoin/bitcoin/pull/27675#discussion_r1195879972)
I mostly renamed it to make sure there'd be compiler errors anywhere that it didn't get updated, just in case some type conversion happened automatically.

💬 ajtowns commented on pull request "net_processing: Drop m_recently_announced_invs bloom filter":
(https://github.com/bitcoin/bitcoin/pull/27675#discussion_r1195881028)
Yeah, this is broken as-is. My original idea was to add a timestamp to the `mapRelay` entries too, then it was to drop `mapRelay` and add `vExtraTxnForCompact`-relay, then I compromised by just doing it wrong...

💬 MarcoFalke commented on pull request "Descriptor unit tests and simplifications":
(https://github.com/bitcoin/bitcoin/pull/24361#issuecomment-1550798295)
> Drafted for now given it's (maybe) waiting on https://github.com/bitcoin/bitcoin/pull/26076.

That one was merged last week.

💬 MarcoFalke commented on issue "Mac osx 12.6.5 ":
(https://github.com/bitcoin/bitcoin/issues/27681#issuecomment-1550799971)
Please check your debug.log for possible causes; Alternatively you can upload it here.

You can find the debug.log in your [data dir](https://github.com/bitcoin/bitcoin/blob/master/doc/files.md#data-directory-location).

Please be aware that the debug log might contain personally identifying information.

💬 MarcoFalke commented on issue "Can't compile v24.0.1":
(https://github.com/bitcoin/bitcoin/issues/27680#issuecomment-1550801604)
Closing for now. Let us know if you have any other questions.

✅ MarcoFalke closed an issue: "Can't compile v24.0.1"
(https://github.com/bitcoin/bitcoin/issues/27680)

💬 S3RK commented on pull request "wallet: Have the wallet store the key for automatically generated descriptors":
(https://github.com/bitcoin/bitcoin/pull/26728#discussion_r1196058973)
nit: for the next ~~node~~test