h-REA / hREA

A ValueFlows / REA economic network coordination system implemented on Holochain and with supplied Javascript GraphQL libraries
https://docs.hrea.io
Other
142 stars 15 forks source link

remove client-side ID encoding logic and pass IDs as strings in I/O #325

Open pospi opened 2 years ago

pospi commented 2 years ago

With the changes in #324 we should be able to remove the custom JavaScript-layer de/serialization in connection.ts in vf-graphql-holochain by changing the Rust RPC Response types and construct_response helpers to be passed as strings.

This is easy for revision_id, which can use HeaderHashB64 for its external representation and convert .into() a HeaderHash in the get_revision_id() methods of update request structs.

For id fields a little more work may be needed to handle conversion between string representations and the binary representation. holo_hash::holo_hash_decode_unchecked appears to be useful for this, we can probably just split the input on the colon delimiter and do what HoloHashB64 does with each component-

impl<T: HashType> HoloHashB64<T> {
    /// Read a HoloHash from base64 string
    pub fn from_b64_str(str: &str) -> HoloHashResult<Self> {
        let bytes = holo_hash_decode_unchecked(str)?;
        HoloHash::from_raw_39(bytes).map(Into::into)
    }
}

Ideally we can do this in a way that plays nice with Serde such that the id field types don't have to change and the deserialization happens transparently. If not, will settle for *B64 variants of *Address tuple structs that can convert into their raw form in the same way as HeaderHashB64 --> HeaderHash.

Connoropolous commented 2 years ago

@pospi I am having some discussion in Kasra about this. Michael Dougherty is recommending against use of HoloHashB64 encoding for zomes...

It might actually be the case that the client side encoding will be more idiomatic for holochain

Connoropolous commented 2 years ago

the id values passed from the backend could just be an array of 39 + 39 bytes, i.e. the length of two holohashes. That way it still acts as a single value

pospi commented 2 years ago

I did have custom serialization logic at one point, maybe we should reinstate it then. We could also trim off some extra bytes while we're at it by dropping the hash prefixes.

I take it the reason is basically about code size bloat due to inclusion of the B64 encoding library?

Connoropolous commented 2 years ago

Yeah technically that's the only concrete reason cited. I'm not sure if that's a good enough reason not to use it. It might be. Depends how strongly compression and size optimization becomes a concern.

"HoloHashB64, that was meant to be internal only, but needs to be used between crates so it had to be made public. Also, it bloats the wasm size a bit since it needs to include extra deps for the encoding."