Skip to content

Commit

Permalink
refactor(tm2): remove pkg/maths in favour of min/max (#1746)
Browse files Browse the repository at this point in the history
I was taking a look at what packages we have in tm2/pkg and realised we
had an entire package to do what are, since go 1.21, builtin functions.

This PR removes tm2/pkg/maths in favour of the now standard and generic
go implementation of the same functionality.
  • Loading branch information
thehowl authored Mar 13, 2024
1 parent 82a36f9 commit 40c6362
Show file tree
Hide file tree
Showing 11 changed files with 33 additions and 74 deletions.
5 changes: 2 additions & 3 deletions tm2/pkg/bft/mempool/clist_mempool.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ import (
"github.com/gnolang/gno/tm2/pkg/clist"
"github.com/gnolang/gno/tm2/pkg/errors"
"github.com/gnolang/gno/tm2/pkg/log"
"github.com/gnolang/gno/tm2/pkg/maths"
osm "github.com/gnolang/gno/tm2/pkg/os"
)

Expand Down Expand Up @@ -461,7 +460,7 @@ func (mem *CListMempool) ReapMaxBytesMaxGas(maxDataBytes, maxGas int64) types.Tx
var totalGas int64
// TODO: we will get a performance boost if we have a good estimate of avg
// size per tx, and set the initial capacity based off of that.
// txs := make([]types.Tx, 0, maths.MinInt(mem.txs.Len(), max/mem.avgTxSize))
// txs := make([]types.Tx, 0, min(mem.txs.Len(), max/mem.avgTxSize))
txs := make([]types.Tx, 0, mem.txs.Len())
for e := mem.txs.Front(); e != nil; e = e.Next() {
memTx := e.Value.(*mempoolTx)
Expand Down Expand Up @@ -497,7 +496,7 @@ func (mem *CListMempool) ReapMaxTxs(max int) types.Txs {
time.Sleep(time.Millisecond * 10)
}

txs := make([]types.Tx, 0, maths.MinInt(mem.txs.Len(), max))
txs := make([]types.Tx, 0, min(mem.txs.Len(), max))
for e := mem.txs.Front(); e != nil && len(txs) <= max; e = e.Next() {
memTx := e.Value.(*mempoolTx)
txs = append(txs, memTx.tx)
Expand Down
3 changes: 1 addition & 2 deletions tm2/pkg/bft/rpc/client/rpc_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ import (
rpcclient "github.com/gnolang/gno/tm2/pkg/bft/rpc/lib/client"
rpctest "github.com/gnolang/gno/tm2/pkg/bft/rpc/test"
"github.com/gnolang/gno/tm2/pkg/bft/types"
"github.com/gnolang/gno/tm2/pkg/maths"
)

func getHTTPClient() *client.HTTP {
Expand Down Expand Up @@ -519,7 +518,7 @@ func testBatchedJSONRPCCalls(t *testing.T, c *client.HTTP) {
bresult2, ok := bresults[1].(*ctypes.ResultBroadcastTxCommit)
require.True(t, ok)
require.Equal(t, *bresult2, *r2)
apph := maths.MaxInt64(bresult1.Height, bresult2.Height) + 1
apph := max(bresult1.Height, bresult2.Height) + 1

client.WaitForHeight(c, apph, nil)

Expand Down
37 changes: 18 additions & 19 deletions tm2/pkg/bft/rpc/core/blocks.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import (
rpctypes "github.com/gnolang/gno/tm2/pkg/bft/rpc/lib/types"
sm "github.com/gnolang/gno/tm2/pkg/bft/state"
"github.com/gnolang/gno/tm2/pkg/bft/types"
"github.com/gnolang/gno/tm2/pkg/maths"
)

// Get block headers for minHeight <= height <= maxHeight.
Expand Down Expand Up @@ -95,35 +94,35 @@ func BlockchainInfo(ctx *rpctypes.Context, minHeight, maxHeight int64) (*ctypes.
}, nil
}

// error if either min or max are negative or min < max
// if 0, use 1 for min, latest block height for max
// enforce limit.
// error if min > max
func filterMinMax(height, min, max, limit int64) (int64, int64, error) {
// error if either low or high are negative or low > high
// if low is 0 it defaults to 1, if high is 0 it defaults to height (block height).
// limit sets the maximum amounts of values included within [low,high] (inclusive),
// increasing low as necessary.
func filterMinMax(height, low, high, limit int64) (int64, int64, error) {
// filter negatives
if min < 0 || max < 0 {
return min, max, fmt.Errorf("heights must be non-negative")
if low < 0 || high < 0 {
return low, high, fmt.Errorf("heights must be non-negative")
}

// adjust for default values
if min == 0 {
min = 1
if low == 0 {
low = 1
}
if max == 0 {
max = height
if high == 0 {
high = height
}

// limit max to the height
max = maths.MinInt64(height, max)
// limit high to the height
high = min(height, high)

// limit min to within `limit` of max
// limit low to within `limit` of max
// so the total number of blocks returned will be `limit`
min = maths.MaxInt64(min, max-limit+1)
low = max(low, high-limit+1)

if min > max {
return min, max, fmt.Errorf("min height %d can't be greater than max height %d", min, max)
if low > high {
return low, high, fmt.Errorf("min height %d can't be greater than max height %d", low, high)
}
return min, max, nil
return low, high, nil
}

// Get block at a given height.
Expand Down
3 changes: 1 addition & 2 deletions tm2/pkg/bft/state/store.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import (
abci "github.com/gnolang/gno/tm2/pkg/bft/abci/types"
"github.com/gnolang/gno/tm2/pkg/bft/types"
dbm "github.com/gnolang/gno/tm2/pkg/db"
"github.com/gnolang/gno/tm2/pkg/maths"
osm "github.com/gnolang/gno/tm2/pkg/os"
)

Expand Down Expand Up @@ -221,7 +220,7 @@ func LoadValidators(db dbm.DB, height int64) (*types.ValidatorSet, error) {

func lastStoredHeightFor(height, lastHeightChanged int64) int64 {
checkpointHeight := height - height%valSetCheckpointInterval
return maths.MaxInt64(checkpointHeight, lastHeightChanged)
return max(checkpointHeight, lastHeightChanged)
}

// CONTRACT: Returned ValidatorsInfo can be mutated.
Expand Down
3 changes: 1 addition & 2 deletions tm2/pkg/bft/types/block.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ import (
"github.com/gnolang/gno/tm2/pkg/crypto/merkle"
"github.com/gnolang/gno/tm2/pkg/crypto/tmhash"
"github.com/gnolang/gno/tm2/pkg/errors"
"github.com/gnolang/gno/tm2/pkg/maths"
)

// Block defines the atomic unit of a Tendermint blockchain.
Expand Down Expand Up @@ -718,7 +717,7 @@ func (data *Data) StringIndented(indent string) string {
if data == nil {
return "nil-Data"
}
txStrings := make([]string, maths.MinInt(len(data.Txs), 21))
txStrings := make([]string, min(len(data.Txs), 21))
for i, tx := range data.Txs {
if i == 20 {
txStrings[i] = fmt.Sprintf("... (%v total)", len(data.Txs))
Expand Down
3 changes: 1 addition & 2 deletions tm2/pkg/bft/types/part_set.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import (
"github.com/gnolang/gno/tm2/pkg/bitarray"
"github.com/gnolang/gno/tm2/pkg/crypto/merkle"
"github.com/gnolang/gno/tm2/pkg/errors"
"github.com/gnolang/gno/tm2/pkg/maths"
)

var (
Expand Down Expand Up @@ -107,7 +106,7 @@ func NewPartSetFromData(data []byte, partSize int) *PartSet {
for i := 0; i < total; i++ {
part := &Part{
Index: i,
Bytes: data[i*partSize : maths.MinInt(len(data), (i+1)*partSize)],
Bytes: data[i*partSize : min(len(data), (i+1)*partSize)],
}
parts[i] = part
partsBytes[i] = part.Bytes
Expand Down
3 changes: 1 addition & 2 deletions tm2/pkg/bft/types/signable.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,12 @@ package types

import (
"github.com/gnolang/gno/tm2/pkg/crypto/ed25519"
"github.com/gnolang/gno/tm2/pkg/maths"
)

// MaxSignatureSize is a maximum allowed signature size for the Proposal
// and Vote.
// XXX: secp256k1 does not have Size nor MaxSize defined.
var MaxSignatureSize = maths.MaxInt(ed25519.SignatureSize, 64)
const MaxSignatureSize = max(ed25519.SignatureSize, 64)

// Signable is an interface for all signable things.
// It typically removes signatures before serializing.
Expand Down
3 changes: 1 addition & 2 deletions tm2/pkg/bft/types/validator_set_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ import (
tmtime "github.com/gnolang/gno/tm2/pkg/bft/types/time"
"github.com/gnolang/gno/tm2/pkg/crypto"
"github.com/gnolang/gno/tm2/pkg/crypto/mock"
"github.com/gnolang/gno/tm2/pkg/maths"
"github.com/gnolang/gno/tm2/pkg/random"
)

Expand Down Expand Up @@ -1052,7 +1051,7 @@ func TestValSetUpdatesOrderIndependenceTestsExecute(t *testing.T) {

// perform at most 20 permutations on the updates and call UpdateWithChangeSet()
n := len(tt.updateVals)
maxNumPerms := maths.MinInt(20, n*n)
maxNumPerms := min(20, n*n)
for j := 0; j < maxNumPerms; j++ {
// create a copy of original set and apply a random permutation of updates
valSetCopy := valSet.Copy()
Expand Down
9 changes: 4 additions & 5 deletions tm2/pkg/bitarray/bit_array.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import (
"strings"
"sync"

"github.com/gnolang/gno/tm2/pkg/maths"
"github.com/gnolang/gno/tm2/pkg/random"
)

Expand Down Expand Up @@ -122,8 +121,8 @@ func (bA *BitArray) Or(o *BitArray) *BitArray {
}
bA.mtx.Lock()
o.mtx.Lock()
c := bA.copyBits(maths.MaxInt(bA.Bits, o.Bits))
smaller := maths.MinInt(len(bA.Elems), len(o.Elems))
c := bA.copyBits(max(bA.Bits, o.Bits))
smaller := min(len(bA.Elems), len(o.Elems))
for i := 0; i < smaller; i++ {
c.Elems[i] |= o.Elems[i]
}
Expand All @@ -149,7 +148,7 @@ func (bA *BitArray) And(o *BitArray) *BitArray {
}

func (bA *BitArray) and(o *BitArray) *BitArray {
c := bA.copyBits(maths.MinInt(bA.Bits, o.Bits))
c := bA.copyBits(min(bA.Bits, o.Bits))
for i := 0; i < len(c.Elems); i++ {
c.Elems[i] &= o.Elems[i]
}
Expand Down Expand Up @@ -191,7 +190,7 @@ func (bA *BitArray) Sub(o *BitArray) *BitArray {
// If o is longer, those bits are ignored.
// If bA is longer, then skipping those iterations is equivalent
// to right padding with 0's
smaller := maths.MinInt(len(bA.Elems), len(o.Elems))
smaller := min(len(bA.Elems), len(o.Elems))
for i := 0; i < smaller; i++ {
// &^ is and not in golang
c.Elems[i] &^= o.Elems[i]
Expand Down
31 changes: 0 additions & 31 deletions tm2/pkg/maths/math.go

This file was deleted.

7 changes: 3 additions & 4 deletions tm2/pkg/p2p/conn/connection.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ import (
"github.com/gnolang/gno/tm2/pkg/amino"
"github.com/gnolang/gno/tm2/pkg/errors"
"github.com/gnolang/gno/tm2/pkg/flow"
"github.com/gnolang/gno/tm2/pkg/maths"
"github.com/gnolang/gno/tm2/pkg/service"
"github.com/gnolang/gno/tm2/pkg/timer"
)
Expand Down Expand Up @@ -550,7 +549,7 @@ FOR_LOOP:
// Peek into bufConnReader for debugging
/*
if numBytes := c.bufConnReader.Buffered(); numBytes > 0 {
bz, err := c.bufConnReader.Peek(maths.MinInt(numBytes, 100))
bz, err := c.bufConnReader.Peek(min(numBytes, 100))
if err == nil {
// return
} else {
Expand Down Expand Up @@ -809,14 +808,14 @@ func (ch *Channel) nextPacketMsg() PacketMsg {
packet := PacketMsg{}
packet.ChannelID = ch.desc.ID
maxSize := ch.maxPacketMsgPayloadSize
packet.Bytes = ch.sending[:maths.MinInt(maxSize, len(ch.sending))]
packet.Bytes = ch.sending[:min(maxSize, len(ch.sending))]
if len(ch.sending) <= maxSize {
packet.EOF = byte(0x01)
ch.sending = nil
atomic.AddInt32(&ch.sendQueueSize, -1) // decrement sendQueueSize
} else {
packet.EOF = byte(0x00)
ch.sending = ch.sending[maths.MinInt(maxSize, len(ch.sending)):]
ch.sending = ch.sending[min(maxSize, len(ch.sending)):]
}
return packet
}
Expand Down

0 comments on commit 40c6362

Please sign in to comment.