💬 MarcoFalke commented on issue "ci: failure in Docker build step":
(https://github.com/bitcoin/bitcoin/issues/27492#issuecomment-1514723930)
I guess the `Prebuild` task needs to run after the `merge_base` step, or similar.

💬 MarcoFalke commented on issue "ci: failure in Docker build step":
(https://github.com/bitcoin/bitcoin/issues/27492#issuecomment-1514729331)
Not sure how to easily run a step before the build.

Ideally this is something fixed by upstream, see:

* https://github.com/cirruslabs/cirrus-ci-docs/issues/791
* https://github.com/cirruslabs/cirrus-ci-docs/discussions/1110
* ...

💬 hebasto commented on issue "Replace all of the RecursiveMutex instances with the Mutex ones":
(https://github.com/bitcoin/bitcoin/issues/19303#issuecomment-1514746298)
> You should be able to see it in the tsan CI task or with libc++ locally?

The TSan CI job is pretty stable @ 760651214cd205b91804bc1c40a31c614d3aa26c, and removing the `deadlock:Chainstate::ConnectTip` suppression does not cause any failure.

My guess is that this is a byproduct of clang upgrade.

💬 MarcoFalke commented on pull request "Allow configuring target block time for a signet":
(https://github.com/bitcoin/bitcoin/pull/27446#issuecomment-1514785618)
I guess one can pass all non-default consensus args to a hasher and use that as the network magic, and also use the network magic as the suffix for the signet folder name.

This would also allow to make more consensus args configurable as a non-breaking change in the future. Though, obviously it would be a breaking change now.

💬 pinheadmz commented on issue "Compiling a bitcoin core version that accepts transactions over 100vkb":
(https://github.com/bitcoin/bitcoin/issues/27490#issuecomment-1514785947)
The max standard tx size is defined in this line in `policy.h`:

https://github.com/bitcoin/bitcoin/blob/d908877c4774c2456eed09167a5f382758e4a8a6/src/policy/policy.h#L26-L27

Since there are no configuration options node operators can use to change this value, it's reasonable to assume that most if not all Bitcoin Core nodes on the network enforce it. Therefore, even if you were to alter your own software to accept heavier transactions, they would likely not get relayed to any of your peers.
...

💬 pinheadmz commented on pull request "doc: Improve documentation of rpcallowip":
(https://github.com/bitcoin/bitcoin/pull/27480#discussion_r1171392411)
Sure ;-)

💬 instagibbs commented on pull request "BIP324: ElligatorSwift integrations":
(https://github.com/bitcoin/bitcoin/pull/27479#discussion_r1171357368)
normal *compressed* key

💬 instagibbs commented on pull request "BIP324: ElligatorSwift integrations":
(https://github.com/bitcoin/bitcoin/pull/27479#discussion_r1171373891)
s/64/EllSwiftPubKey::size()/

💬 instagibbs commented on pull request "BIP324: ElligatorSwift integrations":
(https://github.com/bitcoin/bitcoin/pull/27479#discussion_r1171373986)
s/64/EllSwiftPubKey::size()/

💬 instagibbs commented on pull request "BIP324: ElligatorSwift integrations":
(https://github.com/bitcoin/bitcoin/pull/27479#discussion_r1171362440)
Took me too long to figure out what "ent" referred to, just putting the full word "entropy" in the comment would help. If the entropy is not unpredictable this would result in a not uniformly random(?) series of bytes in the resulting pubkey?

💬 pinheadmz commented on pull request "Add wallet method to detect if a key is "active"":
(https://github.com/bitcoin/bitcoin/pull/27216#discussion_r1171399601)
Ah thanks for the ack, I'll update this and the commit message you mentioned if I need to revise again.

💬 sipa commented on pull request "BIP324: ElligatorSwift integrations":
(https://github.com/bitcoin/bitcoin/pull/27479#discussion_r1171402620)
Actually, no, the "create ellswift encoding" algorithm in libsecp256k1 uses an internal RNG which is always at least seeded with the private key; the `ent32` argument here is just auxiliary randomness that gets fed to that RNG.

The output of the encoding is in fact indistinguishable from uniformly random to anyone who doesn't know the private key already. Adding actual randomness is a more belt-and-suspenders step to make sure the result is actually close to uniformly random, rather than just
...

📝 fanquake opened a pull request: "[WIP] build: use LLVM/Clang 14 for Darwin builds"
(https://github.com/bitcoin/bitcoin/pull/27493)
This works for depends.
Guix needs some path wrangling in the preprocessor.
Also switches to using `-isystem` & `-nostd*`. See https://releases.llvm.org/16.0.0/projects/libcxx/docs/UsingLibcxx.html#using-a-custom-built-libc.
Note that the LLVM versions used by depends (14.0.0) and Guix (14.0.3) currently differ.

💬 fanquake commented on pull request "[WIP] ci: standardize custom libc++ usage in MSAN jobs":
(https://github.com/bitcoin/bitcoin/pull/27485#issuecomment-1514841989)
> We can likely do something similar for our Darwin depends build

See #27493

💬 MarcoFalke commented on pull request "move-only: Extract common/args from util/system":
(https://github.com/bitcoin/bitcoin/pull/27419#issuecomment-1514862798)
re-ACK be55f545d53d44fdcf2d8ae802e9eae551d120c6 🚲

<details><summary>Show signature</summary>

Signature:

```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: re-ACK be55f545d53d44fdcf2
...

💬 MarcoFalke commented on pull request "move-only: Extract common/args from util/system":
(https://github.com/bitcoin/bitcoin/pull/27419#issuecomment-1514870994)
unrelated: red CI can be ignored

💬 MarcoFalke commented on pull request "blockstorage: Adjust fastprune limit if block exceeds blockfile size":
(https://github.com/bitcoin/bitcoin/pull/27191#issuecomment-1514877737)
unrelated: Red CI can be ignored, or if you care a lot, you can rebase

💬 jonatack commented on pull request "test: add end-to-end tests for CConnman and PeerManager":
(https://github.com/bitcoin/bitcoin/pull/26812#discussion_r1171479645)
The unit tests are what I run locally over and over, as they run so much more quickly. IIRC there was a project idea a few years ago to convert the functional tests in Python to C++. Having better unit test coverage, or shifting coverage from the Python tests to C++ ones, seems beneficial.

💬 aureleoules commented on pull request "init: Add option for rpccookie permissions":
(https://github.com/bitcoin/bitcoin/pull/26088#issuecomment-1514901152)
I don't have much time to work on Core so feel free to pick this up @willcl-ark.

💬 fanquake commented on pull request "ci: build libc++ in DEBUG mode in MSAN jobs":
(https://github.com/bitcoin/bitcoin/pull/27448#issuecomment-1514902450)
> is DEBUG=1 enabled for them as well in depends?
> Edit: I guess that would make them unusably slow?

They don't run too slow, but they seem to find bugs. #27222 reproduces on `x86_64`:
```bash
wallet/test/feebumper_tests.cpp(18): Entering test suite "feebumper_tests"
wallet/test/feebumper_tests.cpp(42): Entering test case "external_max_weight_test"
2023-04-19T14:50:06Z Seed: Setting random seed for current tests to RANDOM_CTX_SEED=bcbaed399ecb4b53cf5998534b1cb7a9bdddd1c1755a5766212f5af3
...