Indexer appears to return the AAAA... "zero" address instead of no address at all (e.g. undefined, null, or simply absent) for asset information.

[AlterionX]

Describe the bug
The semantics of Option<Address> seems invalid for assets.

To Reproduce
Attempting to get the json from https://algoindexer.testnet.algoexplorerapi.io/v2/assets/78377817 yields:

{
    "asset": {
        "created-at-round": 20385749,
        "deleted": false,
        "index": 78377817,
        "params": {
            "clawback": "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAY5HFKQ",
            "creator": "TALE7TBFEOCK4Q4M4QLYTPYTNTA2L7MYDY7IBYK3UOO4TIXH2YJBBS3EWU",
            "decimals": 0,
            "default-frozen": false,
            "freeze": "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAY5HFKQ",
            "manager": "TALE7TBFEOCK4Q4M4QLYTPYTNTA2L7MYDY7IBYK3UOO4TIXH2YJBBS3EWU",
            "name": "Protagonist C2α - Ancient",
            "name-b64": "UHJvdGFnb25pc3QgQzLOsSAtIEFuY2llbnQ=",
            "reserve": "TALE7TBFEOCK4Q4M4QLYTPYTNTA2L7MYDY7IBYK3UOO4TIXH2YJBBS3EWU",
            "total": 1,
            "unit-name": "🐰",
            "unit-name-b64": "8J+QsA=="
        }
    },
    "current-round": 20388498
}

The freeze and clawback addresses were set to None on creation.
As a result, calling indexer.asset_info yields the following (truncated for brevity).

Asset {
    params: AssetParams {
        clawback: Some(AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAY5HFKQ),
        ..
    }
}

Apparently, attempting to create an asset with the AAAA... address yields an error.... which means that the zero address actually means "missing address".

Expected behavior
I was expecting:

Asset {
    params: AssetParams {
        clawback: None,
        ..
    }
}

Additional context
I personally think that algonaut should understand these zero addresses and parse them as "None"s. This obviously introduces complexities and potential bug in the future if existing behavior ever changes.

The other option is to leave them alone, but get rid of the Option from the various addresses. I don't like this.

Regardless of what happens, this behavior is mildly janky and should be documented.

[ivnsch]

Hmm that seems mainly an API issue. The node's API has this policy (which I dislike) where it omits / expects us to omit values that are considered "zero", which includes arrays with only zeros, or structs with values that are only zeros (this leads to us having to implement "smart" deserialization in some places, like here:

algonaut/algonaut_transaction/src/api_model.rs

Lines 395 to 415 in 0d93ce1

	fn parse_state_schema(
	on_complete: ApplicationCallOnComplete,
	app_id: Option<u64>,
	api_state_schema: Option<ApiStateSchema>,
	) -> Option<StateSchema> {
	match (on_complete, app_id) {
	// App creation (has schema)
	(ApplicationCallOnComplete::NoOp, None) => Some(
	api_state_schema
	.map(|s| s.into())
	// on creation we know that there's a schema, so we map None to schema with 0 values.
	// The API sends None because struct with 0s is considered a "zero value" and skipped.
	.unwrap_or_else(|| StateSchema {
	number_ints: 0,
	number_byteslices: 0,
	}),
	),
	// Not app creation (has no schema)
	_ => None,
	}
	}

)

The indexer sending these zero addresses is also an inconsistency with that. Probably since it uses strings, their serializer doesn't recognize them as a "zero value" and sends it.

I'd be inclined to raise the issue in their repo https://github.com/algorand/go-algorand. But not against patching it in the SDK.

Edit: this is the indexer's repository: https://github.com/algorand/indexer (ignoring the node's zero values policy, this only affects the indexer).

[AlterionX]

We should probably do both, then. (As in, both patching it in the SDK & raising an issue.)

Another thing to think about is if we need different deserialization protocols for algod/indexer/kmd.

Does anyone know if I can just plop this into the indexer issues?

[AlterionX]

Possibly relates to #98?

[ivnsch]

We should probably do both, then. (As in, both patching it in the SDK & raising an issue.)

Agreed.

Another thing to think about is if we need different deserialization protocols for algod/indexer/kmd.

Does anyone know if I can just plop this into the indexer issues?

If with protocols you mean JSON / MessagePack, For algod/indexer/kmd only JSON should be enough - MessagePack is used only to send transactions and TEAL. Currently Transaction can however be deserialized only from MessagePack. We've e.g. this place, where Transaction (which is included in the pending_transactions response) is not defined, maybe because of insecurities about how to handle the JSON deserialization. If we adjust the ApiTransaction visitors to deserialize from both MsgPack and JSON we might be able to reuse ApiTransaction / Transaction for this endpoint (and possibly others that return transactions - would have to check whether they share the same fields). The Transaction deserialization from MsgPack, while not required, should be preserved just for symmetry with the serialization, which you might use in contexts different to the Algorand APIs.

Possibly relates to #98?

It's a bit wider, but yeah it relates.

[ivnsch]

Also relates to #106