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
Branch
libsecp256k1
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 8738
Build 15460: Default Diff Build & Tests
Build 15459: arc lint + arc unit

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.