💬 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.
(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.
(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.
(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
(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?
(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
(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?
(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
(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
...
(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
...
(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
(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)
(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
...
(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
(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.
(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
...
(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
(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
💬 willcl-ark commented on issue "qa: Support git worktrees when running the linters locally via Docker":
(https://github.com/bitcoin/bitcoin/issues/29972#issuecomment-2092689211)
Yes I agree, this bothers me too.
Seems like we could perhaps use mypy's [--cache-dir](https://mypy.readthedocs.io/en/stable/command_line.html#cmdoption-mypy-cache-dir) option to have it use `/dev/null` and avoid modifying source dir. I might try it out with the RO mount soon and see if it works OK that way or if there's anything else trying to make changes to the FS.
I don't want to derail this issue, but in a somewhat-related changeset I was also wondering if I could _speed up_ the linte
...
(https://github.com/bitcoin/bitcoin/issues/29972#issuecomment-2092689211)
Yes I agree, this bothers me too.
Seems like we could perhaps use mypy's [--cache-dir](https://mypy.readthedocs.io/en/stable/command_line.html#cmdoption-mypy-cache-dir) option to have it use `/dev/null` and avoid modifying source dir. I might try it out with the RO mount soon and see if it works OK that way or if there's anything else trying to make changes to the FS.
I don't want to derail this issue, but in a somewhat-related changeset I was also wondering if I could _speed up_ the linte
...
🤔 glozow reviewed a pull request: "test: remove duplicate `WITNESS_SCALE_FACTOR` constant defination"
(https://github.com/bitcoin/bitcoin/pull/30029#pullrequestreview-2037852552)
lgtm ACK af3c18169a075222fe0795ab24b8b20ad5e30ae4
(https://github.com/bitcoin/bitcoin/pull/30029#pullrequestreview-2037852552)
lgtm ACK af3c18169a075222fe0795ab24b8b20ad5e30ae4
💬 maflcko commented on issue "qa: Support git worktrees when running the linters locally via Docker":
(https://github.com/bitcoin/bitcoin/issues/29972#issuecomment-2092726109)
> (as part of a general python revamp in [this branch](https://github.com/bitcoin/bitcoin/compare/master...willcl-ark:bitcoin:fixup-lint-docker-image)).
Not sure about complicating the cirrus yml. I think it should be considered to be set in stone, because any changes will break re-running the lint task on old pull requests, IIRC. At this point, if you want to change it substantially, it could make sense to move it to GHA, along with an update to DrahtBot to be able to re-run GHA tasks. :swea
...
(https://github.com/bitcoin/bitcoin/issues/29972#issuecomment-2092726109)
> (as part of a general python revamp in [this branch](https://github.com/bitcoin/bitcoin/compare/master...willcl-ark:bitcoin:fixup-lint-docker-image)).
Not sure about complicating the cirrus yml. I think it should be considered to be set in stone, because any changes will break re-running the lint task on old pull requests, IIRC. At this point, if you want to change it substantially, it could make sense to move it to GHA, along with an update to DrahtBot to be able to re-run GHA tasks. :swea
...