swarm icon indicating copy to clipboard operation
swarm copied to clipboard

processing cashing transaction result strategy

Open ralph-pichler opened this issue 5 years ago • 6 comments

The transaction scheduler will provide a mechanism to reliably be notified of a cashing transactions result. The question is what do we do when we are notified.

The obvious:

  • if the cheque bounced we drop / blacklist (if available) the peer
  • otherwise at least update some metrics

Questions:

  • should we store a record of this? (originally the issue for this was called "marking cheques cashed")
  • how does this interact with testing? during simulation tests and other tests we want to check wether a transaction was sent and processed successfully. Currently this is based on the cashDone channel on the backend which is suboptimal as it uses both one channel for all running swap instances and requires use of global function pointer variables. this fits even less in the transaction scheduler design which requires a hook in the production code for this to be even possible.
  • what if transaction failed?
    • transaction never sent
    • transaction never confirmed or unknown status
    • transaction confirmed but error status

ralph-pichler avatar Feb 18 '20 15:02 ralph-pichler

I think it might make sense to store (in swap or the cashout processor) extra information for every peer:

  • the last cumulativeAmount that was scheduled to be cashed (and its associated request id). this would also allow a better calculated of the expected profit of cashing a cheque as the current formula computes the wrong value if cashout transaction is already in progress.
  • the last cumulativeAmount that was actually paidOut (this allows querying this in tests).

ralph-pichler avatar Feb 18 '20 18:02 ralph-pichler

Thanks for the write up! My opinion on this is that SWAP assumes a cheque as money from the point that the cheque is received. The exact mechanics on how/when and if a cheque is cashed out is not relevant for SWAP, unless 1) a cheque bounces or 2) when cashing out is really not possible anymore in general.

For 1, you can send a message directly to SWAP and blacklist. For 2, perhaps it makes sense te keep track of a counter internally (non-successful cashout/successful cashout) and when this ratio is (say) more than 10%, send a message to SWAP, making the node shut down.

About 2, I'm not very certain as this might cause network-wide troubles if there is for example a bug in the Ethereum client. Also, some node operators might prefer to get a text message or email or be warned otherwise before their client shuts down.

I see it as a responsibility of SWAP to acquire uncashed cheques, save them to the storage. And then we can even create a completely orthogonal process that can run as a deamon or act as a wallet next to Swarm that looks in the database where cheques are stored and cashes them based on its own logic. We can use the SWAP APIs, to inform SWAP about bounced cheques (referencing the transactionHash of the transaction which bounced).

Eknir avatar Feb 19 '20 09:02 Eknir

The exact mechanics on how/when and if a cheque is cashed out is not relevant for SWAP, unless 1) a cheque bounces or 2) when cashing out is really not possible anymore in general.

I see it as a responsibility of SWAP to acquire uncashed cheques, save them to the storage. And then we can even create a completely orthogonal process that can run as a deamon or act as a wallet next to Swarm that looks in the database where cheques are stored and cashes them based on its own logic. We can use the SWAP APIs, to inform SWAP about bounced cheques (referencing the transactionHash of the transaction which bounced).

It is relevant insofar it might cause problems if some process is attempting to cash out while SWAP tries to deposit from the same account (or do something else). Also introducing another process creates potential for a ton of problems and a lot of new edge cases (e.g. that process dies, io of that process fails but does not for swarm (or reverse), etc.). Keeping this in-process will be much simpler.

ralph-pichler avatar Feb 24 '20 13:02 ralph-pichler

Isn't it an idea to build a general-purpose transaction scheduler that runs as a separate process on your machine? This scheduler will be similar to your transaction queue but has well-defined APIs, and can also be used by other blockchain projects. If we have this, the source code of SWAP can become much simpler and we build a component that has general usability for the Ethereum ecosystem. I can imagine we can get a Gitcoin grand for a project like this in the next funding round. Furthermore, having this, it will also be more trivial to create yet another process that takes care of the cashout logic.

Eknir avatar Feb 25 '20 12:02 Eknir

This sounds much more complex than the current plan. Also I don't think that the SWAP codebase would become much simpler. Instead of the transaction logic we would have a lot of code to ensure SWAPs knowledge about transaction stays in sync with the external process, not to mention all of the potential edge cases the arise from that.

ralph-pichler avatar Feb 25 '20 13:02 ralph-pichler

I think it might make sense to store (in swap or the cashout processor) extra information for every peer: the last cumulativeAmount that was scheduled to be cashed (and its associated request id). this would also allow a better calculated of the expected profit of cashing a cheque as the current formula computes the wrong value if cashout transaction is already in progress. the last cumulativeAmount that was actually paidOut (this allows querying this in tests).

+1

to me, this:

The exact mechanics on how/when and if a cheque is cashed out is not relevant for SWAP, unless 1) a cheque bounces or 2) when cashing out is really not possible anymore in general.

is somewhat contradictory to this:

I see it as a responsibility of SWAP to acquire uncashed cheques, save them to the storage.

i understand they are different things, but they are both related to the same business logic.

About 2, I'm not very certain as this might cause network-wide troubles if there is for example a bug in the Ethereum client. Also, some node operators might prefer to get a text message or email or be warned otherwise before their client shuts down.

we can probably do something like this, but let's make a separate issue (i think the priority is not as high as other stuff right now).

in any case, i am also of the mind of keeping this in-process for now.

as soon as it is implemented, we can look at an alternative design through a draft PR, or something of the sort.

mortelli avatar Feb 28 '20 15:02 mortelli