Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Upgrade the vendored libsecp256k1 code to v0.2.0 #567

Merged
merged 2 commits into from
Dec 21, 2022
Merged

Upgrade the vendored libsecp256k1 code to v0.2.0 #567

merged 2 commits into from
Dec 21, 2022

Conversation

tcharding
Copy link
Member

@tcharding tcharding commented Dec 16, 2022 •
edited
Loading

This bumps secp256k1 to v0.26.0 and secp256k1-sys to v0.8.0

libsecp256k1 v0.2.0 was just released.

Update the vendored code using

./vendor-libsecp.sh depend 0_8_0 21ffe4b

git show 21ffe4b
commit 21ffe4b22a9683cf24ae0763359e401d1284cc7a (tag: v0.2.0)
Merge: 8c949f5 e025ccd
Author: Pieter Wuille <[email protected]>
Date:   Mon Dec 12 17:00:52 2022 -0500

    Merge bitcoin-core/secp256k1#1055: Prepare initial release

    e025ccdf7473702a76bb13d763dc096548ffefba release: prepare for initial release 0.2.0 (Jonas Nick)
    6d1784a2e2c1c5a8d89ffb08a7f76fa15e84fff5 build: add missing files to EXTRA_DIST (Jonas Nick)
    13bf1b6b324f2ed1c1fb4c8d17a4febd3556839e changelog: make order of change types match keepachangelog.com (Jonas Nick)
    b1f992a552785395d2e60b10862626fd11f66f84 doc: improve release process (Jonas Nick)
    ad39e2dc417f85c1577a6a6a9c519f5c60453def build: change package version to 0.1.0-dev (Jonas Nick)
    90618e9263ebc2a0d73d487d6d94fd3af96b973c doc: move CHANGELOG from doc/ to root directory (Jonas Nick)

    Pull request description:

      Based on #964

    ACKs for top commit:
      sipa:
        ACK e025ccdf7473702a76bb13d763dc096548ffefba

    Tree-SHA512: b9ab71d7362537d383a32b5e321ef44069f00e3e92340375bcd662267bc5a60c2bad60222998e6602cfac24ad65efb23d772eac37c86065036b90ef090b54c49

Requires a new version of secp256k1-sys, use v0.8.0

  • Update the secp256k1-sys manifest (including links field)
  • Update symbols to use 0_8_0
  • Add a changelog entry
  • depend on the new version in secp256k1

Which in turn requires a new version of secp256k1, use v0.26.0

@tcharding
Copy link
Member Author

Hope I'm not stealing anyones thunder here, I gave you guys a day or two to do it :)

@tcharding tcharding marked this pull request as draft December 16, 2022 00:55
@tcharding tcharding force-pushed the 12-16-secp-sys-0.2.0 branch from ccc5d29 to 9907c88 Compare December 16, 2022 01:05
@elichai
Copy link
Member

elichai commented Dec 16, 2022

I think the scratch_impl.h.patch is outdated or something, because it seems that the malloc and free calls are still there

@apoelstra
Copy link
Member

Nice, thanks!! You're not stepping on my thunder -- but yeah, it looks like me (or one of the folks involved with the vendoring patch stuff) will have to dig into it.

@tcharding
Copy link
Member Author

Ah cheers, that's enough info for me to have another play with it. But by all means don't let me hold you back :)

@tcharding
Update scratch_impl.h patch file
2d4aacc 
To mirror recent changes to the `scratch_impl.h` file update the patch
file.
@tcharding tcharding force-pushed the 12-16-secp-sys-0.2.0 branch from 9907c88 to 7a515d7 Compare December 19, 2022 04:16
@tcharding
Copy link
Member Author

ok, that took an embarrassingly long time to figure out. I just had to work out how to modify one of the patch files to resemble the code in v0.2.0 - see patch 1 of the PR.

@tcharding tcharding marked this pull request as ready for review December 19, 2022 04:31
@apoelstra
Copy link
Member

Lol, we should really document how to generate these patch files ... IIRC I've done it twice, but several times I've just edited the patchfiles directly in vim.

@apoelstra
Copy link
Member

The commit message shows an old commit ID; it should be 9a8d65f.

Otherwise looks good, ACK!

@tcharding
Copy link
Member Author

tcharding commented Dec 19, 2022 •
edited
Loading

Lol, we should really document how to generate these patch files ... IIRC I've done it twice, but several times I've just edited the patchfiles directly in vim.

It was obvious what needed doing, I just never use patch because I always use git - also I was up all night loosing money on France :)

@tcharding
Copy link
Member Author

The commit message shows an old commit ID; it should be 9a8d65f.

Are you sure, I used the tagged commit. Isn't 02ebc29 release cleanup: bump version after 0.2.0 post release changes?

@apoelstra
Copy link
Member

Yeah, the ID in your commit message would've made more sense, but the one you actually used, according to depend/secp256k1-HEAD-revision.txt, was 9a8d65f07f171b07bd7a33828dce073d819fbdef.

I would accept either one, but right now they're not consistent.

@tcharding tcharding force-pushed the 12-16-secp-sys-0.2.0 branch from 7a515d7 to 4662d83 Compare December 20, 2022 02:27
@tcharding
Copy link
Member Author

Oh I see what you mean, thanks. I must have botched the ./vendor-libsecp.sh command at some stage by omitting the commit id.

Fixed now.

@apoelstra
Copy link
Member

Looks like you swapped the commit ID and the version # in the script :)

@tcharding
Upgrade the vendored libsecp256k1 code
2dad589 
`libsecp256k1` v0.2.0 was just released.

Update the vendored code using

 `./vendor-libsecp.sh depend 0_8_0 21ffe4b`

```
git show 21ffe4b
commit 21ffe4b (tag: v0.2.0)
Merge: 8c949f5 e025ccd
Author: Pieter Wuille <[email protected]>
Date:   Mon Dec 12 17:00:52 2022 -0500

    Merge bitcoin-core/secp256k1#1055: Prepare initial release

    e025ccd release: prepare for initial release 0.2.0 (Jonas Nick)
    6d1784a build: add missing files to EXTRA_DIST (Jonas Nick)
    13bf1b6 changelog: make order of change types match keepachangelog.com (Jonas Nick)
    b1f992a doc: improve release process (Jonas Nick)
    ad39e2d build: change package version to 0.1.0-dev (Jonas Nick)
    90618e9 doc: move CHANGELOG from doc/ to root directory (Jonas Nick)

    Pull request description:

      Based on #964

    ACKs for top commit:
      sipa:
        ACK e025ccd

    Tree-SHA512: b9ab71d7362537d383a32b5e321ef44069f00e3e92340375bcd662267bc5a60c2bad60222998e6602cfac24ad65efb23d772eac37c86065036b90ef090b54c49
    ```

Requires a new version of `secp256k1-sys`, use v0.8.0

- Update the `secp256k1-sys` manifest (including links field)
- Update symbols to use 0_8_0
- Add a changelog entry
- depend on the new version in `secp256k1`

Which in turn requires a new version of `secp256k1`, use v0.26.0
@tcharding tcharding force-pushed the 12-16-secp-sys-0.2.0 branch from 4662d83 to 2dad589 Compare December 20, 2022 21:13
@tcharding
Copy link
Member Author

Might have been quicker for you to do this one yourself :)

apoelstra
apoelstra approved these changes Dec 21, 2022
View reviewed changes
Copy link
Member

@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.

ACK 2dad589

@apoelstra apoelstra merged commit 748dcd9 into rust-bitcoin:master Dec 21, 2022
@tcharding tcharding deleted the 12-16-secp-sys-0.2.0 branch December 29, 2022 02:02
@apoelstra apoelstra mentioned this pull request Jan 15, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@apoelstra apoelstra apoelstra approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@tcharding @elichai @apoelstra