PeerAssets / pypeerassets

Reference implementation of the PeerAssets protocol.
BSD 3-Clause "New" or "Revised" License
12 stars 16 forks source link

WIP - DONT MERGE Issue#70 validateaddress #89

Closed belovachap closed 6 years ago

belovachap commented 6 years ago

@peerchemist @saeveritt

This started out with me staring at Provider and trying to think of a way to add validateaddress without hard coding logic specific to Peercoin. After a few false starts I tried upgrading the NetworkParams tuples to classes and compose them with Providers (i.e. a Provider requires a Network to instantiate) to answer "network questions" like "is this address hash valid?" and "is this a testnet?".

Good news: I think the Network classes simplified some of the logic and functionality in the Providers and give us a model for being more "Blockchain Agnostic" as we move forward.

Bad news: It's a lot of change from someone that's only been working on this for two days :p Our test suite is still pretty beat-up so it's also difficult to say how many things broke from my changes.

Worse news: btcpy's Address.is_valid() doesn't appear to work... so I guess I'll go look at our fork of that project next :\

But overall, I think it might be a good direction. Lemme know what you think when you have a chance :rainbow:

peerchemist commented 6 years ago

It would certainly help if this was split into several PR's. But I'll do by best to review this.

youngpascal commented 6 years ago

Btcpy has refactored a ton of their codebase and unfortunately PA is way behind on this matter. You can see our current progress on refactoring to catch up to new btcpy / overhaul btcpy to be more accepting to Peercoin on our btcpy forked repo.

https://github.com/PeerAssets/btcpy/commit/03357d8747b59c5b0e4b3c83f2386ef7a32309af

Check this commit on the ALLPY branch by @saeveritt to get a look into why Peercoin addresses and such won't operate natively with btcpy.

peerchemist commented 6 years ago

About re-working networks.py, I'd rather do it in format which is used with btcpy (https://github.com/chainside/btcpy/blob/master/btcpy/constants.py) we can monkeypatch btcpy with our constants upon loading.

To explain further this strategy would allow us to support many networks with same btcpy upstream while relying on constants provided by pypeerassets.

belovachap commented 6 years ago

Sounds like we want to use btcpy in some form or fashion for this sort of stuff... will close this for now and come back to this ticket later since there's active development over in btcpy for multiple blockchain support.