Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fairly confident the test suite has got invalid base64 responseJsons and they are passing because of this line? #333

Closed
FrankSzendzielarz opened this issue Jun 13, 2022 · 6 comments

Comments

@FrankSzendzielarz
Copy link

FrankSzendzielarz commented Jun 13, 2022

expectedString = string(sdk_json.Encode(generic))

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
Copy link
Contributor

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:

{
  "top-transactions": [
    {
      "sig:b64": "FzxjLZYDl2tLXM1keGg7mRC6m/NftiS6hlQWBw0EXjzru/MmcdzH8t3xiFPDZ1+JbVj1ZomLXiwwL9o5F0kzCw==",
      "txn": {
        "amt": 123456789,
        "fee": 1000,
        "fv": 1400,
        "gen:b64": "ZXZhbnN0ZXN0bmV0d29yay12MQ==",
        "gh:b64": "CGABgymS8U8z84VeQcD5/opKT764Sv696/+9dZKeK4Y=",
        "lv": 2400,
        "note:b64": "EjT4wUzCqg4=",
        "rcv:b64": "wW6UkqPB8Ta1TOTbPXGVU9hDtsB/TXi1k3hpB3mP3fQ=",
        "snd:b64": "ZQbY+M4dGhkkZ5yDpfmRyvBFMdrIwpiTYQLD7Emd7Sk=",
        "type:b64": "cGF5"
      }
    },
    {
      "sig:b64": "iRCZwC97wFayWg2epJ4coVOex7K+kOF6Ck0JmVr//O14DYvwHhA87SyupSv8YtW4QMHpF5oVZ+qvg+9iE5CoDw==",
      "txn": {
        "amt": 987654321,
        "fee": 1000,
        "fv": 1400,
        "gen:b64": "ZXZhbnN0ZXN0bmV0d29yay12MQ==",
        "gh:b64": "CGABgymS8U8z84VeQcD5/opKT764Sv696/+9dZKeK4Y=",
        "lv": 2400,
        "note:b64": "PQ0P5pN4O4Q=",
        "rcv:b64": "wW6UkqPB8Ta1TOTbPXGVU9hDtsB/TXi1k3hpB3mP3fQ=",
        "snd:b64": "ZQbY+M4dGhkkZ5yDpfmRyvBFMdrIwpiTYQLD7Emd7Sk=",
        "type:b64": "cGF5"
      }
    },
    {
      "sig:b64": "soYwYx9YdLvwrmGzFEOlQhviM2p6hB2ohxkhpdBh+ZebEQZeoZpqOC15/Ooff6PNCu6MmqWafAVx5hXVar3gBg==",
      "txn": {
        "amt": 100000001,
        "fee": 1000,
        "fv": 1400,
        "gen:b64": "ZXZhbnN0ZXN0bmV0d29yay12MQ==",
        "gh:b64": "CGABgymS8U8z84VeQcD5/opKT764Sv696/+9dZKeK4Y=",
        "lv": 2400,
        "note:b64": "InpgL/kIryk=",
        "rcv:b64": "wW6UkqPB8Ta1TOTbPXGVU9hDtsB/TXi1k3hpB3mP3fQ=",
        "snd:b64": "ZQbY+M4dGhkkZ5yDpfmRyvBFMdrIwpiTYQLD7Emd7Sk=",
        "type:b64": "cGF5"
      }
    }
  ],
  "total-transactions": 3
}

You can independently verify the values are correct by using an SDK to decode the msgpack. Here's an example in Javascript:

const algosdk = require("algosdk");

let r = algosdk.decodeObj(Buffer.from('grB0b3AtdHJhbnNhY3Rpb25zk4Kjc2lnxEAXPGMtlgOXa0tczWR4aDuZELqb81+2JLqGVBYHDQRePOu78yZx3Mfy3fGIU8NnX4ltWPVmiYteLDAv2jkXSTMLo3R4boqiZnbNBXiiZ2jEIAhgAYMpkvFPM/OFXkHA+f6KSk++uEr+vev/vXWSniuGomx2zQlgo2FtdM4HW80Vo2ZlZc0D6KNnZW7EE2V2YW5zdGVzdG5ldHdvcmstdjGjcmN2xCDBbpSSo8HxNrVM5Ns9cZVT2EO2wH9NeLWTeGkHeY/d9KNzbmTEIGUG2PjOHRoZJGecg6X5kcrwRTHayMKYk2ECw+xJne0ppG5vdGXECBI0+MFMwqoOpHR5cGXEA3BheYKjc2lnxECJEJnAL3vAVrJaDZ6knhyhU57Hsr6Q4XoKTQmZWv/87XgNi/AeEDztLK6lK/xi1bhAwekXmhVn6q+D72ITkKgPo3R4boqiZnbNBXiiZ2jEIAhgAYMpkvFPM/OFXkHA+f6KSk++uEr+vev/vXWSniuGomx2zQlgo2FtdM463mixo2ZlZc0D6KNnZW7EE2V2YW5zdGVzdG5ldHdvcmstdjGjcmN2xCDBbpSSo8HxNrVM5Ns9cZVT2EO2wH9NeLWTeGkHeY/d9KNzbmTEIGUG2PjOHRoZJGecg6X5kcrwRTHayMKYk2ECw+xJne0ppG5vdGXECD0ND+aTeDuEpHR5cGXEA3BheYKjc2lnxECyhjBjH1h0u/CuYbMUQ6VCG+IzanqEHaiHGSGl0GH5l5sRBl6hmmo4LXn86h9/o80K7oyapZp8BXHmFdVqveAGo3R4boqiZnbNBXiiZ2jEIAhgAYMpkvFPM/OFXkHA+f6KSk++uEr+vev/vXWSniuGomx2zQlgo2FtdM4F9eEBo2ZlZc0D6KNnZW7EE2V2YW5zdGVzdG5ldHdvcmstdjGjcmN2xCDBbpSSo8HxNrVM5Ns9cZVT2EO2wH9NeLWTeGkHeY/d9KNzbmTEIGUG2PjOHRoZJGecg6X5kcrwRTHayMKYk2ECw+xJne0ppG5vdGXECCJ6YC/5CK8ppHR5cGXEA3BhebJ0b3RhbC10cmFuc2FjdGlvbnMD', 'base64'));

// inspect the object however you like
console.log(r);

@FrankSzendzielarz
Copy link
Author

FrankSzendzielarz commented 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
Copy link
Contributor

Pls check - why is type cGF5 ?

cGF5 is the base64-encoded string "pay".

If I call Algod API (PendingTransactionInformation) with format request JSON, will I get the same value?

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 see pay as the type, since the type field is defined as a unicode-safe string. However, if you look at a binary field, like gh (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.

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?

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
Copy link
Author

FrankSzendzielarz commented Jun 14, 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):

`{
  "top-transactions": [
    {
      "sig": "FzxjLZYDl2tLXM1keGg7mRC6m/NftiS6hlQWBw0EXjzru/MmcdzH8t3xiFPDZ1+JbVj1ZomLXiwwL9o5F0kzCw==",
      "txn": {
        "fv": 1400,
        "gh": "CGABgymS8U8z84VeQcD5/opKT764Sv696/+9dZKeK4Y=",
        "lv": 2400,
        "amt": 123456789,
        "fee": 1000,
        "gen": "ZXZhbnN0ZXN0bmV0d29yay12MQ==",
        "rcv": "wW6UkqPB8Ta1TOTbPXGVU9hDtsB/TXi1k3hpB3mP3fQ=",
        "snd": "ZQbY+M4dGhkkZ5yDpfmRyvBFMdrIwpiTYQLD7Emd7Sk=",
        "note": "EjT4wUzCqg4=",
        "type": "cGF5"
      }
    },
    {
      "sig": "iRCZwC97wFayWg2epJ4coVOex7K+kOF6Ck0JmVr//O14DYvwHhA87SyupSv8YtW4QMHpF5oVZ+qvg+9iE5CoDw==",
      "txn": {
        "fv": 1400,
        "gh": "CGABgymS8U8z84VeQcD5/opKT764Sv696/+9dZKeK4Y=",
        "lv": 2400,
        "amt": 987654321,
        "fee": 1000,
        "gen": "ZXZhbnN0ZXN0bmV0d29yay12MQ==",
        "rcv": "wW6UkqPB8Ta1TOTbPXGVU9hDtsB/TXi1k3hpB3mP3fQ=",
        "snd": "ZQbY+M4dGhkkZ5yDpfmRyvBFMdrIwpiTYQLD7Emd7Sk=",
        "note": "PQ0P5pN4O4Q=",
        "type": "cGF5"
      }
    },
    {
      "sig": "soYwYx9YdLvwrmGzFEOlQhviM2p6hB2ohxkhpdBh+ZebEQZeoZpqOC15/Ooff6PNCu6MmqWafAVx5hXVar3gBg==",
      "txn": {
        "fv": 1400,
        "gh": "CGABgymS8U8z84VeQcD5/opKT764Sv696/+9dZKeK4Y=",
        "lv": 2400,
        "amt": 100000001,
        "fee": 1000,
        "gen": "ZXZhbnN0ZXN0bmV0d29yay12MQ==",
        "rcv": "wW6UkqPB8Ta1TOTbPXGVU9hDtsB/TXi1k3hpB3mP3fQ=",
        "snd": "ZQbY+M4dGhkkZ5yDpfmRyvBFMdrIwpiTYQLD7Emd7Sk=",
        "note": "InpgL/kIryk=",
        "type": "cGF5"
      }
    }
  ],
  "total-transactions": 3
}`

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
Copy link
Author

FrankSzendzielarz commented 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

@winder
Copy link
Contributor

winder commented Jul 14, 2022

Issue moved to algorand/generator #39 via ZenHub

@winder winder closed this as completed Jul 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants