cashubtc / cashu-ts

A TypeScript library for building Cashu wallets
MIT License
57 stars 33 forks source link

Increase timeouts to infinite #78

Closed callebtc closed 9 months ago

callebtc commented 1 year ago

The axios requests needs infinite timeouts for the endpoints /split, /melt, POST /mint so that in case of a slow mint, things don't go south (for example, mint responds to POST /mint with BlindSignatures after the client has already timed out, then refuses to issue tokens again (because they've been issued already).

See #72

BilligsterUser commented 1 year ago

you could use

setupAxios({timeout:0})

before making requests to those endpoints and set it to something else after the request

setupAxios({timeout:60000})

if we hard code the timeout user of the lib cannot override it.

callebtc commented 1 year ago

if we hard code the timeout user of the lib cannot override it.

Being able to override it without having to set global axios settings would be good enough for me. I can't set global settings because there are potentially multiple different requests going on at the same time.

But I'm arguing that actually everyone should wait as long as they can because of safety.

You'd rather want to wait a long time and risk sacrificing a connection than risk missing a transaction.

Here is one thing that happened to me: cashu.me has an invoice check worker (checks every n seconds). If the mint has a backend or database lag (I simulated with a breakpoint), the POST /mint times out and the invoice checker keeps checking for the token. If, in the mean time, the mint then suddenly responds with BlindSignatures to the dropped request, your coins are gone.

BilligsterUser commented 1 year ago

but I'm arguing that actually everyone should wait as long as they can because of safety.

You'd rather want to wait a long time and risk sacrificing a connection than risk missing a transaction.

i get it!

Here is one thing that happened to me: cashu.me has an invoice check worker (checks every n seconds). If the mint has a backend or database lag (I simulated with a breakpoint), the POST /mint times out and the invoice checker keeps checking for the token. If, in the mean time, the mint then suddenly responds with BlindSignatures to the dropped request, your coins are gone.

That's because cashu. me overrides the default timeout which is 0

callebtc commented 1 year ago

Really? I thought the default timeout is 15s in axios. I don't remember overriding anything (btw, cashu.me uses cashu-ts now)

BilligsterUser commented 1 year ago

Really? I thought the default timeout is 15s in axios. I don't remember overriding anything (btw, cashu.me uses cashu-ts now)

timeout: 1000, // default is 0 (no timeout)

https://github.com/axios/axios/blob/main/README.md#request-config

BilligsterUser commented 1 year ago

But we could add an optional parameter to set the timeout per function call. @gandlafbtc what do you think?

callebtc commented 1 year ago

timeout: 1000, // default is 0 (no timeout)

That's odd. All my requests time out after 15s here...

callebtc commented 1 year ago

If, in the mean time, the mint then suddenly responds with BlindSignatures to the dropped request, your coins are gone.

This can additinoally be fixed if the mint would reissue signatures for the same set of outputs. Will fix (see https://github.com/cashubtc/cashu/issues/257)

BilligsterUser commented 1 year ago

That's odd. All my requests time out after 15s here...

https://github.com/cashubtc/cashu.me/blob/234560e107e47b846e8f05340239c71cc71e06ce/src/boot/cashu.js#L8

BilligsterUser commented 1 year ago

Fifteen seconds is kind of short.

callebtc commented 1 year ago

https://github.com/cashubtc/cashu.me/blob/234560e107e47b846e8f05340239c71cc71e06ce/src/boot/cashu.js#L8

You're amazing man, you know my code better than I do.

I'd be ok with 15 seconds for all other endpoints, but for those mentioned above, I'd set to 0. Could we have that?

BilligsterUser commented 1 year ago

I'd be ok with 15 seconds for all other endpoints, but for those mentioned above, I'd set to 0. Could we have that?

But we could add an optional parameter to set the timeout per function call. @gandlafbtc what do you think?

gandlafbtc commented 1 year ago

Optional could be ok. Using the default (if it's already 0) seems also fine. I haven't experienced any issues yet, so you guys probably know better than me