near / borsh-js

TypeScript/JavaScript implementation of Binary Object Representation Serializer for Hashing
Apache License 2.0
112 stars 38 forks source link

Is it possible to have a property with 2 possible values for the length in a borsh-js(v.1.0.0) schema? #85

Closed gtsonevv closed 3 months ago

gtsonevv commented 4 months ago

Is it possible to have a property with 2 possible values for the length in a borsh-js(v.1.0.0) schema?

For example, I want to have a class with a publicKey property of type Uint8Array that has a length of 32 or 65 bytes.

Basically what I'm trying to do is this: https://github.com/near/near-api-js/pull/985/files#diff-dfff95256a781858ca8cc28ea405fc0e61dee26055dd6544465040df364a6ea4R183 but I don't see how this can be done with borsh-js v1.0.0.

This is how we are using borsh-js v1.0.0 https://github.com/near/near-api-js/blob/master/packages/transactions/src/schema.ts#L90-L230

Tried removing len: 32 but that didn't help.

gagdiez commented 3 months ago

sounds like you need to use an enum

gtsonevv commented 3 months ago

So, I will have to do something like that: from this

export class PublicKey extends Assignable {
    keyType: KeyType;
    data: Uint8Array;
}
export const SCHEMA = new class BorshSchema {
    PublicKey: Schema = {
        struct: {
            keyType: 'u8',
            data: { array: { type: 'u8', len: 32 } },
        }
    };
}

to this:

class ED25519PublicKey { keyType: KeyType = KeyType.ED25519; data: Uint8Array; }
class SECP256K1PublicKey { keyType: KeyType = KeyType.ED25519; data: Uint8Array; }

export class PublicKey extends Enum {
    ed25519Key?: ED25519PublicKey;
    secp256k1Key?: SECP256K1PublicKey;
}
    ED25519_DATA: Schema = {
        struct: {
            keyType: 'u8',
            data: { array: { type: 'u8', len: 32 } },
        }
    };
    SECP256K1_DATA: Schema = {
        struct: {
            keyType: 'u8',
            data: { array: { type: 'u8', len: 64 } },
        }
    };
    PublicKey: Schema = {
        enum: [
            { struct: { ed25519Key: this.ED25519_DATA } },
            { struct: { secp256k1Key: this.SECP256K1_DATA } },
        ]
    };

right? @gagdiez

gagdiez commented 3 months ago

@gtsonevv that looks right, let me know if it doesn't work and we can think other solutions

gtsonevv commented 3 months ago

@gagdiez Hey, I tried it but unfortunately it didn't work.

gtsonevv commented 3 months ago

@gagdiez Hey, this is the test branch with enum changes https://github.com/near/near-api-js/tree/secp256k1-test. After the changes some tests fail. To test you need to run: pnpm i pnpm build pnpm test

gagdiez commented 3 months ago

@gtsonevv I check how things were made [1] and I am quite certain that you don't need the keyType for the schemas "ED25519_DATA" and "SECP256K1_DATA"

class ED25519PublicKey { data: Uint8Array; }
class SECP256K1PublicKey { data: Uint8Array; }

export class PublicKey extends Enum {
    ed25519Key?: ED25519PublicKey;
    secp256k1Key?: SECP256K1PublicKey;
}

    ED25519_DATA: Schema = {
        struct: {
            data: { array: { type: 'u8', len: 32 } },
        }
    };
    SECP256K1_DATA: Schema = {
        struct: {
            data: { array: { type: 'u8', len: 64 } },
        }
    };
    PublicKey: Schema = {
        enum: [
            { struct: { ed25519Key: this.ED25519_DATA } },
            { struct: { secp256k1Key: this.SECP256K1_DATA } },
        ]
    };

This way you are saying: A Public Key is either a ed25519Key or a secp256k1Key, which are arrays of different size. Then, the serialization automatically handles the "KeyType" that was previously being manually hardcoded in [1]

please check https://github.com/near/near-api-js/pull/1354/files to see if it is working now

IMPORTANT: There was a hardcoded value that needed to be modified, which I assume was the borsh representation, that now lost the "00" keytype... I do not know if this is right, please double check and merge at your own risk

[1] You can see in https://github.com/near/near-api-js/pull/985/files#diff-a300f87801fdf0e1d8db018ff4994a8c20a2bcc27152cb7d595f1f8bde52bcc9R77 that they were hardcoding the borsh serialization, since the previous version did not handle enums

gtsonevv commented 3 months ago

@gagdiez Nice! I thought the keyType was required for the schema. Anyway, good job and thanks for the help!

gagdiez commented 3 months ago

@gtsonevv

My PR still does not passes testing, but it is other tests!

I didn't change the classes to remove the keytype

class ED25519PublicKey { keytype, data: Uint8Array; }
class SECP256K1PublicKey { keytype, data: Uint8Array; }

and I guess they will need to be changed and used accordingly to pass the other tests

gtsonevv commented 3 months ago

@gagdiez This is the final version:

class ED25519PublicKey extends Assignable { keyType: KeyType = KeyType.ED25519; data: Uint8Array; }
class SECP256K1PublicKey extends Assignable { keyType: KeyType = KeyType.ED25519; data: Uint8Array; }

It works fine, all tests pass(after similar changes in Signature schema/class).

gagdiez commented 3 months ago

amazing, very glad to hear! will close this issue then