feat: Update types according to (discrepancies) in the docs.#3
feat: Update types according to (discrepancies) in the docs.#3
Conversation
| */ | ||
| events: Emitter<InjectedEvents>; | ||
| }; | ||
| arweaveWallet: ArweaveWallet; |
There was a problem hiding this comment.
I exported the standalone interface below so that it can be used in other projects easily. In my case, I do class Othent implements Omit<ArConnect, "connect">.
| * | ||
| * @returns Promise of the user's Arweave config | ||
| */ | ||
| getArweaveConfig(): Promise<GatewayConfig>; |
There was a problem hiding this comment.
Before:
getArweaveConfig(): Promise<{
host: string;
port: number;
protocol: "http" | "https";
}>;
But you already had the GatewayConfig defined, so I'm just replacing the hardcoded type.
| */ | ||
| encrypt( | ||
| data: BinaryDataType, | ||
| options?: RsaOaepParams | AesCtrParams | AesCbcParams | AesGcmParams, |
There was a problem hiding this comment.
Before:
encrypt(
data: string,
options: {
algorithm: string;
hash: string;
salt?: string;
}
): Promise<Uint8Array>;
Which didn't match the docs:
I see the implementation supports two different options, the legacy ones and the new ones. I would suggest using overloading in the types if both version needs to be supported, or otherwise export two interfaces separately. Also, I don't see why the new one could not accept both a binary data type data as well as string data, just like with the legacy version: https://github.com/arconnectio/ArConnect/blob/production/src/api/modules/encrypt/encrypt.background.ts#L70
Lastly, I guess if Othent allowed users to import/export keys, they should be able to encrypt something with Othent and decrypt it with ArConnect, or viceversa, and I don't think that's the case at the moment with the legacy version. Not sure if it might be worth at least adding a check in Othent to detect the legacy version and throw an error?
| */ | ||
| decrypt( | ||
| data: BinaryDataType, | ||
| options?: RsaOaepParams | AesCtrParams | AesCbcParams | AesGcmParams, |
There was a problem hiding this comment.
Before:
decrypt(
data: Uint8Array,
options: {
algorithm: string;
hash: string;
salt?: string;
}
): Promise<string>;
Which, similarly to encrypt, didn't match the docs:
See how also the return type is different, the previous types said this returns a string, but in the docs we can clearly see that's not the case as the returned value is being decoded.
| | BigInt64Array | ||
| | BigUint64Array; | ||
|
|
||
| export type BinaryDataType = ArrayBuffer | TypedArray | DataView; // | Buffer; |
| data: BinaryDataType, | ||
| // https://developer.mozilla.org/en-US/docs/Web/API/SubtleCrypto/sign#algorithm | ||
| algorithm?: RsaPssParams | EcdsaParams | { name: "RSASSA-PKCS1-v1_5" } | { name: "HMAC" }, | ||
| ): Promise<Uint8Array>; |
There was a problem hiding this comment.
Before:
signature(
data: Uint8Array,
// https://developer.mozilla.org/en-US/docs/Web/API/SubtleCrypto/sign#parameters
algorithm: AlgorithmIdentifier | RsaPssParams | EcdsaParams
): Promise<Uint8Array>;
But AlgorithmIdentifier is more broad than what's defined in the docs.
| * | ||
| * @returns Dispatched transaction ID and type | ||
| */ | ||
| dispatch(transaction: Transaction): Promise<DispatchResult>; |
There was a problem hiding this comment.
The docs here say:
The function returns the result of the API call.
But DispatchResult only includes two properties, not the whole result. Additionally, when uploading to Turbo, it returns:
return {
arConfetti: await arconfettiIcon(),
res: {
id: dataEntry.id,
type: "BUNDLED"
}
};
I think it should not be dataEntry.id, which is set by dataEntry.sign(dataSigner), but the response from Turbo that includes the transaction id (uploadDataToTurbo()).
| * | ||
| * @returns Hash array | ||
| */ | ||
| privateHash(data: ArrayBuffer, options: SignMessageOptions): Promise<Uint8Array>; // TODO: data is ArrayBuffer or BinaryDataType |
There was a problem hiding this comment.
Not sure why data here can only be ArrayBuffer. It seems inconsistent with the other functions.
Also, according to the example in the docs (but not the arguments in the docs), that seems to be the case (data can be BinaryDataType):
As Uint8Array (from new TextEncoder().encode()) is not assignable to ArrayBuffer.
| * | ||
| * @returns Signature | ||
| */ | ||
| signMessage(data: ArrayBuffer, options?: SignMessageOptions): Promise<Uint8Array>; // TODO: data is ArrayBuffer or BinaryDataType |
There was a problem hiding this comment.
Same as with privateHash(). See above.
| signature: ArrayBuffer | string, // TODO: signature is ArrayBuffer or BinaryDataType | ||
| publicKey?: string, | ||
| options?: SignMessageOptions | ||
| ): Promise<boolean>; |
| * | ||
| * @returns Buffer of the signed data item | ||
| */ | ||
| signDataItem(dataItem: DataItem): Promise<ArrayBufferLike>; // TODO: Is this ArrayBufferLike or Buffer based on the example? `const dataItem = new DataItem(Buffer.from(result));` The implementation says `return Array.from<number>(dataEntry.getRaw());` in https://github.com/arconnectio/ArConnect/blob/production/src/api/modules/sign_data_item/sign_data_item.background.ts |
There was a problem hiding this comment.
I didn't change these types in the PR, but see my notes:
-
The example in the docs take the returned value and pass it directly to
DataItem(), butDataItemonly takesBuffer, so that's wrong (type-wise): -
However, the implementation says
return Array.from<number>(dataEntry.getRaw()), which is not the same asArrayBufferLike, but happens to pass asBuffer.
In this case, in Othent, I just made sure I'm returning ArrayBuffer (return dataItemInstance.getRaw().buffer) and transformed it before passing it to DataItem: new DataItem(Buffer.from(result)). Alternatively, this could just return Buffer (return dataItemInstance.getRaw()) and the example would be valid.






This PR highlights discrepancies between:
Additionally, even thought I've ignored this while writing this PR, there might also be discrepancies with the types in https://github.com/arconnectio/ArConnect.
Lastly, while digging into some of the issues I'm highlighting in this PR, I also noticed some potential issues on the implementation side, at least related to types (TypeScript) and type conversions. Note I didn't review the implementaiton code thoroughly.