Open steveluscher opened 2 years ago
Yeah, good points. It's true that it's not guaranteed to hit the same thread. We also probably lose these lockouts and the txids (#1) if the function becomes idle. The IP-based ratelimiting is subject to the same problem.
We could use an external store for all this persistent info. A downside is that it makes this more complex to deploy. Perhaps we can find a way to make it optional/modular with some kind of drop-in Store
interface.
📝 Note: Implementing #1 would pretty much solve this too, with some light tweaks.
I'm not sure using txids solves this problem, because a spammer could pick a bunch of recent blockhashes that all target the same source token account and thus have different signatures. Maybe you have another solution in mind though?
A related issue: assuming there are several instances of Octane (let's say, run by different parties), a spammer could hit all of them and make them race for it, wasting gas for everyone but the winner.
I'm not sure using txids solves this problem, because a spammer could…
Oh yeah; I just meant that if we stood up a persistent store in #1 we could use it to store these locks.
Perhaps we can find a way to make it optional/modular with some kind of drop-in Store interface.
I like this. Optional, with a build time warning that you should really consider standing up a for-real persistent store.
I feel like there's a startup somewhere in here.
spammer could hit all of them and make them race
This is probably solvable with captchas, but I think that reduces the chance of getting integrated with wallets. In general it would be nice to not hurt the UX like that.
make it optional/modular with some kind of drop-in
Store
interface
Turns out https://github.com/BryanDonovan/node-cache-manager#store-engines already does this.
We could provide config for the Redis store, maybe along with https://github.com/wyattjoh/rate-limit-redis, and add it to the docs or examples or something.
📝 Note: Implementing #1 would pretty much solve this too, with some light tweaks.
Preamble
Consider this code that's designed to prevent attackers from spamming Octane's generous offer to sign and simulate a transaction:
https://github.com/solana-labs/octane/blob/master/src/api/transfer.ts#L23-L32
That code uses a module-local
Set
to track which source accounts have in-flight transactions:https://github.com/solana-labs/octane/blob/master/src/api/transfer.ts#L7
Problem
Consecutive requests from an attacker are likely to hit the same thread/instance of the serverless function, but from what I understand this is not guaranteed.
From the Vercel docs:
If consecutive requests by the attacker hit different threads running our serverless API function, they will each have different
Sets
of source ids, and will permit the simulation/broadcast on each thread, rendering the defence mechanism ineffective.Possible solution
In general, with horizontally scalable systems of lambdas, you can rely on a shared state service to implement a mutex. We could consider building in support for something like Upstash – a Redis service. We could implement the source account lock as a [Redis distributed lock[(https://redis.io/topics/distlock).
https://docs.upstash.com/redis/howto/vercelintegration