diff --git a/swap/cheque.go b/swap/cheque.go index 7760a88db7..e3ce106037 100644 --- a/swap/cheque.go +++ b/swap/cheque.go @@ -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 { diff --git a/swap/honeyOracle.go b/swap/honeyOracle.go deleted file mode 100644 index 50ae044936..0000000000 --- a/swap/honeyOracle.go +++ /dev/null @@ -1,41 +0,0 @@ -// Copyright 2019 The go-ethereum Authors -// This file is part of the go-ethereum library. -// -// The go-ethereum library is free software: you can redistribute it and/or modify -// it under the terms of the GNU Lesser General Public License as published by -// the Free Software Foundation, either version 3 of the License, or -// (at your option) any later version. -// -// The go-ethereum library is distributed in the hope that it will be useful, -// but WITHOUT ANY WARRANTY; without even the implied warranty of -// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -// GNU Lesser General Public License for more details. -// -// You should have received a copy of the GNU Lesser General Public License -// along with the go-ethereum library. If not, see . - -package swap - -// HoneyOracle is the interface through which Oracles will deliver prices -type HoneyOracle interface { - GetPrice(honey uint64) (uint64, error) -} - -// NewHoneyPriceOracle returns the actual oracle to be used for discovering the price -// It will return a default one -func NewHoneyPriceOracle() HoneyOracle { - - return &fixedPriceOracle{ - honeyPrice: defaultHoneyPrice, - } -} - -// fixedPriceOracle is a price oracle which which returns a fixed price -type fixedPriceOracle struct { - honeyPrice uint64 -} - -// GetPrice returns the actual price for honey -func (cpo *fixedPriceOracle) GetPrice(honey uint64) (uint64, error) { - return honey * cpo.honeyPrice, nil -} diff --git a/swap/peer.go b/swap/peer.go index 5614a36da8..61e7d7a0e4 100644 --- a/swap/peer.go +++ b/swap/peer.go @@ -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 } diff --git a/swap/prices.go b/swap/prices.go index 60a1ec2240..0b6bf3860d 100644 --- a/swap/prices.go +++ b/swap/prices.go @@ -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) ) diff --git a/swap/swap.go b/swap/swap.go index 913a432252..558a6fc045 100644 --- a/swap/swap.go +++ b/swap/swap.go @@ -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 } @@ -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, @@ -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)) } @@ -458,29 +456,22 @@ 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 { @@ -488,7 +479,7 @@ func (s *Swap) processAndVerifyCheque(cheque *Cheque, p *Peer) (*int256.Uint256, // 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 diff --git a/swap/swap_test.go b/swap/swap_test.go index b562a0c35f..487d592376 100644 --- a/swap/swap_test.go +++ b/swap/swap_test.go @@ -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 @@ -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") } @@ -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") } } @@ -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) @@ -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) } @@ -1205,7 +1197,7 @@ 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") } @@ -1213,7 +1205,7 @@ func TestPeerProcessAndVerifyChequeInvalid(t *testing.T) { 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) } @@ -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") }