search

namecoin / electrum-nmc

Namecoin port of Electrum Bitcoin client.
https://www.namecoin.org/
MIT License
29 stars 24 forks source link

Unit tests won't pass until you fix this subtle bug #207

Closed sidhujag closed 4 years ago

sidhujag commented 4 years ago

Going to save you time when you port over, I am doing the Syscoin integration rebasing off their latest commits and they added setters for locktime/version so you should be using _version = 0 and _locktime = 0 and not use the setters, for example:

https://github.com/namecoin/electrum-nmc/blob/auxpow/electrum/transaction.py#L551

JeremyRand commented 4 years ago

Ah nice, thanks for the heads up. At a first glance all the setter does is invalidate a serialization cache; is there any particular reason that this is harmful?

sidhujag commented 4 years ago

Yea it relies on the setter and its a subtle bug, the tests will show you though because they fail and its not 100% obvious as to why but this is the reason, let me know if theres any issues.

Off top of my head it was because if you don't invalidate the cache the next time it builds the tx it won't build it properly, the code can be refactored to be better but it is what it is.

and no problem! I used your work instead of redoing auxpow from scratch so thanks for the hard work and tests on auxpow.

JeremyRand commented 4 years ago

@sidhujag AFAICT all unit tests are passing on the auxpow branch: https://travis-ci.org/github/namecoin/electrum-nmc/builds/671407007 . Can you confirm whether this is fixed?

sidhujag commented 4 years ago

Yup looks good!