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.

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

Restore the ‘forked from’ for rust-secp256k1-zkp and secp256k1-zkp #30

Open
yeastplume opened this issue Sep 25, 2018 · 4 comments
Open

Comments

@yeastplume
Copy link
Member

from @garyyu:

The github project fork network is important, it will bring much more eyes on the fixes on the whole fork network, and will bring more developers to contribute on it.

Unfortunately, for some history reason (there were a lot of changes and do-overs of the Grin secp256k libs… they used to be integrated right into the grin repo but were pulled out and rebuilt, etc), The forked from info is lost, both at https://github.com/mimblewimble/rust-secp256k1-zkp 1 and at https://github.com/mimblewimble/rust-secp256k1-zkp 1

I propose to restore this forked from info.

As a demo here: https://github.com/garyyu/rust-secp256k1 3, I can ‘restore’ the forked from. (Note: it’s not a true restore! because github don’t support this function and I guess it’s not allowed in github, forever. More exactly, it’s a re-creation.)

https://github.com/rust-bitcoin/rust-secp256k1 1 is the original forked from of https://github.com/mimblewimble/rust-secp256k1-zkp 1 . Btw, rust-bitcoin/rust-secp256k1 has 53 fork, but mimblewimble/rust-secp256k1-zkp only have 8.

How to restore the forked from?

fork rust-bitcoin/rust-secp256k1 into mimblewimble
find the real fork point, for example: https://github.com/garyyu/rust-secp256k1/releases/tag/fork-point
remove the master branch, and create new master branch from fork-point
merge from master at mimblewimble/rust-secp256k1-zkp
remove all exising tags
push all mimblewimble/rust-secp256k1-zkp tags
Done.

After the new repo is ready, we can change its name as rust-secp256k1-zkp and rename the old one as (obsoleted)rust-secp256k1-zkp.

A small problem: there’s no good way to restore the exising github issues and github pull-request. But since only 5 closed issues and 21 closed PR, it should be acceptable.

And one more thing: rust-bitcoin/rust-secp256k1 doesn’t split the depend/secp256k1 as an indepent repo, so we can do the same thing, to obsolete the mimblewimble/secp256k1-zkp. Or perhaps do the same thing for mimblewimble/secp256k1-zkp.

@yeastplume
Copy link
Member Author

yeastplume commented Sep 25, 2018

Just to keep in mind:

  • secp256k1-zkp is already very well in sync with upstream. There's a vendor branch in the respository that's quite up-to-date. We will want to perform another merge from upstream after Bulletproofs (rangeproofs only) BlockstreamResearch/secp256k1-zkp#23 is merged, so best to wait for that.
  • The upstream rust-secp245k1-zkp has changed somewhat, but our branch has quite a lot of features and additions around aggsig and bulletproofs that don't exist upstream, as well as some that aren't likely to exist upstream for a while (multi-party bulletproofs, messages in bulletproofs, our own particular aggsig API)

Also, unless there's a breaking change I'd prefer we wait until well after T4 is underway to look at this. There's already quite a lot to juggle for T4 already, what we have here at the moment is working and I'd rather not add another set of changes to the pile right at the moment.

@yeastplume
Copy link
Member Author

Also, just to add I'm not concerned with whether there's a 'forked from' tag or not, just that upstream changes can be merged in the most painless way possible. We're about as close as we can get already with secp256k1-zkp, but we still need a bit of investigation into the exact changes between rust-secp245k1 and our current branch. It may be the case that it's easier to split off altogether and not worry about it.

@garyyu
Copy link
Contributor

garyyu commented Sep 25, 2018

👍 Ok, after T4. Thanks~

@ignopeverell
Copy link
Contributor

I agree both with the intent and the caveats to wait a bit longer.

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

No branches or pull requests

3 participants