Unit test for Transaction's to/from_json

[belovachap]

Hello Chainside and @peerchemist 👋

I was working on a downstream branch ( https://github.com/PeerAssets/btcpy ) and wrote a test for the Transaction's to_json and from_json methods and noticed that the last test transaction (a SegWit transaction) fails :(

python -m unittest tests.unit.TestTransaction is the quickest way I could find to run it if you're curious.

I'll try to figure out how to get the test to pass eventually but I think from_json doesn't notice that the incoming transaction json is for a segwit transaction and drops that information.

[SimoneBronzini]

Thanks a lot for your contribution! I already had a plan on adding better testing of from/to json methods. In fact #39 was not failing tests but was actually incorrect in the json conversions. I fixed it before merging but scheduled to add more json tests in the future. This is another case that proves we need more of those, I'll look into this as soon as possible!

[SimoneBronzini]

Just a heads-up: in fact there are some inconsistencies in the interface. While Transaction.unhexlify instantiates either a Transaction or a SegWitTransaction depending on the hex provided, on the other hand, Transaction.from_json and SegWitTransaction.from_json always instantiate an object of the class they were called upon.
I think a solution to this would be implementing the following logics:

• Having unhexlify and from_json always fail when called on a hex/json that do not match the type they were called upon
• Creating a TransactionFactory class that provides generic unhexlify and from_json methods which instantiate either a Transaction or a SegWitTransaction depending on the data provided

Already working on this fix.

[belovachap]

I like the sound of both of those ideas. I haven't paid much attention to SegWit so I don't know if there are any drawbacks to what you suggested... but after playing with the btcpy code a little more I think you might be pointing in the right direction :)

[SimoneBronzini]

d9196b9 fixes this