Fix Browserify support

[mappum]

The procedural require()s in lib/messages/builder.js were causing problems for Browserify (message building would fail since the constructors were not detected as dependencies and would not be included in the bundle). This PR changes builder.js to use an explicit require() for each command.

Previously, the builder contained a map of the commands, the values being the names. I changed this so the values are now the required constructors, which each now have a _name property.

The tests were also updated to work with this change.

[coveralls]

Coverage remained the same at 100.0% when pulling ecf7db6770f2f583554ed7daa3a1e9065813d3ef on mappum:master into a603858c4bc7dba853112d85df974e0855290119 on bitpay:master.

[maraoz]

Interesting! Our trials to make this module work in the browser had failed. Did you manage to connect to a node from the browser? I'll check the code to see how you did this, I'm intrigued.

EDIT: hmmm those changes don't seem to fix the issues we were having. Did you try connecting to a node from the browser? If that doesn't work, it seems like it makes no sense to add browserify support

[mappum]

I'm connecting to nodes in the browser using WebRTC data channels in my project, webcoin (https://github.com/mappum/webcoin). Of course, it's not possible to create real TCP sockets in the browser, so browser nodes can only connect to other nodes that support WebRTC. However, Node.js nodes can connect to both and bridge the p2p networks. I'm using the same Bitcoin network protocol in both TCP and WebRTC connections (which is why I need bitcore-p2p).

[maraoz]

Nice. Didn't realize bitcore-p2p could be useful without the networking part. ACK. @braydonf comments?

[mappum]

I rebased this change on 0.14.0. Could you guys take a look and think about merging this PR?

[coveralls]

Coverage remained the same at 100.0% when pulling dded8c092add196573aaae460719aca19171b8d4 on mappum:master into 05605b7b7c7f90bf6ee9b6d0ba4e3a71c97e0341 on bitpay:master.

[maraoz]

I'm all for it. Waiting for someone else to review. Thanks for your work @mappum!

[braydonf]

As previously mentioned, support in a browser with browserify would require many other updates, as the networking portions wouldn't work. I also didn't realize it could be useful without the networking portions.... However we still wouldn't have tests for browser/browserify support.

There are a few other similar related open issues, for reference:

• https://github.com/bitpay/bitcore-p2p/issues/49 - Add support for Foxtrot Sockets
• https://github.com/bitpay/bitcore-p2p/issues/36 - Support FirefoxOS & ChromeOS TCP Sockets
• https://github.com/bitpay/bitcore-p2p/issues/21 - Support Connecting to Peers in a Browser

[mappum]

To clarify, I'm not opening an issue or suggesting to make changes to support browserify. This is a PR that includes all the necessary changes to make bitcore-p2p build in browserify. The connection code does not automatically port to browsers, but that is only a small part of this package (if you look at it, most of the volume of code is for bitcoin protocol serialization/deserialization, not managing connections).

@braydonf Please consider pulling this PR, as it will make Bitcoin network protocol serialization possible in the browser. Thank you :+1:

[braydonf]

We don't have a test that reveals and covers the problem that is being fixed here, since we're not running any browser tests yet.

[braydonf]

We may have a few paths to do that:

• Run browser tests and build a bitcore-p2p browserify bundle without the network portions.
• Separate message serialization code into a separate module that is fully Node.js/browser compatible.

The previous may be more simple, especially considering we may be able to make peer/pool more extensible.

[mappum]

Is there a way we can skip certain tests in the browser? All the tests pass in the browser except the Peer/Pool tests (I just added a commit to fix a bug with the message serialization tests).

[coveralls]

Coverage remained the same at 100.0% when pulling 0c01a566f314e4a97c8864cde6584cc3757d840a on mappum:master into 05605b7b7c7f90bf6ee9b6d0ba4e3a71c97e0341 on bitpay:master.

[fanatid]

@mappum something like this?

bxit.js:

module.exports = typeof window === 'undefined' ? it : xit

peer.js:

var bxit = require('./bxit')
describe(..., function () {
  it('...', function () { ... })
  bxit('...', function () { ... })
})

[mappum]

I made the tests run in the browser, and they pass. :+1: (However, we are skipping the Peer and Pool tests since they are not compatible with the browser).

[coveralls]

Coverage remained the same at 100.0% when pulling e733a8e264df4f26bbb9443b344e104b64a714ca on mappum:master into 05605b7b7c7f90bf6ee9b6d0ba4e3a71c97e0341 on bitpay:master.

[mappum]

Karma appears to have a problem in Travis, but it passes successfully on my machine. Any idea how to fix that?

[coveralls]

Coverage remained the same at 100.0% when pulling 5d31001b77f70ed8db894ca15a847ebb8caa6e25 on mappum:master into 05605b7b7c7f90bf6ee9b6d0ba4e3a71c97e0341 on bitpay:master.

[mappum]

Now this PR has been open for a month, and I've addressed your concerns by enabling browser tests. Could you please take one last look when you get a chance and pull this if it is to your satisfaction? Thanks :)

[maraoz]

LGTM

[braydonf]

Adding browser support or partial browser support isn't currently a priority (for maintenance purposes), as every use case we need this module for is with Node.js. And Copay uses BWS (bitcore wallet service): https://github.com/bitpay/bitcore-wallet-service @matiu

Also in context: We've previously separated this module from Bitcore, so that the Bitcore module would be 100% compatible between browser and Node.js, due to problems and confusion between having partial browser support and extra unneeded dependencies in the browserify bundle. The same problems would arise here as well. Separate modules solve this problem much more cleanly.

This PR helps to identify which pieces are working in a browser, that there can be partial browser support, and that in developing web applications there is a use for messages because alternative connections can be made to the Bitcoin network. So in summary:

Features that do work in the browser:

• All of the message handling and creation

Features that do not work in a browser:

• Connecting to a Peer, because of the TCP socket dependency
• Creating a Pool, because of dns dependency for seeding

Something that hasn't been mentioned yet, is to move the features that are working in a browser into the Bitcore module in a similar way that many necessary pieces such as Block, MerkleBlock and Transaction. Would this be satisfactory for your needs? Would need to look into what this would mean for the file-size increase, and if it was an issue, we could add a new module that would have all of the message handling.

[mappum]

Would need to look into what this would mean for the file-size increase, and if it was an issue, we could add a new module that would have all of the message handling.

After being Browserified/minified, the network protocol code increases the size of Bitcore from 500kb to slightly under 700kb. I think it would be a good move to put it in its own package (called something like bitcoin-net). The network (de)serialization code can be very useful on its own, for instance in building node status monitors, proxies, and many other applications.

[braydonf]

Created an issue for this: https://github.com/bitpay/bitcore/issues/1260 to track progress for the above issue.

[eordano]

Would this be OK if we don't publish to bower? What about a note on the README?

I really love the idea of a WebRTC-based bitcoin subnetwork, à la webtorrent. This need not be a priority for maintenance

[braydonf]

There are several changes that need to be done to enable that, which why implementing a new Peer I think makes sense. We can track with https://github.com/bitpay/bitcore/issues/1260

[stefanhuber]

bitcore-p2p can be browserified withound changing the builder.js, one needs to require the messages explicitly in the browserify comand. An example can be found here: https://github.com/getbitpocket/bitcore-p2p-cordova/blob/master/gulpfile.js