Skip to content

Conversation

Copy link

Copilot AI commented Nov 7, 2025

Thanks for assigning this issue to me. I'm starting to work on it and will keep this PR's description up to date as I form a plan and make progress.

Original prompt

This section details on the original issue you should resolve

<issue_title>_ecmult_wnaf relies on int having at least 32 value bits</issue_title>
<issue_description>The current secp256k1_ecmult_wnaf needs the unstated and unchecked assumption that int has at least 32 value bits when it VERIFY_CHECKs that w <= 31. In practice, we call it only with WINDOW_A == 5 and WINDOW_G == ECMULT_WINDOW_SIZE where the latter is configurable in the range 2..24.

A consequence of this "bug" is that the code fails on a 16-bit platform if you set ECMULT_WINDOW_SIZE > 16. I don't think we need to support this, but code without unchecked assumptions is bad. So I suggest that we rewrite the function to use int32_t instead of int even if we don't use my macro approach. (We can keep the "bit position" arguments and variables int.)

Alternatively, we could add the assumption that INT_MAX >= INT32_MAX but this forbids 16-bit platforms, and the code seems to work on them in principle; see #792 (comment).

Originally posted by @real-or-random in #1761 (review)
</issue_description>

<agent_instructions>Use the suggestion to use int32_t instead of int. Keep the "bit positions" variables and arguments int.</agent_instructions>

Comments on the Issue (you are @copilot in this section)


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

@real-or-random
Copy link
Contributor

@copilot Are you still alive?

@real-or-random
Copy link
Contributor

This was a GitHub Actions permissions issue: https://github.com/orgs/community/discussions/179396

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

_ecmult_wnaf relies on int having at least 32 value bits

2 participants