Fix #566: serialize commit to hex

[mslipper]

No description provided.

[antiochp]

We have the following representations of an Output in the api code -

• Output
• OutputPrintable
• OutputSwitch

Seems like we may be able to consolidate these into just a single representation (this is likely what you are doing by using OutputPrintable and not Output).

As these are all http/json serialization (via the api) - it seems like everything should be "printable".
I don't think there is much need for non-printable versions of an output (whatever that actually means).

[mslipper]

I can give that a shot. Full disclosure, I'm still a Rust noob so it'll take a little time for me to refactor all the places that these output types are used and to test them thoroughly.

[ignopeverell]

Any interest in picking this up?

[antiochp]

I'm actually deep in the output serialization code due to us picking #215 back up.
We are getting rid of the "output_by_commit" index - so we have very limited info available via the api when retrieving outputs by their commitments.
The only way to get heights, output_types etc. now will be to go via the byheight api endpoint and look outputs up via their containing block.

So I'd recommend holding off on changing any of the output serialization until #215 is merged (assuming we don't find any blockers with this proposed approach).

[Bringing this up because I'm expecting api codebase to change a fair bit in the next couple of days.]

[mslipper]

Ok great. I'll hold off then until #215 is merged.

[antiochp]

Ok - #215 ended up not getting merged.
But - we merged #615 instead.

So now we have the following "models" in api -

• OutputPrintable (everything is basically a string here)
• Utxo (just a wrapper for a commitment, replaces the old OutputSwitch model)

A Utxo currently looks like this -

curl "127.0.0.1:13413/v1/chain/utxos/byids?id=0960dd49bc81aa33fe094d98126ad3f759eb0ff5ee36c75ae82acd2486253df18f"
[{"commit":[9,96,221,73,188,129,170,51,254,9,77,152,18,106,211,247,89,235,15,245,238,54,199,90,232,42,205,36,134,37,61,241,143]}]

And OutputPrintables look like this (nested in a BlockPrintable here) -

curl "127.0.0.1:13413/v1/chain/utxos/byheight?startheight=1&endheight=1"
[
  {
    "header":{
      "hash":"7272a182aed9268920d4949929490511d2006864ee5bdd976d3a4595cf1c8f94",
      "height":1,
      "previous":"b83053b4b2e29376d6850de4fdaa60366300520bc6957cc69bec7089fcb1bfd8"
    },
    "outputs":[
      {
        "output_type":"Coinbase",
        "commit":"083eafae5d61a85ab07b12e1a51b3918d8e6de11fc6cde641d54af53608aa77b9f",
        "switch_commit_hash":"85daaf11011dc11e52af84ebe78e2f2d19cbdc76",
        "spent":false,
        "proof":null,
        "proof_hash":"ed6ba96009b86173bade6a9227ed60422916593fa32dd6d78b25b7a4eeef4946"
      }
    ]
  }
]

So -

1. I think we should serialize the commit in the Utxo as a hex string (in the json).
2. I do think we are currently approaching the serialization problem the wrong way in the api code - OutputPrintable (and other api models) should maintain its internal state using Commitment, Hash etc. And we should serialize these to hex strings when necessary (rather than maintaining state internally in strings etc.)

[mslipper]

Agreed on your points. I can pick this back up and look into using Rust's serde library to automatically convert between hex internal representations of these objects.

[antiochp]

Sounds like a good plan.
We're doing similar stuff in the wallet code (to serialize outputs out to wallet.dat).

For example -

grin/wallet/src/types.rs

Lines 246 to 253 in f1bbf53

	impl serde::ser::Serialize for BlockIdentifier {
	fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
	where
	S: serde::ser::Serializer,
	{
	serializer.serialize_str(&self.0.to_hex())
	}
	}

Edit: One thing to point out - we wrapped a Hash up in a BlockIdentifier basically so we could add this custom serialization/deserialization to it.
It was not possible to add it directly to Hash itself because we do not want to assume we will always serialize/deserialize a Hash in this way (and for many uses we do not).

[mslipper]

Great, this is really helpful!

[mslipper]

@antiochp FYI will have this ready by EOD today.

[antiochp]

👍