Skip to content

Conversation

@dcousens
Copy link
Contributor

This is a 3.0.0 PR.

Mostly looking for feedback on the naming and structure behind the bitcoin.address.[decode/encode] use cases.

I included the network search inside decode rather than in toOutputScript because it is very useful to be able to check whether a particular address is on the same network as yourself when verifying it.

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling 13d978f on addr300 into 771111a on 3.0.0.

@weilu
Copy link
Contributor

weilu commented Oct 24, 2014

+1 for address instead of Address now that we no longer instantiate it.

If all you wanted is to check the address' version, why not provide address.getVersion function which does return base58check.decode(address).readUInt8() then user can carry out their own checks. Any other use case you have in mind for encode/decode?

@dcousens
Copy link
Contributor Author

function validateAddress(address, network) {
  var decode
  try {
    decode = bitcoin.address.decode(address)
  } catch (e) {
    return false
  }

  return decode.network === network
}

If you just have a version, the user then needs to search over the network space themselves to see if it is a version they accept, in a network they know.

@weilu
Copy link
Contributor

weilu commented Oct 24, 2014

Indeed, it does force one to explicitly whitelist the versions they'd like to accept which isn't necessarily a bad thing. They can implement their own network validation function easily with getVersion:

function validateAddress(addr, network) {
  var version = address.getVersion(addr)
  return [network.pubKeyHash, network.scriptHash].indexOf(version) >= 0
}

@dcousens
Copy link
Contributor Author

Is that ideal though? It does mean they suddenly have to be more aware than before of address versions.

I agree a getVersion would probably be useful though... but that is already part of the decode? Why not just give them both.

@weilu
Copy link
Contributor

weilu commented Oct 27, 2014

The main reason I don't quite like decode is that the return value is some arbitrary object.

@dcousens
Copy link
Contributor Author

The fact is though, that its a parsing operation, it will have arbitrary
values unless we push it back into an Address object...

On Mon, Oct 27, 2014 at 1:11 PM, Wei Lu notifications@github.com wrote:

The main reason I don't quite like decode is that the return value is
some arbitrary object.


Reply to this email directly or view it on GitHub
#306 (comment)
.

@dcousens
Copy link
Contributor Author

@weilu further thoughts on this?

@weilu
Copy link
Contributor

weilu commented Nov 24, 2014

I still think address.getVersion is a neater interface without an arbitrary object being returned. If you want to make it network detection friendly, we can very well do what we did in scripts.js -- add address.classifyNetwork which searches through known networks and returns on the first version match. The point is, I'd like to design for use cases while expose the smallest interface possible.

@dcousens
Copy link
Contributor Author

I think that's a good solution. I'll iterate on this a bit more today see
what I come up with.
On Nov 25, 2014 1:55 AM, "Wei Lu" notifications@github.com wrote:

I still think address.getVersion is a neater interface without an
arbitrary object being returned. If you want to make it network detection
friendly, we can very well have address.classifyNetwork which searches
through known networks and returns on the first version match. The point
is, I'd like to design for use cases while expose the smallest interface
possible.


Reply to this email directly or view it on GitHub
#306 (comment)
.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants