Add decode_type method to BitcoinEx.Address#12
Merged
Conversation
leishman
reviewed
Jun 10, 2020
| @spec decode_type(String.t(), Bitcoinex.Network.network_name()) :: | ||
| {:ok, address_type} | {:error, term()} | ||
| def decode_type(address, network_name) do | ||
| case Enum.find(@address_type, &is_valid?(address, network_name, &1)) do |
Member
There was a problem hiding this comment.
we should probably rename this to @address_types since it's a list
kafaichoi
requested changes
Jun 11, 2020
| end | ||
|
|
||
| @spec decode_type(String.t(), Bitcoinex.Network.network_name()) :: | ||
| {:ok, address_type} | {:error, term()} |
Contributor
There was a problem hiding this comment.
the only possible error is :decode_error. Let's change term() to :decode_error that help people using dialyzer for type checking.
kafaichoi
reviewed
Jun 11, 2020
| case Segwit.decode_address(address) do | ||
| {:ok, {^network_name, _, _}} -> | ||
| true | ||
| {:ok, {^network_name, witness_version, witness_program}} -> |
Contributor
There was a problem hiding this comment.
a cleaner way will be
{:ok, {^network_name, 0, witness_program}} when length(witness_program) == 20 ->
true
So we can get ride of one unncessary if expression before. Same for line 61
Contributor
Author
There was a problem hiding this comment.
Right! Changed
kafaichoi
approved these changes
Jun 12, 2020
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds a decode_type method to BitcoinEx.Address in order to return the type of address.
Split the current is_valid?() method in BitcoinEx.Address for segwit addresses into two methods and add a guard to check for witness program length.
We previously were returning is_valid true for segwit addresses that are not infact valid for mainnet because they were v1 segwit addresses. Think we should return false for for v1 segwit addresses.