SDK does not recognize offline keyreg transaction

[ejbaran]

There is no offline key registration type, so you have to instantiate it with the base Transaction class and set the type manually.
txn = transaction.Transaction(addr, sp, None, None, constants.keyreg_txn, None)

This is not ideal but fine, until you go to use transaction.retrieve_from_file(<offline-txn-file>) and then it fails because it tries to look for partkey info.

[jkschin]

Hey @ejbaran, not sure if the team is taking PRs for this. I took a look at the codebase and found that you can actually call transaction.KeyregTxn to create the the offline key registration transaction. 10 parameters are necessary to create this KeyregTxn. The code sample here reproduces this issue (parameters referenced from here):

from algosdk.v2client import algod
from algosdk import transaction

algod_address = "https://algoexplorerapi.io"
algod_token = ""
# See https://github.com/algorand/py-algorand-sdk/issues/169 on User-Agent.
algod_client = algod.AlgodClient(algod_token, algod_address, headers={'User-Agent': 'Random'})
status = algod_client.status()
params = algod_client.suggested_params()

data = {
    "sender": "EW64GC6F24M7NDSC5R3ES4YUVE3ZXXNMARJHDCCCLIHZU6TBEOC7XRSBG4",
    "votekey": None,
    "selkey": None,
    "votefst": None,
    "votelst": None,
    "votekd": None,
    "fee": 1000,
    "flat_fee": True,
    "first": 7000000,
    "last": 7001000,
    "gen": params.gen,
    "gh": params.gh
}
txn = transaction.KeyregTxn(**data)
transaction.write_to_file([txn], "dump.txn")
txn2 = transaction.retrieve_from_file("dump.txn")

This code has a KeyError: 'votekey' when it's trying to retrieve_from_file, and I think I might know why this is happening (probably here). Am I understanding your problem correctly?

[ejbaran]

That's about right. You can form the transaction using the base class for keyreg, but the read/write logic doesn't know how to process it (looks like the dictify/undictify functions require the partkey fields).

I think the right solution here is to add constructors specifically for OnlineKeyRegTxn and OfflineKeyRegTxn which are the two instantiations, in practice, of that KeyRegTxn class.

[jkschin]

The problem lies with encoding.msgpack_encode. In the documentation, it states that "maps must omit key-value pairs where the value is a zero-value". In encoding._sort_dict, it removes 0 values and in this specific situation, None is evaluated to False and thus the value is omitted.

Using the same example above, the object that is dictified right before encoding in the write phase looks like this:

OrderedDict([('fee', 1000),
             ('fv', 7000000),
             ('gen', 'mainnet-v1.0'),
             ('gh',
              b'\xc0a\xc4\xd8\xfc\x1d\xbd\xde\xd2\xd7`K\xe4V\x8e?m\x04\x19\x87'
              b'\xac7\xbd\xe4\xb6 \xb5\xab9$\x8a\xdf'),
             ('lv', 7001000),
             ('selkey', None),
             ('snd',
              b'%\xbd\xc3\x0b\xc5\xd7\x19\xf6\x8eB\xecvIs\x14\xa97\x9b\xdd\xac'
              b'\x04Rq\x88BZ\x0f\x9aza#\x85'),
             ('type', 'keyreg'),
             ('votefst', None),
             ('votekd', None),
             ('votekey', None),
             ('votelst', None)])

However, the object that's supposed to be undictified in the read phase looks like this (values with None have their keys omitted):

{'fee': 1000,
 'fv': 7000000,
 'gen': 'mainnet-v1.0',
 'gh': b'\xc0a\xc4\xd8\xfc\x1d\xbd\xde\xd2\xd7`K\xe4V\x8e?m\x04\x19\x87'
       b'\xac7\xbd\xe4\xb6 \xb5\xab9$\x8a\xdf',
 'lv': 7001000,
 'snd': b'%\xbd\xc3\x0b\xc5\xd7\x19\xf6\x8eB\xecvIs\x14\xa97\x9b\xdd\xac'
        b'\x04Rq\x88BZ\x0f\x9aza#\x85',
 'type': 'keyreg'}

Two questions:

1. OnlineKeyregTxn would have the same required and optional parameters as the current implementation of KeyregTxn?
2. OfflineKeyregTxn would be the same as OnlineKeyregTxn but have selkey, votefst, votekd, votekey, votelst removed from the required parameters.

If the team is happy to review a PR on this I'll be keen to try one.

[jasonpaulos]

We'd be happy to take a PR for this. If it helps, here is the relevant code in the Javascript SDK that properly deals with the case when nonParticipation is true (i.e. offline key reg transaction): https://github.com/algorand/js-algorand-sdk/blob/e43e31231542b790b39e16faf09dbd2801b01c7f/src/transaction.ts#L461-L895

@jkschin I believe the answer to your questions are:

1. OnlineKeyregTxn should have exactly the same parameters as the current KeyregTxn except for nonpart, which will always be false for this type of transaction.
2. You are correct that OfflineKeyregTxn should omit those 5 parameters. Additionally, I don't think it should take nonpart as a parameter because it always needs to be true for this type of transaction.

[jkschin]

@jasonpaulos I assume the change should be made in future.transaction instead of transaction? The nonpart only appears in the future folder. When I run the same test above, but with future.transaction, I actually got this error

TypeError: __init__() got an unexpected keyword argument 'fee'

I looked at the implementation of KeyregTxn in future.transaction and it is indeed missing a fee parameter, but the documents suggest that fee should be a parameter (the second parameter).

1. Should this new class be made in future instead?
2. At a higher level, is the future folder a set of code that's perhaps meant for an eventual migration?

[jasonpaulos]

@jkschin Yes I believe the changes should happen only in the future folder.

The KeyregTxn from the future folder should not take a fee parameter, instead the sp (SuggestedParams) parameter contains information about the fee and other low-level transaction parameters. If you are still seeing that error when calling the class properly, please post the code that causes it.

And yes, the plan is to move the contents of the future folder into the main library in the next major release.