💬 josibake commented on pull request "ci: Remove second user account":
(https://github.com/bitcoin/bitcoin/pull/27376#issuecomment-1497316092)
> The lint system is completely different from ./ci/test/.

yeah, just realized there is no overlap. seems like we just need to apply the same fix the lint system is using in `./ci/test/` ?

💬 MarcoFalke commented on pull request "ci: Remove second user account":
(https://github.com/bitcoin/bitcoin/pull/27376#issuecomment-1497316877)
As for the fix, anything should be fine. git `safe.directory`, or restoring the `chown`, or fiddling with the `rsync` command.

💬 josibake commented on pull request "ci: Remove second user account":
(https://github.com/bitcoin/bitcoin/pull/27376#issuecomment-1497318317)
> As for the fix, anything should be fine. git safe.directory, or restoring the chown, or fiddling with the rsync command.

cool, I'll open a PR

💬 Ayush170-Future commented on pull request "logging, net: add ASN from peers on logs":
(https://github.com/bitcoin/bitcoin/pull/27412#issuecomment-1497321029)
I agree with @Sjors here. In my last review comment, I suggested the same idea. Though I'm not sure how the peer privacy issue is relevant here. But, it makes more sense to only log ASN when <code>fLogIPs</code> is already true.

Maybe changing the name of <code>-logips</code> flag to something like <code>-lognetinfo</code> will better reflect its use and avoid confusion, but that would require a release note. (just a school of thought)

💬 MarcoFalke commented on pull request "ci: use clang-16 in tidy task":
(https://github.com/bitcoin/bitcoin/pull/27404#issuecomment-1497326083)
lgtm ACK a56c96507a9e943bbcd7e126bc827de9495f0ebd

🚀 fanquake merged a pull request: "depends: add `NO_HARDEN=` option"
(https://github.com/bitcoin/bitcoin/pull/27406)

📝 josibake opened a pull request: "ci: fix git dubious permissions error"
(https://github.com/bitcoin/bitcoin/pull/27423)
fixes https://github.com/bitcoin/bitcoin/pull/27376#issuecomment-1496449588

this appears to be caused by a more recent version of git being sensitive to mismatched permissions on directories. we didn't notice this before because we were using two separate user accounts to fix up dir permissions in the container , but the second account was removed in #27376

there might be a more elegant way to do this, but this does the trick and seems to be the way others are fixing this issue around the
...

💬 josibake commented on pull request "ci: fix git dubious permissions error":
(https://github.com/bitcoin/bitcoin/pull/27423#issuecomment-1497346650)
cc @hebasto @MarcoFalke

💬 Sjors commented on pull request "logging, net: add ASN from peers on logs":
(https://github.com/bitcoin/bitcoin/pull/27412#issuecomment-1497347621)
Reading that original discussion I think the current code is fine. The main concern was exposing the origin of transactions in log files under the _default_ configuration. Since we don't log transaction relay by default (it's extremely noisy) we can safely log the ASN (or even IP) of a node when it connects.

We can clean up the documentation and/or behavior of `-logips` in another PR.

💬 josibake commented on pull request "ci: use clang-16 in tidy task":
(https://github.com/bitcoin/bitcoin/pull/27404#issuecomment-1497365274)
ACK https://github.com/bitcoin/bitcoin/pull/27404/commits/a56c96507a9e943bbcd7e126bc827de9495f0ebd

> Included this. I think we could merge this as-is, but should also figure out why it's needed. It feels like there is another issue here.

did a little digging and found this: https://github.com/include-what-you-use/include-what-you-use/issues/679#issuecomment-482798379 , which seems to still be an issue based on the discussion in the linked solution. fwiw, the fix you have here seems to be o
...

👍 hebasto approved a pull request: "ci: fix git dubious permissions error"
(https://github.com/bitcoin/bitcoin/pull/27423)
ACK 2c11e94131447bc605b7698a80d02c2523497a4b, tested on Ubuntu 22.04.

💬 hebasto commented on pull request "ci: fix git dubious permissions error":
(https://github.com/bitcoin/bitcoin/pull/27423#discussion_r1158438615)
A stray `#`?

💬 josibake commented on pull request "ci: fix git dubious permissions error":
(https://github.com/bitcoin/bitcoin/pull/27423#discussion_r1158448194)
fixed

💬 furszy commented on pull request "wallet: coin selection, don't return results that exceed the max allowed weight":
(https://github.com/bitcoin/bitcoin/pull/26720#discussion_r1158456950)
yep thanks, fixed

👍 hebasto approved a pull request: "ci: fix git dubious permissions error"
(https://github.com/bitcoin/bitcoin/pull/27423)
re-ACK ed4a8339b8fe796b4668e206d7fb9c2b120f8eb2

💬 fanquake commented on pull request "build: Pointer Authentication and Branch Target Identification for aarch64 Linux (Guix)":
(https://github.com/bitcoin/bitcoin/pull/24123#issuecomment-1497434168)
Rebased past #27406.

Note that the branch protection option being added to libevent here, can now exist inside the `NO_HARDEN` clause.

🚀 fanquake merged a pull request: "ci: use clang-16 in tidy task"
(https://github.com/bitcoin/bitcoin/pull/27404)

💬 fanquake commented on pull request "contrib: allow multi-sig binary verification v2":
(https://github.com/bitcoin/bitcoin/pull/27358#discussion_r1158537830)
`--no-trust-builder-keys` is no-longer a thing? Arguments here need re-ordering as well.

💬 fanquake commented on pull request "contrib: allow multi-sig binary verification v2":
(https://github.com/bitcoin/bitcoin/pull/27358#discussion_r1158526427)
Can drop `VERSIONPREFIX` from this output.

💬 fanquake commented on pull request "contrib: allow multi-sig binary verification v2":
(https://github.com/bitcoin/bitcoin/pull/27358#discussion_r1158530776)
note that the keyserver we currently "recommend" is `hkps://keys.openpgp.org`. See #22688, #23466.