💬 ryanofsky commented on pull request "kernel: Streamline util library":
(https://github.com/bitcoin/bitcoin/pull/29015#issuecomment-1850626464)
Added 1 commits b23409474f6fe874a25374bb978db9e5682954e6 -> 6d8cb4d4cd6062ad06cd351439e9139e028995d3 ([`pr/rmutil.5`](https://github.com/ryanofsky/bitcoin/commits/pr/rmutil.5) -> [`pr/rmutil.6`](https://github.com/ryanofsky/bitcoin/commits/pr/rmutil.6), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/rmutil.5...pr/rmutil.6)) updating libraries.md to describe differences between util and common libraries better.

re: https://github.com/bitcoin/bitcoin/pull/29015#issuecomment-184902640
...

💬 kashifs commented on pull request "Make bitcoin-tx replaceable value optional":
(https://github.com/bitcoin/bitcoin/pull/29022#discussion_r1422933362)
Yes, that's correct. I've updated the text to read as you suggested. Thanks!

💬 eragmus commented on pull request "datacarriersize: Match more datacarrying":
(https://github.com/bitcoin/bitcoin/pull/28408#issuecomment-1850654602)
> If you can't see yourself that the thousands of tokens created with the inscriptions are scams I really can't do anything for you, for the images it's the same, inscribe ten times the same things with a slight declination and then move on to something else when people are no longer interested. This cycle continues indefinitely.

People lose their money by voluntarily choosing to gamble in the casino (the house always wins), so do we ban casinos? We don't, not in the US at least. Inscriptions a
...

💬 Retropex commented on pull request "datacarriersize: Match more datacarrying":
(https://github.com/bitcoin/bitcoin/pull/28408#issuecomment-1850681975)
> People lose their money by voluntarily choosing to gamble in the casino

It's true, but going to a casino does not oblige the entire planet to have an indelible mark of fact that you went to a casino. Inscriptions do.

> People also use their bitcoin for other distasteful things

Yes and it doesn't bother me because they are financial txs.


> Bitcoin is a decentralized permissionless network

Indeed, therefore, nodes runners have the right to have options to protect themselves from
...

💬 stickies-v commented on pull request "[WIP] C++20 std::views::reverse":
(https://github.com/bitcoin/bitcoin/pull/28687#issuecomment-1850688351)
> Needs rebase

Thx, rebased to include #28349 & updated PR description

💬 brunoerg commented on pull request "doc/reduce-traffic: update/clarify max outbound connection count":
(https://github.com/bitcoin/bitcoin/pull/29052#discussion_r1422994881)
nit:
```suggestion
block-relay-only ones and occasionally 1 short-lived feeler or an extra block-relay-only connection.
```

💬 MarnixCroes commented on pull request "doc/reduce-traffic: update/clarify max outbound connection count":
(https://github.com/bitcoin/bitcoin/pull/29052#discussion_r1423007653)
my bad,
done

💬 theuni commented on pull request "[POC] C++20 `std::endian`":
(https://github.com/bitcoin/bitcoin/pull/28674#discussion_r1423020120)
> I think I'd prefer to leave this with its comment. Note that we don't use `std::counl_zero` directly, we want kinda the opposite: `(sizeof(x) * 8 )- std::countl_zero(x);`

Actually, as it turns out, [std::bit_width](https://en.cppreference.com/w/cpp/numeric/bit_width) is exactly what we want (and it's even described in terms of `countl_zero`). I'll see if it works to use your suggestion with that function instead.

💬 murchandamus commented on pull request "wallet: skip BnB when SFFO is enabled":
(https://github.com/bitcoin/bitcoin/pull/28994#discussion_r1423022711)
Ah oops my bad. I was backporting my proposed refactor from https://github.com/bitcoin/bitcoin/pull/28985/commits/5b8c165c2ae0448b802c0c4907303d016f301f0a. I use a feerate of 3000 ṩ/kvB there.

🤔 amitiuttarwar reviewed a pull request: "doc/reduce-traffic: update/clarify max outbound connection count"
(https://github.com/bitcoin/bitcoin/pull/29052#pullrequestreview-1776011334)
ACK d58f89d355faf46b68d0ff8095699d0aff41959c

💬 glozow commented on pull request "[WIP] p2p: opportunistically accept 1-parent-1-child packages":
(https://github.com/bitcoin/bitcoin/pull/28970#issuecomment-1850749413)
Rebased and fixed CI. This is in draft because I'm focusing on v3 stuff, can be reviewed for its approach.

🤔 amitiuttarwar reviewed a pull request: "addrman, refactor: improve stochastic test in `AddSingle`"
(https://github.com/bitcoin/bitcoin/pull/27319#pullrequestreview-1776020630)
ACK e064487ca28c12ba774c2f43a3c7acbdb1a278c9

💬 glozow commented on pull request "test: Add and use option for tx-version in MiniWallet methods":
(https://github.com/bitcoin/bitcoin/pull/28972#issuecomment-1850757594)
Nice, I shall cherry-pick this

💬 glozow commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1423035255)
thanks :+1:

💬 achow101 commented on pull request "wallet: Have the wallet store the key for automatically generated descriptors":
(https://github.com/bitcoin/bitcoin/pull/26728#issuecomment-1850768688)
I've pushed a solution for the downgrade and encrypt issue. This will store an extra boolean in `ACTIVEHDKEY` which indicates whether the wallet was encrypted at the time that record was written. Then, when we detect that the wallet is now encrypted, but the `ACTIVEHDKEY` was written when the wallet was unencrypted, we will run the upgrade again to detect the new hd key.

For wallets that are encrypted with software supporting the record, the encryption status flag will be updated when the new
...

👍 ryanofsky approved a pull request: "depends: fix libmultiprocess build on aarch64"
(https://github.com/bitcoin/bitcoin/pull/28846#pullrequestreview-1776000110)
Code review ACK 965d38d0ab2cfb9b40827e422f354a061fa31209. Left a suggestion to simplify the second commit, but it is not important and this change looks good as.

💬 ryanofsky commented on pull request "depends: fix libmultiprocess build on aarch64":
(https://github.com/bitcoin/bitcoin/pull/28846#discussion_r1423019899)
In commit "depends: always install capnp to /lib" (6d7e71ed47f9f540d981d5433b8f4efcd0767641)

I think probably it would be good revert this line and only make this change in `capnp.mk` not `native_capnp.mk` since the PKG_CONFIG_PATH mentioned in the comment above is used to find cross-compiled dependencies, not native dependencies, so the reasoning in the comment doesn't really apply here. Reverting this change would also make the native_capnp package definition simpler and more consistent wit
...

💬 ryanofsky commented on pull request "depends: fix libmultiprocess build on aarch64":
(https://github.com/bitcoin/bitcoin/pull/28846#discussion_r1423040107)
re: https://github.com/bitcoin/bitcoin/pull/28846#discussion_r1417239636

In commit "depends: build libmultiprocess with position independant code" (965d38d0ab2cfb9b40827e422f354a061fa31209)

> Yes, the switch to CMake in #28856 didn't fix this issue.

I'm still not exactly clear why this change is needed here and also why `--with-pic` options seem to be used for many other packages as well. But at least after #28856 the PIC option is used more consistently, and it should cause no harm in
...

💬 brunoerg commented on pull request "wallet: Add CoinGrinder coin selection algorithm":
(https://github.com/bitcoin/bitcoin/pull/27877#discussion_r1423048346)
Perhaps the comment is not wrong, the parameter should be updated. Doesn't it?

💬 amitiuttarwar commented on pull request "rpc: addpeeraddress tried return error on failure":
(https://github.com/bitcoin/bitcoin/pull/28998#issuecomment-1850781513)
approach ACK

> (the address manager interface does not support removing addresses at the moment and adding this seems to be a bigger effort)

since right now the only way addresses are kicked off is triggered by a collision, removing addresses from addrman would require introducing new test-only code paths. in addition to being a bigger effort, it would increase the surface area of potential issues in the future. eg. if someone tried to use it with mainnet code. when I was learning addrman
...