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

swap: remove honeyOracle #2132

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all 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
8 changes: 4 additions & 4 deletions swap/cheque.go
Original file line number Diff line number Diff line change
Expand Up @@ -132,22 +132,22 @@ func (cheque *Cheque) verifyChequeProperties(p *Peer, expectedBeneficiary common

// verifyChequeAgainstLast verifies that the amount is higher than in the previous cheque and the increase is as expected
// returns the actual amount received in this cheque
func (cheque *Cheque) verifyChequeAgainstLast(lastCheque *Cheque, expectedAmount *int256.Uint256) (*int256.Uint256, error) {
func (cheque *Cheque) verifyChequeAgainstLast(lastCheque *Cheque, expectedAmount *int256.Uint256) error {
actualAmount := cheque.CumulativePayout.Copy()

if lastCheque != nil {
if cheque.CumulativePayout.Cmp(lastCheque.CumulativePayout) < 1 {
return nil, fmt.Errorf("wrong cheque parameters: expected cumulative payout larger than %v, was: %v", lastCheque.CumulativePayout, cheque.CumulativePayout)
return fmt.Errorf("wrong cheque parameters: expected cumulative payout larger than %v, was: %v", lastCheque.CumulativePayout, cheque.CumulativePayout)
}

actualAmount.Sub(actualAmount, lastCheque.CumulativePayout)
}

if !expectedAmount.Equals(actualAmount) {
return nil, fmt.Errorf("unexpected amount for honey, expected %v was %v", expectedAmount, actualAmount)
return fmt.Errorf("unexpected amount for honey, expected %v was %v", expectedAmount, actualAmount)
}

return actualAmount, nil
return nil
}

func (cheque *Cheque) String() string {
Expand Down
41 changes: 0 additions & 41 deletions swap/honeyOracle.go

This file was deleted.

9 changes: 1 addition & 8 deletions swap/peer.go
Original file line number Diff line number Diff line change
Expand Up @@ -161,15 +161,8 @@ func (p *Peer) createCheque() (*Cheque, error) {
}
// the balance should be negative here, we take the absolute value:
honey := uint64(-p.getBalance())

oraclePrice, err := p.swap.honeyPriceOracle.GetPrice(honey)
if err != nil {
return nil, fmt.Errorf("error getting price from oracle: %v", err)
}
price := int256.Uint256From(oraclePrice)

cumulativePayout := p.getLastSentCumulativePayout()
newCumulativePayout, err := new(int256.Uint256).Add(cumulativePayout, price)
newCumulativePayout, err := new(int256.Uint256).Add(cumulativePayout, int256.Uint256From(honey))
if err != nil {
return nil, err
}
Expand Down
2 changes: 0 additions & 2 deletions swap/prices.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,4 @@ allowing for a multi-currency design.
const (
RetrieveRequestPrice = uint64(8043036262)
ChunkDeliveryPrice = uint64(17672687)
// default conversion of honey into output currency - currently ETH in Wei
defaultHoneyPrice = uint64(1)
)
23 changes: 7 additions & 16 deletions swap/swap.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,6 @@ type Swap struct {
params *Params // economic and operational parameters
contract contract.Contract // reference to the smart contract
chequebookFactory contract.SimpleSwapFactory // the chequebook factory used
honeyPriceOracle HoneyOracle // oracle which resolves the price of honey (in Wei)
cashoutProcessor *CashoutProcessor // processor for cashing out
logger Logger //Swap Logger
}
Expand Down Expand Up @@ -92,7 +91,6 @@ func newSwapInstance(stateStore state.Store, owner *Owner, backend chain.Backend
owner: owner,
params: params,
chequebookFactory: chequebookFactory,
honeyPriceOracle: NewHoneyPriceOracle(),
chainID: chainID,
cashoutProcessor: newCashoutProcessor(backend, owner.privateKey),
logger: logger,
Expand Down Expand Up @@ -360,7 +358,7 @@ func (s *Swap) handleEmitChequeMsg(ctx context.Context, p *Peer, msg *EmitCheque
})
}

_, err := s.processAndVerifyCheque(cheque, p)
err := s.processAndVerifyCheque(cheque, p)
if err != nil {
return protocols.Break(fmt.Errorf("processing and verifying received cheque: %w", err))
}
Expand Down Expand Up @@ -458,37 +456,30 @@ func cashCheque(s *Swap, cheque *Cheque) {
// processAndVerifyCheque verifies the cheque and compares it with the last received cheque
// if the cheque is valid it will also be saved as the new last cheque
// the caller is expected to hold p.lock
func (s *Swap) processAndVerifyCheque(cheque *Cheque, p *Peer) (*int256.Uint256, error) {
func (s *Swap) processAndVerifyCheque(cheque *Cheque, p *Peer) error {
if err := cheque.verifyChequeProperties(p, s.owner.address); err != nil {
return nil, err
return err
}

lastCheque := p.getLastReceivedCheque()

// TODO: there should probably be a lock here?
expectedAmount, err := s.honeyPriceOracle.GetPrice(cheque.Honey)
if err != nil {
return nil, err
}

actualAmount, err := cheque.verifyChequeAgainstLast(lastCheque, int256.Uint256From(expectedAmount))
if err != nil {
return nil, err
if err := cheque.verifyChequeAgainstLast(lastCheque, int256.Uint256From(cheque.Honey)); err != nil {
return err
}

// calculate tentative new balance after cheque is processed
newBalance := p.getBalance() - int64(cheque.Honey)
// check if this new balance would put creditor into debt
if newBalance < -int64(ChequeDebtTolerance) {
return nil, fmt.Errorf("received cheque would result in balance %d which exceeds tolerance %d and would cause debt", newBalance, ChequeDebtTolerance)
return fmt.Errorf("received cheque would result in balance %d which exceeds tolerance %d and would cause debt", newBalance, ChequeDebtTolerance)
}

if err := p.setLastReceivedCheque(cheque); err != nil {
p.logger.Error(HandleChequeAction, "error while saving last received cheque", "err", err.Error())
// TODO: what do we do here? Related issue: https://github.com/ethersphere/swarm/issues/1515
}

return actualAmount, nil
return nil
}

// loadLastReceivedCheque loads the last received cheque for the peer from the store
Expand Down
24 changes: 8 additions & 16 deletions swap/swap_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1110,14 +1110,10 @@ func TestPeerVerifyChequeAgainstLast(t *testing.T) {
t.Fatal(err)
}

actualAmount, err := newCheque.verifyChequeAgainstLast(oldCheque, increase)
err = newCheque.verifyChequeAgainstLast(oldCheque, increase)
if err != nil {
t.Fatalf("failed to verify cheque compared to old cheque: %v", err)
}

if !actualAmount.Equals(increase) {
t.Fatalf("wrong actual amount, expected: %v, was: %v", increase, actualAmount)
}
}

// TestPeerVerifyChequeAgainstLastInvalid tests that verifyChequeAgainstLast rejects cheques with lower amount or an unexpected value
Expand All @@ -1128,7 +1124,7 @@ func TestPeerVerifyChequeAgainstLastInvalid(t *testing.T) {
oldCheque := newTestCheque()
newCheque := newTestCheque()

if _, err := newCheque.verifyChequeAgainstLast(oldCheque, increase); err == nil {
if err := newCheque.verifyChequeAgainstLast(oldCheque, increase); err == nil {
t.Fatal("accepted a cheque with same amount")
}

Expand All @@ -1145,7 +1141,7 @@ func TestPeerVerifyChequeAgainstLastInvalid(t *testing.T) {
t.Fatal(err)
}

if _, err := newCheque.verifyChequeAgainstLast(oldCheque, increase); err == nil {
if err := newCheque.verifyChequeAgainstLast(oldCheque, increase); err == nil {
t.Fatal("accepted a cheque with unexpected amount")
}
}
Expand All @@ -1159,15 +1155,11 @@ func TestPeerProcessAndVerifyCheque(t *testing.T) {
cheque := newTestCheque()
cheque.Signature, _ = cheque.Sign(ownerKey)

actualAmount, err := swap.processAndVerifyCheque(cheque, peer)
err := swap.processAndVerifyCheque(cheque, peer)
if err != nil {
t.Fatalf("failed to process cheque: %s", err)
}

if !actualAmount.Equals(cheque.CumulativePayout) {
t.Fatalf("computed wrong actual amount: was %v, expected: %v", actualAmount, cheque.CumulativePayout)
}

// verify that it was indeed saved
if !peer.getLastReceivedCheque().CumulativePayout.Equals(cheque.CumulativePayout) {
t.Fatalf("last received cheque has wrong cumulative payout, was: %v, expected: %v", peer.lastReceivedCheque.CumulativePayout, cheque.CumulativePayout)
Expand All @@ -1182,7 +1174,7 @@ func TestPeerProcessAndVerifyCheque(t *testing.T) {
otherCheque.Honey = 10
otherCheque.Signature, _ = otherCheque.Sign(ownerKey)

if _, err := swap.processAndVerifyCheque(otherCheque, peer); err != nil {
if err := swap.processAndVerifyCheque(otherCheque, peer); err != nil {
t.Fatalf("failed to process cheque: %s", err)
}

Expand All @@ -1205,15 +1197,15 @@ func TestPeerProcessAndVerifyChequeInvalid(t *testing.T) {
cheque.Beneficiary = ownerAddress
cheque.Signature, _ = cheque.Sign(ownerKey)

if _, err := swap.processAndVerifyCheque(cheque, peer); err == nil {
if err := swap.processAndVerifyCheque(cheque, peer); err == nil {
t.Fatal("accecpted an invalid cheque as first cheque")
}

// valid cheque
cheque = newTestCheque()
cheque.Signature, _ = cheque.Sign(ownerKey)

if _, err := swap.processAndVerifyCheque(cheque, peer); err != nil {
if err := swap.processAndVerifyCheque(cheque, peer); err != nil {
t.Fatalf("failed to process cheque: %s", err)
}

Expand All @@ -1230,7 +1222,7 @@ func TestPeerProcessAndVerifyChequeInvalid(t *testing.T) {
otherCheque.Honey = 10
otherCheque.Signature, _ = otherCheque.Sign(ownerKey)

if _, err := swap.processAndVerifyCheque(otherCheque, peer); err == nil {
if err := swap.processAndVerifyCheque(otherCheque, peer); err == nil {
t.Fatal("accepted a cheque with lower amount")
}

Expand Down