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 1 commit
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 @@ -133,22 +133,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 *uint256.Uint256) (*uint256.Uint256, error) {
func (cheque *Cheque) verifyChequeAgainstLast(lastCheque *Cheque, expectedAmount *uint256.Uint256) error {
mortelli marked this conversation as resolved.
Show resolved Hide resolved
actualAmount := uint256.New().Copy(cheque.CumulativePayout)

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 := uint256.FromUint64(oraclePrice)

cumulativePayout := p.getLastSentCumulativePayout()
newCumulativePayout, err := uint256.New().Add(cumulativePayout, price)
newCumulativePayout, err := uint256.New().Add(cumulativePayout, uint256.FromUint64(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) (*uint256.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, uint256.FromUint64(expectedAmount))
if err != nil {
return nil, err
if err := cheque.verifyChequeAgainstLast(lastCheque, uint256.FromUint64(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