Comparison of Transaction objects

[gokselcoban]

Comparison (__eq__) of transaction objects (Ex. ApplicationCallTxn)

Because of on_complete (apan) attribute of the ApplicationCallTxn, the comparison of two transactions gives the wrong output.
The reason is OnComplete.NoOpOC (0) is not equal to None.

I think the possible and clear solution is to use transaction id for comparison. If you think this is the right approach, I can open a PR.

Your environment

py-algorand-sdk==1.11.0

Steps to reproduce

from algosdk.future.transaction import ApplicationNoOpTxn
from algosdk import account
from algosdk.encoding import future_msgpack_decode, msgpack_encode


suggested_params = algod_client.suggested_params()
sender = account.generate_account()[1]

txn_a = ApplicationNoOpTxn(
   index=1,
   sender=sender,
   sp=suggested_params,
   app_args=['asd', ],
   foreign_assets=[],
   accounts=[],
   note=b'asd'
)
txn_b = future_msgpack_decode(msgpack_encode(txn_a))

txn_a == txn_b  # Actual behaviour
Out: False

txn_a.get_txid() == txn_b.get_txid()  # Expected behaviour
Out: True

txn_a.dictify()
Out:
OrderedDict([('apaa', [b'asd']),
             ('apan', <OnComplete.NoOpOC: 0>),
             ('apid', 1),
             ('fee', 1000),
             ('fv', 20646759),
             ('gen', 'testnet-v1.0'),
             ('gh',
              b'Hc\xb5\x18\xa4\xb3\xc8N\xc8\x10\xf2-O\x10\x81\xcb\x0fq\xf0Y\xa7\xac \xde\xc6/\x7fp\xe5\t:"'),
             ('lv', 20647759),
             ('note', b'asd'),
             ('snd',
              b'LW\xbb\xe9B^\xe9Z\x1f%\xbe\xf8BM\x06\xe3\x11\xf4\x19\xc2D.\xaf*\xd3t\xab\xaang\xee\x97'),
             ('type', 'appl')])

txn_b.dictify()
Out:
OrderedDict([('apaa', [b'asd']),
             ('apan', None),
             ('apid', 1),
             ('fee', 1000),
             ('fv', 20646759),
             ('gen', 'testnet-v1.0'),
             ('gh',
              b'Hc\xb5\x18\xa4\xb3\xc8N\xc8\x10\xf2-O\x10\x81\xcb\x0fq\xf0Y\xa7\xac \xde\xc6/\x7fp\xe5\t:"'),
             ('lv', 20647759),
             ('note', b'asd'),
             ('snd',
              b'LW\xbb\xe9B^\xe9Z\x1f%\xbe\xf8BM\x06\xe3\x11\xf4\x19\xc2D.\xaf*\xd3t\xab\xaang\xee\x97'),
             ('type', 'appl')])

Expected behaviour

txn_a == txn_b should return True

Actual behaviour

txn_a == txn_b returns False

[algochoi]

Thanks for reporting this! I agree that this could be a problem and have replicated it on my machine.

I think there could be a problem where someone might try to compare the apan values of two Transaction objects as well. In this case, we might want to change the dictify method in ApplicationCallTxn so we initialize the apan value to 0 if the entry is None.

[gokselcoban]

Yes, I agree apan should be fixed.

What do you think about comparing transaction ids in __eq__?

[algochoi]

I am leaning towards changing the dictify method instead because the apan really should be 0 there, instead of None. Then the __eq__ should work, in addition to users pulling the apan field from the dict and comparing that by hand. Checking the txid in the object's __eq__ method seems a little unpythonic to me, personally.

Let me know your thoughts and whether this makes sense. Also feel free to open a PR and I can take a look at the changes as well.

[gokselcoban]

Checking the txid in the object's eq method seems a little unpythonic to me, personally.

I see, It may make sense when you think that these are two python objects.

I think txid is the unique representation of these python objects on the network. If txid calculation is correct, we can simply make sure that these objects represent the same transaction on the blockchain. Possibly this bug can happen again because of another field in the future. I wanted to remove this possibility.