Page MenuHomePhabricator

Update secp256k1 README
ClosedPublic

Authored by Mengerian on Jan 2 2020, 21:07.

Details

Summary

Now that secp256k1 is being bundled as it's own repo on Github,
it would be nice to have a more accurate and complete README.

Test Plan

Read it, make sure it's complete and accurate.

Diff Detail

Repository
rABC Bitcoin ABC
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

@markblundeberg It may also be good to add some Schnorr information to the "Implementation Details" portion, and maybe a link to the BCH Schnorr spec on bitcoincash.org. Please let me know if you have any suggestions.

src/secp256k1/README.md
4 ↗(On Diff #15113)

@jasonbcox Is there some link we could use for the equivalent information for Bitcoin ABC's automated builds?

src/secp256k1/README.md
4 ↗(On Diff #15113)

It's not a perfect comparison, but our default master CI config builds and tests libsecp256k1 in addition to bitcoind: https://build.bitcoinabc.org/viewType.html?buildTypeId=BitcoinABCMasterLinux If this build is green, so is libsecp256k1.

That said, it would be pretty easy to move it to a stand-alone build (see contrib/teamcity/build-configurations.sh)

@jasonbcox, @Mengerian FYI I plan to update the current build-default by splitting it into 3: build-diff, build-master and build-secp256k1, the later will cover the standalone build and all the unit tests.

src/secp256k1/README.md
8 ↗(On Diff #15113)

Is there any possible legal consequence with this removal ?

src/secp256k1/README.md
4 ↗(On Diff #15113)

Or we can just use travis, the adaptation from core's shouldn't be very difficult.

Add a bit more explanation of how the repository is maintained, so that people can understand what's going on.
Re-add "Use at your own risk" disclaimer as recommended by @Fabien
Add link and mention that this is the Bitcoin Cash variant of the Schnorr algorithm.

src/secp256k1/README.md
8 ↗(On Diff #15113)

I doubt it's an issue, but I can re-add the "Use at your own risk" statement.

I will also add a bit more explanation of how the repository is maintained, so that people can understand what's going on.

src/secp256k1/README.md
4 ↗(On Diff #15131)

Is travis actually broken for us? If not, then why not use it?

src/secp256k1/README.md
4 ↗(On Diff #15131)

IDK, I don't have sufficient privileges in the Bitcoin ABC Github to enable Travis.

Just by clicking around, it looks doable, so that's probably a good idea to enable it.

If someone wants to give me the needed Github privileges I can try to figure it out, or alternatively someone familiar with Travis can do it.

src/secp256k1/README.md
4 ↗(On Diff #15131)

I granted you the right to the github repo so you can do it. Getting the cmake build to also be tested would be a plus. I think @Fabien can help here.

Add link directing contributors to Bitcoin ABC Phabricator repo
Re-add Travis link, now that Travis is enabled for the repo

This revision is now accepted and ready to land.Jan 5 2020, 09:27
This revision was automatically updated to reflect the committed changes.