Skip to content

secp256k1-sys: fix lowmemory feature#799

Merged
apoelstra merged 1 commit intorust-bitcoin:masterfrom
apoelstra:2025-06--lowmemory
Jun 12, 2025
Merged

secp256k1-sys: fix lowmemory feature#799
apoelstra merged 1 commit intorust-bitcoin:masterfrom
apoelstra:2025-06--lowmemory

Conversation

@apoelstra
Copy link
Copy Markdown
Member

When upstream switched to multicomb they changed the #define flags needed to reduce the size of the ecmult_gen precomp table. We should have updated our bulid.rs.

Before this change, on my system the secp256k1-sys rlib with the lowmemory feature has size 630602. After this change, it has size 610090, a 3.3% reduction. Probably not worth backporting, although it's not a breaking change and we totally could backport it.

Fixes #795

When upstream switched to multicomb they changed the #define flags
needed to reduce the size of the ecmult_gen precomp table. We should
have updated our bulid.rs.

Before this change, on my system the secp256k1-sys rlib with the
lowmemory feature has size 630602. After this change, it has size
610090, a 3.3% reduction. Probably not worth backporting, although
it's not a breaking change and we totally could backport it.

Fixes rust-bitcoin#795
Copy link
Copy Markdown
Member Author

@apoelstra apoelstra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On 37876a7 successfully ran local tests

@tcharding
Copy link
Copy Markdown
Member

Man do you have any reviewers in this crate ATM? I'm starting to have enough life force back to start expanding my work horizons again. I can't meaningfully review this without digging back into this crate and upstream. Thought I'd ask first to see if its a useful use of my time. Equally I could expand up to miniscript instead, which I've also been ignoring, probably best to only go one way at first. Which is more valuable to you?

@apoelstra
Copy link
Copy Markdown
Member Author

@tcharding no, there are no reviewers here. Things just rot unless there is enough popular demand for me to start phoning people. I would greatly appreciate your reviews.

In this case the change looks scary but it's really not. You can see some magic #define strings have changed. You just need to look upstream for commits that change those names, and look just at the changes to configure.ac. You don't need to look at any actual source code.

Copy link
Copy Markdown
Member

@tcharding tcharding left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK 37876a7

@tcharding
Copy link
Copy Markdown
Member

Confirmed the config values were sane by reading the PR description: bitcoin-core/secp256k1#1058

@apoelstra apoelstra merged commit ba90a01 into rust-bitcoin:master Jun 12, 2025
28 checks passed
@apoelstra apoelstra deleted the 2025-06--lowmemory branch June 12, 2025 13:36
chain-forgexcr45 added a commit to chain-forgexcr45/rust-secp256k1 that referenced this pull request Sep 28, 2025
… feature

37876a700237a55abb9eb4b506556d750e7072e8 secp256k1-sys: fix `lowmemory` feature (Andrew Poelstra)

Pull request description:

  When upstream switched to multicomb they changed the #define flags needed to reduce the size of the ecmult_gen precomp table. We should have updated our bulid.rs.

  Before this change, on my system the secp256k1-sys rlib with the lowmemory feature has size 630602. After this change, it has size 610090, a 3.3% reduction. Probably not worth backporting, although it's not a breaking change and we totally could backport it.

  Fixes #795

ACKs for top commit:
  tcharding:
    ACK 37876a700237a55abb9eb4b506556d750e7072e8

Tree-SHA512: 6470a297f601a3f47c423f3e2c128c30d969fc397733b73acd9781d2b6e9760b80e113c4e7f024cda22c0bca6e3072cdaf90e403f6d7196809f55f5b573b99cc
william2332-limf added a commit to william2332-limf/rust-secp256k1 that referenced this pull request Oct 2, 2025
… feature

37876a700237a55abb9eb4b506556d750e7072e8 secp256k1-sys: fix `lowmemory` feature (Andrew Poelstra)

Pull request description:

  When upstream switched to multicomb they changed the #define flags needed to reduce the size of the ecmult_gen precomp table. We should have updated our bulid.rs.

  Before this change, on my system the secp256k1-sys rlib with the lowmemory feature has size 630602. After this change, it has size 610090, a 3.3% reduction. Probably not worth backporting, although it's not a breaking change and we totally could backport it.

  Fixes #795

ACKs for top commit:
  tcharding:
    ACK 37876a700237a55abb9eb4b506556d750e7072e8

Tree-SHA512: 6470a297f601a3f47c423f3e2c128c30d969fc397733b73acd9781d2b6e9760b80e113c4e7f024cda22c0bca6e3072cdaf90e403f6d7196809f55f5b573b99cc
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.

Configurable precomputation tables

2 participants