Bug: Library does not return errors for `receive`

[callebtc]

cashu-ts tries to receive multiple tokens from multiple mints at once and returns tokens with errors in receive. I think this is not a great approach because the implementor does not receive the error.

I am also not sure if the treatment of the counter for deterministic derivation still would work as intended. The counter should depend on the keyset which implies that a function using it should only create outputs of a single keyset, otherwise that function would need multiple counters.

I would suggest that the library should only receive tokens from a single mint with receive and multiple mints should be handled by an additional function. That means receive can return the error to the implementor. We should also check other functions that do not return errors and make them return it.

Any thoughts on this @gandlafbtc?

I'm c/p the relevant code here to make it easier to reason about:

    /**
     * Receive an encoded or raw Cashu token
     * @param {(string|Token)} token - Cashu token
     * @param preference optional preference for splitting proofs into specific amounts
     * @param counter? optionally set counter to derive secret deterministically. CashuWallet class must be initialized with seed phrase to take effect
     * @param pubkey? optionally locks ecash to pubkey. Will not be deterministic, even if counter is set!
     * @param privkey? will create a signature on the @param token secrets if set
     * @returns New token with newly created proofs, token entries that had errors
     */
    async receive(
        token: string | Token,
        options?: {
            preference?: Array<AmountPreference>;
            counter?: number;
            pubkey?: string;
            privkey?: string;
        }
    ): Promise<ReceiveResponse> {
        let decodedToken: Array<TokenEntry>;
        if (typeof token === 'string') {
            decodedToken = cleanToken(getDecodedToken(token)).token;
        } else {
            decodedToken = token.token;
        }
        const tokenEntries: Array<TokenEntry> = [];
        const tokenEntriesWithError: Array<TokenEntry> = [];
        for (const tokenEntry of decodedToken) {
            if (!tokenEntry?.proofs?.length) {
                continue;
            }
            try {
                const { proofs, proofsWithError } = await this.receiveTokenEntry(tokenEntry, {
                    preference: options?.preference,
                    counter: options?.counter,
                    pubkey: options?.pubkey,
                    privkey: options?.privkey
                });
                if (proofsWithError?.length) {
                    tokenEntriesWithError.push(tokenEntry);
                    continue;
                }
                tokenEntries.push({ mint: tokenEntry.mint, proofs: [...proofs] });
            } catch (error) {
                console.error(error);
                tokenEntriesWithError.push(tokenEntry);
            }
        }
        return {
            token: { token: tokenEntries },
            tokensWithErrors: tokenEntriesWithError.length ? { token: tokenEntriesWithError } : undefined
        };
    }

[Dayvvo]

@callebtc looking at the I don't think the receive function not returning errors for tokens that failed to be redeemed is a result/ consequence of the receive function being able to accept tokens from different mints and as such doesn't in itself hinder errors with failed tokens being returned. I'm not particularly sure if all errors would be useful but if we wanted to return errors here we could have them returned from the receiveTokenEntry function being called in receive.

Secondly regarding the counter I think changing it's application might need fixes across multiple functions as it seems to be consistently used the same way across multiple functions in the CashuWallet class. So personally I think that might be a lot of work.

let me know your thoughts on this @callebtc @gandlafbtc

[gandlafbtc]

I think we should aim to simplify the datastructure of tokens to not allow multiple tokens inside a token.

Reasons:

• less complexity and confusion
• we can disregard this change, and return errors properly
• if we must, we can achieve the same by chaining tokens and processing them sequentially

[gandlafbtc]

86 has some additional discussion on this topic

[Dayvvo]

@gandlafbtc as per the issue closed on #86 this issue can be closed out too, correct?

[gudnuf]

The api has been simplified and now either throws an error or returns the successfully swapped proofs. I think this can be closed @gandlafbtc