From 3b4759b7f8219865c0803fc838808e966e940e16 Mon Sep 17 00:00:00 2001 From: Morgan Date: Wed, 13 Mar 2024 15:34:07 +0100 Subject: [PATCH] refactor(tm2): remove pkg/maths in favour of min/max (#1746) 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. --- tm2/pkg/bft/mempool/clist_mempool.go | 5 ++-- tm2/pkg/bft/rpc/client/rpc_test.go | 3 +- tm2/pkg/bft/rpc/core/blocks.go | 37 ++++++++++++------------- tm2/pkg/bft/state/store.go | 3 +- tm2/pkg/bft/types/block.go | 3 +- tm2/pkg/bft/types/part_set.go | 3 +- tm2/pkg/bft/types/signable.go | 3 +- tm2/pkg/bft/types/validator_set_test.go | 3 +- tm2/pkg/bitarray/bit_array.go | 9 +++--- tm2/pkg/maths/math.go | 31 --------------------- tm2/pkg/p2p/conn/connection.go | 7 ++--- 11 files changed, 33 insertions(+), 74 deletions(-) delete mode 100644 tm2/pkg/maths/math.go diff --git a/tm2/pkg/bft/mempool/clist_mempool.go b/tm2/pkg/bft/mempool/clist_mempool.go index 47f3419222b..38c24866892 100644 --- a/tm2/pkg/bft/mempool/clist_mempool.go +++ b/tm2/pkg/bft/mempool/clist_mempool.go @@ -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" ) @@ -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) @@ -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) diff --git a/tm2/pkg/bft/rpc/client/rpc_test.go b/tm2/pkg/bft/rpc/client/rpc_test.go index 78cfdc7e4ee..f1d908810fc 100644 --- a/tm2/pkg/bft/rpc/client/rpc_test.go +++ b/tm2/pkg/bft/rpc/client/rpc_test.go @@ -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 { @@ -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) diff --git a/tm2/pkg/bft/rpc/core/blocks.go b/tm2/pkg/bft/rpc/core/blocks.go index 7603a1d5c1b..06bb3de1174 100644 --- a/tm2/pkg/bft/rpc/core/blocks.go +++ b/tm2/pkg/bft/rpc/core/blocks.go @@ -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. @@ -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. diff --git a/tm2/pkg/bft/state/store.go b/tm2/pkg/bft/state/store.go index 0f54ec547d2..3b1e3d015d7 100644 --- a/tm2/pkg/bft/state/store.go +++ b/tm2/pkg/bft/state/store.go @@ -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" ) @@ -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. diff --git a/tm2/pkg/bft/types/block.go b/tm2/pkg/bft/types/block.go index b9ca0e405df..a8501775c4b 100644 --- a/tm2/pkg/bft/types/block.go +++ b/tm2/pkg/bft/types/block.go @@ -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. @@ -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)) diff --git a/tm2/pkg/bft/types/part_set.go b/tm2/pkg/bft/types/part_set.go index a5cfe1d59ee..f8dc69d51d7 100644 --- a/tm2/pkg/bft/types/part_set.go +++ b/tm2/pkg/bft/types/part_set.go @@ -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 ( @@ -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 diff --git a/tm2/pkg/bft/types/signable.go b/tm2/pkg/bft/types/signable.go index d0c1fed1381..f71b0798bef 100644 --- a/tm2/pkg/bft/types/signable.go +++ b/tm2/pkg/bft/types/signable.go @@ -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. diff --git a/tm2/pkg/bft/types/validator_set_test.go b/tm2/pkg/bft/types/validator_set_test.go index 0fafb6fca9e..e543104b15d 100644 --- a/tm2/pkg/bft/types/validator_set_test.go +++ b/tm2/pkg/bft/types/validator_set_test.go @@ -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" ) @@ -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() diff --git a/tm2/pkg/bitarray/bit_array.go b/tm2/pkg/bitarray/bit_array.go index 320b39e0c6a..2f7b184b938 100644 --- a/tm2/pkg/bitarray/bit_array.go +++ b/tm2/pkg/bitarray/bit_array.go @@ -7,7 +7,6 @@ import ( "strings" "sync" - "github.com/gnolang/gno/tm2/pkg/maths" "github.com/gnolang/gno/tm2/pkg/random" ) @@ -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] } @@ -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] } @@ -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] diff --git a/tm2/pkg/maths/math.go b/tm2/pkg/maths/math.go deleted file mode 100644 index e31ef415930..00000000000 --- a/tm2/pkg/maths/math.go +++ /dev/null @@ -1,31 +0,0 @@ -package maths - -func MaxInt64(a, b int64) int64 { - if a > b { - return a - } - return b -} - -func MaxInt(a, b int) int { - if a > b { - return a - } - return b -} - -//----------------------------------------------------------------------------- - -func MinInt64(a, b int64) int64 { - if a < b { - return a - } - return b -} - -func MinInt(a, b int) int { - if a < b { - return a - } - return b -} diff --git a/tm2/pkg/p2p/conn/connection.go b/tm2/pkg/p2p/conn/connection.go index 41b6a06629e..6b7400600d3 100644 --- a/tm2/pkg/p2p/conn/connection.go +++ b/tm2/pkg/p2p/conn/connection.go @@ -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" ) @@ -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 { @@ -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 }