lint: modernise lint tooling #34547

willcl-ark commented at 10:25 AM on February 10, 2026: member

Modernise our lint tooling by:

- Replacing pyenv + pip with uv for better Python environment and dependency management - Move uv ruff and ty to install via COPY --from multi-stage Docker image imports - Moving ruff lint rules from hardcoded Rust array (in lint_py.rs) into a top-level ruff.toml - Extracting all remaining pip dependencies into dedicated ci/lint/requirements.txt

Extra rationale:

COPY --from pulls pre-built binaries from upstream images instead of compiling/downloading at runtime. Containerfile layer optimisations reduce rebuild frequency further.

Pinning tool versions in the dockerfile makes it more excplicit and easier to find.

The tradeoff we make here is that there is no longer a single install script to install tooling on a local machine. However I think this is OK, as it currently only works for apt-based OSes anyway, and I don't think running the linter outside of the container is such a valuable use-case as it is with some of the other CI jobs.

DrahtBot added the label Tests on Feb 10, 2026

DrahtBot commented at 10:25 AM on February 10, 2026: contributor

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/34547.

Reviews

See the guideline for information on the review process.

Type	Reviewers
ACK	maflcko, sedited
Concept ACK	fanquake
Stale ACK	stickies-v

If your review is incorrectly listed, please copy-paste <code></code> into the comment that the bot should ignore.

Conflicts

No conflicts as of last run.

fanquake commented at 10:39 AM on February 10, 2026: member

Tried running this locally, but it currently fails on arm64, because there is no arm64 variant for becheran/mlc:

Error: choosing an image from manifest list docker://becheran/mlc:latest: no image found in image index for architecture "arm64", variant "v8", OS "linux"

willcl-ark marked this as a draft on Feb 10, 2026

in ci/lint_imagefile:15 in b63444e4d1 outdated

  11 | +ENV LC_ALL=C
  12 | +ENV PATH="/python_env/bin:${PATH}"
  13 | +ENV VIRTUAL_ENV="/python_env"
  14 | +
  15 | +COPY --from=ghcr.io/astral-sh/uv:latest /uv /uvx /bin/
  16 | +COPY --from=ghcr.io/astral-sh/ruff:0.14.4 /ruff /bin/

maflcko commented at 10:58 AM on February 10, 2026:

Is there any value in having ruff separate? It would appear more consistent to install it next to the other ci/lint/requirements.txt (lief, etc)

willcl-ark commented at 12:16 PM on February 10, 2026:

The main advantage is that the COPY --from instructions are standalone layers and are not invalidated when things below change (they are just "replayed" over the top). The more tools we can install from them, the better (usually).

No strong opinion here, as the end result is the same. But IMO it's better practice to install from layers like I have it.

maflcko commented at 12:53 PM on February 10, 2026:

Sure, makes sense to use COPY --from to keep the layers in the cache.

I guess the main question is whether we want to force all devs to use the docker image. I think it should be fine for devs to pick pip and then install ci/lint/requirements.txt and get all the stuff they want. But no strong opinion, I rely on the GHA-ci for the lint anyway.

in ci/lint_imagefile:29 in b63444e4d1 outdated

  29 | +  uv python install && \
  30 |    echo 'alias lint="./ci/lint/06_script.sh"' >> ~/.bashrc && \
  31 | -  chmod 755 /entrypoint.sh && \
  32 |    rm -rf /var/lib/apt/lists/*
  33 |  
  34 | +COPY ./ci/lint/container-entrypoint.sh /entrypoint.sh

maflcko commented at 10:59 AM on February 10, 2026:

This seems to conflict with #34427. Maybe that should be merged/closed first?

willcl-ark commented at 12:59 PM on February 10, 2026:

Agree. I've acked that PR already, and would be happy to rebase this on it (or see it closed) -- no strong opinion either way really.

willcl-ark force-pushed on Feb 10, 2026

willcl-ark commented at 12:17 PM on February 10, 2026: member

Tried running this locally, but it currently fails on arm64, because there is no arm64 variant for becheran/mlc:
Error: choosing an image from manifest list docker://becheran/mlc:latest: no image found in image index for architecture "arm64", variant "v8", OS "linux"

This should be fixed in the latest push. We currently (and still here) use an x86_64 MLC binary, but this works in an arm64 container (e.g on Mac) because the Rosetta emulation is passed in/used inside the container.

I keep this behaviour for now, but ideally MLC woudl start publishing multi-platform docker images.

willcl-ark marked this as ready for review on Feb 10, 2026

DrahtBot added the label Needs rebase on Feb 12, 2026

willcl-ark force-pushed on Feb 12, 2026

DrahtBot removed the label Needs rebase on Feb 12, 2026

DrahtBot added the label Needs rebase on Mar 10, 2026

willcl-ark force-pushed on Mar 12, 2026

DrahtBot removed the label Needs rebase on Mar 12, 2026

willcl-ark force-pushed on Mar 12, 2026

DrahtBot added the label CI failed on Mar 12, 2026

willcl-ark commented at 11:59 AM on March 12, 2026: member

bumped to latest ruff (0.15.5) and ty (0.0.21) and added newly-needed rule ignores.

DrahtBot removed the label CI failed on Mar 12, 2026

in ci/lint/01_install.sh:33 in 10332d0129 outdated

  44 | +export PATH="/python_env/bin:${PATH}"
  45 |  command -v python3
  46 |  python3 --version
  47 |  
  48 | -${CI_RETRY_EXE} pip3 install \
  49 | +uv pip install --python /python_env \

maflcko commented at 3:32 PM on March 13, 2026:

nit in 10332d01290366c4e6f0b29e5e94aabe744452cc: Why drop the retry? Is this something that happens in uv? Why not just leave this line as-is?

willcl-ark commented at 8:01 PM on March 13, 2026:

Correct (it's configurable with https://docs.astral.sh/uv/reference/environment/#uv_http_retries)

in ci/lint_imagefile:11 in 87c0592779 outdated

   7 | @@ -8,6 +8,7 @@ FROM mirror.gcr.io/ubuntu:24.04
   8 |  
   9 |  COPY --from=ghcr.io/astral-sh/uv:latest /uv /uvx /bin/
  10 |  COPY --from=ghcr.io/astral-sh/ruff:0.15.5 /ruff /bin/
  11 | +COPY --from=docker.io/koalaman/shellcheck:v0.11.0 /bin/shellcheck /usr/bin/

maflcko commented at 3:37 PM on March 13, 2026:

nit: IIRC docker.io doesn't support ipv6? Also, they are heavily rate-limiting some ipv4 (or even blocking them). This was the reason to use gcr.

My preference would be to just leave this as-is :sweat_smile:

willcl-ark commented at 8:02 PM on March 13, 2026:

Oh yeah :'(

Will fix that up shortly then.

in ruff.toml:3 in 73eae6f76e outdated

   0 | @@ -0,0 +1,42 @@
   1 | +[lint]
   2 | +select = [
   3 | +    "B006", # mutable-argument-default

maflcko commented at 3:38 PM on March 13, 2026:

nit in 73eae6f76e5c008ca1933dd2edfb592796940289: Would be nice to squash this with a4200d48bf6e591e59a6eba4c5771251956b2409. Otherwise, it is harder to review.

willcl-ark commented at 8:13 PM on March 13, 2026:

Done in 7d936ac2111

maflcko commented at 3:39 PM on March 13, 2026: member

left some comments, but feel free to ignore them

c7e4a67086626b8832372cf461cee30c22c62ea0~8 🥝

<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: c7e4a67086626b8832372cf461cee30c22c62ea0~8 🥝
IVhYiz19o3yQe68zl8z74QmB+pkeGtEnNKMDk40qNOxOuO20cBwlEWe2l+juySUkL1C+N5ejQa0svHEqvOa2BQ==

</details>

willcl-ark force-pushed on Mar 13, 2026

in ci/lint_imagefile:11 in e613229f1a outdated

   7 | @@ -8,6 +8,7 @@ FROM mirror.gcr.io/ubuntu:24.04
   8 |  
   9 |  COPY --from=ghcr.io/astral-sh/uv:latest /uv /uvx /bin/
  10 |  COPY --from=ghcr.io/astral-sh/ruff:0.15.5 /ruff /bin/
  11 | +COPY --from=ghcr.io/astral-sh/ty:0.0.21 /ty /bin/ 8399357d746 (lint: install ty typechecker from docker and use)

maflcko commented at 7:52 AM on March 14, 2026:

Nit in e613229f1a3734f8a8a12211415cc3b68471df35: :eyes:

Also, could squash 5b756860bd7d84665cdfa55873db410e90532158 into this one?

willcl-ark commented at 10:51 AM on March 14, 2026:

squashed in 28945207085

in ci/lint_imagefile:12 in 0b7919a504 outdated

   5 | @@ -6,15 +6,32 @@
   6 |  
   7 |  FROM mirror.gcr.io/ubuntu:24.04
   8 |  
   9 | +ENV LC_ALL=C
  10 | +ENV PATH="/python_env/bin:${PATH}"
  11 | +ENV VIRTUAL_ENV="/python_env"
  12 | +ENV MLC_VERSION=v1

maflcko commented at 7:56 AM on March 14, 2026:

0b7919a5044836b34df02cfc11cd3b951b904132: should be 1.2?

maflcko commented at 8:03 AM on March 14, 2026:

Also, you are removing the comments as to which apt package is needed for what? It may be good to keep them. I think this is possible via syntax like:

$ echo apt install bla `# bla` \
>  foo `# foo` \
>  bar `# bar`

(Haven't tried this in docker)

maflcko commented at 8:04 AM on March 14, 2026:

Also, the comment about the python version file is gone.

If this is not possible to put the comments in the dockerfile, my preference would be to keep the shell script as a snippet.

willcl-ark commented at 10:32 AM on March 14, 2026:

It seems its not possible to use multi-line and comments, so will revert to shell script

in test/lint/lint-format.py:96 in 2464142c1d outdated

  91 | +        print(
  92 | +            "These files were ruff-formatted on the base branch but are no longer formatted:"
  93 | +        )
  94 | +        for f in failures:
  95 | +            print(f"  {f}")
  96 | +        print("\nRun 'ruff format' on these files to fix.")

maflcko commented at 8:10 AM on March 14, 2026:

I think it is one thing to use ruff in the lint CI, but possibly requiring it for all devs (is this a requirement?) seems spicy. What happened to yapf, yapf-diff, and black? How do they interact?

Maybe a separate pull for this commit?

willcl-ark commented at 10:49 AM on March 14, 2026:

Agreed, I'll drop this commit from this PR (but I would like it still eventually)

I don't think requiring ruff to format python code is a high bar. As you mention, we have previously "required" yapf, and do require things like clang-format.

The idea behind this commit is that the "bar" for ruff usage starts extremely low:

if the file is not (yet) formatted, it does not need to be formatted in the PR.
if the file is formatted, then you cannot break the formatting in your PR.

Only once we start formatting files (which we should) does ruff start to become required to avoid regressions.

What happened to yapf, yapf-diff, and black? How do they interact?

IIUC yapf/[-diff] and black is not enforced anywhere, so I don't think anything changes with them practically 😄

The idea behind the "astral stack" (uv, ruff, ty) is that they take over all of those roles (and more) much more comprehensively. Regarding formatting ruff format is 100% black-compatible formatting, and inherits also it's "non-configurable" ethos (well, you can configure some parts e.g. line-length, but not much).

Ruff can't format-diff, but we won't need that with this commit; you either format the entire file, or nothing at all. No diff-wise formatting introduced piecemeal; it's messy, error-prone etc..

IMO the ideal scenario for us to reach is that you can write code and either configure your editor to save+format, use a format bind in your editor, a pre-commit hook, or after you're done editing just run:

ruff format <file>
# or eventually
ruff format

and it's all done. No thinking, nop opinions, no drama :)

My intuition of how to phase it in is:

format less-often-touched files all in one go, taking a bit of an up-front hit
try to encourage contributors to add a format commit to their PR if they touch a python file and go one-at-a-time until we approach 100% formatting

maflcko commented at 7:04 AM on March 16, 2026:

I don't think requiring ruff to format python code is a high bar. As you mention, we have previously "required" yapf, and do require things like clang-format.

I don't think we do "require" them. Just now there was a clang-format unclean diff merged:

diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp
index 3724c3f2b3..333c733ce5 100644
--- a/src/wallet/wallet.cpp
+++ b/src/wallet/wallet.cpp
@@ -586 +586 @@ static bool EncryptMasterKey(const SecureString& wallet_passphrase, const CKeyin
-    for (int i = 0; i < 2; i++){
+    for (int i = 0; i < 2; i++) {

Regarding formatting ruff format is 100% black-compatible formatting, and inherits also it's "non-configurable" ethos (well, you can configure some parts e.g. line-length, but not much).

Ok, that sounds fair. So devs are free to use either black or ruff, and aren't forced to use a specific one. There could still be the issue where the auto-format doesn't look nice and a dev prefers to manually format, but maybe this is an acceptable price to pay. (I don't mind, but it could make sense to ask other devs as well)

Also, I think this script should print the diff that is expected. So that devs who want to refuse to use any tool at all can just copy-paste the diff from CI (like the iwyu diff).

Finally, the commit should probably remove .style.yapf, because yapf is not 100% compatible with black, so I guess it won't be compatible with ruff either.

willcl-ark commented at 9:55 AM on March 16, 2026:

Thanks, will bear these in mind if I re-open the lint-format.py in a future followup :)

So that devs who want to refuse to use any tool at all can just copy-paste the diff from CI (like the iwyu diff)

Sure I guess we can accommodate that 🥲

There could still be the issue where the auto-format doesn't look nice and a dev prefers to manually format...

this commit shows (partially) how to disable formatting on sections where manual layour is preferred, basically as simple as:

# fmt: off
<unformatted code>
# fmt: on

Those were pretty much the only places I (personally) thought it would be beneficial when I looked 9 months ago; looks like 12 sites to me.

maflcko commented at 8:11 AM on March 14, 2026: member

seems fine, but I left some comments about the comments

2464142c1d31db482bd6a24fc3d9af14c8519847~1 🏉

<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: 2464142c1d31db482bd6a24fc3d9af14c8519847~1 🏉
YM0UGboaJZYuNZ4u9P2+S/U0wzmQDkKBRpY/jdBJQthxEqAsYg7gntYun4QLbrOhHuDVA1gccnYaM+Uptq9CBQ==

</details>

willcl-ark force-pushed on Mar 14, 2026

DrahtBot added the label CI failed on Mar 14, 2026

DrahtBot removed the label CI failed on Mar 14, 2026

DrahtBot added the label Needs rebase on Mar 16, 2026

in ci/lint/01_install.sh:33 in cc0cb5b202 outdated

  29 | @@ -30,10 +30,7 @@ export PATH="/python_env/bin:${PATH}"
  30 |  command -v python3
  31 |  python3 --version
  32 |  
  33 | -uv pip install --python /python_env \
  34 | -  lief==0.16.6 \
  35 | -  pyzmq==27.1.0 \
  36 | -  vulture==2.14
  37 | +uv pip install --python /python_env -r /ci/lint/requirements.txt

maflcko commented at 6:54 AM on March 16, 2026:

nano nit in cc0cb5b202d523e19f0f8fbdcd7b594e6faa887a: In scripts I prefer the long option, for better self-documentation: -r -> --requirement

willcl-ark commented at 9:55 AM on March 16, 2026:

taken in 4ab42247c87

maflcko approved

maflcko commented at 7:09 AM on March 16, 2026: member

Looks like a nice change to reduce some complexity:

Use uv over manual python installation (less stuff to maintain for us)
This also allows to drop the patch version (less stuff to maintain for us)
It moves some stuff, which all beneficial (using standard files like lint/requirements.txt or ruff.toml)

I have not reviewed the ty change or the ty.toml, but it looks harmless.

in test/lint/README.md:60 in 94676f96e1

  57 |  | [`lint-shell.py`](/test/lint/lint-shell.py) | [ShellCheck](https://github.com/koalaman/shellcheck)
  58 |  | `py_lint` | [ruff](https://github.com/astral-sh/ruff)
  59 |  | markdown link check | [mlc](https://github.com/becheran/mlc)
  60 |  
  61 | -In use versions and install instructions are available in the [CI setup](../../ci/lint/01_install.sh).
  62 | +In use versions and install instructions are available in the [CI setup](../../ci/lint/01_install.sh) and the [lint_imagefile](../../ci/lint_imagefile) (for tools where an OCI imagefile exists).

maflcko commented at 7:10 AM on March 16, 2026:

llm nit: [“In use versions” is awkward; “Versions in use” (or “In-use versions”) is clearer and avoids misreading.]

willcl-ark commented at 9:55 AM on March 16, 2026:

Tried to clarify more in 720fa75f0c4

willcl-ark force-pushed on Mar 16, 2026

maflcko commented at 9:48 AM on March 16, 2026: member

Wrong dropped hunk in rebase:

3:  2894520708 ! 3:  b831bc9f08 lint: switch to ty for python typechecking
    @@ Commit message
         ty is faster and more maintained than mypy. Comes with better editor
         integrations, a language server and is written in rust (tm).
     
    - ## ci/lint/01_install.sh ##
    -@@ ci/lint/01_install.sh: python3 --version
    - 
    - uv pip install --python /python_env \
    -   lief==0.16.6 \
    --  mypy==1.19.1 \
    -   pyzmq==27.1.0 \
    -   vulture==2.14
    - 
    -

re-review ACK 720fa75f0c4ea39aeffe52c23fee121d2316fe21 🛑

<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-review ACK 720fa75f0c4ea39aeffe52c23fee121d2316fe21 🛑
6AAWiWc6jsszVbCY/bf3qZsTvNaLWMvqNSo/wSVbptKhyyEp8ke7Ji8B605qfBjvF1Om79MWFCEit1jKxq39DQ==

</details>

willcl-ark force-pushed on Mar 16, 2026

DrahtBot added the label CI failed on Mar 16, 2026

willcl-ark force-pushed on Mar 16, 2026

willcl-ark commented at 10:01 AM on March 16, 2026: member

Wrong dropped hunk in rebase:

Whoops, fixed now

maflcko commented at 10:10 AM on March 16, 2026: member

re-ACK a7bcd34c172a8d91f3f2118c6e43b6672113b652 📟

<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 a7bcd34c172a8d91f3f2118c6e43b6672113b652  📟
ebip6gE7EEveMNp94Tpa7FjH8NdH8dSLiEIXgfn987tFiwlhfUGMDPE9Wu8olVojfD2PaTtf0nAyOZ4jWHsFBQ==

</details>

DrahtBot removed the label CI failed on Mar 16, 2026

fanquake commented at 10:45 AM on March 16, 2026: member

Concept ACK

DrahtBot removed the label Needs rebase on Mar 16, 2026

maflcko commented at 11:43 AM on March 16, 2026: member

cc @Sjors (as this includes your commit)

fanquake referenced this in commit dc104cc333 on Mar 17, 2026

Sjors commented at 3:34 PM on March 19, 2026: member

27f89f92a11f7f37a2f650cd7a90e358ea158ae4 lgtm

I haven't used uv myself, but local development with PyEnv is not impacted by the CI changes in this PR.

DrahtBot added the label Needs rebase on Mar 20, 2026

willcl-ark force-pushed on Mar 20, 2026

maflcko commented at 8:55 AM on March 20, 2026: member

re-lgtm, with my initial review comment: #34547#pullrequestreview-3951783614

re-ACK b497401dd5ee1d84dbaa9217e9ebb983d590067b 🖌

<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 b497401dd5ee1d84dbaa9217e9ebb983d590067b 🖌
UAIzmRiMXOsI88wMwAzZstJBqd1l8a8yLuoZomV/Oai1H9sYRzCh8vN4r+KXJW3i6e5n5woFW3iM7NsQhj05Bg==

</details>

DrahtBot requested review from fanquake on Mar 20, 2026

DrahtBot removed the label Needs rebase on Mar 20, 2026

in ci/lint_imagefile:12 in b497401dd5

   5 | @@ -6,8 +6,13 @@
   6 |  
   7 |  FROM mirror.gcr.io/ubuntu:24.04
   8 |  
   9 | +COPY --from=ghcr.io/astral-sh/uv:latest /uv /uvx /bin/
  10 | +COPY --from=ghcr.io/astral-sh/ruff:0.15.5 /ruff /bin/
  11 | +COPY --from=ghcr.io/astral-sh/ty:0.0.21 /ty /bin/
  12 | +

stickies-v commented at 10:59 AM on March 20, 2026:

ty interface changes can happen between any versions, so pinning patch version makes sense.

Both uv and ruff use minor for breaking changes, patch is guaranteed to be non-breaking. So, I suggest we pin both uv and ruff to minor version?

willcl-ark commented at 11:47 AM on March 20, 2026:

Thank's for review these pins. I took your suggestion in the latest push.

in ci/lint/01_install.sh:26 in b497401dd5

  36 | -    clang
  37 | -  env CC=clang python-build "$(cat "/.python-version")" "${PYTHON_PATH}"
  38 | -fi
  39 | -export PATH="${PYTHON_PATH}/bin:${PATH}"
  40 | +# Install Python and create venv using uv (reads version from .python-version)
  41 | +uv python install

stickies-v commented at 11:14 AM on March 20, 2026:

nit: I don't think this line is necessary? uv venv automatically installs python

willcl-ark commented at 11:28 AM on March 20, 2026:

This does seem to be correct.

willcl-ark commented at 11:48 AM on March 20, 2026:

done in 152a04857e0

maflcko commented at 12:26 PM on March 20, 2026:

Ah interesting. I read the upstream docs on this and wasn't sure if the .python-version does apply when it is dropped, but looking at the ci log, it seems to be correct/fine to drop:

...
[#20](/bitcoin-bitcoin/20/) 11.50 Running hooks in /etc/ca-certificates/update.d...
[#20](/bitcoin-bitcoin/20/) 11.50 done.
[#20](/bitcoin-bitcoin/20/) 11.53 + uv venv /python_env
[#20](/bitcoin-bitcoin/20/) 11.60 Downloading cpython-3.10.20-linux-x86_64-gnu (download) (28.5MiB)
[#20](/bitcoin-bitcoin/20/) 12.17  Downloaded cpython-3.10.20-linux-x86_64-gnu (download)
[#20](/bitcoin-bitcoin/20/) 12.29 Using CPython 3.10.20
[#20](/bitcoin-bitcoin/20/) 12.29 Creating virtual environment at: /python_env
...

in ci/lint/01_install.sh:29 in b497401dd5 outdated

  39 | -export PATH="${PYTHON_PATH}/bin:${PATH}"
  40 | +# Install Python and create venv using uv (reads version from .python-version)
  41 | +uv python install
  42 | +uv venv /python_env
  43 | +
  44 | +export PATH="/python_env/bin:${PATH}"

stickies-v commented at 11:16 AM on March 20, 2026:

What's the point of installing into a venv and then adding the venv to PATH? At that point, doesn't it make more sense to just uv pip install --system? It seems like that was true before this PR too, though.

willcl-ark commented at 11:31 AM on March 20, 2026:

Is the system python the correct version though? I'm not sure how that works, and pretty sure --system will just print a few warnings about being dangerous each run.

For me:

download python (if needed)
create venv, and activate it (or add to front of PATH)
install deps

...is what python users have always done and would probably expect to do, and is easy to follow.

Regarding adding to $PATH, without that the rust test_runner would have to activate the venv (or we'd have to use your system install)

stickies-v commented at 11:40 AM on March 20, 2026:

is what python users have always done and would probably expect to do,

Yeah venv is definitely the right approach for most contexts, just feels a bit silly on an ephemeral CI environment if we always want to use the venv anyway. But I haven't properly thought through implications/interactions etc, so this might very well be a terrible idea. Let's not do it in this PR, or at all.

willcl-ark commented at 11:48 AM on March 20, 2026:

kept it using a venv + PATH for now, as it ain't broke :)

stickies-v approved

stickies-v commented at 11:18 AM on March 20, 2026: contributor

Light code review ACK b497401dd5ee1d84dbaa9217e9ebb983d590067b

I've been happily using uv and ruff for my own projects, and recently also switched to ty. No issues with it so far, and generally I think this is the right direction. Well maintained, documented, performant, ...

willcl-ark force-pushed on Mar 20, 2026

in ci/lint_imagefile:9 in 0809f765a0

   5 | @@ -6,8 +6,14 @@
   6 |  
   7 |  FROM mirror.gcr.io/ubuntu:24.04
   8 |  
   9 | +# Pin uv and ruff to minor version and ty to a patch version to avoid breaking changes

maflcko commented at 12:34 PM on March 20, 2026:

I think it could make sense to link to the upstream docs on breaking changes. Otherwise, the doc link has to be manually researched every time this changes, or comes up.

I am confident it will change for ty 0.1.0 (if and once it exists)?

willcl-ark commented at 9:00 PM on April 29, 2026:

done in 2424e5283672e10bc45cdbca1a8851308716b50c

maflcko commented at 12:34 PM on March 20, 2026: member

Haven't read the upstream docs on versioning, but lgtm.

review ACK 0809f765a080faf51cdabb2bb4382aad0c06780f 🦃

<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: review ACK 0809f765a080faf51cdabb2bb4382aad0c06780f 🦃
fPIwXhbOOb7U10ERDsxCSSrZ7d9fVM4vQKi3HDoQGuetw8DWu76Rvcnp/TkTed8qxJOKlQs/qgMxZ3NJEPGwDg==

</details>

DrahtBot requested review from stickies-v on Mar 20, 2026

stickies-v commented at 2:19 PM on March 20, 2026: contributor

re-lightcodereview ACK 0809f765a080faf51cdabb2bb4382aad0c06780f

in ty.toml:8 in 0809f765a0

   0 | @@ -0,0 +1,22 @@
   1 | +[src]
   2 | +include = ["test/functional/**", "contrib/devtools/**"]
   3 | +
   4 | +[environment]
   5 | +root = ["test/functional"]
   6 | +
   7 | +[rules]
   8 | +deprecated = "ignore"

fanquake commented at 1:30 AM on March 23, 2026:

deprecated = "ignore"

This can be removed.

willcl-ark commented at 9:00 PM on April 29, 2026:

fixed by dropping ty migration from this pull

DrahtBot requested review from fanquake on Mar 23, 2026

fanquake commented at 2:18 AM on March 23, 2026: member

I tried testing this by introducing typing errors that are picked up in master (559df68240b9c74184a7da005f9ac2bbc52248a5):

diff --git a/test/functional/feature_assumeutxo.py b/test/functional/feature_assumeutxo.py
index 90f597de17..1b3f2a26cd 100755
--- a/test/functional/feature_assumeutxo.py
+++ b/test/functional/feature_assumeutxo.py
@@ -558,7 +558,7 @@ class AssumeutxoTest(BitcoinTestFramework):
         txout_result = n1.gettxout(coinbase_tx['txid'], 0)
         assert_equal(txout_result['scriptPubKey']['desc'], coinbase_output_descriptor)
 
-        def check_tx_counts(final: bool) -> None:
+        def check_tx_counts(final: bool) -> bool:
             """Check nTx and nChainTx intermediate values right after loading
             the snapshot, and final values after the snapshot is validated."""
             for height, block in blocks.items():
diff --git a/test/functional/p2p_addrfetch.py b/test/functional/p2p_addrfetch.py
index 49c131f105..18e676c989 100755
--- a/test/functional/p2p_addrfetch.py
+++ b/test/functional/p2p_addrfetch.py
@@ -23,7 +23,7 @@ from test_framework.util import assert_equal
 ADDR = CAddress()
 ADDR.time = int(time.time())
 ADDR.nServices = P2P_SERVICES
-ADDR.ip = "192.0.0.8"
+ADDR.ip: bool = "192.0.0.8"
 ADDR.port = 18444

test/functional/p2p_addrfetch.py:26: error: Type cannot be declared in assignment to non-self attribute  [misc]
test/functional/feature_assumeutxo.py:561: error: Missing return statement  [return]
Found 2 errors in 2 files (checked 311 source files)
^---- ⚠️ Failure generated from lint-python.py
^^^

^---- ⚠️ Failure generated from lint check 'all_python_linters' (Run all linters of the form: test/lint/lint-*.py)!

However this branch (rebased) passes fine?

willcl-ark commented at 1:02 PM on March 23, 2026: member

I tried testing this by introducing typing errors that are picked up in master (559df68): However this branch (rebased) passes fine?

Yes, this is expected and documented (poorly, apologies) in PR description here:

Further work can drop individual rules from ty.toml fixing up the infringing code as necessary.

My bad; I should have fleshed out what this means more concretely in the description: whilst we have ignore rules in ty.toml then certain things will go undetected (ignored), and this may be a temporary regression against master until the infracting code is fixed and the rule is un-ignored.

If this is undesirable, then we should also bundle the fixes and rule-removals into this PR. I did not do this originally as some of the rules required quite invasive changes, so the plan was to add these incrementally in reviewable-chunks, if this change made it in.

I will update the PR description to outline this more clearly.

However to avoid the most egregious regressions I propose that we include fixes for the 5 least-invasive rules in this PR, which I will add in the next push to this branch. This is ~ 10 files changed, 28 insertions(+), 23 deletions(-). This will leave us with 11 ignore rules on master to work through.

willcl-ark marked this as a draft on Mar 23, 2026

maflcko commented at 1:33 PM on April 7, 2026: member

Maybe b45433f76147690bf62cfac86e2fc118049d833d can be done in a separate pull. The other commits should be fine, after the nit in #34547 (review) is addressed.

lint: switch to uv for python management in linter

https://docs.astral.sh/uv/

Install python in the linter using uv and a venv.
This is faster and more simple than building pyenv.

a53b81ce4e

lint: switch to ruff for formatting and linting

- use dedicated ruff.toml for configuration
- download via docker image layer at build time

5f4d3383da

lint: use requirements.txt fd15b55c2e

Don't pin Python patch version

.python-version always matches the minimum supported Python version.
It's main purpose is to catch accidental use of too modern syntax
in scripts and functional tests.

We (currently) don't specify a minimum patch version, so it's not
necessary to do so here. The minor verion is enough.

This also avoids requiring users to keep a potentially unsafe old
patch version installed.

5fefa5a654

lint: doc: detail lint tool install methods

Installing tools in the dockerfile using `COPY --from` is better , but
not all tools we use publish an OCI image to a non-docker.io registry.

As we are frequently rate-limited from docker.io, only install tools
which publish to another registry, e.g. ghcr.io.

2424e52836

willcl-ark force-pushed on Apr 29, 2026

willcl-ark marked this as ready for review on Apr 29, 2026

willcl-ark commented at 8:59 PM on April 29, 2026: member

Sorry for the delay here, thank you for the review so far.

Maybe b45433f can be done in a separate pull. The other commits should be fine, after the nit in #34547 (comment) is addressed.

This is sensisble advice and I've taken it, thanks.

Having ruminated more on this, I think the ty migration needs to occur in reverse order ideally: i) fix all issues up-front and ii) make the switch with all rules enabled. Otherwise we will either need to do it all in one large PR, or suffer a temporary period where errors like @fanquake demonstrated can slip through, until we enable all rules some time in the future. This doesn't make good sense.

I have commits to fix (most of) our current typechecker failures, but will keep these seperate from this PR.

<details> <summary>Details</summary>

646f6bbb3fc (HEAD -> modernise-linter-ty) lint: suppress pycapnp ty false positives
a5997ba4ab7 (origin/typechecker-fixes, typechecker-fixes) lint: fix unresolved-attribute
3f8d880b03f lint: fix unsupported-operator
7d6bd928fcc lint: fix invalid-argument-type
db57cdfd2c2 lint: fix not-subscriptable
ee05f604f38 lint: fix invalid-method-override
a5b70b0389a lint: fix invalid-assignment
968675b29ab lint: fix possibly-missing-submodule
f61228cab6f lint: fix not-iterable
164d805fbd9 lint: fix no-matching-overload
e2e744ecebf lint: fix unused-type-ignore-comment

</details>

DrahtBot added the label CI failed on Apr 29, 2026

DrahtBot removed the label CI failed on Apr 30, 2026

maflcko commented at 7:17 AM on May 13, 2026: member

Only change is dropping the ty commit, so this is mostly move-only.

review ACK 2424e5283672e10bc45cdbca1a8851308716b50c 🗿

<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: review ACK 2424e5283672e10bc45cdbca1a8851308716b50c 🗿
dSNJtVoo3uFK3C5b19VzoPlR0CEQ/gV2sTN3o7hP2UL8kB62P0Fhbi5Pote3e7DRbJml4i2rFo1H5Wo2xbY0Dw==

</details>

sedited approved

sedited commented at 8:21 AM on May 13, 2026: contributor

ACK 2424e5283672e10bc45cdbca1a8851308716b50c

sedited merged this on May 13, 2026

sedited closed this on May 13, 2026