pokt-network / pocket-core

Official implementation of the Pocket Network Protocol
http://www.pokt.network
MIT License
210 stars 104 forks source link

[BUG REPORT] Updating Evidences doesn't guarantee an atomic write #1457

Open nodiesBlade opened 2 years ago

nodiesBlade commented 2 years ago

Describe the bug With the current implementation of SetProof, this results in a race condition where evidences are not written into the cache storage atomically. This can result in evidences overwriting each other whenever concurrent relays are handled, and the node not being paid for the right amount of work when submitting a claim.

Consider this scenario:

  1. Two relays are sent to the node concurrently, labeled respectively A and B relay.
  2. Node validates both A and B relays
  3. Node retrieves the evidence to update relays A's proof, then subsequently grabs the evidence to update relays B proofs.
  4. The node at this time currently has a local copy of evidence for Relay A and another copy for Relay B. (two separate evidences in memory at this point!)
  5. The local copy of evidence A is updated with the proof, then updated into cache storage.
  6. The local copy of evidence B is updated with the proof, then updated into cache storage - overwriting the previous written evidence.
  7. Evidence storage now only contains 1 proof, where the expected amount is to be two.

Proof of concept test here I wrote it really quickly, might've missed something

Expected behavior We need a way to ensure that proofs are written to the evidence storage atomically. There's multiple ways to do this, you can add a lock within the SetProof function, and then every call to SetProof can be completed in a different go routine. By putting it in a different go routine, it won't block the response back to the caller due to lock contention (i.e under high load)

Or the more proper way is to set up a queue using channels that will buffer relay proofs to a worker that will update the evidences one by one, similarly to the consumers/producer problem.

POKT-Discourse commented 1 year ago

This issue has been mentioned on Pocket Network Forum. There might be relevant details there:

https://forum.pokt.network/t/poktscan-geo-mesh-reimbursement/3945/3