Skip to content
This repository has been archived by the owner on Aug 2, 2021. It is now read-only.

swap: prevent bad cheques from being sent #1925

Draft
wants to merge 51 commits into
base: master
Choose a base branch
from

Conversation

mortelli
Copy link
Contributor

@mortelli mortelli commented Nov 1, 2019

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 once AvailableBalance was called during accounting, it attempts to call the Cheques 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 swap: expand RPC calls for cheque info querying #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 original maxCheques value of 10, since it seems the cumulative payouts amount to more than the chequebook's available balance.

@mortelli mortelli added in progress incentives builds on open PR Builds on a PR that is not yet merged. It is blocked until then and the diff won't make sense yet. labels Nov 1, 2019
@mortelli mortelli removed the builds on open PR Builds on a PR that is not yet merged. It is blocked until then and the diff won't make sense yet. label Nov 5, 2019
…eques

# Conflicts:
#	swap/peer.go
#	swap/protocol_test.go
#	swap/swap.go
@mortelli
Copy link
Contributor Author

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 once AvailableBalance was called during accounting, it attempts to call the Cheques 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 mortelli removed the on hold label Dec 9, 2019
@Eknir
Copy link
Contributor

Eknir commented Feb 6, 2020

@mortelli , what is the status here?

@mortelli
Copy link
Contributor Author

mortelli commented Feb 25, 2020

@mortelli , what is the status here?

still in progress, but not actively being worked on right now

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

Successfully merging this pull request may close these issues.

Don't let swarm cause ChequeBounced events
3 participants