Repository: algorand/py-algorand-sdk

Issue:
Payment transaction decode with no Receiver
While decoding transactions from blocks, if a Payment Transaction includes no receiver but uses only close-to, the `future_msgpack_decode` fails with a complaint about no value set for rcv

We can either tweak this line to set the zero address instead of None
https://github.com/algorand/py-algorand-sdk/blob/ee24b9efd039bdb5473de3b9508183f55dc78dbb/algosdk/future/transaction.py#L395-L397

Or, maybe a better option is just to not require/error out when no rcv is set here
https://github.com/algorand/py-algorand-sdk/blob/ee24b9efd039bdb5473de3b9508183f55dc78dbb/algosdk/future/transaction.py#L358-L361




Discussion:
barnjamin: >nit: not sure if ZERO_ADDRESS should live in transaction.py or constants.py. I don't have a strong opinion so I'll just leave it as a thought.

Good point, moved it.
jasonpaulos: To clarify, is this change meant to fix decoding transactions properly from a block, or to let users instantiate transactions in a more useful way?
barnjamin: >This seems like a good idea but it does open up a new edge case: what if amount is > 0? In that case the amount gets sent to the receiver (which is the zero address now), and the account balance minus that amount gets sent to the close to address.

Good point, I guess your suggestion of checking amt >0 should also be implemented.

> To clarify, is this change meant to fix decoding transactions properly from a block, or to let users instantiate transactions in a more useful way?

In this case the bug report I got was when decoding a transaction included in a block where apparently no `rcv` was set. Locally they can just patch it to set the zero address if it isnt already present


jasonpaulos: Thanks for addressing the concerns I raised offline. This looks good, but it would be amazing if you could also add a test to `test_unit.py` that makes sure calling `future_msgpack_decode` on a txn where the receiver is the zero address works properly now

Patch:
diff --git a/algosdk/constants.py b/algosdk/constants.py
index 7da3840..b12a609 100644
--- a/algosdk/constants.py
+++ b/algosdk/constants.py
@@ -89,6 +89,9 @@ LOGIC_SIG_MAX_COST = 20000
 LOGIC_SIG_MAX_SIZE = 1000
 """int: max size of a teal program and its arguments in bytes"""
 
+ZERO_ADDRESS = "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAY5HFKQ"
+"""str: algorand encoded address of 32 zero bytes"""
+
 # for backward compatibility:
 kmd_auth_header = KMD_AUTH_HEADER
 algod_auth_header = ALGOD_AUTH_HEADER
diff --git a/algosdk/future/transaction.py b/algosdk/future/transaction.py
index 69b6a29..6c6da6e 100644
--- a/algosdk/future/transaction.py
+++ b/algosdk/future/transaction.py
@@ -394,7 +394,7 @@ class PaymentTxn(Transaction):
             "amt": d["amt"] if "amt" in d else 0,
             "receiver": encoding.encode_address(d["rcv"])
             if "rcv" in d
-            else None,
+            else constants.ZERO_ADDRESS,
         }
         return args
 
@@ -1348,6 +1348,7 @@ class AssetTransferTxn(Transaction):
             self.receiver = receiver
         else:
             raise error.ZeroAddressError
+
         self.amount = amt
         if (not isinstance(self.amount, int)) or self.amount < 0:
             raise error.WrongAmountType
@@ -1386,7 +1387,7 @@ class AssetTransferTxn(Transaction):
         args = {
             "receiver": encoding.encode_address(d["arcv"])
             if "arcv" in d
-            else None,
+            else constants.ZERO_ADDRESS,
             "amt": d["aamt"] if "aamt" in d else 0,
             "index": d["xaid"] if "xaid" in d else None,
             "close_assets_to": encoding.encode_address(d["aclose"])


Test patch:
diff --git a/algosdk/testing/dryrun.py b/algosdk/testing/dryrun.py
index 2957d76..0c54a1a 100644
--- a/algosdk/testing/dryrun.py
+++ b/algosdk/testing/dryrun.py
@@ -4,7 +4,7 @@ import string
 from dataclasses import dataclass
 from typing import List, Union
 
-from algosdk.constants import payment_txn, appcall_txn
+from algosdk.constants import payment_txn, appcall_txn, ZERO_ADDRESS
 from algosdk.future import transaction
 from algosdk.encoding import encode_address, msgpack_encode
 from algosdk.v2client.models import (
@@ -18,7 +18,6 @@ from algosdk.v2client.models import (
 )
 
 
-ZERO_ADDRESS = encode_address(bytes(32))
 PRINTABLE = frozenset(string.printable)
 
 
@@ -478,7 +477,11 @@ class DryrunTestCaseMixin:
         return drr
 
     def _checked_request(
-        self, prog_drr_txns, lsig=None, app=None, sender=ZERO_ADDRESS
+        self,
+        prog_drr_txns,
+        lsig=None,
+        app=None,
+        sender=ZERO_ADDRESS,
     ):
         """
         Helper function to make a dryrun request and perform basic validation
diff --git a/test_unit.py b/test_unit.py
index 86b6c82..3ffb796 100644
--- a/test_unit.py
+++ b/test_unit.py
@@ -1942,6 +1942,30 @@ class TestMsgpack(unittest.TestCase):
             paytxn, encoding.msgpack_encode(encoding.msgpack_decode(paytxn))
         )
 
+    def test_payment_txn_future(self):
+        paytxn = (
+            "iKVjbG9zZcQgYMak0FPHfqBp4So5wS5p7g+O4rLkqwo/ILSjXWQVKpGjZmVlzQPoom"
+            "Z2KqNnZW6qc2FuZG5ldC12MaJnaMQgNCTHAIMgeYC+4MCSbMinkrlsgtRD6jhfJEXz"
+            "IP3mH9SibHbNBBKjc25kxCARM5ng7Z1RkubT9fUef5nT9w0MGQKRGbwgOva8/tx3qqR"
+            "0eXBlo3BheQ=="
+        )
+        self.assertEqual(
+            paytxn,
+            encoding.msgpack_encode(encoding.future_msgpack_decode(paytxn)),
+        )
+
+    def test_asset_xfer_txn_future(self):
+        axfer = (
+            "iaZhY2xvc2XEIGDGpNBTx36gaeEqOcEuae4PjuKy5KsKPyC0o11kFSqRo2ZlZc0D6KJmdi"
+            "qjZ2VuqnNhbmRuZXQtdjGiZ2jEIDQkxwCDIHmAvuDAkmzIp5K5bILUQ+o4XyRF8yD95h/U"
+            "omx2zQQSo3NuZMQgETOZ4O2dUZLm0/X1Hn+Z0/cNDBkCkRm8IDr2vP7cd6qkdHlwZaVheGZ"
+            "lcqR4YWlkCg=="
+        )
+        self.assertEqual(
+            axfer,
+            encoding.msgpack_encode(encoding.future_msgpack_decode(axfer)),
+        )
+
     def test_multisig_txn(self):
         msigtxn = (
             "gqRtc2lng6ZzdWJzaWeSgqJwa8Qg1ke3gkLuR0MUN/Ku0oyiRVIm9P1QFDaiEhT5v"