Skip to content

Commit

Permalink
Release v1.2.5 (#1479)
Browse files Browse the repository at this point in the history
* Cache currency comparisons when building block
* Fix getExchange for goldCurrency
* Adds unit tests for currency comparator
* Fix imports
* Decrease default tx pool size from 4096 to 2048
* Update version to 1.2.5-stable

Co-authored-by: Mariano Cortesi <[email protected]>
  • Loading branch information
jcortejoso and mcortesi authored Apr 4, 2021
1 parent 8ae8f03 commit cc43ad2
Show file tree
Hide file tree
Showing 8 changed files with 279 additions and 21 deletions.
12 changes: 6 additions & 6 deletions consensus/istanbul/core/roundstate_db_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@ func TestRSDBRoundStateDB(t *testing.T) {
pubkey2 := blscrypto.SerializedPublicKey{3, 1, 4}
dummyRoundState := func() RoundState {
valSet := validator.NewSet([]istanbul.ValidatorData{
{Address: common.BytesToAddress([]byte(string(2))), BLSPublicKey: pubkey1},
{Address: common.BytesToAddress([]byte(string(4))), BLSPublicKey: pubkey2},
{Address: common.BytesToAddress([]byte(string(rune(2)))), BLSPublicKey: pubkey1},
{Address: common.BytesToAddress([]byte(string(rune(4)))), BLSPublicKey: pubkey2},
})
return newRoundState(newView(2, 1), valSet, valSet.GetByIndex(0))
}
Expand Down Expand Up @@ -60,8 +60,8 @@ func TestRSDBDeleteEntriesOlderThan(t *testing.T) {
pubkey2 := blscrypto.SerializedPublicKey{3, 1, 4}
createRoundState := func(view *istanbul.View) RoundState {
valSet := validator.NewSet([]istanbul.ValidatorData{
{Address: common.BytesToAddress([]byte(string(2))), BLSPublicKey: pubkey1},
{Address: common.BytesToAddress([]byte(string(4))), BLSPublicKey: pubkey2},
{Address: common.BytesToAddress([]byte(string(rune(2)))), BLSPublicKey: pubkey1},
{Address: common.BytesToAddress([]byte(string(rune(4)))), BLSPublicKey: pubkey2},
})
return newRoundState(view, valSet, valSet.GetByIndex(0))
}
Expand Down Expand Up @@ -134,8 +134,8 @@ func TestRSDBGetOldestValidView(t *testing.T) {
pubkey1 := blscrypto.SerializedPublicKey{1, 2, 3}
pubkey2 := blscrypto.SerializedPublicKey{3, 1, 4}
valSet := validator.NewSet([]istanbul.ValidatorData{
{Address: common.BytesToAddress([]byte(string(2))), BLSPublicKey: pubkey1},
{Address: common.BytesToAddress([]byte(string(4))), BLSPublicKey: pubkey2},
{Address: common.BytesToAddress([]byte(string(rune(2)))), BLSPublicKey: pubkey1},
{Address: common.BytesToAddress([]byte(string(rune(4)))), BLSPublicKey: pubkey2},
})
sequencesToSave := uint64(100)
runTestCase := func(name string, viewToStore, expectedView *istanbul.View) {
Expand Down
16 changes: 8 additions & 8 deletions consensus/istanbul/validator/default_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ func testAddAndRemoveValidator(t *testing.T) {
if !valSet.AddValidators(
[]istanbul.ValidatorData{
{
Address: common.BytesToAddress([]byte(string(3))),
Address: common.BytesToAddress([]byte(string(rune(3)))),
BLSPublicKey: blscrypto.SerializedPublicKey{},
},
},
Expand All @@ -125,7 +125,7 @@ func testAddAndRemoveValidator(t *testing.T) {
if valSet.AddValidators(
[]istanbul.ValidatorData{
{
Address: common.BytesToAddress([]byte(string(3))),
Address: common.BytesToAddress([]byte(string(rune(3)))),
BLSPublicKey: blscrypto.SerializedPublicKey{},
},
},
Expand All @@ -135,11 +135,11 @@ func testAddAndRemoveValidator(t *testing.T) {
valSet.AddValidators(
[]istanbul.ValidatorData{
{
Address: common.BytesToAddress([]byte(string(2))),
Address: common.BytesToAddress([]byte(string(rune(2)))),
BLSPublicKey: blscrypto.SerializedPublicKey{},
},
{
Address: common.BytesToAddress([]byte(string(1))),
Address: common.BytesToAddress([]byte(string(rune(1)))),
BLSPublicKey: blscrypto.SerializedPublicKey{},
},
},
Expand All @@ -150,7 +150,7 @@ func testAddAndRemoveValidator(t *testing.T) {

expectedOrder := []int{3, 2, 1}
for i, v := range valSet.List() {
expected := common.BytesToAddress([]byte(string(expectedOrder[i])))
expected := common.BytesToAddress([]byte(string(rune(expectedOrder[i]))))
if v.Address() != expected {
t.Errorf("the order of validators is wrong: have %v, want %v", v.Address().Hex(), expected.Hex())
}
Expand Down Expand Up @@ -215,7 +215,7 @@ func testQuorumSizes(t *testing.T) {

func TestValidatorRLPEncoding(t *testing.T) {

val := New(common.BytesToAddress([]byte(string(2))), blscrypto.SerializedPublicKey{1, 2, 3})
val := New(common.BytesToAddress([]byte(string(rune(2)))), blscrypto.SerializedPublicKey{1, 2, 3})

rawVal, err := rlp.EncodeToBytes(val)
if err != nil {
Expand All @@ -235,8 +235,8 @@ func TestValidatorRLPEncoding(t *testing.T) {
func TestValidatorSetRLPEncoding(t *testing.T) {

valSet := NewSet([]istanbul.ValidatorData{
{Address: common.BytesToAddress([]byte(string(2))), BLSPublicKey: blscrypto.SerializedPublicKey{1, 2, 3}},
{Address: common.BytesToAddress([]byte(string(4))), BLSPublicKey: blscrypto.SerializedPublicKey{3, 1, 4}},
{Address: common.BytesToAddress([]byte(string(rune(2)))), BLSPublicKey: blscrypto.SerializedPublicKey{1, 2, 3}},
{Address: common.BytesToAddress([]byte(string(rune(4)))), BLSPublicKey: blscrypto.SerializedPublicKey{3, 1, 4}},
})

rawVal, err := rlp.EncodeToBytes(valSet)
Expand Down
67 changes: 67 additions & 0 deletions contract_comm/currency/currency.go
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,73 @@ func Convert(val *big.Int, currencyFrom *common.Address, currencyTo *common.Addr
return new(big.Int).Div(numerator, denominator), nil
}

type CurrencyComparator struct {
exchangeRates map[common.Address]*exchangeRate
_getExchangeRate func(*common.Address) (*exchangeRate, error)
}

func NewComparator() *CurrencyComparator {
return newComparator(getExchangeRate)
}

func newComparator(_getExchangeRate func(*common.Address) (*exchangeRate, error)) *CurrencyComparator {
return &CurrencyComparator{
exchangeRates: make(map[common.Address]*exchangeRate),
_getExchangeRate: _getExchangeRate,
}
}

func (cc *CurrencyComparator) getExchangeRate(currency *common.Address) (*exchangeRate, error) {
if currency == nil {
return &exchangeRate{cgExchangeRateNum, cgExchangeRateDen}, nil
}

val, ok := cc.exchangeRates[*currency]
if ok {
return val, nil
}

val, err := cc._getExchangeRate(currency)
if err != nil {
return nil, err
}

cc.exchangeRates[*currency] = val

return val, nil
}

func (cc *CurrencyComparator) Cmp(val1 *big.Int, currency1 *common.Address, val2 *big.Int, currency2 *common.Address) int {
// Short circuit if the fee currency is the same. nil currency => native currency
if (currency1 == nil && currency2 == nil) || (currency1 != nil && currency2 != nil && *currency1 == *currency2) {
return val1.Cmp(val2)
}

exchangeRate1, err1 := cc.getExchangeRate(currency1)
exchangeRate2, err2 := cc.getExchangeRate(currency2)

if err1 != nil || err2 != nil {
currency1Output := "nil"
if currency1 != nil {
currency1Output = currency1.Hex()
}
currency2Output := "nil"
if currency2 != nil {
currency2Output = currency2.Hex()
}
log.Warn("Error in retrieving exchange rate. Will do comparison of two values without exchange rate conversion.", "currency1", currency1Output, "err1", err1, "currency2", currency2Output, "err2", err2)
return val1.Cmp(val2)
}

// Below code block is basically evaluating this comparison:
// val1 * exchangeRate1.Denominator/exchangeRate1.Numerator < val2 * exchangeRate2.Denominator/exchangeRate2.Numerator
// It will transform that comparison to this, to remove having to deal with fractional values.
// val1 * exchangeRate1.Denominator * exchangeRate2.Numerator < val2 * exchangeRate2.Denominator * exchangeRate1.Numerator
leftSide := new(big.Int).Mul(val1, new(big.Int).Mul(exchangeRate1.Denominator, exchangeRate2.Numerator))
rightSide := new(big.Int).Mul(val2, new(big.Int).Mul(exchangeRate2.Denominator, exchangeRate1.Numerator))
return leftSide.Cmp(rightSide)
}

func Cmp(val1 *big.Int, currency1 *common.Address, val2 *big.Int, currency2 *common.Address) int {
if currency1 == currency2 {
return val1.Cmp(val2)
Expand Down
184 changes: 184 additions & 0 deletions contract_comm/currency/currency_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,184 @@
package currency

import (
"errors"
"testing"

"github.com/ethereum/go-ethereum/common"
. "github.com/onsi/gomega"
)

type getExchangeRateMock struct {
calls []*common.Address
returns []struct {
rate *exchangeRate
err error
}
returnIdx int
}

func (m *getExchangeRateMock) totalCalls() int {
return len(m.calls)
}

func (m *getExchangeRateMock) getExchangeRate(currency *common.Address) (*exchangeRate, error) {
m.calls = append(m.calls, currency)

if len(m.returns) <= m.returnIdx {
return nil, errors.New("mock: missing return info")
}

ret := m.returns[m.returnIdx]
m.returnIdx++
return ret.rate, ret.err
}

func (m *getExchangeRateMock) nextReturn(rate *exchangeRate, err error) {
m.returns = append(m.returns, struct {
rate *exchangeRate
err error
}{
rate, err,
})
}

func TestCurrencyComparator(t *testing.T) {
twoToOne := exchangeRate{
Numerator: common.Big1, Denominator: common.Big2,
}
oneToTwo := exchangeRate{
Numerator: common.Big2, Denominator: common.Big1,
}

t.Run("should not call getExchange rate if both currencies are gold", func(t *testing.T) {
g := NewGomegaWithT(t)
mock := getExchangeRateMock{}
comparator := newComparator(mock.getExchangeRate)

g.Expect(comparator.Cmp(common.Big1, nil, common.Big2, nil)).To(Equal(-1))
// no call to getExchange Rate
g.Expect(mock.totalCalls()).To(BeZero())
})

t.Run("should not call getExchange rate if both currencies are the same", func(t *testing.T) {
g := NewGomegaWithT(t)
mock := getExchangeRateMock{}
comparator := newComparator(mock.getExchangeRate)

g.Expect(comparator.Cmp(common.Big1, &common.Address{12}, common.Big2, &common.Address{12})).To(Equal(-1))
// no call to getExchange Rate
g.Expect(mock.totalCalls()).To(BeZero())
})

t.Run("should not call getExchange rate on goldToken currency", func(t *testing.T) {
g := NewGomegaWithT(t)

mock := getExchangeRateMock{}

mock.nextReturn(&twoToOne, nil)

comparator := newComparator(mock.getExchangeRate)

g.Expect(comparator.Cmp(common.Big1, nil, common.Big1, &common.Address{12})).To(Equal(-1))
// call to the exchange rate only for non goldToken currency
g.Expect(mock.totalCalls()).To(Equal(1))
g.Expect(*mock.calls[0]).To(Equal(common.Address{12}))
})

t.Run("should use returned exchange rate", func(t *testing.T) {
g := NewGomegaWithT(t)

mock := getExchangeRateMock{}
comparator := newComparator(mock.getExchangeRate)

// case 1: 2 gold = 1 usd
// then 1 gold < 1 usd
mock.nextReturn(&twoToOne, nil)
g.Expect(comparator.Cmp(common.Big1, nil, common.Big1, &common.Address{10})).To(Equal(-1))

// case 2: 1 gold = 2 usd
// then 1 gold > 1 usd
mock.nextReturn(&oneToTwo, nil)
g.Expect(comparator.Cmp(common.Big1, nil, common.Big1, &common.Address{20})).To(Equal(1))

// case 3: 1 gold = 2 usd && 1 gold = 2 eur
// then 2 eur > 1 usd
mock.nextReturn(&oneToTwo, nil)
mock.nextReturn(&oneToTwo, nil)
g.Expect(comparator.Cmp(common.Big2, &common.Address{30}, common.Big1, &common.Address{40})).To(Equal(1))
})

t.Run("should work with zero values", func(t *testing.T) {
g := NewGomegaWithT(t)

mock := getExchangeRateMock{}
comparator := newComparator(mock.getExchangeRate)

// case 1: both values == 0
g.Expect(comparator.Cmp(common.Big0, nil, common.Big0, nil)).To(Equal(0))

// case 2: first value == 0
g.Expect(comparator.Cmp(common.Big0, nil, common.Big1, nil)).To(Equal(-1))

// case 3: second value == 0
g.Expect(comparator.Cmp(common.Big1, nil, common.Big0, nil)).To(Equal(1))
})

t.Run("should compare value if first get exchange rate fails", func(t *testing.T) {
g := NewGomegaWithT(t)

mock := getExchangeRateMock{}
mock.nextReturn(&twoToOne, nil)
mock.nextReturn(nil, errors.New("boom!"))

comparator := newComparator(mock.getExchangeRate)
g.Expect(comparator.Cmp(common.Big2, &common.Address{30}, common.Big1, &common.Address{12})).To(Equal(1))
})

t.Run("should compare value if second get exchange rate fails", func(t *testing.T) {
g := NewGomegaWithT(t)

mock := getExchangeRateMock{}
mock.nextReturn(nil, errors.New("boom!"))
mock.nextReturn(&twoToOne, nil)

comparator := newComparator(mock.getExchangeRate)
g.Expect(comparator.Cmp(common.Big2, &common.Address{30}, common.Big1, &common.Address{12})).To(Equal(1))
})

t.Run("should cache exchange rate on subsequent calls", func(t *testing.T) {
g := NewGomegaWithT(t)

mock := getExchangeRateMock{}
mock.nextReturn(&twoToOne, nil)
mock.nextReturn(&oneToTwo, nil)

comparator := newComparator(mock.getExchangeRate)

for i := 0; i < 10; i++ {
g.Expect(comparator.Cmp(common.Big1, &common.Address{30}, common.Big1, &common.Address{12})).To(Equal(1))
}

// call to the exchange rate only for non goldToken currency
g.Expect(mock.totalCalls()).To(Equal(2))
g.Expect(*mock.calls[0]).To(Equal(common.Address{30}))
g.Expect(*mock.calls[1]).To(Equal(common.Address{12}))
})

t.Run("should NOT cache exchange rate on errors", func(t *testing.T) {
g := NewGomegaWithT(t)

mock := getExchangeRateMock{}
// default return is an error

comparator := newComparator(mock.getExchangeRate)

for i := 0; i < 10; i++ {
g.Expect(comparator.Cmp(common.Big1, &common.Address{30}, common.Big1, &common.Address{12})).To(Equal(0))
}

// expect 10 call for address{30} and 10 for address{12}
g.Expect(mock.totalCalls()).To(Equal(20))
})

}
2 changes: 1 addition & 1 deletion core/tx_pool.go
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ var DefaultTxPoolConfig = TxPoolConfig{
PriceBump: 10,

AccountSlots: 16,
GlobalSlots: 4096,
GlobalSlots: 2048,
AccountQueue: 64,
GlobalQueue: 1024,

Expand Down
1 change: 1 addition & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ require (
github.com/naoina/go-stringutil v0.1.0 // indirect
github.com/naoina/toml v0.1.2-0.20170918210437-9fafd6967416
github.com/olekukonko/tablewriter v0.0.2-0.20190409134802-7e037d187b0c
github.com/onsi/gomega v1.4.3
github.com/pborman/uuid v0.0.0-20170112150404-1b00554d8222
github.com/peterh/liner v1.1.1-0.20190123174540-a2c9a5303de7
github.com/prometheus/tsdb v0.6.2-0.20190402121629-4f204dcbc150
Expand Down
Loading

0 comments on commit cc43ad2

Please sign in to comment.