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

Swap Bounced Cheque Verification Process #2134

Open
wants to merge 13 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 12 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion contracts/swap/factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import (

"github.com/ethereum/go-ethereum/accounts/abi/bind"
"github.com/ethereum/go-ethereum/common"
chequebookFactory "github.com/ethersphere/go-sw3/contracts-v0-2-0/simpleswapfactory"
chequebookFactory "github.com/ethersphere/go-sw3/contracts-v0-2-3/simpleswapfactory"
"github.com/ethersphere/swarm/swap/chain"
)

Expand Down
10 changes: 9 additions & 1 deletion contracts/swap/swap.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ import (
"github.com/ethereum/go-ethereum/accounts/abi/bind"
"github.com/ethereum/go-ethereum/common"
"github.com/ethereum/go-ethereum/core/types"
contract "github.com/ethersphere/go-sw3/contracts-v0-2-0/erc20simpleswap"
contract "github.com/ethersphere/go-sw3/contracts-v0-2-3/erc20simpleswap"
"github.com/ethersphere/swarm/swap/chain"
"github.com/ethersphere/swarm/swap/int256"
)
Expand All @@ -53,6 +53,8 @@ type Contract interface {
Issuer(opts *bind.CallOpts) (common.Address, error)
// PaidOut returns the total paid out amount for the given address
PaidOut(opts *bind.CallOpts, addr common.Address) (*big.Int, error)
// Bounced returns if there has been a bounced cheque from the chequebook
Bounced(opts *bind.CallOpts) (bool, error)
}

// CashChequeResult summarizes the result of a CashCheque or CashChequeBeneficiary call
Expand Down Expand Up @@ -91,6 +93,12 @@ func InstanceAt(address common.Address, backend chain.Backend) (Contract, error)
return c, err
}

// Bounced obtains if a cheque has been bounced previously of a contract at a specific address.
mortelli marked this conversation as resolved.
Show resolved Hide resolved
// Once it's bounced, it will not change it's state again, unless re deployed.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo in its

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you fixed the one that was correct, it should read:

Once it's bounced, it will not change its state again

func (s simpleContract) Bounced(opts *bind.CallOpts) (bool, error) {
return s.instance.Bounced(opts)
}

// Withdraw withdraws amount from the chequebook and blocks until the transaction is mined
func (s simpleContract) Withdraw(auth *bind.TransactOpts, amount *big.Int) (*types.Receipt, error) {
tx, err := s.instance.Withdraw(auth, amount)
Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ require (
github.com/docker/go-connections v0.4.0 // indirect
github.com/docker/go-units v0.4.0 // indirect
github.com/ethereum/go-ethereum v1.9.2
github.com/ethersphere/go-sw3 v0.2.1
github.com/ethersphere/go-sw3 v0.2.3
github.com/fatih/color v1.7.0 // indirect
github.com/fjl/memsize v0.0.0-20180418122429-ca190fb6ffbc
github.com/go-kit/kit v0.9.0 // indirect
Expand Down
2 changes: 2 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,8 @@ github.com/ethereum/go-ethereum v1.9.2 h1:RMIHDO/diqXEgORSVzYx8xW9x2+S32PoAX5lQw
github.com/ethereum/go-ethereum v1.9.2/go.mod h1:PwpWDrCLZrV+tfrhqqF6kPknbISMHaJv9Ln3kPCZLwY=
github.com/ethersphere/go-sw3 v0.2.1 h1:+i660uWzhRbT1YO7MAeuxzn+jUeYOTc8rGRVjsKaw+4=
github.com/ethersphere/go-sw3 v0.2.1/go.mod h1:HukT0aZ6QdW/d7zuD/0g5xlw6ewu9QeqHojxLDsaERQ=
github.com/ethersphere/go-sw3 v0.2.3 h1:7fmXB26sDvwoWiQ2wA53x9GRhi5+hPn4mUTuClx7wPc=
github.com/ethersphere/go-sw3 v0.2.3/go.mod h1:HukT0aZ6QdW/d7zuD/0g5xlw6ewu9QeqHojxLDsaERQ=
github.com/evanphx/json-patch v4.2.0+incompatible/go.mod h1:50XU6AFN0ol/bzJsmQLiYLvXMP4fmwYFNcr97nuDLSk=
github.com/fatih/color v1.7.0 h1:DkWD4oS2D8LGGgTQ6IvwJJXSL5Vp2ffcQg58nFV38Ys=
github.com/fatih/color v1.7.0/go.mod h1:Zm6kSWBoL9eyXnKyktHP6abPY2pDugNf5KwzbycvMj4=
Expand Down
2 changes: 1 addition & 1 deletion swap/common_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import (
"github.com/ethereum/go-ethereum/log"
"github.com/ethereum/go-ethereum/p2p"
"github.com/ethereum/go-ethereum/p2p/simulations/adapters"
contractFactory "github.com/ethersphere/go-sw3/contracts-v0-2-0/simpleswapfactory"
contractFactory "github.com/ethersphere/go-sw3/contracts-v0-2-3/simpleswapfactory"
cswap "github.com/ethersphere/swarm/contracts/swap"
"github.com/ethersphere/swarm/network"
"github.com/ethersphere/swarm/p2p/protocols"
Expand Down
25 changes: 25 additions & 0 deletions swap/peer.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,15 @@ func NewPeer(p *protocols.Peer, s *Swap, beneficiary common.Address, contractAdd
return nil, err
}

bounced := false
bounced, err = peer.getBouncedCheque()
if err != nil {
return nil, err
}
if bounced {
return nil, ErrBouncedCheque
}

return peer, nil
}

Expand Down Expand Up @@ -221,3 +230,19 @@ func (p *Peer) sendCheque() error {
Cheque: cheque,
})
}

// getBouncedCheque retrieves if a bounced cheque exists at address from the blockchain
mortelli marked this conversation as resolved.
Show resolved Hide resolved
func (p *Peer) getBouncedCheque() (bool, error) {
mortelli marked this conversation as resolved.
Show resolved Hide resolved
bounced := false
var err error

// if chequebook not defined don't call contract methods
if p.swap.contract != nil {
bounced, err = p.swap.contract.Bounced(nil)
if err != nil {
return false, err
}
}

return bounced, nil
}
3 changes: 3 additions & 0 deletions swap/protocol.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,9 @@ var (
// structure of the HandshakeMsg
ErrInvalidHandshakeMsg = errors.New("invalid handshake message")

// ErrBouncedCheque is used when the peer has a bounced cheque and should be disconnected
ErrBouncedCheque = errors.New("bounced cheque detected")

// Spec is the swap protocol specification
Spec = &protocols.Spec{
Name: "swap",
Expand Down
9 changes: 6 additions & 3 deletions swap/protocol_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -549,7 +549,10 @@ func TestSwapRPC(t *testing.T) {
defer clean()

// need to have a dummy contract or the call will fail at `GetParams` due to `NewAPI`
swap.contract, err = contract.InstanceAt(common.Address{}, swap.backend)
depositAmount := int256.Uint256From(9000 * RetrieveRequestPrice)

// deploy a chequebook
err = testDeploy(context.TODO(), swap, depositAmount)
mortelli marked this conversation as resolved.
Show resolved Hide resolved
if err != nil {
t.Fatal(err)
}
Expand Down Expand Up @@ -594,7 +597,7 @@ func TestSwapRPC(t *testing.T) {
t.Fatalf("Expected balance to be 0 but it is %d", balance)
}

peer1, err := swap.addPeer(dummyPeer1.Peer, common.Address{}, common.Address{})
peer1, err := swap.addPeer(dummyPeer1.Peer, common.Address{}, swap.GetParams().ContractAddress)
if err != nil {
t.Fatal(err)
}
Expand All @@ -603,7 +606,7 @@ func TestSwapRPC(t *testing.T) {
t.Fatal(err)
}

peer2, err := swap.addPeer(dummyPeer2.Peer, common.Address{}, common.Address{})
peer2, err := swap.addPeer(dummyPeer2.Peer, common.Address{}, swap.GetParams().ContractAddress)
if err != nil {
t.Fatal(err)
}
Expand Down
2 changes: 1 addition & 1 deletion swap/simulations_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ import (
"github.com/ethereum/go-ethereum/p2p/simulations"
"github.com/ethereum/go-ethereum/p2p/simulations/adapters"
"github.com/ethereum/go-ethereum/rpc"
contractFactory "github.com/ethersphere/go-sw3/contracts-v0-2-0/simpleswapfactory"
contractFactory "github.com/ethersphere/go-sw3/contracts-v0-2-3/simpleswapfactory"
cswap "github.com/ethersphere/swarm/contracts/swap"
"github.com/ethersphere/swarm/network/simulation"
"github.com/ethersphere/swarm/p2p/protocols"
Expand Down
8 changes: 8 additions & 0 deletions swap/swap.go
Original file line number Diff line number Diff line change
Expand Up @@ -463,6 +463,14 @@ func (s *Swap) processAndVerifyCheque(cheque *Cheque, p *Peer) (*int256.Uint256,
return nil, err
}

bounced, err := p.getBouncedCheque()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hasBouncedCheque is probably a more accurate variable name here, what do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

p.getBouncedCheque() is now p.hasBouncedCheque()
bounced could be rename to bouncedCheque so it's not verboes

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the bouncedCheque variable is not a cheque, therefore it should be called something boolean e.g. hasBouncedCheque

if err != nil {
return nil, err
}
if bounced {
return nil, ErrBouncedCheque
}

lastCheque := p.getLastReceivedCheque()

// TODO: there should probably be a lock here?
Expand Down
Loading