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

swap, uint256: unify variable types, pt. 1 #2063

Merged
merged 67 commits into from
Jan 15, 2020
Merged
Show file tree
Hide file tree
Changes from 63 commits
Commits
Show all changes
67 commits
Select commit Hold shift + click to select a range
ce6d7e0
swap: add skeleton for Uint256
mortelli Dec 24, 2019
48e842f
swap: iterate Uint256 struct
mortelli Dec 24, 2019
d10c32a
swap: iterate Uint256 struct
mortelli Dec 24, 2019
8e933b0
swap: iterate Uint256 struct
mortelli Dec 24, 2019
3be2ed8
swap: iterate Uint256 struct
mortelli Dec 24, 2019
44aa724
swap: iterate Uint256 struct
mortelli Dec 24, 2019
5802fde
swap: iterate Uint256 struct
mortelli Dec 24, 2019
93722f9
swap: iterate Uint256 struct
mortelli Dec 26, 2019
c8a1ee4
swap: change ChequeParams CumulativePayout field to Uint256, rewrite …
mortelli Dec 26, 2019
5ee8279
swap: add Cmp function for Uint256 type
mortelli Dec 26, 2019
068881c
swap: rewrite verifyChequeAgainstLast function to work with Uint256 v…
mortelli Dec 26, 2019
c4c2a99
swap: iterate Uint256 struct
mortelli Dec 26, 2019
60b9b99
swap: iterate Uint256
mortelli Dec 26, 2019
388ed44
swap: add Uint64ToUint256 function
mortelli Dec 26, 2019
566230f
swap: add Mul function to Uint256 struct
mortelli Dec 27, 2019
2682e46
swap: adapt handleEmitChequeMsg function to work with Uint256
mortelli Dec 27, 2019
4dd0330
swap: iterate Uint256
mortelli Dec 27, 2019
e92d085
swap: refactor AvailableBalance
mortelli Dec 27, 2019
587b345
swap: iterate Uint256
mortelli Dec 27, 2019
87a52f1
swap: iterate Uint256
mortelli Dec 27, 2019
031b3c4
swap: make tests compile
mortelli Dec 27, 2019
2732ac8
swap: add MarshalJSON and UnmarshalJSON functions to Uint256
mortelli Dec 27, 2019
d38cede
swap: fix nil reference in TestEmitCheque
mortelli Dec 30, 2019
933606d
swap: fix bug in encodeForSignature function
mortelli Jan 2, 2020
4d2c0c2
swap: change FromUint64 signature to have no receiver
mortelli Jan 2, 2020
7b2d982
swap: simplify Uint64ToUint256 calls
mortelli Jan 2, 2020
964d564
swap: fix bug in Uint64ToUint256 function
mortelli Jan 2, 2020
f5b90bb
swap: add NewUint256 function
mortelli Jan 2, 2020
70cb0bb
swap: add comment to NewUint256
mortelli Jan 2, 2020
efc4a25
swap: remove unnecessary comment
mortelli Jan 2, 2020
a9dc476
swap: simplify encodeForSignature function
mortelli Jan 2, 2020
f650487
Revert "swap: simplify encodeForSignature function"
mortelli Jan 2, 2020
b36c9d5
swap: refactor Uint256 funcs
mortelli Jan 2, 2020
2bf0b2d
swap: add String method for Uint256 struct
mortelli Jan 2, 2020
a793a34
swap: make Uint256 value public and remove Value func
mortelli Jan 3, 2020
23e2150
swap: add Equals function for Uint256 struct
mortelli Jan 3, 2020
b5b19c2
swap: fix bug in TestPeerVerifyChequeAgainstLastInvalid
mortelli Jan 3, 2020
960341a
swap: fix bug in createCheque
mortelli Jan 3, 2020
bab6443
swap: fix bug in handleEmitChequeMsg
mortelli Jan 3, 2020
f6448b8
swap: add Copy method for Uint256 struct
mortelli Jan 3, 2020
c76d7cf
swap: fix wrong comparisons of Uint256 in swap tests
mortelli Jan 3, 2020
052fc3b
swap: fix missing mem allocation in AvailableBalance
mortelli Jan 3, 2020
098cb0d
Merge remote-tracking branch 'origin/master' into swap-unify-variable…
mortelli Jan 6, 2020
ee024f1
swap: refactor Uint256 struct funcs
mortelli Jan 6, 2020
98ff83f
swap: refactor Uint256 struct funcs
mortelli Jan 6, 2020
74f7fac
swap: add first tests for Uint256 type
mortelli Jan 6, 2020
8945021
swap: iterate Uint256 tests
mortelli Jan 6, 2020
043ccaa
swap: iterate Uint256 tests
mortelli Jan 6, 2020
af7619d
swap: add TestCopyUint256 func
mortelli Jan 6, 2020
ed06ee6
swap: add randomUint256 TestUint256Store functions
mortelli Jan 6, 2020
f57314a
swap: remove unneeded marshaling/unmarshaling funcs and tests for Uin…
mortelli Jan 6, 2020
7678775
swap: change return of AvailableBalance in case of error
mortelli Jan 6, 2020
03604ed
swap: remove if statement in encodeForSignature function
mortelli Jan 6, 2020
dacd377
swap: change return of verifyChequeAgainstLast function in case of error
mortelli Jan 6, 2020
2b2de30
swap: minor correction in handleEmitChequeMsg
mortelli Jan 6, 2020
3cf2163
swap: change return of processAndVerifyCheque function in case of error
mortelli Jan 6, 2020
f14b322
swap: iterate Uint256 tests
mortelli Jan 6, 2020
f7380f2
swap: make Uint256 value field private and non-pointer (#2069)
mortelli Jan 8, 2020
0014fe1
Merge remote-tracking branch 'origin/master' into swap-unify-variable…
mortelli Jan 8, 2020
6a469d8
Merge remote-tracking branch 'origin/master' into swap-unify-variable…
mortelli Jan 8, 2020
2025565
swap: fix unintended shared pointer references in NewUint256, Uint64T…
mortelli Jan 8, 2020
d48d980
Merge remote-tracking branch 'origin/master' into swap-unify-variable…
mortelli Jan 9, 2020
b45c506
swap: small change to TestCopyUint256 func
mortelli Jan 9, 2020
810f75f
Merge remote-tracking branch 'origin/master' into swap-unify-variable…
mortelli Jan 13, 2020
5c2ddde
swap: small refactor of Uint256 funcs after merge
mortelli Jan 13, 2020
74043d7
swap, uint256: move Uint256 implementation to new swarm/uint256 packa…
mortelli Jan 15, 2020
3a400df
uint256: call Set on Uint256 UnmarshalJSON and DecodeRLP funcs
mortelli Jan 15, 2020
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
28 changes: 17 additions & 11 deletions swap/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package swap
import (
"encoding/json"
"fmt"
"math/big"

"github.com/ethereum/go-ethereum/p2p/enode"
"github.com/ethereum/go-ethereum/rpc"
Expand All @@ -23,7 +24,7 @@ func (s *Swap) APIs() []rpc.API {
}

type swapAPI interface {
AvailableBalance() (uint64, error)
AvailableBalance() (*Uint256, error)
PeerBalance(peer enode.ID) (int64, error)
Balances() (map[enode.ID]int64, error)
PeerCheques(peer enode.ID) (PeerCheques, error)
Expand Down Expand Up @@ -52,22 +53,22 @@ func NewAPI(s *Swap) *API {
}

// AvailableBalance returns the total balance of the chequebook against which new cheques can be written
func (s *Swap) AvailableBalance() (uint64, error) {
func (s *Swap) AvailableBalance() (*Uint256, error) {
// get the LiquidBalance of the chequebook
liquidBalance, err := s.contract.LiquidBalance(nil)
contractLiquidBalance, err := s.contract.LiquidBalance(nil)
if err != nil {
return 0, err
return nil, err
}

// get all cheques
cheques, err := s.Cheques()
if err != nil {
return 0, err
return nil, err
}

// Compute the total worth of cheques sent and how much of of this is cashed
var sentChequesWorth uint64
var cashedChequesWorth uint64
sentChequesWorth := new(big.Int)
cashedChequesWorth := new(big.Int)
for _, peerCheques := range cheques {
var sentCheque *Cheque
if peerCheques.PendingCheque != nil {
Expand All @@ -77,14 +78,19 @@ func (s *Swap) AvailableBalance() (uint64, error) {
} else {
continue
}
sentChequesWorth += sentCheque.ChequeParams.CumulativePayout
cumulativePayout := sentCheque.ChequeParams.CumulativePayout.Value()
sentChequesWorth.Add(sentChequesWorth, &cumulativePayout)
paidOut, err := s.contract.PaidOut(nil, sentCheque.ChequeParams.Beneficiary)
if err != nil {
return 0, err
return nil, err
}
cashedChequesWorth += paidOut.Uint64()
cashedChequesWorth.Add(cashedChequesWorth, paidOut)
}
return liquidBalance.Uint64() + cashedChequesWorth - sentChequesWorth, nil

totalChequesWorth := new(big.Int).Sub(cashedChequesWorth, sentChequesWorth)
tentativeLiquidBalance := new(big.Int).Add(contractLiquidBalance, totalChequesWorth)

return NewUint256().Set(*tentativeLiquidBalance)
}

// PeerBalance returns the balance for a given peer
Expand Down
24 changes: 13 additions & 11 deletions swap/cheque.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ package swap
import (
"bytes"
"crypto/ecdsa"
"encoding/binary"
"fmt"

"github.com/ethereum/go-ethereum/common"
Expand All @@ -31,7 +30,10 @@ func (cheque *ChequeParams) encodeForSignature() []byte {
cumulativePayoutBytes := make([]byte, 32)
// we need to write the last 8 bytes as we write a uint64 into a 32-byte array
// encoded in BigEndian because EVM uses BigEndian encoding
binary.BigEndian.PutUint64(cumulativePayoutBytes[24:], cheque.CumulativePayout)
cumulativePayout := cheque.CumulativePayout.Value()
chequePayoutBytes := (&cumulativePayout).Bytes()
copy(cumulativePayoutBytes[32-len(chequePayoutBytes):], chequePayoutBytes)

// construct the actual cheque
input := cheque.Contract.Bytes()
input = append(input, cheque.Beneficiary.Bytes()...)
Expand Down Expand Up @@ -94,7 +96,7 @@ func (cheque *Cheque) Equal(other *Cheque) bool {
return false
}

if cheque.CumulativePayout != other.CumulativePayout {
if !cheque.CumulativePayout.Equals(other.CumulativePayout) {
holisticode marked this conversation as resolved.
Show resolved Hide resolved
return false
}

Expand Down Expand Up @@ -130,24 +132,24 @@ 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 uint64) (uint64, error) {
actualAmount := cheque.CumulativePayout
func (cheque *Cheque) verifyChequeAgainstLast(lastCheque *Cheque, expectedAmount *Uint256) (*Uint256, error) {
actualAmount := NewUint256().Copy(cheque.CumulativePayout)

if lastCheque != nil {
if cheque.CumulativePayout <= lastCheque.CumulativePayout {
return 0, fmt.Errorf("wrong cheque parameters: expected cumulative payout larger than %d, was: %d", lastCheque.CumulativePayout, cheque.CumulativePayout)
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)
}

actualAmount -= lastCheque.CumulativePayout
actualAmount.Sub(actualAmount, lastCheque.CumulativePayout)
}

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

return actualAmount, nil
}

func (cheque *Cheque) String() string {
return fmt.Sprintf("Contract: %x Beneficiary: %x CumulativePayout: %d Honey: %d", cheque.Contract, cheque.Beneficiary, cheque.CumulativePayout, cheque.Honey)
return fmt.Sprintf("Contract: %x Beneficiary: %x CumulativePayout: %v Honey: %d", cheque.Contract, cheque.Beneficiary, cheque.CumulativePayout, cheque.Honey)
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd probably print %s, cheque.CumulativePayout.String()

Copy link
Contributor Author

Choose a reason for hiding this comment

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

we shouldn't need to call String(), right? Uint256 implements the Stringer interface, ergo it knows how to represent itself as a string without calling the function explicitly, no?

now: do we use %v or %s? as far as i know there is no difference (in practice) as long as the interface is implemented, so i have no preference here.

Copy link
Contributor

Choose a reason for hiding this comment

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

If it is a String I'd use %s, which is what I actually meant in the first place

}
15 changes: 10 additions & 5 deletions swap/peer.go
Original file line number Diff line number Diff line change
Expand Up @@ -116,12 +116,12 @@ func (p *Peer) setPendingCheque(cheque *Cheque) error {

// getLastSentCumulativePayout returns the cumulative payout of the last sent cheque or 0 if there is none
// the caller is expected to hold p.lock
func (p *Peer) getLastSentCumulativePayout() uint64 {
func (p *Peer) getLastSentCumulativePayout() *Uint256 {
lastCheque := p.getLastSentCheque()
if lastCheque != nil {
return lastCheque.CumulativePayout
}
return 0
return NewUint256()
}

// the caller is expected to hold p.lock
Expand Down Expand Up @@ -162,16 +162,21 @@ func (p *Peer) createCheque() (*Cheque, error) {
// the balance should be negative here, we take the absolute value:
honey := uint64(-p.getBalance())

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

total := p.getLastSentCumulativePayout()
cumulativePayout := p.getLastSentCumulativePayout()
newCumulativePayout, err := NewUint256().Add(cumulativePayout, castedPrice)
if err != nil {
return nil, err
}

cheque = &Cheque{
ChequeParams: ChequeParams{
CumulativePayout: total + amount,
CumulativePayout: newCumulativePayout,
Contract: p.swap.GetParams().ContractAddress,
Beneficiary: p.beneficiary,
},
Expand Down
13 changes: 7 additions & 6 deletions swap/protocol_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -245,9 +245,10 @@ func TestEmitCheque(t *testing.T) {
// gasPrice on testBackend == 1
// estimated gas costs == 50000
// cheque should be sent if the accumulated amount of uncashed cheques is worth more than 100000
balance := uint64(100001)
balance := Uint64ToUint256(100001)
balanceValue := balance.Value()

if err := testDeploy(context.Background(), debitorSwap, big.NewInt(int64(balance))); err != nil {
if err := testDeploy(context.Background(), debitorSwap, &balanceValue); err != nil {
t.Fatal(err)
}

Expand All @@ -260,7 +261,7 @@ func TestEmitCheque(t *testing.T) {

debitor := creditorSwap.getPeer(protocolTester.Nodes[0].ID())
// set balance artificially
if err = debitor.setBalance(int64(balance)); err != nil {
if err = debitor.setBalance(balanceValue.Int64()); err != nil {
t.Fatal(err)
}

Expand All @@ -275,7 +276,7 @@ func TestEmitCheque(t *testing.T) {
Beneficiary: creditorSwap.owner.address,
CumulativePayout: balance,
},
Honey: balance,
Honey: balanceValue.Uint64(),
}
cheque.Signature, err = cheque.Sign(debitorSwap.owner.privateKey)
if err != nil {
Expand Down Expand Up @@ -380,8 +381,8 @@ func TestTriggerPaymentThreshold(t *testing.T) {
t.Fatal("Expected pending cheque")
}

if pending.CumulativePayout != expectedAmount {
t.Fatalf("Expected cheque cumulative payout to be %d, but is %d", expectedAmount, pending.CumulativePayout)
if !pending.CumulativePayout.Equals(Uint64ToUint256(expectedAmount)) {
t.Fatalf("Expected cheque cumulative payout to be %d, but is %v", expectedAmount, pending.CumulativePayout)
}

if pending.Honey != expectedAmount {
Expand Down
18 changes: 9 additions & 9 deletions swap/simulations_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -368,11 +368,11 @@ func TestPingPongChequeSimulation(t *testing.T) {
}

expected := uint64(maxCheques) / 2 * (DefaultPaymentThreshold + 1)
if ch1.CumulativePayout != expected {
t.Fatalf("expected cumulative payout to be %d, but is %d", expected, ch1.CumulativePayout)
if !ch1.CumulativePayout.Equals(Uint64ToUint256(expected)) {
t.Fatalf("expected cumulative payout to be %d, but is %v", expected, ch1.CumulativePayout)
}
if ch2.CumulativePayout != expected {
t.Fatalf("expected cumulative payout to be %d, but is %d", expected, ch2.CumulativePayout)
if !ch2.CumulativePayout.Equals(Uint64ToUint256(expected)) {
t.Fatalf("expected cumulative payout to be %d, but is %v", expected, ch2.CumulativePayout)
}

log.Info("Simulation ended")
Expand Down Expand Up @@ -502,15 +502,15 @@ func TestMultiChequeSimulation(t *testing.T) {
}

// both cheques (at issuer and beneficiary) should have same cumulative value
if cheque1.CumulativePayout != cheque2.CumulativePayout {
t.Fatalf("Expected symmetric cheques payout, but they are not: %d vs %d", cheque1.CumulativePayout, cheque2.CumulativePayout)
if !cheque1.CumulativePayout.Equals(cheque2.CumulativePayout) {
t.Fatalf("Expected symmetric cheques payout, but they are not: %v vs %v", cheque1.CumulativePayout, cheque2.CumulativePayout)
}

// check also the actual expected amount
expectedPayout = uint64(maxCheques) * (DefaultPaymentThreshold + 1)

if cheque2.CumulativePayout != expectedPayout {
t.Fatalf("Expected %d in cumulative payout, got %d", expectedPayout, cheque1.CumulativePayout)
if !cheque2.CumulativePayout.Equals(Uint64ToUint256(expectedPayout)) {
t.Fatalf("Expected %d in cumulative payout, got %v", expectedPayout, cheque1.CumulativePayout)
}

log.Info("Simulation ended")
Expand Down Expand Up @@ -745,7 +745,7 @@ func waitForChequeProcessed(t *testing.T, backend *swapTestBackend, counter metr
p.lock.Lock()
lastPayout := p.getLastSentCumulativePayout()
p.lock.Unlock()
if lastPayout != expectedLastPayout {
if !lastPayout.Equals(Uint64ToUint256(expectedLastPayout)) {
time.Sleep(5 * time.Millisecond)
continue
} else {
Expand Down
34 changes: 27 additions & 7 deletions swap/swap.go
Original file line number Diff line number Diff line change
Expand Up @@ -444,12 +444,31 @@ func (s *Swap) handleEmitChequeMsg(ctx context.Context, p *Peer, msg *EmitCheque
return err
}
transactionCosts := gasPrice.Uint64() * 50000 // cashing a cheque is approximately 50000 gas
castedTransactionCosts := Uint64ToUint256(transactionCosts)
costsMultiplier := Uint64ToUint256(2)

costThreshold, err := NewUint256().Mul(castedTransactionCosts, costsMultiplier)
if err != nil {
return err
}

paidOut, err := otherSwap.PaidOut(nil, cheque.Beneficiary)
if err != nil {
return err
}

castedPaidOut, err := NewUint256().Set(*paidOut)
if err != nil {
return err
}

transactionProfit, err := NewUint256().Sub(cheque.CumulativePayout, castedPaidOut)
if err != nil {
return err
}

// do a payout transaction if we get 2 times the gas costs
if (cheque.CumulativePayout - paidOut.Uint64()) > 2*transactionCosts {
if transactionProfit.Cmp(costThreshold) == 1 {
opts := bind.NewKeyedTransactor(s.owner.privateKey)
opts.Context = ctx
// cash cheque in async, otherwise this blocks here until the TX is mined
Expand Down Expand Up @@ -491,7 +510,8 @@ func (s *Swap) handleConfirmChequeMsg(ctx context.Context, p *Peer, msg *Confirm
// The function cashes the cheque by sending it to the blockchain
func cashCheque(s *Swap, otherSwap contract.Contract, opts *bind.TransactOpts, cheque *Cheque) {
// blocks here, as we are waiting for the transaction to be mined
result, receipt, err := otherSwap.CashChequeBeneficiary(opts, s.GetParams().ContractAddress, big.NewInt(int64(cheque.CumulativePayout)), cheque.Signature)
cumulativePayout := cheque.CumulativePayout.Value()
result, receipt, err := otherSwap.CashChequeBeneficiary(opts, s.GetParams().ContractAddress, &cumulativePayout, cheque.Signature)
if err != nil {
// TODO: do something with the error
// and we actually need to log this error as we are in an async routine; nobody is handling this error for now
Expand All @@ -513,22 +533,22 @@ func cashCheque(s *Swap, otherSwap contract.Contract, opts *bind.TransactOpts, c

// 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
func (s *Swap) processAndVerifyCheque(cheque *Cheque, p *Peer) (uint64, error) {
func (s *Swap) processAndVerifyCheque(cheque *Cheque, p *Peer) (*Uint256, error) {
if err := cheque.verifyChequeProperties(p, s.owner.address); err != nil {
return 0, err
return nil, err
}

lastCheque := p.getLastReceivedCheque()

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

actualAmount, err := cheque.verifyChequeAgainstLast(lastCheque, expectedAmount)
actualAmount, err := cheque.verifyChequeAgainstLast(lastCheque, Uint64ToUint256(expectedAmount))
if err != nil {
return 0, err
return nil, err
}

if err := p.setLastReceivedCheque(cheque); err != nil {
Expand Down
Loading