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

🐞 Bug: JSON roundtrip property broken for TxMetadataWithSchema. #4647

Open
jonathanknowles opened this issue Jun 26, 2024 · 2 comments
Open
Labels

Comments

@jonathanknowles
Copy link
Member

jonathanknowles commented Jun 26, 2024

Version

fae66a5

Description

The JSON encoding roundtrip property is not always satisfied for the TxMetadataWithSchema type.

Here's a PR that demonstrates the failure:

Why is this a bug?

There is a long-standing convention that types used in the HTTP API should satisfy the following JSON round trip property:

decode . encode == id

@jonathanknowles jonathanknowles changed the title JSON roundtrip property is not always satisfied for TxMetadataWithSchema. JSON roundtrip property broken for TxMetadataWithSchema. Jun 26, 2024
@jonathanknowles
Copy link
Member Author

jonathanknowles commented Jun 27, 2024

This appears to be specific to the TxMetadataNoSchema option, defined within the Cardano.Api.TxMetadata module of the cardano-api package.

Given the following import:

> import Cardano.Api.TxMetadata

And the following metadata:

> m =
    TxMetadata
      { unTxMetadata = fromList
        [ ( 0
          , TxMetaMap
            [ ( TxMetaText "k", TxMetaText "v1" )
            , ( TxMetaText "k", TxMetaText "v2" )
            ]
          )
        ]
      }

It is possible to perform a JSON round trip with the DetailedSchema option:

> metadataFromJson TxMetadataJsonDetailedSchema
   (metadataToJson TxMetadataJsonDetailedSchema m)
   == Right m
True

But not possible to perform a JSON round trip with the NoSchema option.

> metadataFromJson TxMetadataJsonNoSchema
   (metadataToJson TxMetadataJsonNoSchema m)
   == Right m
False

This is because the NoSchema option encodes a TxMetaMap as a JSON object, which does not permit duplicate keys:

> metadataToJson TxMetadataJsonNoSchema m
Object (fromList [("0",Object (fromList [("k",String "v2")]))])

Which when rendered as JSON produces the following string:

> BL.putStrLn $ Aeson.encode $ metadataToJson TxMetadataJsonNoSchema m
{"0":{"k":"v2"}}

The preservation of only the latest key-value mapping is consistent with the behaviour of Data.Aeson.object.

Going back to DetailedSchema, this encoding uses JSON arrays of key-value pairs to represent TxMetaMap objects:

> BL.putStrLn $ Aeson.encode $ metadataToJson TxMetadataJsonDetailedSchema m
{ "0":
  { "map":
    [ { "k": {"string":"k"}
      , "v": {"string":"v1"}
      }
    , { "k": {"string":"k"}
      , "v": {"string":"v2"}
      }
    ]
  }
}

Thus making it possible to satisfy the JSON round-trip property.

@jonathanknowles
Copy link
Member Author

jonathanknowles commented Jun 27, 2024

From the perspective of the upstream cardano-api library, if the TxMetadataJsonNoSchema option is used, then it seems that not satisfying the round trip property decode . encode == id is by design:

https://github.com/IntersectMBO/cardano-api/blob/2b5e9485df5a771fef4b15e0e1a4ba936c5d20d6/cardano-api/test/cardano-api-test/Test/Cardano/Api/Metadata.hs#L186

Source code extract:

-- This uses the \"no schema\" mapping. Note that with this mapping it is /not/
-- the case that any tx metadata can be converted to JSON and back to give the
-- original value.

@jonathanknowles jonathanknowles changed the title JSON roundtrip property broken for TxMetadataWithSchema. 🐞 Bug: JSON roundtrip property broken for TxMetadataWithSchema. Jun 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant