I'm not sure exactly what is going on. I will try to rebase all these diffs onto D16687 when I get it sorted out. Hopefully it will fix the situation. I could just rebase everything onto master after I land any diff in the stack, but that would be tedious and noisy in the phabricator logs for a stack of 10+ commits.

In D16681#378434, @bytesofman wrote:

still not able to arc patch this one, which could mean we see issues when it is landed. may want to wait until this is next off the rank.

did points at infinity or not on curve not being properly handled impact app performance? why was this not handled before this diff?

It wasn't properly handled in this class because the code guarding against this problem is currently still using the ecdsa.ellipticalcurve.Point objects we want to get rid of. You could for instance have a sum of elliptical curve points result in point at inifinity which would be an unsecure or unusuable key to use in the cryptography in Cash Fusion code. If we want the Cash Fusion code to not lose any security features when switching to using this ECPubkey class in D16682, I need to implement it. But it is unlikely to ever be triggered except in tests where it is done on purpose.

Wrt to points not on curve, this diff basically just catches existing exceptions that would be raised when constructing the ecdsa.ellipticcurve.Point object and raise our custom exception instead. So later when we move away from using ecdsa.ellipticcurve.Point we can maintain the same behavior by client code by manually raising that exception depending on the return value of some libsecp256k1 function. See D16689 in the _x_and_y_from_pubkey_bytes function for instance.