Hashpack / hashconnect

Hashconnect library, readme in progress
BSD Zero Clause License
45 stars 38 forks source link

Some methods may never resolve due to async race conditions and losing references #127

Closed pwoosam closed 9 months ago

pwoosam commented 2 years ago

sendTransaction, requestAdditionalAccounts, and authenticate all return a Promise which may never resolve due to race conditions.

For example, if we call sendTransaction twice rapidly, then transactionResolver will be set twice. This will cause the first call to sendTransaction to never resolve because the first reference to transactionResolver will be lost before MessageHandler calls it receiving a RelayMessageType.TransactionResponse payload.

The following three properties are set by their corresponding methods, and may never resolve due to the error described above.

export class HashConnect implements IHashConnect {
    transactionResolver: (value: MessageTypes.TransactionResponse | PromiseLike<MessageTypes.TransactionResponse>) => void;
    additionalAccountResolver: (value: MessageTypes.AdditionalAccountResponse | PromiseLike<MessageTypes.AdditionalAccountResponse>) => void;
    authResolver: (value: MessageTypes.AuthenticationResponse | PromiseLike<MessageTypes.AuthenticationResponse>) => void;

    async sendTransaction(topic: string, transaction: MessageTypes.Transaction): Promise<MessageTypes.TransactionResponse> {
        ...
        return await new Promise<MessageTypes.TransactionResponse>(resolve => this.transactionResolver = resolve)
    }
    async requestAdditionalAccounts(topic: string, message: MessageTypes.AdditionalAccountRequest): Promise<MessageTypes.AdditionalAccountResponse> {
        ...
        return await new Promise<MessageTypes.AdditionalAccountResponse>(resolve => this.additionalAccountResolver = resolve)
    }
    async authenticate(topic: string, account_id: string, server_signing_account: string, serverSignature: Uint8Array, payload: { url: string, data: any }): Promise<MessageTypes.AuthenticationResponse> {
        ...
        return await new Promise<MessageTypes.AuthenticationResponse>(resolve => this.authResolver = resolve)
    }
}
teacoat commented 2 years ago

I will look in to how to make this able to support multiple requests, for the time being you should lock the UI until the current transaction is resolved

pwoosam commented 2 years ago

I tried to resolve this by turning transactionResolver into a map where the key was the message's id and the value is the resolver. This should work, but the message we're sending to hashpack has a different id from the message we are receiving from hashpack. This may be intended because messages are technically different messages, but there does not seem to be a way to relate the message going to hashpack to the message coming from hashpack.

You may need to add some additional metadata to the messages that are persisted going to/from hashpack. Maybe an End-to-end ID or thread id, or something along those lines that let's hashconnect know that the messages are related and are part of the same interaction.