tests: lint wycheproof's python script #1279

pull RandomLattice wants to merge 1 commits into bitcoin-core:master from RandomLattice:lint changing 1 files +22 −21
  1. RandomLattice commented at 11:40 AM on April 14, 2023: contributor

    This PR lints tests_wycheproof_generate.py according to bitcoin's python linting scripts. This is a follow-up to PR #1245.

  2. sipa commented at 12:46 PM on April 14, 2023: contributor

    Some of the changes here are clear improvements (dropping unnecessary imports, for example), and others probably do make the code more "standard" Python, so I'm inclined to take it - not because it satisfies some linter but just because it improves the code.

    But if that's the motivation, perhaps we can go a bit further, and see what other linters say. pylint in addition complains about:

    • Too long lines (meh)
    • Variables whose names are too short (meh)
    • Constants that are not UPPER_CASE styled (mildly in favor)
    • Using f-strings for formatting instead of using string manipulation (definitely more readable, I think)
    • On line 59 and 72, "Consider iterating the dictionary directly instead of calling .keys()" (seems reasonable).

    WDYT?

  3. real-or-random commented at 1:16 PM on April 14, 2023: contributor

    @sipa Makes sense.

    But if your "disable linting" in https://github.com/bitcoin/bitcoin/pull/27445 is just a WIP commit you don't want to have there right now, I'm also happy to merge this PR here quickly. We know that the current changes make Core's linter happy. We should then address pylint stuff in a separate PR.

  4. sipa commented at 1:18 PM on April 14, 2023: contributor

    I don't think we should make any further changes in this repo.

    I'll update the PR there to disable linting of the subtree in a less WIPy way.

  5. real-or-random commented at 1:21 PM on April 14, 2023: contributor

    Ok, just sounds good! Either approach is fine with me.

  6. RandomLattice commented at 1:24 PM on April 14, 2023: contributor

    Sounds like we can update this PR to make pylint happier (addressing the points that @sipa raised), but let's decouple this PR from the ongoing secp256k1 bump in bitcoin. Does this sound good?

  7. sipa commented at 1:25 PM on April 14, 2023: contributor

    @RandomLattice SGTM

  8. RandomLattice force-pushed on Apr 14, 2023
  9. RandomLattice commented at 4:38 PM on April 14, 2023: contributor

    I added some pylint suggestions but not all. I took:

    • Using f-strings for formatting (makes it more readable)
    • Iterate the dictionary instead of calling .keys()

    I did not take:

    • Lines too long
    • Variable names are too short
    • UPPER_CASE style for variable names, since those aren't constant

    These changes do not change the input/output behavior of the script.

  10. tests: lint wycheproof's python script
    This PR lints tests_wycheproof_generate.py according to pylint.
This is a follow-up to PR #1245.

Co-authored-by: Sean Andersen <6730974+andozw@users.noreply.github.com>
    35ada3b954
  11. in tools/tests_wycheproof_generate.py:84 in ecbbdbbc2e outdated 
      88 | -                offset_sig,
  89 | -                sig_size,
  90 | -                expected_verify) + " },\n"
  91 | -        if new_msg: offset_msg_running += msg_size
  92 | -        if new_pk: offset_pk_running += 65
  93 | +        out += "  {" + f"{pk_offset}, {msg_offset}, {msg_size}, {offset_sig}, {sig_size}, {expected_verify}" + " },\n"
    sipa commented at 4:40 PM on April 14, 2023:

    The " {" and " },\n" parts can be part of the f-string too.

    RandomLattice commented at 4:57 PM on April 14, 2023:

    👍 addressed, thanks. The single { needs escaping into {{ inside a f-string but probably still readable

  12. RandomLattice force-pushed on Apr 14, 2023
  13. real-or-random approved
  14. real-or-random commented at 12:09 PM on April 19, 2023: contributor

    utACK 35ada3b954ccc6c54628fb3bcc0365d176297019

  15. sipa commented at 1:48 PM on April 19, 2023: contributor

    utACK 35ada3b954ccc6c54628fb3bcc0365d176297019

  16. real-or-random merged this on Apr 19, 2023
  17. real-or-random closed this on Apr 19, 2023
  18. sipa referenced this in commit b4eb644b6c on May 12, 2023
  19. hebasto referenced this in commit 49c52ea2b1 on May 13, 2023
  20. vmta referenced this in commit e1120c94a1 on Jun 4, 2023
  21. sipa referenced this in commit 901336eee7 on Jun 21, 2023
  22. vmta referenced this in commit 8f03457eed on Jul 1, 2023
  23. delta1 referenced this in commit 3f32c20932 on Aug 8, 2023
  24. delta1 referenced this in commit 31ac0c1081 on Aug 31, 2023
  25. janus referenced this in commit c4348d88db on Sep 11, 2023
  26. div72 referenced this in commit af627d47c3 on Apr 12, 2025
  27. str4d referenced this in commit 5a6bf5f178 on Jun 4, 2025
  28. Fabcien referenced this in commit b5f0ce615a on Apr 20, 2026
  29. Fabcien referenced this in commit 1af86307c0 on Apr 20, 2026
Contributors
RandomLatticesipareal-or-random
Linked (view graph)
#1245 tests: Add Wycheproof ECDSA vectors
github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin-core/secp256k1. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-05-19 06:52 UTC