omni / poa-bridge

POA <-> Ethereum bridge for self transfers of POA native token to POA20 (ERC20 representation). Not supported. Use TokenBridge instead
https://github.com/poanetwork/token-bridge
GNU General Public License v3.0
79 stars 38 forks source link

Problem: potential loss of database updates #95

Closed yrashk closed 6 years ago

yrashk commented 6 years ago

chance of loss of database updates that would cause it to redo transactions it already did.

Let's say we've got some database updates through deposit relaying: https://github.com/poanetwork/poa-bridge/blob/master/bridge/src/bridge/mod.rs#L164-L165

Then, during relaying withdrawals, an error happened: https://github.com/poanetwork/poa-bridge/blob/master/bridge/src/bridge/mod.rs#L171-L172

This means that we won't reach https://github.com/poanetwork/poa-bridge/blob/master/bridge/src/bridge/mod.rs#L185-L193 to save the result.

Also, in a similar vein, if one of these streams was to end (Ready(None)) we'd experience a similar loss of updates: https://github.com/poanetwork/poa-bridge/blob/master/bridge/src/macros.rs#L5

Solution: refactor bridge into two streams, splitting responsibilities

One stream (BridgeEventStream) returns BridgeChecked and the other (Bridge) writes those checks down to the database.

This way we're not accumulating chcecks before we serialize them, risking not serializing them at all in the event of an unrelated error.

Fixes #84