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

rebase on bitcoin-core/secp256k1 #85

Closed
benma opened this issue Oct 8, 2019 · 8 comments
Closed

rebase on bitcoin-core/secp256k1 #85

benma opened this issue Oct 8, 2019 · 8 comments

Comments

@benma
Copy link
Contributor

benma commented Oct 8, 2019

Hi

I would have done a PR, but I was not sure how you handle rebasing.

Anyway, it would be great to rebase this on upstream. I am particularly interested in pulling in this change: bitcoin-core/secp256k1#337, so I can skip some custom commits on my fork of libwally-core.

@real-or-random
Copy link
Collaborator

The last rebase has been 6 months ago, so yes we can do this if it's helpful for you. I can't promise you a date though.

We typically we go with at least two people in parallel through the process and check that we arrive at the same code at a few checkpoints on the way. So yes, no need to open a PR, we'll handle it.

@benma
Copy link
Contributor Author

benma commented Oct 8, 2019

Ah, didn't realize it was so involved. In this case, thanks! No need to rush, I can continue with manual patching for a while.

@apoelstra
Copy link
Contributor

We should also warn the Trezor people and find a minimally disruptive way to do things, this time. (Maybe it is sufficient to just rename the old branch and then they need to change nothing? I don't know.)

@real-or-random
Copy link
Collaborator

I think we settled on renaming the old branch to secp256k1-zkp-YYYY-MM-DD, see your comment here: #68 (comment)

@benma
Copy link
Contributor Author

benma commented Apr 21, 2020

Hello again. Are there any plans to rebase? There have been a bunch of interesting but also invasive PRs which makes it harder to apply custom patches on both secp256k1 repos (upstream and this one):

bitcoin-core/secp256k1#708
bitcoin-core/secp256k1#709
bitcoin-core/secp256k1#710

Would be great to update this repo again. I am guessing that more frequent rebasing is also less effort overall.

Happy to help if I can be of assistance, let me know.

@real-or-random
Copy link
Collaborator

Good points, we should think about this now. On the other hand, there will hopefully be a secp256k1 release soon, so maybe it's better to wait? No idea at the moment, we should discuss this.

Independently of this, I suggested a while ago to move some code here around. The goal is not to touch any upstream in our fork here (except maybe build system files). This is realistic because we only add features to upstream but don't change then. This will make rebasing a little bit easier.

@benma
Copy link
Contributor Author

benma commented Apr 21, 2020

On the other hand, there will hopefully be a secp256k1 release soon

What do you mean? The secp256k1 repo doesn't have releases nor tags.

Independently of this, I suggested a while ago to move some code here around.

Sounds good. Whatever is fastest :)

tomtau pushed a commit to crypto-com/secp256k1-zkp that referenced this issue Jul 9, 2020
…ck-rust-ecosystem

Fix cc dep as the rust ecosystem is terrible
@jonasnick
Copy link
Contributor

Fixed by #91

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 a pull request may close this issue.

4 participants