-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
BS58 module #228
BS58 module #228
Conversation
Force browserify is my strong opinion here. If there's no reason to expose an API for purposes of bitcoin work, let's not do it. People using these libraries below BitcoinJS is a smell that should be resolved by providing the things they need in the library directly. |
bitcoinlib-js currently offers no way for anyone to verify an uncompressed public key. In the old version 0.1.3, you'd just do, In the new version there is just no way to do it that I can find, I have spend 2 days working on this.. simply trying to figure out how to verify an uncompressed public key; Suggest to me so far has been; Browserify my code, which doesn't really work for me given I just want to include the bitcoinlib-js library and then perform this public key verification adhoc in some inline js tags... The other way I thought was to basically provide an open channel to ecurve by adding in a custom function inside one of the existing files (eg: ecpubkey.js) to give me access to ecurve externally. So far I have tried;
also failed.. It just seems like with bitcoinlib-js ~ verifying an uncompressed public key should be easy.. but its not.. it feels really difficult and arkward.. |
@kyledrake I am strongly opposed to forcing browserify. |
@NTOM is providing a precompiled browser version of bitcoinjs-lib a possible alternative. So developers that just want to get started quickly can simply download a bitcoinjs-lib.min.js from a CDN. A checksum can be provided against each version to verify the integrity of the source |
@sidazhang yeah I'm all for using browserify as the package management thing for this library... I am just miffed because ecurve is used in bitcoinlib-js ~ yet I can't access it externally, which means i'd have to reinclude it separately, or alternatively browserify my whole code base... when all I want to do is just use it to verify a uncompressed public key... |
In 0.1.3. .. one could do; Crypto.SHA256(Crypto.SHA256(r,{asBytes: true}),{asBytes: true}); Both or which I think are useful and related to bitcoin, yet not included in the package cuz its browserifyied vs the original versions which were easier for 'normal' devs like me to just plug in and user without having to browserify one's code.. So instead I'd have to include Crypto-js as a seperate entity in my site.. even thought its already been included and utilized in bitcoinlib-js .. I hope I am making sense here. 'Average joe' style comments from ur typical lay person who is looking to make use of the library and t he difficulties / challenges I am facing vs the prior version. |
@NTOM you are being confused though, both examples you gave of not working were simply because you gave the completely wrong input. var pub = ECPubKey.fromHex('04cecce48ed9b1865f68db5f57f34de979397664e69f7db2c02469abb4a15919fdf7b0fd3e03ae96b7f8f2af1881eb9010807bb9fe68f4c7fa995ccc7c0b4e00b7') That will import the public key, but it is not verified as being on the secp256k1 curve. If you really need to check the point, you will need to use var ecurve = require('ecurve')
var curve = ecurve.getCurveByName('secp256k1')
curve.validate(pub.Q) // throws if false The real issue you describe is that perhaps we should be providing better validation primitives in the |
Things people can't do with the library involving Bitcoin are a bug that needs to be fixed. Things people want to do that are out of scope for Bitcoin but happen to involve some of our dependencies are out of the scope of this library. I'm not keen on dumping our internal dependencies, and it makes it a lot harder for us to switch them for something else in the future. I would much rather people include these libraries on their own projects directly if they need them. |
I'm also opposed to distributing a compiled copy of bitcoinjs-lib in our repository. Minified files are not auditable, and introduce security problems and weird diffs on commits, and also introduces a commit requirement to reminify for each commit. If the problem here is you just want a file to download, it would be best to provide it as a separate thing. Maybe on the BitcoinJS web site I made, or maybe on a distribution server. But if I compile this file for other developers, it means that you cannot audit my compile, and you have to trust that I (or an attacker) has not manipulated the file to steal your Bitcoins, or conduct other harmful activities. It feels like the wrong thing to do. Node.js works on all platforms, and you can very easily compile your own bitcoinjs-lib.min.js with just a few commands, so it's not that much more convenient. If this is something you can't figure out how to do, you probably shouldn't be writing bitcoin code anyways. I'm opposed to providing a pre-compiled file. Use browserify. It's the future of JS programming. |
If you think "If this is something you can't figure out how to do, you probably shouldn't be writing bitcoin code anyways" is harsh, observe the incredibly simple process to compile your own minified bitcoinjs-lib:
Is this just a documentation problem? |
This pull request removes
src/base58.js
and replaces it with https://github.com/cryptocoinjs/bs58.All our changes and tests have been cross ported to that module, and there is little reason not to move it out given the API is not going to change (as far as I can see).
Its up for debate whether we should force users to use
browserify
when they might want to usebs58
themselves, or whether we should continue to export it asbitcoinjs.base58 = require('bs58')
.I think we need a consistent policy here, because it currently impacts on
BigInteger
,ecurve
and nowbase58
.The ideological solution would be that we just ask/force users to use
browersify
.