swarm
swarm copied to clipboard
swap: prevent bad cheques from being sent
WIP
This PR modifies the SWAP Add
function so that chequebook balance is inspected before writing a
cheque.
If the cheque's increase of cumulative payout is larger than the chequebook's available balance, the cheque will not be set nor sent and an error will be returned.
closes #1885
Current Issues
- due to new deadlock, peer lock calls in the
Cheques
function had to be (temporarily) commented out.-
Add
activates a peer lock when called, and onceAvailableBalance
was called during accounting, it attempts to call theCheques
function. As an RPC,Cheques
attempts to hold this peer lock as well, which causes a deadlock. - ~i'm not convinced with the idea of using RPCs internally, i.e. for "regular" SWAP logic. i'd prefer (at least) wrapper functions for RPCs which are not used anywhere else.~
- ~this also may bring back the discussion of whether (or not) to have specific functions for sent and received cheques (see points raised by @acud on #1863), be it for a specific peer or for all of them.~
- ~this now also has to be updated to include pending cheques instead, when they exist.~
-
- ~
TestMultiChequeSimulation
is failing with its originalmaxCheques
value of10
, since it seems the cumulative payouts amount to more than the chequebook's available balance.~
Adding the help needed
tag in an attempt to solve current issues and move ahead with development.
Slightly modified from the discussion here we want to use this formula for determining the available balance for a peer:
s.contract.liquidBalanceFor(peer) - (SUM(forall peers p, p.getLastCheque*().CumulativeAmount) - s.contract.totalPaidOut())
.
- The
totalPaidOut
call is only available in the new ERC20 chequebook. - In the sum the pending cheque must be used if it exists
- To avoid having to load the last sent / pending cheque for every peer we can also separately store the sum of the
CumulativeAmount
s or compute it at startup. As this value only increases when we create a pending cheque we just have to update the cached value there.
Rationale:
-
SUM(forall peers p, p.getLastCheque*().CumulativeAmount) - s.contract.totalPaidOut()
is the total amount of still uncashed cheques.s.contract.liquidBalanceFor(peer)
is the onchain balance available for a peer (including hard deposits). By reducing it by the first value we get the amount of the liquid balance that is not yet promised by another cheque. - This formula has the advantage that it only requires 2
eth_Call
operations to get the latest value.
i'm not convinced with the idea of using RPCs internally, i.e. for "regular" SWAP logic. i'd prefer (at least) wrapper functions for RPCs which are not used anywhere else.
we've discussed with the rest of the incentives team on 19/11, and came to the conclusion that we should use wrappers to expose business logic methods as RPC functions.
due to new deadlock, peer lock calls in the
Cheques
function had to be (temporarily) commented out.
Add
activates a peer lock when called, and onceAvailableBalance
was called during accounting, it attempts to call theCheques
function. As an RPC,Cheques
attempts to hold this peer lock as well, which causes a deadlock.
also discussed on 19/11: we're going to wait for the new implementation of AvailableBalance
before revisiting this issue.
@mortelli , what is the status here?
@mortelli , what is the status here?
still in progress, but not actively being worked on right now