Skip to content
This repository has been archived by the owner on Feb 8, 2018. It is now read-only.

forestall hole in payday #160

Closed
chadwhitacre opened this issue Jul 18, 2012 · 5 comments
Closed

forestall hole in payday #160

chadwhitacre opened this issue Jul 18, 2012 · 5 comments

Comments

@chadwhitacre
Copy link
Contributor

We may have a race condition that could be abused. When we transfer money from one person to another during payday, we only want to use their balance as it stands at the beginning of payday. What happens if a participant withdraws all their money while payday is happening? We would make transfers assuming the money was there, meaning that balance would drop below zero. Receivers of gifts made after the withdrawal would be able to withdraw money themselves that they weren't really entitled to. An attacker could set up accounts on both sides of this equation and wouldn't even need stolen credit cards to take advantage of this.

This is something to be sure we protect against when we implement automated withdrawals (#22).

We should probably put a db constraint on balance that it must be greater than zero (#161).

@jackdempsey
Copy link

This part confuses me:

"What happens if a participant withdraws all their money while payday is happening?"

What does "while payday is happening" mean? I'd imagine that you're dealing with money inside a transaction so you never have a case where that value can be updated by multiple people at the same time?

Money here. . . . . . (can i do something, answer: NO) . . . . . . ok money over here now. . . .now you can do something

If someone can update a value while payday is happening, that's obviously bad, right?

I'm either really missing this and completely don't follow what you're saying...or this is really not designed correctly.

Your note on twitter almost made it seem like the money would be eventually consistent?

@chadwhitacre
Copy link
Contributor Author

@jackdempsey The payday algorithm is designed to be crash-resistant and parallelizable, but it's not eventually consistent in the strict sense (iinm) because consistency is always apodeictically knowable.

Exchanges (moving money between Gittip and the outside world) and transfers (moving money amongst Gittip users) happen within an isolated event called payday. This event has duration (it's not punctiliar). It is started transactionally, and it ends transactionally, and inside of it, exchanges and transfers happen transactionally (though the link between our db and our processor's db could be tightened up; see #213). These exchanges and transfers accrue against a "pending" column in the database. Once the payday event has completed successfully, it ends with the pending column being applied to the balance column and reset to NULL in a single transaction.

Last week, week 8, payday took 16 minutes to run. During week 7 it crashed part-way through (#169). In week 7 I was able to determine the problem and when I re-ran the payday script, it skipped tips that had already resulted in transfers for the current payday (this pruning happens in get_tips_and_total).

Admittedly the week 6 corruption and the week 7 crash were due to a bug in this very algorithm. I still believe payday should be resilient against other unforeseen crashes, and as mentioned I believe we want payday to be parallelizable as well. My understanding is that we can parallelize the current algorithm by having multiple cores work on subsets of the list of participants in parallel. Their activity will be coordinated by the transactions per-exchange and per-transfer, and a master process can be responsible for starting and ending payday.

I appreciate feedback on this algorithm.

@chadwhitacre
Copy link
Contributor Author

consistency is always apodeictically knowable.

In other words, it's always possible to determine whether you're inside a payday event and how far you are through it.

@chadwhitacre
Copy link
Contributor Author

Okay, so two ways to protect against this:

  1. Constrain payouts to only happen as part of payday itself (payday becomes two-phase).
  2. If we allow manual payouts, only allow them outside of payday.

I'm going to say that #22 is about automatic payouts that are part of payday. I've made a new ticket for one-off payouts (#242) and have referenced this ticket there.

@chadwhitacre
Copy link
Contributor Author

For the record (because I'm about to maybe reference this ticket), here's the Twitter thread that brought @jackdempsey here:

https://twitter.com/jackdempsey/status/227555081455808513

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants