Get rid of untested sizeof(secp256k1_ge_storage) == 64 code path #1480

pull real-or-random wants to merge 3 commits into bitcoin-core:master from real-or-random:202401-static-assert changing 3 files +81 −74
  1. real-or-random commented at 3:25 PM on January 8, 2024: contributor

    This gets rid of an untested code path. Resolves #1352.

    This is a bit opinionated in the sense that it adds a static assertion where it's needed in secp256k1_pubkey_save and secp256k1_pubkey_load. I think this is justified in this case. It helps the reviewer verify that these functions are correct.

    See individual commit messages.

  2. util: Add STATIC_ASSERT macro d0ba2abbff
  3. Require that sizeof(secp256k1_ge_storage) == 64
    This gets rid of an untested code path. Resolves #1352.
    
    secp256k1_ge_storage is a struct with two secp256k1_fe_storage fields.
    The C standard allows the compiler to add padding between the fields and
    at the end of the struct, but no sane compiler in the end would do this:
    The only reason to add padding is to ensure alignment, but such padding
    is never necessary between two fields of the same type.
    
    Similarly, secp256k1_fe_storage is a struct with a single array of
    uintXX_t. No padding is allowed between array elements. Again, C allows
    the compiler to insert padding at the end of the struct, but there's no
    absolute reason to do so in this case.
    
    For the uintXX_t itself, this guaranteed to have no padding bits, i.e.,
    it's guaranteed to have exactly XX bits.
    
    So I claim that for any existing compiler in the real world,
    sizeof(secp256k1_ge_storage) == 64.
    e53c2d9ffc
  4. assumptions: Use new STATIC_ASSERT macro
    This also splits the big "&&" expression into separate expressions. If
    we ever see an assertion fail, the error message will tell it precisely
    which one failed.
    ba5d72d626
  5. real-or-random added the label assurance on Jan 9, 2024
  6. sipa commented at 3:50 PM on January 9, 2024: contributor

    utACK ba5d72d62659f9305d2be30b2ac89ce9480a0e78

  7. jonasnick approved
  8. jonasnick commented at 4:46 PM on January 9, 2024: contributor

    ACK ba5d72d62659f9305d2be30b2ac89ce9480a0e78

  9. real-or-random merged this on Jan 9, 2024
  10. real-or-random closed this on Jan 9, 2024

  11. vmta referenced this in commit 9cc5271211 on Jan 15, 2024
  12. achow101 referenced this in commit 9d962e5e0d on Jan 15, 2024
  13. fjahr referenced this in commit d5f970e647 on Feb 25, 2024
  14. achow101 referenced this in commit edc3e9107b on Mar 20, 2024
  15. achow101 referenced this in commit b525369f8e on Mar 25, 2024
  16. achow101 referenced this in commit 2189d2f841 on Apr 1, 2024
  17. fanquake referenced this in commit 5354807b00 on Apr 4, 2024
  18. fanquake referenced this in commit 53eec53dca on Apr 4, 2024
  19. hebasto referenced this in commit b6de625950 on May 11, 2024
  20. janus referenced this in commit 939262869a on Jun 6, 2024
  21. delta1 referenced this in commit 6089844b3c on Apr 2, 2025
  22. div72 referenced this in commit af627d47c3 on Apr 12, 2025
  23. oskarszoon referenced this in commit 0d5c9260f4 on Jul 1, 2025

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