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

Add WriteStream, ReadStream, Converter and HexHelper #741

Closed
cpl121 opened this issue Jul 10, 2023 · 6 comments
Closed

Add WriteStream, ReadStream, Converter and HexHelper #741

cpl121 opened this issue Jul 10, 2023 · 6 comments
Assignees
Milestone

Comments

@cpl121
Copy link
Member

cpl121 commented Jul 10, 2023

Description

Currently WriteStream, ReadStream, Converter and HexHelper iota.js dependencies are being used in Firefly and they need to be added in the sdk

Also, the encodings have been modified to make transactions between layer 1 and layer 2, as can be seen in the evm-toolkit.
To fit these changes, a new type SpecialStream has been created in Firefly. It would be necessary to add this class together with size64Encode.
On the other hand, although it is still in development, it would also be necessary to add the SpecialStream to be able to read these new transactions, as well as their decode

Motivation

The iota.js dependencies used in Firefly need to be incorporated into the sdk, and the way to encode/decode transactions from layer 1 to layer 2 has been updated, thus the WriteSpecialStream/ReadSpecialStream classes needs to be added too

Requirements

  1. Add WriteStream and WriteSpecialStream
  2. Add ReadStream and ReadSpecialStream
  3. Add Converter
  4. Add HexHelper

Are you planning to do it yourself in a pull request?

No.

@cpl121 cpl121 changed the title Update writeStream for the new encoding Add writeStream for the new encoding Jul 10, 2023
@cpl121 cpl121 changed the title Add writeStream for the new encoding Add writeStream, Converter and HexHelper Jul 10, 2023
@cpl121 cpl121 changed the title Add writeStream, Converter and HexHelper Add WriteStream, ReadStream, Converter and HexHelper Jul 10, 2023
@thibault-martinez thibault-martinez added this to the v1.0.0 milestone Jul 11, 2023
@thibault-martinez thibault-martinez modified the milestones: v1.0.0, v1.1.0 Jul 21, 2023
@kwek20
Copy link
Contributor

kwek20 commented Aug 23, 2023

Hi @cpl121 is this still needed? If so, why? Adding a whole bunch of "custom" serialization really defeats the purpose of bindings and single source of truth. Is there a way we can change this so you dont need any of the streams?

As for Converter and HexHelper, we should have added all the missing utilities to our utils package. Let me know specifically what method you are missing and ill add it.

@kwek20
Copy link
Contributor

kwek20 commented Sep 7, 2023

Meeting notes:
HexHelper: Not used
Converter: Methods have been moved to utils
Read/write streams: Possibly provide a read/write() that takes ReadStream / WriteStream and Serialize / deserialize bytes from there. (read takes Uint8Array and returns object, write takes object and returns Uint8Array)

  • Q: How do we know what were serializing/deserializing? Is there a default entry point like "Transaction"?

Specialstream: Custom u64 encoding from wasp

Need a way to parse NewTransactionDetails for L1-L2 transfer https://github.com/iotaledger/firefly/blob/develop/packages/shared/lib/core/layer-2/utils/getLayer2MetadataForTransfer.ts#L9

Q: Is the reason for these streams that svelte stores requires them?

@kwek20
Copy link
Contributor

kwek20 commented Sep 10, 2023

@begonaalvarezd Your feedback is welcome please :)

@thibault-martinez thibault-martinez modified the milestones: v1.1.0, Q4 Sep 29, 2023
@begonaalvarezd
Copy link
Member

After seeing our dependencies in Firefly, apart from utils needed for L2 which should come from a wasp client util libary (so out of the scope in this issue), we use iota-js for 2 things in Firefly that perhaps could be provided by the sdk somehow

@kwek20
Copy link
Contributor

kwek20 commented Oct 3, 2023

@begonaalvarezd the hex conversion we already have added in utils/utf8.ts: hexToUtf8 and utf8tobytes.
I will add a hexToBytes so we dont need to do a 2 step

So that leaves us with the ReadStream, which i think we can already do in standard js code:

import { Readable } from "stream";

export async function parseGovernanceMetadata(metadata: string): Promise<IParticipation[]> {
    const read = async (a: Readable, b: number) => {
        let c = [b];
        for (let i = 0; i < b; i++) {
            c[i] = await a.read();
        }
        return c;
    };
    
    const readStream = Readable.from(hexToBytes(metadata));
    const participations: IParticipation[] = []
    const amountParticipations = await readStream.read();
    for (let index = 0; index < amountParticipations; index++) {
        const eventId = bytesToHex(await read(readStream, 32));
        const amountAnswers = await readStream.read();

        const answers: number[] = []
        for (let index = 0; index < amountAnswers; index++) {
            const answer = await readStream.read();
            answers.push(answer)
        }

        participations.push({ eventId, answers })
    }

    return participations;
}

What we lose with this is the error logging name when reading, which we had when we specified the name in the readUInt8('amountParticipations').

@thibault-martinez
Copy link
Member

thibault-martinez commented Oct 30, 2023

Closing this as part of it has been addressed and it's been decided that the SDK won't contain anything EVM related. Please (re)open issues if you feel like you still miss something.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: 🎉 Done
Status: Done
Development

No branches or pull requests

4 participants