Nostr Web Client

utxo the webmaster 🧑‍💻 2y ago

Here is the code that allowed someone to double wirhdraw from nodeless.

Can you spot why this doesn't work?

Reply to this note

Please Login to reply.

Discussion

boston wine 2y ago

I’m not a programmer so this is a guess

Is it because the first “withdrawal” line is related to User ID, but the “withdrawal lock” line relates to User Email?

Again, not trained here but looking for discrepancies…

utxo the webmaster 🧑‍💻 2y ago

Nah, one is a key inside a rate limiter, the other is just for logging so I can see it. Doesn't affect the bug

boston wine 2y ago

Makes sense. Thanks for the feedback 🤙

Lucas X. 2y ago

This is what our friend GPT-4 said:

utxo the webmaster 🧑‍💻 2y ago

Yup that's right. And basically how it was fixed

hodlbod 2y ago

`withdrawal_lock` wasn't set, or was set to a non-boolean?

utxo the webmaster 🧑‍💻 2y ago

It gets set later in the code before the withdrawal happens

Cameri🐦‍🔥 2y ago

The if clause for that env var is empty. It should have returned or thrown an error.

utxo the webmaster 🧑‍💻 2y ago

Lol yes but no, didn't want to globally disable withdrawals in this case

Cameri🐦‍🔥 2y ago

I can’t see where you set the lock. It’s possible the lock was never set.

utxo the webmaster 🧑‍💻 2y ago

Lock does get set, but apparently not fast enough if two requests come in concurrently :/

Big problem was app is running serverless, so requests can process at identical time before database actually records it 😅

ben 2y ago

missed the return, homie

utxo the webmaster 🧑‍💻 2y ago

Lol nah that's on purpose

Cameri🐦‍🔥 2y ago

I’m surprised no one said it: PHP

utxo the webmaster 🧑‍💻 2y ago

PHP is awesome now, give it another try!! Ain't your 2004 PHP!!

Cameri🐦‍🔥 2y ago

I love PHP but I don’t use it anymore

thesimplekid 2y ago

I thought about it but felt bad lol

ben 2y ago

deeply considered “php is a hell of a drug” 😂

SirGalahodl 2y ago

`withdrawal_lock` was not set?

utxo the webmaster 🧑‍💻 2y ago

In a sense yes, but it was not set due to concurrent requests, not due to the code not setting it

SirGalahodl 2y ago

I see that now... what's the purpose of `sleep` statement when you already have the `RateLimiter`?

utxo the webmaster 🧑‍💻 2y ago

So that the rate limiters have lower odds of being fired at the same time.

Didn't work, though