List Nano (NANO)

[n9Mtq4]

• Official project URL: https://nano.org
• Official block explorer URL: https://www.nanode.co

[blabno]

Applied Bisq code style. Please squash those commits.

[n9Mtq4]

Squashed.

[blabno]

Seems that Tree class inside Nano is not used.
There are also many methods that are not used at all.
@cbeams what do you think about this?

[cbeams]

Thanks for catching this, @blabno.

Sorry @n9Mtq4, this implementation is far too complicated. I'm not saying it's not correct, I'm saying that I'm not going to spend the time to figure it out for a coin I've never heard of. Please radically simplify this, being pragmatic where necessary, just use a regex to get a first approximation of correctness. Look through the codebase to find other uses of RegexAddressValidator and follow suit. Thanks.

[n9Mtq4]

Nano addresses can't be fully checked with a regex expression. It contains the public key encoded with base 32 with a unique char set and a blake2b checksum also base 32 encoded. While the format of the address can be validated with regex, validating the checksum requires decoding the public key, hashing it, and b32 encoding that hash.

While it seems complicated, the majority of the code was copy-pasted from other places. Blake2b was an exact duplicate from here, and the base 32 came from here with only the slightest modifications possible to use nano's base 32 character set. I did this in the hopes that you could see that this code came from established, open source libraries and while being long and complicated doesn't do anything malicious.

Possible solutions:

1. Only use regex. I don't want to do this since the addresses this implementation reports valid would been seen as invalid from other wallets. Address checksums are a nice thing to have to prevent any typos or extra characters that might have been pasted in with the address. I would rather have a full address checker than an incomplete one.
2. I could simplify the code down to only what is needed. This would remove the symmetry between the code in here and the other code I used, but be bare-bones. I could also change the checking to only use base 32 decoding, which would remove even more code. However, I am not sure how much code this solution will remove and I don't have the knowledge of the blake2b hashing algorithm to do anything other than remove unused methods.
3. Split up this class. Is there any way to move the blake2b hashing code and base 32 code into some common crypto package or find some other dependency that handles this case?

[cbeams]

It's really about being pragmatic here. This implementation as-is adds 2K+ lines of code to a codebase that currently defines 100+ assets while remaining under 10K LoC total. The next longest class in the codebase is 10x shorter than the one in this implementation. It just doesn't work, in the name of full validation, to grow the codebase by 20% for a single new asset.

A regex that captures the allowed prefixes and underscore, validates that everything is lowercase, and that the address is of the correct length will go a long way toward doing 'enough' validation. If we find that Nano is a huge hit on Bisq, and we start to see validation errors cropping up due to invalid checksums, we can revisit a more complete implementation then.

Note that we're going to ship v0.7.1 within the next couple days, so if you can touch this up in the meantime, we can still get it in by the release. Best regards.

[n9Mtq4]

@cbeams Alright. It's just regex now.

[cbeams]

Thanks, @n9Mtq4. You'll see I've touched up the style a bit further in the commit above. Merging now.