💬 willcl-ark commented on issue "qa: Support git worktrees when running the linters locally via Docker":
(https://github.com/bitcoin/bitcoin/issues/29972#issuecomment-2092489093)
Update:

To avoid creating root-owned mypy files you can run the container with your user:group specified:

```bash
DOCKER_BUILDKIT=1 docker build -t bitcoin-linter --file "./ci/lint_imagefile" ./ && GIT_DIR=$(git rev-parse --path-format=absolute --git-common-dir) docker run --rm -v $(pwd):/bitcoin -v "$GIT_DIR":"$GIT_DIR" -u $(id -u):$(id -g) -it bitcoin-linter
```

This allows `git worktree remove` to succeed.

💬 Sjors commented on pull request "Stratum v2 Template Provider (take 3)":
(https://github.com/bitcoin/bitcoin/pull/29432#issuecomment-2092515772)
(ignoring CI failure, as I mentioned above I plan on making the tests more robust after #26812)

💬 sipsorcery commented on pull request "refactor, test: Always initialize pointer":
(https://github.com/bitcoin/bitcoin/pull/30026#issuecomment-2092525878)
utACK bd2de7ac591d7704b79304089ad1fb57e085da8b.

💬 Sjors commented on pull request "Testnet4 including PoW difficulty adjustment fix":
(https://github.com/bitcoin/bitcoin/pull/29775#discussion_r1588924764)
By the way, even if we do implement a higher minimum difficult, imo this should be done in `chainparams.cpp` by setting `consensus.powLimit`. If that makes it harder to grind a genesis block, perhaps that's a good reason not change it.

💬 Sjors commented on pull request "Testnet4 including PoW difficulty adjustment fix":
(https://github.com/bitcoin/bitcoin/pull/29775#discussion_r1588881978)
4e8b29ae234a05181ff9fd64ab3a74ba997e2eac: you can add `seed.testnet4.bitcoin.sprovoost.nl` here. I'll update it if the magic bytes change.

⚠️ maflcko opened an issue: "ci: Support running from a worktree"
(https://github.com/bitcoin/bitcoin/issues/30028)
Currently, when running the CI from a worktree, it will fail with:

```
++ git config --global ci.base-install-done
fatal: not a git repository: .git/worktrees/test2
```

`git config --global` is used here, because it allows to write a flag to storage on macOS and Linux. If someone knows of a different way to persist a flag on macOS and Linux, it should be used instead.

💬 maflcko commented on pull request "doc: fix broken relative md links":
(https://github.com/bitcoin/bitcoin/pull/30025#issuecomment-2092630176)
ACK 8e394d1d3b6ead130515222f5b34d509fff200a8

💬 maflcko commented on pull request "doc: fix broken relative md links":
(https://github.com/bitcoin/bitcoin/pull/30025#issuecomment-2092630583)
Is there an easy way to find those?

🤔 ismaelsadeeq reviewed a pull request: "doc: fix broken relative md links"
(https://github.com/bitcoin/bitcoin/pull/30025#pullrequestreview-2037752097)
utACK 8e394d1d3b6ead130515222f5b34d509fff200a8

💬 maflcko commented on issue "qa: Support git worktrees when running the linters locally via Docker":
(https://github.com/bitcoin/bitcoin/issues/29972#issuecomment-2092643562)
> To avoid creating root-owned mypy files

Probably unrelated, but ideally, I'd say that the container should not modify anything on the host. The "real" CI script use a `readonly` `--mount`.

If something needs to be cached, it can be done in a persisted volume?

💬 maflcko commented on pull request "miniscript: make operator""_mst consteval":
(https://github.com/bitcoin/bitcoin/pull/28657#issuecomment-2092644888)
@sipa

📝 ismaelsadeeq opened a pull request: "test: remove duplicate `WITNESS_SCALE_FACTOR` constant defination"
(https://github.com/bitcoin/bitcoin/pull/30029)
Notice this while working on #29523

- `blocktools.py` and `messages.py` both define `WITNESS_SCALE_FACTOR` factor constant

https://github.com/bitcoin/bitcoin/blob/99d7538cdb2a0ab7a7a2116cd5f33b95fc52b00e/test/functional/test_framework/blocktools.py#L48

https://github.com/bitcoin/bitcoin/blob/99d7538cdb2a0ab7a7a2116cd5f33b95fc52b00e/test/functional/test_framework/messages.py#L68
- This PR deletes the one in `blocktools.py` and update the tests to only use `WITNESS_SCALE_FACTOR` from `me
...

💬 maflcko commented on pull request "build: no-longer disable WARN_CXXFLAGS when CXXFLAGS is set":
(https://github.com/bitcoin/bitcoin/pull/25972#issuecomment-2092655487)
Would it make sense to mention that `-Wno-error=...` should be passed, when needed, in the compile docs?

utACK f0e22be69a15248c42964d57f44ce8c37a36081d 🍡

<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/Kl
...

💬 maflcko commented on pull request "test: remove duplicate `WITNESS_SCALE_FACTOR` constant defination":
(https://github.com/bitcoin/bitcoin/pull/30029#issuecomment-2092663858)
ACK af3c18169a075222fe0795ab24b8b20ad5e30ae4

💬 ismaelsadeeq commented on pull request "Wallet: Add `max_tx_weight` to transaction funding options (take 2)":
(https://github.com/bitcoin/bitcoin/pull/29523#issuecomment-2092667450)
Rebased and force pushed from 3d76c37106a4f80fd2ac410696b1049c3aa321c5 to b24b7a9a6a1a10024c639e9d4ee27aa4d0ce653e to fix C.I failure [compare diff](https://github.com/bitcoin/bitcoin/compare/3d76c37106a4f80fd2ac410696b1049c3aa321c5..b24b7a9a6a1a10024c639e9d4ee27aa4d0ce653e)

⚠️ maflcko opened an issue: "Intermittent issue in p2p_i2p_ports.py AssertionError: [node 0] Expected messages "['Error connecting to [...].b32.i2p:0: Cannot connect to 127.0.0.1:60000']" does not partially match log:"
(https://github.com/bitcoin/bitcoin/issues/30030)
https://cirrus-ci.com/task/4526270581571584?logs=ci#L3634

```
test 2024-04-28T17:46:07.431000Z TestFramework.node0 (DEBUG): RPC successfully started
test 2024-04-28T17:46:07.431000Z TestFramework (DEBUG): Generate a block with current time
test 2024-04-28T17:46:07.431000Z TestFramework.node0 (DEBUG): TestNode.generate() dispatches `generate` call to `generatetoaddress`
node0 2024-04-28T17:46:07.431322Z [http] [httpserver.cpp:306] [http_request_cb] [http] Received a POST request
...

💬 maflcko commented on issue "ci: failure in `wallet_multiwallet.py --legacy-wallet`":
(https://github.com/bitcoin/bitcoin/issues/29073#issuecomment-2092678570)
https://cirrus-ci.com/task/6041608645246976?logs=ci#L3376

💬 Sjors commented on pull request "Testnet4 including PoW difficulty adjustment fix":
(https://github.com/bitcoin/bitcoin/pull/29775#discussion_r1588929328)
4e8b29ae234a05181ff9fd64ab3a74ba997e2eac: in order to not run out of ports by testnet6, testnet5 should probably use ports 1833x again.

🤔 Sjors reviewed a pull request: "Testnet4 including PoW difficulty adjustment fix"
(https://github.com/bitcoin/bitcoin/pull/29775#pullrequestreview-2037697561)
Partial code review: the first commit 4e8b29ae234a05181ff9fd64ab3a74ba997e2eac looks good to me (of course it would partially change if we get a new genesis block).

I thought a bit more about block storms...

The only (relevant) thing that distinguishes testnet3 from mainnet is `fPowAllowMinDifficultyBlocks`. In our `miner.cpp` code we potentially lower `pblock->nBits` of the block we're trying to mine once enough time has passed. This works because both the miner and validation rely on `Ge
...

💬 GregTonoski commented on pull request "doc: replace remaining "520" magic nums with MAX_SCRIPT_ELEMENT_SIZE":
(https://github.com/bitcoin/bitcoin/pull/30024#issuecomment-2092687448)
NACK unless clearly explained (reference to specification like BIP or relevant disucssion would be highly appreciated). Why not fixing the policy standard rules so that "Nodes must NEVER send a data item > 520 bytes"?

See also: https://bitcoin.stackexchange.com/questions/38937/what-was-the-original-rationale-for-limiting-the-maximum-push-size