Fairly confident the test suite has got invalid base64 responseJsons and they are passing because of this line? #39
Labels
Team Lamprey
tech debt
Things that need re-work for simplification / sanitization to reduce implementation overhead
Acceptance Criteria
In the algorand-sdk-testing repo:
Original issue below
@FrankSzendzielarz commented on Mon Jun 13 2022
https://github.com/algorand/go-algorand-sdk/blob/adb6d4be485483d4b95d5eb7742943dc2f2109e9/test/utilities.go#L44
Looks to me that the pendingTransactions.base64 json is a b64 encoded msgpack transaction, where the values are themselves further b64 encoded values, invalid as transaction data. (EDIT TO ADD: STRINGS are serialised as b64, which differs from what Algod does and what jSON format clients would normally expect)
It looks to me, and I might be wrong (it's really late!) that this is passing in the line above because it uses the same approach to just b64 decode a stream and then msgpack decode a bunch of text into an object and compare it with the response, both of which are invalid?
The supply txn is coming from here https://github.com/algorand/algorand-sdk-testing
Specifically here https://github.com/algorand/algorand-sdk-testing/blob/master/features/unit/v2algodclient_responsejsons/pendingTransactions.base64
If I b64 decode that and msgpack read it, I get a structure that shows further b64 encoded elements. Is this somehow intended?
@jasonpaulos commented on Mon Jun 13 2022
Hi @FrankSzendzielarz, I'm not sure what you're seeing, but when I base64 decode https://github.com/algorand/algorand-sdk-testing/blob/master/features/unit/v2algodclient_responsejsons/pendingTransactions.base64 and parse it, the fields I see are expected.
Note if you do this with
msgpacktool
(i.e.echo -n grB0b3AtdHJhbnNhY3Rpb25zk4K... | base64 -d | msgpacktool -d
), you will see base64 values, but this is how the tool shows binary data. You'll notice it adds a:b64
suffix to keys when it does this:You can independently verify the values are correct by using an SDK to decode the msgpack. Here's an example in Javascript:
@FrankSzendzielarz commented on Mon Jun 13 2022
Pls check - why is type cGF5 ? If I call Algod API (PendingTransactionInformation) with format request JSON, will I get the same value? If I call Algod API with format request "msgpack" will we expect the Algod Node to b64 encode strings? So far that's not what I am seeing, but please verify?
@jasonpaulos commented on Mon Jun 13 2022
cGF5
is the base64-encoded string "pay".This is where it gets a little tricky. When you ask algod for a JSON response, it will base64 binary fields, but it will not base64 encode unicode-safe fields, since those can be represented in JSON. Whether a field is binary or unicode-safe is defined for each field and is not dependent on the actual value.
So for the
/v2/transactions/pending
endpoint, you will seepay
as the type, since the type field is defined as a unicode-safe string. However, if you look at a binary field, likegh
(genesis hash), you will see a base64 encoded value.I admit it isn't clear at first glance which fields are base64 encoded and which aren't, but if you know what the field should be in the API response, you can figure out if algod always base64 encodes that field or not.
Algod will never base64 encode a string in msgpack. Since msgpack is a binary format, it can store arbitrary binary values, so this is not necessary. However, some msgpack viewers may show you a base64 encoding of the field, so please be aware of that possibility.
@FrankSzendzielarz commented on Mon Jun 13 2022
Ok this is not really clarifying things for me at the moment. I am aware that cGF5 is pay, but why is it base64 encoded?
I don't know what your msgpacktool is but an online decoder shows this (mismatch because there is no b64 specifier when unnecessary):
The above is also exactly the same as what Newtonsoft MessagePack decoder generates.
What I need is for the MessagePack and Json deserialisers to provide both the same values into the same POCO structures, and right now the test data is not. I suspect if you were to call AlgoD and ask for Format=msgpack it will not yield the same data as in the test suite, but I need to verify this tomorrow.
@FrankSzendzielarz commented on Tue Jun 14 2022
Ok so I checked the txn file produced by goal and you get something like in the attachment:
test2.txt
The above in b64 is gaN0eG6Ko2FtdM4HW80Vo2ZlZc0D6KJmdsy8o2dlbqpzYW5kbmV0LXYxomdoxCBrG1W8fCk1UFj7FD3KW2YxPebIM19C6yJsKkDl7BfDr6Jsds0EpKRub3RlxAhRD7AaWZKhTaNyY3bEIHtrZcY4YjtNRiPoQKNJz3XWiSZsJWV6Svbck6iAGBnUo3NuZMQge2tlxjhiO01GI+hAo0nPddaJJmwlZXpK9tyTqIAYGdSkdHlwZaNwYXk=
which when deserialised, gives you the correct class properties (yes the byte arrays will be deserialised from b64 whether json or not):
{
"txn": {
"amt": 123456789,
"fee": 1000,
"fv": 188,
"gen": "sandnet-v1",
"gh": "axtVvHwpNVBY+xQ9yltmMT3myDNfQusibCpA5ewXw68=",
"lv": 1188,
"note": "UQ+wGlmSoU0=",
"rcv": "e2tlxjhiO01GI+hAo0nPddaJJmwlZXpK9tyTqIAYGdQ=",
"snd": "e2tlxjhiO01GI+hAo0nPddaJJmwlZXpK9tyTqIAYGdQ=",
"type": "pay"
}
}
Note that string is "pay" . Byte arrays are b64.
The test JSON strings are incorrectly encoded. The test JSON strings are simply being used to compare if the mock HttpResponse, when pushed into an object, match the mock HttpResponse. By pushing a b64 encoded string into a type field, the test is incorrectly passing what should be plaintext.
The base64 unit test case strings are wrong, but the actual test method is also wrong. A correct test should a) deserialise the type and other fields correctly b) make sure that the values are correct and consistent with other fields on the object
The text was updated successfully, but these errors were encountered: