Global setting for mainnet vs testnet

[sidazhang]

For example, bitcoin-ruby allows you to do this:

# use testnet so you don't acidentially your whole money!
Bitcoin.network = :testnet3

Or cryptocoinjs

PackageName.defaultNetwork = 'testnet'

If we can globally set network type then I will be able to start writing some integration tests for bitcoinjs-lib

[sidazhang]

I have some thoughts how we could do this but I want to first open a discussion.

For example:
https://github.com/bitcoinjs/bitcoinjs-lib/blob/master/src/script.js#L310-L319

Let's say I am running in production and someone supplies a testnet address. In the current code, I will simply send the coin the corresponding hash160, however, ideally, I want this to return an error saying that it is not a valid address.

Script.createOutputScript = function(address) {
  var script = new Script()
  address = new Address(address)
  if (address.version == network.mainnet.p2shVersion ||
      address.version == network.testnet.p2shVersion) {
    // Standard pay-to-script-hash
    script.writeOp(Opcode.map.OP_HASH160)
    script.writeBytes(address.hash)
    script.writeOp(Opcode.map.OP_EQUAL)
  }

[weilu]

I don't have a strong opinion for or against global network settings. @dcousens might.

[sidazhang]

I think if we don't have global settings which is fine, we should have explicit settings for version bytes, so that we can prevent the situation like the one I pointed out above, when running in production, I should not be able to create an output script for an testnet address

[dcousens]

@sidazhang, if you want to arbitrarily limit your code to only accept particular networks, then you should be capturing/blocking that information at a higher level (for example, manually verifying the user input).

If you want to make sure no malicious transactions that contain misleading addresses occur at this level, it is clearly an exceptional circumstance and you should treat it as such (maybe use assert).

You could do this in the following way (using new constructor syntax, soon to be merged):

var addr = Address.fromString('mo6LvR64pruZEQJ4qApmtBhKgBXaB2Az2J')
assert(addr.version === network.bitcoin.pubkeyhash)

// ...
Script.createOutputScript(addr)

[sidazhang]

I feel it is within the responsibility of the library to ensure that. Because, technically speaking, when running in production anything other than address.version == network.mainnet.p2shVersion will be incorrect.

It is easier for the library to have that rather than as a user of the library basically write a wrapper over every function to do a network version check.

perhaps, you can supply a second argument here, if you don't like the idea of global states: Script.createOutputScript = function(address, network)

[dcousens]

Script.createOutputScript should simply not be accepting a string address as an argument.

Instead, it should support only the Address abstraction; which already provides you with a version to check.

This is only occurring because of the lack of error checking/specificity on the address parsing in new Address(address) inside the createOutputScript function.

[sidazhang]

oh right I missed that, in that case I think the code should be changed to:
address.getType() === 'scripthash'
Maybe that would be more intuitive, and a nicer lead in if we want do altcoin support in the future

[dcousens]

@sidazhang agreed, and this is something I'm definitely working towards.

I still feel there is a disconnect between Script and Address that needs to be determined.
The two are fundamentally related, however the top level API hides away one from the other.

[dcousens]

Resolved in part by Address.prototype.toScriptPubKey.